KeyPolicy: add custom constructor and make all fields private (#7543)

Change how goodkey.KeyPolicy keeps track of allowed RSA and ECDSA key
sizes, to make it slightly more flexible while still retaining the very
locked-down allowlist of only 6 acceptable key sizes (RSA 2048, 3076,
and 4092, and ECDSA P256, P384, and P521). Add a new constructor which
takes in a collection of allowed key sizes, so that users of the goodkey
package can customize which keys they accept. Rename the existing
constructor to make it clear that it uses hardcoded default values.

With these new constructors available, make all of the goodkey.KeyPolicy
member fields private, so that a KeyPolicy can only be built via these
constructors.

ca/ca_test.go

@@ -198,11 +198,9 @@ func setup(t *testing.T) *testCtx {
 		test.AssertNotError(t, err, "Couldn't load test issuer")
 	}
 
-	keyPolicy := goodkey.KeyPolicy{
-		AllowRSA:           true,
-		AllowECDSANISTP256: true,
-		AllowECDSANISTP384: true,
-	}
+	keyPolicy, err := goodkey.NewPolicy(nil, nil)
+	test.AssertNotError(t, err, "Failed to create test keypolicy")
+
 	signatureCount := prometheus.NewCounterVec(
 		prometheus.CounterOpts{
 			Name: "signatures",

cmd/boulder-ca/main.go

@@ -233,7 +233,7 @@ func main() {
 	cmd.FailOnError(err, "Failed to load credentials and create gRPC connection to SA")
 	sa := sapb.NewStorageAuthorityClient(conn)
 
-	kp, err := sagoodkey.NewKeyPolicy(&c.CA.GoodKey, sa.KeyBlocked)
+	kp, err := sagoodkey.NewPolicy(&c.CA.GoodKey, sa.KeyBlocked)
 	cmd.FailOnError(err, "Unable to create key policy")
 
 	var ecdsaAllowList *ca.ECDSAAllowList

cmd/boulder-ra/main.go

@@ -254,7 +254,7 @@ func main() {
 		cmd.Fail("finalizeTimeout must be supplied when AsyncFinalize feature is enabled")
 	}
 
-	kp, err := sagoodkey.NewKeyPolicy(&c.RA.GoodKey, sac.KeyBlocked)
+	kp, err := sagoodkey.NewPolicy(&c.RA.GoodKey, sac.KeyBlocked)
 	cmd.FailOnError(err, "Unable to create key policy")
 
 	if c.RA.MaxNames == 0 {

cmd/boulder-wfe2/main.go

@@ -311,7 +311,7 @@ func main() {
 
 	rac, sac, gnc, rnc, npKey := setupWFE(c, stats, clk)
 
-	kp, err := sagoodkey.NewKeyPolicy(&c.WFE.GoodKey, sac.KeyBlocked)
+	kp, err := sagoodkey.NewPolicy(&c.WFE.GoodKey, sac.KeyBlocked)
 	cmd.FailOnError(err, "Unable to create key policy")
 
 	if c.WFE.StaleTimeout.Duration == 0 {

cmd/ceremony/main.go

@@ -34,7 +34,7 @@ var kp goodkey.KeyPolicy
 
 func init() {
 	var err error
-	kp, err = goodkey.NewKeyPolicy(&goodkey.Config{FermatRounds: 100}, nil)
+	kp, err = goodkey.NewPolicy(&goodkey.Config{FermatRounds: 100}, nil)
 	if err != nil {
 		log.Fatal("Could not create goodkey.KeyPolicy")
 	}

cmd/cert-checker/main.go

@@ -553,7 +553,7 @@ func main() {
 	if config.CertChecker.GoodKey.BlockedKeyFile != "" {
 		cmd.Fail("cert-checker does not support checking against blocked key files")
 	}
-	kp, err := sagoodkey.NewKeyPolicy(&config.CertChecker.GoodKey, nil)
+	kp, err := sagoodkey.NewPolicy(&config.CertChecker.GoodKey, nil)
 	cmd.FailOnError(err, "Unable to create key policy")
 
 	saDbMap, err := sa.InitWrappedDb(config.CertChecker.DB, prometheus.DefaultRegisterer, logger)

cmd/cert-checker/main_test.go

@@ -59,7 +59,7 @@ func init() {
 	if err != nil {
 		log.Fatal(err)
 	}
-	kp, err = sagoodkey.NewKeyPolicy(&goodkey.Config{FermatRounds: 100}, nil)
+	kp, err = sagoodkey.NewPolicy(&goodkey.Config{FermatRounds: 100}, nil)
 	if err != nil {
 		log.Fatal(err)
 	}

csr/csr_test.go

@@ -20,12 +20,6 @@ import (
 	"github.com/letsencrypt/boulder/test"
 )
 
-var testingPolicy = &goodkey.KeyPolicy{
-	AllowRSA:           true,
-	AllowECDSANISTP256: true,
-	AllowECDSANISTP384: true,
-}
-
 type mockPA struct{}
 
 func (pa *mockPA) ChallengesFor(identifier identifier.ACMEIdentifier) (challenges []core.Challenge, err error) {
@@ -78,87 +72,79 @@ func TestVerifyCSR(t *testing.T) {
 	*signedReqWithAllLongSANs = *signedReq
 	signedReqWithAllLongSANs.DNSNames = []string{"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.com"}
 
+	keyPolicy, err := goodkey.NewPolicy(nil, nil)
+	test.AssertNotError(t, err, "creating test keypolicy")
+
 	cases := []struct {
 		csr           *x509.CertificateRequest
 		maxNames      int
-		keyPolicy     *goodkey.KeyPolicy
 		pa            core.PolicyAuthority
 		expectedError error
 	}{
 		{
 			&x509.CertificateRequest{},
 			100,
-			testingPolicy,
 			&mockPA{},
 			invalidPubKey,
 		},
 		{
 			&x509.CertificateRequest{PublicKey: &private.PublicKey},
 			100,
-			testingPolicy,
 			&mockPA{},
 			unsupportedSigAlg,
 		},
 		{
 			brokenSignedReq,
 			100,
-			testingPolicy,
 			&mockPA{},
 			invalidSig,
 		},
 		{
 			signedReq,
 			100,
-			testingPolicy,
 			&mockPA{},
 			invalidNoDNS,
 		},
 		{
 			signedReqWithLongCN,
 			100,
-			testingPolicy,
 			&mockPA{},
 			berrors.BadCSRError("CN was longer than %d bytes", maxCNLength),
 		},
 		{
 			signedReqWithHosts,
 			1,
-			testingPolicy,
 			&mockPA{},
 			berrors.BadCSRError("CSR contains more than 1 DNS names"),
 		},
 		{
 			signedReqWithBadNames,
 			100,
-			testingPolicy,
 			&mockPA{},
 			errors.New("policy forbids issuing for identifier"),
 		},
 		{
 			signedReqWithEmailAddress,
 			100,
-			testingPolicy,
 			&mockPA{},
 			invalidEmailPresent,
 		},
 		{
 			signedReqWithIPAddress,
 			100,
-			testingPolicy,
 			&mockPA{},
 			invalidIPPresent,
 		},
 		{
 			signedReqWithAllLongSANs,
 			100,
-			testingPolicy,
 			&mockPA{},
 			nil,
 		},
 	}
 
 	for _, c := range cases {
-		err := VerifyCSR(context.Background(), c.csr, c.maxNames, c.keyPolicy, c.pa)
+		err := VerifyCSR(context.Background(), c.csr, c.maxNames, &keyPolicy, c.pa)
 		test.AssertDeepEquals(t, c.expectedError, err)
 	}
 }
@@ -239,6 +225,9 @@ func TestNamesFromCSR(t *testing.T) {
 func TestSHA1Deprecation(t *testing.T) {
 	features.Reset()
 
+	keyPolicy, err := goodkey.NewPolicy(nil, nil)
+	test.AssertNotError(t, err, "creating test keypolicy")
+
 	private, err := rsa.GenerateKey(rand.Reader, 2048)
 	test.AssertNotError(t, err, "error generating test key")
 
@@ -254,7 +243,7 @@ func TestSHA1Deprecation(t *testing.T) {
 		csr, err := x509.ParseCertificateRequest(csrBytes)
 		test.AssertNotError(t, err, "parsing test CSR")
 
-		return VerifyCSR(context.Background(), csr, 100, testingPolicy, &mockPA{})
+		return VerifyCSR(context.Background(), csr, 100, &keyPolicy, &mockPA{})
 	}
 
 	err = makeAndVerifyCsr(x509.SHA256WithRSA)

goodkey/blocked_test.go

@@ -6,10 +6,11 @@ import (
 	"os"
 	"testing"
 
+	yaml "gopkg.in/yaml.v3"
+
 	"github.com/letsencrypt/boulder/core"
 	"github.com/letsencrypt/boulder/test"
 	"github.com/letsencrypt/boulder/web"
-	yaml "gopkg.in/yaml.v3"
 )
 
 func TestBlockedKeys(t *testing.T) {
@@ -76,11 +77,9 @@ func TestBlockedKeys(t *testing.T) {
 	test.AssertNotError(t, err, "unexpected error loading empty blockedKeys yaml file")
 
 	// Create a test policy that doesn't reference the blocked list
-	testingPolicy := &KeyPolicy{
-		AllowRSA:           true,
-		AllowECDSANISTP256: true,
-		AllowECDSANISTP384: true,
-	}
+	testingPolicy := &KeyPolicy{allowedKeys: AllowedKeys{
+		RSA2048: true, RSA3072: true, RSA4096: true, ECDSAP256: true, ECDSAP384: true,
+	}}
 
 	// All of the test keys should not be considered blocked
 	for _, k := range blockedKeys {