1. openvpn
  2. Commit 21a540f92bf65f39eb92967476eba0bcd2a34ef6
Browse code

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.

Signed-off-by: Steffan Karger <steffan@karger.me>
Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <1495285075-4957-1-git-send-email-steffan@karger.me>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14711.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>

Steffan Karger authored on 2017/05/20 21:57:55
Showing 2 changed files
Changes.rst
History View file @ 21a540f
... ... 
@@ -305,10 +305,26 @@ Maintainer-visible changes
305 305
306 306 
 Version 2.4.3
307 307 
 =============
308 
+
309 
+User-visible Changes
310 
+--------------------
308 311 
 - ``--verify-hash`` can now take an optional flag which changes the hashing
309 312 
   algorithm. It can be either SHA1 or SHA256.  The default if not provided is
310 313 
   SHA1 to preserve backwards compatibility with existing configurations.
311 314
315 
+Bugfixes
316 
+--------
317 
+- Fix fingerprint calculation in mbed TLS builds.  This means that mbed TLS users
318 
+  of OpenVPN 2.4.0, 2.4.1 and 2.4.2 that rely on the values of the
319 
+  ``tls_digest_*`` env vars, or that use `--verify-hash` will have to change
320 
+  the fingerprint values they check against.  The security impact of the
321 
+  incorrect calculation is very minimal; the last few bytes (max 4, typically
322 
+  4) are not verified by the fingerprint.  We expect no real-world impact,
323 
+  because users that used this feature before will notice that it has suddenly
324 
+  stopped working, and users that didn't will notice that connection setup
325 
+  fails if they specify correct fingerprints.
326 
+
327 
+
312 328 
 Version 2.4.1
313 329 
 =============
314 330 
 - ``--remote-cert-ku`` now only requires the certificate to have at least the
src/openvpn/ssl_verify_mbedtls.c
History View file @ 21a540f
... ... 
@@ -208,7 +208,7 @@ x509_get_fingerprint(const mbedtls_md_info_t *md_info, mbedtls_x509_crt *cert,
208 208 
 {
209 209 
     const size_t md_size = mbedtls_md_get_size(md_info);
210 210 
     struct buffer fingerprint = alloc_buf_gc(md_size, gc);
211 
-    mbedtls_md(md_info, cert->raw.p, cert->tbs.len, BPTR(&fingerprint));
211 
+    mbedtls_md(md_info, cert->raw.p, cert->raw.len, BPTR(&fingerprint));
212 212 
     ASSERT(buf_inc_len(&fingerprint, md_size));
213 213 
     return fingerprint;
214 214 
 }