diff mbox series

[Openvpn-devel,v2] Check for null-bytes in certificate subjects

Message ID 20261102130953.21115-1-gert@greenie.muc.de
State New
Headers
Series [Openvpn-devel,v2] Check for null-bytes in certificate subjects |

Commit Message

Gert Doering Nov. 2, 2026, 1:09 p.m. UTC 
  From: Max Fillinger <maximilian.fillinger@sentyron.com>

This is a partial backport of commit cf4384ee "Check for null-bytes in
certificate subjects" from the release/2.7 branch.

When using the OpenSSL library, we escaped embedded null-bytes in a
certificate's subject in x509_get_subject(), but in
extract_x509_field_ssl(), we copied any null-bytes that are contained in
the field value. When using the option --verify-x509-name, this could
lead to an incorrect name being accepted. For example, the common name
"admin\0impersonator" would be accepted when running with
--verify-x509-name admin name.

To fix this, this commit makes x509_get_subject() return an error if the
subject contains an embedded null-byte. This way, OpenVPN won't connect
with peers that present such certificates.

As a defense-in-depth measure, we also check for null-bytes in
extract_x509_field_ssl() in case a future version of OpenVPN has a code
path that avoids x509_get_subject().

With Mbed TLS, the x509_get_subject() function already returned an error
when there is an embedded null-byte. (As of Mbed TLS 3.5.) Since the
mbedtls backend code changed significantly between 2.6 and 2.7 and 2.6
is in maintenance mode, we're not backporting the Mbed TLS defense in
depth measure back to the ssl_verify_mbedtls.c.

Discovered and reported by BreachX Zero Day Labs, using Typhon AI Mil v2.
Contributing Researcher: Vivek Parikh.

CVE: 2026-84790
Reported-by: Vivek Parikh <vivek.parikh@breachx.ai>
Github: OpenVPN/openvpn-private-issues#163

Change-Id: I88b17af9988aa640ca04c765b6e79dd1c8f822c7
Signed-off-by: Steffan Karger <steffan.karger@sentyron.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1998
---

This change was reviewed on Gerrit and approved by at least one
developer. I request to merge it to release/2.6.

Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1998
This mail reflects revision 2 of this Change.

Acked-by according to Gerrit (reflected above):
Gert Doering <gert@greenie.muc.de>

Comments

Gert Doering Nov. 2, 2026, 1:50 p.m. UTC | #1 
Thanks for the backport.  I have compared the patch against commit cf4384ee
(the 2.7 version of this) and the OpenSSL part is the same.

Tested via a push to GHA and via BB.  The BB runs do not come back "all
green", because the test environment has moved on (builders with
mbedtls 4 only, which does not build at all, and OpenSSL 4 which is
not -Werror free due to deprecated functions).  GHA is fine.

Your patch has been applied to the release/2.6 branch.

commit e47a507ce3787037ea761f1d56610d829439fdb4
Author: Max Fillinger
Date:   Mon Nov 2 14:09:48 2026 +0100

     Check for null-bytes in certificate subjects

     Signed-off-by: Steffan Karger <steffan.karger@sentyron.com>
     Acked-by: Gert Doering <gert@greenie.muc.de>
     Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1998
     Message-Id: <20261102130953.21115-1-gert@greenie.muc.de>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg40429.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering
diff mbox series

Patch

diff --git a/src/openvpn/ssl_verify_openssl.c b/src/openvpn/ssl_verify_openssl.c
index ec3a7cf..ead961f 100644
--- a/src/openvpn/ssl_verify_openssl.c
+++ b/src/openvpn/ssl_verify_openssl.c
@@ -206,12 +206,14 @@ 
     X509_NAME_ENTRY *x509ne = NULL;
     ASN1_STRING *asn1 = NULL;
     unsigned char *buf = NULL;
+    result_t ret = FAILURE;
+
     ASN1_OBJECT *field_name_obj = OBJ_txt2obj(field_name, 0);
 
     if (field_name_obj == NULL)
     {
         msg(D_TLS_ERRORS, "Invalid X509 attribute name '%s'", field_name);
-        return FAILURE;
+        goto exit;
     }
 
     ASSERT(size > 0);
@@ -227,32 +229,33 @@ 
     /* Nothing found */
     if (lastpos == -1)
     {
-        return FAILURE;
+        goto exit;
     }
 
     x509ne = X509_NAME_get_entry(x509, lastpos);
     if (!x509ne)
     {
-        return FAILURE;
+        goto exit;
     }
 
     asn1 = X509_NAME_ENTRY_get_data(x509ne);
     if (!asn1)
     {
-        return FAILURE;
+        goto exit;
     }
-    if (ASN1_STRING_to_UTF8(&buf, asn1) < 0)
+    int length = ASN1_STRING_to_UTF8(&buf, asn1);
+    if (length < 0 || (size_t)length != strlen((char *)buf))
     {
-        return FAILURE;
+        goto exit;
     }
 
     strncpynt(out, (char *)buf, size);
 
-    {
-        const result_t ret = (strlen((char *)buf) < size) ? SUCCESS : FAILURE;
-        OPENSSL_free(buf);
-        return ret;
-    }
+    ret = (strlen((char *)buf) < size) ? SUCCESS : FAILURE;
+
+exit:
+    OPENSSL_free(buf);
+    return ret;
 }
 
 result_t
@@ -376,9 +379,9 @@ 
         goto err;
     }
 
-    X509_NAME_print_ex(subject_bio, X509_get_subject_name(cert),
-                       0, XN_FLAG_SEP_CPLUS_SPC | XN_FLAG_FN_SN
-                       |ASN1_STRFLGS_UTF8_CONVERT | ASN1_STRFLGS_ESC_CTRL);
+    /* Get the subject with unescaped control characters so that we can check for null bytes. */
+    X509_NAME_print_ex(subject_bio, X509_get_subject_name(cert), 0,
+                       XN_FLAG_SEP_CPLUS_SPC | XN_FLAG_FN_SN | ASN1_STRFLGS_UTF8_CONVERT);
 
     if (BIO_eof(subject_bio))
     {
@@ -386,6 +389,25 @@ 
     }
 
     BIO_get_mem_ptr(subject_bio, &subject_mem);
+    if (memchr(subject_mem->data, 0, subject_mem->length) != NULL)
+    {
+        msg(M_WARN, "ERROR: Certificate subject contains a '\\0' byte.");
+        goto err;
+    }
+
+    /* Now output the subject with escaped control characters. */
+    if (BIO_reset(subject_bio) != 1)
+    {
+        goto err;
+    }
+    X509_NAME_print_ex(subject_bio, X509_get_subject_name(cert), 0,
+                       XN_FLAG_SEP_CPLUS_SPC | XN_FLAG_FN_SN | ASN1_STRFLGS_UTF8_CONVERT
+                       | ASN1_STRFLGS_ESC_CTRL);
+    if (BIO_eof(subject_bio))
+    {
+        goto err;
+    }
+    BIO_get_mem_ptr(subject_bio, &subject_mem);
 
     subject = gc_malloc(subject_mem->length + 1, false, gc);