From 6db1aeb9c69c9693b0dd91c28b9c4e1951993dfa Mon Sep 17 00:00:00 2001 From: Nick Meves Date: Thu, 6 Aug 2020 15:43:01 -0700 Subject: [PATCH] Validate Redis session store health on startup --- CHANGELOG.md | 3 + pkg/sessions/redis/redis_store.go | 6 +- pkg/validation/options.go | 1 + pkg/validation/sessions.go | 51 ++++++ pkg/validation/sessions_test.go | 278 +++++++++++++++++++++++++++--- 5 files changed, 308 insertions(+), 31 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f8ff3c33..4919b34a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,9 +11,12 @@ ## Breaking Changes +- [#722](https://github.com/oauth2-proxy/oauth2-proxy/pull/722) When a Redis session store is configured, OAuth2-Proxy will fail to start up unless connection and health checks to Redis pass + ## Changes since v6.1.1 - [#575](https://github.com/oauth2-proxy/oauth2-proxy/pull/575) Stop accepting legacy SHA1 signed cookies (@NickMeves) +- [#722](https://github.com/oauth2-proxy/oauth2-proxy/pull/722) Validate Redis configuration options at startup (@NickMeves) - [#764](https://github.com/oauth2-proxy/oauth2-proxy/pull/764) Document bcrypt encryption for htpasswd (and hide SHA) (@lentzi90) - [#616](https://github.com/oauth2-proxy/oauth2-proxy/pull/616) Add support to ensure user belongs in required groups when using the OIDC provider (@stefansedich) diff --git a/pkg/sessions/redis/redis_store.go b/pkg/sessions/redis/redis_store.go index 10d99347..dcdf6a27 100644 --- a/pkg/sessions/redis/redis_store.go +++ b/pkg/sessions/redis/redis_store.go @@ -23,7 +23,7 @@ type SessionStore struct { // NewRedisSessionStore initialises a new instance of the SessionStore and wraps // it in a persistence.Manager func NewRedisSessionStore(opts *options.SessionOptions, cookieOpts *options.Cookie) (sessions.SessionStore, error) { - client, err := newRedisClient(opts.Redis) + client, err := NewRedisClient(opts.Redis) if err != nil { return nil, fmt.Errorf("error constructing redis client: %v", err) } @@ -64,9 +64,9 @@ func (store *SessionStore) Clear(ctx context.Context, key string) error { return nil } -// newRedisClient makes a redis.Client (either standalone, sentinel aware, or +// NewRedisClient makes a redis.Client (either standalone, sentinel aware, or // redis cluster) -func newRedisClient(opts options.RedisStoreOptions) (Client, error) { +func NewRedisClient(opts options.RedisStoreOptions) (Client, error) { if opts.UseSentinel && opts.UseCluster { return nil, fmt.Errorf("options redis-use-sentinel and redis-use-cluster are mutually exclusive") } diff --git a/pkg/validation/options.go b/pkg/validation/options.go index 75f678be..9964d2d8 100644 --- a/pkg/validation/options.go +++ b/pkg/validation/options.go @@ -28,6 +28,7 @@ import ( func Validate(o *options.Options) error { msgs := validateCookie(o.Cookie) msgs = append(msgs, validateSessionCookieMinimal(o)...) + msgs = append(msgs, validateRedisSessionStore(o)...) if o.SSLInsecureSkipVerify { // InsecureSkipVerify is a configurable option we allow diff --git a/pkg/validation/sessions.go b/pkg/validation/sessions.go index db4ba9fd..1d61b92c 100644 --- a/pkg/validation/sessions.go +++ b/pkg/validation/sessions.go @@ -1,9 +1,13 @@ package validation import ( + "context" + "fmt" "time" "github.com/oauth2-proxy/oauth2-proxy/pkg/apis/options" + "github.com/oauth2-proxy/oauth2-proxy/pkg/encryption" + "github.com/oauth2-proxy/oauth2-proxy/pkg/sessions/redis" ) func validateSessionCookieMinimal(o *options.Options) []string { @@ -30,3 +34,50 @@ func validateSessionCookieMinimal(o *options.Options) []string { } return msgs } + +// validateRedisSessionStore builds a Redis Client from the options and +// attempts to connect, Set, Get and Del a random health check key +func validateRedisSessionStore(o *options.Options) []string { + if o.Session.Type != options.RedisSessionStoreType { + return []string{} + } + + client, err := redis.NewRedisClient(o.Session.Redis) + if err != nil { + return []string{fmt.Sprintf("unable to initialize a redis client: %v", err)} + } + + nonce, err := encryption.Nonce() + if err != nil { + return []string{fmt.Sprintf("unable to generate a redis initialization test key: %v", err)} + } + + key := fmt.Sprintf("%s-healthcheck-%s", o.Cookie.Name, nonce) + return sendRedisConnectionTest(client, key, nonce) +} + +func sendRedisConnectionTest(client redis.Client, key string, val string) []string { + msgs := []string{} + ctx := context.Background() + + err := client.Set(ctx, key, []byte(val), time.Duration(60)*time.Second) + if err != nil { + msgs = append(msgs, fmt.Sprintf("unable to set a redis initialization key: %v", err)) + } else { + gval, err := client.Get(ctx, key) + if err != nil { + msgs = append(msgs, + fmt.Sprintf("unable to retrieve redis initialization key: %v", err)) + } + if string(gval) != val { + msgs = append(msgs, + "the retrieved redis initialization key did not match the value we set") + } + } + + err = client.Del(ctx, key) + if err != nil { + msgs = append(msgs, fmt.Sprintf("unable to delete the redis initialization key: %v", err)) + } + return msgs +} diff --git a/pkg/validation/sessions_test.go b/pkg/validation/sessions_test.go index a6ffdbd0..edd06acf 100644 --- a/pkg/validation/sessions_test.go +++ b/pkg/validation/sessions_test.go @@ -1,14 +1,17 @@ package validation import ( - "testing" "time" + "github.com/Bose/minisentinel" + "github.com/alicebob/miniredis/v2" "github.com/oauth2-proxy/oauth2-proxy/pkg/apis/options" + . "github.com/onsi/ginkgo" + . "github.com/onsi/ginkgo/extensions/table" . "github.com/onsi/gomega" ) -func Test_validateSessionCookieMinimal(t *testing.T) { +var _ = Describe("Sessions", func() { 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" @@ -16,11 +19,16 @@ func Test_validateSessionCookieMinimal(t *testing.T) { cookieRefreshMsg = "cookie_refresh > 0 requires oauth tokens in sessions. session_cookie_minimal cannot be set" ) - testCases := map[string]struct { + type cookieMinimalTableInput struct { opts *options.Options errStrings []string - }{ - "No minimal cookie session": { + } + + DescribeTable("validateSessionCookieMinimal", + func(o *cookieMinimalTableInput) { + Expect(validateSessionCookieMinimal(o.opts)).To(ConsistOf(o.errStrings)) + }, + Entry("No minimal cookie session", &cookieMinimalTableInput{ opts: &options.Options{ Session: options.SessionOptions{ Cookie: options.CookieStoreOptions{ @@ -29,8 +37,8 @@ func Test_validateSessionCookieMinimal(t *testing.T) { }, }, errStrings: []string{}, - }, - "No minimal cookie session & passAuthorization": { + }), + Entry("No minimal cookie session & passAuthorization", &cookieMinimalTableInput{ opts: &options.Options{ Session: options.SessionOptions{ Cookie: options.CookieStoreOptions{ @@ -40,8 +48,8 @@ func Test_validateSessionCookieMinimal(t *testing.T) { PassAuthorization: true, }, errStrings: []string{}, - }, - "Minimal cookie session no conflicts": { + }), + Entry("Minimal cookie session no conflicts", &cookieMinimalTableInput{ opts: &options.Options{ Session: options.SessionOptions{ Cookie: options.CookieStoreOptions{ @@ -50,8 +58,8 @@ func Test_validateSessionCookieMinimal(t *testing.T) { }, }, errStrings: []string{}, - }, - "PassAuthorization conflict": { + }), + Entry("PassAuthorization conflict", &cookieMinimalTableInput{ opts: &options.Options{ Session: options.SessionOptions{ Cookie: options.CookieStoreOptions{ @@ -61,8 +69,8 @@ func Test_validateSessionCookieMinimal(t *testing.T) { PassAuthorization: true, }, errStrings: []string{passAuthorizationMsg}, - }, - "SetAuthorization conflict": { + }), + Entry("SetAuthorization conflict", &cookieMinimalTableInput{ opts: &options.Options{ Session: options.SessionOptions{ Cookie: options.CookieStoreOptions{ @@ -72,8 +80,8 @@ func Test_validateSessionCookieMinimal(t *testing.T) { SetAuthorization: true, }, errStrings: []string{setAuthorizationMsg}, - }, - "PassAccessToken conflict": { + }), + Entry("PassAccessToken conflict", &cookieMinimalTableInput{ opts: &options.Options{ Session: options.SessionOptions{ Cookie: options.CookieStoreOptions{ @@ -83,8 +91,8 @@ func Test_validateSessionCookieMinimal(t *testing.T) { PassAccessToken: true, }, errStrings: []string{passAccessTokenMsg}, - }, - "CookieRefresh conflict": { + }), + Entry("CookieRefresh conflict", &cookieMinimalTableInput{ opts: &options.Options{ Cookie: options.Cookie{ Refresh: time.Hour, @@ -96,8 +104,8 @@ func Test_validateSessionCookieMinimal(t *testing.T) { }, }, errStrings: []string{cookieRefreshMsg}, - }, - "Multiple conflicts": { + }), + Entry("Multiple conflicts", &cookieMinimalTableInput{ opts: &options.Options{ Session: options.SessionOptions{ Cookie: options.CookieStoreOptions{ @@ -108,14 +116,228 @@ func Test_validateSessionCookieMinimal(t *testing.T) { PassAccessToken: true, }, errStrings: []string{passAuthorizationMsg, passAccessTokenMsg}, - }, + }), + ) + + const ( + clusterAndSentinelMsg = "unable to initialize a redis client: options redis-use-sentinel and redis-use-cluster are mutually exclusive" + parseWrongSchemeMsg = "unable to initialize a redis client: unable to parse redis url: invalid redis URL scheme: https" + parseWrongFormatMsg = "unable to initialize a redis client: unable to parse redis url: invalid redis database number: \"wrong\"" + invalidPasswordSetMsg = "unable to set a redis initialization key: WRONGPASS invalid username-password pair" + invalidPasswordDelMsg = "unable to delete the redis initialization key: WRONGPASS invalid username-password pair" + unreachableRedisSetMsg = "unable to set a redis initialization key: dial tcp 127.0.0.1:65535: connect: connection refused" + unreachableRedisDelMsg = "unable to delete the redis initialization key: dial tcp 127.0.0.1:65535: connect: connection refused" + unreachableSentinelSetMsg = "unable to set a redis initialization key: redis: all sentinels are unreachable" + unrechableSentinelDelMsg = "unable to delete the redis initialization key: redis: all sentinels are unreachable" + ) + + type redisStoreTableInput struct { + // miniredis setup details + password string + useSentinel bool + setAddr bool + setSentinelAddr bool + setMasterName bool + + opts *options.Options + errStrings []string } - 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)) - }) - } -} + DescribeTable("validateRedisSessionStore", + func(o *redisStoreTableInput) { + mr, err := miniredis.Run() + Expect(err).ToNot(HaveOccurred()) + mr.RequireAuth(o.password) + defer mr.Close() + + if o.setAddr && !o.useSentinel { + o.opts.Session.Redis.ConnectionURL = "redis://" + mr.Addr() + } + + if o.useSentinel { + ms := minisentinel.NewSentinel(mr) + Expect(ms.Start()).To(Succeed()) + defer ms.Close() + + if o.setSentinelAddr { + o.opts.Session.Redis.SentinelConnectionURLs = []string{"redis://" + ms.Addr()} + } + if o.setMasterName { + o.opts.Session.Redis.SentinelMasterName = ms.MasterInfo().Name + } + } + + Expect(validateRedisSessionStore(o.opts)).To(ConsistOf(o.errStrings)) + }, + Entry("cookie sessions are skipped", &redisStoreTableInput{ + opts: &options.Options{ + Session: options.SessionOptions{ + Type: options.CookieSessionStoreType, + }, + }, + errStrings: []string{}, + }), + Entry("connect successfully to pure redis", &redisStoreTableInput{ + setAddr: true, + + opts: &options.Options{ + Session: options.SessionOptions{ + Type: options.RedisSessionStoreType, + }, + }, + errStrings: []string{}, + }), + Entry("failed redis connection with wrong address", &redisStoreTableInput{ + opts: &options.Options{ + Session: options.SessionOptions{ + Type: options.RedisSessionStoreType, + Redis: options.RedisStoreOptions{ + ConnectionURL: "redis://127.0.0.1:65535", + }, + }, + }, + errStrings: []string{unreachableRedisSetMsg, unreachableRedisDelMsg}, + }), + Entry("fail to parse an invalid connection URL with wrong scheme", &redisStoreTableInput{ + opts: &options.Options{ + Session: options.SessionOptions{ + Type: options.RedisSessionStoreType, + Redis: options.RedisStoreOptions{ + ConnectionURL: "https://example.com", + }, + }, + }, + errStrings: []string{parseWrongSchemeMsg}, + }), + Entry("fail to parse an invalid connection URL with invalid format", &redisStoreTableInput{ + opts: &options.Options{ + Session: options.SessionOptions{ + Type: options.RedisSessionStoreType, + Redis: options.RedisStoreOptions{ + ConnectionURL: "redis://127.0.0.1:6379/wrong", + }, + }, + }, + errStrings: []string{parseWrongFormatMsg}, + }), + Entry("connect successfully to pure redis with password", &redisStoreTableInput{ + password: "abcdef123", + setAddr: true, + + opts: &options.Options{ + Session: options.SessionOptions{ + Type: options.RedisSessionStoreType, + Redis: options.RedisStoreOptions{ + Password: "abcdef123", + }, + }, + }, + errStrings: []string{}, + }), + Entry("failed connection with wrong password", &redisStoreTableInput{ + password: "abcdef123", + setAddr: true, + + opts: &options.Options{ + Session: options.SessionOptions{ + Type: options.RedisSessionStoreType, + Redis: options.RedisStoreOptions{ + Password: "zyxwtuv987", + }, + }, + }, + errStrings: []string{invalidPasswordSetMsg, invalidPasswordDelMsg}, + }), + Entry("connect successfully to sentinel redis", &redisStoreTableInput{ + useSentinel: true, + setSentinelAddr: true, + setMasterName: true, + + opts: &options.Options{ + Session: options.SessionOptions{ + Type: options.RedisSessionStoreType, + Redis: options.RedisStoreOptions{ + UseSentinel: true, + }, + }, + }, + errStrings: []string{}, + }), + Entry("connect successfully to sentinel redis with password", &redisStoreTableInput{ + password: "abcdef123", + useSentinel: true, + setSentinelAddr: true, + setMasterName: true, + + opts: &options.Options{ + Session: options.SessionOptions{ + Type: options.RedisSessionStoreType, + Redis: options.RedisStoreOptions{ + Password: "abcdef123", + UseSentinel: true, + }, + }, + }, + errStrings: []string{}, + }), + Entry("failed connection to sentinel redis with wrong password", &redisStoreTableInput{ + password: "abcdef123", + useSentinel: true, + setSentinelAddr: true, + setMasterName: true, + + opts: &options.Options{ + Session: options.SessionOptions{ + Type: options.RedisSessionStoreType, + Redis: options.RedisStoreOptions{ + Password: "zyxwtuv987", + UseSentinel: true, + }, + }, + }, + errStrings: []string{invalidPasswordSetMsg, invalidPasswordDelMsg}, + }), + Entry("failed connection to sentinel redis with wrong master name", &redisStoreTableInput{ + useSentinel: true, + setSentinelAddr: true, + + opts: &options.Options{ + Session: options.SessionOptions{ + Type: options.RedisSessionStoreType, + Redis: options.RedisStoreOptions{ + UseSentinel: true, + SentinelMasterName: "WRONG", + }, + }, + }, + errStrings: []string{unreachableSentinelSetMsg, unrechableSentinelDelMsg}, + }), + Entry("failed connection to sentinel redis with wrong address", &redisStoreTableInput{ + useSentinel: true, + setMasterName: true, + + opts: &options.Options{ + Session: options.SessionOptions{ + Type: options.RedisSessionStoreType, + Redis: options.RedisStoreOptions{ + UseSentinel: true, + SentinelConnectionURLs: []string{"redis://127.0.0.1:65535"}, + }, + }, + }, + errStrings: []string{unreachableSentinelSetMsg, unrechableSentinelDelMsg}, + }), + Entry("sentinel and cluster both enabled fails", &redisStoreTableInput{ + opts: &options.Options{ + Session: options.SessionOptions{ + Type: options.RedisSessionStoreType, + Redis: options.RedisStoreOptions{ + UseSentinel: true, + UseCluster: true, + }, + }, + }, + errStrings: []string{clusterAndSentinelMsg}, + }), + ) +})