Skip to content

Commit

Permalink
Merge pull request #1136 from nickysemenza/fix-null-commonname
Browse files Browse the repository at this point in the history
fix(certdb): allow reading other null columns (part 2 of #1135)
  • Loading branch information
nickysemenza committed Oct 21, 2020
2 parents 8fb5413 + e8f9337 commit d6ad84e
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 15 deletions.
21 changes: 11 additions & 10 deletions certdb/certdb.go
Expand Up @@ -11,16 +11,17 @@ import (
// CertificateRecord encodes a certificate and its metadata
// that will be recorded in a database.
type CertificateRecord struct {
Serial string `db:"serial_number"`
AKI string `db:"authority_key_identifier"`
CALabel string `db:"ca_label"`
Status string `db:"status"`
Reason int `db:"reason"`
Expiry time.Time `db:"expiry"`
RevokedAt time.Time `db:"revoked_at"`
PEM string `db:"pem"`
IssuedAt time.Time `db:"issued_at"`
NotBefore time.Time `db:"not_before"`
Serial string `db:"serial_number"`
AKI string `db:"authority_key_identifier"`
CALabel string `db:"ca_label"`
Status string `db:"status"`
Reason int `db:"reason"`
Expiry time.Time `db:"expiry"`
RevokedAt time.Time `db:"revoked_at"`
PEM string `db:"pem"`
// the following fields will be empty for data inserted before migrate 002 has been run.
IssuedAt *time.Time `db:"issued_at"`
NotBefore *time.Time `db:"not_before"`
MetadataJSON types.JSONText `db:"metadata"`
SANsJSON types.JSONText `db:"sans"`
CommonName sql.NullString `db:"common_name"`
Expand Down
13 changes: 11 additions & 2 deletions certdb/sql/database_accessor.go
Expand Up @@ -101,6 +101,15 @@ func (d *Accessor) InsertCertificate(cr certdb.CertificateRecord) error {
return err
}

var issuedAt, notBefore *time.Time
if cr.IssuedAt != nil {
t := cr.IssuedAt.UTC()
issuedAt = &t
}
if cr.NotBefore != nil {
t := cr.NotBefore.UTC()
notBefore = &t
}
res, err := d.db.NamedExec(insertSQL, &certdb.CertificateRecord{
Serial: cr.Serial,
AKI: cr.AKI,
Expand All @@ -110,8 +119,8 @@ func (d *Accessor) InsertCertificate(cr certdb.CertificateRecord) error {
Expiry: cr.Expiry.UTC(),
RevokedAt: cr.RevokedAt.UTC(),
PEM: cr.PEM,
IssuedAt: cr.IssuedAt.UTC(),
NotBefore: cr.NotBefore.UTC(),
IssuedAt: issuedAt,
NotBefore: notBefore,
MetadataJSON: cr.MetadataJSON,
SANsJSON: cr.SANsJSON,
CommonName: cr.CommonName,
Expand Down
7 changes: 6 additions & 1 deletion certdb/sql/sql_test.go
Expand Up @@ -172,7 +172,12 @@ func testInsertCertificateAndGetUnexpiredCertificateNullCommonName(ta TestAccess
}

// simulate situation where there are rows before migrate 002 has been run
ta.DB.MustExec("update certificates set common_name = NULL")
ta.DB.MustExec(`update certificates
set issued_at = NULL,
not_before = NULL,
metadata = NULL,
sans = NULL,
common_name = NULL;`)

rets, err := ta.Accessor.GetCertificate(want.Serial, want.AKI)
if err != nil {
Expand Down
5 changes: 3 additions & 2 deletions signer/local/local.go
Expand Up @@ -507,6 +507,7 @@ func (s *Signer) Sign(req signer.SignRequest) (cert []byte, err error) {
parsedCert, _ := helpers.ParseCertificatePEM(signedCert)

if s.dbAccessor != nil {
now := time.Now()
var certRecord = certdb.CertificateRecord{
Serial: certTBS.SerialNumber.String(),
// this relies on the specific behavior of x509.CreateCertificate
Expand All @@ -516,8 +517,8 @@ func (s *Signer) Sign(req signer.SignRequest) (cert []byte, err error) {
Status: "good",
Expiry: certTBS.NotAfter,
PEM: string(signedCert),
IssuedAt: time.Now(),
NotBefore: certTBS.NotBefore,
IssuedAt: &now,
NotBefore: &certTBS.NotBefore,
CommonName: sql.NullString{String: certTBS.Subject.CommonName, Valid: true},
}

Expand Down

0 comments on commit d6ad84e

Please sign in to comment.