feat: reload TLS certificates when their files change (#625)

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

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) {

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")
 		}
 	}

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
+}

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
+}