From dce27129b74f906e0d1c086858f360228d5cbc83 Mon Sep 17 00:00:00 2001 From: Dimitri Staessens Date: Fri, 12 Jun 2026 20:26:27 +0200 Subject: irmd: Reject OAP peer crt with unusable CN Added checks for CN > NAME_SIZE. Signed-off-by: Dimitri Staessens Signed-off-by: Sander Vrijders --- include/test/certs/ecdsa.h | 20 +++++++++++ src/irmd/oap/auth.c | 10 ++++-- src/irmd/oap/auth.h | 2 +- src/lib/crypt/openssl.c | 85 +++++++++++++++++++++++++--------------------- src/lib/tests/auth_test.c | 46 +++++++++++++++++++++++++ 5 files changed, 120 insertions(+), 43 deletions(-) diff --git a/include/test/certs/ecdsa.h b/include/test/certs/ecdsa.h index 989b5bc6..cbc4ed06 100644 --- a/include/test/certs/ecdsa.h +++ b/include/test/certs/ecdsa.h @@ -138,5 +138,25 @@ static __attribute__((unused)) const char * server_crt_ec = \ "gRo=\n" "-----END CERTIFICATE-----\n"; +/* + * Name-confusion fixture: real CN is "attacker.unittest.o7s", but the + * O field value is "CN=victim.unittest.o7s" so the oneline subject is + * "/O=CN=victim.unittest.o7s/CN=attacker.unittest.o7s". A strstr("CN=") + * scan latches onto the decoy. The real CN must win. + */ +static __attribute__((unused)) const char * confused_crt_ec = \ +"-----BEGIN CERTIFICATE-----\n" +"MIIB1jCCAX2gAwIBAgIUCfXJzDQ3Sx5qcyVB9Rb4/FdZ+QowCgYIKoZIzj0EAwIw\n" +"QTEfMB0GA1UECgwWQ049dmljdGltLnVuaXR0ZXN0Lm83czEeMBwGA1UEAwwVYXR0\n" +"YWNrZXIudW5pdHRlc3QubzdzMB4XDTI2MDYxNDE5MDcwMVoXDTQ2MDYwOTE5MDcw\n" +"MVowQTEfMB0GA1UECgwWQ049dmljdGltLnVuaXR0ZXN0Lm83czEeMBwGA1UEAwwV\n" +"YXR0YWNrZXIudW5pdHRlc3QubzdzMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAE\n" +"oLwrbLs3diGcjyY2ErvO/U6CoyyKfl/8e1nxBKXHSOkO5xVmFu+EobEQVFvabxE/\n" +"x4RttKcGJqUe8vlyQexQq6NTMFEwHQYDVR0OBBYEFGBaOBzTsCakjBN61x0ZnHSk\n" +"04T3MB8GA1UdIwQYMBaAFGBaOBzTsCakjBN61x0ZnHSk04T3MA8GA1UdEwEB/wQF\n" +"MAMBAf8wCgYIKoZIzj0EAwIDRwAwRAIgFtBeVxlRuI7y9Bo/Dh97ajTbHJXYMkc6\n" +"ZqflSN3Q/uACIHWoCVn6u6+JjF+Kj9zubFJ49RIQJthSeP8xj7yTeV17\n" +"-----END CERTIFICATE-----\n"; + #endif /* TEST_CERTS_H */ diff --git a/src/irmd/oap/auth.c b/src/irmd/oap/auth.c index ebe1949b..60bd5f97 100644 --- a/src/irmd/oap/auth.c +++ b/src/irmd/oap/auth.c @@ -266,9 +266,13 @@ int oap_auth_peer(char * name, goto fail_pin; } - if (crypt_get_crt_name(crt, name) < 0) { - log_warn_id(id, "Failed to extract name from certificate."); - name[0] = '\0'; + ret = crypt_get_crt_name(crt, name); + if (ret < 0) { + if (ret == -ENAME) + log_err_id(id, "Certificate CN too long."); + else + log_err_id(id, "No name in certificate."); + goto fail_pin; } if (pin != NULL) diff --git a/src/irmd/oap/auth.h b/src/irmd/oap/auth.h index be8d2cae..4a350120 100644 --- a/src/irmd/oap/auth.h +++ b/src/irmd/oap/auth.h @@ -29,7 +29,7 @@ int oap_check_hdr(const struct oap_hdr * hdr); -/* name is updated with the peer's certificate name if available */ +/* name is set to the peer crt CN, "" if no crt was presented */ int oap_auth_peer(char * name, const struct sec_config * cfg, const struct oap_hdr * local_hdr, diff --git a/src/lib/crypt/openssl.c b/src/lib/crypt/openssl.c index 2ea35a17..d4ffc00b 100644 --- a/src/lib/crypt/openssl.c +++ b/src/lib/crypt/openssl.c @@ -30,6 +30,7 @@ #include #include #include +#include #include #include @@ -1552,65 +1553,71 @@ void openssl_free_key(EVP_PKEY * key) int openssl_check_crt_name(void * crt, const char * name) { - char * subj; - char * cn; - X509 * xcrt; + const unsigned char * cn; + ASN1_STRING * val; + X509_NAME * nm; + int idx; + int len; - xcrt = (X509 *) crt; + nm = X509_get_subject_name((X509 *) crt); + if (nm == NULL) + return -1; - subj = X509_NAME_oneline(X509_get_subject_name(xcrt), NULL, 0); - if (subj == NULL) - goto fail_subj; + idx = X509_NAME_get_index_by_NID(nm, NID_commonName, -1); + if (idx < 0) + return -1; - cn = strstr(subj, "CN="); - if (cn == NULL) - goto fail_cn; + val = X509_NAME_ENTRY_get_data(X509_NAME_get_entry(nm, idx)); + cn = ASN1_STRING_get0_data(val); + len = ASN1_STRING_length(val); - if (strcmp(cn + 3, name) != 0) - goto fail_cn; + if (len < 0 || (size_t) len != strlen(name)) + return -1; - free(subj); + if (memchr(cn, '\0', (size_t) len) != NULL) + return -1; + + if (memcmp(cn, name, (size_t) len) != 0) + return -1; return 0; - fail_cn: - free(subj); - fail_subj: - return -1; } int openssl_get_crt_name(void * crt, char * name) { - char * subj; - char * cn; - char * end; - X509 * xcrt; + const unsigned char * cn; + ASN1_STRING * val; + X509_NAME * nm; + int idx; + int len; - xcrt = (X509 *) crt; + nm = X509_get_subject_name((X509 *) crt); + if (nm == NULL) + return -1; - subj = X509_NAME_oneline(X509_get_subject_name(xcrt), NULL, 0); - if (subj == NULL) - goto fail_subj; + idx = X509_NAME_get_index_by_NID(nm, NID_commonName, -1); + if (idx < 0) + return -1; - cn = strstr(subj, "CN="); - if (cn == NULL) - goto fail_cn; + val = X509_NAME_ENTRY_get_data(X509_NAME_get_entry(nm, idx)); + cn = ASN1_STRING_get0_data(val); + len = ASN1_STRING_length(val); - cn += 3; /* Skip "CN=" */ + if (len < 0) + return -1; + + if ((size_t) len > NAME_SIZE) + return -ENAME; - /* Find end of CN (comma or slash for next field) */ - end = strpbrk(cn, ",/"); - if (end != NULL) - *end = '\0'; + /* Reject an embedded NUL that would truncate the parsed name. */ + if (memchr(cn, '\0', (size_t) len) != NULL) + return -1; - strcpy(name, cn); - free(subj); + memcpy(name, cn, (size_t) len); + name[len] = '\0'; return 0; - fail_cn: - free(subj); - fail_subj: - return -1; } int openssl_crt_str(const void * crt, diff --git a/src/lib/tests/auth_test.c b/src/lib/tests/auth_test.c index 6a7666c1..af7cf81c 100644 --- a/src/lib/tests/auth_test.c +++ b/src/lib/tests/auth_test.c @@ -24,11 +24,14 @@ #include #include +#include #include #include #include +#include + #define TEST_MSG_SIZE 1500 static int test_auth_create_destroy_ctx(void) @@ -138,6 +141,47 @@ static int test_check_crt_name(void) return TEST_RC_FAIL; } +static int test_crt_name_confusion(void) +{ + char name[NAME_SIZE + 1]; + void * crt; + + TEST_START(); + + if (crypt_load_crt_str(confused_crt_ec, &crt) < 0) { + printf("Failed to load name-confusion certificate.\n"); + goto fail_load; + } + + /* Must extract the real CN, not the "CN=" decoy in the O field. */ + if (crypt_get_crt_name(crt, name) < 0) { + printf("Failed to extract name from certificate.\n"); + goto fail_check; + } + + if (strcmp(name, "attacker.unittest.o7s") != 0) { + printf("Extracted '%s', expected real CN.\n", name); + goto fail_check; + } + + /* The decoy name in the O field must never authenticate. */ + if (crypt_check_crt_name(crt, "victim.unittest.o7s") == 0) { + printf("Accepted spoofed name from O field.\n"); + goto fail_check; + } + + crypt_free_crt(crt); + + TEST_SUCCESS(); + + return TEST_RC_SUCCESS; + fail_check: + crypt_free_crt(crt); + fail_load: + TEST_FAIL(); + return TEST_RC_FAIL; +} + static int test_load_free_privkey(void) { void * key; @@ -665,6 +709,7 @@ int auth_test(int argc, #ifdef HAVE_OPENSSL ret |= test_load_free_crt(); ret |= test_check_crt_name(); + ret |= test_crt_name_confusion(); ret |= test_crypt_get_pubkey_crt(); ret |= test_load_free_privkey(); ret |= test_load_free_pubkey(); @@ -679,6 +724,7 @@ int auth_test(int argc, #else (void) test_load_free_crt; (void) test_check_crt_name; + (void) test_crt_name_confusion; (void) test_crypt_get_pubkey_crt; (void) test_load_free_privkey; (void) test_load_free_pubkey; -- cgit v1.2.3