Backport 2.7: Fix compat.sh with ubuntu 16.04 gnutls 2.7 #3594
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Recent GnuTLS packages on Ubuntu 16.04 have them disabled.
From /usr/share/doc/libgnutls30/changelog.Debian.gz:
gnutls28 (3.4.10-4ubuntu1.5) xenial-security; urgency=medium
* SECURITY UPDATE: Lucky-13 issues
[...]
- debian/patches/CVE-2018-1084x-4.patch: hmac-sha384 and sha256
ciphersuites were removed from defaults in lib/gnutls_priority.c,
tests/priorities.c.
Since we do want to test the ciphersuites, explicitly re-enable them in the
server's priority string. (This is a no-op with versions of GnuTLS where those
are already enabled by default.)
Signed-off-by: Manuel Pégourié-Gonnard <[email protected]>
Replace server2.crt with server2-sha256.crt which, as the name implies, is just the SHA-256 version of the same certificate. Replace server1.crt with cert_sha256.crt which, as the name doesn't imply, is associated with the same key and just have a slightly different Subject Name, which doesn't matter in this instance. The other certificates used in this script (server5.crt and server6.crt) are already signed with SHA-256. This change is motivated by the fact that recent versions of GnuTLS (or older versions with the Debian patches) reject SHA-1 in certificates by default, as they should. There are options to still accept it (%VERIFY_ALLOW_BROKEN and %VERIFY_ALLOW_SIGN_WITH_SHA1) but: - they're not available in all versions that reject SHA-1-signed certs; - moving to SHA-2 just seems cleaner anyway. Signed-off-by: Manuel Pégourié-Gonnard <[email protected]>
After the changes of certificates, it's no longer needed. Signed-off-by: Manuel Pégourié-Gonnard <[email protected]>
For some reason, RIPEMD160, SHA224 and SHA384 were not supported. This fixes the build recipes for tests/data_files/cert_sha224.crt and tests/data_files/cert_sha384.crt . Signed-off-by: Gilles Peskine <[email protected]>
The test certificate used for clients in compat.sh, cert_sha256.crt, had the issuer ON and CN encoded as UTF8String, but the corresponding CA certificate test-ca_cat12.crt had them encoded as PrintableString. The strings matched, which is sufficient according to RFC 5280 §7.1 and RFC 4518 §2.1. However, GnuTLS 3.4.10 requires the strings to have the same encoding, so it did not accept that the certificate issued by UTF8String "PolarSSL Test CA" was validly issued by the PrintableString "PolarSSL Test CA" CA. ebc1f40, merged via Mbed-TLS#1641 and released in Mbed TLS 2.14, updated these certificates. 4f928c0 merged, via Mbed-TLS#2418 fixed this in the 2.7 LTS branch for the SHA-1 certificate which was used at the time. The present commit applies the same fix for the SHA-256 certificate that is now in use. For uniformity, this commit regenerates all the cert_*.crt. Signed-off-by: Gilles Peskine <[email protected]>
server2-sha256.crt had the issuer ON and CN encoded as UTF8String, but the corresponding CA certificate test-ca_cat12.crt had them encoded as PrintableString. The strings matched, which is sufficient according to RFC 5280 §7.1 and RFC 4518 §2.1. However, GnuTLS 3.4.10 requires the strings to have the same encoding, so it did not accept that the UTF8String "PolarSSL Test CA" certificate was signed by the PrintableString "PolarSSL Test CA" CA. Since Mbed TLS 2.14 (specifically ebc1f40 merged via Mbed-TLS#1641), server2-sha256.crt is generated by Mbed TLS's own cert_write program, which emits a PrintableString. In older versions, this file was generated by OpenSSL, which started emitting UTF8String at some point. 4f928c0 merged via Mbed-TLS#2418 fixed this for the SHA-1 certificate which was used at the time. The present commit applies the same fix for the SHA-256 certificate that is now in use. Signed-off-by: Gilles Peskine <[email protected]>
gilles-peskine-arm
added
bug
needs-review
This was referenced Aug 21, 2020
mpg
previously approved these changes
Aug 24, 2020
| else if( strcmp( q, "MD5" ) == 0 ) | ||
| opt.md = MBEDTLS_MD_MD5; | ||
| else | ||
| const mbedtls_md_info_t *md_info = |
hanno-becker
suggested changes
Aug 24, 2020
There was a problem hiding this comment.
library/certs.c needs to be updated to contain the new server2-sha256.crt certificate. You might want to use #2596 for that. Other than that, the PR looks good to me.
|
@gilles-peskine-arm @mpg https://github.com/ARMmbed/mbedtls/blob/mbedtls-2.7/library/certs.c#L207 suggests that |
hanno-becker
previously approved these changes
Aug 24, 2020
Before this commit, certs.c had a copy of a different version of tests/data_files/server2-sha256.crt (from the then development branch) which was generated by cert_write. Update certs.c with the new tests/data_files/server2-sha256.crt which is also generated by cert_write. The new copy has the same size as the old copy so there is no concern about existing application binaries relying on the size. (The old tests/data_files/server2-sha256.crt had a different size because it had been generated by openssl and so had slightly different content.) Signed-off-by: Gilles Peskine <[email protected]>
gilles-peskine-arm
dismissed stale reviews from hanno-becker and mpg
via
1323fba
gilles-peskine-arm
force-pushed
the
fix-compat.sh-with-ubuntu-16.04-gnutls-2.7
branch
from
e5bac40 to
1323fba
Compare
|
I rewrote the last commit because I'd accidentally committed a test version of Regarding the conundrum of changing the certificate size vs keeping the file in |
hanno-becker
approved these changes
Aug 25, 2020
gilles-peskine-arm
removed
the
needs-review
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix #3520 (travis build) for 2.7. Backport of #3590.
2.7 required more work due to a limitation in older versions of GnuTLS including our default one (3.4.10) but not the one we use as
GNUTLS_NEXT(3.6.5): it compares distinguished names by their representation, so aPrintableStringcannot be equal to aUTF8String. In#1641 we patched some of our code and test data to cope with this limitation. In #2418 we made a partial backport of the test data patches, but we didn't update the SHA-256 client certificate. This PR updates the whole
tests/data_files/cert_${hash}.crtfamily to havePrintableStringDN for the issuer.The commit “cert_write: support all hash algorithms” is included here because it was necessary to regenerate the certificates. I made a separate PR for some other minor things that I stumbled on while working on this: #3597.