Skip to content

Commit 8300ced

Browse files
committed
* 'bug65729' of https://github.com/datibbaw/php-src: DNS name comparison is now case insensitive. Use zend_bool as return value for _match() Added two more test cases for CN matching. yay, reduced one variable Fixed bug that would lead to out of bounds memory access added better wildcard matching for CN
2 parents 0d8c83a + 6106896 commit 8300ced

File tree

3 files changed

+115
-13
lines changed

3 files changed

+115
-13
lines changed

ext/openssl/openssl.c

Lines changed: 33 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -4951,6 +4951,38 @@ static int verify_callback(int preverify_ok, X509_STORE_CTX *ctx) /* {{{ */
49514951
}
49524952
/* }}} */
49534953

4954+
static zend_bool php_openssl_match_cn(const char *subjectname, const char *certname)
4955+
{
4956+
char *wildcard;
4957+
int prefix_len, suffix_len, subject_len;
4958+
4959+
if (strcasecmp(subjectname, certname) == 0) {
4960+
return 1;
4961+
}
4962+
4963+
if (!(wildcard = strchr(certname, '*'))) {
4964+
return 0;
4965+
}
4966+
4967+
// 1) prefix, if not empty, must match subject
4968+
prefix_len = wildcard - certname;
4969+
if (prefix_len && strncasecmp(subjectname, certname, prefix_len) != 0) {
4970+
return 0;
4971+
}
4972+
4973+
suffix_len = strlen(wildcard + 1);
4974+
subject_len = strlen(subjectname);
4975+
if (suffix_len <= subject_len) {
4976+
/* 2) suffix must match
4977+
* 3) no . between prefix and suffix
4978+
**/
4979+
return strcasecmp(wildcard + 1, subjectname + subject_len - suffix_len) == 0 &&
4980+
memchr(subjectname + prefix_len, '.', subject_len - suffix_len - prefix_len) == NULL;
4981+
}
4982+
4983+
return 0;
4984+
}
4985+
49544986
int php_openssl_apply_verification_policy(SSL *ssl, X509 *peer, php_stream *stream TSRMLS_DC) /* {{{ */
49554987
{
49564988
zval **val = NULL;
@@ -5003,7 +5035,6 @@ int php_openssl_apply_verification_policy(SSL *ssl, X509 *peer, php_stream *stre
50035035
/* Does the common name match ? (used primarily for https://) */
50045036
GET_VER_OPT_STRING("CN_match", cnmatch);
50055037
if (cnmatch) {
5006-
int match = 0;
50075038
int name_len = X509_NAME_get_text_by_NID(name, NID_commonName, buf, sizeof(buf));
50085039

50095040
if (name_len == -1) {
@@ -5014,18 +5045,7 @@ int php_openssl_apply_verification_policy(SSL *ssl, X509 *peer, php_stream *stre
50145045
return FAILURE;
50155046
}
50165047

5017-
match = strcmp(cnmatch, buf) == 0;
5018-
if (!match && strlen(buf) > 3 && buf[0] == '*' && buf[1] == '.') {
5019-
/* Try wildcard */
5020-
5021-
if (strchr(buf+2, '.')) {
5022-
char *tmp = strstr(cnmatch, buf+1);
5023-
5024-
match = tmp && strcmp(tmp, buf+2) && tmp == strchr(cnmatch, '.');
5025-
}
5026-
}
5027-
5028-
if (!match) {
5048+
if (!php_openssl_match_cn(cnmatch, buf)) {
50295049
/* didn't match */
50305050
php_error_docref(NULL TSRMLS_CC, E_WARNING, "Peer certificate CN=`%.*s' did not match expected CN=`%s'", name_len, buf, cnmatch);
50315051
return FAILURE;

ext/openssl/tests/bug65729.pem

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
-----BEGIN CERTIFICATE-----
2+
MIICCTCCAXICCQDNMI29sowT7TANBgkqhkiG9w0BAQUFADBJMQswCQYDVQQGEwJT
3+
RzESMBAGA1UECBMJVGVzdHZpbGxlMREwDwYDVQQKEwhkYXRpYmJhdzETMBEGA1UE
4+
AxQKKi50ZXN0LmNvbTAeFw0xMzA5MjEwNzUyMjRaFw0xNDA5MjEwNzUyMjRaMEkx
5+
CzAJBgNVBAYTAlNHMRIwEAYDVQQIEwlUZXN0dmlsbGUxETAPBgNVBAoTCGRhdGli
6+
YmF3MRMwEQYDVQQDFAoqLnRlc3QuY29tMIGfMA0GCSqGSIb3DQEBAQUAA4GNADCB
7+
iQKBgQCdzVnic8K5W4SVbwVuqezcTjeqVLoQ91vVNZB0Jnsuz6q3DoK03oAd1jTe
8+
Vd0k+MQDbXpHoc37lA4+8z/g5Bs0UXxNx+nkbFTE7Ba2/G24caI9/cOXZPG3UViD
9+
rtqXKL6h5/umqRG9Dt5liF2MVP9XFAesVC7B8+Ca+PbPlQoYzwIDAQABMA0GCSqG
10+
SIb3DQEBBQUAA4GBAAS07u/Ke+EhEHidz6CG3Qcr+zg483JKRgZFyGz+YUKyyKKy
11+
fmLs7JieGJxYQjOmIpj/6X9Gnb2HjIPDnI6A+MV1emXDTnnmsgf2/lZGcthhpZn2
12+
rMbj9bI0iH6HwOVGtp4ZJA5fB7nj3J+gWNTCQzDDOxwX36d2LL9ua+UMnk/g
13+
-----END CERTIFICATE-----
14+
-----BEGIN RSA PRIVATE KEY-----
15+
MIICXQIBAAKBgQCdzVnic8K5W4SVbwVuqezcTjeqVLoQ91vVNZB0Jnsuz6q3DoK0
16+
3oAd1jTeVd0k+MQDbXpHoc37lA4+8z/g5Bs0UXxNx+nkbFTE7Ba2/G24caI9/cOX
17+
ZPG3UViDrtqXKL6h5/umqRG9Dt5liF2MVP9XFAesVC7B8+Ca+PbPlQoYzwIDAQAB
18+
AoGAeyzTwKPDl5QMRejHQL57GOwlH1vLcXrjv+VzwHZZKQ0IoKM++5fCQYf29KXp
19+
XPahaluGW2u9sWa8R/7wGcd0Q4RtquGzsgT3+AQsIc5KfIamyOyDaRVM/ymX3fWg
20+
gHIU7OOzB+ihOU8sHyRIwfbk01/kmrBXLRj8E31sy3i3PIECQQDQQYE+aN7Acrdt
21+
yN5CaqvbkiCGjRvASlemiTzPosgOtndyp21w1gakJwKYhYDk1N6A6Qb8REMZqM/U
22+
wFypldV/AkEAwfq6NFuhpGL6hDA7MvlyY1KiZ0cHetPUX+PgdNqy2DA+1Sv4i7gm
23+
Wd/uA651K7aPXuUaf9dKtPCmZwI4M6SEsQJBALW89HTqP7niYoDEEnITdPaghxHk
24+
gptERUln6lGo1L1CLus3gSI/JHyMLo+7scgAnEwTD62GRKhX0Ubwt+ymfTECQAY5
25+
fHYnppU20+EgBxZIqOIFCc8UmWnYmE0Ha/Fz/x8u1SVUBuK84wYpSGL32yyu7ATY
26+
hzQo/W229zABAzqtAdECQQCUdB7IBFpPnsfv/EUBFX7X/7zAc9JpACmu9It5ju8C
27+
KIsMuz/02D+TQoJNjdAngBM+4AJDIaGFgTMIfaDMh5L7
28+
-----END RSA PRIVATE KEY-----

ext/openssl/tests/bug65729.phpt

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
--TEST--
2+
Bug #65729: CN_match gives false positive when wildcard is used
3+
--SKIPIF--
4+
<?php
5+
if (!extension_loaded("openssl")) die("skip");
6+
if (!function_exists('pcntl_fork')) die("skip no fork");
7+
--FILE--
8+
<?php
9+
$context = stream_context_create();
10+
11+
stream_context_set_option($context, 'ssl', 'local_cert', __DIR__ . "/bug65729.pem");
12+
stream_context_set_option($context, 'ssl', 'allow_self_signed', true);
13+
$server = stream_socket_server('ssl://127.0.0.1:64321', $errno, $errstr,
14+
STREAM_SERVER_BIND|STREAM_SERVER_LISTEN, $context);
15+
16+
$expected_names = array('foo.test.com.sg', 'foo.test.com', 'FOO.TEST.COM', 'foo.bar.test.com');
17+
18+
$pid = pcntl_fork();
19+
if ($pid == -1) {
20+
die('could not fork');
21+
} else if ($pid) {
22+
foreach ($expected_names as $expected_name) {
23+
$contextC = stream_context_create(array(
24+
'ssl' => array(
25+
'verify_peer' => true,
26+
'allow_self_signed' => true,
27+
'CN_match' => $expected_name,
28+
)
29+
));
30+
var_dump(stream_socket_client("ssl://127.0.0.1:64321", $errno, $errstr, 1,
31+
STREAM_CLIENT_CONNECT, $contextC));
32+
}
33+
} else {
34+
@pcntl_wait($status);
35+
foreach ($expected_names as $name) {
36+
@stream_socket_accept($server, 1);
37+
}
38+
}
39+
--EXPECTF--
40+
Warning: stream_socket_client(): Peer certificate CN=`*.test.com' did not match expected CN=`foo.test.com.sg' in %s on line %d
41+
42+
Warning: stream_socket_client(): Failed to enable crypto in %s on line %d
43+
44+
Warning: stream_socket_client(): unable to connect to ssl://127.0.0.1:64321 (Unknown error) in %s on line %d
45+
bool(false)
46+
resource(%d) of type (stream)
47+
resource(%d) of type (stream)
48+
49+
Warning: stream_socket_client(): Peer certificate CN=`*.test.com' did not match expected CN=`foo.bar.test.com' in %s on line %d
50+
51+
Warning: stream_socket_client(): Failed to enable crypto in %s on line %d
52+
53+
Warning: stream_socket_client(): unable to connect to ssl://127.0.0.1:64321 (Unknown error) in %s on line %d
54+
bool(false)

0 commit comments

Comments
 (0)