From fd09728bb6a5c8f7c7320ae1e60a2db48c765ce6 Mon Sep 17 00:00:00 2001 From: James Lucas Date: Fri, 28 Apr 2023 11:02:49 +1000 Subject: [PATCH] Fix bug GH-9356: Incomplete SAN validation of IPv6 address 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. Closes GH-11145 --- NEWS | 4 ++ ext/openssl/tests/san_ipv6_peer_matching.phpt | 69 +++++++++++++++++++ ext/openssl/xp_ssl.c | 47 +++++++++++-- 3 files changed, 116 insertions(+), 4 deletions(-) create mode 100644 ext/openssl/tests/san_ipv6_peer_matching.phpt diff --git a/NEWS b/NEWS index 33d789b6b17..8eae4ccaa5e 100644 --- a/NEWS +++ b/NEWS @@ -34,6 +34,10 @@ PHP NEWS . Fixed bug GH-11336 (php still tries to unlock the shared memory ZendSem with opcache.file_cache_only=1 but it was never locked). (nielsdos) +- OpenSSL: + . Fixed bug GH-9356 Incomplete validation of IPv6 Address fields in + subjectAltNames (James Lucas, Jakub Zelenka). + - SPL: . Fixed bug GH-11338 (SplFileInfo empty getBasename with more than one slash). (nielsdos) 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 00000000000..81966025d39 --- /dev/null +++ b/ext/openssl/tests/san_ipv6_peer_matching.phpt @@ -0,0 +1,69 @@ +--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]:64324"; + $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]:64324 (Unknown error) in %s on line %d +bool(false) diff --git a/ext/openssl/xp_ssl.c b/ext/openssl/xp_ssl.c index 9aac4a0b70a..5b3ad2c1f88 100644 --- a/ext/openssl/xp_ssl.c +++ b/ext/openssl/xp_ssl.c @@ -39,6 +39,7 @@ #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 @@ -46,6 +47,10 @@ #undef X509_EXTENSIONS #endif +#ifdef HAVE_ARPA_INET_H +#include +#endif + /* Flags for determining allowed stream crypto methods */ #define STREAM_CRYPTO_IS_CLIENT (1<<0) #define STREAM_CRYPTO_METHOD_SSLv2 (1<<1) @@ -110,6 +115,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, 40, "%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 @@ -421,6 +441,18 @@ static bool php_openssl_matches_san_list(X509 *peer, const char *subject_name) / 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); +#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(subject_name_ipv6_expanded, ipv6); + subject_name_is_ipv6 = true; + } +#endif + for (i = 0; i < alt_name_count; i++) { GENERAL_NAME *san = sk_GENERAL_NAME_value(alt_names, i); @@ -459,10 +491,17 @@ static bool php_openssl_matches_san_list(X509 *peer, const char *subject_name) / 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. - */ +#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*)subject_name_ipv6_expanded, (const char*)ipbuffer) == 0) { + sk_GENERAL_NAME_pop_free(alt_names, GENERAL_NAME_free); + + return 1; + } + } +#endif } }