Merge pull request ghostunnel#679 from ghostunnel/claude/review-next-priorities-gzd8k

Extract baseCertificate struct to deduplicate shared methods

Author: csstaub

certloader/certificate.go

@@ -19,8 +19,36 @@ package certloader
 import (
 	"crypto/tls"
 	"crypto/x509"
+	"sync/atomic"
 )
 
+// baseCertificate holds cached certificate and trust store pointers shared by
+// concrete Certificate types. Embed it and implement only Reload().
+type baseCertificate struct {
+	cachedCertificate atomic.Pointer[tls.Certificate]
+	cachedCertPool    atomic.Pointer[x509.CertPool]
+}
+
+// GetCertificate retrieves the actual underlying tls.Certificate.
+func (b *baseCertificate) GetCertificate(clientHello *tls.ClientHelloInfo) (*tls.Certificate, error) {
+	return b.cachedCertificate.Load(), nil
+}
+
+// GetClientCertificate retrieves the certificate for mTLS client authentication.
+func (b *baseCertificate) GetClientCertificate(certInfo *tls.CertificateRequestInfo) (*tls.Certificate, error) {
+	return b.cachedCertificate.Load(), nil
+}
+
+// GetTrustStore returns the most up-to-date version of the trust store / CA bundle.
+func (b *baseCertificate) GetTrustStore() *x509.CertPool {
+	return b.cachedCertPool.Load()
+}
+
+// GetIdentifier returns an identifier for the certificate for logging.
+func (b *baseCertificate) GetIdentifier() string {
+	return b.cachedCertificate.Load().Leaf.Subject.String()
+}
+
 // Certificate wraps a TLS certificate and supports reloading at runtime.
 type Certificate interface {
 	// Reload will reload the certificate and private key. Subsequent calls

certloader/certstore_enabled.go

@@ -24,12 +24,12 @@ import (
 	"fmt"
 	"log"
 	"sort"
-	"sync/atomic"
 
 	"github.com/ghostunnel/ghostunnel/certstore"
 )
 
 type certstoreCertificate struct {
+	baseCertificate
 	// Common name or serial number of keychain identity
 	commonNameOrSerial string
 	// Issuer name of keychain identity
@@ -38,10 +38,6 @@ type certstoreCertificate struct {
 	caBundlePath string
 	// Require use of hardware token?
 	requireToken bool
-	// Cached *tls.Certificate
-	cachedCertificate atomic.Pointer[tls.Certificate]
-	// Cached *x509.CertPool
-	cachedCertPool atomic.Pointer[x509.CertPool]
 	// Added logger, useful for certstore logging
 	logger *log.Logger
 }
@@ -162,27 +158,6 @@ func (c *certstoreCertificate) Reload() error {
 	return nil
 }
 
-// GetIdentifier returns an identifier for the certificate for logging.
-func (c *certstoreCertificate) GetIdentifier() string {
-	cert, _ := c.GetCertificate(nil)
-	return cert.Leaf.Subject.String()
-}
-
-// GetCertificate retrieves the actual underlying tls.Certificate.
-func (c *certstoreCertificate) GetCertificate(clientHello *tls.ClientHelloInfo) (*tls.Certificate, error) {
-	return c.cachedCertificate.Load(), nil
-}
-
-// GetClientCertificate retrieves the actual underlying tls.Certificate.
-func (c *certstoreCertificate) GetClientCertificate(certInfo *tls.CertificateRequestInfo) (*tls.Certificate, error) {
-	return c.cachedCertificate.Load(), nil
-}
-
-// GetTrustStore returns the most up-to-date version of the trust store / CA bundle.
-func (c *certstoreCertificate) GetTrustStore() *x509.CertPool {
-	return c.cachedCertPool.Load()
-}
-
 func serializeChain(chain []*x509.Certificate) [][]byte {
 	out := [][]byte{}
 	for _, cert := range chain {

certloader/keystore.go

@@ -22,10 +22,10 @@ import (
 	"crypto/x509"
 	"encoding/pem"
 	"fmt"
-	"sync/atomic"
 )
 
 type keystoreCertificate struct {
+	baseCertificate
 	// Keystore or PEM files path
 	keystorePaths []string
 	// Password for keystore (may be empty)
@@ -34,10 +34,6 @@ type keystoreCertificate struct {
 	caBundlePath string
 	// File format indicator (e.g. "PEM", "PKCS12", "JCEKS", "DER", or "" for auto-detect)
 	format string
-	// Cached *tls.Certificate
-	cachedCertificate atomic.Pointer[tls.Certificate]
-	// Cached *x509.CertPool
-	cachedCertPool atomic.Pointer[x509.CertPool]
 }
 
 // CertificateFromPEMFiles creates a reloadable certificate from a set of PEM files.
@@ -105,24 +101,3 @@ func (c *keystoreCertificate) Reload() error {
 
 	return nil
 }
-
-// GetIdentifier returns an identifier for the certificate for logging.
-func (c *keystoreCertificate) GetIdentifier() string {
-	cert, _ := c.GetCertificate(nil)
-	return cert.Leaf.Subject.String()
-}
-
-// GetCertificate retrieves the actual underlying tls.Certificate.
-func (c *keystoreCertificate) GetCertificate(clientHello *tls.ClientHelloInfo) (*tls.Certificate, error) {
-	return c.cachedCertificate.Load(), nil
-}
-
-// GetClientCertificate retrieves the actual underlying tls.Certificate.
-func (c *keystoreCertificate) GetClientCertificate(certInfo *tls.CertificateRequestInfo) (*tls.Certificate, error) {
-	return c.cachedCertificate.Load(), nil
-}
-
-// GetTrustStore returns the most up-to-date version of the trust store / CA bundle.
-func (c *keystoreCertificate) GetTrustStore() *x509.CertPool {
-	return c.cachedCertPool.Load()
-}

certloader/pkcs11_enabled.go

@@ -20,24 +20,19 @@ package certloader
 
 import (
 	"crypto/tls"
-	"crypto/x509"
 	"log"
-	"sync/atomic"
 
 	pkcs11key "github.com/letsencrypt/pkcs11key/v4"
 )
 
 type pkcs11Certificate struct {
+	baseCertificate
 	// Certificate chain corresponding to key
 	certificatePath string
 	// Root CA bundle path
 	caBundlePath string
 	// Params for loading key from a PKCS#11 module
 	modulePath, tokenLabel, pin string
-	// Cached *tls.Certificate
-	cachedCertificate atomic.Pointer[tls.Certificate]
-	// Cached *x509.CertPool
-	cachedCertPool atomic.Pointer[x509.CertPool]
 	// Added logger, useful for PKCS#11 logging
 	logger *log.Logger
 }
@@ -84,9 +79,8 @@ func (c *pkcs11Certificate) Reload() error {
 	// Reuse previously loaded PKCS11 private key if we already have it.
 	// We want to avoid reloading the key every time the cert reloads, as it's
 	// a potentially expensive operation that calls out into a shared library.
-	if c.cachedCertificate.Load() != nil {
+	if old := c.cachedCertificate.Load(); old != nil {
 		c.logger.Printf("pkcs11: re-using previously cached private key handle from module")
-		old, _ := c.GetCertificate(nil)
 		certAndKey.PrivateKey = old.PrivateKey
 	} else {
 		c.logger.Printf("pkcs11: loading private key for given certificate from module")
@@ -107,24 +101,3 @@ func (c *pkcs11Certificate) Reload() error {
 
 	return nil
 }
-
-// GetIdentifier returns an identifier for the certificate for logging.
-func (c *pkcs11Certificate) GetIdentifier() string {
-	cert, _ := c.GetCertificate(nil)
-	return cert.Leaf.Subject.String()
-}
-
-// GetCertificate retrieves the actual underlying tls.Certificate.
-func (c *pkcs11Certificate) GetCertificate(clientHello *tls.ClientHelloInfo) (*tls.Certificate, error) {
-	return c.cachedCertificate.Load(), nil
-}
-
-// GetClientCertificate retrieves the actual underlying tls.Certificate.
-func (c *pkcs11Certificate) GetClientCertificate(certInfo *tls.CertificateRequestInfo) (*tls.Certificate, error) {
-	return c.cachedCertificate.Load(), nil
-}
-
-// GetTrustStore returns the most up-to-date version of the trust store / CA bundle.
-func (c *pkcs11Certificate) GetTrustStore() *x509.CertPool {
-	return c.cachedCertPool.Load()
-}