Attention is currently required from: Arne Schwabe.
Hello Arne Schwabe,
I'd like you to do a code review.
Please visit
https://gerrit.openvpn.net/c/openvpn/+/2121?usp=email
to review the following change.
Change subject: pkcs11: bound certificate size in pkcs11_management_id_get()
......................................................................
pkcs11: bound certificate size in pkcs11_management_id_get()
0a8e80aa ("Avoid unbounded allocations in pkcs11_mbedtls.c") capped
the certificate blob size reported by the PKCS#11 provider only in the
mbedTLS backend. pkcs11_management_id_get() still allocated whatever
size the provider returned. Move the limit to pkcs11_backend.h as
PKCS11_MAX_CERT_SIZE and check it there too.
Change-Id: Ia9f328b1c0ea29adf811a933d1fd22cb32e35e6d
Signed-off-by: Razvan Cojocaru <[email protected]>
---
M src/openvpn/pkcs11.c
M src/openvpn/pkcs11_backend.h
M src/openvpn/pkcs11_mbedtls.c
3 files changed, 22 insertions(+), 12 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/21/2121/1
diff --git a/src/openvpn/pkcs11.c b/src/openvpn/pkcs11.c
index f00ba9b..52658c2 100644
--- a/src/openvpn/pkcs11.c
+++ b/src/openvpn/pkcs11.c
@@ -504,6 +504,13 @@
goto cleanup;
}
+ if (certificate_blob_size > PKCS11_MAX_CERT_SIZE)
+ {
+ msg(M_WARN, "PKCS#11: Certificate too large: %zu bytes, maximum is
%zu",
+ certificate_blob_size, PKCS11_MAX_CERT_SIZE);
+ goto cleanup;
+ }
+
if ((certificate_blob = (unsigned char *)malloc(certificate_blob_size)) ==
NULL)
{
msg(M_FATAL, "PKCS#11: Cannot allocate memory");
diff --git a/src/openvpn/pkcs11_backend.h b/src/openvpn/pkcs11_backend.h
index b8b0eb0..11e8498 100644
--- a/src/openvpn/pkcs11_backend.h
+++ b/src/openvpn/pkcs11_backend.h
@@ -37,6 +37,18 @@
#include <pkcs11-helper-1.0/pkcs11h-certificate.h>
+/*
+ * Maximum size of a certificate retrieved from a PKCS #11 provider, so that
the 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.
+ */
+#define PKCS11_MAX_CERT_SIZE ((size_t)100000)
+
/**
* Retrieve PKCS #11 Certificate's DN in a printable format.
*
diff --git a/src/openvpn/pkcs11_mbedtls.c b/src/openvpn/pkcs11_mbedtls.c
index 2183217..3ee3837 100644
--- a/src/openvpn/pkcs11_mbedtls.c
+++ b/src/openvpn/pkcs11_mbedtls.c
@@ -42,16 +42,6 @@
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;
@@ -62,9 +52,10 @@
goto cleanup;
}
- if (cert_blob_size > max_cert_size)
+ if (cert_blob_size > PKCS11_MAX_CERT_SIZE)
{
- msg(M_WARN, "PKCS#11: Certificate too large: %zu bytes, maximum is
%zu", cert_blob_size, max_cert_size);
+ msg(M_WARN, "PKCS#11: Certificate too large: %zu bytes, maximum is
%zu", cert_blob_size,
+ PKCS11_MAX_CERT_SIZE);
goto cleanup;
}
--
To view, visit https://gerrit.openvpn.net/c/openvpn/+/2121?usp=email
To unsubscribe, or for help writing mail filters, visit
https://gerrit.openvpn.net/settings?usp=email
Gerrit-MessageType: newchange
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Ia9f328b1c0ea29adf811a933d1fd22cb32e35e6d
Gerrit-Change-Number: 2121
Gerrit-PatchSet: 1
Gerrit-Owner: Răzvan Cojocaru <[email protected]>
Gerrit-Reviewer: Arne Schwabe <[email protected]>
Gerrit-CC: openvpn-devel <[email protected]>
Gerrit-Attention: Arne Schwabe <[email protected]>
_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
