Attention is currently required from: plaisthos.
Hello plaisthos,
I'd like you to reexamine a change. Please visit
http://gerrit.openvpn.net/c/openvpn/+/1801?usp=email
to look at the new patch set (#2).
Change subject: Make --x509-username-field work with Mbed TLS
......................................................................
Make --x509-username-field work with Mbed TLS
In pre-2.7 versions, this option was not available in the Mbed TLS
build. This was changed in 2.7, but the option did not do anything when
Mbed TLS was selected as the crypto library. Regardless of the field
chosen with --x509-username-field, OpenVPN would always extract the CN
as username.
This could lead to a situation where an unintended certificate gets
accepted by OpenVPN: If we run with "--x509-username-field serialNumber"
and "--verify-x509-name 0x05 name", OpenVPN would accept a certificate
with CN=0x05 and an incorrect serial number, while a certificate with
the correct serial number would be rejected. (Though note that to
exploit this, an attacker needs to make the CA sign a certificate
with a hexadecimal number in the CN.)
This commit adds code to extract the correct field values from X509
certificates. It also adds unit tests for extracting the values of
different fields.
Despite this commit fixing a CVE, we have decided not to keep it under
embargo until the release of the next version because it is unlikely to
be exploitable in practice: Someone has to run OpenVPN 2.7 with Mbed
TLS, use the --x509-username-field option even though it didn't exist in
Mbed TLS builds of earlier versions, not notice that the intended
certificate is *not* accepted, and then an attacker has to get the CA to
sign a certificate with a weird common name.
CVE: 2026-63650
Github: openvpn/openvpn-private-issues#144
Reported-By: Hcamael
Reported-By: 章鱼哥 (www.aipyaipy.com)
Change-Id: Ic183f1f1f90561454b7b1128c95255368427cc4a
Signed-off-by: Max Fillinger <[email protected]>
---
M src/openvpn/ssl_verify_mbedtls.c
M tests/unit_tests/openvpn/test_ssl.c
2 files changed, 181 insertions(+), 22 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/01/1801/2
diff --git a/src/openvpn/ssl_verify_mbedtls.c b/src/openvpn/ssl_verify_mbedtls.c
index ad5479c..eb96796 100644
--- a/src/openvpn/ssl_verify_mbedtls.c
+++ b/src/openvpn/ssl_verify_mbedtls.c
@@ -137,19 +137,133 @@
return false;
}
+static const char *
+fieldname_to_oid(const char *fieldname)
+{
+ if (strcmp(fieldname, "C") == 0)
+ {
+ return MBEDTLS_OID_AT_COUNTRY;
+ }
+ else if (strcmp(fieldname, "ST") == 0)
+ {
+ return MBEDTLS_OID_AT_STATE;
+ }
+ else if (strcmp(fieldname, "LOCALITY") == 0)
+ {
+ return MBEDTLS_OID_AT_LOCALITY;
+ }
+ else if (strcmp(fieldname, "O") == 0)
+ {
+ return MBEDTLS_OID_AT_ORGANIZATION;
+ }
+ else if (strcmp(fieldname, "OU") == 0)
+ {
+ return MBEDTLS_OID_AT_ORG_UNIT;
+ }
+ else if (strcmp(fieldname, "CN") == 0)
+ {
+ return MBEDTLS_OID_AT_CN;
+ }
+ else if (strcmp(fieldname, "GN") == 0)
+ {
+ return MBEDTLS_OID_AT_GIVEN_NAME;
+ }
+ else if (strcmp(fieldname, "SN") == 0)
+ {
+ return MBEDTLS_OID_AT_SUR_NAME;
+ }
+ else if (strcmp(fieldname, "initials") == 0)
+ {
+ return MBEDTLS_OID_AT_INITIALS;
+ }
+ else if (strcmp(fieldname, "pseudonym") == 0)
+ {
+ return MBEDTLS_OID_AT_PSEUDONYM;
+ }
+ else if (strcmp(fieldname, "title") == 0)
+ {
+ return MBEDTLS_OID_AT_TITLE;
+ }
+ else if (strcmp(fieldname, "generationQualifier") == 0)
+ {
+ return MBEDTLS_OID_AT_GENERATION_QUALIFIER;
+ }
+ else if (strcmp(fieldname, "postalAddress") == 0)
+ {
+ return MBEDTLS_OID_AT_POSTAL_ADDRESS;
+ }
+ else if (strcmp(fieldname, "postalCode") == 0)
+ {
+ return MBEDTLS_OID_AT_POSTAL_CODE;
+ }
+ else if (strcmp(fieldname, "emailAddress") == 0)
+ {
+ return MBEDTLS_OID_PKCS9_EMAIL;
+ }
+ else if (strcmp(fieldname, "uid") == 0)
+ {
+ return MBEDTLS_OID_AT_UNIQUE_IDENTIFIER;
+ }
+ else if (strcmp(fieldname, "dnQualifier") == 0)
+ {
+ return MBEDTLS_OID_AT_DN_QUALIFIER;
+ }
+ else
+ {
+ return NULL;
+ }
+}
+
result_t
backend_x509_get_username(char *cn, size_t cn_len, char *x509_username_field,
mbedtls_x509_crt *cert)
{
- mbedtls_x509_name *name;
+ ASSERT(cn != NULL && cert != NULL);
- ASSERT(cn != NULL);
+ if (x509_username_field == NULL)
+ {
+ goto fail;
+ }
- name = &cert->subject;
+ if (strcmp(x509_username_field, "serialNumber") == 0)
+ {
+ if (cn_len < 2)
+ {
+ goto fail;
+ }
+ cn[0] = '0';
+ cn[1] = 'x';
+ size_t cn_index = 2;
+ bool leading_zeros = true;
+ for (size_t i = 0; i < cert->serial.len; i++)
+ {
+ uint8_t serial_byte = cert->serial.p[i];
+ if (leading_zeros && serial_byte == 0)
+ {
+ continue;
+ }
+ leading_zeros = false;
+ if (cn_index >= cn_len)
+ {
+ goto fail;
+ }
+ snprintf(&cn[cn_index], cn_len - cn_index, "%02X", serial_byte);
+ cn_index += 2;
+ }
+ return SUCCESS;
+ }
- /* Find common name */
+ const char *field_oid = fieldname_to_oid(x509_username_field);
+ if (field_oid == NULL)
+ {
+ goto fail;
+ }
+
+ /* Find field_oid in the subject name. */
+ mbedtls_x509_name *name = &cert->subject;
while (name != NULL)
{
- if (0 == memcmp(name->oid.p, MBEDTLS_OID_AT_CN,
MBEDTLS_OID_SIZE(MBEDTLS_OID_AT_CN)))
+ if (strlen(field_oid) == name->oid.len
+ && 0 == memcmp(name->oid.p, field_oid, name->oid.len))
{
break;
}
@@ -160,10 +274,10 @@
/* Not found, return an error if this is the peer's certificate */
if (name == NULL)
{
- return FAILURE;
+ goto fail;
}
- /* Found, extract CN */
+ /* Found, extract the value */
if (cn_len > name->val.len)
{
memcpy(cn, name->val.p, name->val.len);
@@ -171,11 +285,18 @@
}
else
{
- memcpy(cn, name->val.p, cn_len);
+ memcpy(cn, name->val.p, cn_len - 1);
cn[cn_len - 1] = '\0';
}
return SUCCESS;
+
+fail:
+ if (cn_len > 0)
+ {
+ cn[0] = '\0';
+ }
+ return FAILURE;
}
#if MBEDTLS_VERSION_NUMBER >= 0x04000000
diff --git a/tests/unit_tests/openvpn/test_ssl.c
b/tests/unit_tests/openvpn/test_ssl.c
index 0e9cecf..4ab6ee7 100644
--- a/tests/unit_tests/openvpn/test_ssl.c
+++ b/tests/unit_tests/openvpn/test_ssl.c
@@ -775,7 +775,7 @@
static openvpn_x509_cert_t *
get_certificate(const char *cert_str)
{
- BIO *in = BIO_new_mem_buf((char *)cert1, -1);
+ BIO *in = BIO_new_mem_buf((char *)cert_str, -1);
assert_non_null(in);
X509 *cert = PEM_read_bio_X509(in, NULL, NULL, NULL);
assert_non_null(cert);
@@ -790,10 +790,35 @@
}
#endif
+/* Generated with:
+ * openssl req -x509 -newkey ec -pkeyopt ec_paramgen_curve:secp384r1 -keyout -
-noenc -sha256 -days 3650 \
+ * -subj '/CN=ovpn-test-secp384r1/O=OpenVPN Unit Test Example Corp./OU=Cert
Details Dept.'
+ * -addext 'subjectAltName=DNS:unittest.example.com' -addext
'extendedKeyUsage=clientAuth' */
+static const char *cert_details_test_cert =
+ "-----BEGIN CERTIFICATE-----\n"
+ "MIICkzCCAhqgAwIBAgIUKDsZM+PApGdaD2QF9iYaxoFJAkowCgYIKoZIzj0EAwIw\n"
+ "ZTEcMBoGA1UEAwwTb3Zwbi10ZXN0LXNlY3AzODRyMTEoMCYGA1UECgwfT3BlblZQ\n"
+ "TiBVbml0IFRlc3QgRXhhbXBsZSBDb3JwLjEbMBkGA1UECwwSQ2VydCBEZXRhaWxz\n"
+ "IERlcHQuMB4XDTI2MDcwNzEwNTIxNloXDTM2MDcwNDEwNTIxNlowZTEcMBoGA1UE\n"
+ "AwwTb3Zwbi10ZXN0LXNlY3AzODRyMTEoMCYGA1UECgwfT3BlblZQTiBVbml0IFRl\n"
+ "c3QgRXhhbXBsZSBDb3JwLjEbMBkGA1UECwwSQ2VydCBEZXRhaWxzIERlcHQuMHYw\n"
+ "EAYHKoZIzj0CAQYFK4EEACIDYgAEOlKoQVk+wbBD6V/6kg+/oHfqF0Dq08LlCL+B\n"
+ "om4RhutG99QDrow251Ps+Ds/7LQYYRA8+hHyEFrmGM+j2o6KhS5K2uA6dIZL4zLK\n"
+ "vl0NeF2M61Z8tt/IjrFZd+CrEANco4GKMIGHMB0GA1UdDgQWBBSEyG6m+QdWazeg\n"
+ "0CHN7q0edJlqMjAfBgNVHSMEGDAWgBSEyG6m+QdWazeg0CHN7q0edJlqMjAPBgNV\n"
+ "HRMBAf8EBTADAQH/MB8GA1UdEQQYMBaCFHVuaXR0ZXN0LmV4YW1wbGUuY29tMBMG\n"
+ "A1UdJQQMMAoGCCsGAQUFBwMCMAoGCCqGSM49BAMCA2cAMGQCMCyK7aQcyKGW8BWQ\n"
+ "UOYqbJUJZJcviP6ACgJRzK6pgkqt9gY0E0Tb00Qh6D5dBV5i3wIwCMhSgpVJxDrc\n"
+ "pRfligoK8bmv4HEgnV6BDeoDYd41WVMpE9u1issQDHY0SnWC7d9q\n"
+ "-----END CERTIFICATE-----\n";
+const char *const cert_details_cname = "ovpn-test-secp384r1";
+const char *const cert_details_org = "OpenVPN Unit Test Example Corp.";
+const char *const cert_details_org_unit = "Cert Details Dept.";
+
void
crypto_test_print_cert_details(void **state)
{
- openvpn_x509_cert_t *cert = get_certificate(cert1);
+ openvpn_x509_cert_t *cert = get_certificate(cert_details_test_cert);
struct gc_arena gc = gc_new();
const char *fp = backend_x509_get_serial_hex(cert, &gc);
@@ -801,29 +826,42 @@
/* we messed this up between TLS libraries. But let's at least notice in
* the future ...*/
#if defined(ENABLE_CRYPTO_MBEDTLS)
- assert_string_equal(fp, "82:6B:DD:CC:BD:E5:5E:B7:08:F1:2D:68:00:3C:24:DE");
+ assert_string_equal(fp,
"28:3B:19:33:E3:C0:A4:67:5A:0F:64:05:F6:26:1A:C6:81:49:02:4A");
#else
- assert_string_equal(fp, "82:6b:dd:cc:bd:e5:5e:b7:08:f1:2d:68:00:3c:24:de");
+ assert_string_equal(fp,
"28:3b:19:33:e3:c0:a4:67:5a:0f:64:05:f6:26:1a:c6:81:49:02:4a");
#endif
const char *sn = backend_x509_get_serial(cert, &gc);
- assert_string_equal(sn, "173359713849739808110610111821055272158");
+ assert_string_equal(sn,
"229677570263950905252266749734450551528150860362");
char username[TLS_USERNAME_LEN + 1] = { 0 }; /* null-terminated */
- int ret = backend_x509_get_username(username, sizeof(username), "CN",
- cert);
+ int ret = backend_x509_get_username(username, sizeof(username), "CN",
cert);
- assert_string_equal(username, "ovpn-test-ec1");
+ assert_string_equal(username, cert_details_cname);
assert_int_equal(ret, SUCCESS);
-#ifndef ENABLE_CRYPTO_MBEDTLS
- /* mbed TLS does not implement this */
- ret = backend_x509_get_username(username, sizeof(username), "serialNumber",
- cert);
+ ret = backend_x509_get_username(username, sizeof(username),
"serialNumber", cert);
assert_int_equal(ret, SUCCESS);
- assert_string_equal(username, "0x826BDDCCBDE55EB708F12D68003C24DE");
-#endif
+ assert_string_equal(username,
"0x283B1933E3C0A4675A0F6405F6261AC68149024A");
+
+ ret = backend_x509_get_username(username, sizeof(username), "O", cert);
+
+ assert_string_equal(username, cert_details_org);
+ assert_int_equal(ret, SUCCESS);
+
+ ret = backend_x509_get_username(username, sizeof(username), "OU", cert);
+
+ assert_string_equal(username, cert_details_org_unit);
+ assert_int_equal(ret, SUCCESS);
+
+ /* Check that FAILURE is returned if a field does not exist. */
+ ret = backend_x509_get_username(username, sizeof(username), "SN", cert);
+ assert_int_equal(ret, FAILURE);
+
+ /* Check that FAILURE is returned for invalid field names. */
+ ret = backend_x509_get_username(username, sizeof(username),
"invalidField", cert);
+ assert_int_equal(ret, FAILURE);
gc_free(&gc);
free_certificate(cert);
--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1801?usp=email
To unsubscribe, or for help writing mail filters, visit
http://gerrit.openvpn.net/settings?usp=email
Gerrit-MessageType: newpatchset
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Ic183f1f1f90561454b7b1128c95255368427cc4a
Gerrit-Change-Number: 1801
Gerrit-PatchSet: 2
Gerrit-Owner: MaxF <[email protected]>
Gerrit-Reviewer: plaisthos <[email protected]>
Gerrit-CC: openvpn-devel <[email protected]>
Gerrit-Attention: plaisthos <[email protected]>
_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
