acme/autocert: treat invalid cert as a cache miss
A cached cert data may be corrupted or simply contain an expired
certificate, which results in GetCertificate returning an error.
This change makes the Manager ignore those invalid and expired
cache entries, treating them as nonexistent.
Fixes golang/go#20035.
Change-Id: I5345291ecb1aab1cf19671cf0a383135c7102038
Reviewed-on: https://go-review.googlesource.com/41690
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
diff --git a/acme/autocert/autocert.go b/acme/autocert/autocert.go
index 98842b4..679ff4d 100644
--- a/acme/autocert/autocert.go
+++ b/acme/autocert/autocert.go
@@ -244,6 +244,7 @@
}
// cacheGet always returns a valid certificate, or an error otherwise.
+// If a cached certficate exists but is not valid, ErrCacheMiss is returned.
func (m *Manager) cacheGet(ctx context.Context, domain string) (*tls.Certificate, error) {
if m.Cache == nil {
return nil, ErrCacheMiss
@@ -256,7 +257,7 @@
// private
priv, pub := pem.Decode(data)
if priv == nil || !strings.Contains(priv.Type, "PRIVATE") {
- return nil, errors.New("acme/autocert: no private key found in cache")
+ return nil, ErrCacheMiss
}
privKey, err := parsePrivateKey(priv.Bytes)
if err != nil {
@@ -274,13 +275,14 @@
pubDER = append(pubDER, b.Bytes)
}
if len(pub) > 0 {
- return nil, errors.New("acme/autocert: invalid public key")
+ // Leftover content not consumed by pem.Decode. Corrupt. Ignore.
+ return nil, ErrCacheMiss
}
// verify and create TLS cert
leaf, err := validCert(domain, pubDER, privKey)
if err != nil {
- return nil, err
+ return nil, ErrCacheMiss
}
tlscert := &tls.Certificate{
Certificate: pubDER,
diff --git a/acme/autocert/autocert_test.go b/acme/autocert/autocert_test.go
index 6df4cf3..3166c1f 100644
--- a/acme/autocert/autocert_test.go
+++ b/acme/autocert/autocert_test.go
@@ -178,6 +178,38 @@
}
}
+func TestGetCertificate_expiredCache(t *testing.T) {
+ // Make an expired cert and cache it.
+ pk, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
+ if err != nil {
+ t.Fatal(err)
+ }
+ tmpl := &x509.Certificate{
+ SerialNumber: big.NewInt(1),
+ Subject: pkix.Name{CommonName: "example.org"},
+ NotAfter: time.Now(),
+ }
+ pub, err := x509.CreateCertificate(rand.Reader, tmpl, tmpl, &pk.PublicKey, pk)
+ if err != nil {
+ t.Fatal(err)
+ }
+ tlscert := &tls.Certificate{
+ Certificate: [][]byte{pub},
+ PrivateKey: pk,
+ }
+
+ man := &Manager{Prompt: AcceptTOS, Cache: newMemCache()}
+ defer man.stopRenew()
+ if err := man.cachePut(context.Background(), "example.org", tlscert); err != nil {
+ t.Fatalf("man.cachePut: %v", err)
+ }
+
+ // The expired cached cert should trigger a new cert issuance
+ // and return without an error.
+ hello := &tls.ClientHelloInfo{ServerName: "example.org"}
+ testGetCertificate(t, man, "example.org", hello)
+}
+
// startACMEServerStub runs an ACME server
// The domain argument is the expected domain name of a certificate request.
func startACMEServerStub(t *testing.T, man *Manager, domain string) (url string, finish func()) {
