Ask before renewing and uncache rejected certs; fix certs path

acmeclient.go

@@ -291,7 +291,7 @@ func (c *acmeClient) nextChallenge(available []challenge.Type) (challenge.Type,
 
 func (c *acmeClient) throttle(ctx context.Context, names []string) error {
 	// throttling is scoped to CA + account email
-	rateLimiterKey := c.mgr.CA + "," + c.mgr.Email
+	rateLimiterKey := c.caURL + "," + c.mgr.Email
 	rateLimitersMu.Lock()
 	rl, ok := rateLimiters[rateLimiterKey]
 	if !ok {

async.go

@@ -23,6 +23,9 @@ type namedJob struct {
 	job  func() error
 }
 
+// Submit enqueues the given job with the given name. If a job
+// with name is already enqueued or running, this is a no-op.
+// The job manager will then run this job as soon as it is able.
 func (jm *jobManager) Submit(name string, job func() error) {
 	jm.mu.Lock()
 	defer jm.mu.Unlock()

config.go

@@ -203,7 +203,8 @@ func newWithCache(certCache *Cache, cfg Config) *Config {
 		}
 		// issuer and revoker go together; if user
 		// specifies their own issuer, we don't want
-		// to override their revoker
+		// to override their revoker, hence we only
+		// do this if Issuer was also nil
 		if cfg.Revoker == nil {
 			cfg.Revoker = Default.Revoker
 			if cfg.Revoker == nil {

handshake.go

@@ -289,7 +289,9 @@ func (cfg *Config) obtainOnDemandCertificate(hello *tls.ClientHelloInfo) (Certif
 
 	// obtain the certificate
 	log.Printf("[INFO] Obtaining new certificate for %s", name)
-	err := cfg.ObtainCert(context.TODO(), name, false) // TODO: use a proper context
+	ctx, cancel := context.WithTimeout(context.TODO(), 2*time.Minute) // TODO: use a proper context; we use one with timeout because retries are enabled
+	defer cancel()
+	err := cfg.ObtainCert(ctx, name, false) // TODO: Should we set interactive=true to disable retries?
 
 	// immediately unblock anyone waiting for it; doing this in
 	// a defer would risk deadlock because of the recursive call
@@ -363,9 +365,21 @@ func (cfg *Config) renewDynamicCertificate(hello *tls.ClientHelloInfo, currentCe
 	obtainCertWaitChans[name] = wait
 	obtainCertWaitChansMu.Unlock()
 
+	// Make sure a certificate for this name should be obtained on-demand
+	err := cfg.checkIfCertShouldBeObtained(name)
+	if err != nil {
+		// if not, remove from cache (it will be deleted from storage later)
+		cfg.certCache.mu.Lock()
+		cfg.certCache.removeCertificate(currentCert)
+		cfg.certCache.mu.Unlock()
+		return Certificate{}, err
+	}
+
 	// renew and reload the certificate
 	log.Printf("[INFO] Renewing certificate for %s", name)
-	err := cfg.RenewCert(context.TODO(), name, false) // TODO: use a proper context
+	ctx, cancel := context.WithTimeout(context.TODO(), 2*time.Minute) // TODO: use a proper context; we use one with timeout because retries are enabled
+	defer cancel()
+	err = cfg.RenewCert(ctx, name, false) // TODO: Should we set interactive=true to disable retries?
 	if err == nil {
 		// even though the recursive nature of the dynamic cert loading
 		// would just call this function anyway, we do it here to

maintain.go

@@ -311,7 +311,7 @@ func (certCache *Cache) updateOCSPStaples(ctx context.Context) {
 		renewName := oldCert.Names[0]
 		cfg := configs[renewName]
 
-		// TODO: consider using a new key in this situation
+		// TODO: consider using a new key in this situation, but we don't know if key storage has been compromised...
 		err := cfg.RenewCert(ctx, renewName, false)
 		if err != nil {
 			// probably better to not serve a revoked certificate at all
@@ -386,14 +386,14 @@ func deleteOldOCSPStaples(storage Storage) error {
 }
 
 func deleteExpiredCerts(storage Storage, gracePeriod time.Duration) error {
-	acmeKeys, err := storage.List(prefixACME, false)
+	issuerKeys, err := storage.List(prefixCerts, false)
 	if err != nil {
 		// maybe just hasn't been created yet; no big deal
 		return nil
 	}
 
-	for _, acmeKey := range acmeKeys {
-		siteKeys, err := storage.List(path.Join(acmeKey, "sites"), false)
+	for _, issuerKey := range issuerKeys {
+		siteKeys, err := storage.List(issuerKey, false)
 		if err != nil {
 			continue
 		}