expiration-mailer: don't re-attempt bad addresses (#6246)

When expiration-mailer attempts to send nag emails, and
the result is a "Bad Address" error, mark the certificates in
question as having had their last expiration nag sent, so we
don't keep retrying them every time expiration-mailer runs.

To facilitate this, factor out more of the code which performs
the database updates into a more robust helper function, and
optimize it to perform all of the updates at once.

Fixes #6185

cmd/expiration-mailer/main.go

@@ -188,11 +188,28 @@ func (m *mailer) sendNags(conn bmail.Conn, contacts []string, certs []*x509.Cert
 	return nil
 }
 
-func (m *mailer) updateCertStatus(ctx context.Context, serial string) error {
-	_, err := m.dbMap.WithContext(ctx).Exec(
-		"UPDATE certificateStatus SET lastExpirationNagSent = ? WHERE serial = ?",
-		m.clk.Now(), serial)
-	return err
+// updateLastNagTimestamps updates the lastExpirationNagSent column for every cert in
+// the given list. Even though it can encounter errors, it only logs them and
+// does not return them, because we always prefer to simply continue.
+func (m *mailer) updateLastNagTimestamps(ctx context.Context, certs []*x509.Certificate) {
+	qmarks := make([]string, len(certs))
+	params := make([]interface{}, len(certs)+1)
+	for i, cert := range certs {
+		qmarks[i] = "?"
+		params[i+1] = core.SerialToString(cert.SerialNumber)
+	}
+
+	query := fmt.Sprintf(
+		"UPDATE certificateStatus SET lastExpirationNagSent = ? WHERE serial IN (%s)",
+		strings.Join(qmarks, ","),
+	)
+	params[0] = m.clk.Now()
+
+	_, err := m.dbMap.WithContext(ctx).Exec(query, params...)
+	if err != nil {
+		m.log.AuditErrf("Error updating certificate status for %d certs: %s", len(certs), err)
+		m.stats.errorCount.With(prometheus.Labels{"type": "UpdateCertificateStatus"}).Inc()
+	}
 }
 
 func (m *mailer) certIsRenewed(ctx context.Context, names []string, issued time.Time) (bool, error) {
@@ -301,11 +318,7 @@ func (m *mailer) sendToOneRegID(ctx context.Context, conn bmail.Conn, regID int6
 		} else if renewed {
 			m.log.Debugf("Cert %s is already renewed", cert.Serial)
 			m.stats.certificatesAlreadyRenewed.Add(1)
-			err := m.updateCertStatus(ctx, cert.Serial)
-			if err != nil {
-				m.log.AuditErrf("Error updating certificate status for %s: %s", cert.Serial, err)
-				m.stats.errorCount.With(prometheus.Labels{"type": "UpdateCertificateStatus"}).Inc()
-			}
+			m.updateLastNagTimestamps(ctx, []*x509.Certificate{parsedCert})
 			continue
 		}
 
@@ -321,21 +334,18 @@ func (m *mailer) sendToOneRegID(ctx context.Context, conn bmail.Conn, regID int6
 
 	err = m.sendNags(conn, reg.Contact, parsedCerts)
 	if err != nil {
+		// Check to see if the error was due to the mail being undeliverable,
+		// in which case we don't want to try again later.
+		var badAddrErr *bmail.BadAddressSMTPError
+		if ok := errors.As(err, &badAddrErr); ok {
+			m.updateLastNagTimestamps(ctx, parsedCerts)
+		}
+
 		m.stats.errorCount.With(prometheus.Labels{"type": "SendNags"}).Inc()
 		return fmt.Errorf("sending nag emails: %s", err)
 	}
-	for _, cert := range parsedCerts {
-		if ctx.Err() != nil {
-			return ctx.Err()
-		}
-		serial := core.SerialToString(cert.SerialNumber)
-		err = m.updateCertStatus(ctx, serial)
-		if err != nil {
-			m.log.AuditErrf("Error updating certificate status for %s: %s", serial, err)
-			m.stats.errorCount.With(prometheus.Labels{"type": "UpdateCertificateStatus"}).Inc()
-			continue
-		}
-	}
+
+	m.updateLastNagTimestamps(ctx, parsedCerts)
 	return nil
 }
 

mail/mailer.go

@@ -347,6 +347,7 @@ var badAddressErrorCodes = map[int]bool{
 	422: true, // Recipient mailbox is full
 	441: true, // Recipient server is not responding
 	450: true, // User's mailbox is not available
+	501: true, // Bad recipient address syntax
 	510: true, // Invalid recipient
 	511: true, // Invalid recipient
 	513: true, // Address type invalid