From 90f8117fba5138aed8880fd373d9c2d04055232b Mon Sep 17 00:00:00 2001 From: Paul Groudas Date: Fri, 20 Dec 2019 09:44:59 -0500 Subject: [PATCH 1/3] Fix typos in doc strings. --- oauthproxy.go | 2 +- pkg/cookies/cookies.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/oauthproxy.go b/oauthproxy.go index 72ab580b..40e43b6d 100644 --- a/oauthproxy.go +++ b/oauthproxy.go @@ -195,7 +195,7 @@ func NewWebSocketOrRestReverseProxy(u *url.URL, opts *Options, auth hmacauth.Hma } } -// NewOAuthProxy creates a new instance of OOuthProxy from the options provided +// NewOAuthProxy creates a new instance of OAuthProxy from the options provided func NewOAuthProxy(opts *Options, validator func(string) bool) *OAuthProxy { serveMux := http.NewServeMux() var auth hmacauth.HmacAuth diff --git a/pkg/cookies/cookies.go b/pkg/cookies/cookies.go index 75b93e4d..4a8bed93 100644 --- a/pkg/cookies/cookies.go +++ b/pkg/cookies/cookies.go @@ -34,7 +34,7 @@ func MakeCookie(req *http.Request, name string, value string, path string, domai } } -// MakeCookieFromOptions constructs a cookie based on the givemn *options.CookieOptions, +// MakeCookieFromOptions constructs a cookie based on the given *options.CookieOptions, // value and creation time func MakeCookieFromOptions(req *http.Request, name string, value string, opts *options.CookieOptions, expiration time.Duration, now time.Time) *http.Cookie { return MakeCookie(req, name, value, opts.CookiePath, opts.CookieDomain, opts.CookieHTTPOnly, opts.CookieSecure, expiration, now) From 5d0827a028bf543c8dbd40ddfcdf2697d5c93b70 Mon Sep 17 00:00:00 2001 From: Paul Groudas Date: Mon, 16 Dec 2019 13:10:04 -0500 Subject: [PATCH 2/3] Add configuration for cookie 'SameSite' value. Values of 'lax' and 'strict' can improve and mitigate some categories of cross-site traffic tampering. Given that the nature of this proxy is often to proxy private tools, this is useful to take advantage of. See: https://www.owasp.org/index.php/SameSite --- contrib/oauth2_proxy_autocomplete.sh | 2 +- docs/configuration/configuration.md | 1 + main.go | 1 + oauthproxy.go | 6 +++++- options.go | 6 ++++++ pkg/apis/options/cookie.go | 1 + pkg/cookies/cookies.go | 22 ++++++++++++++++++++-- pkg/sessions/session_store_test.go | 12 ++++++++++-- 8 files changed, 45 insertions(+), 6 deletions(-) diff --git a/contrib/oauth2_proxy_autocomplete.sh b/contrib/oauth2_proxy_autocomplete.sh index fd9d87a4..e0823635 100644 --- a/contrib/oauth2_proxy_autocomplete.sh +++ b/contrib/oauth2_proxy_autocomplete.sh @@ -20,7 +20,7 @@ _oauth2_proxy() { COMPREPLY=( $(compgen -W "google azure facebook github keycloak gitlab linkedin login.gov" -- ${cur}) ) return 0 ;; - -@(http-address|https-address|redirect-url|upstream|basic-auth-password|skip-auth-regex|flush-interval|extra-jwt-issuers|email-domain|whitelist-domain|keycloak-group|azure-tenant|bitbucket-team|bitbucket-repository|github-org|github-team|gitlab-group|google-group|google-admin-email|google-service-account-json|client-id|client_secret|banner|footer|proxy-prefix|ping-path|cookie-name|cookie-secret|cookie-domain|cookie-path|cookie-expire|cookie-refresh|redist-sentinel-master-name|redist-sentinel-connection-urls|logging-max-size|logging-max-age|logging-max-backups|standard-logging-format|request-logging-format|exclude-logging-paths|auth-logging-format|oidc-issuer-url|oidc-jwks-url|login-url|redeem-url|profile-url|resource|validate-url|scope|approval-prompt|signature-key|acr-values|jwt-key|pubjwk-url)) + -@(http-address|https-address|redirect-url|upstream|basic-auth-password|skip-auth-regex|flush-interval|extra-jwt-issuers|email-domain|whitelist-domain|keycloak-group|azure-tenant|bitbucket-team|bitbucket-repository|github-org|github-team|gitlab-group|google-group|google-admin-email|google-service-account-json|client-id|client_secret|banner|footer|proxy-prefix|ping-path|cookie-name|cookie-secret|cookie-domain|cookie-path|cookie-expire|cookie-refresh|cookie-samesite|redist-sentinel-master-name|redist-sentinel-connection-urls|logging-max-size|logging-max-age|logging-max-backups|standard-logging-format|request-logging-format|exclude-logging-paths|auth-logging-format|oidc-issuer-url|oidc-jwks-url|login-url|redeem-url|profile-url|resource|validate-url|scope|approval-prompt|signature-key|acr-values|jwt-key|pubjwk-url)) return 0 ;; esac diff --git a/docs/configuration/configuration.md b/docs/configuration/configuration.md index 8ff64c18..0f268415 100644 --- a/docs/configuration/configuration.md +++ b/docs/configuration/configuration.md @@ -38,6 +38,7 @@ An example [oauth2_proxy.cfg]({{ site.gitweb }}/contrib/oauth2_proxy.cfg.example | `-cookie-refresh` | duration | refresh the cookie after this duration; `0` to disable | | | `-cookie-secret` | string | the seed string for secure cookies (optionally base64 encoded) | | | `-cookie-secure` | bool | set secure (HTTPS) cookie flag | true | +| `-cookie-samesite` | string | set SameSite cookie attribute (ie: `"lax"`, `"strict"`, `"none"`, or `""`). | `""` | | `-custom-templates-dir` | string | path to custom html templates | | | `-display-htpasswd-form` | bool | display username / password login form if an htpasswd file is provided | true | | `-email-domain` | string | authenticate emails with the specified domain (may be given multiple times). Use `*` to authenticate any email | | diff --git a/main.go b/main.go index 16bafbc0..9105390f 100644 --- a/main.go +++ b/main.go @@ -86,6 +86,7 @@ func main() { flagSet.Duration("cookie-refresh", time.Duration(0), "refresh the cookie after this duration; 0 to disable") flagSet.Bool("cookie-secure", true, "set secure (HTTPS) cookie flag") flagSet.Bool("cookie-httponly", true, "set HttpOnly cookie flag") + flagSet.String("cookie-samesite", "", "set SameSite cookie attribute (ie: \"lax\", \"strict\", \"none\", or \"\"). ") flagSet.String("session-store-type", "cookie", "the session storage provider to use") flagSet.String("redis-connection-url", "", "URL of redis server for redis session storage (eg: redis://HOST[:PORT])") diff --git a/oauthproxy.go b/oauthproxy.go index 40e43b6d..b44ffabc 100644 --- a/oauthproxy.go +++ b/oauthproxy.go @@ -20,6 +20,7 @@ import ( "github.com/coreos/go-oidc" "github.com/mbland/hmacauth" sessionsapi "github.com/pusher/oauth2_proxy/pkg/apis/sessions" + "github.com/pusher/oauth2_proxy/pkg/cookies" "github.com/pusher/oauth2_proxy/pkg/encryption" "github.com/pusher/oauth2_proxy/pkg/logger" "github.com/pusher/oauth2_proxy/providers" @@ -68,6 +69,7 @@ type OAuthProxy struct { CookieHTTPOnly bool CookieExpire time.Duration CookieRefresh time.Duration + CookieSameSite string Validator func(string) bool RobotsPath string @@ -260,7 +262,7 @@ func NewOAuthProxy(opts *Options, validator func(string) bool) *OAuthProxy { refresh = fmt.Sprintf("after %s", opts.CookieRefresh) } - logger.Printf("Cookie settings: name:%s secure(https):%v httponly:%v expiry:%s domain:%s path:%s refresh:%s", opts.CookieName, opts.CookieSecure, opts.CookieHTTPOnly, opts.CookieExpire, opts.CookieDomain, opts.CookiePath, refresh) + logger.Printf("Cookie settings: name:%s secure(https):%v httponly:%v expiry:%s domain:%s path:%s samesite:%s refresh:%s", opts.CookieName, opts.CookieSecure, opts.CookieHTTPOnly, opts.CookieExpire, opts.CookieDomain, opts.CookiePath, opts.CookieSameSite, refresh) return &OAuthProxy{ CookieName: opts.CookieName, @@ -272,6 +274,7 @@ func NewOAuthProxy(opts *Options, validator func(string) bool) *OAuthProxy { CookieHTTPOnly: opts.CookieHTTPOnly, CookieExpire: opts.CookieExpire, CookieRefresh: opts.CookieRefresh, + CookieSameSite: opts.CookieSameSite, Validator: validator, RobotsPath: "/robots.txt", @@ -380,6 +383,7 @@ func (p *OAuthProxy) makeCookie(req *http.Request, name string, value string, ex HttpOnly: p.CookieHTTPOnly, Secure: p.CookieSecure, Expires: now.Add(expiration), + SameSite: cookies.ParseSameSite(p.CookieSameSite), } } diff --git a/options.go b/options.go index 5bb829b0..5c83958f 100644 --- a/options.go +++ b/options.go @@ -372,6 +372,12 @@ func (o *Options) Validate() error { } } + switch o.CookieSameSite { + case "", "none", "lax", "strict": + default: + msgs = append(msgs, fmt.Sprintf("cookie_samesite (%s) must be one of ['', 'lax', 'strict', 'none']", o.CookieSameSite)) + } + msgs = parseSignatureKey(o, msgs) msgs = validateCookieName(o, msgs) msgs = setupLogger(o, msgs) diff --git a/pkg/apis/options/cookie.go b/pkg/apis/options/cookie.go index 80ecf575..dcb6c75a 100644 --- a/pkg/apis/options/cookie.go +++ b/pkg/apis/options/cookie.go @@ -12,4 +12,5 @@ type CookieOptions struct { CookieRefresh time.Duration `flag:"cookie-refresh" cfg:"cookie_refresh" env:"OAUTH2_PROXY_COOKIE_REFRESH"` CookieSecure bool `flag:"cookie-secure" cfg:"cookie_secure" env:"OAUTH2_PROXY_COOKIE_SECURE"` CookieHTTPOnly bool `flag:"cookie-httponly" cfg:"cookie_httponly" env:"OAUTH2_PROXY_COOKIE_HTTPONLY"` + CookieSameSite string `flag:"cookie-samesite" cfg:"cookie_samesite" env:"OAUTH2_PROXY_COOKIE_SAMESITE"` } diff --git a/pkg/cookies/cookies.go b/pkg/cookies/cookies.go index 4a8bed93..b2a02a12 100644 --- a/pkg/cookies/cookies.go +++ b/pkg/cookies/cookies.go @@ -1,6 +1,7 @@ package cookies import ( + "fmt" "net" "net/http" "strings" @@ -12,7 +13,7 @@ import ( // MakeCookie constructs a cookie from the given parameters, // discovering the domain from the request if not specified. -func MakeCookie(req *http.Request, name string, value string, path string, domain string, httpOnly bool, secure bool, expiration time.Duration, now time.Time) *http.Cookie { +func MakeCookie(req *http.Request, name string, value string, path string, domain string, httpOnly bool, secure bool, expiration time.Duration, now time.Time, sameSite http.SameSite) *http.Cookie { if domain != "" { host := req.Host if h, _, err := net.SplitHostPort(host); err == nil { @@ -31,11 +32,28 @@ func MakeCookie(req *http.Request, name string, value string, path string, domai HttpOnly: httpOnly, Secure: secure, Expires: now.Add(expiration), + SameSite: sameSite, } } // MakeCookieFromOptions constructs a cookie based on the given *options.CookieOptions, // value and creation time func MakeCookieFromOptions(req *http.Request, name string, value string, opts *options.CookieOptions, expiration time.Duration, now time.Time) *http.Cookie { - return MakeCookie(req, name, value, opts.CookiePath, opts.CookieDomain, opts.CookieHTTPOnly, opts.CookieSecure, expiration, now) + return MakeCookie(req, name, value, opts.CookiePath, opts.CookieDomain, opts.CookieHTTPOnly, opts.CookieSecure, expiration, now, ParseSameSite(opts.CookieSameSite)) +} + +// Parse a valid http.SameSite value from a user supplied string for use of making cookies. +func ParseSameSite(v string) http.SameSite { + switch v { + case "lax": + return http.SameSiteLaxMode + case "strict": + return http.SameSiteStrictMode + case "none": + return http.SameSiteNoneMode + case "": + return http.SameSiteDefaultMode + default: + panic(fmt.Sprintf("Invalid value for SameSite: %s", v)) + } } diff --git a/pkg/sessions/session_store_test.go b/pkg/sessions/session_store_test.go index fd0b0e58..f8162c1b 100644 --- a/pkg/sessions/session_store_test.go +++ b/pkg/sessions/session_store_test.go @@ -15,7 +15,7 @@ import ( . "github.com/onsi/gomega" "github.com/pusher/oauth2_proxy/pkg/apis/options" sessionsapi "github.com/pusher/oauth2_proxy/pkg/apis/sessions" - "github.com/pusher/oauth2_proxy/pkg/cookies" + cookiesapi "github.com/pusher/oauth2_proxy/pkg/cookies" "github.com/pusher/oauth2_proxy/pkg/encryption" "github.com/pusher/oauth2_proxy/pkg/sessions" sessionscookie "github.com/pusher/oauth2_proxy/pkg/sessions/cookie" @@ -79,6 +79,12 @@ var _ = Describe("NewSessionStore", func() { } }) + It("have the correct SameSite set", func() { + for _, cookie := range cookies { + Expect(cookie.SameSite).To(Equal(cookiesapi.ParseSameSite(cookieOpts.CookieSameSite))) + } + }) + It("have a signature timestamp matching session.CreatedAt", func() { for _, cookie := range cookies { if cookie.Value != "" { @@ -159,7 +165,7 @@ var _ = Describe("NewSessionStore", func() { By("Using a valid cookie with a different providers session encoding") broken := "BrokenSessionFromADifferentSessionImplementation" value := encryption.SignedValue(cookieOpts.CookieSecret, cookieOpts.CookieName, broken, time.Now()) - cookie := cookies.MakeCookieFromOptions(request, cookieOpts.CookieName, value, cookieOpts, cookieOpts.CookieExpire, time.Now()) + cookie := cookiesapi.MakeCookieFromOptions(request, cookieOpts.CookieName, value, cookieOpts, cookieOpts.CookieExpire, time.Now()) request.AddCookie(cookie) err := ss.Save(response, request, session) @@ -338,6 +344,7 @@ var _ = Describe("NewSessionStore", func() { CookieSecure: false, CookieHTTPOnly: false, CookieDomain: "example.com", + CookieSameSite: "strict", } var err error @@ -379,6 +386,7 @@ var _ = Describe("NewSessionStore", func() { CookieRefresh: time.Duration(1) * time.Hour, CookieSecure: true, CookieHTTPOnly: true, + CookieSameSite: "", } session = &sessionsapi.SessionState{ From afb7247ad5e49e9df5b1966a9170c42aebd8897f Mon Sep 17 00:00:00 2001 From: Paul Groudas Date: Thu, 19 Dec 2019 15:40:38 -0500 Subject: [PATCH 3/3] Add item to CHANGELOG for SameSite configuration. --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 90b7ccf5..1f0482c9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ ## Breaking Changes ## Changes since v4.1.0 +- [#339](https://github.com/pusher/oauth2_proxy/pull/339) Add configuration for cookie 'SameSite' value. (@pgroudas) - [#325](https://github.com/pusher/oauth2_proxy/pull/325) dist.sh: use sha256sum (@syscll) - [#179](https://github.com/pusher/oauth2_proxy/pull/179) Add Nextcloud provider (@Ramblurr)