Skip to content

Commit

Permalink
CA: Create ECDSA issuance allowlist (#5258)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
aarongable committed Feb 1, 2021
1 parent 2a8f0fe commit 68c393b
Show file tree
Hide file tree
Showing 5 changed files with 121 additions and 26 deletions.
24 changes: 20 additions & 4 deletions ca/ca.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -257,6 +259,7 @@ func NewCertificateAuthorityImpl(
cfsslECDSAProfile string,
cfsslIssuers []Issuer,
boulderIssuers []*issuance.Issuer,
ecdsaAllowedRegIDs []int64,
certExpiry time.Duration,
certBackdate time.Duration,
serialPrefix int,
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -371,6 +379,7 @@ func NewCertificateAuthorityImpl(
issuers: issuers,
cfsslRSAProfile: cfsslRSAProfile,
cfsslECDSAProfile: cfsslECDSAProfile,
ecdsaAllowedRegIDs: ecdsaAllowedRegIDsMap,
validityPeriod: certExpiry,
backdate: certBackdate,
prefix: serialPrefix,
Expand Down Expand Up @@ -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)
}
Expand Down
104 changes: 84 additions & 20 deletions ca/ca_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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() {
Expand All @@ -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 {
Expand Down Expand Up @@ -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,
},
Expand Down Expand Up @@ -321,6 +338,7 @@ func TestFailNoSerialPrefix(t *testing.T) {
testCtx.cfsslECDSAProfile,
testCtx.cfsslIssuers,
nil,
nil,
testCtx.certExpiry,
testCtx.certBackdate,
0,
Expand Down Expand Up @@ -433,6 +451,7 @@ func issueCertificateSubTestSetup(t *testing.T, boulderIssuer bool) (*Certificat
testCtx.cfsslECDSAProfile,
cfsslIssuers,
boulderIssuers,
nil,
testCtx.certExpiry,
testCtx.certBackdate,
testCtx.serialPrefix,
Expand Down Expand Up @@ -497,6 +516,7 @@ func TestMultipleIssuers(t *testing.T) {
testCtx.cfsslECDSAProfile,
newIssuers,
nil,
nil,
testCtx.certExpiry,
testCtx.certBackdate,
testCtx.serialPrefix,
Expand All @@ -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{}
Expand All @@ -532,6 +584,7 @@ func TestOCSP(t *testing.T) {
testCtx.cfsslECDSAProfile,
testCtx.cfsslIssuers,
nil,
nil,
testCtx.certExpiry,
testCtx.certBackdate,
testCtx.serialPrefix,
Expand Down Expand Up @@ -593,6 +646,7 @@ func TestOCSP(t *testing.T) {
testCtx.cfsslECDSAProfile,
newIssuers,
nil,
nil,
testCtx.certExpiry,
testCtx.certBackdate,
testCtx.serialPrefix,
Expand Down Expand Up @@ -700,6 +754,7 @@ func TestInvalidCSRs(t *testing.T) {
testCtx.cfsslECDSAProfile,
testCtx.cfsslIssuers,
nil,
nil,
testCtx.certExpiry,
testCtx.certBackdate,
testCtx.serialPrefix,
Expand Down Expand Up @@ -741,6 +796,7 @@ func TestRejectValidityTooLong(t *testing.T) {
testCtx.cfsslECDSAProfile,
testCtx.cfsslIssuers,
nil,
nil,
testCtx.certExpiry,
testCtx.certBackdate,
testCtx.serialPrefix,
Expand Down Expand Up @@ -802,6 +858,7 @@ func TestSingleAIAEnforcement(t *testing.T) {
ecdsaProfileName,
nil,
nil,
nil,
8760*time.Hour,
time.Hour,
1,
Expand Down Expand Up @@ -920,6 +977,7 @@ func TestIssueCertificateForPrecertificate(t *testing.T) {
testCtx.cfsslECDSAProfile,
testCtx.cfsslIssuers,
testCtx.boulderIssuers,
nil,
testCtx.certExpiry,
testCtx.certBackdate,
testCtx.serialPrefix,
Expand Down Expand Up @@ -1012,6 +1070,7 @@ func TestIssueCertificateForPrecertificateDuplicateSerial(t *testing.T) {
testCtx.cfsslECDSAProfile,
testCtx.cfsslIssuers,
nil,
nil,
testCtx.certExpiry,
testCtx.certBackdate,
testCtx.serialPrefix,
Expand Down Expand Up @@ -1058,6 +1117,7 @@ func TestIssueCertificateForPrecertificateDuplicateSerial(t *testing.T) {
testCtx.cfsslECDSAProfile,
testCtx.cfsslIssuers,
nil,
nil,
testCtx.certExpiry,
testCtx.certBackdate,
testCtx.serialPrefix,
Expand Down Expand Up @@ -1141,6 +1201,7 @@ func TestPrecertOrphanQueue(t *testing.T) {
testCtx.cfsslECDSAProfile,
testCtx.cfsslIssuers,
nil,
nil,
testCtx.certExpiry,
testCtx.certBackdate,
testCtx.serialPrefix,
Expand Down Expand Up @@ -1213,6 +1274,7 @@ func TestOrphanQueue(t *testing.T) {
testCtx.cfsslECDSAProfile,
testCtx.cfsslIssuers,
nil,
nil,
testCtx.certExpiry,
testCtx.certBackdate,
testCtx.serialPrefix,
Expand Down Expand Up @@ -1335,6 +1397,7 @@ func TestIssuePrecertificateLinting(t *testing.T) {
testCtx.cfsslECDSAProfile,
testCtx.cfsslIssuers,
nil,
nil,
testCtx.certExpiry,
testCtx.certBackdate,
testCtx.serialPrefix,
Expand Down Expand Up @@ -1401,6 +1464,7 @@ func TestGenerateOCSPWithIssuerID(t *testing.T) {
testCtx.cfsslECDSAProfile,
testCtx.cfsslIssuers,
nil,
nil,
testCtx.certExpiry,
testCtx.certBackdate,
testCtx.serialPrefix,
Expand Down
10 changes: 10 additions & 0 deletions cmd/boulder-ca/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down Expand Up @@ -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,
Expand Down
5 changes: 3 additions & 2 deletions features/featureflag_string.go

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

4 changes: 4 additions & 0 deletions features/features.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -86,6 +89,7 @@ var features = map[FeatureFlag]bool{
FasterNewOrdersRateLimit: false,
BlockedKeyTable: false,
NonCFSSLSigner: false,
ECDSAForAll: false,
}

var fMu = new(sync.RWMutex)
Expand Down

0 comments on commit 68c393b

Please sign in to comment.