diff --git a/ca/certificate-authority.go b/ca/certificate-authority.go index 573f02b7e..22cf53501 100644 --- a/ca/certificate-authority.go +++ b/ca/certificate-authority.go @@ -169,15 +169,17 @@ func loadIssuerKey(filename string) (issuerKey crypto.Signer, err error) { return } -func dupeNames(names []string) bool { +func uniqueNames(names []string) (unique []string) { nameMap := make(map[string]int, len(names)) for _, name := range names { nameMap[name] = 1 } - if len(names) != len(nameMap) { - return true + + unique = make([]string, 0, len(nameMap)) + for name := range nameMap { + unique = append(unique, name) } - return false + return } // GenerateOCSP produces a new OCSP response and returns it @@ -236,12 +238,14 @@ func (ca *CertificateAuthorityImpl) IssueCertificate(csr x509.CertificateRequest return emptyCert, fmt.Errorf("Invalid public key in CSR.") } - // XXX Take in authorizations and verify that union covers CSR? // Pull hostnames from CSR - hostNames := csr.DNSNames // DNSNames + CN from CSR - var commonName string + // Authorization is checked by the RA + commonName := "" + hostNames := make([]string, len(csr.DNSNames)) + copy(hostNames, csr.DNSNames) if len(csr.Subject.CommonName) > 0 { commonName = csr.Subject.CommonName + hostNames = append(hostNames, csr.Subject.CommonName) } else if len(hostNames) > 0 { commonName = hostNames[0] } else { @@ -250,11 +254,8 @@ func (ca *CertificateAuthorityImpl) IssueCertificate(csr x509.CertificateRequest return emptyCert, err } - if dupeNames(hostNames) { - err = errors.New("Cannot issue a certificate with duplicate DNS names.") - ca.log.WarningErr(err) - return emptyCert, err - } + // Collapse any duplicate names. Note that this operation may re-order the names + hostNames = uniqueNames(hostNames) if ca.NotAfter.Before(time.Now().Add(ca.ValidityPeriod)) { err = errors.New("Cannot issue a certificate that expires after the intermediate certificate.") diff --git a/ca/certificate-authority_test.go b/ca/certificate-authority_test.go index 9489ff258..21b3e9566 100644 --- a/ca/certificate-authority_test.go +++ b/ca/certificate-authority_test.go @@ -117,8 +117,8 @@ var CA_CERT_PEM = "-----BEGIN CERTIFICATE-----\n" + // CSR generated by Go: // * Random public key -// * CN = example.com -// * DNSNames = example.com, www.example.com +// * CN = not-example.com +// * DNSNames = not-example.com, www.not-example.com var CN_AND_SAN_CSR_HEX = "308202a130820189020100301a311830160603550403130f6e6f742d6578" + "616d706c652e636f6d30820122300d06092a864886f70d01010105000382" + "010f003082010a0282010100e56ccbe37003c150202e6f543f9eb1d0e590" + @@ -225,22 +225,31 @@ var NO_NAME_CSR_HEX = "308202523082013a020100300d310b300906035504061302555330820 // CSR generated by Go: // * Random public key -// * CN = example.com +// * CN = [none] // * DNSNames = a.example.com, a.example.com -var DUPE_NAME_CSR_HEX = "3082018d3081f90201003016311430120603550403130b6578616d706c65" + - "2e636f6d30819f300d06092a864886f70d010101050003818d0030818902" + - "818100cc4a0cf2cf67811e4457fe1106597013e84be141c583b663f2ef6d" + - "a0c9254ca4c37fcd1945fdddc6db66f395c679de33501d333efd60d941d5" + - "a32d29a1e5af6da853ba28419b471081a8476d7bdf7159cc09606eec807f" + - "da89586ebee0e46a5f53a14c2210a934e92afd314c0bc1b6946afce63a21" + - "0b6eac62eca728efbb36c70203010001a03a303806092a864886f70d0109" + - "0e312b302930270603551d110420301e820d612e6578616d706c652e636f" + - "6d820d612e6578616d706c652e636f6d300b06092a864886f70d01010b03" + - "818100604965228739c63f5d94d29295a7c327f70c08f361d4873166f112" + - "d420ca424d9a86cfb49483cf54090d1d81e56b1aeea09cafd783e7ef4fb8" + - "fdbd43e1918e474abb2ea8962960c5c77ac5be5cbf67e515d8234ca7fe4e" + - "5b7c0134e95b77a43a6b5789ff97b3262f949e75690314e417c4c2bd3d1f" + - "7bedb21db1dd5dd4f71b82" +var DUPE_NAME_CSR_HEX = "308202943082017c020100300d310b300906035504061302555330820122" + + "300d06092a864886f70d01010105000382010f003082010a0282010100ee" + + "7d298c2a8237dd84e75e71dcfbbf8e1124327b103b01f3a99bc76b29be64" + + "55329dc523ad1372ed12853dc74a775f2c79d1e4e28ae2a3ce69b78ec161" + + "76b31056a72b5b87bcd7501ab9f15e83346e788c085e472718494849c012" + + "ff6c25f12c040a0c68e99a46e8646523bf2b4103dcc7c4e583c6bf02c896" + + "09b55e470beb15eaefc0bb6d57aca39e75312833e87faf2ba33e12a3204c" + + "a19e5a1016c85b750619be9cb76fb93e653f3169b873dce42deab328a034" + + "57454fd8e135e125bdb4529144a34bcc5ab20f25c6b33300ab2ad994d251" + + "9261920da78e05b1c8847852e93a74000e7b29304ef2cccfa5315888150c" + + "14e9d51e2c343de44e6bd2903740ef0203010001a042304006092a864886" + + "f70d01090e31333031302f0603551d11042830268211612e6e6f742d6578" + + "616d706c652e636f6d8211612e6e6f742d6578616d706c652e636f6d300d" + + "06092a864886f70d01010b050003820101001358a156338bacd21bd66f35" + + "83bf26afbaa6750f1d5d8e6285d062d5ae4f379000cd53568e8ef11bb542" + + "97d906879614960266fece8f9060881c3b0ec34a1bb4b28f62cc444e5831" + + "09c278d21265b2ef6f120ae8bc2554ba46b60e8d46a8cd3093ea6b72cb42" + + "9d2d6072273e4a0c1a5ec3316cc2495a2776698f12cd4f4226f4b3e85f87" + + "0b0f32785cf823b9dfa7116eec8a6f9bdc5cf01c1fff46afbb55e636ced4" + + "0ccdf4cab09e7717aa2a67c5573590bb5d461cb13cc57b8451314ed3613e" + + "21757aeb846f5b33f107a01f93a718b3a83539562e6486a72f114df898ed" + + "c34e38d9bbdb96d0b8ab243c46da274cece70deb771985c477098468063a" + + "8400ecb6" // CFSSL config const hostPort = "localhost:9000" @@ -457,32 +466,50 @@ func TestRejectNoName(t *testing.T) { if err == nil { t.Errorf("CA improperly agreed to create a certificate with no name") } +} - // Test that the CA rejects CSRs with duplicate names - csrDER, _ = hex.DecodeString(DUPE_NAME_CSR_HEX) - csr, _ = x509.ParseCertificateRequest(csrDER) - _, err = ca.IssueCertificate(*csr, 1) - if err == nil { - t.Errorf("CA improperly agreed to create a certificate with duplicate names") +func TestDeduplication(t *testing.T) { + cadb, storageAuthority, caConfig := setup(t) + ca, err := NewCertificateAuthorityImpl(cadb, caConfig) + test.AssertNotError(t, err, "Failed to create CA") + ca.SA = storageAuthority + + // Test that the CA collapses duplicate names + csrDER, _ := hex.DecodeString(DUPE_NAME_CSR_HEX) + csr, _ := x509.ParseCertificateRequest(csrDER) + cert, err := ca.IssueCertificate(*csr, 1) + test.AssertNotError(t, err, "Failed to gracefully handle a CSR with duplicate names") + if err != nil { + return } + parsedCert, err := x509.ParseCertificate(cert.DER) + test.AssertNotError(t, err, "Error parsing certificate produced by CA") + if err != nil { + return + } + + correctName := "a.not-example.com" + correctNames := len(parsedCert.DNSNames) == 1 && + parsedCert.DNSNames[0] == correctName && + parsedCert.Subject.CommonName == correctName + test.Assert(t, correctNames, "Incorrect set of names in deduplicated certificate") +} + +func TestRejectValidityTooLong(t *testing.T) { + cadb, storageAuthority, caConfig := setup(t) + ca, err := NewCertificateAuthorityImpl(cadb, caConfig) + test.AssertNotError(t, err, "Failed to create CA") + ca.SA = storageAuthority + // Test that the CA rejects CSRs that would expire after the intermediate cert - csrDER, _ = hex.DecodeString(NO_CN_CSR_HEX) - csr, _ = x509.ParseCertificateRequest(csrDER) + csrDER, _ := hex.DecodeString(NO_CN_CSR_HEX) + csr, _ := x509.ParseCertificateRequest(csrDER) ca.NotAfter = time.Now() _, err = ca.IssueCertificate(*csr, 1) test.AssertEquals(t, err.Error(), "Cannot issue a certificate that expires after the intermediate certificate.") } -func TestDupeNames(t *testing.T) { - unique := []string{"a", "b"} - notUnique := []string{"a", "a"} - - test.Assert(t, !dupeNames([]string{}), "Empty list can't contain duplicates") - test.Assert(t, !dupeNames(unique), "Unique list doesn't have duplicates") - test.Assert(t, dupeNames(notUnique), "Non-unique list does have duplicates") -} - func TestShortKey(t *testing.T) { cadb, storageAuthority, caConfig := setup(t) ca, err := NewCertificateAuthorityImpl(cadb, caConfig)