square / certigo

A utility to examine and validate certificates in a variety of formats
Apache License 2.0
940 stars 71 forks source link

Unit tests fail on macOS with Go 1.18 because of 1024-bit RSA test certificate #264

Closed bdd closed 2 years ago

bdd commented 2 years ago

Problem:

TestConnect fails on macOS when using Go 1.18

% ~/go/bin/go1.18.1 test ./...
?       github.com/square/certigo   [no test files]
--- FAIL: TestConnect (0.02s)
    cli_test.go:210:
            Error Trace:    cli_test.go:210
            Error:          Not equal:
                            expected: "** TLS Connection **\nVersion: TLS 1.3\nCipher Suite: AES_128_GCM_SHA256 cipher\n\n** CERTIFICATE 1 **\nSerial: 64483185769360960274258770740570494187\nValid: 1970-01-01 00:00 UTC to 2084-01-29 16:00 UTC\nSignature: SHA256-RSA (self-signed)\nSubject Info:\n\tOrganization: Acme Co\nIssuer Info:\n\tOrganization: Acme Co\nBasic Constraints: CA:true\nKey Usage:\n\tDigital Signature\n\tKey Encipherment\n\tCert Sign\nExtended Key Usage:\n\tServer Auth\nDNS Names:\n\texample.com\nIP Addresses:\n\t127.0.0.1, ::1\nWarnings:\n\tSize of RSA key should be at least 2048 bits\n\nFailed to verify certificate chain:\n\tx509: certificate signed by unknown authority\n** TLS Connection **\nVersion: TLS 1.3\nCipher Suite: AES_128_GCM_SHA256 cipher\n\n** CERTIFICATE 1 **\nSerial: 64483185769360960274258770740570494187\nValid: 1970-01-01 00:00 UTC to 2084-01-29 16:00 UTC\nSignature: SHA256-RSA (self-signed)\nSubject Info:\n\tOrganization: Acme Co\nIssuer Info:\n\tOrganization: Acme Co\nBasic Constraints: CA:true\nKey Usage:\n\tDigital Signature\n\tKey Encipherment\n\tCert Sign\nExtended Key Usage:\n\tServer Auth\nDNS Names:\n\texample.com\nIP Addresses:\n\t127.0.0.1, ::1\nWarnings:\n\tSize of RSA key should be at least 2048 bits\n\nFailed to verify certificate chain:\n\tx509: certificate signed by unknown authority\n"
                            actual  : "** TLS Connection **\nVersion: TLS 1.3\nCipher Suite: AES_128_GCM_SHA256 cipher\n\n** CERTIFICATE 1 **\nSerial: 64483185769360960274258770740570494187\nValid: 1970-01-01 00:00 UTC to 2084-01-29 16:00 UTC\nSignature: SHA256-RSA (self-signed)\nSubject Info:\n\tOrganization: Acme Co\nIssuer Info:\n\tOrganization: Acme Co\nBasic Constraints: CA:true\nKey Usage:\n\tDigital Signature\n\tKey Encipherment\n\tCert Sign\nExtended Key Usage:\n\tServer Auth\nDNS Names:\n\texample.com\nIP Addresses:\n\t127.0.0.1, ::1\nWarnings:\n\tSize of RSA key should be at least 2048 bits\n\nFailed to verify certificate chain:\n\tx509: “Acme Co” certificate is using a broken key size\n** TLS Connection **\nVersion: TLS 1.3\nCipher Suite: AES_128_GCM_SHA256 cipher\n\n** CERTIFICATE 1 **\nSerial: 64483185769360960274258770740570494187\nValid: 1970-01-01 00:00 UTC to 2084-01-29 16:00 UTC\nSignature: SHA256-RSA (self-signed)\nSubject Info:\n\tOrganization: Acme Co\nIssuer Info:\n\tOrganization: Acme Co\nBasic Constraints: CA:true\nKey Usage:\n\tDigital Signature\n\tKey Encipherment\n\tCert Sign\nExtended Key Usage:\n\tServer Auth\nDNS Names:\n\texample.com\nIP Addresses:\n\t127.0.0.1, ::1\nWarnings:\n\tSize of RSA key should be at least 2048 bits\n\nFailed to verify certificate chain:\n\tx509: “Acme Co” certificate is using a broken key size\n"

                            Diff:
                            --- Expected
                            +++ Actual
                            @@ -27,3 +27,3 @@
                             Failed to verify certificate chain:
                            -   x509: certificate signed by unknown authority
                            +   x509: “Acme Co” certificate is using a broken key size
                             ** TLS Connection **
                            @@ -55,3 +55,3 @@
                             Failed to verify certificate chain:
                            -   x509: certificate signed by unknown authority
                            +   x509: “Acme Co” certificate is using a broken key size

            Test:           TestConnect
FAIL
FAIL    github.com/square/certigo/cli   0.233s

Dive:

Where does this certificate is using a broken key size error come from?

Looks like from from Apple Security Framework https://cs.github.com/apple-open-source/macos/blob/4c64a93f78278a48fd0c9bce26737010c16668e6/Security/OSX/sec/Security/SecFrameworkStrings.h#L246.

Apple's App Transport Security (ATS) on all platforms now requires:

The server certificate must be signed with either a Rivest-Shamir-Adleman (RSA) key of at least 2048 bits, or an Elliptic-Curve Cryptography (ECC) key of at least 256 bits.

Go uses Apple Security Framework now?

Go 1.18 switched TLS verification path to platform APIs for macOS and iOS. From: Go 1.18 Release Notes:

Certificate.Verify now uses platform APIs to verify certificate validity on macOS and iOS when it is called with a nil VerifyOpts.Roots or when using the root pool returned from SystemCertPool.

Next:

Update localhostKey to at least 2048-bits and generating a new localhostCert with it in cli/cli_test.go. https://github.com/square/certigo/blob/41b5b73f75ee1b5817a8d32951f9dd9c12324391/cli/cli_test.go#L19

bdd commented 2 years ago

Well, simply updating the test cert to RSA 2048 will drop the lumped in test case for key size warning emitted.

Warnings:
        Size of RSA key should be at least 2048 bits

Moreover, the chain verification failure messages are not different between platforms:

e.g.

Failed to verify certificate chain:
-   x509: certificate signed by unknown authority
+   x509: “Acme Co” certificate is not trusted

The new error, citing the issuer name is coming from Apple Security Framework, particularly this format string with its fancy quotes.

So the PR would be more involved than new key, new cert and change expected output.

jdtw commented 2 years ago

Thanks for the detailed writeup, we'll take a look!

jdtw commented 2 years ago

I decided that we should just change the test to successfully verify the cert chain instead of relying on a platform-specific error string.