Skip to content

Commit

Permalink
orphan-finder: set cert issued date based on notbefore. (letsencrypt#…
Browse files Browse the repository at this point in the history 
…3651)

The Boulder orphan-finder command uses the SA's AddCertificate RPC to add orphaned certificates it finds back to the DB. Prior to this commit this RPC always set the core.Certificate.Issued field to the
current time. For the orphan-finder case this meant that the Issued date would incorrectly be set to when the certificate was found, not when it was actually issued. This could cause cert-checker to alarm based on the unusual delta between the cert NotBefore and the core.Certificate.Issued value.

This PR updates the AddCertificate RPC to accept an optional issued timestamp in the request arguments. In the SA layer we address deployability concerns by setting a default value of the current time when none is explicitly provided. This matches the classic behaviour and will let an old RA communicate with a new SA.

This PR updates the orphan-finder to provide an explicit issued time to sa.AddCertificate. The explicit issued time is calculated using the found certificate's NotBefore and the configured backdate.
This lets the orphan-finder set the true issued time in the core.Certificate object, avoiding any cert-checker alarms.

Resolves letsencrypt#3624
  • Loading branch information
@cpu
cpu authored and Roland Bracewell Shoemaker committed Apr 19, 2018
1 parent cb548e3 commit f8f9a15
Show file tree
Hide file tree
Showing 14 changed files with 311 additions and 166 deletions.
5 changes: 3 additions & 2 deletions ca/ca.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ const (
)

type certificateStorage interface {
AddCertificate(context.Context, []byte, int64, []byte) (string, error)
AddCertificate(context.Context, []byte, int64, []byte, *time.Time) (string, error)
}

type certificateType string
Expand Down Expand Up @@ -663,7 +663,8 @@ func (ca *CertificateAuthorityImpl) generateOCSPAndStoreCertificate(
// and generate the initial response in this case.
}

_, err = ca.sa.AddCertificate(ctx, certDER, regID, ocspResp)
now := ca.clk.Now()
_, err = ca.sa.AddCertificate(ctx, certDER, regID, ocspResp, &now)
if err != nil {
err = berrors.InternalServerError(err.Error())
// Note: This log line is parsed by cmd/orphan-finder. If you make any
Expand Down
2 changes: 1 addition & 1 deletion ca/ca_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ type mockSA struct {
certificate core.Certificate
}

func (m *mockSA) AddCertificate(ctx context.Context, der []byte, _ int64, _ []byte) (string, error) {
func (m *mockSA) AddCertificate(ctx context.Context, der []byte, _ int64, _ []byte, _ *time.Time) (string, error) {
m.certificate.DER = der
return "", nil
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/cert-checker/main_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,7 @@ func TestGetAndProcessCerts(t *testing.T) {
rawCert.SerialNumber = big.NewInt(mrand.Int63())
certDER, err := x509.CreateCertificate(rand.Reader, &rawCert, &rawCert, &testKey.PublicKey, testKey)
test.AssertNotError(t, err, "Couldn't create certificate")
_, err = sa.AddCertificate(context.Background(), certDER, reg.ID, nil)
_, err = sa.AddCertificate(context.Background(), certDER, reg.ID, nil, nil)
test.AssertNotError(t, err, "Couldn't add certificate")
}

Expand Down
32 changes: 16 additions & 16 deletions cmd/ocsp-updater/main_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ func TestGenerateAndStoreOCSPResponse(t *testing.T) {
reg := satest.CreateWorkingRegistration(t, sa)
parsedCert, err := core.LoadCert("test-cert.pem")
test.AssertNotError(t, err, "Couldn't read test certificate")
_, err = sa.AddCertificate(ctx, parsedCert.Raw, reg.ID, nil)
_, err = sa.AddCertificate(ctx, parsedCert.Raw, reg.ID, nil, nil)
test.AssertNotError(t, err, "Couldn't add test-cert.pem")

status, err := sa.GetCertificateStatus(ctx, core.SerialToString(parsedCert.SerialNumber))
Expand Down Expand Up @@ -214,11 +214,11 @@ func TestGenerateOCSPResponses(t *testing.T) {
reg := satest.CreateWorkingRegistration(t, sa)
parsedCertA, err := core.LoadCert("test-cert.pem")
test.AssertNotError(t, err, "Couldn't read test certificate")
_, err = sa.AddCertificate(ctx, parsedCertA.Raw, reg.ID, nil)
_, err = sa.AddCertificate(ctx, parsedCertA.Raw, reg.ID, nil, nil)
test.AssertNotError(t, err, "Couldn't add test-cert.pem")
parsedCertB, err := core.LoadCert("test-cert-b.pem")
test.AssertNotError(t, err, "Couldn't read test certificate")
_, err = sa.AddCertificate(ctx, parsedCertB.Raw, reg.ID, nil)
_, err = sa.AddCertificate(ctx, parsedCertB.Raw, reg.ID, nil, nil)
test.AssertNotError(t, err, "Couldn't add test-cert-b.pem")

// We need to set a fake "ocspLastUpdated" value for the two certs we created
Expand Down Expand Up @@ -263,7 +263,7 @@ func TestFindStaleOCSPResponses(t *testing.T) {
reg := satest.CreateWorkingRegistration(t, sa)
parsedCert, err := core.LoadCert("test-cert.pem")
test.AssertNotError(t, err, "Couldn't read test certificate")
_, err = sa.AddCertificate(ctx, parsedCert.Raw, reg.ID, nil)
_, err = sa.AddCertificate(ctx, parsedCert.Raw, reg.ID, nil, nil)
test.AssertNotError(t, err, "Couldn't add test-cert.pem")

// We need to set a fake "ocspLastUpdated" value for the cert we created
Expand Down Expand Up @@ -300,11 +300,11 @@ func TestFindStaleOCSPResponsesStaleMaxAge(t *testing.T) {
reg := satest.CreateWorkingRegistration(t, sa)
parsedCertA, err := core.LoadCert("test-cert.pem")
test.AssertNotError(t, err, "Couldn't read test certificate")
_, err = sa.AddCertificate(ctx, parsedCertA.Raw, reg.ID, nil)
_, err = sa.AddCertificate(ctx, parsedCertA.Raw, reg.ID, nil, nil)
test.AssertNotError(t, err, "Couldn't add test-cert.pem")
parsedCertB, err := core.LoadCert("test-cert-b.pem")
test.AssertNotError(t, err, "Couldn't read test certificate")
_, err = sa.AddCertificate(ctx, parsedCertB.Raw, reg.ID, nil)
_, err = sa.AddCertificate(ctx, parsedCertB.Raw, reg.ID, nil, nil)
test.AssertNotError(t, err, "Couldn't add test-cert-b.pem")

// Set a "ocspLastUpdated" value of 3 days ago for parsedCertA
Expand Down Expand Up @@ -340,7 +340,7 @@ func TestGetCertificatesWithMissingResponses(t *testing.T) {
reg := satest.CreateWorkingRegistration(t, sa)
cert, err := core.LoadCert("test-cert.pem")
test.AssertNotError(t, err, "Couldn't read test certificate")
_, err = sa.AddCertificate(ctx, cert.Raw, reg.ID, nil)
_, err = sa.AddCertificate(ctx, cert.Raw, reg.ID, nil, nil)
test.AssertNotError(t, err, "Couldn't add test-cert.pem")

statuses, err := updater.getCertificatesWithMissingResponses(10)
Expand All @@ -355,7 +355,7 @@ func TestFindRevokedCertificatesToUpdate(t *testing.T) {
reg := satest.CreateWorkingRegistration(t, sa)
cert, err := core.LoadCert("test-cert.pem")
test.AssertNotError(t, err, "Couldn't read test certificate")
_, err = sa.AddCertificate(ctx, cert.Raw, reg.ID, nil)
_, err = sa.AddCertificate(ctx, cert.Raw, reg.ID, nil, nil)
test.AssertNotError(t, err, "Couldn't add test-cert.pem")

statuses, err := updater.findRevokedCertificatesToUpdate(10)
Expand All @@ -377,7 +377,7 @@ func TestNewCertificateTick(t *testing.T) {
reg := satest.CreateWorkingRegistration(t, sa)
parsedCert, err := core.LoadCert("test-cert.pem")
test.AssertNotError(t, err, "Couldn't read test certificate")
_, err = sa.AddCertificate(ctx, parsedCert.Raw, reg.ID, nil)
_, err = sa.AddCertificate(ctx, parsedCert.Raw, reg.ID, nil, nil)
test.AssertNotError(t, err, "Couldn't add test-cert.pem")

prev := fc.Now().Add(-time.Hour)
Expand All @@ -396,7 +396,7 @@ func TestOldOCSPResponsesTick(t *testing.T) {
reg := satest.CreateWorkingRegistration(t, sa)
parsedCert, err := core.LoadCert("test-cert.pem")
test.AssertNotError(t, err, "Couldn't read test certificate")
_, err = sa.AddCertificate(ctx, parsedCert.Raw, reg.ID, nil)
_, err = sa.AddCertificate(ctx, parsedCert.Raw, reg.ID, nil, nil)
test.AssertNotError(t, err, "Couldn't add test-cert.pem")

updater.ocspMinTimeToExpiry = 1 * time.Hour
Expand Down Expand Up @@ -428,7 +428,7 @@ func TestOldOCSPResponsesTickIsExpired(t *testing.T) {
serial := core.SerialToString(parsedCert.SerialNumber)

// Add a new test certificate
_, err = sa.AddCertificate(ctx, parsedCert.Raw, reg.ID, nil)
_, err = sa.AddCertificate(ctx, parsedCert.Raw, reg.ID, nil, nil)
test.AssertNotError(t, err, "Couldn't add test-cert.pem")

// We need to set a fake "ocspLastUpdated" value for the cert we created
Expand Down Expand Up @@ -471,7 +471,7 @@ func TestMissingReceiptsTick(t *testing.T) {
parsedCert, err := core.LoadCert("test-cert.pem")
test.AssertNotError(t, err, "Couldn't read test certificate")
fc.Set(parsedCert.NotBefore.Add(time.Minute))
_, err = sa.AddCertificate(ctx, parsedCert.Raw, reg.ID, nil)
_, err = sa.AddCertificate(ctx, parsedCert.Raw, reg.ID, nil, nil)
test.AssertNotError(t, err, "Couldn't add test-cert.pem")

updater.oldestIssuedSCT = 2 * time.Hour
Expand Down Expand Up @@ -512,7 +512,7 @@ func TestMissingOnlyReceiptsTick(t *testing.T) {
parsedCert, err := core.LoadCert("test-cert.pem")
test.AssertNotError(t, err, "Couldn't read test certificate")
fc.Set(parsedCert.NotBefore.Add(time.Minute))
_, err = sa.AddCertificate(ctx, parsedCert.Raw, reg.ID, nil)
_, err = sa.AddCertificate(ctx, parsedCert.Raw, reg.ID, nil, nil)
test.AssertNotError(t, err, "Couldn't add test-cert.pem")

updater.oldestIssuedSCT = 2 * time.Hour
Expand Down Expand Up @@ -605,7 +605,7 @@ func TestRevokedCertificatesTick(t *testing.T) {
reg := satest.CreateWorkingRegistration(t, sa)
parsedCert, err := core.LoadCert("test-cert.pem")
test.AssertNotError(t, err, "Couldn't read test certificate")
_, err = sa.AddCertificate(ctx, parsedCert.Raw, reg.ID, nil)
_, err = sa.AddCertificate(ctx, parsedCert.Raw, reg.ID, nil, nil)
test.AssertNotError(t, err, "Couldn't add test-cert.pem")

err = sa.MarkCertificateRevoked(ctx, core.SerialToString(parsedCert.SerialNumber), revocation.KeyCompromise)
Expand All @@ -631,7 +631,7 @@ func TestStoreResponseGuard(t *testing.T) {
reg := satest.CreateWorkingRegistration(t, sa)
parsedCert, err := core.LoadCert("test-cert.pem")
test.AssertNotError(t, err, "Couldn't read test certificate")
_, err = sa.AddCertificate(ctx, parsedCert.Raw, reg.ID, nil)
_, err = sa.AddCertificate(ctx, parsedCert.Raw, reg.ID, nil, nil)
test.AssertNotError(t, err, "Couldn't add test-cert.pem")

status, err := sa.GetCertificateStatus(ctx, core.SerialToString(parsedCert.SerialNumber))
Expand Down Expand Up @@ -716,7 +716,7 @@ func TestGetSubmittedReceipts(t *testing.T) {
parsedCert, err := core.LoadCert("test-cert.pem")
test.AssertNotError(t, err, "Couldn't read test certificate")
fc.Set(parsedCert.NotBefore.Add(time.Minute))
_, err = sa.AddCertificate(ctx, parsedCert.Raw, reg.ID, nil)
_, err = sa.AddCertificate(ctx, parsedCert.Raw, reg.ID, nil, nil)
test.AssertNotError(t, err, "Couldn't add test-cert.pem")

// Before adding any SCTs, there should be no receipts or errors for serial 00
Expand Down
41 changes: 29 additions & 12 deletions cmd/orphan-finder/main.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"regexp"
"strconv"
"strings"
"time"

"golang.org/x/net/context"

Expand Down Expand Up @@ -41,11 +42,15 @@ type config struct {
TLS cmd.TLSConfig
SAService *cmd.GRPCClientConfig
Syslog cmd.SyslogConfig
Features map[string]bool
// Backdate specifies how to adjust a certificate's NotBefore date to get back
// to the original issued date. It should match the value used in
// `test/config/ca.json` for the CA "backdate" value.
Backdate cmd.ConfigDuration
Features map[string]bool
}

type certificateStorage interface {
AddCertificate(context.Context, []byte, int64, []byte) (string, error)
AddCertificate(context.Context, []byte, int64, []byte, *time.Time) (string, error)
GetCertificate(ctx context.Context, serial string) (core.Certificate, error)
}

Expand All @@ -55,20 +60,22 @@ var (
errAlreadyExists = fmt.Errorf("Certificate already exists in DB")
)

func checkDER(sai certificateStorage, der []byte) error {
var backdateDuration time.Duration

func checkDER(sai certificateStorage, der []byte) (*x509.Certificate, error) {
ctx := context.Background()
cert, err := x509.ParseCertificate(der)
if err != nil {
return fmt.Errorf("Failed to parse DER: %s", err)
return nil, fmt.Errorf("Failed to parse DER: %s", err)
}
_, err = sai.GetCertificate(ctx, core.SerialToString(cert.SerialNumber))
if err == nil {
return errAlreadyExists
return nil, errAlreadyExists
}
if berrors.Is(err, berrors.NotFound) {
return nil
return cert, nil
}
return fmt.Errorf("Existing certificate lookup failed: %s", err)
return nil, fmt.Errorf("Existing certificate lookup failed: %s", err)
}

func parseLogLine(sa certificateStorage, logger blog.Logger, line string) (found bool, added bool) {
Expand All @@ -86,7 +93,7 @@ func parseLogLine(sa certificateStorage, logger blog.Logger, line string) (found
logger.AuditErr(fmt.Sprintf("Couldn't decode hex: %s, [%s]", err, line))
return true, false
}
err = checkDER(sa, der)
cert, err := checkDER(sa, der)
if err != nil {
logFunc := logger.Err
if err == errAlreadyExists {
Expand All @@ -107,8 +114,13 @@ func parseLogLine(sa certificateStorage, logger blog.Logger, line string) (found
return true, false
}
// OCSP-Updater will do the first response generation for this cert so pass an
// empty OCSP response
_, err = sa.AddCertificate(ctx, der, int64(regID), nil)
// empty OCSP response. We use `cert.NotBefore` as the issued date to avoid
// the SA tagging this certificate with an issued date of the current time
// when we know it was an orphan issued in the past. Because certificates are
// backdated we need to add the backdate duration to find the true issued
// time.
issuedDate := cert.NotBefore.Add(backdateDuration)
_, err = sa.AddCertificate(ctx, der, int64(regID), nil, &issuedDate)
if err != nil {
logger.AuditErr(fmt.Sprintf("Failed to store certificate: %s, [%s]", err, line))
return true, false
Expand All @@ -133,6 +145,8 @@ func setup(configFile string) (blog.Logger, core.StorageAuthority) {
conn, err := bgrpc.ClientSetup(conf.SAService, tlsConfig, clientMetrics)
cmd.FailOnError(err, "Failed to load credentials and create gRPC connection to SA")
sac := bgrpc.NewStorageAuthorityClient(sapb.NewStorageAuthorityClient(conn))

backdateDuration = conf.Backdate.Duration
return logger, sac
}

Expand Down Expand Up @@ -192,9 +206,12 @@ func main() {
}
der, err := ioutil.ReadFile(*derPath)
cmd.FailOnError(err, "Failed to read DER file")
err = checkDER(sa, der)
cert, err := checkDER(sa, der)
cmd.FailOnError(err, "Pre-AddCertificate checks failed")
_, err = sa.AddCertificate(ctx, der, int64(*regID), nil)
// Because certificates are backdated we need to add the backdate duration
// to find the true issued time.
issuedDate := cert.NotBefore.Add(1 * backdateDuration)
_, err = sa.AddCertificate(ctx, der, int64(*regID), nil, &issuedDate)
cmd.FailOnError(err, "Failed to add certificate to database")

default:
Expand Down
Loading

0 comments on commit f8f9a15

Please sign in to comment.