Skip to content

Commit

Permalink
ra: log previous certificate timestamp
Browse files Browse the repository at this point in the history
  • Loading branch information
jsha committed Dec 13, 2024
1 parent cd202c0 commit 070fdc4
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 13 deletions.
31 changes: 20 additions & 11 deletions ra/ra.go
Original file line number Diff line number Diff line change
Expand Up @@ -337,6 +337,10 @@ type certificateRequestEvent struct {
// CertProfileHash is SHA256 sum over every exported field of an
// issuance.ProfileConfig, represented here as a hexadecimal string.
CertProfileHash string `json:",omitempty"`
// PreviousCertificateIssued is present when this certificate uses the same set
// of FQDNs as a previous certificate (from any account) and contains the
// notBefore of the most recent such certificate.
PreviousCertificateIssued time.Time `json:",omitempty"`
}

// certificateRevocationEvent is a struct for holding information that is logged
Expand Down Expand Up @@ -1135,9 +1139,23 @@ func (ra *RegistrationAuthorityImpl) issueCertificateOuter(
ra.inflightFinalizes.Inc()
defer ra.inflightFinalizes.Dec()

isRenewal := false
timestamps, err := ra.SA.FQDNSetTimestampsForWindow(ctx, &sapb.CountFQDNSetsRequest{
DnsNames: order.DnsNames,
Window: durationpb.New(120 * 24 * time.Hour),
Limit: 1,
})
if err != nil {
return nil, fmt.Errorf("checking if certificate is a renewal: %w", err)
}
if len(timestamps.Timestamps) > 0 {
isRenewal = true
logEvent.PreviousCertificateIssued = timestamps.Timestamps[0].AsTime()
}

// Step 3: Issue the Certificate
cert, cpId, err := ra.issueCertificateInner(
ctx, csr, order.CertificateProfileName, accountID(order.RegistrationID), orderID(order.Id))
ctx, csr, isRenewal, order.CertificateProfileName, accountID(order.RegistrationID), orderID(order.Id))

// Step 4: Fail the order if necessary, and update metrics and log fields
var result string
Expand Down Expand Up @@ -1245,6 +1263,7 @@ type certProfileID struct {
func (ra *RegistrationAuthorityImpl) issueCertificateInner(
ctx context.Context,
csr *x509.CertificateRequest,
isRenewal bool,
profileName string,
acctID accountID,
oID orderID) (*x509.Certificate, *certProfileID, error) {
Expand Down Expand Up @@ -1290,16 +1309,6 @@ func (ra *RegistrationAuthorityImpl) issueCertificateInner(
return nil, nil, wrapError(err, "getting SCTs")
}

timestamps, err := ra.SA.FQDNSetTimestampsForWindow(ctx, &sapb.CountFQDNSetsRequest{
DnsNames: parsedPrecert.DNSNames,
Window: durationpb.New(120 * 24 * time.Hour),
Limit: 1,
})
if err != nil {
return nil, nil, wrapError(err, "checking if certificate is a renewal")
}
isRenewal := len(timestamps.Timestamps) > 0

cert, err := ra.CA.IssueCertificateForPrecertificate(ctx, &capb.IssueCertificateForPrecertificateRequest{
DER: precert.DER,
SCTs: scts,
Expand Down
4 changes: 2 additions & 2 deletions ra/ra_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3904,7 +3904,7 @@ func TestIssueCertificateInnerErrs(t *testing.T) {
// Mock the CA
ra.CA = tc.Mock
// Attempt issuance
_, _, err = ra.issueCertificateInner(ctx, csrOb, order.CertificateProfileName, accountID(Registration.Id), orderID(order.Id))
_, _, err = ra.issueCertificateInner(ctx, csrOb, false, order.CertificateProfileName, accountID(Registration.Id), orderID(order.Id))
// We expect all of the testcases to fail because all use mocked CAs that deliberately error
test.AssertError(t, err, "issueCertificateInner with failing mock CA did not fail")
// If there is an expected `error` then match the error message
Expand Down Expand Up @@ -3985,7 +3985,7 @@ func TestIssueCertificateInnerWithProfile(t *testing.T) {

// Call issueCertificateInner with the CSR generated above and the profile
// name "default", which will cause the mockCA to return a specific hash.
_, cpId, err := ra.issueCertificateInner(context.Background(), csr, "default", 1, 1)
_, cpId, err := ra.issueCertificateInner(context.Background(), csr, false, "default", 1, 1)
test.AssertNotError(t, err, "issuing cert with profile name")
test.AssertEquals(t, mockCA.profileName, cpId.name)
test.AssertByteEquals(t, mockCA.profileHash, cpId.hash)
Expand Down
2 changes: 2 additions & 0 deletions sa/db/boulder_sa/20230419000000_CombinedSchema.sql
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,8 @@ CREATE TABLE `fqdnSets` (
`id` bigint(20) UNSIGNED NOT NULL AUTO_INCREMENT,
`setHash` binary(32) NOT NULL,
`serial` varchar(255) NOT NULL,
-- Note: This should actually be called "notBefore" since it is set
-- based on the certificate's notBefore field, not the issuance time.
`issued` datetime NOT NULL,
`expires` datetime NOT NULL,
PRIMARY KEY (`id`),
Expand Down

0 comments on commit 070fdc4

Please sign in to comment.