Skip to content

Commit a9359a5

Browse files
bukkabukka
committed
Fix session errors, session_id_context handling and tests
1 parent e17bb5d commit a9359a5

8 files changed

+44
-45
lines changed

‎ext/openssl/tests/ServerClientTestCase.inc‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,9 @@ class ServerClientTestCase
179179
if (empty($addr)) {
180180
throw new \Exception("Failed server start");
181181
}
182+
if (strpos($addr, 'SERVER_EXCEPTION') !== false) {
183+
echo $addr;
184+
}
182185
if ($code === false) {
183186
$clientCode = preg_replace('/{{\s*ADDR\s*}}/', $addr, $clientCode);
184187
} else {

‎ext/openssl/tests/session_resumption_client_basic.phpt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ $serverCode = <<<'CODE'
1515
$ctx = stream_context_create(['ssl' => [
1616
'local_cert' => '%s',
1717
'session_cache' => true,
18+
'session_id_context' => 'test-basic',
1819
]]);
1920
2021
$server = stream_socket_server('tls://127.0.0.1:0', $errno, $errstr, $flags, $ctx);

‎ext/openssl/tests/session_resumption_get_cb_num_tickets_zero.phpt‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -103,9 +103,10 @@ ServerClientTestCase::getInstance()->run($clientCode, $serverCode);
103103
<?php
104104
@unlink(__DIR__ . DIRECTORY_SEPARATOR . 'session_no_tickets_zero.pem.tmp');
105105
?>
106-
--EXPECTF--
106+
--EXPECT--
107+
Client first connection resumed: no
107108
Response 1
108109
Client received tickets on first connection: 0
110+
Client second connection resumed: no
109111
Response 2
110-
Client second connection resumed: yes
111112
Server: NEW_CB_CALLS:0

‎ext/openssl/tests/session_resumption_new_cb_no_context.phpt‎

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -28,12 +28,16 @@ $serverCode = <<<'CODE'
2828
$server = @stream_socket_server('tls://127.0.0.1:0', $errno, $errstr, $flags, $ctx);
2929
phpt_notify_server_start($server);
3030
31-
$client = @stream_socket_accept($server, 30);
32-
if ($client === false) {
33-
phpt_notify(message: "SERVER_FAILED_AS_EXPECTED");
34-
} else {
35-
phpt_notify(message: "SERVER_CREATED_UNEXPECTEDLY");
36-
fclose($server);
31+
try {
32+
$client = @stream_socket_accept($server, 30);
33+
if ($client === false) {
34+
phpt_notify(message: "SERVER_FAILED_UNEXPECTEDLY");
35+
} else {
36+
phpt_notify(message: "SERVER_CREATED_UNEXPECTEDLY");
37+
fclose($server);
38+
}
39+
} catch (\Throwable $e) {
40+
phpt_notify(message: "SERVER_EXCEPTION: " . $e->getMessage());
3741
}
3842
CODE;
3943
$serverCode = sprintf($serverCode, $certFile, $caCertFile);
@@ -73,4 +77,4 @@ ServerClientTestCase::getInstance()->run($clientCode, $serverCode);
7377
?>
7478
--EXPECT--
7579
Connection failed as expected
76-
SERVER_FAILED_AS_EXPECTED
80+
SERVER_EXCEPTION: session_id_context must be set if session_new_cb is set

‎ext/openssl/tests/session_resumption_require_new_cb.phpt‎

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -21,16 +21,19 @@ $serverCode = <<<'CODE'
2121
}
2222
]]);
2323
24-
$server = @stream_socket_server('tls://127.0.0.1:0', $errno, $errstr, $flags, $ctx);
25-
phpt_notify_server_start($server);
24+
try {
25+
$server = @stream_socket_server('tls://127.0.0.1:0', $errno, $errstr, $flags, $ctx);
26+
phpt_notify_server_start($server);
2627
27-
$client = @stream_socket_accept($server, 30);
28-
29-
if ($client === false) {
30-
phpt_notify(message: "SERVER_FAILED_AS_EXPECTED");
31-
} else {
32-
phpt_notify(message: "SERVER_CREATED_UNEXPECTEDLY");
33-
fclose($server);
28+
$client = @stream_socket_accept($server, 30);
29+
if ($client === false) {
30+
phpt_notify(message: "SERVER_FAILED_UNEXPECTEDLY");
31+
} else {
32+
phpt_notify(message: "SERVER_CREATED_UNEXPECTEDLY");
33+
fclose($server);
34+
}
35+
} catch (\Throwable $e) {
36+
phpt_notify(message: "SERVER_EXCEPTION: " . $e->getMessage());
3437
}
3538
CODE;
3639
$serverCode = sprintf($serverCode, $certFile);
@@ -67,5 +70,4 @@ ServerClientTestCase::getInstance()->run($clientCode, $serverCode);
6770
@unlink(__DIR__ . DIRECTORY_SEPARATOR . 'session_require_new_cb.pem.tmp');
6871
?>
6972
--EXPECT--
70-
Connection failed as expected
71-
SERVER_FAILED_AS_EXPECTED
73+
SERVER_EXCEPTION: session_new_cb is required when session_get_cb is providedConnection failed as expected

‎ext/openssl/tests/session_resumption_server_external_with_context_id.phpt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ $serverCode = <<<'CODE'
1818
$flags = STREAM_SERVER_BIND|STREAM_SERVER_LISTEN;
1919
$ctx = stream_context_create(['ssl' => [
2020
'local_cert' => '%s',
21-
'session_id_context' => 'test-server', // Proper configuration
21+
'session_id_context' => 'test-server',
2222
'session_new_cb' => function($stream, $sessionId, $sessionData) use (&$sessionStore, &$newCbCalled) {
2323
$key = bin2hex($sessionId);
2424
$sessionStore[$key] = $sessionData;

‎ext/openssl/tests/session_resumption_server_internal.phpt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ $serverCode = <<<'CODE'
1414
$flags = STREAM_SERVER_BIND|STREAM_SERVER_LISTEN;
1515
$ctx = stream_context_create(['ssl' => [
1616
'local_cert' => '%s',
17+
'session_id_context' => 'test-server',
1718
'session_cache' => true,
1819
'session_cache_size' => 1024,
1920
'session_timeout' => 300,

‎ext/openssl/xp_ssl.c‎

Lines changed: 11 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1816,15 +1816,14 @@ static zend_result php_openssl_setup_server_session(php_stream *stream,
18161816

18171817
if (!has_session_id_context &&
18181818
(SSL_CTX_get_verify_mode(sslsock->ctx) & SSL_VERIFY_PEER) != 0) {
1819-
php_error_docref(NULL, E_WARNING,
1820-
"session_new_cb is ignored as no session_id_context is set and verify_peer is enabled");
1819+
zend_value_error("session_id_context must be set if session_new_cb is set");
1820+
return FAILURE;
18211821
}
18221822
}
18231823

18241824
/* Validate: if session_get_cb is provided, session_new_cb is required */
18251825
if (has_get_cb && !has_new_cb) {
1826-
php_error_docref(NULL, E_WARNING,
1827-
"session_new_cb is required when session_get_cb is provided");
1826+
zend_value_error("session_new_cb is required when session_get_cb is provided");
18281827
return FAILURE;
18291828
}
18301829

@@ -1856,34 +1855,24 @@ static zend_result php_openssl_setup_server_session(php_stream *stream,
18561855
// Disable tickets (they won't work anyway) and warn if explicity enabled
18571856
SSL_CTX_set_options(sslsock->ctx, SSL_OP_NO_TICKET);
18581857
if (GET_VER_OPT("no_ticket") && !zend_is_true(val)) {
1859-
php_error_docref(NULL, E_WARNING,
1860-
"Session tickets cannot be enabled when session_get_cb is set");
1858+
zend_value_error("Session tickets cannot be enabled when session_get_cb is set");
18611859
}
18621860
} else if (php_openssl_is_session_cache_enabled(stream, true)) {
1861+
if (!has_session_id_context &&
1862+
(SSL_CTX_get_verify_mode(sslsock->ctx) & SSL_VERIFY_PEER) != 0) {
1863+
zend_value_error("session_id_context must be set for internal session cache");
1864+
}
1865+
18631866
/* Internal cache mode */
18641867
SSL_CTX_set_session_cache_mode(sslsock->ctx, SSL_SESS_CACHE_SERVER);
18651868

1866-
/* Set ID context */
1867-
char *session_id_context = NULL;
1868-
GET_VER_OPT_STRING("session_id_context", session_id_context);
1869-
1870-
if (session_id_context == NULL) {
1871-
/* Default context - could also use script path or similar */
1872-
static const unsigned char default_ctx[] = "PHP";
1873-
SSL_CTX_set_session_id_context(sslsock->ctx, default_ctx, sizeof(default_ctx) - 1);
1874-
} else {
1875-
SSL_CTX_set_session_id_context(sslsock->ctx,
1876-
(unsigned char *)session_id_context,
1877-
strlen(session_id_context));
1878-
}
1879-
18801869
/* Handle session_cache_size */
18811870
if (GET_VER_OPT("session_cache_size")) {
18821871
zend_long cache_size = zval_get_long(val);
18831872
if (cache_size > 0) {
18841873
SSL_CTX_sess_set_cache_size(sslsock->ctx, cache_size);
18851874
} else {
1886-
php_error_docref(NULL, E_WARNING, "session_cache_size must be positive");
1875+
zend_value_error("session_cache_size must be positive");
18871876
}
18881877
} else {
18891878
/* Default cache size from RFC */
@@ -1896,7 +1885,7 @@ static zend_result php_openssl_setup_server_session(php_stream *stream,
18961885
if (timeout > 0) {
18971886
SSL_CTX_set_timeout(sslsock->ctx, timeout);
18981887
} else {
1899-
php_error_docref(NULL, E_WARNING, "session_timeout must be positive");
1888+
zend_value_error("session_timeout must be positive");
19001889
}
19011890
} else {
19021891
/* Default timeout from RFC */
@@ -2118,8 +2107,6 @@ static zend_result php_openssl_setup_crypto(php_stream *stream,
21182107
}
21192108
}
21202109

2121-
sslsock->session_callbacks = NULL;
2122-
21232110
ERR_clear_error();
21242111

21252112
/* We need to do slightly different things based on client/server method

0 commit comments

Comments
 (0)