diff --git a/internal/server/cert_store_archive.go b/internal/server/cert_store_archive.go index 125a2d3..e7d0ff4 100644 --- a/internal/server/cert_store_archive.go +++ b/internal/server/cert_store_archive.go @@ -111,6 +111,10 @@ type certStoreArchive struct { accountKey []byte dynamicDomains []byte + // extraAccountKeys holds the per-directory account files (--tls-staging + // identities), keyed by filename. + extraAccountKeys map[string][]byte + // certs is keyed by the certificate's directory name (the sanitized // certificate identifier). certs map[string]archiveCertPair @@ -268,6 +272,14 @@ func (a *certStoreArchive) placeEntry(name string, data []byte, rawCerts map[str } if rest, ok := strings.CutPrefix(name, archiveCertsPrefix); ok { + if isExtraAccountKeyFile(rest) { + if a.extraAccountKeys == nil { + a.extraAccountKeys = map[string][]byte{} + } + a.extraAccountKeys[rest] = data + return nil + } + dir, base, found := strings.Cut(rest, "/") // The directory must already be in the sanitized form the exporter // writes: two spellings that sanitize to the same on-disk path would @@ -316,7 +328,7 @@ func (a *certStoreArchive) assembleCertPairs(rawCerts map[string]map[string][]by // validate cross-checks the state file against the archived certificates and // discards an account key that could not carry the ACME identity forward. func (a *certStoreArchive) validate() error { - a.checkAccountKey() + a.checkAccountKeys() if !a.hasState { if len(a.certs) > 0 { @@ -364,41 +376,50 @@ func (a *certStoreArchive) validate() error { return nil } -// checkAccountKey drops an account key entry that does not hold usable key -// material, with a warning: restoring it would make the next boot silently +// checkAccountKeys drops account key entries that do not hold usable key +// material, with a warning: restoring one would make the next boot silently // register a fresh ACME account while the operator believes the identity was // preserved. The estate's certificates still restore. -func (a *certStoreArchive) checkAccountKey() { - if a.accountKey == nil { - return +func (a *certStoreArchive) checkAccountKeys() { + if a.accountKey != nil && !a.usableAccountKey(a.accountKey, "ACME account key") { + a.accountKey = nil } + for _, name := range slices.Sorted(maps.Keys(a.extraAccountKeys)) { + if !a.usableAccountKey(a.extraAccountKeys[name], "ACME account key "+name) { + delete(a.extraAccountKeys, name) + } + } +} + +// usableAccountKey reports whether data parses as an account holding an ECDSA +// private key, warning under the given label otherwise. It mirrors +// loadOrCreateUser exactly: that only accepts an ECDSA key, so any other key +// type would be silently discarded at boot and a fresh account registered -- +// the very outcome this check exists to make loud. +func (a *certStoreArchive) usableAccountKey(data []byte, label string) bool { var user acmeUser - if err := json.Unmarshal(a.accountKey, &user); err != nil { + if err := json.Unmarshal(data, &user); err != nil { a.warnings = append(a.warnings, certArchiveWarning{ kind: warnAccountKey, - text: fmt.Sprintf("the archived ACME account key does not parse and will not be restored; the next boot will register a fresh account: %v", err), + text: fmt.Sprintf("the archived %s does not parse and will not be restored; the next boot will register a fresh account: %v", label, err), }) - a.accountKey = nil - return + return false } - // Mirror loadOrCreateUser exactly: it only accepts an ECDSA key, so any - // other key type would be silently discarded at boot and a fresh account - // registered -- the very outcome this check exists to make loud. key, err := certcrypto.ParsePEMPrivateKey(user.KeyPEM) if err == nil { if _, ok := key.(*ecdsa.PrivateKey); ok { - return + return true } err = errors.New("the key is not an ECDSA key") } a.warnings = append(a.warnings, certArchiveWarning{ kind: warnAccountKey, - text: fmt.Sprintf("the archived ACME account key holds no usable private key and will not be restored; the next boot will register a fresh account: %v", err), + text: fmt.Sprintf("the archived %s holds no usable private key and will not be restored; the next boot will register a fresh account: %v", label, err), }) - a.accountKey = nil + return false } // validateManagerState checks the invariants a healthy manager always diff --git a/internal/server/cert_store_export.go b/internal/server/cert_store_export.go index c1d5633..4acff9b 100644 --- a/internal/server/cert_store_export.go +++ b/internal/server/cert_store_export.go @@ -30,6 +30,34 @@ const ( acmeUserFile = "acme_user.json" ) +// isExtraAccountKeyFile matches the per-directory ACME account files that sit +// beside the primary acme_user.json when services use their own ACME +// directory (--tls-staging). The set is closed to exactly the names +// accountFileForDirectory generates — acme_user_staging.json or an +// 8-character lowercase hex hash — so the archive's entry set stays +// enumerable and arbitrary acme_user_*.json files are not silently adopted +// as ACME identities. +func isExtraAccountKeyFile(name string) bool { + if name == "acme_user_staging.json" { + return true + } + + hash, ok := strings.CutPrefix(name, "acme_user_") + if !ok { + return false + } + hash, ok = strings.CutSuffix(hash, ".json") + if !ok || len(hash) != 8 { + return false + } + for _, c := range hash { + if (c < '0' || c > '9') && (c < 'a' || c > 'f') { + return false + } + } + return true +} + // ErrCertStoreEmpty reports an export attempt against a store with nothing in // it. Failing loudly beats a cron job faithfully archiving nothing. var ErrCertStoreEmpty = errors.New("certificate store is empty; nothing to export") @@ -108,7 +136,13 @@ func ExportCertificateStore(paths CertStorePaths, outputPath string) (CertsExpor // The account key alone is not a certificate: a fresh estate that has only // registered an account still gets its backup. certsWithoutState := slices.ContainsFunc(certFiles, func(file archiveFile) bool { - return file.name != archiveAccountKeyEntry + if file.name == archiveAccountKeyEntry { + return false + } + // Per-directory account keys are estate metadata like the primary + // one: an account-only store (registered but nothing issued yet) + // still gets its backup. + return !isExtraAccountKeyFile(strings.TrimPrefix(file.name, archiveCertsPrefix)) }) if !hasState && certsWithoutState { return summary, fmt.Errorf("the certificate store has certificates but no state file at %s; refusing to export an unrestorable archive", paths.ACMEStatePath) @@ -200,6 +234,10 @@ func collectCertsEntries(certsPath string, state managerState, summary *CertsExp if file, ok := collectOptionalJSON(filepath.Join(certsPath, name), archiveAccountKeyEntry, summary); ok { files = append(files, file) } + case !entry.IsDir() && isExtraAccountKeyFile(name): + if file, ok := collectOptionalJSON(filepath.Join(certsPath, name), archiveCertsPrefix+name, summary); ok { + files = append(files, file) + } case entry.IsDir() && name == legacyHTTP01CacheDir: summary.Warnings = append(summary.Warnings, fmt.Sprintf("legacy %s cache is not exported: start the proxy once so it is adopted into the store first", legacyHTTP01CacheDir)) diff --git a/internal/server/cert_store_export_test.go b/internal/server/cert_store_export_test.go index b728183..e96c290 100644 --- a/internal/server/cert_store_export_test.go +++ b/internal/server/cert_store_export_test.go @@ -10,6 +10,7 @@ import ( "io" "os" "path/filepath" + "slices" "strings" "testing" "time" @@ -472,3 +473,37 @@ func TestDirInsidePinnedTree(t *testing.T) { require.NoError(t, err) assert.False(t, inside) } + +func TestExportCertificateStore_AccountKeysOnlyStoreExports(t *testing.T) { + // A fresh estate that has registered accounts but issued nothing yet + // still gets its backup — whichever identities it holds. + tests := []struct { + name string + files []string + }{ + {"primary and staging keys", []string{"acme_user.json", "acme_user_staging.json"}}, + {"a lone staging key", []string{"acme_user_staging.json"}}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + paths := testCertStorePaths(t) + require.NoError(t, os.MkdirAll(paths.CertsPath, 0700)) + for _, name := range tt.files { + require.NoError(t, os.WriteFile(filepath.Join(paths.CertsPath, name), + testAccountKeyJSON(t), 0600)) + } + + archivePath := filepath.Join(t.TempDir(), "backup.tar.gz") + summary, err := ExportCertificateStore(paths, archivePath) + require.NoError(t, err) + assert.Empty(t, summary.Warnings) + assert.Zero(t, summary.Certificates) + + report, err := VerifyCertificateArchive(archivePath) + require.NoError(t, err) + assert.Empty(t, report.Certificates) + assert.Equal(t, slices.Contains(tt.files, "acme_user.json"), report.HasAccountKey) + }) + } +} diff --git a/internal/server/cert_store_restore.go b/internal/server/cert_store_restore.go index 761eb87..3733bb8 100644 --- a/internal/server/cert_store_restore.go +++ b/internal/server/cert_store_restore.go @@ -94,6 +94,15 @@ func RestoreCertificateStore(opts CertStoreRestoreOptions) (CertsRestoreSummary, summary.AccountKeyRestored = true } + for _, name := range slices.Sorted(maps.Keys(archive.extraAccountKeys)) { + if err := os.MkdirAll(opts.Paths.CertsPath, 0700); err != nil { + return summary, fmt.Errorf("failed to create the certificate directory: %w", err) + } + if err := writeFileStaged(filepath.Join(opts.Paths.CertsPath, name), archive.extraAccountKeys[name]); err != nil { + return summary, fmt.Errorf("failed to restore the ACME account key %s: %w", name, err) + } + } + if archive.dynamicDomains != nil { if err := writeFileStaged(opts.Paths.DynamicDomainsStatePath, archive.dynamicDomains); err != nil { return summary, fmt.Errorf("failed to restore the dynamic domains state: %w", err) diff --git a/internal/server/cert_store_restore_test.go b/internal/server/cert_store_restore_test.go index 949956a..ed6f253 100644 --- a/internal/server/cert_store_restore_test.go +++ b/internal/server/cert_store_restore_test.go @@ -656,3 +656,39 @@ func TestRestoreCertificateStore_DegenerateArchiveIntoFreshStore(t *testing.T) { state := readImportedState(t, paths.ACMEStatePath) assert.Contains(t, state.Certificates, "san:gone") } + +func TestRestoreCertificateStore_RoundTripsExtraAccountKeysAndDirectories(t *testing.T) { + paths := testCertStorePaths(t) + state := populateCertStore(t, paths, []string{"staging.example.com"}) + + // A per-service directory estate: the certificate records its issuing + // directory, and the staging identity's account key sits beside the + // primary one. + for _, cert := range state.Certificates { + cert.Directory = LetsEncryptStaging + } + require.NoError(t, writeManagerStateFile(paths.ACMEStatePath, state)) + stagingKey := testAccountKeyJSON(t) + require.NoError(t, os.WriteFile(filepath.Join(paths.CertsPath, "acme_user_staging.json"), stagingKey, 0600)) + + archivePath := filepath.Join(t.TempDir(), "backup.tar.gz") + summary, err := ExportCertificateStore(paths, archivePath) + require.NoError(t, err) + assert.Empty(t, summary.Warnings, "a per-directory account key is part of the estate, not an unexpected file") + + target := testCertStorePaths(t) + _, err = RestoreCertificateStore(CertStoreRestoreOptions{ArchivePath: archivePath, Paths: target}) + require.NoError(t, err) + + restoredKey, err := os.ReadFile(filepath.Join(target.CertsPath, "acme_user_staging.json")) + require.NoError(t, err) + assert.Equal(t, stagingKey, restoredKey) + + var restored managerState + data, err := os.ReadFile(target.ACMEStatePath) + require.NoError(t, err) + require.NoError(t, json.Unmarshal(data, &restored)) + for _, cert := range restored.Certificates { + assert.Equal(t, LetsEncryptStaging, cert.Directory, "the issuing directory must survive export and restore") + } +} diff --git a/internal/server/domain_renewal.go b/internal/server/domain_renewal.go index 4c13e98..15e9a8b 100644 --- a/internal/server/domain_renewal.go +++ b/internal/server/domain_renewal.go @@ -41,6 +41,15 @@ type renewalInfoGetter interface { GetRenewalInfo(request certificate.RenewalInfoRequest) (*certificate.RenewalInfoResponse, error) } +// directoryObtainer is implemented by obtainers that can pin an order to a +// specific ACME directory. The renewer prefers it when available, so a +// partition whose owners are temporarily unresolvable still orders under the +// certificate's recorded identity instead of the resolver's run-level +// fallback. +type directoryObtainer interface { + ObtainAt(directory string, request certificate.ObtainRequest) (*certificate.Resource, error) +} + type certRenewerConfig struct { Obtainer certObtainer @@ -148,7 +157,10 @@ func (r *certRenewer) reconcile() { continue } - if r.shouldRenew(cert) { + // The directory check comes first: a switched certificate must be + // replaced regardless of what ARI (queried at a directory that may no + // longer be the right one) would say about its timing. + if r.directoryChanged(cert) || r.shouldRenew(cert) { r.renew(cert) } } @@ -156,6 +168,19 @@ func (r *certRenewer) reconcile() { r.reportMetrics() } +// directoryChanged reports whether any of the certificate's domains is owned +// by a service that wants a different ACME directory than the one that issued +// it. After a --tls-staging flip the certificate is replaced on the next +// reconcile rather than at the renewal window: a staging certificate is not +// browser-trusted, and a production one spends rate limits the operator opted +// out of. The renewal then splits along directory boundaries, so each +// replacement is ordered under its owner's identity. A certificate with no +// resolvable owner keeps its recorded directory — services may simply not +// have re-attached yet after a restart. +func (r *certRenewer) directoryChanged(cert *ManagedCert) bool { + return r.manager.certDirectoryMismatched(cert) +} + func (r *certRenewer) shouldRenew(cert *ManagedCert) bool { leaf := certLeaf(cert) @@ -243,23 +268,38 @@ func (r *certRenewer) renew(cert *ManagedCert) { } // A certificate issued before zone mappings existed can span DNS - // providers; its renewal splits along provider boundaries, one order per - // partition. A certificate can only be "replaced" once, so the ARI marker - // rides the first order. + // providers, and a legacy certificate can span services whose directories + // have since diverged; its renewal splits along both boundaries, one + // order per partition. A certificate can only be "replaced" once, and the + // marker only means something to the CA that issued the predecessor (RFC + // 9773 tells a CA to reject a replaces identifier it never issued), so + // the ARI marker rides the first order that stays at the recorded + // directory — a full directory switch sends no marker at all. + recorded := r.manager.normalizeDirectory(cert.Directory) renewedAll := true + ariConsumed := false newIdentifiers := []string{} - for i, partition := range r.manager.splitByProviderZone(domains) { - partitionReplaces := "" - if i == 0 { - partitionReplaces = replaces - } + for _, directoryPart := range r.manager.splitByDesiredDirectory(cert, domains) { + for _, partition := range r.manager.splitByProviderZone(directoryPart.domains) { + partitionReplaces := "" + if !ariConsumed && directoryPart.directory == recorded { + partitionReplaces = replaces + } - renewed, ok := r.renewPartition(cert, partition, partitionReplaces) - if !ok { - renewedAll = false - continue + renewed, adopted, ordered := r.renewPartition(cert, partition, partitionReplaces, directoryPart.directory) + // The marker is spent once the CA accepted an order carrying it — + // adoption can still fail locally, but re-sending an identifier + // the CA already honored would have the next order rejected. An + // order the CA refused leaves the marker for a later partition. + if partitionReplaces != "" && ordered { + ariConsumed = true + } + if !adopted { + renewedAll = false + continue + } + newIdentifiers = append(newIdentifiers, renewed.Identifier) } - newIdentifiers = append(newIdentifiers, renewed.Identifier) } // The old certificate goes only when every partition has a successor: a @@ -273,32 +313,42 @@ func (r *certRenewer) renew(cert *ManagedCert) { } } -// renewPartition runs one renewal order and adopts its certificate. It -// reports success; failures quarantine or log exactly as a whole-certificate -// renewal did. -func (r *certRenewer) renewPartition(cert *ManagedCert, domains []string, replaces string) (*ManagedCert, bool) { +// renewPartition runs one renewal order at the partition's directory and +// adopts its certificate. adopted reports end-to-end success; ordered reports +// that the CA accepted the order (which spends an ARI replaces marker even if +// adoption then fails locally). Failures quarantine or log exactly as a +// whole-certificate renewal did. +func (r *certRenewer) renewPartition(cert *ManagedCert, domains []string, replaces, directory string) (renewed *ManagedCert, adopted, ordered bool) { if r.config.Bucket != nil { if err := r.config.Bucket.Take(r.ctx); err != nil { - return nil, false + return nil, false, false } } - slog.Info("Renewing certificate", "certificate", cert.Identifier, "domains", domains) + slog.Info("Renewing certificate", "certificate", cert.Identifier, "domains", domains, "directory", directory) - resource, err := r.config.Obtainer.Obtain(certificate.ObtainRequest{ + request := certificate.ObtainRequest{ Domains: domains, Bundle: true, ReplacesCertID: replaces, - }) + } + + var resource *certificate.Resource + var err error + if pinned, ok := r.config.Obtainer.(directoryObtainer); ok { + resource, err = pinned.ObtainAt(directory, request) + } else { + resource, err = r.config.Obtainer.Obtain(request) + } if err != nil { r.handleRenewalFailure(cert, domains, err) - return nil, false + return nil, false, false } - renewed, err := r.manager.adoptCertificate(resource, domains) + renewed, err = r.manager.adoptCertificateAt(resource, domains, directory) if err != nil { slog.Error("Failed to adopt renewed certificate", "certificate", cert.Identifier, "error", err) - return nil, false + return nil, false, true } for _, domain := range domains { @@ -306,7 +356,7 @@ func (r *certRenewer) renewPartition(cert *ManagedCert, domains []string, replac metrics.Tracker.IncCertificateRenewals(domain, true) } - return renewed, true + return renewed, true, true } // retireCertificate disposes of a certificate with no renewable domains. A diff --git a/internal/server/domain_renewal_test.go b/internal/server/domain_renewal_test.go index 0bc2b79..67b6fe8 100644 --- a/internal/server/domain_renewal_test.go +++ b/internal/server/domain_renewal_test.go @@ -3,6 +3,7 @@ package server import ( "errors" "fmt" + "sync" "testing" "time" @@ -10,6 +11,8 @@ import ( "github.com/go-acme/lego/v4/certificate" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + acmeconfig "github.com/basecamp/kamal-proxy/internal/server/acme" ) // fakeARIObtainer adds ACME Renewal Information support to fakeObtainer. @@ -502,3 +505,179 @@ func TestCertRenewer_QuarantinesCulpritsOnFailure(t *testing.T) { certs := manager.ManagedCertificates() require.Len(t, certs, 1) } + +func TestCertRenewer_ReissuesImmediatelyWhenDirectoryChanges(t *testing.T) { + obtainer := successfulObtainer(t) + manager := testSANCertManager(t) + + manager.SetDynamicDomains("service1", []string{"tenant.example.com"}) + + // A fresh certificate, nowhere near its renewal window — but the service + // has since flipped to a different ACME directory. + adoptTestCert(t, manager, []string{"tenant.example.com"}, + time.Now().Add(-24*time.Hour), time.Now().Add(89*24*time.Hour)) + manager.SetServiceDirectory("service1", LetsEncryptProduction) + + renewer := newCertRenewer(manager, newDomainQuarantine(), certRenewerConfig{Obtainer: obtainer}) + renewer.reconcile() + + require.Len(t, obtainer.Calls(), 1, "a directory switch must re-issue on the next reconcile") + + // The replacement records the new directory, so the next reconcile is quiet. + certs := manager.ManagedCertificates() + require.Len(t, certs, 1) + assert.Equal(t, LetsEncryptProduction, certs[0].Directory) + + renewer.reconcile() + assert.Len(t, obtainer.Calls(), 1) +} + +func TestCertRenewer_DirectoryChanged(t *testing.T) { + manager := testSANCertManager(t) + renewer := newCertRenewer(manager, newDomainQuarantine(), certRenewerConfig{Obtainer: successfulObtainer(t)}) + + manager.SetDynamicDomains("service1", []string{"tenant.example.com"}) + + legacy := &ManagedCert{Domains: []string{"tenant.example.com"}} + assert.False(t, renewer.directoryChanged(legacy), + "an empty recorded directory reads as the run-level one") + + manager.SetServiceDirectory("service1", LetsEncryptProduction) + assert.True(t, renewer.directoryChanged(legacy)) + + orphan := &ManagedCert{Domains: []string{"gone.example.net"}, Directory: LetsEncryptProduction} + assert.False(t, renewer.directoryChanged(orphan), + "a certificate with no resolvable owner keeps its recorded directory") +} + +func TestCertRenewer_SplitsMixedDirectoryCertificateAtRenewal(t *testing.T) { + obtainer := successfulObtainer(t) + manager := testSANCertManager(t) + + manager.SetDynamicDomains("plain-svc", []string{"plain.example.net"}) + manager.SetDynamicDomains("staged-svc", []string{"staged.example.com"}) + + // A legacy certificate spanning both services, issued at the run-level + // directory before the staged service flipped. + adoptTestCert(t, manager, []string{"plain.example.net", "staged.example.com"}, + time.Now().Add(-24*time.Hour), time.Now().Add(89*24*time.Hour)) + manager.SetServiceDirectory("staged-svc", LetsEncryptProduction) + + renewer := newCertRenewer(manager, newDomainQuarantine(), certRenewerConfig{Obtainer: obtainer}) + renewer.reconcile() + + calls := obtainer.Calls() + require.Len(t, calls, 2, "a mixed-directory certificate must split into one order per directory") + assert.Equal(t, []string{"plain.example.net"}, calls[0].Domains) + assert.Equal(t, []string{"staged.example.com"}, calls[1].Domains) + + // The ARI marker rides the order staying at the issuing CA; the switched + // partition sends none — the old identifier means nothing to the new CA. + assert.NotEmpty(t, calls[0].ReplacesCertID) + assert.Empty(t, calls[1].ReplacesCertID) + + // Each replacement records its owner's directory, and the mixed + // certificate is gone. + plainCert := manager.certificates[manager.certIDForDomain("plain.example.net")] + stagedCert := manager.certificates[manager.certIDForDomain("staged.example.com")] + require.NotNil(t, plainCert) + require.NotNil(t, stagedCert) + assert.Equal(t, manager.config.Directory, manager.normalizeDirectory(plainCert.Directory)) + assert.Equal(t, LetsEncryptProduction, stagedCert.Directory) + assert.Len(t, manager.ManagedCertificates(), 2) +} + +func TestCertRenewer_DirectorySwitchDropsARIReplaces(t *testing.T) { + obtainer := successfulObtainer(t) + manager := testSANCertManager(t) + + manager.SetDynamicDomains("service1", []string{"tenant.example.com"}) + adoptTestCert(t, manager, []string{"tenant.example.com"}, + time.Now().Add(-24*time.Hour), time.Now().Add(89*24*time.Hour)) + manager.SetServiceDirectory("service1", LetsEncryptProduction) + + renewer := newCertRenewer(manager, newDomainQuarantine(), certRenewerConfig{Obtainer: obtainer}) + renewer.reconcile() + + calls := obtainer.Calls() + require.Len(t, calls, 1) + assert.Empty(t, calls[0].ReplacesCertID, + "a replaces identifier from another CA leads the target CA to reject the order (RFC 9773)") +} + +// fakeDirectoryObtainer records which directory each pinned order was placed +// at, as managerObtainer does in production. +type fakeDirectoryObtainer struct { + *fakeObtainer + mu sync.Mutex + directories []string +} + +func (f *fakeDirectoryObtainer) ObtainAt(directory string, request certificate.ObtainRequest) (*certificate.Resource, error) { + f.mu.Lock() + f.directories = append(f.directories, directory) + f.mu.Unlock() + return f.Obtain(request) +} + +func TestCertRenewer_UnresolvedOwnerRenewsAtRecordedDirectory(t *testing.T) { + manager := testSANCertManager(t) + + // A wildcard certificate recorded at production, due for renewal. Its only + // covered member belongs to a run-level service, so it mismatches the + // cover and goes pending (getting a certificate of its own) — leaving the + // wildcard's owner unresolvable. The order must still be pinned to the + // recorded directory, not fall back to the run-level one. + adoptTestCert(t, manager, []string{"*.example.com"}, + time.Now().Add(-70*24*time.Hour), time.Now().Add(20*24*time.Hour)) + certID := manager.certIDForDomain("*.example.com") + manager.certificates[certID].Directory = LetsEncryptProduction + + require.NoError(t, manager.RegisterDomain("member.example.com", "plain-svc")) + _, pending := manager.pendingDomains["member.example.com"] + require.True(t, pending, "fixture: the member must be pending so the wildcard has no resolvable owner") + + obtainer := &fakeDirectoryObtainer{fakeObtainer: successfulObtainer(t)} + renewer := newCertRenewer(manager, newDomainQuarantine(), certRenewerConfig{Obtainer: obtainer}) + renewer.reconcile() + + require.Len(t, obtainer.Calls(), 1) + assert.Equal(t, []string{LetsEncryptProduction}, obtainer.directories, + "the order must be pinned to the certificate's recorded directory") + renewed := manager.certificates[manager.certIDForDomain("*.example.com")] + require.NotNil(t, renewed) + assert.Equal(t, LetsEncryptProduction, renewed.Directory, + "the replacement must record the directory that actually issued it") +} + +func TestCertRenewer_ARIMarkerSurvivesAFailedFirstPartition(t *testing.T) { + manager := testSANCertManager(t) + manager.selection.Zones = map[string]acmeconfig.ProviderName{ + "a.test": "cloudflare", + "b.test": "route53", + } + + manager.SetDynamicDomains("service1", []string{"x.a.test", "y.b.test"}) + adoptTestCert(t, manager, []string{"x.a.test", "y.b.test"}, + time.Now().Add(-70*24*time.Hour), time.Now().Add(20*24*time.Hour)) + + // Two same-directory provider partitions; the first order fails at the + // CA. The marker was not consumed, so the second order must carry it. + failed := false + obtainer := &fakeObtainer{respond: func(request certificate.ObtainRequest) (*certificate.Resource, error) { + if !failed { + failed = true + return nil, errors.New("boom") + } + return testCertResource(t, request.Domains, time.Now().Add(-time.Hour), time.Now().Add(90*24*time.Hour)), nil + }} + + renewer := newCertRenewer(manager, newDomainQuarantine(), certRenewerConfig{Obtainer: obtainer}) + renewer.reconcile() + + calls := obtainer.Calls() + require.Len(t, calls, 2) + assert.NotEmpty(t, calls[0].ReplacesCertID) + assert.NotEmpty(t, calls[1].ReplacesCertID, + "an order the CA refused leaves the ARI marker for the next same-directory partition") +} diff --git a/internal/server/dynamic_domains.go b/internal/server/dynamic_domains.go index 525263a..16799ed 100644 --- a/internal/server/dynamic_domains.go +++ b/internal/server/dynamic_domains.go @@ -556,8 +556,22 @@ func (o managerObtainer) Obtain(request certificate.ObtainRequest) (*certificate return o.manager.obtainCertificate(request) } +// ObtainAt pins the order to a directory the caller already resolved; the +// renewer uses it so a partition keeps its certificate's identity even when +// the domains' owners are temporarily unresolvable. +func (o managerObtainer) ObtainAt(directory string, request certificate.ObtainRequest) (*certificate.Resource, error) { + return o.manager.obtainCertificateAt(directory, request) +} + +// GetRenewalInfo asks the identity that issued the certificate — a staging +// cert's ARI lives at the staging CA, not the run-level one. func (o managerObtainer) GetRenewalInfo(request certificate.RenewalInfoRequest) (*certificate.RenewalInfoResponse, error) { - obtainer := o.manager.acmeCertifier() + var domains []string + if request.Cert != nil { + domains = request.Cert.DNSNames + } + + obtainer := o.manager.renewalInfoObtainer(domains) if obtainer == nil { return nil, ErrManagerNotReady } diff --git a/internal/server/san_cert_directories.go b/internal/server/san_cert_directories.go new file mode 100644 index 0000000..654c410 --- /dev/null +++ b/internal/server/san_cert_directories.go @@ -0,0 +1,426 @@ +package server + +import ( + "crypto/sha256" + "encoding/hex" + "fmt" + "log/slog" + "strings" + + "github.com/go-acme/lego/v4/certcrypto" + "github.com/go-acme/lego/v4/lego" + "github.com/go-acme/lego/v4/registration" + + "github.com/basecamp/kamal-proxy/internal/server/acme" +) + +// Per-service ACME directory selection. +// +// A service deployed with --tls-staging carries its own ACME directory in +// ServiceOptions; the shared SAN manager honors it by keeping one ACME +// identity per directory. The run-level directory keeps the original account +// and clients; any other directory gets its own account and client bundle, +// registered lazily on the first order that needs it. Issuance resolves the +// directory from the domains' owning service, and batches never mix +// directories — the dynamic issuer batches per service, the handshake path +// partitions by directory — so one order always has exactly one identity. + +// directoryClients is the ACME identity for one non-default directory. +type directoryClients struct { + user *acmeUser + httpObtainer certObtainer + dnsObtainer certObtainer + dnsObtainers map[acme.ProviderName]certObtainer +} + +// SetServiceDirectory records a service's ACME directory override. An empty +// directory, or the run-level one, clears the override. +func (m *SANCertManager) SetServiceDirectory(service, directory string) { + m.mu.Lock() + defer m.mu.Unlock() + + if directory == "" || directory == m.config.Directory { + delete(m.serviceDirectories, service) + return + } + m.serviceDirectories[service] = directory +} + +// directoryForService returns the ACME directory a service's certificates are +// issued against. +func (m *SANCertManager) directoryForService(service string) string { + m.mu.RLock() + defer m.mu.RUnlock() + + return m.directoryForServiceLocked(service) +} + +func (m *SANCertManager) directoryForServiceLocked(service string) string { + if directory, ok := m.serviceDirectories[service]; ok { + return directory + } + return m.config.Directory +} + +// ownerOf returns the service that owns a domain, whether registered at +// deploy time, learned from a domain source, or waiting in the pending batch. +func (m *SANCertManager) ownerOf(domain string) (string, bool) { + m.mu.RLock() + defer m.mu.RUnlock() + + return m.ownerOfLocked(domain) +} + +// ownerOfLocked resolves a domain's owning service; a wildcard resolves +// through the lexicographically smallest concrete domain it covers, so the +// answer does not depend on map iteration order. A pending entry with an +// empty service (a batch survivor restored by restorePending) does not name +// an owner. Callers must hold m.mu. +func (m *SANCertManager) ownerOfLocked(domain string) (string, bool) { + if strings.HasPrefix(domain, "*.") { + return m.wildcardOwnerLocked(domain, nil) + } + + if service, ok := m.registeredDomains[domain]; ok && service != "" { + return service, true + } + if service, ok := m.dynamicDomains[domain]; ok && service != "" { + return service, true + } + if service, ok := m.pendingDomains[domain]; ok && service != "" { + return service, true + } + return "", false +} + +// wildcardOwnerLocked resolves a wildcard's owner through the +// lexicographically smallest concrete domain it covers, so the answer does +// not depend on map iteration order. A pending covered domain never speaks +// for the wildcard — pending means it is getting a certificate of its own +// (possibly under a different directory) — and when cert is non-nil, neither +// does a domain this certificate no longer serves. Callers must hold m.mu. +func (m *SANCertManager) wildcardOwnerLocked(wildcard string, cert *ManagedCert) (string, bool) { + bestDomain, bestService := "", "" + scan := func(domains map[string]string) { + for covered, service := range domains { + if service == "" || !matchesWildcard(wildcard, covered) { + continue + } + if _, pending := m.pendingDomains[covered]; pending { + continue + } + if cert != nil && m.certIDCovering(covered) != cert.Identifier { + continue + } + if bestDomain == "" || covered < bestDomain { + bestDomain, bestService = covered, service + } + } + } + scan(m.registeredDomains) + scan(m.dynamicDomains) + return bestService, bestService != "" +} + +// certOwnedDirectoryLocked resolves the directory the owner of one of a +// certificate's domains wants; a wildcard member consults only the domains +// this certificate still serves. Callers must hold m.mu. +func (m *SANCertManager) certOwnedDirectoryLocked(cert *ManagedCert, domain string) (string, bool) { + var service string + var ok bool + if strings.HasPrefix(domain, "*.") { + service, ok = m.wildcardOwnerLocked(domain, cert) + } else { + service, ok = m.ownerOfLocked(domain) + } + if !ok { + return "", false + } + return m.directoryForServiceLocked(service), true +} + +// directoryForDomains resolves the directory an order for these domains must +// use. Concrete domains are consulted before wildcards: a wildcard synthesized +// from a batch appears alongside concrete members (at least the apex), and +// those members name the originating service directly, while a wildcard's +// coverage scan could land on any service under the apex. Batches are +// single-service by construction, so the first resolvable owner speaks for +// the whole order; with no owner at all, the run-level directory answers. +func (m *SANCertManager) directoryForDomains(domains []string) string { + m.mu.RLock() + defer m.mu.RUnlock() + + for _, domain := range domains { + if strings.HasPrefix(domain, "*.") { + continue + } + if service, ok := m.ownerOfLocked(domain); ok { + return m.directoryForServiceLocked(service) + } + } + for _, domain := range domains { + if service, ok := m.ownerOfLocked(domain); ok { + return m.directoryForServiceLocked(service) + } + } + return m.config.Directory +} + +// directoryForDomainLocked is directoryForDomains for one domain with m.mu +// already held, for the handshake batching path. +func (m *SANCertManager) directoryForDomainLocked(domain string) string { + if service, ok := m.ownerOfLocked(domain); ok { + return m.directoryForServiceLocked(service) + } + return m.config.Directory +} + +// certDirectoryMismatched reports whether ANY resolvable owner of the +// certificate's domains wants a different directory than the one that issued +// it — a legacy certificate can span services, and a single first-owner +// answer would let the other services' domains ride the wrong identity. A +// certificate with no resolvable owner at all keeps its recorded directory +// rather than churn against the default, because services may simply not +// have re-attached yet after a restart. +func (m *SANCertManager) certDirectoryMismatched(cert *ManagedCert) bool { + m.mu.RLock() + defer m.mu.RUnlock() + + recorded := m.normalizeDirectory(cert.Directory) + for _, domain := range cert.Domains { + if desired, ok := m.certOwnedDirectoryLocked(cert, domain); ok && desired != recorded { + return true + } + } + return false +} + +// certMatchesServiceDirectoryLocked reports whether a certificate was issued +// by the directory a service wants. Callers must hold m.mu. +func (m *SANCertManager) certMatchesServiceDirectoryLocked(cert *ManagedCert, service string) bool { + return m.normalizeDirectory(cert.Directory) == m.directoryForServiceLocked(service) +} + +// directoryPartition is one directory-homogeneous slice of a renewal order. +type directoryPartition struct { + directory string + domains []string +} + +// splitByDesiredDirectory partitions an identifier set so no ACME order spans +// directories: each domain goes with its owner's desired directory, and a +// domain with no resolvable owner rides with the certificate's recorded one. +// Partitions keep first-appearance order, so a sorted input yields +// deterministic output. +func (m *SANCertManager) splitByDesiredDirectory(cert *ManagedCert, domains []string) []directoryPartition { + m.mu.RLock() + defer m.mu.RUnlock() + + recorded := m.normalizeDirectory(cert.Directory) + + keys := []string{} + partitions := map[string][]string{} + for _, domain := range domains { + directory := recorded + if desired, ok := m.certOwnedDirectoryLocked(cert, domain); ok { + directory = desired + } + if _, ok := partitions[directory]; !ok { + keys = append(keys, directory) + } + partitions[directory] = append(partitions[directory], domain) + } + + split := make([]directoryPartition, 0, len(keys)) + for _, key := range keys { + split = append(split, directoryPartition{directory: key, domains: partitions[key]}) + } + return split +} + +// normalizeDirectory resolves an empty (legacy) recorded directory to the +// run-level one for comparison. +func (m *SANCertManager) normalizeDirectory(directory string) string { + if directory == "" { + return m.config.Directory + } + return directory +} + +// accountFileForDirectory names the account key file for a directory. The +// run-level directory keeps the original acme_user.json — existing accounts +// keep working across upgrades — the well-known Let's Encrypt staging URL +// gets a readable name, and anything else is keyed by a URL hash. +func (m *SANCertManager) accountFileForDirectory(directory string) string { + switch directory { + case m.config.Directory: + return acmeUserFile + case LetsEncryptStaging: + return "acme_user_staging.json" + } + + sum := sha256.Sum256([]byte(directory)) + return "acme_user_" + hex.EncodeToString(sum[:4]) + ".json" +} + +// clientsForDirectory returns the client bundle for a directory other than +// the run-level one, building and registering its account on first use. +func (m *SANCertManager) clientsForDirectory(directory string) (*directoryClients, error) { + m.mu.RLock() + bundle := m.directoryClients[directory] + m.mu.RUnlock() + + if bundle != nil { + return bundle, nil + } + + // Single-flight the build: account registration is a network call, and + // two concurrent orders must not register two accounts for one directory. + m.directoryInitMu.Lock() + defer m.directoryInitMu.Unlock() + + m.mu.RLock() + bundle = m.directoryClients[directory] + m.mu.RUnlock() + if bundle != nil { + return bundle, nil + } + + bundle, err := m.buildDirectoryClients(directory) + if err != nil { + return nil, fmt.Errorf("failed to set up ACME clients for %s: %w", directory, err) + } + + m.mu.Lock() + m.directoryClients[directory] = bundle + m.mu.Unlock() + + return bundle, nil +} + +// buildDirectoryClients constructs the ACME identity for one directory: its +// own account (loaded or created, registered if new) and obtainers mirroring +// the primary clients — HTTP-01 plus whatever DNS-01 solvers the run-level +// configuration names, so a zone mapping answers a staging order the same way +// it answers a production one. +func (m *SANCertManager) buildDirectoryClients(directory string) (*directoryClients, error) { + accountFile := m.accountFileForDirectory(directory) + + user, err := m.loadOrCreateUser(accountFile) + if err != nil { + return nil, fmt.Errorf("failed to setup ACME user: %w", err) + } + + legoConfig := lego.NewConfig(user) + legoConfig.CADirURL = directory + legoConfig.Certificate.KeyType = certcrypto.EC256 + + client, err := lego.NewClient(legoConfig) + if err != nil { + return nil, fmt.Errorf("failed to create ACME client: %w", err) + } + + // The challenge token map is shared: tokens are per-order, so one handler + // serves every identity. + if err := client.Challenge.SetHTTP01Provider(&memoryHTTP01Provider{manager: m}); err != nil { + return nil, fmt.Errorf("failed to set HTTP-01 provider: %w", err) + } + + if user.Registration == nil { + reg, err := client.Registration.Register(registration.RegisterOptions{ + TermsOfServiceAgreed: true, + }) + if err != nil { + return nil, fmt.Errorf("failed to register with ACME: %w", err) + } + user.Registration = reg + + if err := m.saveUser(user, accountFile); err != nil { + slog.Warn("Failed to save ACME user", "directory", directory, "error", err) + } + } + + dnsObtainer, dnsObtainers, err := m.buildDNSObtainers(user, directory) + if err != nil { + return nil, err + } + + slog.Info("ACME clients initialized for directory", + "directory", directory, "account_file", accountFile) + + return &directoryClients{ + user: user, + httpObtainer: client.Certificate, + dnsObtainer: dnsObtainer, + dnsObtainers: dnsObtainers, + }, nil +} + +// obtainersForDirectory resolves the clients answering one order at the given +// directory: the directory picks the ACME identity, and zone selection picks +// the DNS-01 solver within it. A nil DNS obtainer means HTTP-01 territory. +func (m *SANCertManager) obtainersForDirectory(directory string, domains []string) (httpObtainer, dnsObtainer certObtainer, err error) { + if directory == m.config.Directory { + dnsObtainer, err := m.orderObtainer(domains) + if err != nil { + return nil, nil, err + } + + m.mu.RLock() + httpObtainer := m.httpObtainer + m.mu.RUnlock() + + return httpObtainer, dnsObtainer, nil + } + + bundle, err := m.clientsForDirectory(directory) + if err != nil { + return nil, nil, err + } + + dnsObtainer, err = orderObtainerFrom(m.selection, domains, bundle.dnsObtainer, bundle.dnsObtainers) + if err != nil { + return nil, nil, err + } + + return bundle.httpObtainer, dnsObtainer, nil +} + +// renewalInfoObtainer returns the identity that should answer an ARI query +// for a certificate covering these domains: the one that issued it. A bundle +// that has not been built this run returns nil rather than registering an +// account for a read-only query; callers fall back to the renewal window +// heuristic. +func (m *SANCertManager) renewalInfoObtainer(domains []string) renewalInfoGetter { + directory := m.config.Directory + if len(domains) > 0 { + if certID := m.certIDForDomain(domains[0]); certID != "" { + m.mu.RLock() + if cert := m.certificates[certID]; cert != nil { + directory = m.normalizeDirectory(cert.Directory) + } + m.mu.RUnlock() + } + } + + if directory == m.config.Directory { + certifier := m.acmeCertifier() + if certifier == nil { + return nil + } + return certifier + } + + m.mu.RLock() + bundle := m.directoryClients[directory] + m.mu.RUnlock() + if bundle == nil { + return nil + } + + getter, ok := bundle.httpObtainer.(renewalInfoGetter) + if !ok { + return nil + } + return getter +} diff --git a/internal/server/san_cert_directories_test.go b/internal/server/san_cert_directories_test.go new file mode 100644 index 0000000..16acd8e --- /dev/null +++ b/internal/server/san_cert_directories_test.go @@ -0,0 +1,392 @@ +package server + +import ( + "context" + "net/http" + "os" + "testing" + "time" + + "github.com/go-acme/lego/v4/certificate" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestSANCertManager_SetServiceDirectory(t *testing.T) { + manager := testSANCertManager(t) + + // No override: the run-level directory answers. + assert.Equal(t, manager.config.Directory, manager.directoryForService("web")) + + manager.SetServiceDirectory("web", LetsEncryptProduction) + assert.Equal(t, LetsEncryptProduction, manager.directoryForService("web")) + assert.Equal(t, manager.config.Directory, manager.directoryForService("other")) + + // Setting the run-level directory explicitly is the same as no override. + manager.SetServiceDirectory("web", manager.config.Directory) + assert.Equal(t, manager.config.Directory, manager.directoryForService("web")) + + manager.SetServiceDirectory("web", LetsEncryptProduction) + manager.SetServiceDirectory("web", "") + assert.Equal(t, manager.config.Directory, manager.directoryForService("web")) +} + +func TestSANCertManager_DirectoryForDomains(t *testing.T) { + manager := testSANCertManager(t) + manager.SetServiceDirectory("staging-svc", LetsEncryptProduction) + + // The two services own domains under different roots: a wildcard's owner + // is resolved through the concrete domains it covers, so a fixture where + // one wildcard covered both services would be ambiguous by construction. + require.NoError(t, manager.RegisterDomain("registered.example.com", "staging-svc")) + require.NoError(t, manager.RegisterDomain("default.example.net", "plain-svc")) + manager.SetDynamicDomains("staging-svc", []string{"dynamic.example.com"}) + + tests := []struct { + name string + domains []string + expected string + }{ + {"registered domain follows its service", []string{"registered.example.com"}, LetsEncryptProduction}, + {"dynamic domain follows its service", []string{"dynamic.example.com"}, LetsEncryptProduction}, + {"domain of a service with no override uses the default", []string{"default.example.net"}, manager.config.Directory}, + {"unknown domain falls back to the default", []string{"nobody.example.org"}, manager.config.Directory}, + {"wildcard resolves through a covered domain", []string{"*.example.com"}, LetsEncryptProduction}, + {"first resolvable owner decides", []string{"nobody.example.net", "registered.example.com"}, LetsEncryptProduction}, + {"a concrete owner outranks a wildcard's coverage scan", []string{"*.example.com", "default.example.net"}, manager.config.Directory}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.expected, manager.directoryForDomains(tt.domains)) + }) + } +} + +func TestSANCertManager_RegisteredDomainsTrackTheirOwner(t *testing.T) { + manager := testSANCertManager(t) + + require.NoError(t, manager.RegisterDomain("app.example.com", "web")) + + service, ok := manager.ownerOf("app.example.com") + require.True(t, ok) + assert.Equal(t, "web", service) + + require.NoError(t, manager.UnregisterDomain("app.example.com", "web")) + _, ok = manager.ownerOf("app.example.com") + assert.False(t, ok) +} + +func TestSANCertManager_AccountFileForDirectory(t *testing.T) { + manager := testSANCertManager(t) // config.Directory is LetsEncryptStaging here + + tests := []struct { + name string + directory string + expected string + }{ + {"run-level directory keeps the original file", manager.config.Directory, "acme_user.json"}, + {"well-known staging gets a readable name", "https://acme-staging-v02.api.letsencrypt.org/directory", "acme_user.json"}, + {"production override gets its own file", LetsEncryptProduction, "acme_user_5e76d315.json"}, + {"any other directory is keyed by URL hash", "https://acme.example.com/directory", "acme_user_05e6df34.json"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.expected, manager.accountFileForDirectory(tt.directory)) + }) + } +} + +func TestSANCertManager_AccountFileForStagingWhenDefaultIsProduction(t *testing.T) { + manager := testSANCertManager(t) + manager.config.Directory = LetsEncryptProduction + + assert.Equal(t, "acme_user.json", manager.accountFileForDirectory(LetsEncryptProduction)) + assert.Equal(t, "acme_user_staging.json", manager.accountFileForDirectory(LetsEncryptStaging)) +} + +func TestSANCertManager_CertDirectoryMismatched(t *testing.T) { + manager := testSANCertManager(t) + manager.SetServiceDirectory("staging-svc", LetsEncryptProduction) + require.NoError(t, manager.RegisterDomain("app.example.com", "staging-svc")) + require.NoError(t, manager.RegisterDomain("plain.example.net", "plain-svc")) + + matching := &ManagedCert{Domains: []string{"app.example.com"}, Directory: LetsEncryptProduction} + assert.False(t, manager.certDirectoryMismatched(matching)) + + legacy := &ManagedCert{Domains: []string{"app.example.com"}} + assert.True(t, manager.certDirectoryMismatched(legacy), + "an empty recorded directory reads as the run-level one, which this owner has moved away from") + + // A legacy multi-service SAN: the plain domain matches the recorded + // directory, but ANY mismatched owner flags the certificate — a single + // first-owner answer would let the staged domain ride the wrong identity. + mixed := &ManagedCert{Domains: []string{"plain.example.net", "app.example.com"}} + assert.True(t, manager.certDirectoryMismatched(mixed)) + + orphan := &ManagedCert{Domains: []string{"gone.example.org"}, Directory: LetsEncryptProduction} + assert.False(t, manager.certDirectoryMismatched(orphan), + "a certificate with no resolvable owner keeps its recorded directory") +} + +func TestSANCertManager_WildcardOwnerConsultsOnlyDomainsTheCertServes(t *testing.T) { + manager := testSANCertManager(t) + manager.SetServiceDirectory("staged-svc", LetsEncryptProduction) + + wildcard := testSelfSignedCert(t, []string{"*.example.com"}, + time.Now().Add(-time.Hour), time.Now().Add(60*24*time.Hour)) + managed := &ManagedCert{ + Identifier: "wildcard", + Domains: []string{"*.example.com"}, + NotAfter: wildcard.Leaf.NotAfter, + Certificate: wildcard, + } + manager.certificates["wildcard"] = managed + manager.domainToCert["*.example.com"] = "wildcard" + + // A member riding the wildcard anchors its directory. + require.NoError(t, manager.RegisterDomain("member.example.com", "plain-svc")) + assert.False(t, manager.certDirectoryMismatched(managed)) + + // A staged host under the same apex goes pending for its own certificate; + // it must NOT drag the wildcard onto its directory. + require.NoError(t, manager.RegisterDomain("app.example.com", "staged-svc")) + _, pending := manager.pendingDomains["app.example.com"] + require.True(t, pending) + assert.False(t, manager.certDirectoryMismatched(managed), + "a pending covered domain must not speak for the wildcard certificate") +} + +func TestSANCertManager_AdoptCertificateStampsDirectory(t *testing.T) { + manager := testSANCertManager(t) + manager.SetServiceDirectory("prod-svc", LetsEncryptProduction) + require.NoError(t, manager.RegisterDomain("app.example.com", "prod-svc")) + require.NoError(t, manager.RegisterDomain("plain.example.com", "plain-svc")) + + notBefore, notAfter := time.Now().Add(-time.Hour), time.Now().Add(90*24*time.Hour) + + overridden, err := manager.adoptCertificate( + testCertResource(t, []string{"app.example.com"}, notBefore, notAfter), []string{"app.example.com"}) + require.NoError(t, err) + assert.Equal(t, LetsEncryptProduction, overridden.Directory) + + plain, err := manager.adoptCertificate( + testCertResource(t, []string{"plain.example.com"}, notBefore, notAfter), []string{"plain.example.com"}) + require.NoError(t, err) + assert.Equal(t, manager.config.Directory, plain.Directory) +} + +func TestSANCertManager_StateRoundTripsDirectory(t *testing.T) { + manager := testSANCertManager(t) + manager.SetServiceDirectory("prod-svc", LetsEncryptProduction) + require.NoError(t, manager.RegisterDomain("app.example.com", "prod-svc")) + + _, err := manager.adoptCertificate( + testCertResource(t, []string{"app.example.com"}, time.Now().Add(-time.Hour), time.Now().Add(90*24*time.Hour)), + []string{"app.example.com"}) + require.NoError(t, err) + + reloaded, err := NewSANCertManager(manager.config) + require.NoError(t, err) + require.NoError(t, reloaded.loadState()) + + certID := reloaded.domainToCert["app.example.com"] + require.NotEmpty(t, certID) + assert.Equal(t, LetsEncryptProduction, reloaded.certificates[certID].Directory) +} + +func TestSANCertManager_OldStateWithoutDirectoryLoadsAsDefault(t *testing.T) { + manager := testSANCertManager(t) + + legacy := `{"certificates":{"legacy.example.com":{"identifier":"legacy.example.com","domains":["legacy.example.com"],"not_after":"2030-01-01T00:00:00Z"}},"domain_map":{"legacy.example.com":"legacy.example.com"}}` + require.NoError(t, os.WriteFile(manager.config.StatePath, []byte(legacy), 0600)) + + require.NoError(t, manager.loadState()) + + cert := manager.certificates["legacy.example.com"] + require.NotNil(t, cert) + assert.Empty(t, cert.Directory) + assert.Equal(t, manager.config.Directory, manager.normalizeDirectory(cert.Directory)) +} + +func TestSANCertManager_ObtainCertificateUsesTheOwnersDirectory(t *testing.T) { + manager := testSANCertManager(t) + defaultObtainer := successfulObtainer(t) + manager.httpObtainer = defaultObtainer + + prodObtainer := successfulObtainer(t) + manager.directoryClients[LetsEncryptProduction] = &directoryClients{httpObtainer: prodObtainer} + + manager.SetServiceDirectory("prod-svc", LetsEncryptProduction) + require.NoError(t, manager.RegisterDomain("prod.example.com", "prod-svc")) + require.NoError(t, manager.RegisterDomain("plain.example.com", "plain-svc")) + + _, err := manager.obtainCertificate(certificate.ObtainRequest{Domains: []string{"prod.example.com"}}) + require.NoError(t, err) + assert.Len(t, prodObtainer.Calls(), 1, "the overridden service's order must use its own directory") + assert.Empty(t, defaultObtainer.Calls()) + + _, err = manager.obtainCertificate(certificate.ObtainRequest{Domains: []string{"plain.example.com"}}) + require.NoError(t, err) + assert.Len(t, defaultObtainer.Calls(), 1, "a service without an override stays on the run-level directory") + assert.Len(t, prodObtainer.Calls(), 1) +} + +func TestSANCertManager_ObtainCertificatePrefersTheBundlesDNSObtainer(t *testing.T) { + manager := testSANCertManager(t) + manager.httpObtainer = successfulObtainer(t) + + bundleDNS := successfulObtainer(t) + bundleHTTP := successfulObtainer(t) + manager.directoryClients[LetsEncryptProduction] = &directoryClients{ + httpObtainer: bundleHTTP, + dnsObtainer: bundleDNS, + } + + manager.SetServiceDirectory("prod-svc", LetsEncryptProduction) + require.NoError(t, manager.RegisterDomain("prod.example.com", "prod-svc")) + + _, err := manager.obtainCertificate(certificate.ObtainRequest{Domains: []string{"prod.example.com"}}) + require.NoError(t, err) + assert.Len(t, bundleDNS.Calls(), 1, "a bundle with a DNS solver answers over DNS-01 first") + assert.Empty(t, bundleHTTP.Calls()) +} + +func TestSANCertManager_HandshakeBatchNeverMixesDirectories(t *testing.T) { + manager := testSANCertManager(t) + defaultObtainer := successfulObtainer(t) + manager.httpObtainer = defaultObtainer + + prodObtainer := successfulObtainer(t) + manager.directoryClients[LetsEncryptProduction] = &directoryClients{httpObtainer: prodObtainer} + + manager.SetServiceDirectory("prod-svc", LetsEncryptProduction) + require.NoError(t, manager.RegisterDomain("plain-a.example.com", "plain-svc")) + require.NoError(t, manager.RegisterDomain("plain-b.example.com", "plain-svc")) + require.NoError(t, manager.RegisterDomain("prod.example.com", "prod-svc")) + + _, err := manager.provisionCertificate(context.Background(), "plain-a.example.com") + require.NoError(t, err) + + require.Len(t, defaultObtainer.Calls(), 1) + ordered := defaultObtainer.Calls()[0].Domains + assert.ElementsMatch(t, []string{"plain-a.example.com", "plain-b.example.com"}, ordered, + "the batch must take same-directory pending mates and leave the overridden service's domain out") + assert.Empty(t, prodObtainer.Calls()) + + // The overridden domain kept its pending slot for a batch of its own. + _, err = manager.provisionCertificate(context.Background(), "prod.example.com") + require.NoError(t, err) + require.Len(t, prodObtainer.Calls(), 1) + assert.Equal(t, []string{"prod.example.com"}, prodObtainer.Calls()[0].Domains) +} + +func TestRouter_DeployRegistersServiceDirectoryWithSANManager(t *testing.T) { + router := testRouter(t) + manager := testSANCertManager(t) + router.SetSANCertManager(manager) + + _, target := testBackend(t, "first", http.StatusOK) + + serviceOptions := defaultServiceOptions + serviceOptions.TLSEnabled = true + serviceOptions.Hosts = []string{"app.example.com"} + serviceOptions.ACMEDirectory = LetsEncryptProduction + + require.NoError(t, router.DeployService("web", []string{target}, defaultEmptyReaders, + serviceOptions, defaultTargetOptions, defaultDeploymentOptions)) + + assert.Equal(t, LetsEncryptProduction, manager.directoryForService("web"), + "a deploy with --tls-staging (a per-service directory) must reach the shared SAN manager") + + // Redeploying without the override clears it. + serviceOptions.ACMEDirectory = "" + require.NoError(t, router.DeployService("web", []string{target}, defaultEmptyReaders, + serviceOptions, defaultTargetOptions, defaultDeploymentOptions)) + + assert.Equal(t, manager.config.Directory, manager.directoryForService("web")) +} + +func TestSANCertManager_RegisterDomainRejectsCoverageFromAnotherDirectory(t *testing.T) { + manager := testSANCertManager(t) + manager.SetServiceDirectory("staged-svc", LetsEncryptProduction) + + // A valid wildcard issued at the run-level directory covers both hosts. + wildcard := testSelfSignedCert(t, []string{"*.example.com"}, + time.Now().Add(-time.Hour), time.Now().Add(60*24*time.Hour)) + manager.certificates["wildcard"] = &ManagedCert{ + Identifier: "wildcard", + Domains: []string{"*.example.com"}, + NotAfter: wildcard.Leaf.NotAfter, + Certificate: wildcard, + } + manager.domainToCert["*.example.com"] = "wildcard" + + require.NoError(t, manager.RegisterDomain("app.example.com", "staged-svc")) + + _, pending := manager.pendingDomains["app.example.com"] + assert.True(t, pending, "a covering certificate from another directory must not satisfy the registration") + assert.False(t, manager.HasValidCertificate("app.example.com"), + "the wrong-directory cover does not count as valid for its owner") + + // A same-directory sibling still takes the fast path. + require.NoError(t, manager.RegisterDomain("other.example.com", "plain-svc")) + _, pending = manager.pendingDomains["other.example.com"] + assert.False(t, pending) + assert.True(t, manager.HasValidCertificate("other.example.com")) +} + +func TestRouter_RedeployOffTheSANManagerClearsTheDirectoryOverride(t *testing.T) { + router := testRouter(t) + manager := testSANCertManager(t) + router.SetSANCertManager(manager) + + _, target := testBackend(t, "first", http.StatusOK) + + serviceOptions := defaultServiceOptions + serviceOptions.TLSEnabled = true + serviceOptions.Hosts = []string{"app.example.com"} + serviceOptions.ACMEDirectory = LetsEncryptProduction + + require.NoError(t, router.DeployService("web", []string{target}, defaultEmptyReaders, + serviceOptions, defaultTargetOptions, defaultDeploymentOptions)) + require.Equal(t, LetsEncryptProduction, manager.directoryForService("web")) + + // Redeploying with a static certificate leaves the shared manager behind; + // the override must not linger, or retained certificates would keep + // renewing against the old identity. + certPath, keyPath := prepareTestCertificateFiles(t) + serviceOptions.TLSCertificatePath = certPath + serviceOptions.TLSPrivateKeyPath = keyPath + + require.NoError(t, router.DeployService("web", []string{target}, defaultEmptyReaders, + serviceOptions, defaultTargetOptions, defaultDeploymentOptions)) + + assert.Equal(t, manager.config.Directory, manager.directoryForService("web")) +} + +func TestIsExtraAccountKeyFile(t *testing.T) { + tests := []struct { + name string + expected bool + }{ + {"acme_user_staging.json", true}, + {"acme_user_5e76d315.json", true}, + {"acme_user_05e6df34.json", true}, + {"acme_user.json", false}, + {"acme_user_.json", false}, + {"acme_user_5e76d31.json", false}, // 7 hex chars + {"acme_user_5e76d3155.json", false}, // 9 hex chars + {"acme_user_5E76D315.json", false}, // uppercase + {"acme_user_evil.json", false}, + {"acme_user_staging.json.bak", false}, + {"acme_user_deadbeef.txt", false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.expected, isExtraAccountKeyFile(tt.name)) + }) + } +} diff --git a/internal/server/san_cert_dynamic.go b/internal/server/san_cert_dynamic.go index ac740f9..4c97471 100644 --- a/internal/server/san_cert_dynamic.go +++ b/internal/server/san_cert_dynamic.go @@ -106,7 +106,9 @@ func (m *SANCertManager) HasCertificate(domain string) bool { } // HasValidCertificate reports whether the domain is covered by a loaded -// certificate that is not due for replacement. +// certificate that is not due for replacement. A certificate issued by a +// different directory than the domain's owner wants does not count: it is +// exactly what the issuer must replace after a --tls-staging flip. func (m *SANCertManager) HasValidCertificate(domain string) bool { m.mu.RLock() defer m.mu.RUnlock() @@ -117,7 +119,14 @@ func (m *SANCertManager) HasValidCertificate(domain string) bool { } cert := m.certificates[certID] - return cert != nil && cert.Certificate != nil && time.Until(cert.NotAfter) > 24*time.Hour + if cert == nil || cert.Certificate == nil || time.Until(cert.NotAfter) <= 24*time.Hour { + return false + } + + if service, ok := m.ownerOfLocked(domain); ok && !m.certMatchesServiceDirectoryLocked(cert, service) { + return false + } + return true } // requestDynamicCertificate asks the issuer (when wired) to provision a diff --git a/internal/server/san_cert_issuance.go b/internal/server/san_cert_issuance.go index 093c621..b73cbf3 100644 --- a/internal/server/san_cert_issuance.go +++ b/internal/server/san_cert_issuance.go @@ -26,13 +26,21 @@ import ( // It deliberately does NOT take a rate limit token: callers hold one already, // so that a queued order waits before it is assembled rather than after. func (m *SANCertManager) obtainCertificate(request certificate.ObtainRequest) (*certificate.Resource, error) { - dnsObtainer, err := m.orderObtainer(request.Domains) + return m.obtainCertificateAt(m.directoryForDomains(request.Domains), request) +} + +// obtainCertificateAt is obtainCertificate with the ACME directory pinned by +// the caller — the renewer pins a partition to its computed directory so a +// domain whose owner is temporarily unresolvable still renews under its +// certificate's recorded identity instead of falling back to the run-level +// one. +func (m *SANCertManager) obtainCertificateAt(directory string, request certificate.ObtainRequest) (*certificate.Resource, error) { + httpObtainer, dnsObtainer, err := m.obtainersForDirectory(directory, request.Domains) if err != nil { return nil, err } m.mu.RLock() - httpObtainer := m.httpObtainer httpFallback := m.config.HTTPFallback m.mu.RUnlock() diff --git a/internal/server/san_cert_manager.go b/internal/server/san_cert_manager.go index 2bfcb2d..8f361ac 100644 --- a/internal/server/san_cert_manager.go +++ b/internal/server/san_cert_manager.go @@ -128,8 +128,9 @@ type SANCertManager struct { // Pending domains waiting to be batched: domain -> service name pendingDomains map[string]string - // Deploy-registered hosts allowed to provision synchronously - registeredDomains map[string]struct{} + // Deploy-registered hosts allowed to provision synchronously: + // domain -> service name + registeredDomains map[string]string // Runtime-learned domains from tls-domains-source: domain -> service name dynamicDomains map[string]string @@ -141,6 +142,15 @@ type SANCertManager struct { // (see san_cert_batch_guard.go); zero value means unguarded. guard issuanceGuard + // Per-service ACME directory overrides and the lazily built client + // bundles for non-default directories; see san_cert_directories.go. + serviceDirectories map[string]string + directoryClients map[string]*directoryClients + + // directoryInitMu single-flights lazy bundle construction, which does + // network I/O and so cannot ride m.mu. + directoryInitMu sync.Mutex + // Currently provisioning: rootDomain -> done channel provisioning map[string]chan struct{} @@ -157,6 +167,11 @@ type ManagedCert struct { Domains []string `json:"domains"` NotAfter time.Time `json:"not_after"` Certificate *tls.Certificate `json:"-"` // Not persisted, loaded from files + + // Directory is the ACME directory that issued this certificate. Empty in + // state written before per-service directories existed, which reads as + // the run-level directory (normalizeDirectory). + Directory string `json:"directory,omitempty"` } // http01Challenge is one presented challenge: the key authorization to serve, @@ -198,14 +213,16 @@ func NewSANCertManager(config SANCertManagerConfig) (*SANCertManager, error) { Default: config.DNSProvider, Zones: config.DNSProviderZones, }, - bucket: newTokenBucket(DefaultIssuanceBurst, DefaultIssuanceRefillInterval), - certificates: make(map[string]*ManagedCert), - domainToCert: make(map[string]string), - pendingDomains: make(map[string]string), - registeredDomains: make(map[string]struct{}), - dynamicDomains: make(map[string]string), - provisioning: make(map[string]chan struct{}), - challengeTokens: make(map[string]http01Challenge), + bucket: newTokenBucket(DefaultIssuanceBurst, DefaultIssuanceRefillInterval), + certificates: make(map[string]*ManagedCert), + domainToCert: make(map[string]string), + pendingDomains: make(map[string]string), + registeredDomains: make(map[string]string), + dynamicDomains: make(map[string]string), + serviceDirectories: make(map[string]string), + directoryClients: make(map[string]*directoryClients), + provisioning: make(map[string]chan struct{}), + challengeTokens: make(map[string]http01Challenge), } // Ensure cache directory exists @@ -251,7 +268,7 @@ func (m *SANCertManager) initializeClients() error { defer m.mu.Unlock() // Load or create ACME user - user, err := m.loadOrCreateUser() + user, err := m.loadOrCreateUser(acmeUserFile) if err != nil { return fmt.Errorf("failed to setup ACME user: %w", err) } @@ -288,7 +305,7 @@ func (m *SANCertManager) initializeClients() error { user.Registration = reg // Save user with registration - if err := m.saveUser(); err != nil { + if err := m.saveUser(user, acmeUserFile); err != nil { slog.Warn("Failed to save ACME user", "error", err) } } @@ -342,12 +359,16 @@ func (m *SANCertManager) RegisterDomain(domain string, service string) error { return ErrManagerNotReady } - m.registeredDomains[domain] = struct{}{} + m.registeredDomains[domain] = service - // Check if domain already has a certificate, its own or a wildcard's + // Check if domain already has a certificate, its own or a wildcard's. A + // covering certificate from a DIFFERENT directory (a staging host under a + // production wildcard, say) does not satisfy the registration: the domain + // stays pending so its own service's identity issues for it. if certID := m.certIDCovering(domain); certID != "" { cert := m.certificates[certID] - if cert != nil && time.Until(cert.NotAfter) > 24*time.Hour { + if cert != nil && time.Until(cert.NotAfter) > 24*time.Hour && + m.certMatchesServiceDirectoryLocked(cert, service) { slog.Debug("Domain already has valid certificate", "domain", domain, "certificate", certID, @@ -361,6 +382,9 @@ func (m *SANCertManager) RegisterDomain(domain string, service string) error { if time.Until(cert.NotAfter) <= 24*time.Hour { continue } + if !m.certMatchesServiceDirectoryLocked(cert, service) { + continue + } for _, d := range cert.Domains { if d == domain { m.domainToCert[domain] = cert.Identifier @@ -413,8 +437,17 @@ func (m *SANCertManager) GetCertificate(hello *tls.ClientHelloInfo) (*tls.Certif if certID := m.certIDCovering(domain); certID != "" { cert = m.certificates[certID] } - _, isRegistered := m.registeredDomains[domain] + owner, isRegistered := m.registeredDomains[domain] dynamicService, isDynamic := m.dynamicDomains[domain] + if !isRegistered { + owner = dynamicService + } + // A domain covered only by another directory's certificate (a staging + // host under a production wildcard, say) needs one of its own, exactly as + // if the covering certificate were expiring: registered domains + // reprovision on the handshake, dynamic ones through the issuer. + directoryMismatch := cert != nil && (isRegistered || isDynamic) && + !m.certMatchesServiceDirectoryLocked(cert, owner) m.mu.RUnlock() if !ready { @@ -423,15 +456,22 @@ func (m *SANCertManager) GetCertificate(hello *tls.ClientHelloInfo) (*tls.Certif if cert != nil && cert.Certificate != nil { // Return existing valid certificate - if time.Until(cert.NotAfter) > 24*time.Hour { + if time.Until(cert.NotAfter) > 24*time.Hour && !directoryMismatch { return cert.Certificate, nil } if isRegistered { - slog.Info("Certificate expiring soon, will reprovision", - "domain", domain, - "expiresAt", cert.NotAfter, - ) + if directoryMismatch { + slog.Info("Covering certificate is from another ACME directory, will reprovision", + "domain", domain, + "certificate_directory", cert.Directory, + ) + } else { + slog.Info("Certificate expiring soon, will reprovision", + "domain", domain, + "expiresAt", cert.NotAfter, + ) + } } else if time.Until(cert.NotAfter) > 0 { // Dynamic and evicted domains keep serving a still-valid // certificate; the renewal loop is responsible for rotating it. @@ -491,7 +531,11 @@ func (m *SANCertManager) provisionCertificate(ctx context.Context, domain string // Collect ALL pending domains (up to MaxSANsPerCertificate). Quarantined // domains do not consume batch slots: with more quarantined hosts than a - // batch holds, the eligible ones must still fit. + // batch holds, the eligible ones must still fit. One order has exactly one + // ACME identity, so only batch-mates sharing the requested domain's + // directory join; the rest keep their pending slot for a batch of their + // own. + batchDirectory := m.directoryForDomainLocked(domain) candidates := []string{domain} for pendingDomain := range m.pendingDomains { if pendingDomain == domain { @@ -500,6 +544,9 @@ func (m *SANCertManager) provisionCertificate(ctx context.Context, domain string if m.guard.quarantine != nil && m.guard.quarantine.IsQuarantined(pendingDomain) { continue } + if m.directoryForDomainLocked(pendingDomain) != batchDirectory { + continue + } candidates = append(candidates, pendingDomain) if len(candidates) >= MaxSANsPerCertificate { break @@ -595,6 +642,13 @@ func (m *SANCertManager) provisionCertificate(ctx context.Context, domain string // maps, and persists it. sortedDomains must be the sorted identifier set the // certificate was ordered for. func (m *SANCertManager) adoptCertificate(resource *certificate.Resource, sortedDomains []string) (*ManagedCert, error) { + return m.adoptCertificateAt(resource, sortedDomains, m.directoryForDomains(sortedDomains)) +} + +// adoptCertificateAt is adoptCertificate with the recorded directory pinned +// by the caller — used when the order itself was pinned, so the stamp always +// names the directory that actually issued. +func (m *SANCertManager) adoptCertificateAt(resource *certificate.Resource, sortedDomains []string, directory string) (*ManagedCert, error) { // Parse the certificate tlsCert, err := tls.X509KeyPair(resource.Certificate, resource.PrivateKey) if err != nil { @@ -616,6 +670,7 @@ func (m *SANCertManager) adoptCertificate(resource *certificate.Resource, sorted Domains: sortedDomains, NotAfter: notAfter, Certificate: &tlsCert, + Directory: directory, } // The maps are published and the files written under one hold of the @@ -738,8 +793,8 @@ func (m *SANCertManager) GetStats() map[string]interface{} { // Persistence methods -func (m *SANCertManager) loadOrCreateUser() (*acmeUser, error) { - userPath := filepath.Join(m.config.CachePath, acmeUserFile) +func (m *SANCertManager) loadOrCreateUser(filename string) (*acmeUser, error) { + userPath := filepath.Join(m.config.CachePath, filename) data, err := os.ReadFile(userPath) if err == nil { @@ -770,20 +825,20 @@ func (m *SANCertManager) loadOrCreateUser() (*acmeUser, error) { return user, nil } -func (m *SANCertManager) saveUser() error { +func (m *SANCertManager) saveUser(user *acmeUser, filename string) error { if m.config.CachePath == "" { return nil } - keyPEM := certcrypto.PEMEncode(m.user.Key) - m.user.KeyPEM = keyPEM + keyPEM := certcrypto.PEMEncode(user.Key) + user.KeyPEM = keyPEM - data, err := json.MarshalIndent(m.user, "", " ") + data, err := json.MarshalIndent(user, "", " ") if err != nil { return err } - userPath := filepath.Join(m.config.CachePath, acmeUserFile) + userPath := filepath.Join(m.config.CachePath, filename) return os.WriteFile(userPath, data, 0600) } diff --git a/internal/server/san_cert_zones.go b/internal/server/san_cert_zones.go index 28ac2be..5efeaa8 100644 --- a/internal/server/san_cert_zones.go +++ b/internal/server/san_cert_zones.go @@ -21,16 +21,43 @@ import ( // the same ACME account) and routes each order to the provider its zone // names, with the single default provider answering for everything else. -// initDNSClients builds the DNS-01 clients the configuration names. Must be -// called with m.mu held. +// initDNSClients builds the DNS-01 clients the configuration names, on the +// primary account. Must be called with m.mu held. +func (m *SANCertManager) initDNSClients() error { + def, zoned, err := m.buildDNSObtainers(m.user, m.config.Directory) + if err != nil { + return err + } + + if len(zoned) > 0 { + m.dnsObtainers = zoned + slog.Info("Per-zone DNS-01 challenge solvers initialized", + "zones", slices.Sorted(maps.Keys(m.selection.Zones))) + } + + if def != nil { + m.dnsObtainer = def + slog.Info("DNS-01 challenge solver initialized", "provider", m.config.DNSProvider) + } + + if m.dnsObtainer != nil || len(m.dnsObtainers) > 0 { + m.grouper.DNSProviderAvailable = true + } + + return nil +} + +// buildDNSObtainers builds the DNS-01 clients the configuration names, on the +// given ACME identity — the primary account at boot, or a per-service +// directory's account when its bundle is built. // // The two configuration forms fail differently on a broken provider: an // explicit zone mapping is explicit intent — silently continuing without its // provider is the exact failure per-zone selection exists to remove — so it -// fails the boot outright. The default provider keeps its existing softness: -// with HTTP fallback on, a provider that cannot be constructed logs and -// leaves issuance on HTTP-01. -func (m *SANCertManager) initDNSClients() error { +// fails the caller outright. The default provider keeps its existing +// softness: with HTTP fallback on, a provider that cannot be constructed logs +// and leaves issuance on HTTP-01. +func (m *SANCertManager) buildDNSObtainers(user *acmeUser, directory string) (certObtainer, map[acme.ProviderName]certObtainer, error) { clients := map[acme.ProviderName]certObtainer{} for _, zone := range slices.Sorted(maps.Keys(m.selection.Zones)) { @@ -39,50 +66,43 @@ func (m *SANCertManager) initDNSClients() error { continue } - obtainer, err := m.newDNSObtainer(name) + obtainer, err := m.newDNSObtainer(user, directory, name) if err != nil { - return fmt.Errorf("DNS provider %q for zone %q: %w", name, zone, err) + return nil, nil, fmt.Errorf("DNS provider %q for zone %q: %w", name, zone, err) } clients[name] = obtainer } - - if len(clients) > 0 { - m.dnsObtainers = clients - slog.Info("Per-zone DNS-01 challenge solvers initialized", - "zones", slices.Sorted(maps.Keys(m.selection.Zones))) + if len(clients) == 0 { + clients = nil } + var def certObtainer if m.config.DNSProvider != "" && m.config.DNSProvider != "none" { - obtainer, err := m.newDNSObtainer(m.config.DNSProvider) + obtainer, err := m.newDNSObtainer(user, directory, m.config.DNSProvider) if err != nil { if !m.config.HTTPFallback { - return fmt.Errorf("failed to create DNS provider %q: %w", m.config.DNSProvider, err) + return nil, nil, fmt.Errorf("failed to create DNS provider %q: %w", m.config.DNSProvider, err) } slog.Warn("DNS provider not available, staying on HTTP-01", "provider", m.config.DNSProvider, "error", err) } else { - m.dnsObtainer = obtainer - slog.Info("DNS-01 challenge solver initialized", "provider", m.config.DNSProvider) + def = obtainer } } - if m.dnsObtainer != nil || len(m.dnsObtainers) > 0 { - m.grouper.DNSProviderAvailable = true - } - - return nil + return def, clients, nil } -// newDNSObtainer builds a DNS-01 client for one provider on the manager's -// ACME account and returns its certifier. -func (m *SANCertManager) newDNSObtainer(name acme.ProviderName) (certObtainer, error) { +// newDNSObtainer builds a DNS-01 client for one provider on the given ACME +// identity and returns its certifier. +func (m *SANCertManager) newDNSObtainer(user *acmeUser, directory string, name acme.ProviderName) (certObtainer, error) { dnsProvider, err := providers.NewProvider(name) if err != nil { return nil, err } - legoConfig := lego.NewConfig(m.user) - legoConfig.CADirURL = m.config.Directory + legoConfig := lego.NewConfig(user) + legoConfig.CADirURL = directory legoConfig.Certificate.KeyType = certcrypto.EC256 client, err := lego.NewClient(legoConfig) @@ -138,17 +158,25 @@ func (m *SANCertManager) splitByProviderZone(domains []string) [][]string { } // orderObtainer resolves the one DNS obtainer answering for an order's -// domains. Nil with no error means no DNS provider covers them: HTTP-01 -// territory. Batching splits before ordering, so an order spanning providers -// is an invariant violation, refused rather than half-answered. +// domains on the primary identity. Nil with no error means no DNS provider +// covers them: HTTP-01 territory. func (m *SANCertManager) orderObtainer(domains []string) (certObtainer, error) { m.mu.RLock() defer m.mu.RUnlock() + return orderObtainerFrom(m.selection, domains, m.dnsObtainer, m.dnsObtainers) +} + +// orderObtainerFrom resolves the one DNS obtainer answering for an order's +// domains out of the given identity's solvers. Nil with no error means no DNS +// provider covers them: HTTP-01 territory. Batching splits before ordering, +// so an order spanning providers is an invariant violation, refused rather +// than half-answered. +func orderObtainerFrom(selection acme.ProviderSelection, domains []string, def certObtainer, zoned map[acme.ProviderName]certObtainer) (certObtainer, error) { var obtainer certObtainer key := "" for i, domain := range domains { - domainObtainer, domainKey := m.resolveObtainerLocked(domain) + domainObtainer, domainKey := resolveObtainerFrom(selection, domain, def, zoned) if i == 0 { obtainer, key = domainObtainer, domainKey continue @@ -160,14 +188,14 @@ func (m *SANCertManager) orderObtainer(domains []string) (certObtainer, error) { return obtainer, nil } -// resolveObtainerLocked returns the obtainer and partition key for one -// domain. Callers must hold m.mu. -func (m *SANCertManager) resolveObtainerLocked(domain string) (certObtainer, string) { - provider, zone := m.selection.ProviderFor(domain) +// resolveObtainerFrom returns the obtainer and partition key for one domain +// out of the given identity's solvers. +func resolveObtainerFrom(selection acme.ProviderSelection, domain string, def certObtainer, zoned map[acme.ProviderName]certObtainer) (certObtainer, string) { + provider, zone := selection.ProviderFor(domain) if zone != "" { - return m.dnsObtainers[provider], "zone:" + string(provider) + return zoned[provider], "zone:" + string(provider) } - return m.dnsObtainer, "" + return def, "" } // hasDNSProviderFor reports whether some DNS-01 obtainer would answer for a diff --git a/internal/server/service.go b/internal/server/service.go index 78467eb..0cd33ff 100644 --- a/internal/server/service.go +++ b/internal/server/service.go @@ -319,6 +319,10 @@ func (so ServiceOptions) Validate() error { return err } + if err := so.validateACMEDirectory(); err != nil { + return err + } + if err := so.validateDynamicRedirects(); err != nil { return err } @@ -755,6 +759,15 @@ func (s *Service) servesRootPath() bool { } func (s *Service) createCertManager(options ServiceOptions) (CertManager, error) { + // A service that stops using the shared SAN manager on this deploy — + // static certificate, on-demand URL, or TLS disabled — must not leave a + // stale directory override behind: retained certificates would keep + // renewing against the old (possibly staging) identity. The SAN branch + // below re-records the current override. + if s.sanCertManager != nil { + s.sanCertManager.SetServiceDirectory(s.name, "") + } + if !options.TLSEnabled { return nil, nil } @@ -774,6 +787,9 @@ func (s *Service) createCertManager(options ServiceOptions) (CertManager, error) // Use the shared SAN certificate manager when available. An explicit // on-demand URL is a per-service opt-in, so it wins over the shared manager. if s.sanCertManager != nil && options.TLSOnDemandURL == "" { + // The per-service directory (--tls-staging) must be on record before + // any host is registered, so batching and issuance see it. + s.sanCertManager.SetServiceDirectory(s.name, options.ACMEDirectory) for _, host := range options.Hosts { if host == "" { // Catch-all marker, not a provisionable domain diff --git a/internal/server/service_options_validation.go b/internal/server/service_options_validation.go index 005da26..ab95800 100644 --- a/internal/server/service_options_validation.go +++ b/internal/server/service_options_validation.go @@ -45,6 +45,23 @@ func (so ServiceOptions) validateDynamicRedirects() error { return nil } +// validateACMEDirectory rejects a per-service ACME directory that is not an +// http(s) URL. The deploy CLI only ever sets the well-known staging constant, +// but the field arrives over RPC and would otherwise fail much later, inside +// an ACME order. +func (so ServiceOptions) validateACMEDirectory() error { + if so.ACMEDirectory == "" { + return nil + } + + parsed, err := url.Parse(so.ACMEDirectory) + if err != nil || (parsed.Scheme != "http" && parsed.Scheme != "https") || parsed.Hostname() == "" { + return fmt.Errorf("%w: acme directory must be an http(s) URL: %q", ErrServiceOptionsInvalid, so.ACMEDirectory) + } + + return nil +} + func (so ServiceOptions) validateDynamicDomains() error { if so.TLSDomainsSource == "" { if so.TLSDomainsBatchSize != 0 { diff --git a/internal/server/service_test.go b/internal/server/service_test.go index 0ee70ff..99307c3 100644 --- a/internal/server/service_test.go +++ b/internal/server/service_test.go @@ -611,3 +611,30 @@ func TestService_MarshallingPoolState(t *testing.T) { assert.Equal(t, 3*time.Second, restored.targetOptions.DialTimeout) assert.True(t, restored.targetOptions.DisableKeepAlives) } + +func TestServiceOptions_Validate_ACMEDirectory(t *testing.T) { + assertValid := func(options ServiceOptions) { + t.Helper() + require.NoError(t, options.Validate()) + } + + assertNotValid := func(options ServiceOptions, expected string) { + t.Helper() + err := options.Validate() + require.ErrorContains(t, err, expected) + require.ErrorIs(t, err, ErrServiceOptionsInvalid) + } + + hosts := []string{"example.com"} + + assertValid(ServiceOptions{Hosts: hosts, TLSEnabled: true}) + assertValid(ServiceOptions{Hosts: hosts, TLSEnabled: true, ACMEDirectory: ACMEStagingDirectoryURL}) + assertValid(ServiceOptions{Hosts: hosts, TLSEnabled: true, ACMEDirectory: "http://pebble.internal:14000/dir"}) + + assertNotValid(ServiceOptions{Hosts: hosts, TLSEnabled: true, ACMEDirectory: "not-a-url"}, + "acme directory must be an http(s) URL") + assertNotValid(ServiceOptions{Hosts: hosts, TLSEnabled: true, ACMEDirectory: "ftp://example.com/dir"}, + "acme directory must be an http(s) URL") + assertNotValid(ServiceOptions{Hosts: hosts, TLSEnabled: true, ACMEDirectory: "https://"}, + "acme directory must be an http(s) URL") +} diff --git a/internal/server/tls_on_demand_gate_test.go b/internal/server/tls_on_demand_gate_test.go index 8b9069d..e01d455 100644 --- a/internal/server/tls_on_demand_gate_test.go +++ b/internal/server/tls_on_demand_gate_test.go @@ -101,7 +101,7 @@ func BenchmarkRouter_GetCertificate(b *testing.B) { Certificate: &tls.Certificate{}, } manager.domainToCert["app.example.com"] = "app" - manager.registeredDomains["app.example.com"] = struct{}{} + manager.registeredDomains["app.example.com"] = "app" router.SetSANCertManager(manager) router.services.Set(&Service{