From 8075e8d2a3ec27527d43bdf181283005fefc8b7d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antoine=20Desch=C3=AAnes?= Date: Mon, 5 Oct 2026 17:49:35 -0400 Subject: [PATCH] feat: reload TLS certificates when their files change (#625) --- doc/config.md | 8 + pkg/server/pop3/listener.go | 6 +- pkg/server/pop3/transaction_test.go | 63 +++++++ pkg/server/smtp/listener.go | 6 +- pkg/server/tlscert/reloader.go | 116 +++++++++++++ pkg/server/tlscert/reloader_test.go | 254 ++++++++++++++++++++++++++++ 6 files changed, 447 insertions(+), 6 deletions(-) create mode 100644 pkg/server/tlscert/reloader.go create mode 100644 pkg/server/tlscert/reloader_test.go diff --git a/doc/config.md b/doc/config.md index f555d5a..531cb24 100644 --- a/doc/config.md +++ b/doc/config.md @@ -243,6 +243,14 @@ to the public internet. Enable the STARTTLS option for opportunistic TLS support +Inbucket checks the certificate and private key files on each full TLS handshake +and reloads them when either file changes, so renewed certificates (for example +from cert-manager) take effect without a restart. Clients that resume an +earlier TLS session keep that session's certificate until their next full +handshake. If a reload fails, Inbucket logs a warning, keeps serving the +previous certificate, and tries again when the files change or after 30 seconds. +The same applies to the POP3 TLS files. + - Default: `false` - Values: `true` or `false` diff --git a/pkg/server/pop3/listener.go b/pkg/server/pop3/listener.go index ba30242..5254f89 100644 --- a/pkg/server/pop3/listener.go +++ b/pkg/server/pop3/listener.go @@ -9,6 +9,7 @@ import ( "time" "github.com/inbucket/inbucket/v3/pkg/config" + "github.com/inbucket/inbucket/v3/pkg/server/tlscert" "github.com/inbucket/inbucket/v3/pkg/storage" "github.com/rs/zerolog/log" ) @@ -28,14 +29,13 @@ func NewServer(pop3Config config.POP3, store storage.Store) (*Server, error) { slog := log.With().Str("module", "pop3").Str("phase", "tls").Logger() tlsConfig := &tls.Config{} if pop3Config.TLSEnabled { - var err error - tlsConfig.Certificates = make([]tls.Certificate, 1) - tlsConfig.Certificates[0], err = tls.LoadX509KeyPair(pop3Config.TLSCert, pop3Config.TLSPrivKey) + reloader, err := tlscert.NewReloader(pop3Config.TLSCert, pop3Config.TLSPrivKey, slog) if err != nil { slog.Error().Msgf("Failed loading X509 KeyPair: %v", err) return nil, fmt.Errorf("failed to configure TLS; %v", err) // Do not silently turn off Security. } + tlsConfig.GetCertificate = reloader.GetCertificate slog.Debug().Msg("TLS config available") } else { tlsConfig = nil diff --git a/pkg/server/pop3/transaction_test.go b/pkg/server/pop3/transaction_test.go index 2f9fa38..fc16a5a 100644 --- a/pkg/server/pop3/transaction_test.go +++ b/pkg/server/pop3/transaction_test.go @@ -3,12 +3,15 @@ package pop3 import ( "bufio" "context" + "crypto/tls" "errors" "fmt" "io" "net" "net/mail" "net/textproto" + "os" + "path/filepath" "slices" "strings" "testing" @@ -765,6 +768,66 @@ func TestNewServerCertificateError(t *testing.T) { require.Error(t, err) } +// TestForceTLSServesRotatedCertificate verifies that replacing the certificate files on disk +// changes the certificate presented by later full handshakes, without restarting the server, +// while a client resuming an earlier TLS session keeps that session's certificate. +func TestForceTLSServesRotatedCertificate(t *testing.T) { + dir := t.TempDir() + certPath, keyPath := filepath.Join(dir, "cert.pem"), filepath.Join(dir, "key.pem") + writeKeyPair := func(version int) []byte { + cert, privKey, err := generateCertificate(t) + require.NoError(t, err) + mtime := time.Date(2026, 1, 1, 0, 0, version, 0, time.UTC) + require.NoError(t, os.WriteFile(certPath, certToPem(cert), 0o600)) + require.NoError(t, os.WriteFile(keyPath, privKeyToPem(privKey), 0o600)) + require.NoError(t, os.Chtimes(certPath, mtime, mtime)) + require.NoError(t, os.Chtimes(keyPath, mtime, mtime)) + return cert + } + first := writeKeyPair(1) + + server, err := NewServer(config.POP3{ + Addr: "127.0.0.1:0", + Domain: "inbucket.local", + Timeout: 5 * time.Second, + ForceTLS: true, + TLSEnabled: true, + TLSCert: certPath, + TLSPrivKey: keyPath, + }, test.NewStore()) + require.NoError(t, err) + t.Cleanup(server.Drain) + + // peerCertificate starts a session and returns the certificate the server presents, and + // whether the TLS session was resumed. + peerCertificate := func(tlsConfig *tls.Config) ([]byte, bool) { + tlsConn := tls.Client(setupPOPSession(t, server), tlsConfig) + defer func() { _ = tlsConn.Close() }() + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + require.NoError(t, tlsConn.HandshakeContext(ctx)) + // Read the greeting so the server is waiting on input when the client closes. Reading + // also processes the session ticket the server sends after the handshake. + readPOP3Greeting(t, textproto.NewConn(tlsConn)) + state := tlsConn.ConnectionState() + return state.PeerCertificates[0].Raw, state.DidResume + } + fresh := &tls.Config{InsecureSkipVerify: true} + caching := &tls.Config{InsecureSkipVerify: true, ClientSessionCache: tls.NewLRUClientSessionCache(1)} + + cert, _ := peerCertificate(fresh) + assert.Equal(t, first, cert) + cert, _ = peerCertificate(caching) + assert.Equal(t, first, cert) + + second := writeKeyPair(2) + cert, _ = peerCertificate(fresh) + assert.Equal(t, second, cert, "full handshake after rotation") + cert, resumed := peerCertificate(caching) + assert.True(t, resumed, "caching client should resume its session") + assert.Equal(t, first, cert, "resumed session after rotation") +} + // TestValidateMsgNum exercises the shared message-number validation helper // directly. func TestValidateMsgNum(t *testing.T) { diff --git a/pkg/server/smtp/listener.go b/pkg/server/smtp/listener.go index 8301767..b2909a9 100644 --- a/pkg/server/smtp/listener.go +++ b/pkg/server/smtp/listener.go @@ -14,6 +14,7 @@ import ( "github.com/inbucket/inbucket/v3/pkg/message" "github.com/inbucket/inbucket/v3/pkg/metric" "github.com/inbucket/inbucket/v3/pkg/policy" + "github.com/inbucket/inbucket/v3/pkg/server/tlscert" "github.com/rs/zerolog/log" ) @@ -79,14 +80,13 @@ func NewServer( slog := log.With().Str("module", "smtp").Str("phase", "tls").Logger() tlsConfig := &tls.Config{} if smtpConfig.TLSEnabled { - var err error - tlsConfig.Certificates = make([]tls.Certificate, 1) - tlsConfig.Certificates[0], err = tls.LoadX509KeyPair(smtpConfig.TLSCert, smtpConfig.TLSPrivKey) + reloader, err := tlscert.NewReloader(smtpConfig.TLSCert, smtpConfig.TLSPrivKey, slog) if err != nil { slog.Error().Msgf("Failed loading X509 KeyPair: %v", err) slog.Error().Msg("Disabling STARTTLS support") smtpConfig.TLSEnabled = false } else { + tlsConfig.GetCertificate = reloader.GetCertificate slog.Debug().Msg("STARTTLS feature available") } } diff --git a/pkg/server/tlscert/reloader.go b/pkg/server/tlscert/reloader.go new file mode 100644 index 0000000..e4f90d2 --- /dev/null +++ b/pkg/server/tlscert/reloader.go @@ -0,0 +1,116 @@ +// Package tlscert provides X509 key pairs that follow changes to their files on disk. +package tlscert + +import ( + "crypto/tls" + "fmt" + "os" + "sync" + "time" + + "github.com/rs/zerolog" +) + +// retryInterval is the minimum time between attempts to reload a key pair that failed to load +// while its files are unchanged, such as when a chmod or chown fixes its permissions. +const retryInterval = 30 * time.Second + +// fileState identifies a version of a file by its modification time and size. +type fileState struct { + modTime time.Time + size int64 +} + +// Reloader serves a certificate and private key loaded from files, reloading them when either +// file changes. It is safe for concurrent use. +type Reloader struct { + certFile string // X509 public certificate file. + keyFile string // X509 private key file. + logger zerolog.Logger // Logger for reload events. + retryInterval time.Duration // Minimum time between retries of a failed, unchanged pair. + now func() time.Time // Clock used to schedule retries. + + mu sync.Mutex // Guards the fields below. + cert *tls.Certificate // Last successfully loaded key pair. + certState fileState // State of certFile at the last load attempt. + keyState fileState // State of keyFile at the last load attempt. + failedAt time.Time // Time of the last failed load attempt, zero after a success. +} + +// NewReloader loads the key pair from certFile and keyFile, returning an error if it cannot be +// loaded. +func NewReloader(certFile, keyFile string, logger zerolog.Logger) (*Reloader, error) { + r := &Reloader{ + certFile: certFile, + keyFile: keyFile, + logger: logger, + retryInterval: retryInterval, + now: time.Now, + } + certState, keyState, err := r.stat() + if err != nil { + return nil, err + } + cert, err := tls.LoadX509KeyPair(certFile, keyFile) + if err != nil { + return nil, err + } + r.cert = &cert + r.certState = certState + r.keyState = keyState + return r, nil +} + +// GetCertificate returns the current key pair, reloading it first if either file changed since +// the last load attempt, or if the last attempt failed and retryInterval has elapsed. Failures +// are logged and the previous key pair is served. crypto/tls calls it on full handshakes only. +func (r *Reloader) GetCertificate(*tls.ClientHelloInfo) (*tls.Certificate, error) { + r.mu.Lock() + defer r.mu.Unlock() + + // On error, stat returns zero states, so repeated stat failures count as unchanged files. + certState, keyState, err := r.stat() + changed := certState != r.certState || keyState != r.keyState + retryDue := !r.failedAt.IsZero() && r.now().Sub(r.failedAt) >= r.retryInterval + if !changed && !retryDue { + return r.cert, nil + } + + r.certState = certState + r.keyState = keyState + if err == nil { + var cert tls.Certificate + cert, err = tls.LoadX509KeyPair(r.certFile, r.keyFile) + if err == nil { + r.cert = &cert + r.failedAt = time.Time{} + r.logger.Info().Msg("Reloaded X509 KeyPair") + return r.cert, nil + } + } + r.failedAt = r.now() + r.logger.Warn().Err(err).Msg("Failed to reload X509 KeyPair, serving previous certificate") + return r.cert, nil +} + +// stat returns the current state of the certificate and key files, or zero states and an error +// if either cannot be read. +func (r *Reloader) stat() (certState, keyState fileState, err error) { + certState, err = statFile(r.certFile) + if err != nil { + return fileState{}, fileState{}, err + } + keyState, err = statFile(r.keyFile) + if err != nil { + return fileState{}, fileState{}, err + } + return certState, keyState, nil +} + +func statFile(name string) (fileState, error) { + info, err := os.Stat(name) + if err != nil { + return fileState{}, fmt.Errorf("stat %s: %w", name, err) + } + return fileState{modTime: info.ModTime(), size: info.Size()}, nil +} diff --git a/pkg/server/tlscert/reloader_test.go b/pkg/server/tlscert/reloader_test.go new file mode 100644 index 0000000..2456906 --- /dev/null +++ b/pkg/server/tlscert/reloader_test.go @@ -0,0 +1,254 @@ +package tlscert + +import ( + "bytes" + "crypto/rand" + "crypto/rsa" + "crypto/tls" + "crypto/x509" + "crypto/x509/pkix" + "encoding/pem" + "math/big" + "os" + "path/filepath" + "runtime" + "strings" + "testing" + "time" + + "github.com/rs/zerolog" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestNewReloaderMissingFiles(t *testing.T) { + dir := t.TempDir() + _, err := NewReloader(filepath.Join(dir, "cert.pem"), filepath.Join(dir, "key.pem"), testLogger(t)) + require.Error(t, err) +} + +func TestNewReloaderMismatchedPair(t *testing.T) { + dir := t.TempDir() + certA, _ := generatePair(t, "a") + _, keyB := generatePair(t, "b") + certFile, keyFile := filepath.Join(dir, "cert.pem"), filepath.Join(dir, "key.pem") + writeFile(t, certFile, certA, 1) + writeFile(t, keyFile, keyB, 1) + + _, err := NewReloader(certFile, keyFile, testLogger(t)) + require.Error(t, err) +} + +func TestGetCertificateReloadsOnChange(t *testing.T) { + dir := t.TempDir() + certFile, keyFile := filepath.Join(dir, "cert.pem"), filepath.Join(dir, "key.pem") + writePairFiles(t, certFile, keyFile, "a", 1) + + r, err := NewReloader(certFile, keyFile, testLogger(t)) + require.NoError(t, err) + assert.Equal(t, "a", commonName(t, r)) + + writePairFiles(t, certFile, keyFile, "b", 2) + assert.Equal(t, "b", commonName(t, r)) +} + +func TestGetCertificateKeepsPreviousOnFailure(t *testing.T) { + tcs := []struct { + name string + mangle func(t *testing.T, certFile, keyFile string) + }{ + { + name: "invalid certificate", + mangle: func(t *testing.T, certFile, keyFile string) { + t.Helper() + writeFile(t, certFile, []byte("not a certificate"), 2) + }, + }, + { + name: "certificate rotated before key", + mangle: func(t *testing.T, certFile, keyFile string) { + t.Helper() + certB, _ := generatePair(t, "b") + writeFile(t, certFile, certB, 2) + }, + }, + { + name: "files removed", + mangle: func(t *testing.T, certFile, keyFile string) { + t.Helper() + require.NoError(t, os.Remove(certFile)) + require.NoError(t, os.Remove(keyFile)) + }, + }, + } + + for _, tc := range tcs { + t.Run(tc.name, func(t *testing.T) { + dir := t.TempDir() + certFile, keyFile := filepath.Join(dir, "cert.pem"), filepath.Join(dir, "key.pem") + writePairFiles(t, certFile, keyFile, "a", 1) + + r, err := NewReloader(certFile, keyFile, testLogger(t)) + require.NoError(t, err) + + tc.mangle(t, certFile, keyFile) + assert.Equal(t, "a", commonName(t, r)) + + // A valid pair written afterward is picked up. + writePairFiles(t, certFile, keyFile, "c", 3) + assert.Equal(t, "c", commonName(t, r)) + }) + } +} + +// TestGetCertificateRetriesUnchangedFiles covers a pair that fails to load and is then fixed +// without changing its files' modification time or size, as a chmod or chown would. +func TestGetCertificateRetriesUnchangedFiles(t *testing.T) { + dir := t.TempDir() + certFile, keyFile := filepath.Join(dir, "cert.pem"), filepath.Join(dir, "key.pem") + writePairFiles(t, certFile, keyFile, "a", 1) + + r, err := NewReloader(certFile, keyFile, testLogger(t)) + require.NoError(t, err) + now := time.Now() + r.now = func() time.Time { return now } + + // The new key is unreadable on the first attempt. + certB, keyB := generatePair(t, "b") + writeFile(t, certFile, certB, 2) + writeFile(t, keyFile, bytes.Repeat([]byte("x"), len(keyB)), 2) + assert.Equal(t, "a", commonName(t, r)) + + // Fixing the key leaves its modification time and size unchanged. + writeFile(t, keyFile, keyB, 2) + assert.Equal(t, "a", commonName(t, r), "retried before retryInterval elapsed") + + now = now.Add(r.retryInterval) + assert.Equal(t, "b", commonName(t, r)) +} + +// TestGetCertificateFailureWarnings verifies that a persistent failure is attempted, and logged, +// at most once per retryInterval. +func TestGetCertificateFailureWarnings(t *testing.T) { + dir := t.TempDir() + certFile, keyFile := filepath.Join(dir, "cert.pem"), filepath.Join(dir, "key.pem") + writePairFiles(t, certFile, keyFile, "a", 1) + + var logs bytes.Buffer + r, err := NewReloader(certFile, keyFile, zerolog.New(&logs)) + require.NoError(t, err) + now := time.Now() + r.now = func() time.Time { return now } + warnings := func() int { return strings.Count(logs.String(), "Failed to reload X509 KeyPair") } + + require.NoError(t, os.Remove(certFile)) + for range 3 { + assert.Equal(t, "a", commonName(t, r)) + } + assert.Equal(t, 1, warnings()) + + now = now.Add(r.retryInterval) + assert.Equal(t, "a", commonName(t, r)) + assert.Equal(t, 2, warnings()) +} + +// TestGetCertificateSymlinkSwap mirrors how the kubelet updates a mounted Secret: the files are +// symlinks through a ..data symlink, which is atomically repointed at a new directory. +func TestGetCertificateSymlinkSwap(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("Windows cannot rename over an existing directory symlink") + } + dir := t.TempDir() + writePairFiles(t, filepath.Join(dir, "v1", "cert.pem"), filepath.Join(dir, "v1", "key.pem"), "a", 1) + require.NoError(t, os.Symlink("v1", filepath.Join(dir, "..data"))) + certFile, keyFile := filepath.Join(dir, "cert.pem"), filepath.Join(dir, "key.pem") + require.NoError(t, os.Symlink(filepath.Join("..data", "cert.pem"), certFile)) + require.NoError(t, os.Symlink(filepath.Join("..data", "key.pem"), keyFile)) + + r, err := NewReloader(certFile, keyFile, testLogger(t)) + require.NoError(t, err) + assert.Equal(t, "a", commonName(t, r)) + + writePairFiles(t, filepath.Join(dir, "v2", "cert.pem"), filepath.Join(dir, "v2", "key.pem"), "b", 2) + require.NoError(t, os.Symlink("v2", filepath.Join(dir, "..data_tmp"))) + require.NoError(t, os.Rename(filepath.Join(dir, "..data_tmp"), filepath.Join(dir, "..data"))) + assert.Equal(t, "b", commonName(t, r)) +} + +func TestGetCertificateConcurrent(t *testing.T) { + dir := t.TempDir() + certFile, keyFile := filepath.Join(dir, "cert.pem"), filepath.Join(dir, "key.pem") + writePairFiles(t, certFile, keyFile, "a", 1) + + r, err := NewReloader(certFile, keyFile, testLogger(t)) + require.NoError(t, err) + + done := make(chan struct{}) + for range 8 { + go func() { + defer func() { done <- struct{}{} }() + for range 50 { + cert, err := r.GetCertificate(nil) + assert.NoError(t, err) + assert.NotNil(t, cert) + } + }() + } + writePairFiles(t, certFile, keyFile, "b", 2) + for range 8 { + <-done + } + assert.Equal(t, "b", commonName(t, r)) +} + +func testLogger(t *testing.T) zerolog.Logger { + t.Helper() + return zerolog.New(zerolog.NewTestWriter(t)) +} + +// commonName returns the subject common name of the certificate r currently serves. +func commonName(t *testing.T, r *Reloader) string { + t.Helper() + cert, err := r.GetCertificate(&tls.ClientHelloInfo{}) + require.NoError(t, err) + require.NotNil(t, cert) + leaf, err := x509.ParseCertificate(cert.Certificate[0]) + require.NoError(t, err) + return leaf.Subject.CommonName +} + +// writePairFiles writes a new key pair for commonName, stamping both files with a modification +// time derived from version so each version is distinguishable regardless of timestamp +// granularity. +func writePairFiles(t *testing.T, certFile, keyFile, commonName string, version int) { + t.Helper() + certPEM, keyPEM := generatePair(t, commonName) + writeFile(t, certFile, certPEM, version) + writeFile(t, keyFile, keyPEM, version) +} + +func writeFile(t *testing.T, name string, data []byte, version int) { + t.Helper() + require.NoError(t, os.MkdirAll(filepath.Dir(name), 0o700)) + require.NoError(t, os.WriteFile(name, data, 0o600)) + mtime := time.Date(2026, 1, 1, 0, 0, version, 0, time.UTC) + require.NoError(t, os.Chtimes(name, mtime, mtime)) +} + +// generatePair returns PEM encoded certificate and private key for a self-signed certificate. +func generatePair(t *testing.T, commonName string) (certPEM, keyPEM []byte) { + t.Helper() + priv, err := rsa.GenerateKey(rand.Reader, 2048) + require.NoError(t, err) + + template := &x509.Certificate{ + SerialNumber: big.NewInt(1), + Subject: pkix.Name{CommonName: commonName}, + } + der, err := x509.CreateCertificate(rand.Reader, template, template, &priv.PublicKey, priv) + require.NoError(t, err) + + certPEM = pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: der}) + keyPEM = pem.EncodeToMemory(&pem.Block{Type: "RSA PRIVATE KEY", Bytes: x509.MarshalPKCS1PrivateKey(priv)}) + return certPEM, keyPEM +}