CA: document "no duplicates" enforcement. (#4603)

Also, add belt-and-suspenders checking for serials already existing at
issuance time.
This commit is contained in:
Jacob Hoffman-Andrews 2019-12-19 10:29:39 -08:00 committed by Daniel McCarney
parent 97bd7c53dc
commit d9d3be3d2a
3 changed files with 154 additions and 23 deletions

View File

@ -98,6 +98,7 @@ const (
type certificateStorage interface {
AddCertificate(context.Context, []byte, int64, []byte, *time.Time) (string, error)
GetCertificate(context.Context, string) (core.Certificate, error)
AddPrecertificate(ctx context.Context, req *sapb.AddCertificateRequest) (*corepb.Empty, error)
AddSerial(ctx context.Context, req *sapb.AddSerialRequest) (*corepb.Empty, error)
SerialExists(ctx context.Context, req *sapb.Serial) (*sapb.Exists, error)
@ -571,17 +572,43 @@ func (ca *CertificateAuthorityImpl) IssuePrecertificate(ctx context.Context, iss
}, nil
}
// IssueCertificateForPrecertificate takes a precertificate and a set of SCTs for that precertificate
// and uses the signer to create and sign a certificate from them. The poison extension is removed
// and a SCT list extension is inserted in its place. Except for this and the signature the certificate
// exactly matches the precertificate. After the certificate is signed a OCSP response is generated
// and the response and certificate are stored in the database.
// IssueCertificateForPrecertificate takes a precertificate and a set
// of SCTs for that precertificate and uses the signer to create and
// sign a certificate from them. The poison extension is removed and a
// SCT list extension is inserted in its place. Except for this and the
// signature the certificate exactly matches the precertificate. After
// the certificate is signed a OCSP response is generated and the
// response and certificate are stored in the database.
//
// It's critical not to sign two different final certificates for the same
// precertificate. This can happen, for instance, if the caller provides a
// different set of SCTs on subsequent calls to IssueCertificateForPrecertificate.
// We rely on the RA not to call IssueCertificateForPrecertificate twice for the
// same serial. This is accomplished by the fact that
// IssueCertificateForPrecertificate is only ever called in a straight-through
// RPC path without retries. If there is any error, including a networking
// error, the whole certificate issuance attempt fails and any subsequent
// issuance will use a different serial number.
//
// We also check that the provided serial number does not already exist as a
// final certificate, but this is just a belt-and-suspenders measure, since
// there could be race conditions where two goroutines are issuing for the same
// serial number at the same time.
func (ca *CertificateAuthorityImpl) IssueCertificateForPrecertificate(ctx context.Context, req *caPB.IssueCertificateForPrecertificateRequest) (core.Certificate, error) {
emptyCert := core.Certificate{}
precert, err := x509.ParseCertificate(req.DER)
if err != nil {
return emptyCert, err
}
serialHex := core.SerialToString(precert.SerialNumber)
if _, err = ca.sa.GetCertificate(ctx, serialHex); err == nil {
err = berrors.InternalServerError("issuance of duplicate final certificate requested: %s", serialHex)
ca.log.AuditErr(err.Error())
return emptyCert, err
} else if !berrors.Is(err, berrors.NotFound) {
return emptyCert, fmt.Errorf("error checking for duplicate issuance of %s: %s", serialHex, err)
}
var scts []ct.SignedCertificateTimestamp
for _, sctBytes := range req.SCTs {
var sct ct.SignedCertificateTimestamp
@ -595,7 +622,6 @@ func (ca *CertificateAuthorityImpl) IssueCertificateForPrecertificate(ctx contex
if err != nil {
return emptyCert, err
}
serialHex := core.SerialToString(precert.SerialNumber)
block, _ := pem.Decode(certPEM)
if block == nil || block.Type != "CERTIFICATE" {
err = berrors.InternalServerError("invalid certificate value returned")

View File

@ -192,6 +192,10 @@ func (m *mockSA) SerialExists(ctx context.Context, req *sapb.Serial) (*sapb.Exis
return &sapb.Exists{Exists: &e}, nil
}
func (m *mockSA) GetCertificate(ctx context.Context, serial string) (core.Certificate, error) {
return core.Certificate{}, berrors.NotFoundError("cannae find tha cert")
}
var caKey crypto.Signer
var caCert *x509.Certificate
var ctx = context.Background()
@ -838,6 +842,21 @@ func signatureCountByPurpose(signatureType string, signatureCount *prometheus.Co
return test.CountCounterVec("purpose", signatureType, signatureCount)
}
func makeSCTs() ([][]byte, error) {
sct := ct.SignedCertificateTimestamp{
SCTVersion: 0,
Timestamp: 2020,
Signature: ct.DigitallySigned{
Signature: []byte{0},
},
}
sctBytes, err := cttls.Marshal(sct)
if err != nil {
return nil, err
}
return [][]byte{sctBytes}, err
}
func TestIssueCertificateForPrecertificate(t *testing.T) {
testCtx := setup(t)
sa := &mockSA{}
@ -869,18 +888,15 @@ func TestIssueCertificateForPrecertificate(t *testing.T) {
}
test.Assert(t, poisoned, "returned precert not poisoned")
sct := ct.SignedCertificateTimestamp{
SCTVersion: 0,
Timestamp: 2020,
Signature: ct.DigitallySigned{
Signature: []byte{0},
},
sctBytes, err := makeSCTs()
if err != nil {
t.Fatal(err)
}
sctBytes, err := cttls.Marshal(sct)
test.AssertNotError(t, err, "Failed to marshal SCT")
cert, err := ca.IssueCertificateForPrecertificate(ctx, &caPB.IssueCertificateForPrecertificateRequest{
DER: precert.DER,
SCTs: [][]byte{sctBytes},
SCTs: sctBytes,
RegistrationID: &arbitraryRegID,
OrderID: new(int64),
})
@ -904,7 +920,94 @@ func TestIssueCertificateForPrecertificate(t *testing.T) {
test.Assert(t, list, "returned cert doesn't contain SCT list")
}
// dupeSA returns a non-error to GetCertificate in order to simulate a request
// to issue a final certificate with a duplicate serial.
type dupeSA struct {
mockSA
}
func (m *dupeSA) GetCertificate(ctx context.Context, serial string) (core.Certificate, error) {
return core.Certificate{}, nil
}
// getCertErrorSA always returns an error for GetCertificate
type getCertErrorSA struct {
mockSA
}
func (m *getCertErrorSA) GetCertificate(ctx context.Context, serial string) (core.Certificate, error) {
return core.Certificate{}, fmt.Errorf("i don't like it")
}
func TestIssueCertificateForPrecertificateDuplicateSerial(t *testing.T) {
testCtx := setup(t)
sa := &dupeSA{}
ca, err := NewCertificateAuthorityImpl(
testCtx.caConfig,
sa,
testCtx.pa,
testCtx.fc,
testCtx.stats,
testCtx.issuers,
testCtx.keyPolicy,
testCtx.logger,
nil)
test.AssertNotError(t, err, "Failed to create CA")
sctBytes, err := makeSCTs()
if err != nil {
t.Fatal(err)
}
orderID := int64(0)
issueReq := caPB.IssueCertificateRequest{Csr: CNandSANCSR, RegistrationID: &arbitraryRegID, OrderID: &orderID}
precert, err := ca.IssuePrecertificate(ctx, &issueReq)
test.AssertNotError(t, err, "Failed to issue precert")
_, err = ca.IssueCertificateForPrecertificate(ctx, &caPB.IssueCertificateForPrecertificateRequest{
DER: precert.DER,
SCTs: sctBytes,
RegistrationID: &arbitraryRegID,
OrderID: new(int64),
})
if err == nil {
t.Error("Expected error issuing duplicate serial but got none.")
}
if !strings.Contains(err.Error(), "issuance of duplicate final certificate requested") {
t.Errorf("Wrong type of error issuing duplicate serial. Expected 'issuance of duplicate', got '%s'", err)
}
// Now check what happens if there is an error (e.g. timeout) while checking
// for the duplicate.
errorsa := &getCertErrorSA{}
errorca, err := NewCertificateAuthorityImpl(
testCtx.caConfig,
errorsa,
testCtx.pa,
testCtx.fc,
testCtx.stats,
testCtx.issuers,
testCtx.keyPolicy,
testCtx.logger,
nil)
test.AssertNotError(t, err, "Failed to create CA")
_, err = errorca.IssueCertificateForPrecertificate(ctx, &caPB.IssueCertificateForPrecertificateRequest{
DER: precert.DER,
SCTs: sctBytes,
RegistrationID: &arbitraryRegID,
OrderID: new(int64),
})
if err == nil {
t.Fatal("Expected error issuing duplicate serial but got none.")
}
if !strings.Contains(err.Error(), "error checking for duplicate") {
t.Fatalf("Wrong type of error issuing duplicate serial. Expected 'error checking for duplicate', got '%s'", err)
}
}
type queueSA struct {
mockSA
fail bool
duplicate bool
@ -933,15 +1036,6 @@ func (qsa *queueSA) AddPrecertificate(ctx context.Context, req *sapb.AddCertific
return nil, nil
}
func (qsa *queueSA) AddSerial(ctx context.Context, req *sapb.AddSerialRequest) (*corepb.Empty, error) {
return &corepb.Empty{}, nil
}
func (qsa *queueSA) SerialExists(ctx context.Context, req *sapb.Serial) (*sapb.Exists, error) {
e := true
return &sapb.Exists{Exists: &e}, nil
}
// TestPrecertOrphanQueue tests that IssuePrecertificate writes precertificates
// to the orphan queue if storage fails, and that `integrateOrphan` later
// successfully writes those precertificates to the database. To do this, it

View File

@ -1125,6 +1125,17 @@ func (ra *RegistrationAuthorityImpl) issueCertificate(
// issueCertificateInner handles the common aspects of certificate issuance used by
// both the "classic" NewCertificate endpoint (for ACME v1) and the
// FinalizeOrder endpoint (for ACME v2).
//
// This function is responsible for ensuring that we never try to issue a final
// certificate twice for the same precertificate, because that has the potential
// to create certificates with duplicate serials. For instance, this could
// happen if final certificates were created with different sets of SCTs. This
// function accomplishes that by bailing on issuance if there is any error in
// IssueCertificateForPrecertificate; there are no retries, and serials are
// generated in IssuePrecertificate, so serials with errors are dropped and
// never have final certificates issued for them (because there is a possibility
// that the certificate was actually issued but there was an error returning
// it).
func (ra *RegistrationAuthorityImpl) issueCertificateInner(
ctx context.Context,
req core.CertificateRequest,