Fix GH-9356: Incomplete validation of IPv6 Address fields in subjectAltNames

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)
 

ext/openssl/tests/san_ipv6_peer_matching.phpt

@@ -0,0 +1,69 @@
+--TEST--
+IPv6 Peer verification matches SAN names
+--EXTENSIONS--
+openssl
+--SKIPIF--
+<?php
+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--
+<?php
+$certFile = __DIR__ . DIRECTORY_SEPARATOR . 'san_ipv6_peer_matching.pem.tmp';
+$san = 'IP:2001:db8:85a3:8d3:1319:8a2e:370:7348';
+
+$serverCode = <<<'CODE'
+    $serverUri = "ssl://[::1]:64324";
+    $serverFlags = STREAM_SERVER_BIND | STREAM_SERVER_LISTEN;
+    $serverCtx = stream_context_create(['ssl' => [
+        '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--
+<?php
+@unlink(__DIR__ . DIRECTORY_SEPARATOR . 'san_ipv6_peer_matching.pem.tmp');
+?>
+--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)

ext/openssl/xp_ssl.c

@@ -39,13 +39,18 @@
 #ifdef PHP_WIN32
 #include "win32/winutil.h"
 #include "win32/time.h"
+#include <Ws2tcpip.h>
 #include <Wincrypt.h>
 /* These are from Wincrypt.h, they conflict with OpenSSL */
 #undef X509_NAME
 #undef X509_CERT_PAIR
 #undef X509_EXTENSIONS
 #endif
 
+#ifdef HAVE_ARPA_INET_H
+#include <arpa/inet.h>
+#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
 		}
 	}