From abe2dd0d17bafec576de4cf42fb5255bccdc90b1 Mon Sep 17 00:00:00 2001 From: Michael Katzenellenbogen Date: Sun, 9 Feb 2025 12:24:04 -0500 Subject: [PATCH 1/6] reloadable server tls cert --- pkg/http/server.go | 86 ++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 75 insertions(+), 11 deletions(-) diff --git a/pkg/http/server.go b/pkg/http/server.go index fe76427a..df066f74 100644 --- a/pkg/http/server.go +++ b/pkg/http/server.go @@ -8,7 +8,10 @@ import ( "net" "net/http" "os" + "os/signal" "strings" + "sync" + "syscall" "time" "golang.org/x/sync/errgroup" @@ -142,11 +145,12 @@ func (s *server) setupTLSListener(opts Opts) error { if opts.TLS == nil { return errors.New("no TLS config provided") } - cert, err := getCertificate(opts.TLS) + + l, err := getCertificateLoader(opts.TLS) if err != nil { return fmt.Errorf("could not load certificate: %v", err) } - config.Certificates = []tls.Certificate{cert} + config.GetCertificate = l.GetCertificate if len(opts.TLS.CipherSuites) > 0 { cipherSuites, err := parseCipherSuites(opts.TLS.CipherSuites) @@ -174,7 +178,11 @@ func (s *server) setupTLSListener(opts Opts) error { return fmt.Errorf("listen (%s) failed: %v", listenAddr, err) } - s.tlsListener = tls.NewListener(tcpKeepAliveListener{listener.(*net.TCPListener)}, config) + ka := tcpKeepAliveListener{listener.(*net.TCPListener)} + s.tlsListener = reloadableTLSListener{ + Listener: tls.NewListener(ka, config), + loader: l, + } return nil } @@ -194,6 +202,23 @@ func (s *server) Start(ctx context.Context) error { } if s.tlsListener != nil { + rl := s.tlsListener.(reloadableTLSListener) + ch := make(chan os.Signal, 1) + + g.Go(func() error { + for { + select { + case <-ch: + if err := rl.Reload(); err != nil { + logger.Errorf("Error reloading TLS certificate: %v", err) + } + case <-ctx.Done(): + return nil + } + } + }) + signal.Notify(ch, syscall.SIGHUP) + g.Go(func() error { if err := s.startServer(groupCtx, s.tlsListener); err != nil { return fmt.Errorf("error starting secure server: %v", err) @@ -253,24 +278,63 @@ func getListenAddress(addr string) string { return slice[len(slice)-1] } -// getCertificate loads the certificate data from the TLS config. -func getCertificate(opts *options.TLS) (tls.Certificate, error) { - keyData, err := getSecretValue(opts.Key) +type tlsLoader struct { + *options.TLS + + mu sync.Mutex + cert *tls.Certificate +} + +func (t *tlsLoader) LoadCert() error { + t.mu.Lock() + defer t.mu.Unlock() + + keyData, err := getSecretValue(t.Key) if err != nil { - return tls.Certificate{}, fmt.Errorf("could not load key data: %v", err) + return fmt.Errorf("could not load key data: %v", err) } - certData, err := getSecretValue(opts.Cert) + certData, err := getSecretValue(t.Cert) if err != nil { - return tls.Certificate{}, fmt.Errorf("could not load cert data: %v", err) + return fmt.Errorf("could not load cert data: %v", err) } cert, err := tls.X509KeyPair(certData, keyData) if err != nil { - return tls.Certificate{}, fmt.Errorf("could not parse certificate data: %v", err) + return fmt.Errorf("could not parse certificate data: %v", err) } - return cert, nil + t.cert = &cert + return nil +} + +func (t *tlsLoader) GetCertificate(info *tls.ClientHelloInfo) (*tls.Certificate, error) { + if t.cert == nil { + return nil, fmt.Errorf("no certificate") + } + + return t.cert, nil +} + +func getCertificateLoader(opts *options.TLS) (*tlsLoader, error) { + l := &tlsLoader{ + TLS: opts, + } + + if err := l.LoadCert(); err != nil { + return nil, err + } + + return l, nil +} + +type reloadableTLSListener struct { + net.Listener + loader *tlsLoader +} + +func (rl reloadableTLSListener) Reload() error { + return rl.loader.LoadCert() } // getSecretValue wraps util.GetSecretValue so that we can return an error if no From 1f22825e50b993df2bf79857d40de3fbfdc55a34 Mon Sep 17 00:00:00 2001 From: Michael Katzenellenbogen Date: Sun, 9 Feb 2025 12:30:56 -0500 Subject: [PATCH 2/6] make linter happy --- pkg/http/server.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/http/server.go b/pkg/http/server.go index df066f74..844554b9 100644 --- a/pkg/http/server.go +++ b/pkg/http/server.go @@ -308,7 +308,7 @@ func (t *tlsLoader) LoadCert() error { return nil } -func (t *tlsLoader) GetCertificate(info *tls.ClientHelloInfo) (*tls.Certificate, error) { +func (t *tlsLoader) GetCertificate(_ *tls.ClientHelloInfo) (*tls.Certificate, error) { if t.cert == nil { return nil, fmt.Errorf("no certificate") } From 674bbe8224f01b537293b4d3740915928e6607f5 Mon Sep 17 00:00:00 2001 From: Jan Larwig Date: Tue, 19 Aug 2025 08:18:10 +0200 Subject: [PATCH 3/6] apply code review suggestions Signed-off-by: Jan Larwig --- CHANGELOG.md | 1 + pkg/http/server.go | 26 +++++++++++++------------- 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f2c9b441..855c0096 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,7 @@ - [#3166](https://github.com/oauth2-proxy/oauth2-proxy/pull/3166) chore(dep): upgrade to latest golang 1.24.6 (@tuunit) - [#3156](https://github.com/oauth2-proxy/oauth2-proxy/pull/3156) feat: allow disable-keep-alives configuration for upstream (@jet-go) - [#3150](https://github.com/oauth2-proxy/oauth2-proxy/pull/3150) fix: Gitea team membership (@MagicRB, @tuunit) +- [#2953](https://github.com/oauth2-proxy/oauth2-proxy/pull/2953) feat: reloadable server TLS certificate (@emsixteeen) # V7.11.0 diff --git a/pkg/http/server.go b/pkg/http/server.go index 844554b9..e096c8f1 100644 --- a/pkg/http/server.go +++ b/pkg/http/server.go @@ -146,11 +146,11 @@ func (s *server) setupTLSListener(opts Opts) error { return errors.New("no TLS config provided") } - l, err := getCertificateLoader(opts.TLS) + loader, err := getCertificateLoader(opts.TLS) if err != nil { return fmt.Errorf("could not load certificate: %v", err) } - config.GetCertificate = l.GetCertificate + config.GetCertificate = loader.GetCertificate if len(opts.TLS.CipherSuites) > 0 { cipherSuites, err := parseCipherSuites(opts.TLS.CipherSuites) @@ -178,10 +178,12 @@ func (s *server) setupTLSListener(opts Opts) error { return fmt.Errorf("listen (%s) failed: %v", listenAddr, err) } - ka := tcpKeepAliveListener{listener.(*net.TCPListener)} s.tlsListener = reloadableTLSListener{ - Listener: tls.NewListener(ka, config), - loader: l, + Listener: tls.NewListener( + tcpKeepAliveListener{listener.(*net.TCPListener)}, + config, + ), + loader: loader, } return nil } @@ -202,14 +204,14 @@ func (s *server) Start(ctx context.Context) error { } if s.tlsListener != nil { - rl := s.tlsListener.(reloadableTLSListener) + listener := s.tlsListener.(reloadableTLSListener) ch := make(chan os.Signal, 1) - + signal.Notify(ch, syscall.SIGHUP) g.Go(func() error { for { select { case <-ch: - if err := rl.Reload(); err != nil { + if err := listener.Reload(); err != nil { logger.Errorf("Error reloading TLS certificate: %v", err) } case <-ctx.Done(): @@ -217,8 +219,6 @@ func (s *server) Start(ctx context.Context) error { } } }) - signal.Notify(ch, syscall.SIGHUP) - g.Go(func() error { if err := s.startServer(groupCtx, s.tlsListener); err != nil { return fmt.Errorf("error starting secure server: %v", err) @@ -317,15 +317,15 @@ func (t *tlsLoader) GetCertificate(_ *tls.ClientHelloInfo) (*tls.Certificate, er } func getCertificateLoader(opts *options.TLS) (*tlsLoader, error) { - l := &tlsLoader{ + loader := &tlsLoader{ TLS: opts, } - if err := l.LoadCert(); err != nil { + if err := loader.LoadCert(); err != nil { return nil, err } - return l, nil + return loader, nil } type reloadableTLSListener struct { From f3e255304342d7f08c7a5f34d3b739ce11b20deb Mon Sep 17 00:00:00 2001 From: Michael Katzenellenbogen Date: Tue, 19 Aug 2025 18:00:48 -0400 Subject: [PATCH 4/6] refactor how certificates are generated, so that a new one can be created within tests --- pkg/http/http_suite_test.go | 78 ++++++++++++++++++++++++------------- 1 file changed, 51 insertions(+), 27 deletions(-) diff --git a/pkg/http/http_suite_test.go b/pkg/http/http_suite_test.go index 19d4d3ff..43d0bc48 100644 --- a/pkg/http/http_suite_test.go +++ b/pkg/http/http_suite_test.go @@ -16,6 +16,7 @@ import ( . "github.com/onsi/gomega" ) +var ipv4Addr, ipv6Addr = "127.0.0.1", "::1" var ipv4CertData, ipv6CertData []byte var ipv4CertDataSource, ipv4KeyDataSource options.SecretSource var ipv6CertDataSource, ipv6KeyDataSource options.SecretSource @@ -40,49 +41,72 @@ func httpGet(ctx context.Context, url string) (*http.Response, error) { return c.Do(req) } +func generateCert(ipaddr string) (certData, certOutBytes, keyOutBytes []byte, err error) { + certBytes, keyBytes, err := util.GenerateCert(ipaddr) + if err != nil { + return + } + certData = certBytes + + certOut := new(bytes.Buffer) + if err = pem.Encode(certOut, &pem.Block{Type: "CERTIFICATE", Bytes: certBytes}); err != nil { + return + } + certOutBytes = certOut.Bytes() + + keyOut := new(bytes.Buffer) + if err = pem.Encode(keyOut, &pem.Block{Type: "PRIVATE KEY", Bytes: keyBytes}); err != nil { + return + } + keyOutBytes = keyOut.Bytes() + + return +} + +func generateX509Cert(certSource, keySource options.SecretSource) (*x509.Certificate, error) { + cert, err := tls.X509KeyPair(certSource.Value, keySource.Value) + if err != nil { + return nil, err + } + + certificate, err := x509.ParseCertificate(cert.Certificate[0]) + if err != nil { + return nil, err + } + + return certificate, nil +} + +func addCertToTransportRootCAs(transport *http.Transport, cert ...*x509.Certificate) { + transport.TLSClientConfig.RootCAs = x509.NewCertPool() + for _, c := range cert { + transport.TLSClientConfig.RootCAs.AddCert(c) + } +} + var _ = BeforeSuite(func() { By("Generating a ipv4 self-signed cert for TLS tests", func() { - certBytes, keyBytes, err := util.GenerateCert("127.0.0.1") + ipv4Cert, ipv4CertBytes, ipv4KeyBytes, err := generateCert(ipv4Addr) Expect(err).ToNot(HaveOccurred()) - ipv4CertData = certBytes - certOut := new(bytes.Buffer) - Expect(pem.Encode(certOut, &pem.Block{Type: "CERTIFICATE", Bytes: certBytes})).To(Succeed()) - ipv4CertDataSource.Value = certOut.Bytes() - keyOut := new(bytes.Buffer) - Expect(pem.Encode(keyOut, &pem.Block{Type: "PRIVATE KEY", Bytes: keyBytes})).To(Succeed()) - ipv4KeyDataSource.Value = keyOut.Bytes() + ipv4CertData, ipv4CertDataSource.Value, ipv4KeyDataSource.Value = ipv4Cert, ipv4CertBytes, ipv4KeyBytes }) By("Generating a ipv6 self-signed cert for TLS tests", func() { - certBytes, keyBytes, err := util.GenerateCert("::1") + ipv6Cert, ipv6CertBytes, ipv6KeyBytes, err := generateCert(ipv6Addr) Expect(err).ToNot(HaveOccurred()) - ipv6CertData = certBytes - certOut := new(bytes.Buffer) - Expect(pem.Encode(certOut, &pem.Block{Type: "CERTIFICATE", Bytes: certBytes})).To(Succeed()) - ipv6CertDataSource.Value = certOut.Bytes() - keyOut := new(bytes.Buffer) - Expect(pem.Encode(keyOut, &pem.Block{Type: "PRIVATE KEY", Bytes: keyBytes})).To(Succeed()) - ipv6KeyDataSource.Value = keyOut.Bytes() + ipv6CertData, ipv6CertDataSource.Value, ipv6KeyDataSource.Value = ipv6Cert, ipv6CertBytes, ipv6KeyBytes }) By("Setting up a http client", func() { - ipv4cert, err := tls.X509KeyPair(ipv4CertDataSource.Value, ipv4KeyDataSource.Value) - Expect(err).ToNot(HaveOccurred()) - ipv6cert, err := tls.X509KeyPair(ipv6CertDataSource.Value, ipv6KeyDataSource.Value) + ipv4certificate, err := generateX509Cert(ipv4CertDataSource, ipv4KeyDataSource) Expect(err).ToNot(HaveOccurred()) - ipv4certificate, err := x509.ParseCertificate(ipv4cert.Certificate[0]) + ipv6certificate, err := generateX509Cert(ipv6CertDataSource, ipv6KeyDataSource) Expect(err).ToNot(HaveOccurred()) - ipv6certificate, err := x509.ParseCertificate(ipv6cert.Certificate[0]) - Expect(err).ToNot(HaveOccurred()) - - certpool := x509.NewCertPool() - certpool.AddCert(ipv4certificate) - certpool.AddCert(ipv6certificate) transport = http.DefaultTransport.(*http.Transport).Clone() - transport.TLSClientConfig.RootCAs = certpool + addCertToTransportRootCAs(transport, ipv4certificate, ipv6certificate) }) }) From 876dadf39719ceed5699f0aac420cf8f6df1ac36 Mon Sep 17 00:00:00 2001 From: Michael Katzenellenbogen Date: Tue, 19 Aug 2025 18:01:40 -0400 Subject: [PATCH 5/6] add a test case to regenerate and reload a certificate --- pkg/http/server_test.go | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/pkg/http/server_test.go b/pkg/http/server_test.go index 8dfa13af..a27b9485 100644 --- a/pkg/http/server_test.go +++ b/pkg/http/server_test.go @@ -8,6 +8,7 @@ import ( "net" "net/http" "os" + "syscall" "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/options" . "github.com/onsi/ginkgo/v2" @@ -835,6 +836,38 @@ var _ = Describe("Server", func() { Expect(resp.TLS.VerifiedChains[0]).Should(HaveLen(1)) Expect(resp.TLS.VerifiedChains[0][0].Raw).Should(Equal(ipv4CertData)) }) + + It("Reloads the certificate on SIGHUP", func() { + go func() { + defer GinkgoRecover() + Expect(srv.Start(ctx)).To(Succeed()) + }() + + var err error + + ipv4CertData, ipv4CertDataSource.Value, ipv4KeyDataSource.Value, err = generateCert(ipv4Addr) + Expect(err).ToNot(HaveOccurred()) + ipv6CertData, ipv6CertDataSource.Value, ipv6KeyDataSource.Value, err = generateCert(ipv6Addr) + Expect(err).ToNot(HaveOccurred()) + + ipv4Certificate, err := generateX509Cert(ipv4CertDataSource, ipv4KeyDataSource) + Expect(err).ToNot(HaveOccurred()) + ipv6Certificate, err := generateX509Cert(ipv6CertDataSource, ipv6KeyDataSource) + Expect(err).ToNot(HaveOccurred()) + + addCertToTransportRootCAs(transport, ipv4Certificate, ipv6Certificate) + + err = syscall.Kill(syscall.Getpid(), syscall.SIGHUP) + Expect(err).ToNot(HaveOccurred()) + + resp, err := httpGet(ctx, secureListenAddr) + Expect(err).ToNot(HaveOccurred()) + Expect(resp.StatusCode).To(Equal(http.StatusOK)) + + Expect(resp.TLS.VerifiedChains).Should(HaveLen(1)) + Expect(resp.TLS.VerifiedChains[0]).Should(HaveLen(1)) + Expect(resp.TLS.VerifiedChains[0][0].Raw).Should(Equal(ipv4CertData)) + }) }) Context("with a fd ipv4 http and an ipv4 https server", func() { From befd7e858850d139289b7f635d7eb95a141aa3c6 Mon Sep 17 00:00:00 2001 From: Michael Katzenellenbogen Date: Tue, 19 Aug 2025 18:15:35 -0400 Subject: [PATCH 6/6] brief documentation on TLS cert and key reloading --- docs/docs/configuration/tls.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/docs/configuration/tls.md b/docs/docs/configuration/tls.md index 68344b22..cea5e1fe 100644 --- a/docs/docs/configuration/tls.md +++ b/docs/docs/configuration/tls.md @@ -36,6 +36,9 @@ There are two recommended configurations: If not specified, the defaults from [`crypto/tls`](https://pkg.go.dev/crypto/tls#CipherSuites) of the currently used `go` version for building `oauth2-proxy` will be used. A complete list of valid TLS cipher suite names can be found in [`crypto/tls`](https://pkg.go.dev/crypto/tls#pkg-constants). +3. The TLS server certificate and key can be reloaded without restarting `oauth2-proxy` by sending a `SIGHUP` to a running `oauth2-proxy` process. + If the `oauth2-proxy` server encounters a failure while reloading the certificate or key, the existing certificate and key will remain unchanged and an error will be logged. + ### Terminate TLS at Reverse Proxy, e.g. Nginx 1. Configure SSL Termination with [Nginx](http://nginx.org/) (example config below), Amazon ELB, Google Cloud Platform Load Balancing, or ...