Skip to content

Commit cb56bf6

Browse files
authored
ca: log cert signing using JSON objects (#7742)
This makes the log events easier to parse, and makes it easier to consistently use the correct fields from the issuance request. Also, reduce the number of fields that are logged on error events. Logging just the serial and the error in most cases should suffice to cross-reference the error with the item that we attempted to sign. One downside is that this increases the total log size (10kB above, vs 7kB from a similar production issuance) due in part to more repetition. For example, both the "signing cert" and "signing cert success" log lines include the full precert DER. Note that our long-term plan for more structured logs is to have a unique event id to join logs on, which can avoid this repetition. But since we don't currently have convenient ways to do that join, some duplication (as we currently have in the logs) seems reasonable.
1 parent 1fa6678 commit cb56bf6

File tree

4 files changed

+114
-60
lines changed

4 files changed

+114
-60
lines changed

ca/ca.go

Lines changed: 53 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ import (
1515
"fmt"
1616
"math/big"
1717
mrand "math/rand/v2"
18-
"strings"
1918
"time"
2019

2120
ct "github.com/google/certificate-transparency-go"
@@ -51,6 +50,25 @@ const (
5150
certType = certificateType("certificate")
5251
)
5352

53+
// issuanceEvent is logged before and after issuance of precertificates and certificates.
54+
// The `omitempty` fields are not always present.
55+
// CSR, Precertificate, and Certificate are hex-encoded DER bytes to make it easier to
56+
// ad-hoc search for sequences or OIDs in logs. Other data, like public key within CSR,
57+
// is logged as base64 because it doesn't have interesting DER structure.
58+
type issuanceEvent struct {
59+
CSR string `json:",omitempty"`
60+
IssuanceRequest *issuance.IssuanceRequest
61+
Issuer string
62+
OrderID int64
63+
Profile string
64+
ProfileHash string
65+
Requester int64
66+
Result struct {
67+
Precertificate string `json:",omitempty"`
68+
Certificate string `json:",omitempty"`
69+
}
70+
}
71+
5472
// Two maps of keys to Issuers. Lookup by PublicKeyAlgorithm is useful for
5573
// determining the set of issuers which can sign a given (pre)cert, based on its
5674
// PublicKeyAlgorithm. Lookup by NameID is useful for looking up a specific
@@ -428,17 +446,22 @@ func (ca *certificateAuthorityImpl) IssueCertificateForPrecertificate(ctx contex
428446
return nil, err
429447
}
430448

431-
names := strings.Join(issuanceReq.DNSNames, ", ")
432-
ca.log.AuditInfof("Signing cert: issuer=[%s] serial=[%s] regID=[%d] names=[%s] certProfileName=[%s] certProfileHash=[%x] precert=[%s]",
433-
issuer.Name(), serialHex, req.RegistrationID, names, certProfile.name, certProfile.hash, hex.EncodeToString(precert.Raw))
434-
435449
lintCertBytes, issuanceToken, err := issuer.Prepare(certProfile.profile, issuanceReq)
436450
if err != nil {
437-
ca.log.AuditErrf("Preparing cert failed: issuer=[%s] serial=[%s] regID=[%d] names=[%s] certProfileName=[%s] certProfileHash=[%x] err=[%v]",
438-
issuer.Name(), serialHex, req.RegistrationID, names, certProfile.name, certProfile.hash, err)
451+
ca.log.AuditErrf("Preparing cert failed: serial=[%s] err=[%v]", serialHex, err)
439452
return nil, berrors.InternalServerError("failed to prepare certificate signing: %s", err)
440453
}
441454

455+
logEvent := issuanceEvent{
456+
IssuanceRequest: issuanceReq,
457+
Issuer: issuer.Name(),
458+
OrderID: req.OrderID,
459+
Profile: certProfile.name,
460+
ProfileHash: hex.EncodeToString(certProfile.hash[:]),
461+
Requester: req.RegistrationID,
462+
}
463+
ca.log.AuditObject("Signing cert", logEvent)
464+
442465
_, span := ca.tracer.Start(ctx, "signing cert", trace.WithAttributes(
443466
attribute.String("serial", serialHex),
444467
attribute.String("issuer", issuer.Name()),
@@ -448,8 +471,7 @@ func (ca *certificateAuthorityImpl) IssueCertificateForPrecertificate(ctx contex
448471
certDER, err := issuer.Issue(issuanceToken)
449472
if err != nil {
450473
ca.metrics.noteSignError(err)
451-
ca.log.AuditErrf("Signing cert failed: issuer=[%s] serial=[%s] regID=[%d] names=[%s] certProfileName=[%s] certProfileHash=[%x] err=[%v]",
452-
issuer.Name(), serialHex, req.RegistrationID, names, certProfile.name, certProfile.hash, err)
474+
ca.log.AuditErrf("Signing cert failed: serial=[%s] err=[%v]", serialHex, err)
453475
span.SetStatus(codes.Error, err.Error())
454476
span.End()
455477
return nil, berrors.InternalServerError("failed to sign certificate: %s", err)
@@ -462,17 +484,16 @@ func (ca *certificateAuthorityImpl) IssueCertificateForPrecertificate(ctx contex
462484
}
463485

464486
ca.metrics.signatureCount.With(prometheus.Labels{"purpose": string(certType), "issuer": issuer.Name()}).Inc()
465-
ca.log.AuditInfof("Signing cert success: issuer=[%s] serial=[%s] regID=[%d] names=[%s] certificate=[%s] certProfileName=[%s] certProfileHash=[%x]",
466-
issuer.Name(), serialHex, req.RegistrationID, names, hex.EncodeToString(certDER), certProfile.name, certProfile.hash)
487+
logEvent.Result.Certificate = hex.EncodeToString(certDER)
488+
ca.log.AuditObject("Signing cert success", logEvent)
467489

468490
_, err = ca.sa.AddCertificate(ctx, &sapb.AddCertificateRequest{
469491
Der: certDER,
470492
RegID: req.RegistrationID,
471493
Issued: timestamppb.New(ca.clk.Now()),
472494
})
473495
if err != nil {
474-
ca.log.AuditErrf("Failed RPC to store at SA: issuer=[%s] serial=[%s] cert=[%s] regID=[%d] orderID=[%d] certProfileName=[%s] certProfileHash=[%x] err=[%v]",
475-
issuer.Name(), serialHex, hex.EncodeToString(certDER), req.RegistrationID, req.OrderID, certProfile.name, certProfile.hash, err)
496+
ca.log.AuditErrf("Failed RPC to store at SA: serial=[%s] err=[%v]", serialHex, hex.EncodeToString(certDER))
476497
return nil, err
477498
}
478499

@@ -568,7 +589,7 @@ func (ca *certificateAuthorityImpl) issuePrecertificateInner(ctx context.Context
568589

569590
names := csrlib.NamesFromCSR(csr)
570591
req := &issuance.IssuanceRequest{
571-
PublicKey: csr.PublicKey,
592+
PublicKey: issuance.MarshalablePublicKey{PublicKey: csr.PublicKey},
572593
SubjectKeyId: subjectKeyId,
573594
Serial: serialBigInt.Bytes(),
574595
DNSNames: names.SANs,
@@ -579,13 +600,9 @@ func (ca *certificateAuthorityImpl) issuePrecertificateInner(ctx context.Context
579600
NotAfter: notAfter,
580601
}
581602

582-
ca.log.AuditInfof("Signing precert: serial=[%s] regID=[%d] names=[%s] csr=[%s]",
583-
serialHex, issueReq.RegistrationID, strings.Join(req.DNSNames, ", "), hex.EncodeToString(csr.Raw))
584-
585603
lintCertBytes, issuanceToken, err := issuer.Prepare(certProfile.profile, req)
586604
if err != nil {
587-
ca.log.AuditErrf("Preparing precert failed: issuer=[%s] serial=[%s] regID=[%d] names=[%s] certProfileName=[%s] certProfileHash=[%x] err=[%v]",
588-
issuer.Name(), serialHex, issueReq.RegistrationID, strings.Join(req.DNSNames, ", "), certProfile.name, certProfile.hash, err)
605+
ca.log.AuditErrf("Preparing precert failed: serial=[%s] err=[%v]", serialHex, err)
589606
if errors.Is(err, linter.ErrLinting) {
590607
ca.metrics.lintErrorCount.Inc()
591608
}
@@ -608,6 +625,17 @@ func (ca *certificateAuthorityImpl) issuePrecertificateInner(ctx context.Context
608625
return nil, nil, err
609626
}
610627

628+
logEvent := issuanceEvent{
629+
CSR: hex.EncodeToString(csr.Raw),
630+
IssuanceRequest: req,
631+
Issuer: issuer.Name(),
632+
Profile: certProfile.name,
633+
ProfileHash: hex.EncodeToString(certProfile.hash[:]),
634+
Requester: issueReq.RegistrationID,
635+
OrderID: issueReq.OrderID,
636+
}
637+
ca.log.AuditObject("Signing precert", logEvent)
638+
611639
_, span := ca.tracer.Start(ctx, "signing precert", trace.WithAttributes(
612640
attribute.String("serial", serialHex),
613641
attribute.String("issuer", issuer.Name()),
@@ -617,8 +645,7 @@ func (ca *certificateAuthorityImpl) issuePrecertificateInner(ctx context.Context
617645
certDER, err := issuer.Issue(issuanceToken)
618646
if err != nil {
619647
ca.metrics.noteSignError(err)
620-
ca.log.AuditErrf("Signing precert failed: issuer=[%s] serial=[%s] regID=[%d] names=[%s] certProfileName=[%s] certProfileHash=[%x] err=[%v]",
621-
issuer.Name(), serialHex, issueReq.RegistrationID, strings.Join(req.DNSNames, ", "), certProfile.name, certProfile.hash, err)
648+
ca.log.AuditErrf("Signing precert failed: serial=[%s] err=[%v]", serialHex, err)
622649
span.SetStatus(codes.Error, err.Error())
623650
span.End()
624651
return nil, nil, berrors.InternalServerError("failed to sign precertificate: %s", err)
@@ -631,8 +658,11 @@ func (ca *certificateAuthorityImpl) issuePrecertificateInner(ctx context.Context
631658
}
632659

633660
ca.metrics.signatureCount.With(prometheus.Labels{"purpose": string(precertType), "issuer": issuer.Name()}).Inc()
634-
ca.log.AuditInfof("Signing precert success: issuer=[%s] serial=[%s] regID=[%d] names=[%s] precert=[%s] certProfileName=[%s] certProfileHash=[%x]",
635-
issuer.Name(), serialHex, issueReq.RegistrationID, strings.Join(req.DNSNames, ", "), hex.EncodeToString(certDER), certProfile.name, certProfile.hash)
661+
662+
logEvent.Result.Precertificate = hex.EncodeToString(certDER)
663+
// The CSR is big and not that informative, so don't log it a second time.
664+
logEvent.CSR = ""
665+
ca.log.AuditObject("Signing precert success", logEvent)
636666

637667
return certDER, &certProfileWithID{certProfile.name, certProfile.hash, nil}, nil
638668
}

ca/ca_test.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -332,7 +332,6 @@ func TestIssuePrecertificate(t *testing.T) {
332332

333333
var certDER []byte
334334
response, err := ca.IssuePrecertificate(ctx, issueReq)
335-
336335
test.AssertNotError(t, err, "Failed to issue precertificate")
337336
certDER = response.DER
338337

issuance/cert.go

Lines changed: 34 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"crypto/x509"
1010
"crypto/x509/pkix"
1111
"encoding/asn1"
12+
"encoding/json"
1213
"errors"
1314
"fmt"
1415
"math/big"
@@ -153,7 +154,7 @@ func (p *Profile) GenerateValidity(now time.Time) (time.Time, time.Time) {
153154
// requestValid verifies the passed IssuanceRequest against the profile. If the
154155
// request doesn't match the signing profile an error is returned.
155156
func (i *Issuer) requestValid(clk clock.Clock, prof *Profile, req *IssuanceRequest) error {
156-
switch req.PublicKey.(type) {
157+
switch req.PublicKey.PublicKey.(type) {
157158
case *rsa.PublicKey, *ecdsa.PublicKey:
158159
default:
159160
return errors.New("unsupported public key type")
@@ -261,12 +262,36 @@ var mustStapleExt = pkix.Extension{
261262
Value: []byte{0x30, 0x03, 0x02, 0x01, 0x05},
262263
}
263264

265+
// MarshalablePublicKey is a wrapper for crypto.PublicKey with a custom JSON
266+
// marshaller that encodes the public key as a DER-encoded SubjectPublicKeyInfo.
267+
type MarshalablePublicKey struct {
268+
crypto.PublicKey
269+
}
270+
271+
func (pk MarshalablePublicKey) MarshalJSON() ([]byte, error) {
272+
keyDER, err := x509.MarshalPKIXPublicKey(pk.PublicKey)
273+
if err != nil {
274+
return nil, err
275+
}
276+
return json.Marshal(keyDER)
277+
}
278+
279+
type HexMarshalableBytes []byte
280+
281+
func (h HexMarshalableBytes) MarshalJSON() ([]byte, error) {
282+
return json.Marshal(fmt.Sprintf("%x", h))
283+
}
284+
264285
// IssuanceRequest describes a certificate issuance request
286+
//
287+
// It can be marshaled as JSON for logging purposes, though note that sctList and precertDER
288+
// will be omitted from the marshaled output because they are unexported.
265289
type IssuanceRequest struct {
266-
PublicKey crypto.PublicKey
267-
SubjectKeyId []byte
290+
// PublicKey is of type MarshalablePublicKey so we can log an IssuanceRequest as a JSON object.
291+
PublicKey MarshalablePublicKey
292+
SubjectKeyId HexMarshalableBytes
268293

269-
Serial []byte
294+
Serial HexMarshalableBytes
270295

271296
NotBefore time.Time
272297
NotAfter time.Time
@@ -294,7 +319,7 @@ type IssuanceRequest struct {
294319
type issuanceToken struct {
295320
mu sync.Mutex
296321
template *x509.Certificate
297-
pubKey any
322+
pubKey MarshalablePublicKey
298323
// A pointer to the issuer that created this token. This token may only
299324
// be redeemed by the same issuer.
300325
issuer *Issuer
@@ -335,7 +360,7 @@ func (i *Issuer) Prepare(prof *Profile, req *IssuanceRequest) ([]byte, *issuance
335360
}
336361
template.DNSNames = req.DNSNames
337362

338-
switch req.PublicKey.(type) {
363+
switch req.PublicKey.PublicKey.(type) {
339364
case *rsa.PublicKey:
340365
if prof.omitKeyEncipherment {
341366
template.KeyUsage = x509.KeyUsageDigitalSignature
@@ -371,7 +396,7 @@ func (i *Issuer) Prepare(prof *Profile, req *IssuanceRequest) ([]byte, *issuance
371396

372397
// check that the tbsCertificate is properly formed by signing it
373398
// with a throwaway key and then linting it using zlint
374-
lintCertBytes, err := i.Linter.Check(template, req.PublicKey, prof.lints)
399+
lintCertBytes, err := i.Linter.Check(template, req.PublicKey.PublicKey, prof.lints)
375400
if err != nil {
376401
return nil, nil, fmt.Errorf("tbsCertificate linting failed: %w", err)
377402
}
@@ -406,7 +431,7 @@ func (i *Issuer) Issue(token *issuanceToken) ([]byte, error) {
406431
return nil, errors.New("tried to redeem issuance token with the wrong issuer")
407432
}
408433

409-
return x509.CreateCertificate(rand.Reader, template, i.Cert.Certificate, token.pubKey, i.Signer)
434+
return x509.CreateCertificate(rand.Reader, template, i.Cert.Certificate, token.pubKey.PublicKey, i.Signer)
410435
}
411436

412437
// ContainsMustStaple returns true if the provided set of extensions includes
@@ -441,7 +466,7 @@ func RequestFromPrecert(precert *x509.Certificate, scts []ct.SignedCertificateTi
441466
return nil, errors.New("provided certificate doesn't contain the CT poison extension")
442467
}
443468
return &IssuanceRequest{
444-
PublicKey: precert.PublicKey,
469+
PublicKey: MarshalablePublicKey{precert.PublicKey},
445470
SubjectKeyId: precert.SubjectKeyId,
446471
Serial: precert.SerialNumber.Bytes(),
447472
NotBefore: precert.NotBefore,

0 commit comments

Comments
 (0)