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

Remove support for issuing certificates with no CN (#5008) 

We'd like to issue certs with no CN eventually, but it's not
going to happen any time soon. In the mean time, the existing
code never gets exercised and is rather complex, so this
removes it.
This commit is contained in:
Roland Bracewell Shoemaker 2020-08-05 09:15:30 -07:00 committed by GitHub
parent 75b034637b
commit 7853b12cb3
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 49 additions and 111 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
ca/ca.go
View File

@ -130,7 +130,6 @@ type CertificateAuthorityImpl struct {
validityPeriod time.Duration
backdate time.Duration
maxNames int
forceCNFromSAN bool
signatureCount *prometheus.CounterVec
csrExtensionCount *prometheus.CounterVec
orphanCount *prometheus.CounterVec
@ -308,7 +307,6 @@ func NewCertificateAuthorityImpl(
clk: clk,
log: logger,
keyPolicy: keyPolicy,
forceCNFromSAN: !config.DoNotForceCN, // Note the inversion here
signatureCount: signatureCount,
csrExtensionCount: csrExtensionCount,
orphanCount: orphanCount,
@ -694,7 +692,6 @@ func (ca *CertificateAuthorityImpl) issuePrecertificateInner(ctx context.Context
ca.maxNames,
&ca.keyPolicy,
ca.pa,
ca.forceCNFromSAN,
issueReq.RegistrationID,
); err != nil {
ca.log.AuditErr(err.Error())
@ -751,10 +748,6 @@ func (ca *CertificateAuthorityImpl) issuePrecertificateInner(ctx context.Context
serialHex := core.SerialToString(serialBigInt)
if !ca.forceCNFromSAN {
req.Subject.SerialNumber = serialHex
}
ca.log.AuditInfof("Signing: serial=[%s] names=[%s] csr=[%s]",
serialHex, strings.Join(csr.DNSNames, ", "), hex.EncodeToString(csr.Raw))

37
ca/ca_test.go
View File

@ -14,7 +14,6 @@ import (
"io/ioutil"
"math/big"
"os"
"sort"
"strings"
"testing"
"time"
@ -52,13 +51,6 @@ var (
// * DNSNames = not-example.com, www.not-example.com
CNandSANCSR = mustRead("./testdata/cn_and_san.der.csr")
// CSR generated by Go:
// * Random public key
// * C = US
// * CN = [none]
// * DNSNames = not-example.com
NoCNCSR = mustRead("./testdata/no_cn.der.csr")
// CSR generated by Go:
// * Random public key
// * CN = not-example.com
@ -325,7 +317,6 @@ func TestIssuePrecertificate(t *testing.T) {
}{
{"IssuePrecertificate", CNandSANCSR, issueCertificateSubTestIssuePrecertificate},
{"ValidityUsesCAClock", CNandSANCSR, issueCertificateSubTestValidityUsesCAClock},
{"AllowNoCN", NoCNCSR, issueCertificateSubTestAllowNoCN},
{"ProfileSelectionRSA", CNandSANCSR, issueCertificateSubTestProfileSelectionRSA},
{"ProfileSelectionECDSA", ECDSACSR, issueCertificateSubTestProfileSelectionECDSA},
{"MustStaple", MustStapleCSR, issueCertificateSubTestMustStaple},
@ -394,7 +385,6 @@ func issueCertificateSubTestSetup(t *testing.T) (*CertificateAuthorityImpl, *moc
testCtx.logger,
nil)
test.AssertNotError(t, err, "Failed to create CA")
ca.forceCNFromSAN = false
return ca, sa
}
@ -414,11 +404,6 @@ func issueCertificateSubTestIssuePrecertificate(t *testing.T, i *TestCertificate
if len(cert.Subject.Country) > 0 {
t.Errorf("Subject contained unauthorized values: %v", cert.Subject)
}
serialString := core.SerialToString(cert.SerialNumber)
if cert.Subject.SerialNumber != serialString {
t.Errorf("SerialNumber: want %#v, got %#v", serialString, cert.Subject.SerialNumber)
}
}
func issueCertificateSubTestValidityUsesCAClock(t *testing.T, i *TestCertificateIssuance) {
@ -665,7 +650,7 @@ func TestRejectValidityTooLong(t *testing.T) {
test.AssertNotError(t, err, "Failed to parse time")
testCtx.fc.Set(future)
// Test that the CA rejects CSRs that would expire after the intermediate cert
_, err = ca.IssuePrecertificate(ctx, &capb.IssueCertificateRequest{Csr: NoCNCSR, RegistrationID: arbitraryRegID})
_, err = ca.IssuePrecertificate(ctx, &capb.IssueCertificateRequest{Csr: CNandSANCSR, RegistrationID: arbitraryRegID})
test.AssertError(t, err, "Cannot issue a certificate that expires after the intermediate certificate")
test.Assert(t, berrors.Is(err, berrors.InternalServer), "Incorrect error type returned")
}
@ -717,26 +702,6 @@ func TestSingleAIAEnforcement(t *testing.T) {
test.AssertEquals(t, err.Error(), "only one issuer_url supported")
}
func issueCertificateSubTestAllowNoCN(t *testing.T, i *TestCertificateIssuance) {
cert := i.cert
if cert.Subject.CommonName != "" {
t.Errorf("want no CommonName, got %#v", cert.Subject.CommonName)
}
serial := core.SerialToString(cert.SerialNumber)
if cert.Subject.SerialNumber != serial {
t.Errorf("SerialNumber: want %#v, got %#v", serial, cert.Subject.SerialNumber)
}
expected := []string{}
expected = append(expected, i.req.DNSNames...)
sort.Strings(expected)
actual := []string{}
actual = append(actual, cert.DNSNames...)
sort.Strings(actual)
test.AssertDeepEquals(t, actual, expected)
}
func issueCertificateSubTestProfileSelectionRSA(t *testing.T, i *TestCertificateIssuance) {
// Certificates for RSA keys should be marked as usable for signatures and encryption.
expectedKeyUsage := x509.KeyUsageDigitalSignature | x509.KeyUsageKeyEncipherment

5
ca/config/config.go
View File

@ -37,11 +37,6 @@ type CAConfig struct {
MaxNames int
CFSSL cfsslConfig.Config
// DoNotForceCN is a temporary config setting. It controls whether
// to add a certificate's serial to its Subject, and whether to
// not pull a SAN entry to be the CN if no CN was given in a CSR.
DoNotForceCN bool
// WeakKeyFile is the path to a JSON file containing truncated RSA modulus
// hashes of known easily enumerable keys.
WeakKeyFile string

2
cmd/admin-revoker/main_test.go
View File

@ -54,7 +54,7 @@ func TestRevokeBatch(t *testing.T) {
ra := ra.NewRegistrationAuthorityImpl(fc,
log,
metrics.NoopRegisterer,
1, goodkey.KeyPolicy{}, 100, true, false, 300*24*time.Hour, 7*24*time.Hour, nil, nil, 0, nil, nil, &x509.Certificate{})
1, goodkey.KeyPolicy{}, 100, true, 300*24*time.Hour, 7*24*time.Hour, nil, nil, 0, nil, nil, &x509.Certificate{})
ra.SA = ssa
ra.CA = &mockCA{}

4
cmd/boulder-ra/main.go
View File

@ -39,8 +39,7 @@ type config struct {
PublisherService *cmd.GRPCClientConfig
AkamaiPurgerService *cmd.GRPCClientConfig
MaxNames int
DoNotForceCN bool
MaxNames int
// Controls behaviour of the RA when asked to create a new authz for
// a name/regID that already has a valid authz. False preserves historic
@ -218,7 +217,6 @@ func main() {
c.RA.MaxContactsPerRegistration,
kp,
c.RA.MaxNames,
c.RA.DoNotForceCN,
c.RA.ReuseValidAuthz,
authorizationLifetime,
pendingAuthorizationLifetime,

16
csr/csr.go
View File

@ -44,10 +44,10 @@ var (
)
// VerifyCSR checks the validity of a x509.CertificateRequest. Before doing checks it normalizes
// the CSR which lowers the case of DNS names and subject CN, and if forceCNFromSAN is true it
// will hoist a DNS name into the CN if it is empty.
func VerifyCSR(ctx context.Context, csr *x509.CertificateRequest, maxNames int, keyPolicy *goodkey.KeyPolicy, pa core.PolicyAuthority, forceCNFromSAN bool, regID int64) error {
normalizeCSR(csr, forceCNFromSAN)
// the CSR which lowers the case of DNS names and subject CN, and hoist a DNS name into the CN
// if it is empty.
func VerifyCSR(ctx context.Context, csr *x509.CertificateRequest, maxNames int, keyPolicy *goodkey.KeyPolicy, pa core.PolicyAuthority, regID int64) error {
normalizeCSR(csr)
key, ok := csr.PublicKey.(crypto.PublicKey)
if !ok {
return invalidPubKey
@ -73,7 +73,7 @@ func VerifyCSR(ctx context.Context, csr *x509.CertificateRequest, maxNames int,
if len(csr.DNSNames) == 0 && csr.Subject.CommonName == "" {
return invalidNoDNS
}
if forceCNFromSAN && csr.Subject.CommonName == "" {
if csr.Subject.CommonName == "" {
return invalidAllSANTooLong
}
if len(csr.Subject.CommonName) > maxCNLength {
@ -93,9 +93,9 @@ func VerifyCSR(ctx context.Context, csr *x509.CertificateRequest, maxNames int,
}
// normalizeCSR deduplicates and lowers the case of dNSNames and the subject CN.
// If forceCNFromSAN is true it will also hoist a dNSName into the CN if it is empty.
func normalizeCSR(csr *x509.CertificateRequest, forceCNFromSAN bool) {
if forceCNFromSAN && csr.Subject.CommonName == "" {
// It will also hoist a dNSName into the CN if it is empty.
func normalizeCSR(csr *x509.CertificateRequest) {
if csr.Subject.CommonName == "" {
var forcedCN string
// Promote the first SAN that is less than maxCNLength (if any)
for _, name := range csr.DNSNames {

38
csr/csr_test.go
View File

@ -167,7 +167,7 @@ func TestVerifyCSR(t *testing.T) {
}
for _, c := range cases {
err := VerifyCSR(context.Background(), c.csr, c.maxNames, c.keyPolicy, c.pa, true, c.regID)
err := VerifyCSR(context.Background(), c.csr, c.maxNames, c.keyPolicy, c.pa, c.regID)
test.AssertDeepEquals(t, c.expectedError, err)
}
}
@ -178,59 +178,34 @@ func TestNormalizeCSR(t *testing.T) {
cases := []struct {
name string
csr *x509.CertificateRequest
forceCN bool
expectedCN string
expectedNames []string
}{
{
"force CN, no explicit CN",
"no explicit CN",
&x509.CertificateRequest{DNSNames: []string{"a.com"}},
true,
"a.com",
[]string{"a.com"},
},
{
"force CN, explicit uppercase CN",
"explicit uppercase CN",
&x509.CertificateRequest{Subject: pkix.Name{CommonName: "A.com"}, DNSNames: []string{"a.com"}},
true,
"a.com",
[]string{"a.com"},
},
{
"no force CN, no explicit CN",
&x509.CertificateRequest{DNSNames: []string{"a.com"}},
false,
"",
[]string{"a.com"},
},
{
"no force CN, no explicit CN, duplicate SAN",
&x509.CertificateRequest{DNSNames: []string{"a.com", "a.com"}},
false,
"",
[]string{"a.com"},
},
{
"no force CN, explicit uppercase CN, uppercase SAN",
&x509.CertificateRequest{Subject: pkix.Name{CommonName: "A.com"}, DNSNames: []string{"B.com"}},
false,
"a.com",
[]string{"a.com", "b.com"},
},
{
"force CN, no explicit CN, too long leading SANs",
"no explicit CN, too long leading SANs",
&x509.CertificateRequest{DNSNames: []string{
tooLongString + ".a.com",
tooLongString + ".b.com",
"a.com",
"b.com",
}},
true,
"a.com",
[]string{"a.com", tooLongString + ".a.com", tooLongString + ".b.com", "b.com"},
},
{
"force CN, explicit CN, too long leading SANs",
"explicit CN, too long leading SANs",
&x509.CertificateRequest{
Subject: pkix.Name{CommonName: "A.com"},
DNSNames: []string{
@ -239,14 +214,13 @@ func TestNormalizeCSR(t *testing.T) {
"a.com",
"b.com",
}},
true,
"a.com",
[]string{"a.com", tooLongString + ".a.com", tooLongString + ".b.com", "b.com"},
},
}
for _, c := range cases {
t.Run(c.name, func(t *testing.T) {
normalizeCSR(c.csr, c.forceCN)
normalizeCSR(c.csr)
test.AssertEquals(t, c.expectedCN, c.csr.Subject.CommonName)
test.AssertDeepEquals(t, c.expectedNames, c.csr.DNSNames)
})

10
ra/ra.go
View File

@ -72,7 +72,6 @@ type RegistrationAuthorityImpl struct {
rlPolicies ratelimit.Limits
maxContactsPerReg int
maxNames int
forceCNFromSAN bool
reuseValidAuthz bool
orderLifetime time.Duration
@ -99,7 +98,6 @@ func NewRegistrationAuthorityImpl(
maxContactsPerReg int,
keyPolicy goodkey.KeyPolicy,
maxNames int,
forceCNFromSAN bool,
reuseValidAuthz bool,
authorizationLifetime time.Duration,
pendingAuthorizationLifetime time.Duration,
@ -178,7 +176,6 @@ func NewRegistrationAuthorityImpl(
maxContactsPerReg: maxContactsPerReg,
keyPolicy: keyPolicy,
maxNames: maxNames,
forceCNFromSAN: forceCNFromSAN,
reuseValidAuthz: reuseValidAuthz,
publisher: pubc,
caa: caaClient,
@ -651,8 +648,7 @@ func (ra *RegistrationAuthorityImpl) MatchesCSR(parsedCertificate *x509.Certific
if !core.KeyDigestEquals(parsedCertificate.PublicKey, csr.PublicKey) {
return berrors.InternalServerError("generated certificate public key doesn't match CSR public key")
}
if !ra.forceCNFromSAN && len(csr.Subject.CommonName) > 0 &&
parsedCertificate.Subject.CommonName != strings.ToLower(csr.Subject.CommonName) {
if parsedCertificate.Subject.CommonName != strings.ToLower(csr.Subject.CommonName) {
return berrors.InternalServerError("generated certificate CommonName doesn't match CSR CommonName")
}
// Sort both slices of names before comparison.
@ -956,7 +952,7 @@ func (ra *RegistrationAuthorityImpl) FinalizeOrder(ctx context.Context, req *rap
return nil, err
}
if err := csrlib.VerifyCSR(ctx, csrOb, ra.maxNames, &ra.keyPolicy, ra.PA, ra.forceCNFromSAN, *req.Order.RegistrationID); err != nil {
if err := csrlib.VerifyCSR(ctx, csrOb, ra.maxNames, &ra.keyPolicy, ra.PA, *req.Order.RegistrationID); err != nil {
// VerifyCSR returns berror instances that can be passed through as-is
// without wrapping.
return nil, err
@ -1047,7 +1043,7 @@ func (ra *RegistrationAuthorityImpl) FinalizeOrder(ctx context.Context, req *rap
// NewCertificate requests the issuance of a certificate.
func (ra *RegistrationAuthorityImpl) NewCertificate(ctx context.Context, req core.CertificateRequest, regID int64) (core.Certificate, error) {
// Verify the CSR
if err := csrlib.VerifyCSR(ctx, req.CSR, ra.maxNames, &ra.keyPolicy, ra.PA, ra.forceCNFromSAN, regID); err != nil {
if err := csrlib.VerifyCSR(ctx, req.CSR, ra.maxNames, &ra.keyPolicy, ra.PA, regID); err != nil {
return core.Certificate{}, berrors.MalformedError(err.Error())
}
// NewCertificate provides an order ID of 0, indicating this is a classic ACME

38
ra/ra_test.go
View File

@ -330,7 +330,7 @@ func initAuthorities(t *testing.T) (*DummyValidationAuthority, *sa.SQLStorageAut
ra := NewRegistrationAuthorityImpl(fc,
log,
stats,
1, testKeyPolicy, 100, true, false, 300*24*time.Hour, 7*24*time.Hour, nil, noopCAA{}, 0, ctp, nil, nil)
1, testKeyPolicy, 100, true, 300*24*time.Hour, 7*24*time.Hour, nil, noopCAA{}, 0, ctp, nil, nil)
ra.SA = ssa
ra.VA = va
ra.CA = ca
@ -2754,7 +2754,7 @@ func TestNewOrderExpiry(t *testing.T) {
}
func TestFinalizeOrder(t *testing.T) {
_, sa, ra, _, cleanUp := initAuthorities(t)
_, sa, ra, fc, cleanUp := initAuthorities(t)
defer cleanUp()
ra.orderLifetime = time.Hour
@ -2812,10 +2812,26 @@ func TestFinalizeOrder(t *testing.T) {
})
test.AssertNotError(t, err, "Could not add test order with finalized authz IDs, ready status")
// Swallowing errors here because the CSRPEM is hardcoded test data expected
// to parse in all instance
validCSRBlock, _ := pem.Decode(CSRPEM)
validCSR, _ := x509.ParseCertificateRequest(validCSRBlock.Bytes)
k, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
test.AssertNotError(t, err, "failed to generate test key")
validCSR := &x509.CertificateRequest{
PublicKey: k.Public(),
DNSNames: []string{"not-example.com", "www.not-example.com"},
}
validCSRDER, err := x509.CreateCertificateRequest(rand.Reader, validCSR, k)
test.AssertNotError(t, err, "failed to construct csr")
expectedCert := &x509.Certificate{
SerialNumber: big.NewInt(0),
Subject: pkix.Name{CommonName: "not-example.com"},
DNSNames: []string{"not-example.com", "www.not-example.com"},
PublicKey: k.Public(),
NotBefore: fc.Now(),
BasicConstraintsValid: true,
ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth, x509.ExtKeyUsageClientAuth},
}
certDER, err := x509.CreateCertificate(rand.Reader, expectedCert, expectedCert, k.Public(), k)
test.AssertNotError(t, err, "failed to construct test certificate")
ra.CA.(*mocks.MockCA).PEM = pem.EncodeToMemory(&pem.Block{Bytes: certDER, Type: "CERTIFICATE"})
fakeRegID := int64(0xB00)
@ -2880,7 +2896,7 @@ func TestFinalizeOrder(t *testing.T) {
Status: &pendingStatus,
Names: []string{"example.com"},
},
Csr: validCSR.Raw,
Csr: validCSRDER,
},
ExpectIssuance: false,
ExpectedErrMsg: `Order's status ("pending") is not acceptable for finalization`,
@ -2974,7 +2990,7 @@ func TestFinalizeOrder(t *testing.T) {
Name: "Order with correct authorizations, ready status",
OrderReq: &rapb.FinalizeOrderRequest{
Order: modernFinalOrder,
Csr: validCSR.Raw,
Csr: validCSRDER,
},
ExpectIssuance: true,
},
@ -3039,6 +3055,7 @@ func TestFinalizeOrderWithMixedSANAndCN(t *testing.T) {
template := &x509.Certificate{
SerialNumber: big.NewInt(12),
Subject: pkix.Name{CommonName: "not-example.com"},
DNSNames: []string{"www.not-example.com", "not-example.com"},
NotBefore: time.Now(),
BasicConstraintsValid: true,
@ -3054,7 +3071,7 @@ func TestFinalizeOrderWithMixedSANAndCN(t *testing.T) {
}
_, result := ra.FinalizeOrder(context.Background(), &rapb.FinalizeOrderRequest{Order: mixedOrder, Csr: mixedCSR})
test.AssertNotError(t, result, fmt.Sprintf("FinalizeOrder result was %#v, expected nil", result))
test.AssertNotError(t, result, "FinalizeOrder failed")
// Check that the order now has a serial for the issued certificate
updatedOrder, err := sa.GetOrder(
context.Background(),
@ -3087,6 +3104,7 @@ func TestFinalizeOrderWildcard(t *testing.T) {
KeyUsage: x509.KeyUsageDigitalSignature | x509.KeyUsageCertSign,
ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth, x509.ExtKeyUsageClientAuth},
BasicConstraintsValid: true,
Subject: pkix.Name{CommonName: "*.zombo.com"},
DNSNames: []string{"*.zombo.com"},
}
@ -3428,7 +3446,7 @@ func TestCTPolicyMeasurements(t *testing.T) {
ra := NewRegistrationAuthorityImpl(fc,
log,
stats,
1, testKeyPolicy, 0, true, false, 300*24*time.Hour, 7*24*time.Hour, nil, noopCAA{}, 0, ctp, nil, nil)
1, testKeyPolicy, 0, true, 300*24*time.Hour, 7*24*time.Hour, nil, noopCAA{}, 0, ctp, nil, nil)
ra.SA = ssa
ra.CA = ca

2
test/v2_integration.py
View File

@ -1112,7 +1112,7 @@ def test_long_san_no_cn():
except messages.Error as e:
if e.typ != "urn:ietf:params:acme:error:badCSR":
raise(Exception("Expected malformed type problem, got {0}".format(e.typ)))
if e.detail != "Error finalizing order :: issuing precertificate: CSR doesn't contain a SAN short enough to fit in CN":
if e.detail != "Error finalizing order :: CSR doesn't contain a SAN short enough to fit in CN":
raise(Exception("Problem detail did not match expected"))
def test_delete_unused_challenges():

1
wfe/wfe_test.go
View File

@ -904,7 +904,6 @@ func TestIssueCertificate(t *testing.T) {
testKeyPolicy,
100,
true,
false,
300*24*time.Hour,
7*24*time.Hour,
nil,