diff --git a/certificates_test.go b/certificates_test.go index a53abcb5..cc24ceae 100644 --- a/certificates_test.go +++ b/certificates_test.go @@ -27,7 +27,7 @@ func TestUnexportedGetCertificate(t *testing.T) { cfg := &Config{Logger: defaultTestLogger, certCache: certCache} // When cache is empty - if _, matched, defaulted := cfg.getCertificateFromCache(&tls.ClientHelloInfo{ServerName: "example.com"}); matched || defaulted { + if _, matched, defaulted, _ := cfg.getCertificateFromCache(&tls.ClientHelloInfo{ServerName: "example.com"}); matched || defaulted { t.Errorf("Got a certificate when cache was empty; matched=%v, defaulted=%v", matched, defaulted) } @@ -35,19 +35,19 @@ func TestUnexportedGetCertificate(t *testing.T) { firstCert := Certificate{Names: []string{"example.com"}} certCache.cache["0xdeadbeef"] = firstCert certCache.cacheIndex["example.com"] = []string{"0xdeadbeef"} - if cert, matched, defaulted := cfg.getCertificateFromCache(&tls.ClientHelloInfo{ServerName: "example.com"}); !matched || defaulted || cert.Names[0] != "example.com" { + if cert, matched, defaulted, _ := cfg.getCertificateFromCache(&tls.ClientHelloInfo{ServerName: "example.com"}); !matched || defaulted || cert.Names[0] != "example.com" { t.Errorf("Didn't get a cert for 'example.com' or got the wrong one: %v, matched=%v, defaulted=%v", cert, matched, defaulted) } // When retrieving wildcard certificate certCache.cache["0xb01dface"] = Certificate{Names: []string{"*.example.com"}} certCache.cacheIndex["*.example.com"] = []string{"0xb01dface"} - if cert, matched, defaulted := cfg.getCertificateFromCache(&tls.ClientHelloInfo{ServerName: "sub.example.com"}); !matched || defaulted || cert.Names[0] != "*.example.com" { + if cert, matched, defaulted, _ := cfg.getCertificateFromCache(&tls.ClientHelloInfo{ServerName: "sub.example.com"}); !matched || defaulted || cert.Names[0] != "*.example.com" { t.Errorf("Didn't get wildcard cert for 'sub.example.com' or got the wrong one: %v, matched=%v, defaulted=%v", cert, matched, defaulted) } // When no certificate matches and SNI is provided, return no certificate (should be TLS alert) - if cert, matched, defaulted := cfg.getCertificateFromCache(&tls.ClientHelloInfo{ServerName: "nomatch"}); matched || defaulted { + if cert, matched, defaulted, _ := cfg.getCertificateFromCache(&tls.ClientHelloInfo{ServerName: "nomatch"}); matched || defaulted { t.Errorf("Expected matched=false, defaulted=false; but got matched=%v, defaulted=%v (cert: %v)", matched, defaulted, cert) } } diff --git a/handshake.go b/handshake.go index 2b1b7a9c..56df24ac 100644 --- a/handshake.go +++ b/handshake.go @@ -106,12 +106,14 @@ func (cfg *Config) GetCertificateWithContext(ctx context.Context, clientHello *t // If a match is found, matched will be true. If no matches are found, matched // will be false and a "default" certificate will be returned with defaulted // set to true. If defaulted is false, then no certificates were available. +// If matched or defaulted is true, matchedName is the cache lookup name that +// selected the certificate. // // The logic in this function is adapted from the Go standard library, // which is by the Go Authors. // // This function is safe for concurrent use. -func (cfg *Config) getCertificateFromCache(hello *tls.ClientHelloInfo) (cert Certificate, matched, defaulted bool) { +func (cfg *Config) getCertificateFromCache(hello *tls.ClientHelloInfo) (cert Certificate, matched, defaulted bool, matchedName string) { name := normalizedName(hello.ServerName) if name == "" { @@ -120,6 +122,7 @@ func (cfg *Config) getCertificateFromCache(hello *tls.ClientHelloInfo) (cert Cer addr := localIPFromConn(hello.Conn) cert, matched = cfg.selectCert(hello, addr) if matched { + matchedName = addr return } } @@ -129,6 +132,7 @@ func (cfg *Config) getCertificateFromCache(hello *tls.ClientHelloInfo) (cert Cer normDefault := normalizedName(cfg.DefaultServerName) cert, defaulted = cfg.selectCert(hello, normDefault) if defaulted { + matchedName = normDefault return } } @@ -136,6 +140,7 @@ func (cfg *Config) getCertificateFromCache(hello *tls.ClientHelloInfo) (cert Cer // if SNI is specified, try an exact match first cert, matched = cfg.selectCert(hello, name) if matched { + matchedName = name return } @@ -147,6 +152,7 @@ func (cfg *Config) getCertificateFromCache(hello *tls.ClientHelloInfo) (cert Cer candidate := strings.Join(labels, ".") cert, matched = cfg.selectCert(hello, candidate) if matched { + matchedName = candidate return } } @@ -162,6 +168,7 @@ func (cfg *Config) getCertificateFromCache(hello *tls.ClientHelloInfo) (cert Cer normFallback := normalizedName(cfg.FallbackServerName) cert, defaulted = cfg.selectCert(hello, normFallback) if defaulted { + matchedName = normFallback return } } @@ -273,7 +280,8 @@ func (cfg *Config) getCertDuringHandshake(ctx context.Context, hello *tls.Client logger := logWithRemote(cfg.Logger.Named("handshake"), hello) // First check our in-memory cache to see if we've already loaded it - cert, matched, defaulted := cfg.getCertificateFromCache(hello) + cert, matched, defaulted, matchedName := cfg.getCertificateFromCache(hello) + var loadExactFromStorage bool if matched { logger.Debug("matched certificate in cache", zap.Strings("subjects", cert.Names), @@ -281,12 +289,24 @@ func (cfg *Config) getCertDuringHandshake(ctx context.Context, hello *tls.Client zap.Time("expiration", expiresAt(cert.Leaf)), zap.String("hash", cert.hash)) if cert.managed && cfg.OnDemand != nil && loadOrObtainIfNecessary { - // On-demand certificates are maintained in the background, but - // maintenance is triggered by handshakes instead of by a timer - // as in maintain.go. - return cfg.optionalMaintenance(ctx, cfg.Logger.Named("on_demand"), cert, hello) + serverName := normalizedName(hello.ServerName) + if cert.Expired() && matchedName != serverName && strings.Contains(matchedName, "*") { + logger.Debug("cached wildcard certificate is expired; trying exact certificate for SNI", + zap.String("server_name", serverName), + zap.String("matched_identifier", matchedName), + zap.Strings("subjects", cert.Names), + zap.Time("expiration", expiresAt(cert.Leaf))) + loadExactFromStorage = true + } else { + // On-demand certificates are maintained in the background, but + // maintenance is triggered by handshakes instead of by a timer + // as in maintain.go. + return cfg.optionalMaintenance(ctx, cfg.Logger.Named("on_demand"), cert, hello) + } + } + if !loadExactFromStorage { + return cert, nil } - return cert, nil } name, err := cfg.getNameFromClientHello(hello) @@ -383,7 +403,7 @@ func (cfg *Config) getCertDuringHandshake(ctx context.Context, hello *tls.Client if loadDynamically && loadOrObtainIfNecessary { // Check to see if we have one on disk - loadedCert, err := cfg.loadCertFromStorage(ctx, logger, hello) + loadedCert, err := cfg.loadCertFromStorage(ctx, logger, hello, loadExactFromStorage) if err == nil { return loadedCert, nil } @@ -420,14 +440,16 @@ func (cfg *Config) getCertDuringHandshake(ctx context.Context, hello *tls.Client } // loadCertFromStorage loads the certificate for name from storage and maintains it -// (as this is only called with on-demand TLS enabled). -func (cfg *Config) loadCertFromStorage(ctx context.Context, logger *zap.Logger, hello *tls.ClientHelloInfo) (Certificate, error) { +// (as this is only called with on-demand TLS enabled). If exactOnly is true, it +// does not fall back to loading a wildcard certificate when the exact name is not +// in storage. +func (cfg *Config) loadCertFromStorage(ctx context.Context, logger *zap.Logger, hello *tls.ClientHelloInfo, exactOnly bool) (Certificate, error) { name, err := cfg.getNameFromClientHello(hello) if err != nil { return Certificate{}, err } loadedCert, err := cfg.CacheManagedCertificate(ctx, name) - if errors.Is(err, fs.ErrNotExist) { + if errors.Is(err, fs.ErrNotExist) && !exactOnly { // If no exact match, try a wildcard variant, which is something we can still use labels := strings.Split(name, ".") labels[0] = "*" @@ -563,7 +585,7 @@ func (cfg *Config) obtainOnDemandCertificate(ctx context.Context, hello *tls.Cli err = cfg.ObtainCertAsync(ctx, name) if err == nil { // load from storage while others wait to make the op as atomic as possible - cert, err = cfg.loadCertFromStorage(ctx, log, hello) + cert, err = cfg.loadCertFromStorage(ctx, log, hello, false) if err != nil { log.Error("loading newly-obtained certificate from storage", zap.String("server_name", name), zap.Error(err)) } diff --git a/handshake_test.go b/handshake_test.go index 5adc8c37..7829dfc9 100644 --- a/handshake_test.go +++ b/handshake_test.go @@ -15,9 +15,15 @@ package certmagic import ( "context" + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" "crypto/tls" "crypto/x509" + "crypto/x509/pkix" + "encoding/pem" "errors" + "math/big" "net" "runtime" "strings" @@ -113,6 +119,87 @@ func TestGetCertificate(t *testing.T) { } } +func TestGetCertDuringHandshakeExpiredWildcardCacheObtainsExact(t *testing.T) { + const ( + wildcardName = "*.example.com" + serverName = "test.example.com" + ) + + ctx := context.Background() + certCache := &Cache{ + cache: make(map[string]Certificate), + cacheIndex: make(map[string][]string), + logger: defaultTestLogger, + } + + issuer := &handshakeTestIssuer{t: t} + cfg := newWithCache(certCache, Config{ + Issuers: []Issuer{issuer}, + Storage: &FileStorage{Path: t.TempDir()}, + Logger: defaultTestLogger, + OnDemand: &OnDemandConfig{}, + OCSP: OCSPConfig{DisableStapling: true}, + }) + certCache.options = CacheOptions{ + GetConfigForCert: func(Certificate) (*Config, error) { return cfg, nil }, + RenewCheckInterval: DefaultRenewCheckInterval, + OCSPCheckInterval: DefaultOCSPCheckInterval, + Logger: defaultTestLogger, + } + + now := time.Now() + wildcardKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatalf("generating wildcard key: %v", err) + } + wildcardCertPEM := mustIssueCertificateForPublicKey(t, wildcardKey.Public(), []string{wildcardName}, now.Add(-4*time.Hour), now.Add(-2*time.Hour)) + wildcardKeyPEM, err := PEMEncodePrivateKey(wildcardKey) + if err != nil { + t.Fatalf("encoding wildcard private key: %v", err) + } + + err = cfg.saveCertResource(ctx, issuer, CertificateResource{ + SANs: []string{wildcardName}, + CertificatePEM: wildcardCertPEM, + PrivateKeyPEM: wildcardKeyPEM, + }) + if err != nil { + t.Fatalf("saving wildcard certificate resource: %v", err) + } + + wildcardCert, err := cfg.CacheManagedCertificate(ctx, wildcardName) + if err != nil { + t.Fatalf("caching wildcard certificate: %v", err) + } + if !wildcardCert.Expired() { + t.Fatalf("test wildcard certificate should be expired") + } + + l, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("creating listener: %v", err) + } + defer l.Close() + conn, err := net.Dial("tcp", l.Addr().String()) + if err != nil { + t.Fatalf("creating connection: %v", err) + } + defer conn.Close() + + hello := &tls.ClientHelloInfo{ServerName: serverName, Conn: conn} + cert, err := cfg.getCertDuringHandshake(ctx, hello, true) + if err != nil { + t.Fatalf("getting certificate during handshake: %v", err) + } + + if got := cert.Names[0]; got != serverName { + t.Fatalf("expected exact certificate for %s, got %s", serverName, got) + } + if got := issuer.issued.Load(); got != 1 { + t.Fatalf("expected exact certificate to be issued once, got %d", got) + } +} + // TestGetCertDuringHandshakeWaiterErrorPropagation verifies that when the // "leader" goroutine loading/obtaining a certificate for a name fails (e.g. // an on-demand permission denial), the failure is propagated directly to all @@ -227,3 +314,50 @@ func TestGetCertDuringHandshakeWaiterErrorPropagation(t *testing.T) { t.Errorf("expected exactly 1 decision-func call (leader only), got %d", got) } } + +type handshakeTestIssuer struct { + t *testing.T + issued atomic.Int32 +} + +func (i *handshakeTestIssuer) Issue(ctx context.Context, csr *x509.CertificateRequest) (*IssuedCertificate, error) { + i.t.Helper() + i.issued.Add(1) + + now := time.Now() + certPEM := mustIssueCertificateForPublicKey(i.t, csr.PublicKey, csr.DNSNames, now.Add(-time.Hour), now.Add(time.Hour)) + return &IssuedCertificate{Certificate: certPEM}, nil +} + +func (i *handshakeTestIssuer) IssuerKey() string { return "handshake_test" } + +func mustIssueCertificateForPublicKey(t *testing.T, publicKey any, dnsNames []string, notBefore, notAfter time.Time) []byte { + t.Helper() + + signer, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatalf("generating issuer key: %v", err) + } + + commonName := "" + if len(dnsNames) > 0 { + commonName = dnsNames[0] + } + tmpl := &x509.Certificate{ + SerialNumber: big.NewInt(time.Now().UnixNano()), + Subject: pkix.Name{CommonName: commonName}, + DNSNames: dnsNames, + NotBefore: notBefore, + NotAfter: notAfter, + KeyUsage: x509.KeyUsageDigitalSignature | x509.KeyUsageKeyEncipherment, + ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth}, + BasicConstraintsValid: true, + } + + der, err := x509.CreateCertificate(rand.Reader, tmpl, tmpl, publicKey, signer) + if err != nil { + t.Fatalf("creating certificate: %v", err) + } + + return pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: der}) +}