From bb5977095faf0a033a5e01c67597641a7c6db38b Mon Sep 17 00:00:00 2001 From: Nick Meves Date: Tue, 14 Jul 2020 15:02:10 -0700 Subject: [PATCH] Add option to remove tokens from cookie sessions (#673) * Add option to remove tokens from cookie sessions * Move Minimal to be an option on CookieSession * Add sessionOptionsDefaults helper --- CHANGELOG.md | 1 + docs/configuration/configuration.md | 1 + pkg/apis/options/options.go | 25 +++--- pkg/apis/options/sessions.go | 19 ++++- pkg/sessions/cookie/session_store.go | 17 +++- pkg/validation/options.go | 1 + pkg/validation/sessions.go | 32 +++++++ pkg/validation/sessions_test.go | 121 +++++++++++++++++++++++++++ 8 files changed, 199 insertions(+), 18 deletions(-) create mode 100644 pkg/validation/sessions.go create mode 100644 pkg/validation/sessions_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index b78a44b1..bc58b625 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ ## Changes since v6.0.0 +- [#673](https://github.com/oauth2-proxy/oauth2-proxy/pull/673) Add --session-cookie-minimal option to create session cookies with no tokens (@NickMeves) - [#632](https://github.com/oauth2-proxy/oauth2-proxy/pull/632) Reduce session size by encoding with MessagePack and using LZ4 compression (@NickMeves) - [#675](https://github.com/oauth2-proxy/oauth2-proxy/pull/675) Fix required ruby version and deprecated option for building docs (@mkontani) - [#669](https://github.com/oauth2-proxy/oauth2-proxy/pull/669) Reduce docker context to improve build times (@JoelSpeed) diff --git a/docs/configuration/configuration.md b/docs/configuration/configuration.md index 4642e6fa..6bf5c4f5 100644 --- a/docs/configuration/configuration.md +++ b/docs/configuration/configuration.md @@ -107,6 +107,7 @@ An example [oauth2-proxy.cfg]({{ site.gitweb }}/contrib/oauth2-proxy.cfg.example | `--resource` | string | The resource that is protected (Azure AD only) | | | `--reverse-proxy` | bool | are we running behind a reverse proxy, controls whether headers like X-Real-Ip are accepted | false | | `--scope` | string | OAuth scope specification | | +| `--session-cookie-minimal` | bool | strip OAuth tokens from cookie session stores if they aren't needed (cookie session store only) | false | | `--session-store-type` | string | [Session data storage backend](configuration/sessions); redis or cookie | cookie | | `--set-xauthrequest` | bool | set X-Auth-Request-User, X-Auth-Request-Email and X-Auth-Request-Preferred-Username response headers (useful in Nginx auth_request mode) | false | | `--set-authorization-header` | bool | set Authorization Bearer response header (useful in Nginx auth_request mode) | false | diff --git a/pkg/apis/options/options.go b/pkg/apis/options/options.go index 4dd7f1d2..8b9f84b5 100644 --- a/pkg/apis/options/options.go +++ b/pkg/apis/options/options.go @@ -145,19 +145,17 @@ func (o *Options) SetRealClientIPParser(s ipapi.RealClientIPParser) { o.realClie // NewOptions constructs a new Options with defaulted values func NewOptions() *Options { return &Options{ - ProxyPrefix: "/oauth2", - ProviderType: "google", - PingPath: "/ping", - ProxyWebSockets: true, - HTTPAddress: "127.0.0.1:4180", - HTTPSAddress: ":443", - RealClientIPHeader: "X-Real-IP", - ForceHTTPS: false, - DisplayHtpasswdForm: true, - Cookie: cookieDefaults(), - Session: SessionOptions{ - Type: "cookie", - }, + ProxyPrefix: "/oauth2", + ProviderType: "google", + PingPath: "/ping", + ProxyWebSockets: true, + HTTPAddress: "127.0.0.1:4180", + HTTPSAddress: ":443", + RealClientIPHeader: "X-Real-IP", + ForceHTTPS: false, + DisplayHtpasswdForm: true, + Cookie: cookieDefaults(), + Session: sessionOptionsDefaults(), AzureTenant: "common", SetXAuthRequest: false, SkipAuthPreflight: false, @@ -241,6 +239,7 @@ func NewFlagSet() *pflag.FlagSet { flagSet.String("ping-user-agent", "", "special User-Agent that will be used for basic health checks") flagSet.Bool("proxy-websockets", true, "enables WebSocket proxying") flagSet.String("session-store-type", "cookie", "the session storage provider to use") + flagSet.Bool("session-cookie-minimal", false, "strip OAuth tokens from cookie session stores if they aren't needed (cookie session store only)") flagSet.String("redis-connection-url", "", "URL of redis server for redis session storage (eg: redis://HOST[:PORT])") flagSet.Bool("redis-use-sentinel", false, "Connect to redis via sentinels. Must set --redis-sentinel-master-name and --redis-sentinel-connection-urls to use this feature") flagSet.String("redis-sentinel-master-name", "", "Redis sentinel master name. Used in conjunction with --redis-use-sentinel") diff --git a/pkg/apis/options/sessions.go b/pkg/apis/options/sessions.go index 7ff656fe..70482ee5 100644 --- a/pkg/apis/options/sessions.go +++ b/pkg/apis/options/sessions.go @@ -2,8 +2,9 @@ package options // SessionOptions contains configuration options for the SessionStore providers. type SessionOptions struct { - Type string `flag:"session-store-type" cfg:"session_store_type"` - Redis RedisStoreOptions `cfg:",squash"` + Type string `flag:"session-store-type" cfg:"session_store_type"` + Cookie CookieStoreOptions `cfg:",squash"` + Redis RedisStoreOptions `cfg:",squash"` } // CookieSessionStoreType is used to indicate the CookieSessionStore should be @@ -14,6 +15,11 @@ var CookieSessionStoreType = "cookie" // used for storing sessions. var RedisSessionStoreType = "redis" +// CookieStoreOptions contains configuration options for the CookieSessionStore. +type CookieStoreOptions struct { + Minimal bool `flag:"session-cookie-minimal" cfg:"session_cookie_minimal"` +} + // RedisStoreOptions contains configuration options for the RedisSessionStore. type RedisStoreOptions struct { ConnectionURL string `flag:"redis-connection-url" cfg:"redis_connection_url"` @@ -25,3 +31,12 @@ type RedisStoreOptions struct { CAPath string `flag:"redis-ca-path" cfg:"redis_ca_path"` InsecureSkipTLSVerify bool `flag:"redis-insecure-skip-tls-verify" cfg:"redis_insecure_skip_tls_verify"` } + +func sessionOptionsDefaults() SessionOptions { + return SessionOptions{ + Type: CookieSessionStoreType, + Cookie: CookieStoreOptions{ + Minimal: false, + }, + } +} diff --git a/pkg/sessions/cookie/session_store.go b/pkg/sessions/cookie/session_store.go index 69b55d11..18717ed1 100644 --- a/pkg/sessions/cookie/session_store.go +++ b/pkg/sessions/cookie/session_store.go @@ -30,6 +30,7 @@ var _ sessions.SessionStore = &SessionStore{} type SessionStore struct { Cookie *options.Cookie CookieCipher encryption.Cipher + Minimal bool } // Save takes a sessions.SessionState and stores the information from it @@ -39,7 +40,7 @@ func (s *SessionStore) Save(rw http.ResponseWriter, req *http.Request, ss *sessi now := time.Now() ss.CreatedAt = &now } - value, err := cookieForSession(ss, s.CookieCipher) + value, err := s.cookieForSession(ss) if err != nil { return err } @@ -85,8 +86,17 @@ func (s *SessionStore) Clear(rw http.ResponseWriter, req *http.Request) error { } // cookieForSession serializes a session state for storage in a cookie -func cookieForSession(s *sessions.SessionState, c encryption.Cipher) ([]byte, error) { - return s.EncodeSessionState(c, true) +func (s *SessionStore) cookieForSession(ss *sessions.SessionState) ([]byte, error) { + if s.Minimal && (ss.AccessToken != "" || ss.IDToken != "" || ss.RefreshToken != "") { + minimal := *ss + minimal.AccessToken = "" + minimal.IDToken = "" + minimal.RefreshToken = "" + + return minimal.EncodeSessionState(s.CookieCipher, true) + } + + return ss.EncodeSessionState(s.CookieCipher, true) } // sessionFromCookie deserializes a session from a cookie value @@ -146,6 +156,7 @@ func NewCookieSessionStore(opts *options.SessionOptions, cookieOpts *options.Coo return &SessionStore{ CookieCipher: cipher, Cookie: cookieOpts, + Minimal: opts.Cookie.Minimal, }, nil } diff --git a/pkg/validation/options.go b/pkg/validation/options.go index cefbb31a..d0f4ba06 100644 --- a/pkg/validation/options.go +++ b/pkg/validation/options.go @@ -27,6 +27,7 @@ import ( // are of the correct format func Validate(o *options.Options) error { msgs := validateCookie(o.Cookie) + msgs = append(msgs, validateSessionCookieMinimal(o)...) if o.SSLInsecureSkipVerify { insecureTransport := &http.Transport{ diff --git a/pkg/validation/sessions.go b/pkg/validation/sessions.go new file mode 100644 index 00000000..db4ba9fd --- /dev/null +++ b/pkg/validation/sessions.go @@ -0,0 +1,32 @@ +package validation + +import ( + "time" + + "github.com/oauth2-proxy/oauth2-proxy/pkg/apis/options" +) + +func validateSessionCookieMinimal(o *options.Options) []string { + if !o.Session.Cookie.Minimal { + return []string{} + } + + msgs := []string{} + if o.PassAuthorization { + msgs = append(msgs, + "pass_authorization_header requires oauth tokens in sessions. session_cookie_minimal cannot be set") + } + if o.SetAuthorization { + msgs = append(msgs, + "set_authorization_header requires oauth tokens in sessions. session_cookie_minimal cannot be set") + } + if o.PassAccessToken { + msgs = append(msgs, + "pass_access_token requires oauth tokens in sessions. session_cookie_minimal cannot be set") + } + if o.Cookie.Refresh != time.Duration(0) { + msgs = append(msgs, + "cookie_refresh > 0 requires oauth tokens in sessions. session_cookie_minimal cannot be set") + } + return msgs +} diff --git a/pkg/validation/sessions_test.go b/pkg/validation/sessions_test.go new file mode 100644 index 00000000..a6ffdbd0 --- /dev/null +++ b/pkg/validation/sessions_test.go @@ -0,0 +1,121 @@ +package validation + +import ( + "testing" + "time" + + "github.com/oauth2-proxy/oauth2-proxy/pkg/apis/options" + . "github.com/onsi/gomega" +) + +func Test_validateSessionCookieMinimal(t *testing.T) { + const ( + passAuthorizationMsg = "pass_authorization_header requires oauth tokens in sessions. session_cookie_minimal cannot be set" + setAuthorizationMsg = "set_authorization_header requires oauth tokens in sessions. session_cookie_minimal cannot be set" + passAccessTokenMsg = "pass_access_token requires oauth tokens in sessions. session_cookie_minimal cannot be set" + cookieRefreshMsg = "cookie_refresh > 0 requires oauth tokens in sessions. session_cookie_minimal cannot be set" + ) + + testCases := map[string]struct { + opts *options.Options + errStrings []string + }{ + "No minimal cookie session": { + opts: &options.Options{ + Session: options.SessionOptions{ + Cookie: options.CookieStoreOptions{ + Minimal: false, + }, + }, + }, + errStrings: []string{}, + }, + "No minimal cookie session & passAuthorization": { + opts: &options.Options{ + Session: options.SessionOptions{ + Cookie: options.CookieStoreOptions{ + Minimal: false, + }, + }, + PassAuthorization: true, + }, + errStrings: []string{}, + }, + "Minimal cookie session no conflicts": { + opts: &options.Options{ + Session: options.SessionOptions{ + Cookie: options.CookieStoreOptions{ + Minimal: true, + }, + }, + }, + errStrings: []string{}, + }, + "PassAuthorization conflict": { + opts: &options.Options{ + Session: options.SessionOptions{ + Cookie: options.CookieStoreOptions{ + Minimal: true, + }, + }, + PassAuthorization: true, + }, + errStrings: []string{passAuthorizationMsg}, + }, + "SetAuthorization conflict": { + opts: &options.Options{ + Session: options.SessionOptions{ + Cookie: options.CookieStoreOptions{ + Minimal: true, + }, + }, + SetAuthorization: true, + }, + errStrings: []string{setAuthorizationMsg}, + }, + "PassAccessToken conflict": { + opts: &options.Options{ + Session: options.SessionOptions{ + Cookie: options.CookieStoreOptions{ + Minimal: true, + }, + }, + PassAccessToken: true, + }, + errStrings: []string{passAccessTokenMsg}, + }, + "CookieRefresh conflict": { + opts: &options.Options{ + Cookie: options.Cookie{ + Refresh: time.Hour, + }, + Session: options.SessionOptions{ + Cookie: options.CookieStoreOptions{ + Minimal: true, + }, + }, + }, + errStrings: []string{cookieRefreshMsg}, + }, + "Multiple conflicts": { + opts: &options.Options{ + Session: options.SessionOptions{ + Cookie: options.CookieStoreOptions{ + Minimal: true, + }, + }, + PassAuthorization: true, + PassAccessToken: true, + }, + errStrings: []string{passAuthorizationMsg, passAccessTokenMsg}, + }, + } + + for testName, tc := range testCases { + t.Run(testName, func(t *testing.T) { + errStrings := validateSessionCookieMinimal(tc.opts) + g := NewWithT(t) + g.Expect(errStrings).To(ConsistOf(tc.errStrings)) + }) + } +}