From 4fbcf862383a283ab8ca748aef4961ffc99fa870 Mon Sep 17 00:00:00 2001 From: Brian Smith Date: Thu, 13 Jul 2017 08:52:05 -1000 Subject: [PATCH] CA: Refactor invalid CSR tests into table-driven subtests. (#2868) This will make it easier to add new tests of this form and will also make it easier to adapt the tests to also test the precertificate + certificate issuance flow. --- ca/ca_test.go | 162 ++++++++++++++++---------------------------------- 1 file changed, 50 insertions(+), 112 deletions(-) diff --git a/ca/ca_test.go b/ca/ca_test.go index 4360418b6..222182ad9 100644 --- a/ca/ca_test.go +++ b/ca/ca_test.go @@ -49,28 +49,12 @@ var ( // * DNSNames = [none] NoNameCSR = mustRead("./testdata/no_name.der.csr") - // CSR generated by Go: - // * Random public key - // * CN = [none] - // * DNSNames = not-example.com, www.not-example.com, mail.example.com - TooManyNameCSR = mustRead("./testdata/too_many_names.der.csr") - - // CSR generated by Go: - // * Random public key -- 512 bits long - // * CN = (none) - // * DNSNames = not-example.com, www.not-example.com, mail.not-example.com - ShortKeyCSR = mustRead("./testdata/short_key.der.csr") - // CSR generated by Go: // * Random public key // * CN = CapiTalizedLetters.com // * DNSNames = moreCAPs.com, morecaps.com, evenMOREcaps.com, Capitalizedletters.COM CapitalizedCSR = mustRead("./testdata/capitalized_cn_and_san.der.csr") - // CSR generated by OpenSSL: - // Edited signature to become invalid. - WrongSignatureCSR = mustRead("./testdata/invalid_signature.der.csr") - // CSR generated by Go: // * Random public key // * CN = not-example.com @@ -101,18 +85,6 @@ var ( // * DNSNames = example.com, example2.com ECDSACSR = mustRead("./testdata/ecdsa.der.csr") - // CSR generated by Go: - // * Random RSA public key. - // * CN = [none] - // * DNSNames = [none] - NoNamesCSR = mustRead("./testdata/no_names.der.csr") - - // CSR generated by Go: - // * Random RSA public key. - // * CN = aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.com - // * DNSNames = [none] - LongCNCSR = mustRead("./testdata/long_cn.der.csr") - log = blog.UseMock() ) @@ -450,7 +422,47 @@ func TestOCSP(t *testing.T) { test.AssertEquals(t, parsedNewCertOcspResp.SerialNumber.Cmp(parsedNewCert.SerialNumber), 0) } -func TestNoHostnames(t *testing.T) { +func TestInvalidCSRs(t *testing.T) { + testCases := []struct { + name string + csrPath string + errorMessage string + }{ + // Test that the CA rejects CSRs that have no names. + // + // CSR generated by Go: + // * Random RSA public key. + // * CN = [none] + // * DNSNames = [none] + {"RejectNoHostnames", "./testdata/no_names.der.csr", "Issued certificate with no names"}, + + // Test that the CA rejects CSRs that have too many names. + // + // CSR generated by Go: + // * Random public key + // * CN = [none] + // * DNSNames = not-example.com, www.not-example.com, mail.example.com + {"RejectTooManyHostnames", "./testdata/too_many_names.der.csr", "Issued certificate with too many names"}, + + // Test that the CA rejects CSRs that have public keys that are too short. + // + // CSR generated by Go: + // * Random public key -- 512 bits long + // * CN = (none) + // * DNSNames = not-example.com, www.not-example.com, mail.not-example.com + {"RejectShortKey", "./testdata/short_key.der.csr", "Issued a certificate with too short a key."}, + + // CSR generated by Go: + // * Random RSA public key. + // * CN = aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.com + // * DNSNames = [none] + {"RejectLongCommonName", "./testdata/long_cn.der.csr", "Issued a certificate with a CN over 64 bytes."}, + + // CSR generated by OpenSSL: + // Edited signature to become invalid. + {"RejectWrongSignature", "./testdata/invalid_signature.der.csr", "Issued a certificate based on a CSR with an invalid signature."}, + } + testCtx := setup(t) ca, err := NewCertificateAuthorityImpl( testCtx.caConfig, @@ -463,30 +475,15 @@ func TestNoHostnames(t *testing.T) { ca.PA = testCtx.pa ca.SA = &mockSA{} - csr, _ := x509.ParseCertificateRequest(NoNamesCSR) - _, err = ca.IssueCertificate(ctx, *csr, 1001) - test.AssertError(t, err, "Issued certificate with no names") - test.Assert(t, berrors.Is(err, berrors.Malformed), "Incorrect error type returned") -} - -func TestRejectTooManyNames(t *testing.T) { - testCtx := setup(t) - ca, err := NewCertificateAuthorityImpl( - testCtx.caConfig, - testCtx.fc, - testCtx.stats, - testCtx.issuers, - testCtx.keyPolicy, - testCtx.logger) - test.AssertNotError(t, err, "Failed to create CA") - ca.PA = testCtx.pa - ca.SA = &mockSA{} - - // Test that the CA rejects a CSR with too many names - csr, _ := x509.ParseCertificateRequest(TooManyNameCSR) - _, err = ca.IssueCertificate(ctx, *csr, 1001) - test.AssertError(t, err, "Issued certificate with too many names") - test.Assert(t, berrors.Is(err, berrors.Malformed), "Incorrect error type returned") + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + serializedCSR := mustRead(testCase.csrPath) + csr, _ := x509.ParseCertificateRequest(serializedCSR) + _, err = ca.IssueCertificate(ctx, *csr, 1001) + test.AssertError(t, err, testCase.errorMessage) + test.Assert(t, berrors.Is(err, berrors.Malformed), "Incorrect error type returned") + }) + } } func TestRejectValidityTooLong(t *testing.T) { @@ -514,25 +511,6 @@ func TestRejectValidityTooLong(t *testing.T) { test.Assert(t, berrors.Is(err, berrors.InternalServer), "Incorrect error type returned") } -func TestShortKey(t *testing.T) { - testCtx := setup(t) - ca, err := NewCertificateAuthorityImpl( - testCtx.caConfig, - testCtx.fc, - testCtx.stats, - testCtx.issuers, - testCtx.keyPolicy, - testCtx.logger) - ca.PA = testCtx.pa - ca.SA = &mockSA{} - - // Test that the CA rejects CSRs that would expire after the intermediate cert - csr, _ := x509.ParseCertificateRequest(ShortKeyCSR) - _, err = ca.IssueCertificate(ctx, *csr, 1001) - test.AssertError(t, err, "Issued a certificate with too short a key.") - test.Assert(t, berrors.Is(err, berrors.Malformed), "Incorrect error type returned") -} - func TestAllowNoCN(t *testing.T) { testCtx := setup(t) ca, err := NewCertificateAuthorityImpl( @@ -574,46 +552,6 @@ func TestAllowNoCN(t *testing.T) { test.AssertDeepEquals(t, actual, expected) } -func TestLongCommonName(t *testing.T) { - testCtx := setup(t) - ca, err := NewCertificateAuthorityImpl( - testCtx.caConfig, - testCtx.fc, - testCtx.stats, - testCtx.issuers, - testCtx.keyPolicy, - testCtx.logger) - ca.PA = testCtx.pa - ca.SA = &mockSA{} - - csr, _ := x509.ParseCertificateRequest(LongCNCSR) - _, err = ca.IssueCertificate(ctx, *csr, 1001) - test.AssertError(t, err, "Issued a certificate with a CN over 64 bytes.") - test.Assert(t, berrors.Is(err, berrors.Malformed), "Incorrect error type returned") -} - -func TestWrongSignature(t *testing.T) { - testCtx := setup(t) - testCtx.caConfig.MaxNames = 3 - ca, err := NewCertificateAuthorityImpl( - testCtx.caConfig, - testCtx.fc, - testCtx.stats, - testCtx.issuers, - testCtx.keyPolicy, - testCtx.logger) - ca.PA = testCtx.pa - ca.SA = &mockSA{} - - // x509.ParseCertificateRequest() does not check for invalid signatures... - csr, _ := x509.ParseCertificateRequest(WrongSignatureCSR) - - _, err = ca.IssueCertificate(ctx, *csr, 1001) - if err == nil { - t.Fatalf("Issued a certificate based on a CSR with an invalid signature.") - } -} - func TestProfileSelection(t *testing.T) { testCtx := setup(t) testCtx.caConfig.MaxNames = 3