Fix crash in expiration-mailer (#1997)

In #1923 we changed reg.Contact to a pointer, which can be nil if the corresponding data from the DB is the literal string "null". This causes panics in expiration-mailer, which we need to fix.

This change fixes modelToRegistration to always return a pointer to a non-nil slice. It also adds an extra sanity check in expiration-mailer itself.

Fixes #1993

https://github.com/letsencrypt/boulder/pull/1997

cmd/expiration-mailer/main.go

@@ -205,6 +205,10 @@ func (m *mailer) processCerts(allCerts []core.Certificate) {
 			continue
 		}
 
+		if reg.Contact == nil {
+			continue
+		}
+
 		err = m.sendNags(*reg.Contact, parsedCerts)
 		if err != nil {
 			m.log.AuditErr(fmt.Sprintf("Error sending nag emails: %s", err))
@@ -234,7 +238,8 @@ func (m *mailer) findExpiringCertificates() error {
 		}
 		right := now.Add(expiresIn)
 
-		m.log.Info(fmt.Sprintf("expiration-mailer: Searching for certificates that expire between %s and %s and had last nag >%s before expiry", left, right, expiresIn))
+		m.log.Info(fmt.Sprintf("expiration-mailer: Searching for certificates that expire between %s and %s and had last nag >%s before expiry",
+			left.UTC(), right.UTC(), expiresIn))
 		var certs []core.Certificate
 		_, err := m.dbMap.Select(
 			&certs,

sa/model.go

@@ -103,10 +103,19 @@ func modelToRegistration(rm *regModel) (core.Registration, error) {
 		err = fmt.Errorf("unable to unmarshal JsonWebKey in db: %s", err)
 		return core.Registration{}, err
 	}
+	var contact *[]*core.AcmeURL
+	// Contact can be nil when the DB contains the literal string "null". We
+	// prefer to represent this in memory as a pointer to an empty slice rather
+	// than a nil pointer.
+	if rm.Contact == nil {
+		contact = &[]*core.AcmeURL{}
+	} else {
+		contact = &rm.Contact
+	}
 	r := core.Registration{
 		ID:        rm.ID,
 		Key:       *k,
-		Contact:   &rm.Contact,
+		Contact:   contact,
 		Agreement: rm.Agreement,
 		InitialIP: net.IP(rm.InitialIP),
 		CreatedAt: rm.CreatedAt,

sa/model_test.go

@@ -0,0 +1,39 @@
+package sa
+
+import (
+	"testing"
+
+	"github.com/letsencrypt/boulder/core"
+)
+
+func TestModelToRegistrationNilContact(t *testing.T) {
+	reg, err := modelToRegistration(&regModel{
+		Key:     []byte(`{"kty":"RSA","n":"AQAB","e":"AQAB"}`),
+		Contact: nil,
+	})
+	if err != nil {
+		t.Errorf("Got error from modelToRegistration: %s", err)
+	}
+	if reg.Contact == nil {
+		t.Errorf("Expected non-nil Contact field, got %#v", reg.Contact)
+	}
+	if len(*reg.Contact) != 0 {
+		t.Errorf("Expected empty Contact field, got %#v", reg.Contact)
+	}
+}
+
+func TestModelToRegistrationNonNilContact(t *testing.T) {
+	reg, err := modelToRegistration(&regModel{
+		Key:     []byte(`{"kty":"RSA","n":"AQAB","e":"AQAB"}`),
+		Contact: []*core.AcmeURL{},
+	})
+	if err != nil {
+		t.Errorf("Got error from modelToRegistration: %s", err)
+	}
+	if reg.Contact == nil {
+		t.Errorf("Expected non-nil Contact field, got %#v", reg.Contact)
+	}
+	if len(*reg.Contact) != 0 {
+		t.Errorf("Expected empty Contact field, got %#v", reg.Contact)
+	}
+}