From 68c393b081cbf84e7df4d53f78154df21bc2d226 Mon Sep 17 00:00:00 2001 From: Aaron Gable Date: Mon, 1 Feb 2021 09:11:38 -0800 Subject: [PATCH] CA: Create ECDSA issuance allowlist (#5258) Currently, the CA is configured with a set of `internalIssuer`s, and a mapping of public key algorithms (e.g. `x509.RSA`) to which internalIssuer to use. In operation today, we use the same issuer for all kinds of public key algorithms. In the future, we will use different issuers for different algorithms (in particular, we will use R3 to issue for RSA keys, and E1 to issue for ECDSA keys). But we want to roll that out slowly, continuing to use our RSA issuer to issue for all types of public keys, except for ECDSA keys which are presented by a specific set of allowed accounts. This change adds a new config field to the CA, which lets us specify a small list of registration IDs which are allowed to have issuance from our ECDSA issuer. If the config list is empty, then all accounts are allowed. The CA checks to see if the key being issued for is ECDSA: if it is, it then checks to make sure that the associated registration ID is in the allowlist. If the account is not allowed, it then overrides the issuance algorithm to use RSA instead, mimicking our old behavior. It also adds a new feature flag, which can be enabled to skip the allowlist entirely (effectively allowing all registered accounts). This feature flag will be enabled when we're done with our testing and confident in our ECDSA issuance. Fixes #5259 --- ca/ca.go | 24 ++++++-- ca/ca_test.go | 104 ++++++++++++++++++++++++++------- cmd/boulder-ca/main.go | 10 ++++ features/featureflag_string.go | 5 +- features/features.go | 4 ++ 5 files changed, 121 insertions(+), 26 deletions(-) diff --git a/ca/ca.go b/ca/ca.go index 02f5af046..080e24629 100644 --- a/ca/ca.go +++ b/ca/ca.go @@ -130,6 +130,7 @@ type CertificateAuthorityImpl struct { sa certificateStorage pa core.PolicyAuthority issuers issuerMaps + ecdsaAllowedRegIDs map[int64]bool cfsslRSAProfile string cfsslECDSAProfile string prefix int // Prepended to the serial number @@ -186,10 +187,11 @@ func makeInternalIssuers(issuers []*issuance.Issuer, lifespanOCSP time.Duration) boulderIssuer: issuer, } for _, alg := range issuer.Algs() { - if issuersByAlg[alg] != nil { - return issuerMaps{}, fmt.Errorf("Multiple issuer certs for %s are not allowed", alg) + // TODO(#5259): Enforce that there is only one issuer for each algorithm, + // instead of taking the first issuer for each algorithm type. + if issuersByAlg[alg] == nil { + issuersByAlg[alg] = ii } - issuersByAlg[alg] = ii } if issuersByName[issuer.Name()] != nil { return issuerMaps{}, errors.New("Multiple issuer certs with the same CommonName are not supported") @@ -257,6 +259,7 @@ func NewCertificateAuthorityImpl( cfsslECDSAProfile string, cfsslIssuers []Issuer, boulderIssuers []*issuance.Issuer, + ecdsaAllowedRegIDs []int64, certExpiry time.Duration, certBackdate time.Duration, serialPrefix int, @@ -322,6 +325,11 @@ func NewCertificateAuthorityImpl( } } + ecdsaAllowedRegIDsMap := make(map[int64]bool, len(ecdsaAllowedRegIDs)) + for _, regID := range ecdsaAllowedRegIDs { + ecdsaAllowedRegIDsMap[regID] = true + } + csrExtensionCount := prometheus.NewCounterVec( prometheus.CounterOpts{ Name: "csr_extensions", @@ -371,6 +379,7 @@ func NewCertificateAuthorityImpl( issuers: issuers, cfsslRSAProfile: cfsslRSAProfile, cfsslECDSAProfile: cfsslECDSAProfile, + ecdsaAllowedRegIDs: ecdsaAllowedRegIDsMap, validityPeriod: certExpiry, backdate: certBackdate, prefix: serialPrefix, @@ -776,7 +785,14 @@ func (ca *CertificateAuthorityImpl) issuePrecertificateInner(ctx context.Context var issuer *internalIssuer var ok bool if issueReq.IssuerNameID == 0 { - issuer, ok = ca.issuers.byAlg[csr.PublicKeyAlgorithm] + // Use the issuer which corresponds to the algorithm of the public key + // contained in the CSR, unless we have an allowlist of registration IDs + // for ECDSA, in which case switch all not-allowed accounts to RSA issuance. + alg := csr.PublicKeyAlgorithm + if alg == x509.ECDSA && !features.Enabled(features.ECDSAForAll) && !ca.ecdsaAllowedRegIDs[issueReq.RegistrationID] { + alg = x509.RSA + } + issuer, ok = ca.issuers.byAlg[alg] if !ok { return nil, nil, berrors.InternalServerError("no issuer found for public key algorithm %s", csr.PublicKeyAlgorithm) } diff --git a/ca/ca_test.go b/ca/ca_test.go index 01921b354..96f495f7e 100644 --- a/ca/ca_test.go +++ b/ca/ca_test.go @@ -122,6 +122,7 @@ const rsaProfileName = "rsaEE" const ecdsaProfileName = "ecdsaEE" const caKeyFile = "../test/test-ca.key" const caCertFile = "../test/test-ca.pem" +const caCertFile2 = "../test/test-ca2.pem" func mustRead(path string) []byte { b, err := ioutil.ReadFile(path) @@ -172,6 +173,7 @@ func (m *mockSA) GetCertificate(ctx context.Context, serial string) (core.Certif var caKey crypto.Signer var caCert *issuance.Certificate +var caCert2 *issuance.Certificate var ctx = context.Background() func init() { @@ -184,6 +186,10 @@ func init() { if err != nil { panic(fmt.Sprintf("Unable to parse %s: %s", caCertFile, err)) } + caCert2, err = issuance.LoadCertificate(caCertFile2) + if err != nil { + panic(fmt.Sprintf("Unable to parse %s: %s", caCertFile2, err)) + } } func setup(t *testing.T) *testCtx { @@ -254,32 +260,43 @@ func setup(t *testing.T) *testCtx { } cfsslIssuers := []Issuer{{caKey, caCert}} - boulderProfile, _ := issuance.NewProfile( - issuance.ProfileConfig{ - AllowMustStaple: true, - AllowCTPoison: true, - AllowSCTList: true, - AllowCommonName: true, - Policies: []issuance.PolicyInformation{ - {OID: "2.23.140.1.2.1"}, + boulderProfile := func(rsa, ecdsa bool) *issuance.Profile { + res, _ := issuance.NewProfile( + issuance.ProfileConfig{ + AllowMustStaple: true, + AllowCTPoison: true, + AllowSCTList: true, + AllowCommonName: true, + Policies: []issuance.PolicyInformation{ + {OID: "2.23.140.1.2.1"}, + }, + MaxValidityPeriod: cmd.ConfigDuration{Duration: time.Hour * 8760}, + MaxValidityBackdate: cmd.ConfigDuration{Duration: time.Hour}, }, - MaxValidityPeriod: cmd.ConfigDuration{Duration: time.Hour * 8760}, - MaxValidityBackdate: cmd.ConfigDuration{Duration: time.Hour}, - }, - issuance.IssuerConfig{ - UseForECDSALeaves: true, - UseForRSALeaves: true, - IssuerURL: "http://not-example.com/issuer-url", - OCSPURL: "http://not-example.com/ocsp", - CRLURL: "http://not-example.com/crl", - }, - ) + 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 + } boulderLinter, _ := lint.NewLinter(caKey, nil) 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: boulderLinter, + Clk: fc, + }, { Cert: caCert, Signer: caKey, - Profile: boulderProfile, + Profile: boulderProfile(true, true), Linter: boulderLinter, Clk: fc, }, @@ -321,6 +338,7 @@ func TestFailNoSerialPrefix(t *testing.T) { testCtx.cfsslECDSAProfile, testCtx.cfsslIssuers, nil, + nil, testCtx.certExpiry, testCtx.certBackdate, 0, @@ -433,6 +451,7 @@ func issueCertificateSubTestSetup(t *testing.T, boulderIssuer bool) (*Certificat testCtx.cfsslECDSAProfile, cfsslIssuers, boulderIssuers, + nil, testCtx.certExpiry, testCtx.certBackdate, testCtx.serialPrefix, @@ -497,6 +516,7 @@ func TestMultipleIssuers(t *testing.T) { testCtx.cfsslECDSAProfile, newIssuers, nil, + nil, testCtx.certExpiry, testCtx.certBackdate, testCtx.serialPrefix, @@ -521,6 +541,38 @@ func TestMultipleIssuers(t *testing.T) { test.AssertNotError(t, err, "Certificate failed signature validation") } +func TestECDSAAllowList(t *testing.T) { + req := &capb.IssueCertificateRequest{Csr: ECDSACSR, RegistrationID: arbitraryRegID} + + // With allowlist containing arbitraryRegID, issuance should come from ECDSA issuer. + ca, _ := issueCertificateSubTestSetup(t, true) + ca.ecdsaAllowedRegIDs[arbitraryRegID] = true + result, err := ca.IssuePrecertificate(ctx, req) + 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) + + // With allowlist not containing arbitraryRegID, issuance should fall back to RSA issuer. + ca, _ = issueCertificateSubTestSetup(t, true) + ca.ecdsaAllowedRegIDs[2002] = true + result, err = ca.IssuePrecertificate(ctx, req) + 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) + + // With empty allowlist but ECDSAForAll enabled, issuance should come from ECDSA issuer. + ca, _ = issueCertificateSubTestSetup(t, true) + _ = features.Set(map[string]bool{"ECDSAForAll": true}) + defer features.Reset() + result, err = ca.IssuePrecertificate(ctx, req) + 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) +} + func TestOCSP(t *testing.T) { testCtx := setup(t) sa := &mockSA{} @@ -532,6 +584,7 @@ func TestOCSP(t *testing.T) { testCtx.cfsslECDSAProfile, testCtx.cfsslIssuers, nil, + nil, testCtx.certExpiry, testCtx.certBackdate, testCtx.serialPrefix, @@ -593,6 +646,7 @@ func TestOCSP(t *testing.T) { testCtx.cfsslECDSAProfile, newIssuers, nil, + nil, testCtx.certExpiry, testCtx.certBackdate, testCtx.serialPrefix, @@ -700,6 +754,7 @@ func TestInvalidCSRs(t *testing.T) { testCtx.cfsslECDSAProfile, testCtx.cfsslIssuers, nil, + nil, testCtx.certExpiry, testCtx.certBackdate, testCtx.serialPrefix, @@ -741,6 +796,7 @@ func TestRejectValidityTooLong(t *testing.T) { testCtx.cfsslECDSAProfile, testCtx.cfsslIssuers, nil, + nil, testCtx.certExpiry, testCtx.certBackdate, testCtx.serialPrefix, @@ -802,6 +858,7 @@ func TestSingleAIAEnforcement(t *testing.T) { ecdsaProfileName, nil, nil, + nil, 8760*time.Hour, time.Hour, 1, @@ -920,6 +977,7 @@ func TestIssueCertificateForPrecertificate(t *testing.T) { testCtx.cfsslECDSAProfile, testCtx.cfsslIssuers, testCtx.boulderIssuers, + nil, testCtx.certExpiry, testCtx.certBackdate, testCtx.serialPrefix, @@ -1012,6 +1070,7 @@ func TestIssueCertificateForPrecertificateDuplicateSerial(t *testing.T) { testCtx.cfsslECDSAProfile, testCtx.cfsslIssuers, nil, + nil, testCtx.certExpiry, testCtx.certBackdate, testCtx.serialPrefix, @@ -1058,6 +1117,7 @@ func TestIssueCertificateForPrecertificateDuplicateSerial(t *testing.T) { testCtx.cfsslECDSAProfile, testCtx.cfsslIssuers, nil, + nil, testCtx.certExpiry, testCtx.certBackdate, testCtx.serialPrefix, @@ -1141,6 +1201,7 @@ func TestPrecertOrphanQueue(t *testing.T) { testCtx.cfsslECDSAProfile, testCtx.cfsslIssuers, nil, + nil, testCtx.certExpiry, testCtx.certBackdate, testCtx.serialPrefix, @@ -1213,6 +1274,7 @@ func TestOrphanQueue(t *testing.T) { testCtx.cfsslECDSAProfile, testCtx.cfsslIssuers, nil, + nil, testCtx.certExpiry, testCtx.certBackdate, testCtx.serialPrefix, @@ -1335,6 +1397,7 @@ func TestIssuePrecertificateLinting(t *testing.T) { testCtx.cfsslECDSAProfile, testCtx.cfsslIssuers, nil, + nil, testCtx.certExpiry, testCtx.certBackdate, testCtx.serialPrefix, @@ -1401,6 +1464,7 @@ func TestGenerateOCSPWithIssuerID(t *testing.T) { testCtx.cfsslECDSAProfile, testCtx.cfsslIssuers, nil, + nil, testCtx.certExpiry, testCtx.certBackdate, testCtx.serialPrefix, diff --git a/cmd/boulder-ca/main.go b/cmd/boulder-ca/main.go index 59dbb262d..ed528cecc 100644 --- a/cmd/boulder-ca/main.go +++ b/cmd/boulder-ca/main.go @@ -103,6 +103,15 @@ type config struct { // Recommended to be around 500ms. OCSPLogPeriod cmd.ConfigDuration + // List of Registration IDs for which ECDSA issuance is allowed. If an + // account is in this allowlist *and* requests issuance for an ECDSA key + // *and* an ECDSA issuer is configured in the CA, then the certificate + // will be issued from that ECDSA issuer. If this list is empty, then + // ECDSA issuance is allowed for all accounts. + // This is temporary, and will be used for testing and slow roll-out of + // ECDSA issuance, but will then be removed. + ECDSAAllowedAccounts []int64 + Features map[string]bool } @@ -309,6 +318,7 @@ func main() { c.CA.ECDSAProfile, cfsslIssuers, boulderIssuers, + c.CA.ECDSAAllowedAccounts, c.CA.Expiry.Duration, c.CA.Backdate.Duration, c.CA.SerialPrefix, diff --git a/features/featureflag_string.go b/features/featureflag_string.go index 0b841903c..97cd0b261 100644 --- a/features/featureflag_string.go +++ b/features/featureflag_string.go @@ -31,11 +31,12 @@ func _() { _ = x[RestrictRSAKeySizes-20] _ = x[FasterNewOrdersRateLimit-21] _ = x[NonCFSSLSigner-22] + _ = x[ECDSAForAll-23] } -const _FeatureFlag_name = "unusedWriteIssuedNamesPrecertHeadNonceStatusOKRemoveWFE2AccountIDCheckRenewalFirstParallelCheckFailedValidationDeleteUnusedChallengesBlockedKeyTableStoreKeyHashesPrecertificateRevocationCAAValidationMethodsCAAAccountURIEnforceMultiVAMultiVAFullResultsMandatoryPOSTAsGETAllowV1RegistrationV1DisableNewValidationsStripDefaultSchemePortStoreIssuerInfoStoreRevokerInfoRestrictRSAKeySizesFasterNewOrdersRateLimitNonCFSSLSigner" +const _FeatureFlag_name = "unusedWriteIssuedNamesPrecertHeadNonceStatusOKRemoveWFE2AccountIDCheckRenewalFirstParallelCheckFailedValidationDeleteUnusedChallengesBlockedKeyTableStoreKeyHashesPrecertificateRevocationCAAValidationMethodsCAAAccountURIEnforceMultiVAMultiVAFullResultsMandatoryPOSTAsGETAllowV1RegistrationV1DisableNewValidationsStripDefaultSchemePortStoreIssuerInfoStoreRevokerInfoRestrictRSAKeySizesFasterNewOrdersRateLimitNonCFSSLSignerECDSAForAll" -var _FeatureFlag_index = [...]uint16{0, 6, 29, 46, 65, 82, 111, 133, 148, 162, 186, 206, 219, 233, 251, 269, 288, 311, 333, 348, 364, 383, 407, 421} +var _FeatureFlag_index = [...]uint16{0, 6, 29, 46, 65, 82, 111, 133, 148, 162, 186, 206, 219, 233, 251, 269, 288, 311, 333, 348, 364, 383, 407, 421, 432} func (i FeatureFlag) String() string { if i < 0 || i >= FeatureFlag(len(_FeatureFlag_index)-1) { diff --git a/features/features.go b/features/features.go index 29da224a6..b38b59b4f 100644 --- a/features/features.go +++ b/features/features.go @@ -59,6 +59,9 @@ const ( // NonCFSSLSigner enables usage of our own certificate signer instead of the // CFSSL signer. NonCFSSLSigner + // ECDSAForAll enables all accounts, regardless of their presence in the CA's + // ecdsaAllowedAccounts config value, to get issuance from ECDSA issuers. + ECDSAForAll ) // List of features and their default value, protected by fMu @@ -86,6 +89,7 @@ var features = map[FeatureFlag]bool{ FasterNewOrdersRateLimit: false, BlockedKeyTable: false, NonCFSSLSigner: false, + ECDSAForAll: false, } var fMu = new(sync.RWMutex)