Skip to content

Commit 241f3c3

Browse files
rdlowreyrdlowrey
committed
Fixed bug #68920 (use strict peer_fingerprint input checks)
1 parent a29b64f commit 241f3c3

File tree

4 files changed

+82
-8
lines changed

4 files changed

+82
-8
lines changed

‎NEWS

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,8 @@
3434
. Fixed bug #68912 (Segmentation fault at openssl_spki_new). (Laruence)
3535
. Fixed bug #61285, #68329, #68046, #41631 (encrypted streams don't observe
3636
socket timeouts). (Brad Broerman)
37+
. Fixed bug #68920 (use strict peer_fingerprint input checks)
38+
(Daniel Lowrey)
3739

3840
- pgsql:
3941
. Fixed bug #68638 (pg_update() fails to store infinite values).

‎ext/openssl/tests/bug68920.phpt

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
--TEST--
2+
Bug #68920: peer_fingerprint input checks should be strict
3+
--SKIPIF--
4+
<?php
5+
if (!extension_loaded("openssl")) die("skip openssl not loaded");
6+
--FILE--
7+
<?php
8+
error_reporting(E_ALL);
9+
10+
$ctx = stream_context_create(['ssl' => ['verify_peer'=> false, 'peer_fingerprint' => true]]);
11+
$sock = stream_socket_client("ssl://php.net:443", $errno, $errstr, 30, STREAM_CLIENT_CONNECT, $ctx);
12+
var_dump($sock);
13+
14+
$ctx = stream_context_create(['ssl' => ['verify_peer'=> false, 'peer_fingerprint' => null]]);
15+
$sock = stream_socket_client("ssl://php.net:443", $errno, $errstr, 30, STREAM_CLIENT_CONNECT, $ctx);
16+
var_dump($sock);
17+
18+
$ctx = stream_context_create(['ssl' => ['verify_peer'=> false, 'peer_fingerprint' => []]]);
19+
$sock = stream_socket_client("ssl://php.net:443", $errno, $errstr, 30, STREAM_CLIENT_CONNECT, $ctx);
20+
var_dump($sock);
21+
22+
$ctx = stream_context_create(['ssl' => ['verify_peer'=> false, 'peer_fingerprint' => ['foo']]]);
23+
$sock = stream_socket_client("ssl://php.net:443", $errno, $errstr, 30, STREAM_CLIENT_CONNECT, $ctx);
24+
var_dump($sock);
25+
--EXPECTF--
26+
27+
Warning: stream_socket_client(): Expected peer fingerprint must be a string or an array in %s on line %d
28+
29+
Warning: stream_socket_client(): Failed to enable crypto in %s on line %d
30+
31+
Warning: stream_socket_client(): unable to connect to %s (Unknown error) in %s on line %d
32+
bool(false)
33+
34+
Warning: stream_socket_client(): Expected peer fingerprint must be a string or an array in %s on line %d
35+
36+
Warning: stream_socket_client(): Failed to enable crypto in %s on line %d
37+
38+
Warning: stream_socket_client(): unable to connect to %s (Unknown error) in %s on line %d
39+
bool(false)
40+
41+
Warning: stream_socket_client(): Invalid peer_fingerprint array; [algo => fingerprint] form required in %s on line %d
42+
43+
Warning: stream_socket_client(): peer_fingerprint match failure in %s on line %d
44+
45+
Warning: stream_socket_client(): Failed to enable crypto in %s on line %d
46+
47+
Warning: stream_socket_client(): unable to connect to %s (Unknown error) in %s on line %d
48+
bool(false)
49+
50+
Warning: stream_socket_client(): Invalid peer_fingerprint array; [algo => fingerprint] form required in %s on line %d
51+
52+
Warning: stream_socket_client(): peer_fingerprint match failure in %s on line %d
53+
54+
Warning: stream_socket_client(): Failed to enable crypto in %s on line %d
55+
56+
Warning: stream_socket_client(): unable to connect to %s (Unknown error) in %s on line %d
57+
bool(false)

‎ext/openssl/tests/openssl_peer_fingerprint.phpt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ CODE;
4545
include 'ServerClientTestCase.inc';
4646
ServerClientTestCase::getInstance()->run($clientCode, $serverCode);
4747
--EXPECTF--
48-
Warning: stream_socket_client(): Peer fingerprint doesn't match in %s on line %d
48+
Warning: stream_socket_client(): peer_fingerprint match failure in %s on line %d
4949

5050
Warning: stream_socket_client(): Failed to enable crypto in %s on line %d
5151

‎ext/openssl/xp_ssl.c

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -298,28 +298,41 @@ static zend_bool php_x509_fingerprint_match(X509 *peer, zval *val TSRMLS_DC)
298298
}
299299

300300
return method && php_x509_fingerprint_cmp(peer, method, Z_STRVAL_P(val) TSRMLS_CC) == 0;
301+
301302
} else if (Z_TYPE_P(val) == IS_ARRAY) {
302303
HashPosition pos;
303304
zval **current;
304305
char *key;
305306
uint key_len;
306307
ulong key_index;
307308

309+
if (!zend_hash_num_elements(Z_ARRVAL_P(val))) {
310+
php_error_docref(NULL, E_WARNING, "Invalid peer_fingerprint array; [algo => fingerprint] form required");
311+
return 0;
312+
}
313+
308314
for (zend_hash_internal_pointer_reset_ex(Z_ARRVAL_P(val), &pos);
309315
zend_hash_get_current_data_ex(Z_ARRVAL_P(val), (void **)&current, &pos) == SUCCESS;
310316
zend_hash_move_forward_ex(Z_ARRVAL_P(val), &pos)
311317
) {
312318
int key_type = zend_hash_get_current_key_ex(Z_ARRVAL_P(val), &key, &key_len, &key_index, 0, &pos);
313319

314-
if (key_type == HASH_KEY_IS_STRING
315-
&& Z_TYPE_PP(current) == IS_STRING
316-
&& php_x509_fingerprint_cmp(peer, key, Z_STRVAL_PP(current) TSRMLS_CC) != 0
317-
) {
320+
if (!(key_type == HASH_KEY_IS_STRING && Z_TYPE_PP(current) == IS_STRING)) {
321+
php_error_docref(NULL, E_WARNING, "Invalid peer_fingerprint array; [algo => fingerprint] form required");
322+
return 0;
323+
}
324+
if (php_x509_fingerprint_cmp(peer, key, Z_STRVAL_PP(current) TSRMLS_CC) != 0) {
318325
return 0;
319326
}
320327
}
328+
321329
return 1;
330+
331+
} else {
332+
php_error_docref(NULL, E_WARNING,
333+
"Invalid peer_fingerprint value; fingerprint string or array of the form [algo => fingerprint] required");
322334
}
335+
323336
return 0;
324337
}
325338

@@ -439,7 +452,7 @@ static int apply_peer_verification_policy(SSL *ssl, X509 *peer, php_stream *stre
439452
? zend_is_true(*val)
440453
: sslsock->is_client;
441454

442-
must_verify_fingerprint = (GET_VER_OPT("peer_fingerprint") && zend_is_true(*val));
455+
must_verify_fingerprint = GET_VER_OPT("peer_fingerprint");
443456

444457
if ((must_verify_peer || must_verify_peer_name || must_verify_fingerprint) && peer == NULL) {
445458
php_error_docref(NULL TSRMLS_CC, E_WARNING, "Could not get peer certificate");
@@ -474,14 +487,15 @@ static int apply_peer_verification_policy(SSL *ssl, X509 *peer, php_stream *stre
474487
if (Z_TYPE_PP(val) == IS_STRING || Z_TYPE_PP(val) == IS_ARRAY) {
475488
if (!php_x509_fingerprint_match(peer, *val TSRMLS_CC)) {
476489
php_error_docref(NULL TSRMLS_CC, E_WARNING,
477-
"Peer fingerprint doesn't match"
490+
"peer_fingerprint match failure"
478491
);
479492
return FAILURE;
480493
}
481494
} else {
482495
php_error_docref(NULL TSRMLS_CC, E_WARNING,
483496
"Expected peer fingerprint must be a string or an array"
484497
);
498+
return FAILURE;
485499
}
486500
}
487501

@@ -1894,14 +1908,15 @@ static size_t php_openssl_sockop_io(int read, php_stream *stream, char *buf, siz
18941908
break;
18951909

18961910
/* Otherwise, we need to wait again (up to time_left or we get an error) */
1897-
if (blocked)
1911+
if (blocked) {
18981912
if (read) {
18991913
php_pollfd_for(sslsock->s.socket, (err == SSL_ERROR_WANT_WRITE) ?
19001914
(POLLOUT|POLLPRI) : (POLLIN|POLLPRI), has_timeout ? &left_time : NULL);
19011915
} else {
19021916
php_pollfd_for(sslsock->s.socket, (err == SSL_ERROR_WANT_READ) ?
19031917
(POLLIN|POLLPRI) : (POLLOUT|POLLPRI), has_timeout ? &left_time : NULL);
19041918
}
1919+
}
19051920
}
19061921
/* Finally, we keep going until we got data, and an SSL_ERROR_NONE, unless we had an error. */
19071922
} while (retry);

0 commit comments

Comments
 (0)