d82c268 · typeable/oauth2-proxy

git.iko.soy/typeable/oauth2-proxy

Merge pull request #1403 from openstandia/fix-redis-tls

d82c268Joel Speed5 years ago

CHANGELOG.md+1

@@ -48,6 +48,7 @@

48- [#997](https://github.com/oauth2-proxy/oauth2-proxy/pull/997) Allow passing the raw url path when proxying upstream requests - e.g. /%2F/ (@FStelzer)

49- [#1147](https://github.com/oauth2-proxy/oauth2-proxy/pull/1147) Multiarch support for docker image (@goshlanguage)

50- [#1296](https://github.com/oauth2-proxy/oauth2-proxy/pull/1296) Fixed `panic` when connecting to Redis with TLS (@mstrzele)

51+- [#1403](https://github.com/oauth2-proxy/oauth2-proxy/pull/1403) Improve TLS handling for Redis to support non-standalone mode with TLS (@wadahiro)

52# V7.1.3

53# V7.1.3

pkg/sessions/redis/redis_store.go+34−12

@@ -89,28 +89,40 @@ func NewRedisClient(opts options.RedisStoreOptions) (Client, error) {

89// buildSentinelClient makes a redis.Client that connects to Redis Sentinel

90// for Primary/Replica Redis node coordination

91func buildSentinelClient(opts options.RedisStoreOptions) (Client, error) {

92− addrs, err := parseRedisURLs(opts.SentinelConnectionURLs)

92+ addrs, opt, err := parseRedisURLs(opts.SentinelConnectionURLs)

93 if err != nil {

94 return nil, fmt.Errorf("could not parse redis urls: %v", err)

95 }

96+

97+ if err := setupTLSConfig(opts, opt); err != nil {

98+ return nil, err

99+ }

100+

96 client := redis.NewFailoverClient(&redis.FailoverOptions{

101 client := redis.NewFailoverClient(&redis.FailoverOptions{

97 MasterName: opts.SentinelMasterName,

102 MasterName: opts.SentinelMasterName,

98 SentinelAddrs: addrs,

103 SentinelAddrs: addrs,

99 SentinelPassword: opts.SentinelPassword,

104 SentinelPassword: opts.SentinelPassword,

100 Password: opts.Password,

105 Password: opts.Password,

106+ TLSConfig: opt.TLSConfig,

101 })

107 })

102 return newClient(client), nil

108 return newClient(client), nil

103}

109}

104

110

105// buildClusterClient makes a redis.Client that is Redis Cluster aware

111// buildClusterClient makes a redis.Client that is Redis Cluster aware

106func buildClusterClient(opts options.RedisStoreOptions) (Client, error) {

112func buildClusterClient(opts options.RedisStoreOptions) (Client, error) {

107− addrs, err := parseRedisURLs(opts.ClusterConnectionURLs)

113+ addrs, opt, err := parseRedisURLs(opts.ClusterConnectionURLs)

108 if err != nil {

114 if err != nil {

109 return nil, fmt.Errorf("could not parse redis urls: %v", err)

115 return nil, fmt.Errorf("could not parse redis urls: %v", err)

110 }

116 }

117+

118+ if err := setupTLSConfig(opts, opt); err != nil {

119+ return nil, err

120+ }

121+

111 client := redis.NewClusterClient(&redis.ClusterOptions{

122 client := redis.NewClusterClient(&redis.ClusterOptions{

112− Addrs: addrs,

123+ Addrs: addrs,

113− Password: opts.Password,

124+ Password: opts.Password,

125+ TLSConfig: opt.TLSConfig,

114 })

126 })

115 return newClusterClient(client), nil

127 return newClusterClient(client), nil

116}

128}

@@ -127,6 +139,16 @@ func buildStandaloneClient(opts options.RedisStoreOptions) (Client, error) {

127 opt.Password = opts.Password

139 opt.Password = opts.Password

128 }

140 }

129

141

142+ if err := setupTLSConfig(opts, opt); err != nil {

143+ return nil, err

144+ }

145+

146+ client := redis.NewClient(opt)

147+ return newClient(client), nil

148+}

149+

150+// setupTLSConfig sets the TLSConfig if the TLS option is given in redis.Options

151+func setupTLSConfig(opts options.RedisStoreOptions, opt *redis.Options) error {

130 if opts.InsecureSkipTLSVerify {

152 if opts.InsecureSkipTLSVerify {

131 if opt.TLSConfig == nil {

153 if opt.TLSConfig == nil {

132 /* #nosec */

154 /* #nosec */

@@ -146,7 +168,7 @@ func buildStandaloneClient(opts options.RedisStoreOptions) (Client, error) {

146 }

168 }

147 certs, err := ioutil.ReadFile(opts.CAPath)

169 certs, err := ioutil.ReadFile(opts.CAPath)

148 if err != nil {

170 if err != nil {

149− return nil, fmt.Errorf("failed to load %q, %v", opts.CAPath, err)

171+ return fmt.Errorf("failed to load %q, %v", opts.CAPath, err)

150 }

172 }

151

173

152 // Append our cert to the system pool

174 // Append our cert to the system pool

@@ -161,21 +183,21 @@ func buildStandaloneClient(opts options.RedisStoreOptions) (Client, error) {

161

183

162 opt.TLSConfig.RootCAs = rootCAs

184 opt.TLSConfig.RootCAs = rootCAs

163 }

185 }

164−

165− client := redis.NewClient(opt)

166− return newClient(client), nil

186+ return nil

167}

187}

168

188

169// parseRedisURLs parses a list of redis urls and returns a list

189// parseRedisURLs parses a list of redis urls and returns a list

170−// of addresses in the form of host:port that can be used to connect to Redis

190+// of addresses in the form of host:port and redis.Options that can be used to connect to Redis

171−func parseRedisURLs(urls []string) ([]string, error) {

191+func parseRedisURLs(urls []string) ([]string, *redis.Options, error) {

172 addrs := []string{}

192 addrs := []string{}

193+ var redisOptions *redis.Options

173 for _, u := range urls {

194 for _, u := range urls {

174 parsedURL, err := redis.ParseURL(u)

195 parsedURL, err := redis.ParseURL(u)

175 if err != nil {

196 if err != nil {

176− return nil, fmt.Errorf("unable to parse redis url: %v", err)

197+ return nil, nil, fmt.Errorf("unable to parse redis url: %v", err)

177 }

198 }

178 addrs = append(addrs, parsedURL.Addr)

199 addrs = append(addrs, parsedURL.Addr)

200+ redisOptions = parsedURL

179 }

201 }

180− return addrs, nil

202+ return addrs, redisOptions, nil

181}

203}

pkg/sessions/redis/redis_store_test.go+140−25

@@ -47,6 +47,35 @@ func TestSessionStore(t *testing.T) {

47 RunSpecs(t, "Redis SessionStore")

48}

50+var (

51+ cert tls.Certificate

52+ caPath string

53+)

54+

55+var _ = BeforeSuite(func() {

56+ var err error

57+ certBytes, keyBytes, err := util.GenerateCert()

58+ Expect(err).ToNot(HaveOccurred())

59+ certOut := new(bytes.Buffer)

60+ Expect(pem.Encode(certOut, &pem.Block{Type: "CERTIFICATE", Bytes: certBytes})).To(Succeed())

61+ certData := certOut.Bytes()

62+ keyOut := new(bytes.Buffer)

63+ Expect(pem.Encode(keyOut, &pem.Block{Type: "PRIVATE KEY", Bytes: keyBytes})).To(Succeed())

64+ cert, err = tls.X509KeyPair(certData, keyOut.Bytes())

65+ Expect(err).ToNot(HaveOccurred())

66+

67+ certFile, err := os.CreateTemp("", "cert.*.pem")

68+ Expect(err).ToNot(HaveOccurred())

69+ caPath = certFile.Name()

70+ _, err = certFile.Write(certData)

71+ defer certFile.Close()

72+ Expect(err).ToNot(HaveOccurred())

73+})

74+

75+var _ = AfterSuite(func() {

76+ Expect(os.Remove(caPath)).ToNot(HaveOccurred())

77+})

78+

50var _ = Describe("Redis SessionStore Tests", func() {

79var _ = Describe("Redis SessionStore Tests", func() {

51 // helper interface to allow us to close client connections

80 // helper interface to allow us to close client connections

52 // All non-nil redis clients should implement this

81 // All non-nil redis clients should implement this

@@ -229,36 +258,130 @@ var _ = Describe("Redis SessionStore Tests", func() {

229 })

258 })

230 })

259 })

231

260

232− Context("with custom CA path", func() {

261+ Context("with TLS connection", func() {

233− var caPath string

234−

235 BeforeEach(func() {

262 BeforeEach(func() {

236− certBytes, keyBytes, err := util.GenerateCert()

263+ mr.Close()

264+

265+ var err error

266+ mr, err = miniredis.RunTLS(&tls.Config{Certificates: []tls.Certificate{cert}})

237 Expect(err).ToNot(HaveOccurred())

267 Expect(err).ToNot(HaveOccurred())

238− certOut := new(bytes.Buffer)

268+ })

239− Expect(pem.Encode(certOut, &pem.Block{Type: "CERTIFICATE", Bytes: certBytes})).To(Succeed())

269+ AfterEach(func() {

240− certData := certOut.Bytes()

270+ mr.Close()

241− keyOut := new(bytes.Buffer)

271+

242− Expect(pem.Encode(keyOut, &pem.Block{Type: "PRIVATE KEY", Bytes: keyBytes})).To(Succeed())

272+ var err error

243− cert, err := tls.X509KeyPair(certData, keyOut.Bytes())

273+ mr, err = miniredis.Run()

244 Expect(err).ToNot(HaveOccurred())

274 Expect(err).ToNot(HaveOccurred())

275+ })

276+

277+ Context("with standalone", func() {

278+ tests.RunSessionStoreTests(

279+ func(opts *options.SessionOptions, cookieOpts *options.Cookie) (sessionsapi.SessionStore, error) {

280+ // Set the connection URL

281+ opts.Type = options.RedisSessionStoreType

282+ opts.Redis.ConnectionURL = "rediss://" + mr.Addr()

283+ opts.Redis.CAPath = caPath

284+

285+ // Capture the session store so that we can close the client

286+ ss, err := NewRedisSessionStore(opts, cookieOpts)

287+ return ss, err

288+ },

289+ func(d time.Duration) error {

290+ mr.FastForward(d)

291+ return nil

292+ },

293+ )

294+ })

295+

296+ Context("with cluster", func() {

297+ tests.RunSessionStoreTests(

298+ func(opts *options.SessionOptions, cookieOpts *options.Cookie) (sessionsapi.SessionStore, error) {

299+ clusterAddr := "rediss://" + mr.Addr()

300+ opts.Type = options.RedisSessionStoreType

301+ opts.Redis.ClusterConnectionURLs = []string{clusterAddr}

302+ opts.Redis.UseCluster = true

303+ opts.Redis.CAPath = caPath

304+

305+ // Capture the session store so that we can close the client

306+ var err error

307+ ss, err = NewRedisSessionStore(opts, cookieOpts)

308+ return ss, err

309+ },

310+ func(d time.Duration) error {

311+ mr.FastForward(d)

312+ return nil

313+ },

314+ )

315+ })

316+ })

317+

318+ Context("with insecure TLS connection", func() {

319+ BeforeEach(func() {

320+ mr.Close()

245

321

246− certFile, err := os.CreateTemp("", "cert.*.pem")

322+ var err error

323+ mr, err = miniredis.RunTLS(&tls.Config{Certificates: []tls.Certificate{cert}})

247 Expect(err).ToNot(HaveOccurred())

324 Expect(err).ToNot(HaveOccurred())

248− caPath = certFile.Name()

325+ })

249− _, err = certFile.Write(certData)

326+ AfterEach(func() {

250− defer certFile.Close()

327+ mr.Close()

328+

329+ var err error

330+ mr, err = miniredis.Run()

251 Expect(err).ToNot(HaveOccurred())

331 Expect(err).ToNot(HaveOccurred())

332+ })

333+

334+ Context("with standalone", func() {

335+ tests.RunSessionStoreTests(

336+ func(opts *options.SessionOptions, cookieOpts *options.Cookie) (sessionsapi.SessionStore, error) {

337+ // Set the connection URL

338+ opts.Type = options.RedisSessionStoreType

339+ opts.Redis.ConnectionURL = "rediss://" + mr.Addr()

340+ opts.Redis.InsecureSkipTLSVerify = true

341+

342+ // Capture the session store so that we can close the client

343+ ss, err := NewRedisSessionStore(opts, cookieOpts)

344+ return ss, err

345+ },

346+ func(d time.Duration) error {

347+ mr.FastForward(d)

348+ return nil

349+ },

350+ )

351+ })

352+

353+ Context("with cluster", func() {

354+ tests.RunSessionStoreTests(

355+ func(opts *options.SessionOptions, cookieOpts *options.Cookie) (sessionsapi.SessionStore, error) {

356+ clusterAddr := "rediss://" + mr.Addr()

357+ opts.Type = options.RedisSessionStoreType

358+ opts.Redis.ClusterConnectionURLs = []string{clusterAddr}

359+ opts.Redis.UseCluster = true

360+ opts.Redis.InsecureSkipTLSVerify = true

252

361

362+ // Capture the session store so that we can close the client

363+ var err error

364+ ss, err = NewRedisSessionStore(opts, cookieOpts)

365+ return ss, err

366+ },

367+ func(d time.Duration) error {

368+ mr.FastForward(d)

369+ return nil

370+ },

371+ )

372+ })

373+ })

374+

375+ Context("with custom CA path", func() {

376+ BeforeEach(func() {

253 mr.Close()

377 mr.Close()

254

378

379+ var err error

255 mr, err = miniredis.RunTLS(&tls.Config{Certificates: []tls.Certificate{cert}})

380 mr, err = miniredis.RunTLS(&tls.Config{Certificates: []tls.Certificate{cert}})

256 Expect(err).ToNot(HaveOccurred())

381 Expect(err).ToNot(HaveOccurred())

257 })

382 })

258

383

259 AfterEach(func() {

384 AfterEach(func() {

260− Expect(os.Remove(caPath)).ToNot(HaveOccurred())

261−

262 mr.Close()

385 mr.Close()

263

386

264 var err error

387 var err error

@@ -287,17 +410,9 @@ var _ = Describe("Redis SessionStore Tests", func() {

287

410

288 Context("with insecure TLS connection", func() {

411 Context("with insecure TLS connection", func() {

289 BeforeEach(func() {

412 BeforeEach(func() {

290− certBytes, keyBytes, err := util.GenerateCert()

291− Expect(err).ToNot(HaveOccurred())

292− certOut := new(bytes.Buffer)

293− Expect(pem.Encode(certOut, &pem.Block{Type: "CERTIFICATE", Bytes: certBytes})).To(Succeed())

294− keyOut := new(bytes.Buffer)

295− Expect(pem.Encode(keyOut, &pem.Block{Type: "PRIVATE KEY", Bytes: keyBytes})).To(Succeed())

296− cert, err := tls.X509KeyPair(certOut.Bytes(), keyOut.Bytes())

297− Expect(err).ToNot(HaveOccurred())

298−

299 mr.Close()

413 mr.Close()

300

414

415+ var err error

301 mr, err = miniredis.RunTLS(&tls.Config{Certificates: []tls.Certificate{cert}})

416 mr, err = miniredis.RunTLS(&tls.Config{Certificates: []tls.Certificate{cert}})

302 Expect(err).ToNot(HaveOccurred())

417 Expect(err).ToNot(HaveOccurred())

303 })

418 })