diff --git a/ca/ca.go b/ca/ca.go index 2b14b6bf81f..4c80439d37a 100644 --- a/ca/ca.go +++ b/ca/ca.go @@ -121,7 +121,6 @@ type certificateAuthorityImpl struct { // The prefix is prepended to the serial number. prefix byte - maxNames int keyPolicy goodkey.KeyPolicy clk clock.Clock log blog.Logger @@ -140,7 +139,6 @@ func NewCertificateAuthorityImpl( issuers []*issuance.Issuer, profiles map[string]*issuance.Profile, serialPrefix byte, - maxNames int, keyPolicy goodkey.KeyPolicy, logger blog.Logger, metrics *caMetrics, @@ -195,7 +193,6 @@ func NewCertificateAuthorityImpl( issuers: issuers, profiles: profiles, prefix: serialPrefix, - maxNames: maxNames, keyPolicy: keyPolicy, log: logger, metrics: metrics, @@ -239,7 +236,7 @@ func (ca *certificateAuthorityImpl) IssueCertificate(ctx context.Context, req *c return nil, err } - err = csrlib.VerifyCSR(ctx, csr, ca.maxNames, &ca.keyPolicy, ca.pa) + err = csrlib.VerifyCSR(ctx, csr, &ca.keyPolicy, ca.pa) if err != nil { return nil, err } diff --git a/ca/ca_test.go b/ca/ca_test.go index 77c0c4bebb2..bcd9cde8674 100644 --- a/ca/ca_test.go +++ b/ca/ca_test.go @@ -114,7 +114,6 @@ type caArgs struct { issuers []*issuance.Issuer profiles map[string]*issuance.Profile serialPrefix byte - maxNames int keyPolicy goodkey.KeyPolicy logger *blog.Mock metrics *caMetrics @@ -201,7 +200,6 @@ func newCAArgs(t *testing.T) *caArgs { issuers: issuers, profiles: profiles, serialPrefix: 0x11, - maxNames: 2, keyPolicy: keyPolicy, logger: blog.NewMock(), metrics: cametrics, @@ -214,7 +212,7 @@ func newCAArgs(t *testing.T) *caArgs { func (c *caArgs) make() (*certificateAuthorityImpl, error) { return NewCertificateAuthorityImpl( c.sa, c.sctService, c.pa, c.issuers, c.profiles, c.serialPrefix, - c.maxNames, c.keyPolicy, c.logger, c.metrics, c.clk) + c.keyPolicy, c.logger, c.metrics, c.clk) } type mockSA struct{} @@ -542,10 +540,6 @@ func TestIssueCertificate_BadCSR(t *testing.T) { name: "no names", csrPath: "./testdata/no_names.der.csr", }, - { - name: "too many names", - csrPath: "./testdata/too_many_names.der.csr", - }, { name: "short key", csrPath: "./testdata/short_key.der.csr", diff --git a/ca/testdata/too_many_names.der.csr b/ca/testdata/too_many_names.der.csr deleted file mode 100644 index 71771782f21..00000000000 Binary files a/ca/testdata/too_many_names.der.csr and /dev/null differ diff --git a/cmd/boulder-ca/main.go b/cmd/boulder-ca/main.go index a28aad48d50..e1708d5d856 100644 --- a/cmd/boulder-ca/main.go +++ b/cmd/boulder-ca/main.go @@ -67,13 +67,6 @@ type Config struct { // TODO(#7213): Replace `required_without` with `required` when SerialPrefix is removed. SerialPrefixHex string `validate:"required_without=SerialPrefix,omitempty,hexadecimal,len=2"` - // MaxNames is the maximum number of subjectAltNames in a single cert. - // The value supplied MUST be greater than 0 and no more than 100. These - // limits are per section 7.1 of our combined CP/CPS, under "DV-SSL - // Subscriber Certificate". The value must match the RA and WFE - // configurations. - MaxNames int `validate:"required,min=1,max=100"` - // GoodKey is an embedded config stanza for the goodkey library. GoodKey goodkey.Config @@ -139,10 +132,6 @@ func main() { serialPrefix = byte(parsedSerialPrefix) } - if c.CA.MaxNames == 0 { - cmd.Fail("Error in CA config: MaxNames must not be 0") - } - scope, logger, oTelShutdown := cmd.StatsAndLogging(c.Syslog, c.OpenTelemetry, c.CA.DebugAddr) defer oTelShutdown(context.Background()) cmd.LogStartup(logger) @@ -244,7 +233,6 @@ func main() { issuers, profiles, serialPrefix, - c.CA.MaxNames, kp, logger, metrics, diff --git a/csr/csr.go b/csr/csr.go index 34b84c39b3b..581f36298ec 100644 --- a/csr/csr.go +++ b/csr/csr.go @@ -43,7 +43,7 @@ var ( // VerifyCSR checks the validity of a x509.CertificateRequest. It uses // identifier.FromCSR to normalize the DNS names before checking whether we'll // issue for them. -func VerifyCSR(ctx context.Context, csr *x509.CertificateRequest, maxNames int, keyPolicy *goodkey.KeyPolicy, pa core.PolicyAuthority) error { +func VerifyCSR(ctx context.Context, csr *x509.CertificateRequest, keyPolicy *goodkey.KeyPolicy, pa core.PolicyAuthority) error { key, ok := csr.PublicKey.(crypto.PublicKey) if !ok { return invalidPubKey @@ -86,9 +86,6 @@ func VerifyCSR(ctx context.Context, csr *x509.CertificateRequest, maxNames int, if len(idents) == 0 { return invalidNoIdent } - if len(idents) > maxNames { - return berrors.BadCSRError("CSR contains more than %d identifiers", maxNames) - } err = pa.WillingToIssue(idents) if err != nil { diff --git a/csr/csr_test.go b/csr/csr_test.go index d3d3d1bc44f..6f1936449a1 100644 --- a/csr/csr_test.go +++ b/csr/csr_test.go @@ -15,7 +15,6 @@ import ( "testing" "github.com/letsencrypt/boulder/core" - berrors "github.com/letsencrypt/boulder/errors" "github.com/letsencrypt/boulder/features" "github.com/letsencrypt/boulder/goodkey" "github.com/letsencrypt/boulder/identifier" @@ -55,9 +54,6 @@ func TestVerifyCSR(t *testing.T) { brokenSignedReq := new(x509.CertificateRequest) *brokenSignedReq = *signedReq brokenSignedReq.Signature = []byte{1, 1, 1, 1} - signedReqWithHosts := new(x509.CertificateRequest) - *signedReqWithHosts = *signedReq - signedReqWithHosts.DNSNames = []string{"a.com", "b.com"} signedReqWithLongCN := new(x509.CertificateRequest) *signedReqWithLongCN = *signedReq signedReqWithLongCN.Subject.CommonName = strings.Repeat("a", maxCNLength+1) @@ -86,86 +82,68 @@ func TestVerifyCSR(t *testing.T) { cases := []struct { csr *x509.CertificateRequest - maxNames int pa core.PolicyAuthority expectedError error }{ { &x509.CertificateRequest{}, - 100, &mockPA{}, invalidPubKey, }, { &x509.CertificateRequest{PublicKey: &private.PublicKey}, - 100, &mockPA{}, unsupportedSigAlg, }, { brokenSignedReq, - 100, &mockPA{}, invalidSig, }, { signedReq, - 100, &mockPA{}, invalidNoIdent, }, { signedReqWithLongCN, - 100, &mockPA{}, nil, }, - { - signedReqWithHosts, - 1, - &mockPA{}, - berrors.BadCSRError("CSR contains more than 1 identifiers"), - }, { signedReqWithBadNames, - 100, &mockPA{}, errors.New("policy forbids issuing for identifier"), }, { signedReqWithEmailAddress, - 100, &mockPA{}, invalidEmailPresent, }, { signedReqWithIPAddress, - 100, &mockPA{}, nil, }, { signedReqWithIPCN, - 100, &mockPA{}, invalidIPCN, }, { signedReqWithURI, - 100, &mockPA{}, invalidURIPresent, }, { signedReqWithAllLongSANs, - 100, &mockPA{}, nil, }, } for _, c := range cases { - err := VerifyCSR(context.Background(), c.csr, c.maxNames, &keyPolicy, c.pa) + err := VerifyCSR(context.Background(), c.csr, &keyPolicy, c.pa) test.AssertDeepEquals(t, c.expectedError, err) } } @@ -294,7 +272,7 @@ func TestSHA1Deprecation(t *testing.T) { csr, err := x509.ParseCertificateRequest(csrBytes) test.AssertNotError(t, err, "parsing test CSR") - return VerifyCSR(context.Background(), csr, 100, &keyPolicy, &mockPA{}) + return VerifyCSR(context.Background(), csr, &keyPolicy, &mockPA{}) } err = makeAndVerifyCsr(x509.SHA256WithRSA) diff --git a/linter/lints/common.go b/linter/lints/common.go index 4efe482869d..b2f071759ab 100644 --- a/linter/lints/common.go +++ b/linter/lints/common.go @@ -25,6 +25,7 @@ const ( ) var ( + CPSV20Date = time.Date(2017, time.April, 13, 0, 0, 0, 0, time.UTC) CPSV33Date = time.Date(2021, time.June, 8, 0, 0, 0, 0, time.UTC) MozillaPolicy281Date = time.Date(2023, time.February, 15, 0, 0, 0, 0, time.UTC) ) diff --git a/linter/lints/cpcps/lint_cert_has_san_count_out_of_bounds.go b/linter/lints/cpcps/lint_cert_has_san_count_out_of_bounds.go new file mode 100644 index 00000000000..0f50ed8a4b9 --- /dev/null +++ b/linter/lints/cpcps/lint_cert_has_san_count_out_of_bounds.go @@ -0,0 +1,44 @@ +package cpcps + +import ( + "github.com/zmap/zcrypto/x509" + "github.com/zmap/zlint/v3/lint" + "github.com/zmap/zlint/v3/util" + + "github.com/letsencrypt/boulder/linter/lints" +) + +type certSubjectAltNamesCountOutOfBounds struct{} + +func init() { + lint.RegisterCertificateLint(&lint.CertificateLint{ + LintMetadata: lint.LintMetadata{ + Name: "e_cert_has_san_count_out_of_bounds", + Description: "Let's Encrypt Subscriber Certificates subjectAlternativeName must be a sequence of 1 to 100 dNSNames or ipAddresses", + Citation: "CPS: 7.1", + Source: lints.LetsEncryptCPS, + EffectiveDate: lints.CPSV20Date, + }, + Lint: CertNamesCountOutOfBounds, + }) +} + +func CertNamesCountOutOfBounds() lint.CertificateLintInterface { + return &certSubjectAltNamesCountOutOfBounds{} +} + +func (l *certSubjectAltNamesCountOutOfBounds) CheckApplies(c *x509.Certificate) bool { + return util.IsSubscriberCert(c) +} + +func (l *certSubjectAltNamesCountOutOfBounds) Execute(c *x509.Certificate) *lint.LintResult { + totalSANs := len(c.DNSNames) + len(c.IPAddresses) + + // more likely to encounter certs with greater than 100 vs with fewer than 1 + // so testing that failure first + if totalSANs > 100 || totalSANs < 1 { + return &lint.LintResult{Status: lint.Error} + } + + return &lint.LintResult{Status: lint.Pass} +} diff --git a/ra/ra.go b/ra/ra.go index e09e1b65005..6daa0550657 100644 --- a/ra/ra.go +++ b/ra/ra.go @@ -1023,7 +1023,7 @@ func (ra *RegistrationAuthorityImpl) validateFinalizeRequest( ) } - err = csrlib.VerifyCSR(ctx, csr, profile.maxNames, &ra.keyPolicy, ra.PA) + err = csrlib.VerifyCSR(ctx, csr, &ra.keyPolicy, ra.PA) if err != nil { // VerifyCSR returns berror instances that can be passed through as-is // without wrapping. @@ -1033,6 +1033,11 @@ func (ra *RegistrationAuthorityImpl) validateFinalizeRequest( // Dedupe, lowercase and sort both the names from the CSR and the names in the // order. csrIdents := identifier.FromCSR(csr) + // Check that the CSR identifiers count meets our CP/CPS requirements + if len(csrIdents) > profile.maxNames || len(csrIdents) < 1 { + return nil, nil, berrors.UnauthorizedError("CSR identifier count is not at minimum 1 or at maximum %d", profile.maxNames) + } + // Check that the order names and the CSR names are an exact match if !slices.Equal(csrIdents, orderIdents) { return nil, nil, berrors.UnauthorizedError("CSR does not specify same identifiers as Order") diff --git a/test/config-next/ca.json b/test/config-next/ca.json index b2d548ecb6b..bfec4b2556b 100644 --- a/test/config-next/ca.json +++ b/test/config-next/ca.json @@ -174,7 +174,6 @@ ] }, "serialPrefixHex": "6e", - "maxNames": 100, "goodkey": {}, "ocspLogMaxLength": 4000, "ctLogListFile": "test/ct-test-srv/log_list.json", diff --git a/test/config/ca.json b/test/config/ca.json index 59a1f348c5a..49056daaca2 100644 --- a/test/config/ca.json +++ b/test/config/ca.json @@ -181,7 +181,6 @@ ] }, "serialPrefixHex": "6e", - "maxNames": 100, "goodkey": {}, "ocspLogMaxLength": 4000, "ctLogListFile": "test/ct-test-srv/log_list.json",