LocalCache: reload cached certificates due for renewal from storage

config.go

@@ -162,6 +162,8 @@ type Config struct {
 	// cache with a renewed certificate beside the key it replaced.
 	// A certificate the local cache cannot supply a usable pair for
 	// is reloaded from Storage, which replaces the local copies.
+	// So is one that is due for renewal, since the local cache does
+	// not learn of renewals done by other instances.
 	//
 	// Beware that this stores private keys on every instance that
 	// serves them, so the local cache should be at least as secure

crypto.go

@@ -239,6 +239,8 @@ func (cfg *Config) loadCertResourceAnyIssuer(ctx context.Context, certNamesKey s
 
 // loadCertResource loads a certificate resource from the given issuer's storage
 // location, using the given storage: see cachedStorage and groundTruthStorage.
+// A resource from the local cache that is due for renewal is read from Storage
+// instead, in case another instance already renewed it.
 func (cfg *Config) loadCertResource(ctx context.Context, issuer Issuer, certNamesKey string, storage Storage) (CertificateResource, error) {
 	certRes := CertificateResource{issuerKey: issuer.IssuerKey()}
 
@@ -267,6 +269,17 @@ func (cfg *Config) loadCertResource(ctx context.Context, issuer Issuer, certName
 		return CertificateResource{}, fmt.Errorf("decoding certificate metadata: %v", err)
 	}
 
+	// the local cache only learns of renewals this instance does, so a cached
+	// certificate due for renewal is the one a peer most likely replaced; the
+	// Storage read refreshes the local copy, so this is paid once per renewal
+	if ls, ok := storage.(localCacheStorage); ok && ls.readLocal {
+		if _, _, stale := cfg.managedCertNeedsRenewal(certRes, false); stale {
+			if current, err := cfg.loadCertResource(ctx, issuer, certNamesKey, cfg.groundTruthStorage()); err == nil {
+				return current, nil
+			}
+		}
+	}
+
 	return certRes, nil
 }
 

localcache_test.go

@@ -123,8 +123,9 @@ func TestLocalCache(t *testing.T) {
 	cfg, am, storage, localCache := testLocalCacheConfig(t)
 	cfg.LocalCache = localCache
 	certKey := StorageKeys.SiteCert(am.IssuerKey(), domain)
+	saved := testIssuedCertResource(t, am, domain, time.Now())
 
-	err := cfg.saveCertResource(ctx, am, testCertResource(am, domain))
+	err := cfg.saveCertResource(ctx, am, saved)
 	if err != nil {
 		t.Fatalf("Expected no error saving cert resource, got: %v", err)
 	}
@@ -136,8 +137,8 @@ func TestLocalCache(t *testing.T) {
 	if err != nil {
 		t.Fatalf("Expected no error loading cert resource, got: %v", err)
 	}
-	if string(certRes.CertificatePEM) != "certificate" {
-		t.Errorf("Expected 'certificate', got: %s", certRes.CertificatePEM)
+	if !bytes.Equal(certRes.CertificatePEM, saved.CertificatePEM) {
+		t.Error("Expected the saved certificate to be loaded")
 	}
 	if len(storage.calls) > 0 {
 		t.Errorf("Expected no storage calls, got: %v", storage.calls)
@@ -221,6 +222,58 @@ func TestLocalCacheTornWrite(t *testing.T) {
 	}
 }
 
+func TestLocalCacheStaleCertRenewedByPeer(t *testing.T) {
+	ctx := context.Background()
+	const domain = "example.com"
+	now := time.Now()
+
+	cfg, preferred, storage, localCache := testLocalCacheConfig(t)
+	fallback := &ACMEIssuer{CA: "https://fallback.example.com/acme/directory"}
+	fallback.config = cfg
+	cfg.Issuers = []Issuer{preferred, fallback}
+	cfg.LocalCache = localCache
+	cfg.OCSP = OCSPConfig{DisableStapling: true}
+
+	// this instance has served the preferred issuer's certificate, now
+	// expired, and has a fallback certificate that is still fine
+	for issuer, issued := range map[Issuer]time.Time{preferred: now.Add(-100 * 24 * time.Hour), fallback: now.Add(-30 * 24 * time.Hour)} {
+		if err := cfg.saveCertResource(ctx, issuer, testIssuedCertResource(t, issuer, domain, issued)); err != nil {
+			t.Fatalf("Expected no error saving cert resource, got: %v", err)
+		}
+	}
+
+	// another instance renewed the preferred certificate, which only
+	// reaches storage; the fallback would mask that forever, since it
+	// never needs renewal itself and so never triggers a storage read
+	renewed := testIssuedCertResource(t, preferred, domain, now)
+	cfg.LocalCache = nil
+	if err := cfg.saveCertResource(ctx, preferred, renewed); err != nil {
+		t.Fatalf("Expected no error saving renewed cert resource, got: %v", err)
+	}
+	cfg.LocalCache = localCache
+
+	cert, err := cfg.CacheManagedCertificate(ctx, domain)
+	if err != nil {
+		t.Fatalf("Expected no error caching managed certificate, got: %v", err)
+	}
+	expected, err := makeCertificate(renewed.CertificatePEM, renewed.PrivateKeyPEM)
+	if err != nil {
+		t.Fatalf("Expected no error making the renewed certificate, got: %v", err)
+	}
+	if !bytes.Equal(cert.Leaf.Raw, expected.Leaf.Raw) {
+		t.Error("Expected the renewed certificate from the preferred issuer to be loaded")
+	}
+
+	// the local cache has caught up, so the next load stays local
+	storage.calls = nil
+	if _, err = cfg.CacheManagedCertificate(ctx, domain); err != nil {
+		t.Fatalf("Expected no error caching managed certificate, got: %v", err)
+	}
+	if len(storage.calls) > 0 {
+		t.Errorf("Expected no storage calls once the local cache caught up, got: %v", storage.calls)
+	}
+}
+
 func TestLocalCacheUnknownDomainLoadedOnce(t *testing.T) {
 	ctx := context.Background()
 
@@ -334,7 +387,7 @@ func TestWarmLocalCache(t *testing.T) {
 	cfg, am, storage, localCache := testLocalCacheConfig(t)
 
 	// no local cache yet, so this only writes to storage
-	err := cfg.saveCertResource(ctx, am, testCertResource(am, domain))
+	err := cfg.saveCertResource(ctx, am, testIssuedCertResource(t, am, domain, time.Now()))
 	if err != nil {
 		t.Fatalf("Expected no error saving cert resource, got: %v", err)
 	}