Skip to content

Commit db72816

Browse files
committed
Use only OpenSSLSession for session_data and get_cb ret type
1 parent be77f13 commit db72816

File tree

3 files changed

+28
-45
lines changed

3 files changed

+28
-45
lines changed

ext/openssl/tests/session_resumption_invalid_data.phpt

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -36,10 +36,14 @@ $clientCode = <<<'CODE'
3636
'session_data' => 'this_is_invalid_session_data',
3737
]]);
3838
39-
$client = stream_socket_client("tls://{{ ADDR }}", $errno, $errstr, 30, $flags, $ctx);
40-
41-
if ($client === false) {
42-
echo "Connection failed as expected\n";
39+
try {
40+
$client = stream_socket_client("tls://{{ ADDR }}", $errno, $errstr, 30, $flags, $ctx);
41+
42+
if ($client === false) {
43+
echo "Connection failed unexpectedlyd\n";
44+
}
45+
} catch (\Throwable $e) {
46+
echo "Type error thrown: " . $e->getMessage() . "\n";
4347
}
4448
CODE;
4549

@@ -56,9 +60,7 @@ ServerClientTestCase::getInstance()->run($clientCode, $serverCode);
5660
?>
5761
--EXPECTF--
5862

59-
Warning: stream_socket_client(): Invalid or corrupted session_data, falling back to full handshake in %s on line %d
60-
6163
Warning: stream_socket_client(): Failed to enable crypto in %s on line %d
6264

6365
Warning: stream_socket_client(): Unable to connect to %s in %s on line %d
64-
Connection failed as expected
66+
Type error thrown: session_data must be an OpenSSLSession instance

ext/openssl/tests/session_resumption_new_cb_no_context.phpt

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,6 @@ $clientCode = <<<'CODE'
4949
$ctx = stream_context_create(['ssl' => [
5050
'verify_peer' => false,
5151
'verify_peer_name' => false,
52-
'session_data' => 'this_is_invalid_session_data',
5352
]]);
5453
5554
$client = @stream_socket_client("tls://{{ ADDR }}", $errno, $errstr, 30, $flags, $ctx);

ext/openssl/xp_ssl.c

Lines changed: 19 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -1632,12 +1632,11 @@ static SSL_SESSION *php_openssl_session_get_cb(SSL *ssl, const unsigned char *se
16321632
SSL_SESSION_up_ref(obj->session);
16331633
session = obj->session;
16341634
}
1635-
} else if (Z_TYPE(retval) == IS_STRING && Z_STRLEN(retval) > 0) {
1636-
/* Backward compatibility: accept raw DER string */
1637-
const unsigned char *p = (const unsigned char *)Z_STRVAL(retval);
1638-
session = d2i_SSL_SESSION(NULL, &p, Z_STRLEN(retval));
1635+
zval_ptr_dtor(&retval);
1636+
} else if (Z_TYPE(retval) != IS_NULL) {
1637+
zend_type_error("session_get_cb return type must be null or OpenSSLSession");
1638+
return NULL;
16391639
}
1640-
zval_ptr_dtor(&retval);
16411640
}
16421641

16431642
zval_ptr_dtor(&args[1]);
@@ -1729,6 +1728,15 @@ static zend_result php_openssl_setup_client_session(php_stream *stream,
17291728
bool enable_client_cache = false;
17301729
bool is_persistent = php_stream_is_persistent(stream);
17311730

1731+
if (GET_VER_OPT("session_data")) {
1732+
if (php_openssl_is_session_ce(val)) {
1733+
enable_client_cache = true;
1734+
} else if (Z_TYPE_P(val) != IS_NULL) {
1735+
zend_type_error("session_data must be an OpenSSLSession instance");
1736+
return FAILURE;
1737+
}
1738+
}
1739+
17321740
if (GET_VER_OPT("session_new_cb")) {
17331741
if (FAILURE == php_openssl_validate_and_allocate_callback(
17341742
sslsock, val, "session_new_cb", is_persistent)) {
@@ -1740,13 +1748,6 @@ static zend_result php_openssl_setup_client_session(php_stream *stream,
17401748
enable_client_cache = true;
17411749
}
17421750

1743-
if (GET_VER_OPT("session_data")) {
1744-
if (php_openssl_is_session_ce(val) ||
1745-
(Z_TYPE_P(val) == IS_STRING && Z_STRLEN_P(val) > 0)) {
1746-
enable_client_cache = true;
1747-
}
1748-
}
1749-
17501751
if (enable_client_cache) {
17511752
SSL_CTX_set_session_cache_mode(sslsock->ctx,
17521753
SSL_SESS_CACHE_CLIENT | SSL_SESS_CACHE_NO_INTERNAL);
@@ -1920,18 +1921,9 @@ static zend_result php_openssl_apply_client_session_data(php_stream *stream,
19201921
}
19211922
/* Object owns the session, we just borrow it */
19221923
needs_free = false;
1923-
} else if (Z_TYPE_P(val) == IS_STRING && Z_STRLEN_P(val) > 0) {
1924-
/* Legacy: deserialize session from DER format */
1925-
const unsigned char *p = (const unsigned char *)Z_STRVAL_P(val);
1926-
session = d2i_SSL_SESSION(NULL, &p, Z_STRLEN_P(val));
1927-
1928-
if (!session) {
1929-
php_error_docref(NULL, E_WARNING,
1930-
"Invalid or corrupted session_data, falling back to full handshake");
1931-
ERR_clear_error();
1932-
return FAILURE;
1933-
}
1934-
needs_free = true;
1924+
} else if (Z_TYPE_P(val) != IS_NULL) {
1925+
zend_type_error("session_data must be an OpenSSLSession instance");
1926+
return FAILURE;
19351927
}
19361928

19371929
if (session) {
@@ -2076,24 +2068,14 @@ static zend_result php_openssl_create_server_ctx(php_stream *stream,
20762068
SSL_CTX_set_min_proto_version(sslsock->ctx, php_openssl_get_min_proto_version(method_flags));
20772069
SSL_CTX_set_max_proto_version(sslsock->ctx, php_openssl_get_max_proto_version(method_flags));
20782070

2079-
if (sslsock->is_client == 0 &&
2080-
PHP_STREAM_CONTEXT(stream) &&
2081-
FAILURE == php_openssl_set_server_specific_opts(stream, sslsock->ctx)
2082-
) {
2083-
return FAILURE;
2084-
}
2085-
20862071
if (sslsock->is_client) {
20872072
/* Setup client session resumption */
20882073
if (FAILURE == php_openssl_setup_client_session(stream, sslsock)) {
20892074
return FAILURE;
20902075
}
2091-
} else {
2092-
/* Setup server session resumption */
2093-
if (PHP_STREAM_CONTEXT(stream)) {
2094-
if (FAILURE == php_openssl_setup_server_session(stream, sslsock)) {
2095-
return FAILURE;
2096-
}
2076+
} else if (PHP_STREAM_CONTEXT(stream)) {
2077+
if (FAILURE == php_openssl_setup_server_session(stream, sslsock)) {
2078+
return FAILURE;
20972079
}
20982080
/* Original server-specific setup */
20992081
if (FAILURE == php_openssl_set_server_specific_opts(stream, sslsock->ctx)) {

0 commit comments

Comments
 (0)