Initalize TLS.Config when connecting to Redis with TLS (oauth2-proxy#…

…1296) * init TLS.Config when connecting to Redis with TLS * don't overwrite TLS config if it exists * add tests for Redis with TLS * remove hardcoded certs * add GenerateCert func * use GenerateCert util func * fix issue reported by go fmt * limit return statements in GenerateCert
dynata · Oct 19, 2021 · b49e62f · b49e62f
1 parent ea261ca
commit b49e62f
Show file tree

Hide file tree

Showing 7 changed files with 167 additions and 33 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -47,6 +47,7 @@
 - [#1357](https://github.com/oauth2-proxy/oauth2-proxy/pull/1357) Fix unsafe access to session variable (@harzallah)
 - [#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)
 
 # V7.1.3
 

diff --git a/pkg/http/http_suite_test.go b/pkg/http/http_suite_test.go
@@ -2,20 +2,15 @@ package http
 
 import (
 	"bytes"
-	"crypto/rand"
-	"crypto/rsa"
 	"crypto/tls"
 	"crypto/x509"
-	"crypto/x509/pkix"
 	"encoding/pem"
-	"math/big"
-	"net"
 	"net/http"
 	"testing"
-	"time"
 
 	"github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/options"
 	"github.com/oauth2-proxy/oauth2-proxy/v7/pkg/logger"
+	"github.com/oauth2-proxy/oauth2-proxy/v7/pkg/util"
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
 )
@@ -34,38 +29,16 @@ func TestHTTPSuite(t *testing.T) {
 
 var _ = BeforeSuite(func() {
 	By("Generating a self-signed cert for TLS tests", func() {
-		priv, err := rsa.GenerateKey(rand.Reader, 2048)
-		Expect(err).ToNot(HaveOccurred())
-
-		keyOut := bytes.NewBuffer(nil)
-		privBytes, err := x509.MarshalPKCS8PrivateKey(priv)
-		Expect(err).ToNot(HaveOccurred())
-		Expect(pem.Encode(keyOut, &pem.Block{Type: "PRIVATE KEY", Bytes: privBytes})).To(Succeed())
-		keyDataSource.Value = keyOut.Bytes()
-
-		serialNumberLimit := new(big.Int).Lsh(big.NewInt(1), 128)
-		serialNumber, err := rand.Int(rand.Reader, serialNumberLimit)
-		Expect(err).ToNot(HaveOccurred())
-
-		template := x509.Certificate{
-			SerialNumber: serialNumber,
-			Subject: pkix.Name{
-				Organization: []string{"OAuth2 Proxy Test Suite"},
-			},
-			NotBefore:   time.Now(),
-			NotAfter:    time.Now().Add(time.Hour),
-			IPAddresses: []net.IP{net.ParseIP("127.0.0.1")},
-			KeyUsage:    x509.KeyUsageDigitalSignature | x509.KeyUsageKeyEncipherment,
-			ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth},
-		}
-
-		certBytes, err := x509.CreateCertificate(rand.Reader, &template, &template, &priv.PublicKey, priv)
+		certBytes, keyBytes, err := util.GenerateCert()
 		Expect(err).ToNot(HaveOccurred())
 		certData = certBytes
 
-		certOut := bytes.NewBuffer(nil)
+		certOut := new(bytes.Buffer)
 		Expect(pem.Encode(certOut, &pem.Block{Type: "CERTIFICATE", Bytes: certBytes})).To(Succeed())
 		certDataSource.Value = certOut.Bytes()
+		keyOut := new(bytes.Buffer)
+		Expect(pem.Encode(keyOut, &pem.Block{Type: "PRIVATE KEY", Bytes: keyBytes})).To(Succeed())
+		keyDataSource.Value = keyOut.Bytes()
 	})
 
 	By("Setting up a http client", func() {

diff --git a/pkg/sessions/redis/redis_store.go b/pkg/sessions/redis/redis_store.go
@@ -2,6 +2,7 @@ package redis
 
 import (
 	"context"
+	"crypto/tls"
 	"crypto/x509"
 	"fmt"
 	"io/ioutil"
@@ -127,6 +128,11 @@ func buildStandaloneClient(opts options.RedisStoreOptions) (Client, error) {
 	}
 
 	if opts.InsecureSkipTLSVerify {
+		if opt.TLSConfig == nil {
+			/* #nosec */
+			opt.TLSConfig = &tls.Config{}
+		}
+
 		opt.TLSConfig.InsecureSkipVerify = true
 	}
 
@@ -148,6 +154,11 @@ func buildStandaloneClient(opts options.RedisStoreOptions) (Client, error) {
 			logger.Errorf("no certs appended, using system certs only")
 		}
 
+		if opt.TLSConfig == nil {
+			/* #nosec */
+			opt.TLSConfig = &tls.Config{}
+		}
+
 		opt.TLSConfig.RootCAs = rootCAs
 	}
 

diff --git a/pkg/sessions/redis/redis_store_test.go b/pkg/sessions/redis/redis_store_test.go
@@ -1,7 +1,10 @@
 package redis
 
 import (
+	"bytes"
 	"context"
+	"crypto/tls"
+	"encoding/pem"
 	"log"
 	"os"
 	"testing"
@@ -15,6 +18,7 @@ import (
 	"github.com/oauth2-proxy/oauth2-proxy/v7/pkg/logger"
 	"github.com/oauth2-proxy/oauth2-proxy/v7/pkg/sessions/persistence"
 	"github.com/oauth2-proxy/oauth2-proxy/v7/pkg/sessions/tests"
+	"github.com/oauth2-proxy/oauth2-proxy/v7/pkg/util"
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
 )
@@ -224,4 +228,104 @@ var _ = Describe("Redis SessionStore Tests", func() {
 			)
 		})
 	})
+
+	Context("with custom CA path", func() {
+		var caPath string
+
+		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()
+
+			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
+			mr, err = miniredis.Run()
+			Expect(err).ToNot(HaveOccurred())
+		})
+
+		tests.RunSessionStoreTests(
+			func(opts *options.SessionOptions, cookieOpts *options.Cookie) (sessionsapi.SessionStore, error) {
+				// Set the connection URL
+				opts.Type = options.RedisSessionStoreType
+				opts.Redis.ConnectionURL = "redis://" + mr.Addr()
+				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() {
+			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()
+
+			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())
+		})
+
+		tests.RunSessionStoreTests(
+			func(opts *options.SessionOptions, cookieOpts *options.Cookie) (sessionsapi.SessionStore, error) {
+				// Set the connection URL
+				opts.Type = options.RedisSessionStoreType
+				opts.Redis.ConnectionURL = "redis://127.0.0.1:" + mr.Port() // func (*Miniredis) StartTLS listens on 127.0.0.1
+				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
+			},
+		)
+	})
 })
diff --git a/pkg/util/util.go b/pkg/util/util.go
@@ -1,9 +1,15 @@
 package util
 
 import (
+	"crypto/rand"
+	"crypto/rsa"
 	"crypto/x509"
+	"crypto/x509/pkix"
 	"fmt"
 	"io/ioutil"
+	"math/big"
+	"net"
+	"time"
 )
 
 func GetCertPool(paths []string) (*x509.CertPool, error) {
@@ -23,3 +29,40 @@ func GetCertPool(paths []string) (*x509.CertPool, error) {
 	}
 	return pool, nil
 }
+
+// https://golang.org/src/crypto/tls/generate_cert.go as a function
+func GenerateCert() ([]byte, []byte, error) {
+	var err error
+
+	priv, err := rsa.GenerateKey(rand.Reader, 2048)
+	if err != nil {
+		return nil, nil, err
+	}
+
+	keyBytes, err := x509.MarshalPKCS8PrivateKey(priv)
+	if err != nil {
+		return nil, keyBytes, err
+	}
+
+	serialNumber, err := rand.Int(rand.Reader, new(big.Int).Lsh(big.NewInt(1), 128))
+	if err != nil {
+		return nil, keyBytes, err
+	}
+
+	notBefore := time.Now()
+	template := x509.Certificate{
+		SerialNumber: serialNumber,
+		Subject: pkix.Name{
+			Organization: []string{"OAuth2 Proxy Test Suite"},
+		},
+		NotBefore: notBefore,
+		NotAfter:  notBefore.Add(time.Hour),
+		KeyUsage:  x509.KeyUsageDigitalSignature | x509.KeyUsageKeyEncipherment,
+
+		ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth},
+
+		IPAddresses: []net.IP{net.ParseIP("127.0.0.1")},
+	}
+	certBytes, err := x509.CreateCertificate(rand.Reader, &template, &template, &priv.PublicKey, priv)
+	return certBytes, keyBytes, err
+}
diff --git a/watcher.go b/watcher.go
@@ -1,3 +1,4 @@
+//go:build go1.3 && !plan9 && !solaris
 // +build go1.3,!plan9,!solaris
 
 package main

diff --git a/watcher_unsupported.go b/watcher_unsupported.go
@@ -1,3 +1,4 @@
+//go:build !go1.3 || plan9 || solaris
 // +build !go1.3 plan9 solaris
 
 package main