Fixed bug #68879 (IP Address fields in subjectAltNames not used)

Author: rdlowrey

NEWS

@@ -36,6 +36,8 @@
     socket timeouts). (Brad Broerman)
   . Fixed bug #68920 (use strict peer_fingerprint input checks)
     (Daniel Lowrey)
+  . Fixed bug #68879 (IP Address fields in subjectAltNames not used)
+    (Daniel Lowrey)
 
 - pgsql:
   . Fixed bug #68638 (pg_update() fails to store infinite values).

ext/openssl/tests/bug68879.pem

@@ -0,0 +1,33 @@
+-----BEGIN ENCRYPTED PRIVATE KEY-----
+MIICxjBABgkqhkiG9w0BBQ0wMzAbBgkqhkiG9w0BBQwwDgQIuw/AFD7RWcMCAggA
+MBQGCCqGSIb3DQMHBAh98F6GmaGn1ASCAoBpzaFxyttEhyh4dhMjarJIqTz24DjO
+yZnp1K5qthejSYx2P28uUsh+gQOh6F2jbVAq++eAWkTBGuc4pWhhoT7nY8vhf0Y0
+6yTlVrTxuI/8MNo/lfa0xE/+ZD4B5zp0hQxfij4GTd8l6V/kpXMgiYD1JmIXArm7
+sucn+9XV3RucsTBpeIJ1nLEDfpbyEWqNfhoyskQ+S3I6HkMgELI9JpsO6OR9fh1Q
+ttdoYxBU+YjoDYcSWRGkTGrJFeGGhTQzz+L2ijgoqNWDSfrLBoQR1bqNVUuw6gcE
+9PpA/vpRlxcHbUNNkOWft+4e0tV3I2EqscEcsYeNbd2Ta4yu7f6pk4/Kxn40wrQ8
+6Ss9GZylghaFth2xppL/vpmGaCC7FqpZRh+NKqjlcBobIkwyRcsQrPHB0CYLPHA4
+yak/dNTY8L5K8Rtd5XG3+E41CoDF6ssNY0Kw7l9kAn/neDVh+WnQkWIiWPmq210a
+p4L/uiXRK7aYi+UqKJ5+svayNw2w1dkqpbeejwLq2F1+ek/447JFPVJcvP8Nm7sr
+04Mcg+ZHusZdjiWEv4W6CBq8o6eF2JdhfpSDgPkHwiZ/EarHfx0vcYIMJhlEQBmk
+a/XsZPk2wnamKSPfJautO3MIus0M6SniWF6eDA4/AZzSjXV8Vc0unb6lc+Nc8tJa
+6MU1soTsmki/YraCmQswqpL+kXFZVeHuLowOC5oH+CimQoscmiZ9tBvpnYo6XwEZ
+S9jZRIBQ77oMku+rlMPfz2FURgVXZpEfrGmxKvA5Vt3ojrYfTwwD2YqZHVcm39zy
+iKqA1qVt7A2A90ILMAzYnN0VRE4SO3yIDN1ZBp5OOY61AduPrhpaHl81
+-----END ENCRYPTED PRIVATE KEY-----
+-----BEGIN CERTIFICATE-----
+MIICkTCCAfqgAwIBAgIJAPbIVRT31Al1MA0GCSqGSIb3DQEBCwUAMFgxCzAJBgNV
+BAYTAlhYMRUwEwYDVQQHDAxEZWZhdWx0IENpdHkxHDAaBgNVBAoME0RlZmF1bHQg
+Q29tcGFueSBMdGQxFDASBgNVBAMMC3BocCB0ZXN0IGNhMB4XDTE1MDMwNTA0NTQx
+NVoXDTI1MDMwMjA0NTQxNVowUjELMAkGA1UEBhMCVVMxCzAJBgNVBAgMAlNDMRUw
+EwYDVQQHDAxNeXJ0bGUgQmVhY2gxDDAKBgNVBAsMA1BIUDERMA8GA1UEAwwIdGVz
+dC5jb20wgZ8wDQYJKoZIhvcNAQEBBQADgY0AMIGJAoGBAKp5gxUbKvY5eFwZJti0
+6d6YBo400Or6M+bLfIMnz5C1WQ7dMfiQpeFLpSIlOIaFqyrqkeeR9k5dsx1K9FOu
+PAJ4+lmWA4R93RpdJFz8kmQoNu3P59JMATXi8wvNBIrN/Vc08NT0wBRImeyQSVHd
+UcFIXBEbBM0dQsPKQ1k8n5WDAgMBAAGjaTBnMAkGA1UdEwQCMAAwCwYDVR0PBAQD
+AgXgME0GA1UdEQRGMESCCHRlc3QuY29tggx3d3cudGVzdC5jb22CEnN1YmRvbWFp
+bi50ZXN0LmNvbYcQAAAAAAAAAAAAAP//CgIAAYcECgIAATANBgkqhkiG9w0BAQsF
+AAOBgQBZ4TptNXV85gNj3wcB5feWFcwKO8cN4hwnhrbqiHN280r9O/g1CQiLmB4K
+2txrJt06UNCnvWse7CdvsN14wu6rRGRk/+7M36NBw5ERkAzp5HXgZUWVdMl3YltB
+PpqbLhGGrkVn7/jW2FdAxfax7qaGDYgC3qcQNfiK6K92SPxV7Q==
+-----END CERTIFICATE-----

ext/openssl/tests/bug68879.phpt

@@ -0,0 +1,41 @@
+--TEST--
+Bug #68879: Match IP address fields in subjectAltName checks
+--SKIPIF--
+<?php
+if (!extension_loaded("openssl")) die("skip openssl not loaded");
+if (!function_exists("proc_open")) die("skip no proc_open");
+--FILE--
+<?php
+$serverCode = <<<'CODE'
+    $serverUri = "ssl://127.0.0.1:64321";
+    $serverFlags = STREAM_SERVER_BIND | STREAM_SERVER_LISTEN;
+    $serverCtx = stream_context_create(['ssl' => [
+        'local_cert' => __DIR__ . '/bug68879.pem',
+        'passphrase' => 'elephpant',
+    ]]);
+
+    $server = stream_socket_server($serverUri, $errno, $errstr, $serverFlags, $serverCtx);
+    phpt_notify();
+
+    stream_socket_accept($server, 30);
+CODE;
+
+$clientCode = <<<'CODE'
+    $serverUri = "ssl://127.0.0.1:64321";
+    $clientFlags = STREAM_CLIENT_CONNECT;
+    $clientCtx = stream_context_create(['ssl' => [
+        'verify_peer' => false,
+        'verify_peer_name' => true,
+        'peer_name' => '10.2.0.1',
+    ]]);
+
+    phpt_wait();
+
+    var_dump(stream_socket_client($serverUri, $errno, $errstr, 30, $clientFlags, $clientCtx));
+CODE;
+
+include 'ServerClientTestCase.inc';
+ServerClientTestCase::getInstance()->run($clientCode, $serverCode);
+--EXPECTF--
+resource(%d) of type (stream)
+

ext/openssl/xp_ssl.c

@@ -370,40 +370,51 @@ static zend_bool matches_wildcard_name(const char *subjectname, const char *cert
 }
 /* }}} */
 
-static zend_bool matches_san_list(X509 *peer, const char *subject_name TSRMLS_DC) /* {{{ */
+static zend_bool matches_san_list(X509 *peer, const char *subject_name) /* {{{ */
 {
-	int i, san_name_len;
-	zend_bool is_match = 0;
+	int i;
 	unsigned char *cert_name = NULL;
+	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);
 
 	for (i = 0; i < alt_name_count; i++) {
 		GENERAL_NAME *san = sk_GENERAL_NAME_value(alt_names, i);
-		if (san->type != GEN_DNS) {
-			/* we only care about DNS names */
-			continue;
-		}
-
-		san_name_len = ASN1_STRING_length(san->d.dNSName);
-		ASN1_STRING_to_UTF8(&cert_name, san->d.dNSName);
-
-		/* prevent null byte poisoning */
-		if (san_name_len != strlen((const char*)cert_name)) {
-			php_error_docref(NULL TSRMLS_CC, E_WARNING, "Peer SAN entry is malformed");
-		} else {
-			is_match = matches_wildcard_name(subject_name, (const char *)cert_name);
-		}
 
-		OPENSSL_free(cert_name);
+		if (san->type == GEN_DNS) {
+			ASN1_STRING_to_UTF8(&cert_name, san->d.dNSName);
+			if (ASN1_STRING_length(san->d.dNSName) != strlen((const char*)cert_name)) {
+				OPENSSL_free(cert_name);
+				/* prevent null-byte poisoning*/
+				continue;
+			}
 
-		if (is_match) {
-			break;
+			if (matches_wildcard_name(subject_name, (const char *)cert_name)) {
+				OPENSSL_free(cert_name);
+				return 1;
+			}
+			OPENSSL_free(cert_name);
+		} else if (san->type == GEN_IPADD) {
+			if (san->d.iPAddress->length == 4) {
+				sprintf(ipbuffer, "%d.%d.%d.%d",
+					san->d.iPAddress->data[0],
+					san->d.iPAddress->data[1],
+					san->d.iPAddress->data[2],
+					san->d.iPAddress->data[3]
+				);
+				if (strcasecmp(subject_name, (const char*)ipbuffer) == 0) {
+					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.
+			 */
 		}
 	}
 
-	return is_match;
+	return 0;
 }
 /* }}} */
 
@@ -515,7 +526,7 @@ static int apply_peer_verification_policy(SSL *ssl, X509 *peer, php_stream *stre
 		}
 
 		if (peer_name) {
-			if (matches_san_list(peer, peer_name TSRMLS_CC)) {
+			if (matches_san_list(peer, peer_name)) {
 				return SUCCESS;
 			} else if (matches_common_name(peer, peer_name TSRMLS_CC)) {
 				return SUCCESS;