From: Steffan Karger <steffan.karger@fox-it.com>
Date: Tue, 9 May 2017 08:12:43 +0000 (+0200)
Subject: mbedtls: correctly check return value in pkcs11_certificate_dn()
X-Git-Tag: v2.4.2~10
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=1ebd3ade5f3fcdefa40790f2e9d16c473bac370a;p=thirdparty%2Fopenvpn.git

mbedtls: correctly check return value in pkcs11_certificate_dn()

mbedtls_x509_dn_gets() would not always return -1 error, which could cause
us to incorrectly continue after the function call failed.  To fix this,
just call our own x509_get_subject(), which does all the neccesary error
checking correctly.

pkcs11_certificate_dn() is only called by show_pkcs11_ids(), to list the
certificates on the pkcs11 token.  Therefor, this mistake did not have a
security impact.

This issue was found by Quarkslab during the OSTIF-founded security audit
(issue 5.3).

Signed-off-by: Steffan Karger <steffan.karger@fox-it.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1494317563-6303-1-git-send-email-steffan.karger@fox-it.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14591.html
Signed-off-by: David Sommerseth <davids@openvpn.net>
(cherry picked from commit 423bb16e8a8fe22a907f469074a25533208fa0bc)
---

diff --git a/src/openvpn/pkcs11_mbedtls.c b/src/openvpn/pkcs11_mbedtls.c
index bdca893d4..dee97bc47 100644
--- a/src/openvpn/pkcs11_mbedtls.c
+++ b/src/openvpn/pkcs11_mbedtls.c
@@ -39,6 +39,7 @@
 
 #include "errlevel.h"
 #include "pkcs11_backend.h"
+#include "ssl_verify_backend.h"
 #include <mbedtls/pkcs11.h>
 #include <mbedtls/x509.h>
 
@@ -82,8 +83,6 @@ char *
 pkcs11_certificate_dn(pkcs11h_certificate_t cert, struct gc_arena *gc)
 {
     char *ret = NULL;
-    char dn[1024] = {0};
-
     mbedtls_x509_crt mbed_crt = {0};
 
     if (mbedtls_pkcs11_x509_cert_bind(&mbed_crt, cert))
@@ -92,14 +91,12 @@ pkcs11_certificate_dn(pkcs11h_certificate_t cert, struct gc_arena *gc)
         goto cleanup;
     }
 
-    if (-1 == mbedtls_x509_dn_gets(dn, sizeof(dn), &mbed_crt.subject))
+    if (!(ret = x509_get_subject(&mbed_crt, gc)))
     {
         msg(M_FATAL, "PKCS#11: mbed TLS cannot parse subject");
         goto cleanup;
     }
 
-    ret = string_alloc(dn, gc);
-
 cleanup:
     mbedtls_x509_crt_free(&mbed_crt);