From 9b261f52f074fcc11fd705dae63084364c4f7adf Mon Sep 17 00:00:00 2001
From: zeripath <art27@cantab.net>
Date: Sun, 7 Mar 2021 08:12:43 +0000
Subject: Add SameSite setting for cookies (#14900)

Add SameSite setting for cookies and rationalise the cookie setting code. Switches SameSite to Lax by default.

There is a possible future extension of differentiating which cookies could be set at Strict by default but that is for a future PR.

Fix #5583

Signed-off-by: Andrew Thornton <art27@cantab.net>
---
 modules/auth/sso/sso.go          |  5 ++---
 modules/context/auth.go          |  7 +++---
 modules/context/context.go       | 48 +++++++++++++++++++++++++++++++++-------
 modules/context/csrf.go          | 33 ++++++++++++++++++++++-----
 modules/setting/session.go       | 13 +++++++++++
 modules/web/middleware/cookie.go | 41 ++++++++++++++++++++++++++++++++++
 modules/web/middleware/locale.go | 25 ++++++++++++++++++++-
 7 files changed, 151 insertions(+), 21 deletions(-)

(limited to 'modules')

diff --git a/modules/auth/sso/sso.go b/modules/auth/sso/sso.go
index 437bf3af7a..e670f1a8a7 100644
--- a/modules/auth/sso/sso.go
+++ b/modules/auth/sso/sso.go
@@ -13,7 +13,6 @@ import (
 
 	"code.gitea.io/gitea/models"
 	"code.gitea.io/gitea/modules/log"
-	"code.gitea.io/gitea/modules/setting"
 	"code.gitea.io/gitea/modules/web/middleware"
 )
 
@@ -129,8 +128,8 @@ func handleSignIn(resp http.ResponseWriter, req *http.Request, sess SessionStore
 		}
 	}
 
-	middleware.SetCookie(resp, "lang", user.Language, nil, setting.AppSubURL, setting.SessionConfig.Domain, setting.SessionConfig.Secure, true)
+	middleware.SetLocaleCookie(resp, user.Language, 0)
 
 	// Clear whatever CSRF has right now, force to generate a new one
-	middleware.SetCookie(resp, setting.CSRFCookieName, "", -1, setting.AppSubURL, setting.SessionConfig.Domain, setting.SessionConfig.Secure, true)
+	middleware.DeleteCSRFCookie(resp)
 }
diff --git a/modules/context/auth.go b/modules/context/auth.go
index 8be6ed1907..3b4d7fc595 100644
--- a/modules/context/auth.go
+++ b/modules/context/auth.go
@@ -9,6 +9,7 @@ import (
 	"code.gitea.io/gitea/models"
 	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/setting"
+	"code.gitea.io/gitea/modules/web/middleware"
 )
 
 // ToggleOptions contains required or check options
@@ -41,7 +42,7 @@ func Toggle(options *ToggleOptions) func(ctx *Context) {
 					ctx.Data["Title"] = ctx.Tr("auth.must_change_password")
 					ctx.Data["ChangePasscodeLink"] = setting.AppSubURL + "/user/change_password"
 					if ctx.Req.URL.Path != "/user/events" {
-						ctx.SetCookie("redirect_to", setting.AppSubURL+ctx.Req.URL.RequestURI(), 0, setting.AppSubURL)
+						middleware.SetRedirectToCookie(ctx.Resp, setting.AppSubURL+ctx.Req.URL.RequestURI())
 					}
 					ctx.Redirect(setting.AppSubURL + "/user/settings/change_password")
 					return
@@ -69,7 +70,7 @@ func Toggle(options *ToggleOptions) func(ctx *Context) {
 		if options.SignInRequired {
 			if !ctx.IsSigned {
 				if ctx.Req.URL.Path != "/user/events" {
-					ctx.SetCookie("redirect_to", setting.AppSubURL+ctx.Req.URL.RequestURI(), 0, setting.AppSubURL)
+					middleware.SetRedirectToCookie(ctx.Resp, setting.AppSubURL+ctx.Req.URL.RequestURI())
 				}
 				ctx.Redirect(setting.AppSubURL + "/user/login")
 				return
@@ -84,7 +85,7 @@ func Toggle(options *ToggleOptions) func(ctx *Context) {
 		if !options.SignOutRequired && !ctx.IsSigned &&
 			len(ctx.GetCookie(setting.CookieUserName)) > 0 {
 			if ctx.Req.URL.Path != "/user/events" {
-				ctx.SetCookie("redirect_to", setting.AppSubURL+ctx.Req.URL.RequestURI(), 0, setting.AppSubURL)
+				middleware.SetRedirectToCookie(ctx.Resp, setting.AppSubURL+ctx.Req.URL.RequestURI())
 			}
 			ctx.Redirect(setting.AppSubURL + "/user/login")
 			return
diff --git a/modules/context/context.go b/modules/context/context.go
index c06784c116..eecc81406d 100644
--- a/modules/context/context.go
+++ b/modules/context/context.go
@@ -386,9 +386,28 @@ func (ctx *Context) Redirect(location string, status ...int) {
 	http.Redirect(ctx.Resp, ctx.Req, location, code)
 }
 
-// SetCookie set cookies to web browser
-func (ctx *Context) SetCookie(name string, value string, others ...interface{}) {
-	middleware.SetCookie(ctx.Resp, name, value, others...)
+// SetCookie convenience function to set most cookies consistently
+// CSRF and a few others are the exception here
+func (ctx *Context) SetCookie(name, value string, expiry int) {
+	middleware.SetCookie(ctx.Resp, name, value,
+		expiry,
+		setting.AppSubURL,
+		setting.SessionConfig.Domain,
+		setting.SessionConfig.Secure,
+		true,
+		middleware.SameSite(setting.SessionConfig.SameSite))
+}
+
+// DeleteCookie convenience function to delete most cookies consistently
+// CSRF and a few others are the exception here
+func (ctx *Context) DeleteCookie(name string) {
+	middleware.SetCookie(ctx.Resp, name, "",
+		-1,
+		setting.AppSubURL,
+		setting.SessionConfig.Domain,
+		setting.SessionConfig.Secure,
+		true,
+		middleware.SameSite(setting.SessionConfig.SameSite))
 }
 
 // GetCookie returns given cookie value from request header.
@@ -399,6 +418,11 @@ func (ctx *Context) GetCookie(name string) string {
 // GetSuperSecureCookie returns given cookie value from request header with secret string.
 func (ctx *Context) GetSuperSecureCookie(secret, name string) (string, bool) {
 	val := ctx.GetCookie(name)
+	return ctx.CookieDecrypt(secret, val)
+}
+
+// CookieDecrypt returns given value from with secret string.
+func (ctx *Context) CookieDecrypt(secret, val string) (string, bool) {
 	if val == "" {
 		return "", false
 	}
@@ -414,14 +438,21 @@ func (ctx *Context) GetSuperSecureCookie(secret, name string) (string, bool) {
 }
 
 // SetSuperSecureCookie sets given cookie value to response header with secret string.
-func (ctx *Context) SetSuperSecureCookie(secret, name, value string, others ...interface{}) {
+func (ctx *Context) SetSuperSecureCookie(secret, name, value string, expiry int) {
+	text := ctx.CookieEncrypt(secret, value)
+
+	ctx.SetCookie(name, text, expiry)
+}
+
+// CookieEncrypt encrypts a given value using the provided secret
+func (ctx *Context) CookieEncrypt(secret, value string) string {
 	key := pbkdf2.Key([]byte(secret), []byte(secret), 1000, 16, sha256.New)
 	text, err := com.AESGCMEncrypt(key, []byte(value))
 	if err != nil {
 		panic("error encrypting cookie: " + err.Error())
 	}
 
-	ctx.SetCookie(name, hex.EncodeToString(text), others...)
+	return hex.EncodeToString(text)
 }
 
 // GetCookieInt returns cookie result in int type.
@@ -533,6 +564,7 @@ func getCsrfOpts() CsrfOptions {
 		Header:         "X-Csrf-Token",
 		CookieDomain:   setting.SessionConfig.Domain,
 		CookiePath:     setting.SessionConfig.CookiePath,
+		SameSite:       setting.SessionConfig.SameSite,
 	}
 }
 
@@ -597,17 +629,17 @@ func Contexter() func(next http.Handler) http.Handler {
 						middleware.Domain(setting.SessionConfig.Domain),
 						middleware.HTTPOnly(true),
 						middleware.Secure(setting.SessionConfig.Secure),
-						//middlewares.SameSite(opt.SameSite), FIXME: we need a samesite config
+						middleware.SameSite(setting.SessionConfig.SameSite),
 					)
 					return
 				}
 
-				ctx.SetCookie("macaron_flash", "", -1,
+				middleware.SetCookie(ctx.Resp, "macaron_flash", "", -1,
 					setting.SessionConfig.CookiePath,
 					middleware.Domain(setting.SessionConfig.Domain),
 					middleware.HTTPOnly(true),
 					middleware.Secure(setting.SessionConfig.Secure),
-					//middleware.SameSite(), FIXME: we need a samesite config
+					middleware.SameSite(setting.SessionConfig.SameSite),
 				)
 			})
 
diff --git a/modules/context/csrf.go b/modules/context/csrf.go
index 4a26664bf3..ba0e9f6cde 100644
--- a/modules/context/csrf.go
+++ b/modules/context/csrf.go
@@ -22,6 +22,8 @@ import (
 	"net/http"
 	"time"
 
+	"code.gitea.io/gitea/modules/web/middleware"
+
 	"github.com/unknwon/com"
 )
 
@@ -37,6 +39,8 @@ type CSRF interface {
 	GetCookiePath() string
 	// Return the flag value used for the csrf token.
 	GetCookieHTTPOnly() bool
+	// Return cookie domain
+	GetCookieDomain() string
 	// Return the token.
 	GetToken() string
 	// Validate by token.
@@ -93,6 +97,11 @@ func (c *csrf) GetCookieHTTPOnly() bool {
 	return c.CookieHTTPOnly
 }
 
+// GetCookieDomain returns the flag value used for the csrf token.
+func (c *csrf) GetCookieDomain() string {
+	return c.CookieDomain
+}
+
 // GetToken returns the current token. This is typically used
 // to populate a hidden form in an HTML template.
 func (c *csrf) GetToken() string {
@@ -227,10 +236,14 @@ func Csrfer(opt CsrfOptions, ctx *Context) CSRF {
 			if opt.CookieLifeTime == 0 {
 				expires = time.Now().AddDate(0, 0, 1)
 			}
-			ctx.SetCookie(opt.Cookie, x.Token, opt.CookieLifeTime, opt.CookiePath, opt.CookieDomain, opt.Secure, opt.CookieHTTPOnly, expires,
-				func(c *http.Cookie) {
-					c.SameSite = opt.SameSite
-				},
+			middleware.SetCookie(ctx.Resp, opt.Cookie, x.Token,
+				opt.CookieLifeTime,
+				opt.CookiePath,
+				opt.CookieDomain,
+				opt.Secure,
+				opt.CookieHTTPOnly,
+				expires,
+				middleware.SameSite(opt.SameSite),
 			)
 		}
 	}
@@ -248,14 +261,22 @@ func Csrfer(opt CsrfOptions, ctx *Context) CSRF {
 func Validate(ctx *Context, x CSRF) {
 	if token := ctx.Req.Header.Get(x.GetHeaderName()); len(token) > 0 {
 		if !x.ValidToken(token) {
-			ctx.SetCookie(x.GetCookieName(), "", -1, x.GetCookiePath())
+			// Delete the cookie
+			middleware.SetCookie(ctx.Resp, x.GetCookieName(), "",
+				-1,
+				x.GetCookiePath(),
+				x.GetCookieDomain()) // FIXME: Do we need to set the Secure, httpOnly and SameSite values too?
 			x.Error(ctx.Resp)
 		}
 		return
 	}
 	if token := ctx.Req.FormValue(x.GetFormName()); len(token) > 0 {
 		if !x.ValidToken(token) {
-			ctx.SetCookie(x.GetCookieName(), "", -1, x.GetCookiePath())
+			// Delete the cookie
+			middleware.SetCookie(ctx.Resp, x.GetCookieName(), "",
+				-1,
+				x.GetCookiePath(),
+				x.GetCookieDomain()) // FIXME: Do we need to set the Secure, httpOnly and SameSite values too?
 			x.Error(ctx.Resp)
 		}
 		return
diff --git a/modules/setting/session.go b/modules/setting/session.go
index eb5e1a1875..97666c5e53 100644
--- a/modules/setting/session.go
+++ b/modules/setting/session.go
@@ -5,6 +5,7 @@
 package setting
 
 import (
+	"net/http"
 	"path"
 	"path/filepath"
 	"strings"
@@ -31,10 +32,13 @@ var (
 		Secure bool
 		// Cookie domain name. Default is empty.
 		Domain string
+		// SameSite declares if your cookie should be restricted to a first-party or same-site context. Valid strings are "none", "lax", "strict". Default is "lax"
+		SameSite http.SameSite
 	}{
 		CookieName:  "i_like_gitea",
 		Gclifetime:  86400,
 		Maxlifetime: 86400,
+		SameSite:    http.SameSiteLaxMode,
 	}
 )
 
@@ -52,6 +56,15 @@ func newSessionService() {
 	SessionConfig.Gclifetime = sec.Key("GC_INTERVAL_TIME").MustInt64(86400)
 	SessionConfig.Maxlifetime = sec.Key("SESSION_LIFE_TIME").MustInt64(86400)
 	SessionConfig.Domain = sec.Key("DOMAIN").String()
+	samesiteString := sec.Key("SAME_SITE").In("lax", []string{"none", "lax", "strict"})
+	switch strings.ToLower(samesiteString) {
+	case "none":
+		SessionConfig.SameSite = http.SameSiteNoneMode
+	case "strict":
+		SessionConfig.SameSite = http.SameSiteStrictMode
+	default:
+		SessionConfig.SameSite = http.SameSiteLaxMode
+	}
 
 	json := jsoniter.ConfigCompatibleWithStandardLibrary
 	shadowConfig, err := json.Marshal(SessionConfig)
diff --git a/modules/web/middleware/cookie.go b/modules/web/middleware/cookie.go
index 83e365f9c4..cfcc2bbac7 100644
--- a/modules/web/middleware/cookie.go
+++ b/modules/web/middleware/cookie.go
@@ -76,6 +76,47 @@ func NewCookie(name, value string, maxAge int) *http.Cookie {
 	}
 }
 
+// SetRedirectToCookie convenience function to set the RedirectTo cookie consistently
+func SetRedirectToCookie(resp http.ResponseWriter, value string) {
+	SetCookie(resp, "redirect_to", value,
+		0,
+		setting.AppSubURL,
+		"",
+		setting.SessionConfig.Secure,
+		true,
+		SameSite(setting.SessionConfig.SameSite))
+}
+
+// DeleteRedirectToCookie convenience function to delete most cookies consistently
+func DeleteRedirectToCookie(resp http.ResponseWriter) {
+	SetCookie(resp, "redirect_to", "",
+		-1,
+		setting.AppSubURL,
+		"",
+		setting.SessionConfig.Secure,
+		true,
+		SameSite(setting.SessionConfig.SameSite))
+}
+
+// DeleteSesionConfigPathCookie convenience function to delete SessionConfigPath cookies consistently
+func DeleteSesionConfigPathCookie(resp http.ResponseWriter, name string) {
+	SetCookie(resp, name, "",
+		-1,
+		setting.SessionConfig.CookiePath,
+		setting.SessionConfig.Domain,
+		setting.SessionConfig.Secure,
+		true,
+		SameSite(setting.SessionConfig.SameSite))
+}
+
+// DeleteCSRFCookie convenience function to delete SessionConfigPath cookies consistently
+func DeleteCSRFCookie(resp http.ResponseWriter) {
+	SetCookie(resp, setting.CSRFCookieName, "",
+		-1,
+		setting.SessionConfig.CookiePath,
+		setting.SessionConfig.Domain) // FIXME: Do we need to set the Secure, httpOnly and SameSite values too?
+}
+
 // SetCookie set the cookies
 // TODO: Copied from gitea.com/macaron/macaron and should be improved after macaron removed.
 func SetCookie(resp http.ResponseWriter, name string, value string, others ...interface{}) {
diff --git a/modules/web/middleware/locale.go b/modules/web/middleware/locale.go
index 449095f611..a08e5aaeec 100644
--- a/modules/web/middleware/locale.go
+++ b/modules/web/middleware/locale.go
@@ -7,6 +7,7 @@ package middleware
 import (
 	"net/http"
 
+	"code.gitea.io/gitea/modules/setting"
 	"code.gitea.io/gitea/modules/translation"
 
 	"github.com/unknwon/i18n"
@@ -42,8 +43,30 @@ func Locale(resp http.ResponseWriter, req *http.Request) translation.Locale {
 	}
 
 	if changeLang {
-		SetCookie(resp, "lang", lang, 1<<31-1)
+		SetLocaleCookie(resp, lang, 1<<31-1)
 	}
 
 	return translation.NewLocale(lang)
 }
+
+// SetLocaleCookie convenience function to set the locale cookie consistently
+func SetLocaleCookie(resp http.ResponseWriter, lang string, expiry int) {
+	SetCookie(resp, "lang", lang, expiry,
+		setting.AppSubURL,
+		setting.SessionConfig.Domain,
+		setting.SessionConfig.Secure,
+		true,
+		SameSite(setting.SessionConfig.SameSite))
+}
+
+// DeleteLocaleCookie convenience function to delete the locale cookie consistently
+// Setting the lang cookie will trigger the middleware to reset the language ot previous state.
+func DeleteLocaleCookie(resp http.ResponseWriter) {
+	SetCookie(resp, "lang", "",
+		-1,
+		setting.AppSubURL,
+		setting.SessionConfig.Domain,
+		setting.SessionConfig.Secure,
+		true,
+		SameSite(setting.SessionConfig.SameSite))
+}
-- 
cgit v1.2.3