Fix mbedtls fingerprint calculation
PATCH 1/1 Fix mbedtls fingerprint calculation
Commit 'Migrate to mbed TLS 2.x' (86d8cd68) introduced a bug in mbedtls builds where we would calculate the certificate fingerprint over the (too-short) 'to-be-signed' length of the certificate, rather than over the certificate including the signature. Fix that.
The security impact of the incorrect calculation is very minimal; the last few bytes (max 4, typically 4) are not verified by the fingerprint. We expect no real-world impact, because users that used this feature before will notice that it has suddenly stopped working, and users that didn't will notice that connection setup fails.
Even if the user managed to somehow extract the incorrect hash (e.g. by reading out the tls_digest_* env vars using a --tls-verify script), the impact is miminal: the last 4 bytes must still be properly signed by the CA, and typically contain extension fields, or the last bytes of the public key (which are hard to choose). The most important bits of the certificate were always checked: the version, serial, signature algorithm, issuer, validity and subject.
--- Changes.rst | 16 ++++++++++++++++ src/openvpn/ssl_verify_mbedtls.c | 2 +- 2 files changed, 17 insertions(+), 1 deletion(-)
Changes.rst +16 -0
@@ -339,3 +339,19 @@ Security causing a packet id roll over. An attack is rather inefficient; a peer would need to get us to send at least about 196 GB of data. (OSTIF/Quarkslab audit finding 5.2, CVE-2017-7479) + + +Version 2.4.2 +============= + +Bugfixes +-------- +- Fix fingerprint calculation in mbed TLS builds. This means that mbed TLS users + of OpenVPN 2.4.0, 2.4.1 and 2.4.2 that rely on the values of the + ``tls_digest_*`` env vars, or that use `--verify-hash` will have to change + the fingerprint values they check against. The security impact of the + incorrect calculation is very minimal; the last few bytes (max 4, typically + 4) are not verified by the fingerprint. We expect no real-world impact, + because users that used this feature before will notice that it has suddenly + stopped working, and users that didn't will notice that connection setup + fails if they specify correct fingerprints.
src/openvpn/ssl_verify_mbedtls.c +1 -1
@@ -209,7 +209,7 @@ x509_get_fingerprint(const mbedtls_md_info_t *md_info, mbedtls_x509_crt *cert, {const size_t md_size =mbedtls_md_get_size (md_info);struct buffer fingerprint =alloc_buf_gc (md_size, gc); -mbedtls_md (md_info, cert->raw.p, cert->tbs.len,BPTR (&fingerprint)); +mbedtls_md (md_info, cert->raw.p, cert->raw.len,BPTR (&fingerprint));ASSERT (buf_inc_len (&fingerprint, md_size));return fingerprint; }
Signature · 1 line
2.7.4
Am 20.05.17 um 14:57 schrieb Steffan Karger:
Quoted 11 lines
Commit 'Migrate to mbed TLS 2.x' (
86d8cd68) introduced a bug in mbedtls builds where we would calculate the certificate fingerprint over the (too-short) 'to-be-signed' length of the certificate, rather than over the certificate including the signature. Fix that.The security impact of the incorrect calculation is very minimal; the last few bytes (max 4, typically 4) are not verified by the fingerprint. We expect no real-world impact, because users that used this feature before will notice that it has suddenly stopped working, and users that didn't will notice that connection setup fails.
Quoted 8 lines
Even if the user managed to somehow extract the incorrect hash (e.g. by reading out the tls_digest_* env vars using a --tls-verify script), the impact is miminal: the last 4 bytes must still be properly signed by the CA, and typically contain extension fields, or the last bytes of the public key (which are hard to choose). The most important bits of the certificate were always checked: the version, serial, signature algorithm, issuer, validity and subject.
ACK and I agreee that we should not include backbwards compatible behaviour for the buggy hash.
Arne
Your patch has been applied to the master and release/2.4 branch.
Changes.rst has been adjusted to put this under the 2.4.3 heading.
commit 21a540f92bf65f39eb92967476eba0bcd2a34ef6 (master) commit d7b7f93e526a21919ad54ce8cce41c3acc39128d (release/2.4)
Author: Steffan Karger Date: Sat May 20 14:57:55 2017 +0200
Fix mbedtls fingerprint calculation
Signed-off-by: Steffan Karger <[email protected]...>
Acked-by: Arne Schwabe <[email protected]...>
Message-Id: <[email protected]...>
URL: https://www.mail-archive.com/[email protected]/msg14711.html
Signed-off-by: Gert Doering <[email protected]...>--
kind regards,
Gert Doering