fix(certdb): use sql.NullString for CertificateRecord.CommonName

Rows inserted before the migration in #1126 will have the `common_name` set to NULL. This fixes selects for those rows.

Author: nickysemenza

certdb/certdb.go

@@ -1,6 +1,7 @@
 package certdb
 
 import (
+	"database/sql"
 	"encoding/json"
 	"time"
 
@@ -22,7 +23,7 @@ type CertificateRecord struct {
 	NotBefore    time.Time      `db:"not_before"`
 	MetadataJSON types.JSONText `db:"metadata"`
 	SANsJSON     types.JSONText `db:"sans"`
-	CommonName   string         `db:"common_name"`
+	CommonName   sql.NullString `db:"common_name"`
 }
 
 // SetMetadata sets the metadata json

certdb/sql/sql_test.go

@@ -52,6 +52,7 @@ func roughlySameTime(t1, t2 time.Time) bool {
 func testEverything(ta TestAccessor, t *testing.T) {
 	testInsertCertificateAndGetCertificate(ta, t)
 	testInsertCertificateAndGetUnexpiredCertificate(ta, t)
+	testInsertCertificateAndGetUnexpiredCertificateNullCommonName(ta, t)
 	testUpdateCertificateAndGetCertificate(ta, t)
 	testInsertOCSPAndGetOCSP(ta, t)
 	testInsertOCSPAndGetUnexpiredOCSP(ta, t)
@@ -153,6 +154,54 @@ func testInsertCertificateAndGetUnexpiredCertificate(ta TestAccessor, t *testing
 		t.Error("Should have 1 unexpired certificate record:", len(unexpired))
 	}
 }
+func testInsertCertificateAndGetUnexpiredCertificateNullCommonName(ta TestAccessor, t *testing.T) {
+	ta.Truncate()
+
+	expiry := time.Now().Add(time.Minute)
+	want := certdb.CertificateRecord{
+		PEM:    "fake cert data",
+		Serial: "fake serial 2",
+		AKI:    fakeAKI,
+		Status: "good",
+		Reason: 0,
+		Expiry: expiry,
+	}
+
+	if err := ta.Accessor.InsertCertificate(want); err != nil {
+		t.Fatal(err)
+	}
+
+	// simulate situation where there are rows before migrate 002 has been run
+	ta.DB.MustExec("update certificates set common_name = NULL")
+
+	rets, err := ta.Accessor.GetCertificate(want.Serial, want.AKI)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if len(rets) != 1 {
+		t.Fatal("should return exactly one record")
+	}
+
+	got := rets[0]
+
+	// reflection comparison with zero time objects are not stable as it seems
+	if want.Serial != got.Serial || want.Status != got.Status ||
+		want.AKI != got.AKI || !got.RevokedAt.IsZero() ||
+		want.PEM != got.PEM || !roughlySameTime(got.Expiry, expiry) {
+		t.Errorf("want Certificate %+v, got %+v", want, got)
+	}
+
+	unexpired, err := ta.Accessor.GetUnexpiredCertificates()
+
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if len(unexpired) != 1 {
+		t.Error("Should have 1 unexpired certificate record:", len(unexpired))
+	}
+}
 
 func testUpdateCertificateAndGetCertificate(ta TestAccessor, t *testing.T) {
 	ta.Truncate()

signer/local/local.go

@@ -9,6 +9,7 @@ import (
 	"crypto/rand"
 	"crypto/x509"
 	"crypto/x509/pkix"
+	"database/sql"
 	"encoding/asn1"
 	"encoding/hex"
 	"encoding/pem"
@@ -517,7 +518,7 @@ func (s *Signer) Sign(req signer.SignRequest) (cert []byte, err error) {
 			PEM:        string(signedCert),
 			IssuedAt:   time.Now(),
 			NotBefore:  certTBS.NotBefore,
-			CommonName: certTBS.Subject.CommonName,
+			CommonName: sql.NullString{String: certTBS.Subject.CommonName, Valid: true},
 		}
 
 		if err := certRecord.SetMetadata(req.Metadata); err != nil {