From 3d10ba06d4e757fa98db26f79678c52a7d365d92 Mon Sep 17 00:00:00 2001 From: James Lucas Date: Fri, 28 Apr 2023 11:02:49 +1000 Subject: [PATCH 1/9] Fixed bug #9356 Incomplete validation of IP Address fields in subjectAltNames IPv6 addresses are valid entries in subjectAltNames. Certificate Authorities may issue certificates including IPv6 addresses except if they fall within addresses in the RFC 4193 range. Google and CloudFlare provide IPv6 addresses in their DNS over HTTPS services. Internal CAs do not have those restrictions and can issue Unique local addresses in certificates. --- ext/openssl/tests/san_ipv6_peer_matching.phpt | 64 +++++++++++++++++++ ext/openssl/xp_ssl.c | 38 +++++++++-- 2 files changed, 97 insertions(+), 5 deletions(-) create mode 100644 ext/openssl/tests/san_ipv6_peer_matching.phpt diff --git a/ext/openssl/tests/san_ipv6_peer_matching.phpt b/ext/openssl/tests/san_ipv6_peer_matching.phpt new file mode 100644 index 0000000000000..62504390d2e02 --- /dev/null +++ b/ext/openssl/tests/san_ipv6_peer_matching.phpt @@ -0,0 +1,64 @@ +--TEST-- +IPv6 Peer verification matches SAN names +--EXTENSIONS-- +openssl +--SKIPIF-- + +--FILE-- + [ + 'local_cert' => '%s', + ]]); + + $server = stream_socket_server($serverUri, $errno, $errstr, $serverFlags, $serverCtx); + phpt_notify(); + + @stream_socket_accept($server, 1); + @stream_socket_accept($server, 1); +CODE; +$serverCode = sprintf($serverCode, $certFile); + +$clientCode = <<<'CODE' + $serverUri = "ssl://[::1]:64321"; + $clientFlags = STREAM_CLIENT_CONNECT; + $clientCtx = stream_context_create(['ssl' => [ + 'verify_peer' => false, + ]]); + + phpt_wait(); + + stream_context_set_option($clientCtx, 'ssl', 'peer_name', '2001:db8:85a3:8d3:1319:8a2e:370:7348'); + var_dump(stream_socket_client($serverUri, $errno, $errstr, 1, $clientFlags, $clientCtx)); + + stream_context_set_option($clientCtx, 'ssl', 'peer_name', '2001:db8:85a3:8d3:1319:8a2e:370:7349'); + var_dump(stream_socket_client($serverUri, $errno, $errstr, 1, $clientFlags, $clientCtx)); +CODE; + +include 'CertificateGenerator.inc'; +$certificateGenerator = new CertificateGenerator(); +$certificateGenerator->saveNewCertAsFileWithKey(null, $certFile, null, $san); + +include 'ServerClientTestCase.inc'; +ServerClientTestCase::getInstance()->run($clientCode, $serverCode); +?> +--CLEAN-- + +--EXPECTF-- +resource(%d) of type (stream) + +Warning: stream_socket_client(): Unable to locate peer certificate CN in %s on line %d + +Warning: stream_socket_client(): Failed to enable crypto in %s on line %d + +Warning: stream_socket_client(): Unable to connect to ssl://[::1]:64321 (Unknown error) in %s on line %d +bool(false) diff --git a/ext/openssl/xp_ssl.c b/ext/openssl/xp_ssl.c index 9aac4a0b70a28..cbe5b9b9b07e7 100644 --- a/ext/openssl/xp_ssl.c +++ b/ext/openssl/xp_ssl.c @@ -44,6 +44,8 @@ #undef X509_NAME #undef X509_CERT_PAIR #undef X509_EXTENSIONS +#else +#include #endif /* Flags for determining allowed stream crypto methods */ @@ -110,6 +112,21 @@ #define PHP_X509_NAME_ENTRY_TO_UTF8(ne, i, out) \ ASN1_STRING_to_UTF8(&out, X509_NAME_ENTRY_get_data(X509_NAME_get_entry(ne, i))) +/* Used for IPv6 Address peer verification */ +#define EXPAND_IPV6_ADDRESS(_str, _bytes) \ + do { \ + snprintf(_str, 39, "%X:%X:%X:%X:%X:%X:%X:%X", \ + _bytes[0] << 8 | _bytes[1], \ + _bytes[2] << 8 | _bytes[3], \ + _bytes[4] << 8 | _bytes[5], \ + _bytes[6] << 8 | _bytes[7], \ + _bytes[8] << 8 | _bytes[9], \ + _bytes[10] << 8 | _bytes[11], \ + _bytes[12] << 8 | _bytes[13], \ + _bytes[14] << 8 | _bytes[15] \ + ); \ + } while(0) + #if PHP_OPENSSL_API_VERSION < 0x10100 static RSA *php_openssl_tmp_rsa_cb(SSL *s, int is_export, int keylength); #endif @@ -416,11 +433,18 @@ static bool php_openssl_matches_san_list(X509 *peer, const char *subject_name) / { int i, len; unsigned char *cert_name = NULL; - char ipbuffer[64]; + char ipbuffer[64], ipv6_expanded[40]; + unsigned char ipv6[16]; GENERAL_NAMES *alt_names = X509_get_ext_d2i(peer, NID_subject_alt_name, 0, 0); int alt_name_count = sk_GENERAL_NAME_num(alt_names); + /* detect if subject name is an IPv6 address and expand once if required */ + ipv6_expanded[0] = 0; + if (inet_pton(AF_INET6,subject_name,&ipv6)) { + EXPAND_IPV6_ADDRESS(ipv6_expanded, ipv6); + } + for (i = 0; i < alt_name_count; i++) { GENERAL_NAME *san = sk_GENERAL_NAME_value(alt_names, i); @@ -456,13 +480,17 @@ static bool php_openssl_matches_san_list(X509 *peer, const char *subject_name) / if (strcasecmp(subject_name, (const char*)ipbuffer) == 0) { sk_GENERAL_NAME_pop_free(alt_names, GENERAL_NAME_free); + return 1; + } + } else if (san->d.ip->length == 16 && strlen(ipv6_expanded) >= 15) { /* shortest expanded IPv6 address is 0:0:0:0:0:0:0:0 */ + ipbuffer[0] = 0; + EXPAND_IPV6_ADDRESS(ipbuffer, san->d.iPAddress->data); + if (strcasecmp((const char*)ipv6_expanded, (const char*)ipbuffer) == 0) { + sk_GENERAL_NAME_pop_free(alt_names, GENERAL_NAME_free); + return 1; } } - /* No, we aren't bothering to check IPv6 addresses. Why? - * Because IP SAN names are officially deprecated and are - * not allowed by CAs starting in 2015. Deal with it. - */ } } From 70a523dd91d0463e3219edfa75fbd1845931ca52 Mon Sep 17 00:00:00 2001 From: James Lucas Date: Sun, 30 Apr 2023 15:08:45 +1000 Subject: [PATCH 2/9] Conditionally include arpa/inet.h if detected Include for Windows inet_pton support Conditionally do subject_name IPv6 expansion only when both inet_pton is available and IPv6 support is comiled in --- ext/openssl/xp_ssl.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/ext/openssl/xp_ssl.c b/ext/openssl/xp_ssl.c index cbe5b9b9b07e7..35e8314e9d278 100644 --- a/ext/openssl/xp_ssl.c +++ b/ext/openssl/xp_ssl.c @@ -39,12 +39,15 @@ #ifdef PHP_WIN32 #include "win32/winutil.h" #include "win32/time.h" +#include #include /* These are from Wincrypt.h, they conflict with OpenSSL */ #undef X509_NAME #undef X509_CERT_PAIR #undef X509_EXTENSIONS -#else +#endif + +#ifdef HAVE_ARPA_INET_H #include #endif @@ -441,9 +444,11 @@ static bool php_openssl_matches_san_list(X509 *peer, const char *subject_name) / /* detect if subject name is an IPv6 address and expand once if required */ ipv6_expanded[0] = 0; +#if defined(HAVE_IPV6) && defined(HAVE_INET_PTON) if (inet_pton(AF_INET6,subject_name,&ipv6)) { EXPAND_IPV6_ADDRESS(ipv6_expanded, ipv6); } +#endif for (i = 0; i < alt_name_count; i++) { GENERAL_NAME *san = sk_GENERAL_NAME_value(alt_names, i); From d38f3e0c571ef5cc963e76d022a8c1f6d282fb0e Mon Sep 17 00:00:00 2001 From: James Lucas Date: Sun, 7 May 2023 12:47:33 +1000 Subject: [PATCH 3/9] Implement suggested expanded flag instead of checking strlen on each iteration --- ext/openssl/xp_ssl.c | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/ext/openssl/xp_ssl.c b/ext/openssl/xp_ssl.c index 35e8314e9d278..e1c188444f047 100644 --- a/ext/openssl/xp_ssl.c +++ b/ext/openssl/xp_ssl.c @@ -436,17 +436,20 @@ static bool php_openssl_matches_san_list(X509 *peer, const char *subject_name) / { int i, len; unsigned char *cert_name = NULL; - char ipbuffer[64], ipv6_expanded[40]; - unsigned char ipv6[16]; + char ipbuffer[64]; GENERAL_NAMES *alt_names = X509_get_ext_d2i(peer, NID_subject_alt_name, 0, 0); int alt_name_count = sk_GENERAL_NAME_num(alt_names); - /* detect if subject name is an IPv6 address and expand once if required */ - ipv6_expanded[0] = 0; #if defined(HAVE_IPV6) && defined(HAVE_INET_PTON) + /* detect if subject name is an IPv6 address and expand once if required */ + char subject_name_ipv6_expanded[40]; + unsigned char ipv6[16]; + bool subject_name_is_ipv6 = false; + subject_name_ipv6_expanded[0] = 0; if (inet_pton(AF_INET6,subject_name,&ipv6)) { - EXPAND_IPV6_ADDRESS(ipv6_expanded, ipv6); + EXPAND_IPV6_ADDRESS(subject_name_ipv6_expanded, ipv6); + subject_name_is_ipv6 = true; } #endif @@ -487,15 +490,18 @@ static bool php_openssl_matches_san_list(X509 *peer, const char *subject_name) / return 1; } - } else if (san->d.ip->length == 16 && strlen(ipv6_expanded) >= 15) { /* shortest expanded IPv6 address is 0:0:0:0:0:0:0:0 */ + } +#if defined(HAVE_IPV6) && defined(HAVE_INET_PTON) + else if (san->d.ip->length == 16 && subject_name_is_ipv6) { ipbuffer[0] = 0; EXPAND_IPV6_ADDRESS(ipbuffer, san->d.iPAddress->data); - if (strcasecmp((const char*)ipv6_expanded, (const char*)ipbuffer) == 0) { + if (strcasecmp((const char*)subject_name_ipv6_expanded, (const char*)ipbuffer) == 0) { sk_GENERAL_NAME_pop_free(alt_names, GENERAL_NAME_free); return 1; } } +#endif } } From 7da946e0dcff49123aba32865aa0c29072949730 Mon Sep 17 00:00:00 2001 From: James Lucas Date: Sun, 7 May 2023 12:13:31 +1000 Subject: [PATCH 4/9] snprintf buffer size includes the \0 Co-authored-by: Jakub Zelenka --- ext/openssl/xp_ssl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/openssl/xp_ssl.c b/ext/openssl/xp_ssl.c index e1c188444f047..147c74ace7928 100644 --- a/ext/openssl/xp_ssl.c +++ b/ext/openssl/xp_ssl.c @@ -118,7 +118,7 @@ /* Used for IPv6 Address peer verification */ #define EXPAND_IPV6_ADDRESS(_str, _bytes) \ do { \ - snprintf(_str, 39, "%X:%X:%X:%X:%X:%X:%X:%X", \ + snprintf(_str, 40, "%X:%X:%X:%X:%X:%X:%X:%X", \ _bytes[0] << 8 | _bytes[1], \ _bytes[2] << 8 | _bytes[3], \ _bytes[4] << 8 | _bytes[5], \ From d297059d61886b331e0950354b6ce66731f5d548 Mon Sep 17 00:00:00 2001 From: James Lucas Date: Tue, 9 May 2023 09:46:06 +1000 Subject: [PATCH 5/9] CS: Space after comma --- ext/openssl/xp_ssl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/openssl/xp_ssl.c b/ext/openssl/xp_ssl.c index 147c74ace7928..5b3ad2c1f8863 100644 --- a/ext/openssl/xp_ssl.c +++ b/ext/openssl/xp_ssl.c @@ -447,7 +447,7 @@ static bool php_openssl_matches_san_list(X509 *peer, const char *subject_name) / unsigned char ipv6[16]; bool subject_name_is_ipv6 = false; subject_name_ipv6_expanded[0] = 0; - if (inet_pton(AF_INET6,subject_name,&ipv6)) { + if (inet_pton(AF_INET6, subject_name, &ipv6)) { EXPAND_IPV6_ADDRESS(subject_name_ipv6_expanded, ipv6); subject_name_is_ipv6 = true; } From d445ec5d43c7ad2d96398d928b4db45929198262 Mon Sep 17 00:00:00 2001 From: James Lucas Date: Tue, 9 May 2023 10:13:07 +1000 Subject: [PATCH 6/9] Skip test if IPv6 support is not available Use different port number (possible re-use failure in tests) --- ext/openssl/tests/san_ipv6_peer_matching.phpt | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/ext/openssl/tests/san_ipv6_peer_matching.phpt b/ext/openssl/tests/san_ipv6_peer_matching.phpt index 62504390d2e02..37e7689359c25 100644 --- a/ext/openssl/tests/san_ipv6_peer_matching.phpt +++ b/ext/openssl/tests/san_ipv6_peer_matching.phpt @@ -5,6 +5,10 @@ openssl --SKIPIF-- --FILE-- [ 'local_cert' => '%s', @@ -27,7 +31,7 @@ CODE; $serverCode = sprintf($serverCode, $certFile); $clientCode = <<<'CODE' - $serverUri = "ssl://[::1]:64321"; + $serverUri = "ssl://[::1]:64324"; $clientFlags = STREAM_CLIENT_CONNECT; $clientCtx = stream_context_create(['ssl' => [ 'verify_peer' => false, @@ -60,5 +64,5 @@ Warning: stream_socket_client(): Unable to locate peer certificate CN in %s on l Warning: stream_socket_client(): Failed to enable crypto in %s on line %d -Warning: stream_socket_client(): Unable to connect to ssl://[::1]:64321 (Unknown error) in %s on line %d +Warning: stream_socket_client(): Unable to connect to ssl://[::1]:64324 (Unknown error) in %s on line %d bool(false) From 0c6b7cad299aca192d4bc1a61a55a9a09c35757c Mon Sep 17 00:00:00 2001 From: James Lucas Date: Tue, 9 May 2023 10:22:48 +1000 Subject: [PATCH 7/9] Add News --- NEWS | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/NEWS b/NEWS index bebc691d019fe..7114a76a9ac55 100644 --- a/NEWS +++ b/NEWS @@ -20,6 +20,10 @@ PHP NEWS . Fixed bug GH-11134 (Incorrect match default branch optimization). (ilutov) . Fixed too wide OR and AND range inference. (nielsdos) +- OpenSSL: + . Fixed bug GH-9356 Incomplete validation of + IPv6 Address fields in subjectAltNames (James Lucas, Jakub Zelenka) + - PGSQL: . Fixed parameter parsing of pg_lo_export(). (kocsismate) From 4ab2146c43e608b33500d42e3e3dd77beb5ce6d9 Mon Sep 17 00:00:00 2001 From: James Lucas Date: Tue, 9 May 2023 13:38:44 +1000 Subject: [PATCH 8/9] Skip test when IPv6 is not enabled at compile time or when the CI specifies no IPv6 --- ext/openssl/tests/san_ipv6_peer_matching.phpt | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/ext/openssl/tests/san_ipv6_peer_matching.phpt b/ext/openssl/tests/san_ipv6_peer_matching.phpt index 37e7689359c25..7e9abc499ce72 100644 --- a/ext/openssl/tests/san_ipv6_peer_matching.phpt +++ b/ext/openssl/tests/san_ipv6_peer_matching.phpt @@ -5,9 +5,8 @@ openssl --SKIPIF-- --FILE-- From 002fdbbaedfb340e0da79018c3809831c0704317 Mon Sep 17 00:00:00 2001 From: James Lucas Date: Wed, 10 May 2023 09:29:30 +1000 Subject: [PATCH 9/9] Add additional SKIPIF test for IPv6 support on the host (copied from ext/sockets/tests/ipv6_skipif.inc) --- ext/openssl/tests/san_ipv6_peer_matching.phpt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ext/openssl/tests/san_ipv6_peer_matching.phpt b/ext/openssl/tests/san_ipv6_peer_matching.phpt index 7e9abc499ce72..81966025d3969 100644 --- a/ext/openssl/tests/san_ipv6_peer_matching.phpt +++ b/ext/openssl/tests/san_ipv6_peer_matching.phpt @@ -8,6 +8,8 @@ if (!function_exists("proc_open")) die("skip no proc_open"); if (getenv("CI_NO_IPV6") || !defined("AF_INET6")) { die('skip no IPv6 support'); } +if (@stream_socket_client('udp://[::1]:8888') === false) + die('skip no IPv6 support'); ?> --FILE--