diff mbox series

[Openvpn-devel,v1] Avoid unbounded allocations in pkcs11_mbedtls.c

Message ID 20260302142045.5954-1-gert@greenie.muc.de
State New
Headers show
Series [Openvpn-devel,v1] Avoid unbounded allocations in pkcs11_mbedtls.c | expand

Commit Message

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

The PKCS#11 provider can crash OpenVPN by making it try to allocate
2^64 bytes for a certificate. To avoid this, set a maximum size for
certificates. If the size is exceeded, don't try to allocate memory and
instead exit pkcs11_get_x509_cert with an error.

The chosen maximum size is 100.000 bytes which is twice the size of
a SLH-DSA (aka SPHINCS+) signature.

Found-by: ZeroPath (https://zeropath.com/)
Reported-by: Joshua Rogers <contact@joshua.hu>
Github: closes OpenVPN/openvpn-private-issues#42

Change-Id: I53d47e4a0d33c380ee95e0e33aecad3db3197940
Signed-off-by: Max Fillinger <maximilian.fillinger@sentyron.com>
Acked-by: Arne Schwabe <arne-openvpn@rfc2549.org>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1549
---

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

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

Acked-by according to Gerrit (reflected above):
Arne Schwabe <arne-openvpn@rfc2549.org>

Comments

Gert Doering March 2, 2026, 9:50 p.m. UTC | #1 
This is somewhat of a "minibug", but we decided that we want to have
a bit of a safeguard against a pkcs#11 provider that requests us to
allocate impossible amounts of memory.  Of course there are other ways
it could crash, but this one is a bit of "good housekeeping".

I have not tested it beyond "does it compile" - have no smartcard, and
especially not one that misbehaves (and do not feel like setting up
SoftHSM and stuff to test this for real).

Your patch has been applied to the master and release/2.7 branch (minibug).

commit 0a8e80aaf9c96718903251a828bc3e8055014160 (master)
commit bf7f8548c7bfd31f0e6fed880890d5106c2ab982 (release/2.7)
Author: Max Fillinger
Date:   Mon Mar 2 15:20:39 2026 +0100

     Avoid unbounded allocations in pkcs11_mbedtls.c

     Signed-off-by: Max Fillinger <maximilian.fillinger@sentyron.com>
     Acked-by: Arne Schwabe <arne-openvpn@rfc2549.org>
     Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1549
     Message-Id: <20260302142045.5954-1-gert@greenie.muc.de>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg35807.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering
diff mbox series

Patch

diff --git a/src/openvpn/pkcs11_mbedtls.c b/src/openvpn/pkcs11_mbedtls.c
index 66aefac..bf9d953 100644
--- a/src/openvpn/pkcs11_mbedtls.c
+++ b/src/openvpn/pkcs11_mbedtls.c
@@ -42,6 +42,16 @@ 
 static bool
 pkcs11_get_x509_cert(pkcs11h_certificate_t pkcs11_cert, mbedtls_x509_crt *cert)
 {
+    /* We set a maximum size for certificates so that the PKCS provider cannot crash OpenVPN by
+     * making it try to allocate 2^64 bytes. The maximum of 100.000 bytes is picked as a round
+     * number that easily accomodates the currently standardized quantum-safe signature algorithms.
+     * It is twice the size of a SLH-DSA (aka SPHINCS+) signature plus public key.
+     *
+     * However, there are additional digital signature schemes currently on the NIST on-ramp
+     * (e.g., some parameter settings for LESS) that have even larger public keys or signatures, so
+     * if those ever see use on smartcards, we will need to increase this number. */
+    const size_t max_cert_size = 100000;
+
     unsigned char *cert_blob = NULL;
     size_t cert_blob_size = 0;
     bool ret = false;
@@ -52,6 +62,12 @@ 
         goto cleanup;
     }
 
+    if (cert_blob_size > max_cert_size)
+    {
+        msg(M_WARN, "PKCS#11: Certificate too large: %lu bytes, maximum is %lu", cert_blob_size, max_cert_size);
+        goto cleanup;
+    }
+
     check_malloc_return((cert_blob = calloc(1, cert_blob_size)));
     if (pkcs11h_certificate_getCertificateBlob(pkcs11_cert, cert_blob, &cert_blob_size) != CKR_OK)
     {