From 3542f0e842f3cf31fc2f95ceba0ec4633382de42 Mon Sep 17 00:00:00 2001 From: Michel Bardelmeijer Date: Tue, 1 Sep 2026 13:43:29 +0200 Subject: [PATCH] Add optional LoadFirstUsableCert to skip unused issuer loads MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit By default, loading a certificate reads every configured issuer and serves the newest resource. That costs a storage round-trip per issuer on every load, including for issuers that have never issued for that name — the usual case for a failover CA. When LoadFirstUsableCert is set, a load stops once a preferred issuer has a certificate that does not need renewal. Issuers with no cert, or with one due for renewal, are still read so failover certificates are found and the newest still wins when none is usable. --- config.go | 28 ++++++++ crypto.go | 20 +++++- crypto_test.go | 175 +++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 222 insertions(+), 1 deletion(-) diff --git a/config.go b/config.go index 624e0082..b0e50f75 100644 --- a/config.go +++ b/config.go @@ -98,6 +98,34 @@ type Config struct { // turn until one succeeds. Issuers []Issuer + // By default, loading a certificate reads the certificate resource of + // every configured issuer and serves the newest of them, since any issuer + // may hold the most recent certificate for a name. That costs a storage + // round-trip per issuer on every load, including for issuers that have + // never issued for that name -- the usual case for an issuer configured + // only for failover. + // + // If LoadFirstUsableCert is true, a load stops reading issuers once one + // of them has a usable certificate, meaning one that does not need + // renewal. The newest of the resources it did read is still the one + // returned; only issuers that could not have supplied a certificate to + // serve are skipped. + // + // Nothing is skipped before a usable certificate is found: issuers that + // hold no certificate, or hold one that is due for renewal, are read and + // compared as usual, so a certificate obtained during failover is still + // found, and the newest still wins when none of them is usable. + // + // Issuers are documented to be in order of preference, which is what + // makes the certificate found first the right one to settle for. Note + // that an IssuerPolicy of UseFirstRandomIssuer gives up that order, so + // which issuer a load settles on is then arbitrary. + // + // This is worth setting when Storage is remote and the in-memory cache + // cannot hold every certificate being served, so that the round-trip + // skipped is one a TLS handshake would have waited for. + LoadFirstUsableCert bool + // How to select which issuer to use. // Default: UseFirstIssuer (subject to change). IssuerPolicy IssuerPolicy diff --git a/crypto.go b/crypto.go index 9cbbb213..7ce97e6c 100644 --- a/crypto.go +++ b/crypto.go @@ -34,6 +34,7 @@ import ( "strings" "github.com/klauspost/cpuid/v2" + "github.com/mholt/acmez/v3/acme" "github.com/zeebo/blake3" "go.uber.org/zap" "golang.org/x/net/idna" @@ -173,7 +174,9 @@ func (cfg *Config) saveCertResource(ctx context.Context, issuer Issuer, cert Cer // loadCertResourceAnyIssuer loads and returns the certificate resource from any // of the configured issuers. If multiple are found (e.g. if there are 3 issuers // configured, and all 3 have a resource matching certNamesKey), then the newest -// (latest NotBefore date) resource will be chosen. +// (latest NotBefore date) resource will be chosen. If LoadFirstUsableCert is +// set, the issuers after the first one holding a resource that does not need +// renewal are not read. func (cfg *Config) loadCertResourceAnyIssuer(ctx context.Context, certNamesKey string) (CertificateResource, error) { // we can save some extra decoding steps if there's only one issuer, since // we don't need to compare potentially multiple available resources to @@ -213,6 +216,21 @@ func (cfg *Config) loadCertResourceAnyIssuer(ctx context.Context, certNamesKey s issuer: issuer, decoded: certs[0], }) + + // this issuer is preferred over the ones after it, so if its + // certificate is one we would serve as-is, reading theirs can only + // cost round-trips to find certificates we would not use + if cfg.LoadFirstUsableCert { + var ari acme.RenewalInfo + if !cfg.DisableARI { + if ariPtr, err := certRes.getARI(); err == nil && ariPtr != nil { + ari = *ariPtr + } + } + if !cfg.certNeedsRenewal(certs[0], ari, false) { + break + } + } } if len(certResources) == 0 { if lastErr == nil { diff --git a/crypto_test.go b/crypto_test.go index e8955527..8c65c6ec 100644 --- a/crypto_test.go +++ b/crypto_test.go @@ -17,6 +17,7 @@ package certmagic import ( "bytes" + "context" "crypto" "crypto/ecdsa" "crypto/ed25519" @@ -24,7 +25,13 @@ import ( "crypto/rand" "crypto/rsa" "crypto/x509" + "crypto/x509/pkix" + "math/big" + "strings" "testing" + "time" + + "github.com/mholt/acmez/v3/acme" ) func TestEncodeDecodeRSAPrivateKey(t *testing.T) { @@ -99,3 +106,171 @@ func privateKeyBytes(key crypto.PrivateKey) []byte { } return keyBytes } + +// testTwoIssuerConfig returns a config with a preferred and a fallback issuer, +// as one would have for failover between two CAs. +func testTwoIssuerConfig(t *testing.T) (*Config, *ACMEIssuer, *ACMEIssuer, *recordingStorage) { + t.Helper() + + preferred := &ACMEIssuer{CA: "https://preferred.example.com/acme/directory"} + fallback := &ACMEIssuer{CA: "https://fallback.example.com/acme/directory"} + storage := &recordingStorage{Storage: &FileStorage{Path: t.TempDir()}} + cfg := &Config{ + Issuers: []Issuer{preferred, fallback}, + Storage: storage, + RenewalWindowRatio: DefaultRenewalWindowRatio, + Logger: defaultTestLogger, + certCache: &Cache{ + cache: make(map[string]Certificate), + cacheIndex: make(map[string][]string), + logger: defaultTestLogger, + }, + } + preferred.config = cfg + fallback.config = cfg + + return cfg, preferred, fallback, storage +} + +// testIssuedCertResource returns a certificate for domain from issuer, valid +// for 90 days from notBefore. +func testIssuedCertResource(t *testing.T, issuer Issuer, domain string, notBefore time.Time) CertificateResource { + t.Helper() + _, key, certPEM := mustIssueTestCertificate(t, &x509.Certificate{ + SerialNumber: big.NewInt(notBefore.Unix()), + Subject: pkix.Name{CommonName: domain}, + DNSNames: []string{domain}, + NotBefore: notBefore, + NotAfter: notBefore.Add(90 * 24 * time.Hour), + KeyUsage: x509.KeyUsageDigitalSignature | x509.KeyUsageCertSign, + ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth}, + BasicConstraintsValid: true, + IsCA: true, + }, nil, nil) + keyPEM, err := PEMEncodePrivateKey(key) + if err != nil { + t.Fatalf("Expected no error encoding private key, got: %v", err) + } + return CertificateResource{ + SANs: []string{domain}, + CertificatePEM: certPEM, + PrivateKeyPEM: keyPEM, + IssuerData: mustJSON(acme.Certificate{URL: "https://example.com/cert"}), + issuerKey: issuer.IssuerKey(), + } +} + +// saveTestCertResource stores a certificate for domain from issuer, valid for +// 90 days from notBefore. +func saveTestCertResource(t *testing.T, cfg *Config, issuer Issuer, domain string, notBefore time.Time) { + t.Helper() + + err := cfg.saveCertResource(context.Background(), issuer, testIssuedCertResource(t, issuer, domain, notBefore)) + if err != nil { + t.Fatalf("Expected no error saving cert resource, got: %v", err) + } +} + +// readAssetsOf reports whether storage was asked for any of the certificate +// assets belonging to issuer. +func readAssetsOf(storage *recordingStorage, issuer Issuer, domain string) bool { + prefix := StorageKeys.CertsSitePrefix(issuer.IssuerKey(), domain) + for _, call := range storage.calls { + if call.name != "Load" || len(call.args) == 0 { + continue + } + if key, ok := call.args[0].(string); ok && strings.HasPrefix(key, prefix) { + return true + } + } + return false +} + +func TestLoadCertResourceAnyIssuerPrefersNewestByDefault(t *testing.T) { + cfg, preferred, fallback, storage := testTwoIssuerConfig(t) + const domain = "example.com" + now := time.Now() + + saveTestCertResource(t, cfg, preferred, domain, now.Add(-24*time.Hour)) + saveTestCertResource(t, cfg, fallback, domain, now) + + storage.calls = nil + certRes, err := cfg.loadCertResourceAnyIssuer(context.Background(), domain) + if err != nil { + t.Fatalf("Expected no error loading cert resource, got: %v", err) + } + + if certRes.issuerKey != fallback.IssuerKey() { + t.Errorf("Expected the newest certificate, from %s, got one from %s", fallback.IssuerKey(), certRes.issuerKey) + } + if !readAssetsOf(storage, fallback, domain) { + t.Error("Expected all issuers to be read when LoadFirstUsableCert is not set") + } +} + +func TestLoadFirstUsableCertSkipsLaterIssuers(t *testing.T) { + cfg, preferred, fallback, storage := testTwoIssuerConfig(t) + cfg.LoadFirstUsableCert = true + const domain = "example.com" + now := time.Now() + + // the fallback's certificate is newer, so it is the one that would win + // without LoadFirstUsableCert + saveTestCertResource(t, cfg, preferred, domain, now.Add(-24*time.Hour)) + saveTestCertResource(t, cfg, fallback, domain, now) + + storage.calls = nil + certRes, err := cfg.loadCertResourceAnyIssuer(context.Background(), domain) + if err != nil { + t.Fatalf("Expected no error loading cert resource, got: %v", err) + } + + if certRes.issuerKey != preferred.IssuerKey() { + t.Errorf("Expected the certificate from %s, got one from %s", preferred.IssuerKey(), certRes.issuerKey) + } + if readAssetsOf(storage, fallback, domain) { + t.Error("Expected the fallback issuer's assets not to be read") + } +} + +// The preferred certificate has to be one we would serve as-is for its issuer +// to end the search; otherwise a newer certificate from a later issuer is +// still the better one to serve. +func TestLoadFirstUsableCertLooksPastCertNeedingRenewal(t *testing.T) { + cfg, preferred, fallback, storage := testTwoIssuerConfig(t) + cfg.LoadFirstUsableCert = true + const domain = "example.com" + now := time.Now() + + saveTestCertResource(t, cfg, preferred, domain, now.Add(-85*24*time.Hour)) + saveTestCertResource(t, cfg, fallback, domain, now) + + storage.calls = nil + certRes, err := cfg.loadCertResourceAnyIssuer(context.Background(), domain) + if err != nil { + t.Fatalf("Expected no error loading cert resource, got: %v", err) + } + + if certRes.issuerKey != fallback.IssuerKey() { + t.Errorf("Expected the newest certificate, from %s, got one from %s", fallback.IssuerKey(), certRes.issuerKey) + } +} + +// A certificate obtained during failover lives under an issuer that is not the +// preferred one, and still has to be found. +func TestLoadFirstUsableCertFindsFailoverCert(t *testing.T) { + cfg, _, fallback, _ := testTwoIssuerConfig(t) + cfg.LoadFirstUsableCert = true + const domain = "example.com" + + saveTestCertResource(t, cfg, fallback, domain, time.Now()) + + certRes, err := cfg.loadCertResourceAnyIssuer(context.Background(), domain) + if err != nil { + t.Fatalf("Expected no error loading cert resource, got: %v", err) + } + + if certRes.issuerKey != fallback.IssuerKey() { + t.Errorf("Expected the certificate from %s, got one from %s", fallback.IssuerKey(), certRes.issuerKey) + } +}