Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RA: Propagate profile name and hash from SA to CA #7367

Merged
merged 4 commits into from
Mar 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 37 additions & 25 deletions core/proto/core.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion core/proto/core.proto
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ message Authorization {
}

message Order {
// Next unused field number: 14
// Next unused field number: 15
int64 id = 1;
int64 registrationID = 2;
reserved 3; // Previously expiresNS
Expand All @@ -115,6 +115,7 @@ message Order {
reserved 10; // Previously createdNS
google.protobuf.Timestamp created = 13;
repeated int64 v2Authorizations = 11;
string certificateProfileName = 14;
}

message CRLEntry {
Expand Down
12 changes: 8 additions & 4 deletions mocks/ca.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,17 @@ package mocks

import (
"context"
"crypto/sha256"
"crypto/x509"
"encoding/pem"
"fmt"
"time"

capb "github.com/letsencrypt/boulder/ca/proto"
corepb "github.com/letsencrypt/boulder/core/proto"
"google.golang.org/grpc"
"google.golang.org/protobuf/types/known/timestamppb"

capb "github.com/letsencrypt/boulder/ca/proto"
corepb "github.com/letsencrypt/boulder/core/proto"
)

// MockCA is a mock of a CA that always returns the cert from PEM in response to
Expand All @@ -20,7 +22,7 @@ type MockCA struct {
}

// IssuePrecertificate is a mock
func (ca *MockCA) IssuePrecertificate(ctx context.Context, _ *capb.IssueCertificateRequest, _ ...grpc.CallOption) (*capb.IssuePrecertificateResponse, error) {
func (ca *MockCA) IssuePrecertificate(ctx context.Context, req *capb.IssueCertificateRequest, _ ...grpc.CallOption) (*capb.IssuePrecertificateResponse, error) {
if ca.PEM == nil {
return nil, fmt.Errorf("MockCA's PEM field must be set before calling IssueCertificate")
}
Expand All @@ -29,8 +31,10 @@ func (ca *MockCA) IssuePrecertificate(ctx context.Context, _ *capb.IssueCertific
if err != nil {
return nil, err
}
profHash := sha256.Sum256([]byte(req.CertProfileName))
return &capb.IssuePrecertificateResponse{
DER: cert.Raw,
DER: cert.Raw,
CertProfileHash: profHash[:8],
jsha marked this conversation as resolved.
Show resolved Hide resolved
}, nil
}

Expand Down
19 changes: 11 additions & 8 deletions ra/ra.go
Original file line number Diff line number Diff line change
Expand Up @@ -1219,7 +1219,7 @@ func (ra *RegistrationAuthorityImpl) issueCertificateOuter(

// Step 3: Issue the Certificate
cert, err := ra.issueCertificateInner(
ctx, csr, accountID(order.RegistrationID), orderID(order.Id))
ctx, csr, 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 @@ -1276,6 +1276,7 @@ func (ra *RegistrationAuthorityImpl) issueCertificateOuter(
func (ra *RegistrationAuthorityImpl) issueCertificateInner(
ctx context.Context,
csr *x509.CertificateRequest,
profileName string,
acctID accountID,
oID orderID) (*x509.Certificate, error) {
if features.Get().AsyncFinalize {
Expand All @@ -1297,9 +1298,10 @@ func (ra *RegistrationAuthorityImpl) issueCertificateInner(
}

issueReq := &capb.IssueCertificateRequest{
Csr: csr.Raw,
RegistrationID: int64(acctID),
OrderID: int64(oID),
Csr: csr.Raw,
RegistrationID: int64(acctID),
OrderID: int64(oID),
CertProfileName: profileName,
}
precert, err := ra.CA.IssuePrecertificate(ctx, issueReq)
if err != nil {
Expand All @@ -1317,10 +1319,11 @@ func (ra *RegistrationAuthorityImpl) issueCertificateInner(
}

cert, err := ra.CA.IssueCertificateForPrecertificate(ctx, &capb.IssueCertificateForPrecertificateRequest{
DER: precert.DER,
SCTs: scts,
RegistrationID: int64(acctID),
OrderID: int64(oID),
DER: precert.DER,
SCTs: scts,
RegistrationID: int64(acctID),
OrderID: int64(oID),
CertProfileHash: precert.CertProfileHash,
})
if err != nil {
return nil, wrapError(err, "issuing certificate for precertificate")
Expand Down
56 changes: 55 additions & 1 deletion ra/ra_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3559,7 +3559,7 @@ func TestIssueCertificateInnerErrs(t *testing.T) {
// Mock the CA
ra.CA = tc.Mock
// Attempt issuance
_, err = ra.issueCertificateInner(ctx, csrOb, accountID(Registration.Id), orderID(order.Id))
_, err = ra.issueCertificateInner(ctx, csrOb, 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 All @@ -3578,6 +3578,60 @@ func TestIssueCertificateInnerErrs(t *testing.T) {
}
}

type MockCARecordingProfile struct {
inner *mocks.MockCA
profileName string
profileHash []byte
}

func (ca *MockCARecordingProfile) IssuePrecertificate(ctx context.Context, req *capb.IssueCertificateRequest, _ ...grpc.CallOption) (*capb.IssuePrecertificateResponse, error) {
ca.profileName = req.CertProfileName
return ca.inner.IssuePrecertificate(ctx, req)
}

func (ca *MockCARecordingProfile) IssueCertificateForPrecertificate(ctx context.Context, req *capb.IssueCertificateForPrecertificateRequest, _ ...grpc.CallOption) (*corepb.Certificate, error) {
ca.profileHash = req.CertProfileHash
return ca.inner.IssueCertificateForPrecertificate(ctx, req)
}

func TestIssueCertificateInnerWithProfile(t *testing.T) {
_, _, ra, fc, cleanup := initAuthorities(t)
defer cleanup()

// Generate a reasonable-looking CSR and cert to pass the matchesCSR check.
testKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
test.AssertNotError(t, err, "generating test key")
csrDER, err := x509.CreateCertificateRequest(rand.Reader, &x509.CertificateRequest{DNSNames: []string{"example.com"}}, testKey)
test.AssertNotError(t, err, "creating test csr")
csr, err := x509.ParseCertificateRequest(csrDER)
test.AssertNotError(t, err, "parsing test csr")
certDER, err := x509.CreateCertificate(rand.Reader, &x509.Certificate{
SerialNumber: big.NewInt(1),
DNSNames: []string{"example.com"},
NotBefore: fc.Now(),
BasicConstraintsValid: true,
ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth, x509.ExtKeyUsageClientAuth},
}, &x509.Certificate{}, testKey.Public(), testKey)
test.AssertNotError(t, err, "creating test cert")
certPEM := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: certDER})

// Use a mock CA that will record the profile name and profile hash included
// in the RA's request messages. Populate it with the cert generated above.
mockCA := MockCARecordingProfile{inner: &mocks.MockCA{PEM: certPEM}}
ra.CA = &mockCA

// The basic mocks.StorageAuthority always succeeds on FinalizeOrder, which is
// the only SA call that issueCertificateInner makes.
ra.SA = &mocks.StorageAuthority{}

// Call issueCertificateInner with the CSR generated above and the profile
// name "default", which will cause the mockCA to return a specific hash.
_, err = ra.issueCertificateInner(context.Background(), csr, "default", 1, 1)
test.AssertNotError(t, err, "issuing cert with profile name")
test.AssertEquals(t, mockCA.profileName, "default")
test.AssertByteEquals(t, mockCA.profileHash, []byte{0x37, 0xa8, 0xee, 0xc1, 0xce, 0x19, 0x68, 0x7d})
}

func TestNewOrderMaxNames(t *testing.T) {
_, _, ra, _, cleanUp := initAuthorities(t)
defer cleanUp()
Expand Down