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

Deprecate ECDSAForAll feature and remove ECDSAAllowList (#7560) 

`ECDSAForAll` feature is now enabled by default (due to it not being
referenced in any issuance path) and as a result the `ECDSAAllowlist`
has been deleted.

Fixes https://github.com/letsencrypt/boulder/issues/7535
This commit is contained in:
Phil Porada 2024-06-26 10:38:51 -04:00 committed by GitHub
parent 483062e0b1
commit 9207669755
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
15 changed files with 7 additions and 213 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
ca/ca.go
View File

@ -34,7 +34,6 @@ import (
corepb "github.com/letsencrypt/boulder/core/proto"
csrlib "github.com/letsencrypt/boulder/csr"
berrors "github.com/letsencrypt/boulder/errors"
"github.com/letsencrypt/boulder/features"
"github.com/letsencrypt/boulder/goodkey"
"github.com/letsencrypt/boulder/issuance"
"github.com/letsencrypt/boulder/linter"
@ -128,9 +127,6 @@ type certificateAuthorityImpl struct {
issuers issuerMaps
certProfiles certProfilesMaps
// This is temporary, and will be used for testing and slow roll-out
// of ECDSA issuance, but will then be removed.
ecdsaAllowList *ECDSAAllowList
prefix int // Prepended to the serial number
validityPeriod time.Duration
backdate time.Duration
@ -250,7 +246,6 @@ func NewCertificateAuthorityImpl(
defaultCertProfileName string,
certificateProfiles map[string]issuance.ProfileConfig,
lints lint.Registry,
ecdsaAllowList *ECDSAAllowList,
certExpiry time.Duration,
certBackdate time.Duration,
serialPrefix int,
@ -302,7 +297,6 @@ func NewCertificateAuthorityImpl(
log: logger,
metrics: metrics,
clk: clk,
ecdsaAllowList: ecdsaAllowList,
}
return ca, nil
@ -566,11 +560,8 @@ func (ca *certificateAuthorityImpl) issuePrecertificateInner(ctx context.Context
}
// Select which pool of issuers to use, based on the to-be-issued cert's key
// type and whether we're using the ECDSA Allow List.
// type.
alg := csr.PublicKeyAlgorithm
if alg == x509.ECDSA && !features.Get().ECDSAForAll && ca.ecdsaAllowList != nil && !ca.ecdsaAllowList.permitted(issueReq.RegistrationID) {
alg = x509.RSA
}
// Select a random issuer from among the active issuers of this key type.
issuerPool, ok := ca.issuers.byAlg[alg]

54
ca/ca_test.go
View File

@ -275,7 +275,6 @@ func TestSerialPrefix(t *testing.T) {
"",
nil,
nil,
nil,
testCtx.certExpiry,
testCtx.certBackdate,
0,
@ -293,7 +292,6 @@ func TestSerialPrefix(t *testing.T) {
"",
nil,
nil,
nil,
testCtx.certExpiry,
testCtx.certBackdate,
128,
@ -352,7 +350,7 @@ func TestIssuePrecertificate(t *testing.T) {
// the precertificates previously generated from the preceding
// "precertificate" test.
for _, mode := range []string{"precertificate", "certificate-for-precertificate"} {
ca, sa := issueCertificateSubTestSetup(t, nil)
ca, sa := issueCertificateSubTestSetup(t)
t.Run(fmt.Sprintf("%s - %s", mode, testCase.name), func(t *testing.T) {
t.Parallel()
req, err := x509.ParseCertificateRequest(testCase.csr)
@ -388,12 +386,8 @@ func TestIssuePrecertificate(t *testing.T) {
}
}
func issueCertificateSubTestSetup(t *testing.T, e *ECDSAAllowList) (*certificateAuthorityImpl, *mockSA) {
func issueCertificateSubTestSetup(t *testing.T) (*certificateAuthorityImpl, *mockSA) {
testCtx := setup(t)
ecdsaAllowList := &ECDSAAllowList{}
if e == nil {
e = ecdsaAllowList
}
sa := &mockSA{}
ca, err := NewCertificateAuthorityImpl(
sa,
@ -402,7 +396,6 @@ func issueCertificateSubTestSetup(t *testing.T, e *ECDSAAllowList) (*certificate
testCtx.defaultCertProfileName,
testCtx.certProfiles,
testCtx.lints,
e,
testCtx.certExpiry,
testCtx.certBackdate,
testCtx.serialPrefix,
@ -450,7 +443,6 @@ func TestNoIssuers(t *testing.T) {
testCtx.defaultCertProfileName,
testCtx.certProfiles,
testCtx.lints,
nil,
testCtx.certExpiry,
testCtx.certBackdate,
testCtx.serialPrefix,
@ -475,7 +467,6 @@ func TestMultipleIssuers(t *testing.T) {
testCtx.defaultCertProfileName,
testCtx.certProfiles,
testCtx.lints,
nil,
testCtx.certExpiry,
testCtx.certBackdate,
testCtx.serialPrefix,
@ -553,7 +544,6 @@ func TestUnpredictableIssuance(t *testing.T) {
testCtx.defaultCertProfileName,
testCtx.certProfiles,
testCtx.lints,
nil,
testCtx.certExpiry,
testCtx.certBackdate,
testCtx.serialPrefix,
@ -722,7 +712,6 @@ func TestProfiles(t *testing.T) {
tc.defaultName,
tc.profileConfigs,
testCtx.lints,
nil,
testCtx.certExpiry,
testCtx.certBackdate,
testCtx.serialPrefix,
@ -766,39 +755,6 @@ func TestProfiles(t *testing.T) {
}
}
func TestECDSAAllowList(t *testing.T) {
t.Parallel()
req := &capb.IssueCertificateRequest{Csr: ECDSACSR, RegistrationID: arbitraryRegID}
// With allowlist containing arbitraryRegID, issuance should come from ECDSA issuer.
regIDMap := makeRegIDsMap([]int64{arbitraryRegID})
ca, _ := issueCertificateSubTestSetup(t, &ECDSAAllowList{regIDMap})
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.AssertEquals(t, cert.SignatureAlgorithm, x509.ECDSAWithSHA384)
// With allowlist not containing arbitraryRegID, issuance should fall back to RSA issuer.
regIDMap = makeRegIDsMap([]int64{2002})
ca, _ = issueCertificateSubTestSetup(t, &ECDSAAllowList{regIDMap})
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.AssertEquals(t, cert.SignatureAlgorithm, x509.SHA256WithRSA)
// With empty allowlist but ECDSAForAll enabled, issuance should come from ECDSA issuer.
ca, _ = issueCertificateSubTestSetup(t, nil)
features.Set(features.Config{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.AssertEquals(t, cert.SignatureAlgorithm, x509.ECDSAWithSHA384)
}
func TestInvalidCSRs(t *testing.T) {
t.Parallel()
testCases := []struct {
@ -864,7 +820,6 @@ func TestInvalidCSRs(t *testing.T) {
testCtx.defaultCertProfileName,
testCtx.certProfiles,
testCtx.lints,
nil,
testCtx.certExpiry,
testCtx.certBackdate,
testCtx.serialPrefix,
@ -903,7 +858,6 @@ func TestRejectValidityTooLong(t *testing.T) {
testCtx.defaultCertProfileName,
testCtx.certProfiles,
testCtx.lints,
nil,
testCtx.certExpiry,
testCtx.certBackdate,
testCtx.serialPrefix,
@ -1004,7 +958,6 @@ func TestIssueCertificateForPrecertificate(t *testing.T) {
testCtx.defaultCertProfileName,
testCtx.certProfiles,
testCtx.lints,
nil,
testCtx.certExpiry,
testCtx.certBackdate,
testCtx.serialPrefix,
@ -1073,7 +1026,6 @@ func TestIssueCertificateForPrecertificateWithSpecificCertificateProfile(t *test
testCtx.defaultCertProfileName,
testCtx.certProfiles,
testCtx.lints,
nil,
testCtx.certExpiry,
testCtx.certBackdate,
testCtx.serialPrefix,
@ -1193,7 +1145,6 @@ func TestIssueCertificateForPrecertificateDuplicateSerial(t *testing.T) {
testCtx.defaultCertProfileName,
testCtx.certProfiles,
testCtx.lints,
nil,
testCtx.certExpiry,
testCtx.certBackdate,
testCtx.serialPrefix,
@ -1244,7 +1195,6 @@ func TestIssueCertificateForPrecertificateDuplicateSerial(t *testing.T) {
testCtx.defaultCertProfileName,
testCtx.certProfiles,
testCtx.lints,
nil,
testCtx.certExpiry,
testCtx.certBackdate,
testCtx.serialPrefix,

45
ca/ecdsa_allow_list.go
View File

@ -1,45 +0,0 @@
package ca
import (
"os"
"github.com/letsencrypt/boulder/strictyaml"
)
// ECDSAAllowList acts as a container for a map of Registration IDs.
type ECDSAAllowList struct {
regIDsMap map[int64]bool
}
// permitted checks if ECDSA issuance is permitted for the specified
// Registration ID.
func (e *ECDSAAllowList) permitted(regID int64) bool {
return e.regIDsMap[regID]
}
func makeRegIDsMap(regIDs []int64) map[int64]bool {
regIDsMap := make(map[int64]bool)
for _, regID := range regIDs {
regIDsMap[regID] = true
}
return regIDsMap
}
// NewECDSAAllowListFromFile is exported to allow `boulder-ca` to construct a
// new `ECDSAAllowList` object. It returns the ECDSAAllowList, the size of allow
// list after attempting to load it (for CA logging purposes so inner fields don't need to be exported), or an error.
func NewECDSAAllowListFromFile(filename string) (*ECDSAAllowList, int, error) {
configBytes, err := os.ReadFile(filename)
if err != nil {
return nil, 0, err
}
var regIDs []int64
err = strictyaml.Unmarshal(configBytes, &regIDs)
if err != nil {
return nil, 0, err
}
allowList := &ECDSAAllowList{regIDsMap: makeRegIDsMap(regIDs)}
return allowList, len(allowList.regIDsMap), nil
}

70
ca/ecdsa_allow_list_test.go
View File

@ -1,70 +0,0 @@
package ca
import (
"testing"
)
func TestNewECDSAAllowListFromFile(t *testing.T) {
t.Parallel()
type args struct {
filename string
}
tests := []struct {
name string
args args
want1337Permitted bool
wantEntries int
wantErrBool bool
}{
{
name: "one entry",
args: args{"testdata/ecdsa_allow_list.yml"},
want1337Permitted: true,
wantEntries: 1,
wantErrBool: false,
},
{
name: "one entry but it's not 1337",
args: args{"testdata/ecdsa_allow_list2.yml"},
want1337Permitted: false,
wantEntries: 1,
wantErrBool: false,
},
{
name: "should error due to no file",
args: args{"testdata/ecdsa_allow_list_no_exist.yml"},
want1337Permitted: false,
wantEntries: 0,
wantErrBool: true,
},
{
name: "should error due to malformed YAML",
args: args{"testdata/ecdsa_allow_list_malformed.yml"},
want1337Permitted: false,
wantEntries: 0,
wantErrBool: true,
},
}
for _, tt := range tests {
// TODO(Remove this >= go1.22.3) This shouldn't be necessary due to
// go1.22 changing loopvars.
// https://github.com/golang/go/issues/65612#issuecomment-1943342030
tt := tt
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
allowList, gotEntries, err := NewECDSAAllowListFromFile(tt.args.filename)
if (err != nil) != tt.wantErrBool {
t.Errorf("NewECDSAAllowListFromFile() error = %v, wantErr %v", err, tt.wantErrBool)
t.Error(allowList, gotEntries, err)
return
}
if allowList != nil && allowList.permitted(1337) != tt.want1337Permitted {
t.Errorf("NewECDSAAllowListFromFile() allowList = %v, want %v", allowList, tt.want1337Permitted)
}
if gotEntries != tt.wantEntries {
t.Errorf("NewECDSAAllowListFromFile() gotEntries = %v, want %v", gotEntries, tt.wantEntries)
}
})
}
}

1
ca/ocsp_test.go
View File

@ -36,7 +36,6 @@ func TestOCSP(t *testing.T) {
testCtx.defaultCertProfileName,
testCtx.certProfiles,
testCtx.lints,
nil,
testCtx.certExpiry,
testCtx.certBackdate,
testCtx.serialPrefix,

2
ca/testdata/ecdsa_allow_list.yml vendored
View File

@ -1,2 +0,0 @@
---
- 1337

2
ca/testdata/ecdsa_allow_list2.yml vendored
View File

@ -1,2 +0,0 @@
---
- 1338

1
ca/testdata/ecdsa_allow_list_malformed.yml vendored
View File

@ -1 +0,0 @@
not yaml

14
cmd/boulder-ca/main.go
View File

@ -100,10 +100,6 @@ type Config struct {
// Recommended to be around 500ms.
OCSPLogPeriod config.Duration
// Path of a YAML file containing the list of int64 RegIDs
// allowed to request ECDSA issuance
ECDSAAllowListFilename string
// CTLogListFile is the path to a JSON file on disk containing the set of
// all logs trusted by Chrome. The file must match the v3 log list schema:
// https://www.gstatic.com/ct/log_list/v3/log_list_schema.json
@ -236,15 +232,6 @@ func main() {
kp, err := sagoodkey.NewPolicy(&c.CA.GoodKey, sa.KeyBlocked)
cmd.FailOnError(err, "Unable to create key policy")
var ecdsaAllowList *ca.ECDSAAllowList
var entries int
if c.CA.ECDSAAllowListFilename != "" {
// Create an allow list object.
ecdsaAllowList, entries, err = ca.NewECDSAAllowListFromFile(c.CA.ECDSAAllowListFilename)
cmd.FailOnError(err, "Unable to load ECDSA allow list from YAML file")
logger.Infof("Loaded an ECDSA allow list with %d entries", entries)
}
srv := bgrpc.NewServer(c.CA.GRPCCA, logger)
if !c.CA.DisableOCSPService {
@ -286,7 +273,6 @@ func main() {
c.CA.Issuance.DefaultCertificateProfileName,
c.CA.Issuance.CertProfiles,
lints,
ecdsaAllowList,
c.CA.Expiry.Duration,
c.CA.Backdate.Duration,
c.CA.SerialPrefix,

5
features/features.go
View File

@ -27,10 +27,7 @@ type Config struct {
EnforceMultiVA bool
MultiVAFullResults bool
CertCheckerRequiresCorrespondence bool
// ECDSAForAll enables all accounts, regardless of their presence in the CA's
// ecdsaAllowedAccounts config value, to get issuance from ECDSA issuers.
ECDSAForAll bool
ECDSAForAll bool
// ServeRenewalInfo exposes the renewalInfo endpoint in the directory and for
// GET requests. WARNING: This feature is a draft and highly unstable.

3
issuance/issuer.go
View File

@ -157,8 +157,7 @@ type IssuerConfig struct {
// (for which an issuance token is presented), OCSP responses, and CRLs.
// All Active issuers of a given key type (RSA or ECDSA) are part of a pool
// and each precertificate will be issued randomly from a selected pool.
// The selection of which pool depends on the precertificate's key algorithm,
// the ECDSAForAll feature flag, and the ECDSAAllowListFilename config field.
// The selection of which pool depends on the precertificate's key algorithm.
Active bool
IssuerURL string `validate:"required,url"`

4
test/config-next/ca.json
View File

@ -149,9 +149,7 @@
"ocspLogMaxLength": 4000,
"ocspLogPeriod": "500ms",
"ctLogListFile": "test/ct-test-srv/log_list.json",
"features": {
"ECDSAForAll": true
}
"features": {}
},
"pa": {
"challenges": {

2
test/config-next/ecdsaAllowList.yml
View File

@ -1,2 +0,0 @@
---
- 1337

4
test/config/ca.json
View File

@ -144,9 +144,7 @@
},
"ocspLogMaxLength": 4000,
"ocspLogPeriod": "500ms",
"features": {
"ECDSAForAll": true
}
"features": {}
},
"pa": {
"challenges": {

2
test/config/ecdsaAllowList.yml
View File

@ -1,2 +0,0 @@
---
- 1337