diff --git a/config.go b/config.go index e2924ed5..9a442996 100644 --- a/config.go +++ b/config.go @@ -108,6 +108,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 5fea2339..7fc75a00 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,10 +174,11 @@ 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. Callers pass the storage -// to load from: see cachedStorage and groundTruthStorage. -func (cfg *Config) loadCertResourceAnyIssuer(ctx context.Context, certNamesKey string, storage Storage) (CertificateResource, error) { - // we can save some extra decoding steps if there's only one issuer, since +// (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. Callers pass the storage to load from: see +// cachedStorage and groundTruthStorage. +func (cfg *Config) loadCertResourceAnyIssuer(ctx context.Context, certNamesKey string, storage Storage) (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 // select the best one, when there's only one choice anyway if len(cfg.Issuers) == 1 { @@ -214,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..e6e04765 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,9 @@ import ( "crypto/rand" "crypto/rsa" "crypto/x509" + "strings" "testing" + "time" ) func TestEncodeDecodeRSAPrivateKey(t *testing.T) { @@ -99,3 +102,143 @@ 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 +} + +// 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, cfg.Storage) + 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, cfg.Storage) + 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, cfg.Storage) + 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, cfg.Storage) + 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) + } +}