letsencrypt
/
boulder
mirror of https://github.com/letsencrypt/boulder.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Return proto from ca.IssueCertificateFromPrecertificate (#4982) 

This is the only method on the ca which uses a non-proto
type as its request or response value. Changing this to
use a proto removes the last logic from the wrappers,
allowing them to be removed in a future CL. It also makes
the interface more uniform and easier to reason about.

Issue: #4940
This commit is contained in:
Aaron Gable 2020-07-23 18:39:10 -07:00 committed by GitHub
parent 62eae60711
commit ffdae2d338
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 55 additions and 53 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

42
ca/ca.go
View File

@ -595,52 +595,65 @@ func (ca *CertificateAuthorityImpl) IssuePrecertificate(ctx context.Context, iss
// 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{}
func (ca *CertificateAuthorityImpl) IssueCertificateForPrecertificate(ctx context.Context, req *capb.IssueCertificateForPrecertificateRequest) (*corepb.Certificate, error) {
// issueReq.orderID may be zero, for ACMEv1 requests.
if core.IsAnyNilOrZero(req, req.DER, req.SCTs, req.RegistrationID) {
return emptyCert, berrors.InternalServerError("Incomplete cert for precertificate request")
return nil, berrors.InternalServerError("Incomplete cert for precertificate request")
}
precert, err := x509.ParseCertificate(req.DER)
if err != nil {
return emptyCert, err
return nil, 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
return nil, err
} else if !berrors.Is(err, berrors.NotFound) {
return emptyCert, fmt.Errorf("error checking for duplicate issuance of %s: %s", serialHex, err)
return nil, 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
_, err = cttls.Unmarshal(sctBytes, &sct)
if err != nil {
return emptyCert, err
return nil, err
}
scts = append(scts, sct)
}
certPEM, err := ca.defaultIssuer.eeSigner.SignFromPrecert(precert, scts)
if err != nil {
return emptyCert, err
return nil, err
}
ca.signatureCount.WithLabelValues(string(certType)).Inc()
block, _ := pem.Decode(certPEM)
if block == nil || block.Type != "CERTIFICATE" {
err = berrors.InternalServerError("invalid certificate value returned")
ca.log.AuditErrf("PEM decode error, aborting: serial=[%s] pem=[%s] err=[%v]", serialHex, certPEM, err)
return emptyCert, err
return nil, err
}
certDER := block.Bytes
ca.log.AuditInfof("Signing success: serial=[%s] names=[%s] certificate=[%s]",
serialHex, strings.Join(precert.DNSNames, ", "), hex.EncodeToString(req.DER),
hex.EncodeToString(certDER))
return ca.storeCertificate(ctx, *req.RegistrationID, *req.OrderID, precert.SerialNumber, certDER)
err = ca.storeCertificate(ctx, *req.RegistrationID, *req.OrderID, precert.SerialNumber, certDER)
if err != nil {
return nil, err
}
serialString := core.SerialToString(precert.SerialNumber)
digest := core.Fingerprint256(certDER)
issued := precert.NotBefore.UnixNano()
expires := precert.NotAfter.UnixNano()
return &corepb.Certificate{
RegistrationID: req.RegistrationID,
Serial: &serialString,
Der: certDER,
Digest: &digest,
Issued: &issued,
Expires: &expires,
}, nil
}
type validity struct {
@ -794,7 +807,7 @@ func (ca *CertificateAuthorityImpl) storeCertificate(
regID int64,
orderID int64,
serialBigInt *big.Int,
certDER []byte) (core.Certificate, error) {
certDER []byte) error {
var err error
now := ca.clk.Now()
_, err = ca.sa.AddCertificate(ctx, certDER, regID, nil, &now)
@ -811,10 +824,9 @@ func (ca *CertificateAuthorityImpl) storeCertificate(
RegID: regID,
})
}
return core.Certificate{}, err
return err
}
return core.Certificate{DER: certDER}, nil
return nil
}
type orphanedCert struct {

6
ca/ca_test.go
View File

@ -870,7 +870,7 @@ func TestIssueCertificateForPrecertificate(t *testing.T) {
OrderID: new(int64),
})
test.AssertNotError(t, err, "Failed to issue cert from precert")
parsedCert, err := x509.ParseCertificate(cert.DER)
parsedCert, err := x509.ParseCertificate(cert.Der)
test.AssertNotError(t, err, "Failed to parse cert")
// Check for SCT list extension
@ -1111,7 +1111,7 @@ func TestOrphanQueue(t *testing.T) {
}
certDER, err := x509.CreateCertificate(rand.Reader, tmpl, tmpl, k.Public(), k)
test.AssertNotError(t, err, "Failed to generate test cert")
_, err = ca.storeCertificate(
err = ca.storeCertificate(
context.Background(),
1,
1,
@ -1152,7 +1152,7 @@ func TestOrphanQueue(t *testing.T) {
// add cert to queue, and recreate queue to make sure it still has the cert
qsa.fail = true
qsa.duplicate = false
_, err = ca.storeCertificate(
err = ca.storeCertificate(
context.Background(),
1,
1,

2
core/interfaces.go
View File

@ -97,7 +97,7 @@ type CertificateAuthority interface {
IssuePrecertificate(ctx context.Context, issueReq *capb.IssueCertificateRequest) (*capb.IssuePrecertificateResponse, error)
// [RegistrationAuthority]
IssueCertificateForPrecertificate(ctx context.Context, req *capb.IssueCertificateForPrecertificateRequest) (Certificate, error)
IssueCertificateForPrecertificate(ctx context.Context, req *capb.IssueCertificateForPrecertificateRequest) (*corepb.Certificate, error)
GenerateOCSP(ctx context.Context, ocspReq *capb.GenerateOCSPRequest) (*capb.OCSPResponse, error)
}

14
grpc/ca-wrappers.go
View File

@ -27,12 +27,8 @@ func (cac CertificateAuthorityClientWrapper) IssuePrecertificate(ctx context.Con
return cac.inner.IssuePrecertificate(ctx, issueReq)
}
func (cac CertificateAuthorityClientWrapper) IssueCertificateForPrecertificate(ctx context.Context, req *capb.IssueCertificateForPrecertificateRequest) (core.Certificate, error) {
res, err := cac.inner.IssueCertificateForPrecertificate(ctx, req)
if err != nil {
return core.Certificate{}, err
}
return PBToCert(res)
func (cac CertificateAuthorityClientWrapper) IssueCertificateForPrecertificate(ctx context.Context, req *capb.IssueCertificateForPrecertificateRequest) (*corepb.Certificate, error) {
return cac.inner.IssueCertificateForPrecertificate(ctx, req)
}
func (cac CertificateAuthorityClientWrapper) GenerateOCSP(ctx context.Context, req *capb.GenerateOCSPRequest) (*capb.OCSPResponse, error) {
@ -65,11 +61,7 @@ func (cas *CertificateAuthorityServerWrapper) IssuePrecertificate(ctx context.Co
}
func (cas *CertificateAuthorityServerWrapper) IssueCertificateForPrecertificate(ctx context.Context, req *capb.IssueCertificateForPrecertificateRequest) (*corepb.Certificate, error) {
cert, err := cas.inner.IssueCertificateForPrecertificate(ctx, req)
if err != nil {
return nil, err
}
return CertToPB(cert), nil
return cas.inner.IssueCertificateForPrecertificate(ctx, req)
}
func (cas *CertificateAuthorityServerWrapper) GenerateOCSP(ctx context.Context, req *capb.GenerateOCSPRequest) (*capb.OCSPResponse, error) {

30
mocks/ca.go
View File

@ -7,7 +7,7 @@ import (
"fmt"
capb "github.com/letsencrypt/boulder/ca/proto"
"github.com/letsencrypt/boulder/core"
corepb "github.com/letsencrypt/boulder/core/proto"
"github.com/letsencrypt/boulder/revocation"
)
@ -17,21 +17,6 @@ type MockCA struct {
PEM []byte
}
// IssueCertificate is a mock
func (ca *MockCA) IssueCertificate(ctx context.Context, _ *capb.IssueCertificateRequest) (core.Certificate, error) {
if ca.PEM == nil {
return core.Certificate{}, fmt.Errorf("MockCA's PEM field must be set before calling IssueCertificate")
}
block, _ := pem.Decode(ca.PEM)
cert, err := x509.ParseCertificate(block.Bytes)
if err != nil {
return core.Certificate{}, err
}
return core.Certificate{
DER: cert.Raw,
}, nil
}
// IssuePrecertificate is a mock
func (ca *MockCA) IssuePrecertificate(ctx context.Context, _ *capb.IssueCertificateRequest) (*capb.IssuePrecertificateResponse, error) {
if ca.PEM == nil {
@ -48,8 +33,17 @@ func (ca *MockCA) IssuePrecertificate(ctx context.Context, _ *capb.IssueCertific
}
// IssueCertificateForPrecertificate is a mock
func (ca *MockCA) IssueCertificateForPrecertificate(ctx context.Context, req *capb.IssueCertificateForPrecertificateRequest) (core.Certificate, error) {
return core.Certificate{DER: req.DER}, nil
func (ca *MockCA) IssueCertificateForPrecertificate(ctx context.Context, req *capb.IssueCertificateForPrecertificateRequest) (*corepb.Certificate, error) {
var mockInt = int64(1)
var mockString = "mock"
return &corepb.Certificate{
Der: req.DER,
RegistrationID: &mockInt,
Serial: &mockString,
Digest: &mockString,
Issued: &mockInt,
Expires: &mockInt,
}, nil
}
// GenerateOCSP is a mock

10
ra/ra.go
View File

@ -1225,7 +1225,7 @@ func (ra *RegistrationAuthorityImpl) issueCertificateInner(
return emptyCert, wrapError(err, "issuing certificate for precertificate")
}
parsedCertificate, err := x509.ParseCertificate([]byte(cert.DER))
parsedCertificate, err := x509.ParseCertificate([]byte(cert.Der))
if err != nil {
// berrors.InternalServerError because the certificate from the CA should be
// parseable.
@ -1233,7 +1233,7 @@ func (ra *RegistrationAuthorityImpl) issueCertificateInner(
}
// Asynchronously submit the final certificate to any configured logs
go ra.ctpolicy.SubmitFinalCert(cert.DER, parsedCertificate.NotAfter)
go ra.ctpolicy.SubmitFinalCert(cert.Der, parsedCertificate.NotAfter)
err = ra.MatchesCSR(parsedCertificate, csr)
if err != nil {
@ -1246,7 +1246,11 @@ func (ra *RegistrationAuthorityImpl) issueCertificateInner(
logEvent.NotAfter = parsedCertificate.NotAfter
ra.newCertCounter.Inc()
return cert, nil
res, err := bgrpc.PBToCert(cert)
if err != nil {
return emptyCert, nil
}
return res, nil
}
func (ra *RegistrationAuthorityImpl) getSCTs(ctx context.Context, cert []byte, expiration time.Time) (core.SCTDERs, error) {

4
ra/ra_test.go
View File

@ -3524,8 +3524,8 @@ func (ca *mockCAFailCertForPrecert) IssuePrecertificate(_ context.Context, _ *ca
func (ca *mockCAFailCertForPrecert) IssueCertificateForPrecertificate(
_ context.Context,
_ *capb.IssueCertificateForPrecertificateRequest) (core.Certificate, error) {
return core.Certificate{}, ca.err
_ *capb.IssueCertificateForPrecertificateRequest) (*corepb.Certificate, error) {
return &corepb.Certificate{}, ca.err
}
// TestIssueCertificateInnerErrs tests that errors from the CA caught during