Improve TLS handling for Redis to support non-standalone mode with TLS
This commit is contained in:
		
							parent
							
								
									b49e62f9b2
								
							
						
					
					
						commit
						7eb3a4fbd5
					
				|  | @ -48,6 +48,7 @@ | |||
| - [#997](https://github.com/oauth2-proxy/oauth2-proxy/pull/997) Allow passing the raw url path when proxying upstream requests - e.g. /%2F/ (@FStelzer) | ||||
| - [#1147](https://github.com/oauth2-proxy/oauth2-proxy/pull/1147) Multiarch support for docker image (@goshlanguage) | ||||
| - [#1296](https://github.com/oauth2-proxy/oauth2-proxy/pull/1296) Fixed `panic` when connecting to Redis with TLS (@mstrzele) | ||||
| - [#1403](https://github.com/oauth2-proxy/oauth2-proxy/pull/1403) Improve TLS handling for Redis to support non-standalone mode with TLS (@wadahiro) | ||||
| 
 | ||||
| # V7.1.3 | ||||
| 
 | ||||
|  |  | |||
|  | @ -89,28 +89,40 @@ func NewRedisClient(opts options.RedisStoreOptions) (Client, error) { | |||
| // buildSentinelClient makes a redis.Client that connects to Redis Sentinel
 | ||||
| // for Primary/Replica Redis node coordination
 | ||||
| func buildSentinelClient(opts options.RedisStoreOptions) (Client, error) { | ||||
| 	addrs, err := parseRedisURLs(opts.SentinelConnectionURLs) | ||||
| 	addrs, opt, err := parseRedisURLs(opts.SentinelConnectionURLs) | ||||
| 	if err != nil { | ||||
| 		return nil, fmt.Errorf("could not parse redis urls: %v", err) | ||||
| 	} | ||||
| 
 | ||||
| 	if err := setupTLSConfig(opts, opt); err != nil { | ||||
| 		return nil, err | ||||
| 	} | ||||
| 
 | ||||
| 	client := redis.NewFailoverClient(&redis.FailoverOptions{ | ||||
| 		MasterName:       opts.SentinelMasterName, | ||||
| 		SentinelAddrs:    addrs, | ||||
| 		SentinelPassword: opts.SentinelPassword, | ||||
| 		Password:         opts.Password, | ||||
| 		TLSConfig:        opt.TLSConfig, | ||||
| 	}) | ||||
| 	return newClient(client), nil | ||||
| } | ||||
| 
 | ||||
| // buildClusterClient makes a redis.Client that is Redis Cluster aware
 | ||||
| func buildClusterClient(opts options.RedisStoreOptions) (Client, error) { | ||||
| 	addrs, err := parseRedisURLs(opts.ClusterConnectionURLs) | ||||
| 	addrs, opt, err := parseRedisURLs(opts.ClusterConnectionURLs) | ||||
| 	if err != nil { | ||||
| 		return nil, fmt.Errorf("could not parse redis urls: %v", err) | ||||
| 	} | ||||
| 
 | ||||
| 	if err := setupTLSConfig(opts, opt); err != nil { | ||||
| 		return nil, err | ||||
| 	} | ||||
| 
 | ||||
| 	client := redis.NewClusterClient(&redis.ClusterOptions{ | ||||
| 		Addrs:     addrs, | ||||
| 		Password:  opts.Password, | ||||
| 		TLSConfig: opt.TLSConfig, | ||||
| 	}) | ||||
| 	return newClusterClient(client), nil | ||||
| } | ||||
|  | @ -127,6 +139,16 @@ func buildStandaloneClient(opts options.RedisStoreOptions) (Client, error) { | |||
| 		opt.Password = opts.Password | ||||
| 	} | ||||
| 
 | ||||
| 	if err := setupTLSConfig(opts, opt); err != nil { | ||||
| 		return nil, err | ||||
| 	} | ||||
| 
 | ||||
| 	client := redis.NewClient(opt) | ||||
| 	return newClient(client), nil | ||||
| } | ||||
| 
 | ||||
| // setupTLSConfig sets the TLSConfig if the TLS option is given in redis.Options
 | ||||
| func setupTLSConfig(opts options.RedisStoreOptions, opt *redis.Options) error { | ||||
| 	if opts.InsecureSkipTLSVerify { | ||||
| 		if opt.TLSConfig == nil { | ||||
| 			/* #nosec */ | ||||
|  | @ -146,7 +168,7 @@ func buildStandaloneClient(opts options.RedisStoreOptions) (Client, error) { | |||
| 		} | ||||
| 		certs, err := ioutil.ReadFile(opts.CAPath) | ||||
| 		if err != nil { | ||||
| 			return nil, fmt.Errorf("failed to load %q, %v", opts.CAPath, err) | ||||
| 			return fmt.Errorf("failed to load %q, %v", opts.CAPath, err) | ||||
| 		} | ||||
| 
 | ||||
| 		// Append our cert to the system pool
 | ||||
|  | @ -161,21 +183,21 @@ func buildStandaloneClient(opts options.RedisStoreOptions) (Client, error) { | |||
| 
 | ||||
| 		opt.TLSConfig.RootCAs = rootCAs | ||||
| 	} | ||||
| 
 | ||||
| 	client := redis.NewClient(opt) | ||||
| 	return newClient(client), nil | ||||
| 	return nil | ||||
| } | ||||
| 
 | ||||
| // parseRedisURLs parses a list of redis urls and returns a list
 | ||||
| // of addresses in the form of host:port that can be used to connect to Redis
 | ||||
| func parseRedisURLs(urls []string) ([]string, error) { | ||||
| // of addresses in the form of host:port and redis.Options that can be used to connect to Redis
 | ||||
| func parseRedisURLs(urls []string) ([]string, *redis.Options, error) { | ||||
| 	addrs := []string{} | ||||
| 	var redisOptions *redis.Options | ||||
| 	for _, u := range urls { | ||||
| 		parsedURL, err := redis.ParseURL(u) | ||||
| 		if err != nil { | ||||
| 			return nil, fmt.Errorf("unable to parse redis url: %v", err) | ||||
| 			return nil, nil, fmt.Errorf("unable to parse redis url: %v", err) | ||||
| 		} | ||||
| 		addrs = append(addrs, parsedURL.Addr) | ||||
| 		redisOptions = parsedURL | ||||
| 	} | ||||
| 	return addrs, nil | ||||
| 	return addrs, redisOptions, nil | ||||
| } | ||||
|  |  | |||
|  | @ -47,6 +47,35 @@ func TestSessionStore(t *testing.T) { | |||
| 	RunSpecs(t, "Redis SessionStore") | ||||
| } | ||||
| 
 | ||||
| var ( | ||||
| 	cert   tls.Certificate | ||||
| 	caPath string | ||||
| ) | ||||
| 
 | ||||
| var _ = BeforeSuite(func() { | ||||
| 	var err error | ||||
| 	certBytes, keyBytes, err := util.GenerateCert() | ||||
| 	Expect(err).ToNot(HaveOccurred()) | ||||
| 	certOut := new(bytes.Buffer) | ||||
| 	Expect(pem.Encode(certOut, &pem.Block{Type: "CERTIFICATE", Bytes: certBytes})).To(Succeed()) | ||||
| 	certData := certOut.Bytes() | ||||
| 	keyOut := new(bytes.Buffer) | ||||
| 	Expect(pem.Encode(keyOut, &pem.Block{Type: "PRIVATE KEY", Bytes: keyBytes})).To(Succeed()) | ||||
| 	cert, err = tls.X509KeyPair(certData, keyOut.Bytes()) | ||||
| 	Expect(err).ToNot(HaveOccurred()) | ||||
| 
 | ||||
| 	certFile, err := os.CreateTemp("", "cert.*.pem") | ||||
| 	Expect(err).ToNot(HaveOccurred()) | ||||
| 	caPath = certFile.Name() | ||||
| 	_, err = certFile.Write(certData) | ||||
| 	defer certFile.Close() | ||||
| 	Expect(err).ToNot(HaveOccurred()) | ||||
| }) | ||||
| 
 | ||||
| var _ = AfterSuite(func() { | ||||
| 	Expect(os.Remove(caPath)).ToNot(HaveOccurred()) | ||||
| }) | ||||
| 
 | ||||
| var _ = Describe("Redis SessionStore Tests", func() { | ||||
| 	// helper interface to allow us to close client connections
 | ||||
| 	// All non-nil redis clients should implement this
 | ||||
|  | @ -229,36 +258,130 @@ var _ = Describe("Redis SessionStore Tests", func() { | |||
| 		}) | ||||
| 	}) | ||||
| 
 | ||||
| 	Context("with custom CA path", func() { | ||||
| 		var caPath string | ||||
| 
 | ||||
| 	Context("with TLS connection", func() { | ||||
| 		BeforeEach(func() { | ||||
| 			certBytes, keyBytes, err := util.GenerateCert() | ||||
| 			Expect(err).ToNot(HaveOccurred()) | ||||
| 			certOut := new(bytes.Buffer) | ||||
| 			Expect(pem.Encode(certOut, &pem.Block{Type: "CERTIFICATE", Bytes: certBytes})).To(Succeed()) | ||||
| 			certData := certOut.Bytes() | ||||
| 			keyOut := new(bytes.Buffer) | ||||
| 			Expect(pem.Encode(keyOut, &pem.Block{Type: "PRIVATE KEY", Bytes: keyBytes})).To(Succeed()) | ||||
| 			cert, err := tls.X509KeyPair(certData, keyOut.Bytes()) | ||||
| 			Expect(err).ToNot(HaveOccurred()) | ||||
| 
 | ||||
| 			certFile, err := os.CreateTemp("", "cert.*.pem") | ||||
| 			Expect(err).ToNot(HaveOccurred()) | ||||
| 			caPath = certFile.Name() | ||||
| 			_, err = certFile.Write(certData) | ||||
| 			defer certFile.Close() | ||||
| 			Expect(err).ToNot(HaveOccurred()) | ||||
| 
 | ||||
| 			mr.Close() | ||||
| 
 | ||||
| 			var err error | ||||
| 			mr, err = miniredis.RunTLS(&tls.Config{Certificates: []tls.Certificate{cert}}) | ||||
| 			Expect(err).ToNot(HaveOccurred()) | ||||
| 		}) | ||||
| 		AfterEach(func() { | ||||
| 			mr.Close() | ||||
| 
 | ||||
| 			var err error | ||||
| 			mr, err = miniredis.Run() | ||||
| 			Expect(err).ToNot(HaveOccurred()) | ||||
| 		}) | ||||
| 
 | ||||
| 		Context("with standalone", func() { | ||||
| 			tests.RunSessionStoreTests( | ||||
| 				func(opts *options.SessionOptions, cookieOpts *options.Cookie) (sessionsapi.SessionStore, error) { | ||||
| 					// Set the connection URL
 | ||||
| 					opts.Type = options.RedisSessionStoreType | ||||
| 					opts.Redis.ConnectionURL = "rediss://" + mr.Addr() | ||||
| 					opts.Redis.CAPath = caPath | ||||
| 
 | ||||
| 					// Capture the session store so that we can close the client
 | ||||
| 					ss, err := NewRedisSessionStore(opts, cookieOpts) | ||||
| 					return ss, err | ||||
| 				}, | ||||
| 				func(d time.Duration) error { | ||||
| 					mr.FastForward(d) | ||||
| 					return nil | ||||
| 				}, | ||||
| 			) | ||||
| 		}) | ||||
| 
 | ||||
| 		Context("with cluster", func() { | ||||
| 			tests.RunSessionStoreTests( | ||||
| 				func(opts *options.SessionOptions, cookieOpts *options.Cookie) (sessionsapi.SessionStore, error) { | ||||
| 					clusterAddr := "rediss://" + mr.Addr() | ||||
| 					opts.Type = options.RedisSessionStoreType | ||||
| 					opts.Redis.ClusterConnectionURLs = []string{clusterAddr} | ||||
| 					opts.Redis.UseCluster = true | ||||
| 					opts.Redis.CAPath = caPath | ||||
| 
 | ||||
| 					// Capture the session store so that we can close the client
 | ||||
| 					var err error | ||||
| 					ss, err = NewRedisSessionStore(opts, cookieOpts) | ||||
| 					return ss, err | ||||
| 				}, | ||||
| 				func(d time.Duration) error { | ||||
| 					mr.FastForward(d) | ||||
| 					return nil | ||||
| 				}, | ||||
| 			) | ||||
| 		}) | ||||
| 	}) | ||||
| 
 | ||||
| 	Context("with insecure TLS connection", func() { | ||||
| 		BeforeEach(func() { | ||||
| 			mr.Close() | ||||
| 
 | ||||
| 			var err error | ||||
| 			mr, err = miniredis.RunTLS(&tls.Config{Certificates: []tls.Certificate{cert}}) | ||||
| 			Expect(err).ToNot(HaveOccurred()) | ||||
| 		}) | ||||
| 		AfterEach(func() { | ||||
| 			mr.Close() | ||||
| 
 | ||||
| 			var err error | ||||
| 			mr, err = miniredis.Run() | ||||
| 			Expect(err).ToNot(HaveOccurred()) | ||||
| 		}) | ||||
| 
 | ||||
| 		Context("with standalone", func() { | ||||
| 			tests.RunSessionStoreTests( | ||||
| 				func(opts *options.SessionOptions, cookieOpts *options.Cookie) (sessionsapi.SessionStore, error) { | ||||
| 					// Set the connection URL
 | ||||
| 					opts.Type = options.RedisSessionStoreType | ||||
| 					opts.Redis.ConnectionURL = "rediss://" + mr.Addr() | ||||
| 					opts.Redis.InsecureSkipTLSVerify = true | ||||
| 
 | ||||
| 					// Capture the session store so that we can close the client
 | ||||
| 					ss, err := NewRedisSessionStore(opts, cookieOpts) | ||||
| 					return ss, err | ||||
| 				}, | ||||
| 				func(d time.Duration) error { | ||||
| 					mr.FastForward(d) | ||||
| 					return nil | ||||
| 				}, | ||||
| 			) | ||||
| 		}) | ||||
| 
 | ||||
| 		Context("with cluster", func() { | ||||
| 			tests.RunSessionStoreTests( | ||||
| 				func(opts *options.SessionOptions, cookieOpts *options.Cookie) (sessionsapi.SessionStore, error) { | ||||
| 					clusterAddr := "rediss://" + mr.Addr() | ||||
| 					opts.Type = options.RedisSessionStoreType | ||||
| 					opts.Redis.ClusterConnectionURLs = []string{clusterAddr} | ||||
| 					opts.Redis.UseCluster = true | ||||
| 					opts.Redis.InsecureSkipTLSVerify = true | ||||
| 
 | ||||
| 					// Capture the session store so that we can close the client
 | ||||
| 					var err error | ||||
| 					ss, err = NewRedisSessionStore(opts, cookieOpts) | ||||
| 					return ss, err | ||||
| 				}, | ||||
| 				func(d time.Duration) error { | ||||
| 					mr.FastForward(d) | ||||
| 					return nil | ||||
| 				}, | ||||
| 			) | ||||
| 		}) | ||||
| 	}) | ||||
| 
 | ||||
| 	Context("with custom CA path", func() { | ||||
| 		BeforeEach(func() { | ||||
| 			mr.Close() | ||||
| 
 | ||||
| 			var err error | ||||
| 			mr, err = miniredis.RunTLS(&tls.Config{Certificates: []tls.Certificate{cert}}) | ||||
| 			Expect(err).ToNot(HaveOccurred()) | ||||
| 		}) | ||||
| 
 | ||||
| 		AfterEach(func() { | ||||
| 			Expect(os.Remove(caPath)).ToNot(HaveOccurred()) | ||||
| 
 | ||||
| 			mr.Close() | ||||
| 
 | ||||
| 			var err error | ||||
|  | @ -287,17 +410,9 @@ var _ = Describe("Redis SessionStore Tests", func() { | |||
| 
 | ||||
| 	Context("with insecure TLS connection", func() { | ||||
| 		BeforeEach(func() { | ||||
| 			certBytes, keyBytes, err := util.GenerateCert() | ||||
| 			Expect(err).ToNot(HaveOccurred()) | ||||
| 			certOut := new(bytes.Buffer) | ||||
| 			Expect(pem.Encode(certOut, &pem.Block{Type: "CERTIFICATE", Bytes: certBytes})).To(Succeed()) | ||||
| 			keyOut := new(bytes.Buffer) | ||||
| 			Expect(pem.Encode(keyOut, &pem.Block{Type: "PRIVATE KEY", Bytes: keyBytes})).To(Succeed()) | ||||
| 			cert, err := tls.X509KeyPair(certOut.Bytes(), keyOut.Bytes()) | ||||
| 			Expect(err).ToNot(HaveOccurred()) | ||||
| 
 | ||||
| 			mr.Close() | ||||
| 
 | ||||
| 			var err error | ||||
| 			mr, err = miniredis.RunTLS(&tls.Config{Certificates: []tls.Certificate{cert}}) | ||||
| 			Expect(err).ToNot(HaveOccurred()) | ||||
| 		}) | ||||
|  |  | |||
		Loading…
	
		Reference in New Issue