diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index cf0d992c..2eeffd89 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -15,7 +15,7 @@ jobs: strategy: matrix: os: [ ubuntu-latest, macos-latest, windows-latest ] - go: [ '1.21', '1.22' ] + go: [ '1.25', '1.26' ] runs-on: ${{ matrix.os }} diff --git a/README.md b/README.md index 8c176f44..513f781a 100644 --- a/README.md +++ b/README.md @@ -1,5 +1,10 @@
-
+
+
+
The same library used by the Caddy Web Server
diff --git a/account_test.go b/account_test.go index f2a182f1..c6f70a11 100644 --- a/account_test.go +++ b/account_test.go @@ -302,7 +302,7 @@ func TestGetAccountAlreadyExistsSkipsBroken(t *testing.T) { email := "me@foobar.com" // Create a "corrupted" account - am.config.Storage.Store(ctx, am.storageKeyUserReg(am.CA, "notmeatall@foobar.com"), []byte("this is not a valid account")) + _ = am.config.Storage.Store(ctx, am.storageKeyUserReg(am.CA, "notmeatall@foobar.com"), []byte("this is not a valid account")) // Create the actual account account, err := am.newAccount(email) diff --git a/acmeclient.go b/acmeclient.go index 5f746d96..08e1a53d 100644 --- a/acmeclient.go +++ b/acmeclient.go @@ -270,7 +270,13 @@ func (iss *ACMEIssuer) newBasicACMEClient() (*acmez.Client, error) { Directory: caURL, UserAgent: buildUAString(), HTTPClient: iss.httpClient, - Logger: slog.New(zapslog.NewHandler(iss.Logger.Named("acme_client").Core())), + Logger: slog.New(zapslog.NewHandler( + iss.Logger.Core(), + zapslog.WithName(iss.Logger.Name()+".acme_client"), + // the default enables traces at ERROR level, this disables + // them by setting it to a level higher than any other level + zapslog.AddStacktraceAt(slog.Level(127)), + )), }, }, nil } diff --git a/cache.go b/cache.go index 3b5cfb0b..8f325576 100644 --- a/cache.go +++ b/cache.go @@ -16,7 +16,7 @@ package certmagic import ( "fmt" - weakrand "math/rand" + weakrand "math/rand/v2" "strings" "sync" "time" @@ -244,7 +244,7 @@ func (certCache *Cache) unsyncedCacheCertificate(cert Certificate) { // map with less code, that is a heavily skewed eviction // strategy; generating random numbers is cheap and // ensures a much better distribution. - rnd := weakrand.Intn(cacheSize) + rnd := weakrand.IntN(cacheSize) i := 0 for _, randomCert := range certCache.cache { if i >= rnd && randomCert.managed { // don't evict manually-loaded certs diff --git a/certificates.go b/certificates.go index e0246dde..e7cfa5fe 100644 --- a/certificates.go +++ b/certificates.go @@ -21,7 +21,7 @@ import ( "encoding/json" "errors" "fmt" - "math/rand" + "math/rand/v2" "net" "os" "strings" @@ -128,7 +128,7 @@ func (cfg *Config) certNeedsRenewal(leaf *x509.Certificate, ari acme.RenewalInfo if selectedTime.IsZero() && (!ari.SuggestedWindow.Start.IsZero() && !ari.SuggestedWindow.End.IsZero()) { start, end := ari.SuggestedWindow.Start.Unix()+1, ari.SuggestedWindow.End.Unix() - selectedTime = time.Unix(rand.Int63n(end-start)+start, 0).UTC() + selectedTime = time.Unix(rand.Int64N(end-start)+start, 0).UTC() logger.Warn("no renewal time had been selected with ARI; chose an ephemeral one for now", zap.Time("ephemeral_selected_time", selectedTime)) } @@ -435,7 +435,7 @@ func (cfg Config) makeCertificateWithOCSP(ctx context.Context, certPEMBlock, key err = stapleOCSP(ctx, cfg.OCSP, cfg.Storage, &cert, certPEMBlock) if errors.Is(err, ErrNoOCSPServerSpecified) { cfg.Logger.Debug("stapling OCSP", zap.Error(err), zap.Strings("identifiers", cert.Names)) - } else { + } else if err != nil { cfg.Logger.Warn("stapling OCSP", zap.Error(err), zap.Strings("identifiers", cert.Names)) } } @@ -651,6 +651,11 @@ func isInternalIP(addr string) bool { func hostOnly(hostport string) string { host, _, err := net.SplitHostPort(hostport) if err != nil { + // May be a bare IPv6 address in brackets without a port (e.g. "[::1]"). + // net.SplitHostPort requires a port when brackets are present, so strip them. + if len(hostport) > 1 && hostport[0] == '[' && hostport[len(hostport)-1] == ']' { + return hostport[1 : len(hostport)-1] + } return hostport // OK; probably had no port to begin with } return host @@ -665,6 +670,8 @@ func hostOnly(hostport string) string { // It uses DNS wildcard matching logic and is case-insensitive. // https://tools.ietf.org/html/rfc2818#section-3.1 func MatchWildcard(subject, wildcard string) bool { + // Strip brackets from IPv6 addresses (e.g. "[::1]" from HTTP Host headers). + subject = hostOnly(subject) subject, wildcard = strings.ToLower(subject), strings.ToLower(wildcard) if subject == wildcard { return true diff --git a/certificates_test.go b/certificates_test.go index f8ecc3df..a53abcb5 100644 --- a/certificates_test.go +++ b/certificates_test.go @@ -189,6 +189,37 @@ func TestSubjectQualifiesForPublicCert(t *testing.T) { } } +func TestHostOnly(t *testing.T) { + for i, test := range []struct { + input string + expect string + }{ + // hostname without port + {"example.com", "example.com"}, + // hostname with port + {"example.com:443", "example.com"}, + // IPv4 without port + {"1.2.3.4", "1.2.3.4"}, + // IPv4 with port + {"1.2.3.4:80", "1.2.3.4"}, + // IPv6 without port and without brackets + {"::1", "::1"}, + // IPv6 with port (brackets required by RFC 7230) + {"[::1]:80", "::1"}, + // IPv6 without port but with brackets (Go's HTTP server format for host-only) + {"[::1]", "::1"}, + // full IPv6 without port but with brackets + {"[2001:db8::1]", "2001:db8::1"}, + // full IPv6 with port + {"[2001:db8::1]:8080", "2001:db8::1"}, + } { + actual := hostOnly(test.input) + if actual != test.expect { + t.Errorf("Test %d: hostOnly(%q) = %q, want %q", i, test.input, actual, test.expect) + } + } +} + func TestMatchWildcard(t *testing.T) { for i, test := range []struct { subject, wildcard string @@ -217,6 +248,10 @@ func TestMatchWildcard(t *testing.T) { {"1.2.3.4.5.6", "*.*.*.*.*.*", true}, {"0.1.2.3.4.5.6", "*.*.*.*.*.*", false}, {"1.2.3.4", "1.2.3.*", false}, // https://tools.ietf.org/html/rfc2818#section-3.1 + // Bracketed IPv6 subjects (from HTTP Host headers) must match bare IPv6 wildcards. + {"[::1]", "::1", true}, + {"[2001:db8::1]", "2001:db8::1", true}, + {"[::1]", "::2", false}, } { actual := MatchWildcard(test.subject, test.wildcard) if actual != test.expect { diff --git a/config.go b/config.go index b82d9c3a..8a5f5342 100644 --- a/config.go +++ b/config.go @@ -28,7 +28,7 @@ import ( "errors" "fmt" "io/fs" - weakrand "math/rand" + weakrand "math/rand/v2" "net" "net/http" "net/url" @@ -382,11 +382,23 @@ func (cfg *Config) manageAll(ctx context.Context, domainNames []string, async bo continue } - // TODO: consider doing this in a goroutine if async, to utilize multiple cores while loading certs // otherwise, begin management immediately - err := cfg.manageOne(ctx, domainName, async) - if err != nil { - return err + if async { + // don't block loading, since stapling OCSP uses the network and could block all other certs + // from being managed... (kind of tricky to make it truly async any lower-level than this) + go func(subject string) { + err := cfg.manageOne(ctx, subject, async) + if err != nil { + cfg.Logger.Error("initiating certificate management", + zap.String("subject", subject), + zap.Error(err)) + } + }(domainName) + } else { + err := cfg.manageOne(ctx, domainName, async) + if err != nil { + return err + } } } @@ -1120,10 +1132,10 @@ func (cfg *Config) RevokeCert(ctx context.Context, domain string, reason int, in return nil } -// TLSConfig is an opinionated method that returns a recommended, modern -// TLS configuration that can be used to configure TLS listeners. Aside -// from safe, modern defaults, this method sets two critical fields on the -// TLS config which are required to enable automatic certificate +// TLSConfig returns a recommended, modern TLS configuration that can be used +// to configure TLS listeners. Aside from using the safe, modern defaults +// implemented by the Go standard library, this method sets two critical fields +// on the TLS config which are required to enable automatic certificate // management: GetCertificate and NextProtos. // // The GetCertificate field is necessary to get certificates from memory @@ -1147,15 +1159,6 @@ func (cfg *Config) TLSConfig() *tls.Config { // these two fields necessary for TLS-ALPN challenge GetCertificate: cfg.GetCertificate, NextProtos: []string{acmez.ACMETLS1Protocol}, - - // the rest recommended for modern TLS servers - MinVersion: tls.VersionTLS12, - CurvePreferences: []tls.CurveID{ - tls.X25519, - tls.CurveP256, - }, - CipherSuites: preferredDefaultCipherSuites(), - PreferServerCipherSuites: true, } } @@ -1237,11 +1240,20 @@ func (cfg *Config) checkStorage(ctx context.Context) error { } key := fmt.Sprintf("rw_test_%d", weakrand.Int()) contents := make([]byte, 1024*10) // size sufficient for one or two ACME resources - _, err := weakrand.Read(contents) - if err != nil { - return err - } - err = cfg.Storage.Store(ctx, key, contents) + // This is how ChaCha8.Read works, without handling the case where the slice length is not a multiple of 8. + // This also avoids the use of a mutex and an import. + for i := 0; i < len(contents); i += 8 { + v := weakrand.Uint64() + contents[i] = byte(v) + contents[i+1] = byte(v >> 8) + contents[i+2] = byte(v >> 16) + contents[i+3] = byte(v >> 24) + contents[i+4] = byte(v >> 32) + contents[i+5] = byte(v >> 40) + contents[i+6] = byte(v >> 48) + contents[i+7] = byte(v >> 56) + } + err := cfg.Storage.Store(ctx, key, contents) if err != nil { return err } diff --git a/docs/banner/certmagic-banner-dark.png b/docs/banner/certmagic-banner-dark.png new file mode 100644 index 00000000..315010e4 Binary files /dev/null and b/docs/banner/certmagic-banner-dark.png differ diff --git a/docs/banner/certmagic-banner.png b/docs/banner/certmagic-banner.png new file mode 100644 index 00000000..c55392fb Binary files /dev/null and b/docs/banner/certmagic-banner.png differ diff --git a/go.mod b/go.mod index 698a267f..68987a6c 100644 --- a/go.mod +++ b/go.mod @@ -1,25 +1,25 @@ module github.com/caddyserver/certmagic -go 1.24.0 +go 1.25.0 require ( - github.com/caddyserver/zerossl v0.1.4 + github.com/caddyserver/zerossl v0.1.5 github.com/klauspost/cpuid/v2 v2.3.0 github.com/libdns/libdns v1.1.1 - github.com/mholt/acmez/v3 v3.1.4 - github.com/miekg/dns v1.1.69 + github.com/mholt/acmez/v3 v3.1.6 + github.com/miekg/dns v1.1.72 github.com/zeebo/blake3 v0.2.4 go.uber.org/zap v1.27.1 go.uber.org/zap/exp v0.3.0 - golang.org/x/crypto v0.46.0 - golang.org/x/net v0.48.0 + golang.org/x/crypto v0.50.0 + golang.org/x/net v0.53.0 ) require ( go.uber.org/multierr v1.11.0 // indirect - golang.org/x/mod v0.30.0 // indirect - golang.org/x/sync v0.19.0 // indirect - golang.org/x/sys v0.39.0 // indirect - golang.org/x/text v0.32.0 // indirect - golang.org/x/tools v0.39.0 // indirect + golang.org/x/mod v0.35.0 // indirect + golang.org/x/sync v0.20.0 // indirect + golang.org/x/sys v0.43.0 // indirect + golang.org/x/text v0.36.0 // indirect + golang.org/x/tools v0.44.0 // indirect ) diff --git a/go.sum b/go.sum index 4bf75198..9a1aec88 100644 --- a/go.sum +++ b/go.sum @@ -1,17 +1,25 @@ -github.com/caddyserver/zerossl v0.1.4 h1:CVJOE3MZeFisCERZjkxIcsqIH4fnFdlYWnPYeFtBHRw= -github.com/caddyserver/zerossl v0.1.4/go.mod h1:CxA0acn7oEGO6//4rtrRjYgEoa4MFw/XofZnrYwGqG4= +code.pfad.fr/check v1.1.0 h1:GWvjdzhSEgHvEHe2uJujDcpmZoySKuHQNrZMfzfO0bE= +code.pfad.fr/check v1.1.0/go.mod h1:NiUH13DtYsb7xp5wll0U4SXx7KhXQVCtRgdC96IPfoM= +github.com/caddyserver/zerossl v0.1.5 h1:dkvOjBAEEtY6LIGAHei7sw2UgqSD6TrWweXpV7lvEvE= +github.com/caddyserver/zerossl v0.1.5/go.mod h1:CxA0acn7oEGO6//4rtrRjYgEoa4MFw/XofZnrYwGqG4= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/go-jose/go-jose/v4 v4.1.3 h1:CVLmWDhDVRa6Mi/IgCgaopNosCaHz7zrMeF9MlZRkrs= +github.com/go-jose/go-jose/v4 v4.1.3/go.mod h1:x4oUasVrzR7071A4TnHLGSPpNOm2a21K9Kf04k1rs08= github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI= github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= github.com/klauspost/cpuid/v2 v2.3.0 h1:S4CRMLnYUhGeDFDqkGriYKdfoFlDnMtqTiI/sFzhA9Y= github.com/klauspost/cpuid/v2 v2.3.0/go.mod h1:hqwkgyIinND0mEev00jJYCxPNVRVXFQeu1XKlok6oO0= +github.com/letsencrypt/challtestsrv v1.4.2 h1:0ON3ldMhZyWlfVNYYpFuWRTmZNnyfiL9Hh5YzC3JVwU= +github.com/letsencrypt/challtestsrv v1.4.2/go.mod h1:GhqMqcSoeGpYd5zX5TgwA6er/1MbWzx/o7yuuVya+Wk= +github.com/letsencrypt/pebble/v2 v2.10.0 h1:Wq6gYXlsY6ubqI3hhxsTzdyotvfdjFBxuwYqCLCnj/U= +github.com/letsencrypt/pebble/v2 v2.10.0/go.mod h1:Sk8cmUIPcIdv2nINo+9PB4L+ZBhzY+F9A1a/h/xmWiQ= github.com/libdns/libdns v1.1.1 h1:wPrHrXILoSHKWJKGd0EiAVmiJbFShguILTg9leS/P/U= github.com/libdns/libdns v1.1.1/go.mod h1:4Bj9+5CQiNMVGf87wjX4CY3HQJypUHRuLvlsfsZqLWQ= -github.com/mholt/acmez/v3 v3.1.4 h1:DyzZe/RnAzT3rpZj/2Ii5xZpiEvvYk3cQEN/RmqxwFQ= -github.com/mholt/acmez/v3 v3.1.4/go.mod h1:L1wOU06KKvq7tswuMDwKdcHeKpFFgkppZy/y0DFxagQ= -github.com/miekg/dns v1.1.69 h1:Kb7Y/1Jo+SG+a2GtfoFUfDkG//csdRPwRLkCsxDG9Sc= -github.com/miekg/dns v1.1.69/go.mod h1:7OyjD9nEba5OkqQ/hB4fy3PIoxafSZJtducccIelz3g= +github.com/mholt/acmez/v3 v3.1.6 h1:eGVQNObP0pBN4sxqrXeg7MYqTOWyoiYpQqITVWlrevk= +github.com/mholt/acmez/v3 v3.1.6/go.mod h1:5nTPosTGosLxF3+LU4ygbgMRFDhbAVpqMI4+a4aHLBY= +github.com/miekg/dns v1.1.72 h1:vhmr+TF2A3tuoGNkLDFK9zi36F2LS+hKTRW0Uf8kbzI= +github.com/miekg/dns v1.1.72/go.mod h1:+EuEPhdHOsfk6Wk5TT2CzssZdqkmFhf8r+aVyDEToIs= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/stretchr/testify v1.8.1 h1:w7B6lhMri9wdJUVmEZPGGhZzrYTPvgJArz7wNPgYKsk= @@ -30,19 +38,23 @@ go.uber.org/zap v1.27.1 h1:08RqriUEv8+ArZRYSTXy1LeBScaMpVSTBhCeaZYfMYc= go.uber.org/zap v1.27.1/go.mod h1:GB2qFLM7cTU87MWRP2mPIjqfIDnGu+VIO4V/SdhGo2E= go.uber.org/zap/exp v0.3.0 h1:6JYzdifzYkGmTdRR59oYH+Ng7k49H9qVpWwNSsGJj3U= go.uber.org/zap/exp v0.3.0/go.mod h1:5I384qq7XGxYyByIhHm6jg5CHkGY0nsTfbDLgDDlgJQ= -golang.org/x/crypto v0.46.0 h1:cKRW/pmt1pKAfetfu+RCEvjvZkA9RimPbh7bhFjGVBU= -golang.org/x/crypto v0.46.0/go.mod h1:Evb/oLKmMraqjZ2iQTwDwvCtJkczlDuTmdJXoZVzqU0= -golang.org/x/mod v0.30.0 h1:fDEXFVZ/fmCKProc/yAXXUijritrDzahmwwefnjoPFk= -golang.org/x/mod v0.30.0/go.mod h1:lAsf5O2EvJeSFMiBxXDki7sCgAxEUcZHXoXMKT4GJKc= -golang.org/x/net v0.48.0 h1:zyQRTTrjc33Lhh0fBgT/H3oZq9WuvRR5gPC70xpDiQU= -golang.org/x/net v0.48.0/go.mod h1:+ndRgGjkh8FGtu1w1FGbEC31if4VrNVMuKTgcAAnQRY= -golang.org/x/sync v0.19.0 h1:vV+1eWNmZ5geRlYjzm2adRgW2/mcpevXNg50YZtPCE4= -golang.org/x/sync v0.19.0/go.mod h1:9KTHXmSnoGruLpwFjVSX0lNNA75CykiMECbovNTZqGI= -golang.org/x/sys v0.39.0 h1:CvCKL8MeisomCi6qNZ+wbb0DN9E5AATixKsvNtMoMFk= -golang.org/x/sys v0.39.0/go.mod h1:OgkHotnGiDImocRcuBABYBEXf8A9a87e/uXjp9XT3ks= -golang.org/x/text v0.32.0 h1:ZD01bjUt1FQ9WJ0ClOL5vxgxOI/sVCNgX1YtKwcY0mU= -golang.org/x/text v0.32.0/go.mod h1:o/rUWzghvpD5TXrTIBuJU77MTaN0ljMWE47kxGJQ7jY= -golang.org/x/tools v0.39.0 h1:ik4ho21kwuQln40uelmciQPp9SipgNDdrafrYA4TmQQ= -golang.org/x/tools v0.39.0/go.mod h1:JnefbkDPyD8UU2kI5fuf8ZX4/yUeh9W877ZeBONxUqQ= +golang.org/x/crypto v0.50.0 h1:zO47/JPrL6vsNkINmLoo/PH1gcxpls50DNogFvB5ZGI= +golang.org/x/crypto v0.50.0/go.mod h1:3muZ7vA7PBCE6xgPX7nkzzjiUq87kRItoJQM1Yo8S+Q= +golang.org/x/mod v0.34.0 h1:xIHgNUUnW6sYkcM5Jleh05DvLOtwc6RitGHbDk4akRI= +golang.org/x/mod v0.34.0/go.mod h1:ykgH52iCZe79kzLLMhyCUzhMci+nQj+0XkbXpNYtVjY= +golang.org/x/mod v0.35.0 h1:Ww1D637e6Pg+Zb2KrWfHQUnH2dQRLBQyAtpr/haaJeM= +golang.org/x/mod v0.35.0/go.mod h1:+GwiRhIInF8wPm+4AoT6L0FA1QWAad3OMdTRx4tFYlU= +golang.org/x/net v0.53.0 h1:d+qAbo5L0orcWAr0a9JweQpjXF19LMXJE8Ey7hwOdUA= +golang.org/x/net v0.53.0/go.mod h1:JvMuJH7rrdiCfbeHoo3fCQU24Lf5JJwT9W3sJFulfgs= +golang.org/x/sync v0.20.0 h1:e0PTpb7pjO8GAtTs2dQ6jYa5BWYlMuX047Dco/pItO4= +golang.org/x/sync v0.20.0/go.mod h1:9xrNwdLfx4jkKbNva9FpL6vEN7evnE43NNNJQ2LF3+0= +golang.org/x/sys v0.43.0 h1:Rlag2XtaFTxp19wS8MXlJwTvoh8ArU6ezoyFsMyCTNI= +golang.org/x/sys v0.43.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= +golang.org/x/text v0.36.0 h1:JfKh3XmcRPqZPKevfXVpI1wXPTqbkE5f7JA92a55Yxg= +golang.org/x/text v0.36.0/go.mod h1:NIdBknypM8iqVmPiuco0Dh6P5Jcdk8lJL0CUebqK164= +golang.org/x/tools v0.43.0 h1:12BdW9CeB3Z+J/I/wj34VMl8X+fEXBxVR90JeMX5E7s= +golang.org/x/tools v0.43.0/go.mod h1:uHkMso649BX2cZK6+RpuIPXS3ho2hZo4FVwfoy1vIk0= +golang.org/x/tools v0.44.0 h1:UP4ajHPIcuMjT1GqzDWRlalUEoY+uzoZKnhOjbIPD2c= +golang.org/x/tools v0.44.0/go.mod h1:KA0AfVErSdxRZIsOVipbv3rQhVXTnlU6UhKxHd1seDI= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/handshake.go b/handshake.go index d14c1382..a227015b 100644 --- a/handshake.go +++ b/handshake.go @@ -298,9 +298,9 @@ func (cfg *Config) getCertDuringHandshake(ctx context.Context, hello *tls.Client // domain, avoid pounding manager or storage thousands of times simultaneously. We use a similar sync // strategy for obtaining certificate during handshake. certLoadWaitChansMu.Lock() - wait, ok := certLoadWaitChans[name] + waiter, ok := certLoadWaitChans[name] if ok { - // another goroutine is already loading the cert; just wait and we'll get it from the in-memory cache + // another goroutine is already loading the cert; just wait certLoadWaitChansMu.Unlock() timeout := time.NewTimer(2 * time.Minute) @@ -310,33 +310,44 @@ func (cfg *Config) getCertDuringHandshake(ctx context.Context, hello *tls.Client case <-ctx.Done(): timeout.Stop() return Certificate{}, ctx.Err() - case <-wait: + case <-waiter.done: timeout.Stop() } - return cfg.getCertDuringHandshake(ctx, hello, false) - } else { - // no other goroutine is currently trying to load this cert - wait = make(chan struct{}) - certLoadWaitChans[name] = wait - certLoadWaitChansMu.Unlock() + // If the leader got a result from an external cert manager, use it + // directly — these certs are not added to the cache, so a recursive + // cache lookup would miss. For cached certs (on-demand, managed), + // the waiter result will be empty and we fall through to the + // original recursive lookup. + if !waiter.cert.Empty() || waiter.err != nil { + return waiter.cert, waiter.err + } - // unblock others and clean up when we're done - defer func() { - certLoadWaitChansMu.Lock() - close(wait) - delete(certLoadWaitChans, name) - certLoadWaitChansMu.Unlock() - }() + return cfg.getCertDuringHandshake(ctx, hello, false) } + // no other goroutine is currently trying to load this cert + waiter = &certLoadWaiter{done: make(chan struct{})} + certLoadWaitChans[name] = waiter + certLoadWaitChansMu.Unlock() + + // unblock others and clean up when we're done + defer func() { + certLoadWaitChansMu.Lock() + close(waiter.done) + delete(certLoadWaitChans, name) + certLoadWaitChansMu.Unlock() + }() + // If an external Manager is configured, try to get it from them. // Only continue to use our own logic if it returns empty+nil. externalCert, err := cfg.getCertFromAnyCertManager(ctx, hello, logger) if err != nil { + waiter.err = err return Certificate{}, err } if !externalCert.Empty() { + waiter.cert = externalCert return externalCert, nil } @@ -946,9 +957,19 @@ var ( obtainCertWaitChansMu sync.Mutex ) +// certLoadWaiter coordinates concurrent certificate loading for the same name. +// The leader populates the result and closes the channel; waiters read the result +// after the channel is closed. This allows externally-managed certificates (which +// are not cached) to be shared directly with waiting goroutines. +type certLoadWaiter struct { + done chan struct{} + cert Certificate + err error +} + // TODO: this lockset should probably be per-cache var ( - certLoadWaitChans = make(map[string]chan struct{}) + certLoadWaitChans = make(map[string]*certLoadWaiter) certLoadWaitChansMu sync.Mutex ) diff --git a/ocsp.go b/ocsp.go index c87a560f..82b91dc0 100644 --- a/ocsp.go +++ b/ocsp.go @@ -53,7 +53,7 @@ func stapleOCSP(ctx context.Context, ocspConfig OCSPConfig, storage Storage, cer // we need a PEM encoding only for some function calls below bundle := new(bytes.Buffer) for _, derBytes := range cert.Certificate.Certificate { - pem.Encode(bundle, &pem.Block{Type: "CERTIFICATE", Bytes: derBytes}) + _ = pem.Encode(bundle, &pem.Block{Type: "CERTIFICATE", Bytes: derBytes}) } pemBundle = bundle.Bytes() } @@ -235,6 +235,10 @@ func getOCSPForCert(ocspConfig OCSPConfig, bundle []byte) ([]byte, *ocsp.Respons return nil, nil, fmt.Errorf("parsing OCSP response: %v", err) } + if err := validateOCSPResponder(ocspRes, issuerCert); err != nil { + return nil, nil, fmt.Errorf("OCSP responder authorization check failed: %v", err) + } + return ocspResBytes, ocspRes, nil } @@ -253,3 +257,26 @@ func freshOCSP(resp *ocsp.Response) bool { refreshTime := resp.ThisUpdate.Add(nextUpdate.Sub(resp.ThisUpdate) / 2) return time.Now().Before(refreshTime) } + +// validateOCSPResponder enforces RFC 6960 §4.2.2.2: "Systems or applications that +// rely on OCSP responses MUST be capable of detecting and enforcing the use of the +// id-kp-OCSPSigning value." An issuer-signed response (where the embedded Certificate +// field is nil, meaning the issuer signed directly) is always acceptable. +func validateOCSPResponder(ocspResp *ocsp.Response, issuerCert *x509.Certificate) error { + respCert := ocspResp.Certificate + + // if response was signed directly by the issuer, or embedded responder cert IS the issuer, accept + if respCert == nil || respCert.Equal(issuerCert) { + // Response was signed directly by the issuer — always valid. + return nil + } + + // RFC 6960 §4.2.2.2 requires id-kp-OCSPSigning for delegated responders + for _, eku := range respCert.ExtKeyUsage { + if eku == x509.ExtKeyUsageOCSPSigning { + return nil + } + } + + return fmt.Errorf("OCSP responder certificate (subject: %s) is not the issuer and does not carry id-kp-OCSPSigning", respCert.Subject) +} diff --git a/ocsp_test.go b/ocsp_test.go index 4c3df27e..8f832da7 100644 --- a/ocsp_test.go +++ b/ocsp_test.go @@ -4,10 +4,13 @@ import ( "bytes" "context" "crypto" + "crypto/x509" + "crypto/x509/pkix" "errors" "io" "net/http" "net/http/httptest" + "strings" "testing" "golang.org/x/crypto/ocsp" @@ -153,6 +156,69 @@ func TestStapleOCSP(t *testing.T) { }) } +func TestValidateOCSPResponder(t *testing.T) { + issuer := mustMakeCertificate(t, caCert, caKey).Leaf + + tests := []struct { + name string + resp *ocsp.Response + wantErr string + }{ + { + name: "issuer signed response with no embedded cert", + resp: &ocsp.Response{Certificate: nil}, + }, + { + name: "embedded responder cert is issuer cert", + resp: &ocsp.Response{Certificate: issuer}, + }, + { + name: "delegated responder with OCSP signing eku", + resp: &ocsp.Response{Certificate: &x509.Certificate{ + Subject: pkix.Name{CommonName: "Delegated OCSP Responder"}, + ExtKeyUsage: []x509.ExtKeyUsage{ + x509.ExtKeyUsageServerAuth, + x509.ExtKeyUsageOCSPSigning, + }, + }}, + }, + { + name: "delegated responder without OCSP signing eku", + resp: &ocsp.Response{Certificate: &x509.Certificate{ + Subject: pkix.Name{CommonName: "Not Authorized"}, + ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth}, + }}, + wantErr: "does not carry id-kp-OCSPSigning", + }, + { + name: "delegated responder with empty eku", + resp: &ocsp.Response{Certificate: &x509.Certificate{ + Subject: pkix.Name{CommonName: "No EKU"}, + }}, + wantErr: "does not carry id-kp-OCSPSigning", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + err := validateOCSPResponder(tc.resp, issuer) + if tc.wantErr == "" { + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + return + } + + if err == nil { + t.Fatalf("expected error containing %q, got nil", tc.wantErr) + } + if !strings.Contains(err.Error(), tc.wantErr) { + t.Fatalf("expected error containing %q, got %q", tc.wantErr, err.Error()) + } + }) + } +} + func mustMakeCertificate(t *testing.T, cert, key string) Certificate { t.Helper() c, err := makeCertificate([]byte(cert), []byte(key)) diff --git a/solvers.go b/solvers.go index 677fad3f..b4801303 100644 --- a/solvers.go +++ b/solvers.go @@ -768,6 +768,8 @@ func dialTCPSocket(addr string) error { // GetACMEChallenge returns an active ACME challenge for the given identifier, // or false if no active challenge for that identifier is known. func GetACMEChallenge(identifier string) (Challenge, bool) { + // Strip brackets from IPv6 addresses (e.g. "[::1]" from HTTP Host headers). + identifier = hostOnly(identifier) activeChallengesMu.Lock() chalData, ok := activeChallenges[identifier] activeChallengesMu.Unlock() diff --git a/solvers_test.go b/solvers_test.go index d30ce66d..c9fc1974 100644 --- a/solvers_test.go +++ b/solvers_test.go @@ -155,3 +155,25 @@ func Test_challengeKey(t *testing.T) { }) } } + +func TestGetACMEChallenge_IPv6Brackets(t *testing.T) { + // Store a challenge under a bare IPv6 identifier (as CertMagic does internally). + bare := "::1" + activeChallengesMu.Lock() + activeChallenges[bare] = Challenge{} + activeChallengesMu.Unlock() + defer func() { + activeChallengesMu.Lock() + delete(activeChallenges, bare) + activeChallengesMu.Unlock() + }() + + // Lookup with bracketed IPv6 (as received from Go's HTTP server via r.Host). + if _, ok := GetACMEChallenge("[::1]"); !ok { + t.Error("GetACMEChallenge(\"[::1]\") should find challenge stored under \"::1\"") + } + // Lookup with bare IPv6 should still work. + if _, ok := GetACMEChallenge("::1"); !ok { + t.Error("GetACMEChallenge(\"::1\") should find challenge stored under \"::1\"") + } +}