diff --git a/config.go b/config.go index 624e0082..a5b03321 100644 --- a/config.go +++ b/config.go @@ -825,6 +825,25 @@ func (cfg *Config) renewCert(ctx context.Context, name string, force, interactiv name = cfg.transformSubject(ctx, log, name) + // If the certificate's resources are not in storage for any of the + // currently-configured issuers, there is nothing to renew: renewal reuses + // the stored certificate resource (to reuse the private key and to check + // whether renewal is still needed), so a missing resource can never be + // loaded and renewal would fail on every attempt until the certificate + // expires. This commonly happens when the issuer is changed (e.g. a + // different ACME CA) between config reloads while the previously-obtained + // certificate remains cached: the cached certificate belongs to an issuer + // that is no longer configured, so its resources live under a storage path + // that none of the current issuers will look at. In that case, obtain a + // fresh certificate from the currently-configured issuer(s) instead of + // retrying a renewal that can never succeed. + // See https://github.com/caddyserver/caddy/issues/6732 + if !cfg.storageHasCertResourcesAnyIssuer(ctx, name) { + log.Info("certificate resources not found in storage for any configured issuer (issuer may have changed); obtaining a new certificate instead of renewing", + zap.String("identifier", name)) + return cfg.obtainCert(ctx, name, interactive) + } + // ensure storage is writeable and readable // TODO: this is not necessary every time; should only perform check once every so often for each storage, which may require some global state... err := cfg.checkStorage(ctx) diff --git a/config_test.go b/config_test.go index 89a63224..fd4a8a6a 100644 --- a/config_test.go +++ b/config_test.go @@ -17,6 +17,7 @@ package certmagic import ( "bytes" "context" + "crypto/x509" "encoding/json" "os" "reflect" @@ -154,3 +155,86 @@ func mustJSON(val any) []byte { } return result } + +// fakeIssuer is a minimal Issuer implementation for tests. It records whether +// it was asked to issue a certificate and returns a canned certificate so that +// obtain/renew flows can complete without contacting a real CA. The obtain and +// renew code paths do not parse the returned certificate, so placeholder PEM +// contents are sufficient (matching the convention used elsewhere in this file). +type fakeIssuer struct { + key string + certPEM []byte + issued bool +} + +func (f *fakeIssuer) IssuerKey() string { return f.key } + +func (f *fakeIssuer) Issue(_ context.Context, _ *x509.CertificateRequest) (*IssuedCertificate, error) { + f.issued = true + return &IssuedCertificate{Certificate: f.certPEM}, nil +} + +// TestRenewCertObtainsWhenIssuerChanged is a regression test for +// https://github.com/caddyserver/caddy/issues/6732. When the issuer is changed +// between config reloads, the previously-obtained certificate remains stored +// under the OLD issuer's storage path. Because renewal reuses the stored +// certificate resource, a naive renewal looks for the resource under the +// currently-configured (new) issuer only, never finds it, and retries in vain +// until the certificate expires. Renewal must instead fall back to obtaining a +// fresh certificate from the currently-configured issuer. +func TestRenewCertObtainsWhenIssuerChanged(t *testing.T) { + ctx := context.Background() + domain := "example.com" + + storageDir := "./_testdata_renew_issuer_change" + storage := &FileStorage{Path: storageDir} + defer os.RemoveAll(storageDir) + + oldIssuer := &fakeIssuer{key: "old-issuer"} + newIssuer := &fakeIssuer{key: "new-issuer", certPEM: []byte("new issuer certificate")} + + // Seed storage with the existing certificate resource under the OLD issuer's + // key only -- exactly the on-disk state after the issuer is changed. + seedCfg := newWithCache(new(Cache), Config{ + Issuers: []Issuer{oldIssuer}, + Storage: storage, + Logger: defaultTestLogger, + }) + err := seedCfg.saveCertResource(ctx, oldIssuer, CertificateResource{ + SANs: []string{domain}, + CertificatePEM: []byte("old issuer certificate"), + PrivateKeyPEM: []byte("old issuer private key"), + IssuerData: mustJSON(acme.Certificate{URL: "https://old.example/cert"}), + issuerKey: oldIssuer.IssuerKey(), + }) + if err != nil { + t.Fatalf("seeding old certificate resource: %v", err) + } + + // Config is reloaded with a DIFFERENT issuer; the old certificate is still + // managed/cached, so renewal is eventually triggered for it. Use the + // synchronous (interactive) path so a failure surfaces immediately instead + // of being retried until the max retry duration. + cfg := newWithCache(new(Cache), Config{ + Issuers: []Issuer{newIssuer}, + Storage: storage, + Logger: defaultTestLogger, + }) + + // Precondition: the new issuer has no resources in storage yet, so a naive + // renewal would fail to load the certificate resource. + if cfg.storageHasCertResources(ctx, newIssuer, domain) { + t.Fatalf("precondition failed: new issuer unexpectedly already has certificate resources in storage") + } + + if err := cfg.RenewCertSync(ctx, domain, false); err != nil { + t.Fatalf("RenewCertSync after issuer change should obtain a new certificate, got error: %v", err) + } + + if !newIssuer.issued { + t.Error("expected the new issuer to issue a certificate (obtain fallback), but Issue was never called") + } + if !cfg.storageHasCertResources(ctx, newIssuer, domain) { + t.Errorf("expected certificate resources to be stored under the new issuer %q after renewal", newIssuer.IssuerKey()) + } +}