Skip to content

Commit

Permalink
Separate issuance.Profile out from issuance.Issuer (letsencrypt#7285)
Browse files Browse the repository at this point in the history 
Remove the Profile field from issuance.Issuer, to reflect the fact that
profiles are in fact independent pieces of configuration which can be
shared across (and are configured independently of) multiple issuers.

Move the IssuerURL, OCSPUrl, and CRLURL fields from issuance.Profile to
issuance.Issuer, since they reflect fundamental attributes of the
issuer, rather than attributes of a particular profile. This also
reflects the location at which those values are configured, in
issuance.IssuerConfig.

All other changes are fallout from the above: adding a Profile argument
to various methods in the issuance and linting packages, adding a
profile field to the caImpl struct, etc. This change paves the way for
two future changes: moving OCSP and CRL creation into the issuance
package, and supporting multiple simultaneous profiles that the CA can
select between.

Part of letsencrypt#7159
Part of letsencrypt#6316
Part of letsencrypt#6966
  • Loading branch information
@aarongable
aarongable authored Feb 7, 2024
1 parent 0e9f5d3 commit af2c1a5
Show file tree
Hide file tree
Showing 12 changed files with 548 additions and 482 deletions.
11 changes: 9 additions & 2 deletions ca/ca.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ type certificateAuthorityImpl struct {
sa sapb.StorageAuthorityCertificateClient
pa core.PolicyAuthority
issuers issuerMaps
profile *issuance.Profile

// This is temporary, and will be used for testing and slow roll-out
// of ECDSA issuance, but will then be removed.
Expand Down Expand Up @@ -103,6 +104,7 @@ func NewCertificateAuthorityImpl(
sa sapb.StorageAuthorityCertificateClient,
pa core.PolicyAuthority,
boulderIssuers []*issuance.Issuer,
certificateProfile *issuance.Profile,
ecdsaAllowList *ECDSAAllowList,
certExpiry time.Duration,
certBackdate time.Duration,
Expand Down Expand Up @@ -134,6 +136,10 @@ func NewCertificateAuthorityImpl(
return nil, errors.New("must have at least one issuer")
}

if certificateProfile == nil {
return nil, errors.New("must have at least one certificate profile")
}

issuers := makeIssuerMaps(boulderIssuers)

lintErrorCount := prometheus.NewCounter(
Expand All @@ -147,6 +153,7 @@ func NewCertificateAuthorityImpl(
sa: sa,
pa: pa,
issuers: issuers,
profile: certificateProfile,
validityPeriod: certExpiry,
backdate: certBackdate,
prefix: serialPrefix,
Expand Down Expand Up @@ -281,7 +288,7 @@ func (ca *certificateAuthorityImpl) IssueCertificateForPrecertificate(ctx contex
ca.log.AuditInfof("Signing cert: serial=[%s] regID=[%d] names=[%s] precert=[%s]",
serialHex, req.RegistrationID, names, hex.EncodeToString(precert.Raw))

_, issuanceToken, err := issuer.Prepare(issuanceReq)
_, issuanceToken, err := issuer.Prepare(ca.profile, issuanceReq)
if err != nil {
ca.log.AuditErrf("Preparing cert failed: serial=[%s] regID=[%d] names=[%s] err=[%v]",
serialHex, req.RegistrationID, names, err)
Expand Down Expand Up @@ -439,7 +446,7 @@ func (ca *certificateAuthorityImpl) issuePrecertificateInner(ctx context.Context
NotAfter: validity.NotAfter,
}

lintCertBytes, issuanceToken, err := issuer.Prepare(req)
lintCertBytes, issuanceToken, err := issuer.Prepare(ca.profile, req)
if err != nil {
ca.log.AuditErrf("Preparing precert failed: serial=[%s] regID=[%d] names=[%s] err=[%v]",
serialHex, issueReq.RegistrationID, strings.Join(csr.DNSNames, ", "), err)
Expand Down
148 changes: 79 additions & 69 deletions ca/ca_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package ca

import (
"context"
"crypto"
"crypto/ecdsa"
"crypto/elliptic"
"crypto/rand"
Expand Down Expand Up @@ -32,7 +31,6 @@ import (
"github.com/letsencrypt/boulder/features"
"github.com/letsencrypt/boulder/goodkey"
"github.com/letsencrypt/boulder/issuance"
"github.com/letsencrypt/boulder/linter"
blog "github.com/letsencrypt/boulder/log"
"github.com/letsencrypt/boulder/metrics"
"github.com/letsencrypt/boulder/must"
Expand Down Expand Up @@ -110,6 +108,7 @@ type testCtx struct {
pa core.PolicyAuthority
ocsp *ocspImpl
crl *crlImpl
profile *issuance.Profile
certExpiry time.Duration
certBackdate time.Duration
serialPrefix int
Expand Down Expand Up @@ -152,30 +151,8 @@ func (m *mockSA) SetCertificateStatusReady(ctx context.Context, req *sapb.Serial
return &emptypb.Empty{}, nil
}

var caKey crypto.Signer
var caCert *issuance.Certificate
var caCert2 *issuance.Certificate
var caLinter *linter.Linter
var caLinter2 *linter.Linter
var ctx = context.Background()

func init() {
var err error
caCert, caKey, err = issuance.LoadIssuer(issuance.IssuerLoc{
File: caKeyFile,
CertFile: caCertFile,
})
if err != nil {
panic(fmt.Sprintf("Unable to load %q and %q: %s", caKeyFile, caCertFile, err))
}
caCert2, err = issuance.LoadCertificate(caCertFile2)
if err != nil {
panic(fmt.Sprintf("Unable to parse %q: %s", caCertFile2, err))
}
caLinter, _ = linter.New(caCert.Certificate, caKey, []string{"w_subject_common_name_included"})
caLinter2, _ = linter.New(caCert2.Certificate, caKey, []string{"w_subject_common_name_included"})
}

func setup(t *testing.T) *testCtx {
features.Reset()
fc := clock.NewFake()
Expand All @@ -186,46 +163,44 @@ func setup(t *testing.T) *testCtx {
err = pa.LoadHostnamePolicyFile("../test/hostname-policy.yaml")
test.AssertNotError(t, err, "Couldn't set hostname policy")

boulderProfile := func(rsa, ecdsa bool) *issuance.Profile {
res, _ := issuance.NewProfile(
issuance.ProfileConfig{
AllowMustStaple: true,
AllowCTPoison: true,
AllowSCTList: true,
AllowCommonName: true,
Policies: []issuance.PolicyConfig{
{OID: "2.23.140.1.2.1"},
},
MaxValidityPeriod: config.Duration{Duration: time.Hour * 8760},
MaxValidityBackdate: config.Duration{Duration: time.Hour},
boulderProfile, err := issuance.NewProfile(
issuance.ProfileConfig{
AllowMustStaple: true,
AllowCTPoison: true,
AllowSCTList: true,
AllowCommonName: true,
Policies: []issuance.PolicyConfig{
{OID: "2.23.140.1.2.1"},
},
issuance.IssuerConfig{
UseForECDSALeaves: ecdsa,
UseForRSALeaves: rsa,
IssuerURL: "http://not-example.com/issuer-url",
OCSPURL: "http://not-example.com/ocsp",
CRLURL: "http://not-example.com/crl",
},
)
return res
}
boulderIssuers := []*issuance.Issuer{
// Must list ECDSA-only issuer first, so it is the default for ECDSA.
{
Cert: caCert2,
Signer: caKey,
Profile: boulderProfile(false, true),
Linter: caLinter2,
Clk: fc,
MaxValidityPeriod: config.Duration{Duration: time.Hour * 8760},
MaxValidityBackdate: config.Duration{Duration: time.Hour},
},
{
Cert: caCert,
Signer: caKey,
Profile: boulderProfile(true, true),
Linter: caLinter,
Clk: fc,
},
}
[]string{"w_subject_common_name_included"},
)
test.AssertNotError(t, err, "Couldn't create test profile")

ecdsaOnlyIssuer, err := issuance.LoadIssuer(issuance.IssuerConfig{
UseForRSALeaves: false,
UseForECDSALeaves: true,
IssuerURL: "http://not-example.com/issuer-url",
OCSPURL: "http://not-example.com/ocsp",
CRLURL: "http://not-example.com/crl",
Location: issuance.IssuerLoc{File: caKeyFile, CertFile: caCertFile2},
}, fc)
test.AssertNotError(t, err, "Couldn't load test issuer")

ecdsaAndRSAIssuer, err := issuance.LoadIssuer(issuance.IssuerConfig{
UseForRSALeaves: true,
UseForECDSALeaves: true,
IssuerURL: "http://not-example.com/issuer-url",
OCSPURL: "http://not-example.com/ocsp",
CRLURL: "http://not-example.com/crl",
Location: issuance.IssuerLoc{File: caKeyFile, CertFile: caCertFile},
}, fc)
test.AssertNotError(t, err, "Couldn't load test issuer")

// Must list ECDSA-only issuer first, so it is the default for ECDSA.
boulderIssuers := []*issuance.Issuer{ecdsaOnlyIssuer, ecdsaAndRSAIssuer}

keyPolicy := goodkey.KeyPolicy{
AllowRSA: true,
Expand Down Expand Up @@ -258,6 +233,7 @@ func setup(t *testing.T) *testCtx {

crl, err := NewCRLImpl(
boulderIssuers,
boulderProfile.Lints,
time.Hour,
"http://c.boulder.test",
100,
Expand All @@ -269,6 +245,7 @@ func setup(t *testing.T) *testCtx {
pa: pa,
ocsp: ocsp,
crl: crl,
profile: boulderProfile,
certExpiry: 8760 * time.Hour,
certBackdate: time.Hour,
serialPrefix: 17,
Expand All @@ -291,6 +268,7 @@ func TestSerialPrefix(t *testing.T) {
nil,
nil,
nil,
nil,
testCtx.certExpiry,
testCtx.certBackdate,
0,
Expand All @@ -308,6 +286,7 @@ func TestSerialPrefix(t *testing.T) {
nil,
nil,
nil,
nil,
testCtx.certExpiry,
testCtx.certBackdate,
128,
Expand Down Expand Up @@ -400,6 +379,7 @@ func issueCertificateSubTestSetup(t *testing.T, e *ECDSAAllowList) (*certificate
sa,
testCtx.pa,
testCtx.boulderIssuers,
testCtx.profile,
e,
testCtx.certExpiry,
testCtx.certBackdate,
Expand Down Expand Up @@ -445,6 +425,7 @@ func TestNoIssuers(t *testing.T) {
sa,
testCtx.pa,
nil, // No issuers
testCtx.profile,
nil,
testCtx.certExpiry,
testCtx.certBackdate,
Expand All @@ -468,6 +449,7 @@ func TestMultipleIssuers(t *testing.T) {
sa,
testCtx.pa,
testCtx.boulderIssuers,
testCtx.profile,
nil,
testCtx.certExpiry,
testCtx.certBackdate,
Expand All @@ -481,23 +463,46 @@ func TestMultipleIssuers(t *testing.T) {
testCtx.fc)
test.AssertNotError(t, err, "Failed to remake CA")

// Test that an RSA CSR gets issuance from the RSA issuer, caCert.
// Test that an RSA CSR gets issuance from the RSA issuer.
issuedCert, err := ca.IssuePrecertificate(ctx, &capb.IssueCertificateRequest{Csr: CNandSANCSR, RegistrationID: arbitraryRegID})
test.AssertNotError(t, err, "Failed to issue certificate")
cert, err := x509.ParseCertificate(issuedCert.DER)
test.AssertNotError(t, err, "Certificate failed to parse")
err = cert.CheckSignatureFrom(caCert2.Certificate)
err = cert.CheckSignatureFrom(testCtx.boulderIssuers[1].Cert.Certificate)
test.AssertNotError(t, err, "Certificate failed signature validation")

// Test that an ECDSA CSR gets issuance from the ECDSA issuer, caCert2.
// Test that an ECDSA CSR gets issuance from the ECDSA issuer.
issuedCert, err = ca.IssuePrecertificate(ctx, &capb.IssueCertificateRequest{Csr: ECDSACSR, RegistrationID: arbitraryRegID})
test.AssertNotError(t, err, "Failed to issue certificate")
cert, err = x509.ParseCertificate(issuedCert.DER)
test.AssertNotError(t, err, "Certificate failed to parse")
err = cert.CheckSignatureFrom(caCert2.Certificate)
err = cert.CheckSignatureFrom(testCtx.boulderIssuers[0].Cert.Certificate)
test.AssertNotError(t, err, "Certificate failed signature validation")
}

func TestNoProfile(t *testing.T) {
testCtx := setup(t)
sa := &mockSA{}
_, err := NewCertificateAuthorityImpl(
sa,
testCtx.pa,
testCtx.boulderIssuers,
nil, // no profile
nil,
testCtx.certExpiry,
testCtx.certBackdate,
testCtx.serialPrefix,
testCtx.maxNames,
testCtx.keyPolicy,
testCtx.logger,
testCtx.stats,
testCtx.signatureCount,
testCtx.signErrorCount,
testCtx.fc)
test.AssertError(t, err, "No profile found during CA construction.")
test.AssertEquals(t, err.Error(), "must have at least one certificate profile")
}

func TestECDSAAllowList(t *testing.T) {
req := &capb.IssueCertificateRequest{Csr: ECDSACSR, RegistrationID: arbitraryRegID}

Expand All @@ -508,7 +513,7 @@ func TestECDSAAllowList(t *testing.T) {
test.AssertNotError(t, err, "Failed to issue certificate")
cert, err := x509.ParseCertificate(result.DER)
test.AssertNotError(t, err, "Certificate failed to parse")
test.AssertByteEquals(t, cert.RawIssuer, caCert2.RawSubject)
test.AssertByteEquals(t, cert.RawIssuer, ca.issuers.byAlg[x509.ECDSA].Cert.RawSubject)

// With allowlist not containing arbitraryRegID, issuance should fall back to RSA issuer.
regIDMap = makeRegIDsMap([]int64{2002})
Expand All @@ -517,7 +522,7 @@ func TestECDSAAllowList(t *testing.T) {
test.AssertNotError(t, err, "Failed to issue certificate")
cert, err = x509.ParseCertificate(result.DER)
test.AssertNotError(t, err, "Certificate failed to parse")
test.AssertByteEquals(t, cert.RawIssuer, caCert.RawSubject)
test.AssertByteEquals(t, cert.RawIssuer, ca.issuers.byAlg[x509.RSA].Cert.RawSubject)

// With empty allowlist but ECDSAForAll enabled, issuance should come from ECDSA issuer.
ca, _ = issueCertificateSubTestSetup(t, nil)
Expand All @@ -527,7 +532,7 @@ func TestECDSAAllowList(t *testing.T) {
test.AssertNotError(t, err, "Failed to issue certificate")
cert, err = x509.ParseCertificate(result.DER)
test.AssertNotError(t, err, "Certificate failed to parse")
test.AssertByteEquals(t, cert.RawIssuer, caCert2.RawSubject)
test.AssertByteEquals(t, cert.RawIssuer, ca.issuers.byAlg[x509.ECDSA].Cert.RawSubject)
}

func TestInvalidCSRs(t *testing.T) {
Expand Down Expand Up @@ -589,6 +594,7 @@ func TestInvalidCSRs(t *testing.T) {
sa,
testCtx.pa,
testCtx.boulderIssuers,
testCtx.profile,
nil,
testCtx.certExpiry,
testCtx.certBackdate,
Expand Down Expand Up @@ -625,6 +631,7 @@ func TestRejectValidityTooLong(t *testing.T) {
sa,
testCtx.pa,
testCtx.boulderIssuers,
testCtx.profile,
nil,
testCtx.certExpiry,
testCtx.certBackdate,
Expand Down Expand Up @@ -725,6 +732,7 @@ func TestIssueCertificateForPrecertificate(t *testing.T) {
sa,
testCtx.pa,
testCtx.boulderIssuers,
testCtx.profile,
nil,
testCtx.certExpiry,
testCtx.certBackdate,
Expand Down Expand Up @@ -830,6 +838,7 @@ func TestIssueCertificateForPrecertificateDuplicateSerial(t *testing.T) {
sa,
testCtx.pa,
testCtx.boulderIssuers,
testCtx.profile,
nil,
testCtx.certExpiry,
testCtx.certBackdate,
Expand Down Expand Up @@ -871,6 +880,7 @@ func TestIssueCertificateForPrecertificateDuplicateSerial(t *testing.T) {
errorsa,
testCtx.pa,
testCtx.boulderIssuers,
testCtx.profile,
nil,
testCtx.certExpiry,
testCtx.certBackdate,
Expand Down
Loading

0 comments on commit af2c1a5

Please sign in to comment.