[pecl-networking-ssh2] master: fix: isolate SSH authentication state

Author: Raj Siva-Rajah (binaryfire) Committer: Casper Langemeijer (langemeijer) Date: 2026-10-14T21:50:22+02:00 Commit: https://github.com/php/pecl-networking-ssh2/commit/5358efbd641f17846b09f87fc7b5170ef6532dc1 Raw diff: https://github.com/php/pecl-networking-ssh2/commit/5358efbd641f17846b09f87fc7b5170ef6532dc1.diff fix: isolate SSH authentication state Move keyboard-interactive password state from a process-global pointer to the owning SSH session so concurrent authentication attempts cannot share credentials. Copy callback responses using the session allocator contract and clear borrowed password state immediately after authentication. Mark authentication secrets with SensitiveParameter metadata on PHP 8.2 and newer. Add PHPT coverage for sensitive and ordinary parameter metadata, trace redaction, and the distinction between in-memory private keys and key paths. Changed paths: A tests/ssh2_sensitive_auth_parameters.phpt M php_ssh2.h M ssh2.c Diff: diff --git a/php_ssh2.h b/php_ssh2.h index c27e002..7c9cbcb 100644 --- a/php_ssh2.h +++ b/php_ssh2.h @@ -66,6 +66,10 @@ typedef struct _php_ssh2_session_data { zval *disconnect_cb; php_socket_t socket; + + /* Session-local state prevents concurrent authentication attempts from sharing a password. */ + const char *kbd_password; + size_t kbd_password_len; } php_ssh2_session_data; typedef struct _php_ssh2_sftp_data { diff --git a/ssh2.c b/ssh2.c index fca4192..b685b21 100644 --- a/ssh2.c +++ b/ssh2.c @@ -21,6 +21,7 @@ #endif #include "php.h" +#include "Zend/zend_attributes.h" #include "ext/standard/info.h" #include "ext/standard/file.h" #include "php_ssh2.h" @@ -669,8 +670,6 @@ PHP_FUNCTION(ssh2_auth_none) } /* }}} */ -char *password_for_kbd_callback; - static void kbd_callback(const char *name, int name_len, const char *instruction, int instruction_len, int num_prompts, @@ -678,16 +677,31 @@ static void kbd_callback(const char *name, int name_len, LIBSSH2_USERAUTH_KBDINT_RESPONSE *responses, void **abstract) { + php_ssh2_session_data *data; + int i; + (void)name; (void)name_len; (void)instruction; (void)instruction_len; - if (num_prompts == 1) { - responses[0].text = strdup(password_for_kbd_callback); - responses[0].length = responses[0].text ? strlen(password_for_kbd_callback) : 0; - } (void)prompts; - (void)abstract; + + data = abstract ? (php_ssh2_session_data*)*abstract : NULL; + + for (i = 0; i < num_prompts; i++) { + responses[i].text = NULL; + responses[i].length = 0; + } + + if (num_prompts >= 1 && data && data->kbd_password) { + /* libssh2 releases response text through the session's system free() callback. */ + responses[0].text = malloc(data->kbd_password_len + 1); + if (responses[0].text) { + memcpy(responses[0].text, data->kbd_password, data->kbd_password_len); + responses[0].text[data->kbd_password_len] = '\0'; + responses[0].length = (unsigned int)data->kbd_password_len; + } + } } /* {{{ proto bool ssh2_auth_password(resource session, string username, string password) @@ -699,6 +713,8 @@ PHP_FUNCTION(ssh2_auth_password) zval *zsession; zend_string *username, *password; char *userauthlist; + php_ssh2_session_data *data; + int authenticated; if (zend_parse_parameters(ZEND_NUM_ARGS(), "rSS", &zsession, &username, &password) == FAILURE) { return; @@ -709,9 +725,17 @@ PHP_FUNCTION(ssh2_auth_password) userauthlist = libssh2_userauth_list(session, username->val, username->len); if (userauthlist != NULL) { - password_for_kbd_callback = password->val; if (strstr(userauthlist, "keyboard-interactive") != NULL) { - if (libssh2_userauth_keyboard_interactive(session, username->val, &kbd_callback) == 0) { + data = *(php_ssh2_session_data**)libssh2_session_abstract(session); + data->kbd_password = password->val; + data->kbd_password_len = password->len; + + authenticated = libssh2_userauth_keyboard_interactive(session, username->val, &kbd_callback) == 0; + + data->kbd_password = NULL; + data->kbd_password_len = 0; + + if (authenticated) { RETURN_TRUE; } } @@ -1413,6 +1437,25 @@ PHP_MINIT_FUNCTION(ssh2) le_ssh2_sftp = zend_register_list_destructors_ex(php_ssh2_sftp_dtor, NULL, PHP_SSH2_SFTP_RES_NAME, module_number); le_ssh2_pkey_subsys = zend_register_list_destructors_ex(php_ssh2_pkey_subsys_dtor, NULL, PHP_SSH2_PKEY_SUBSYS_RES_NAME, module_number); +#if PHP_VERSION_ID >= 80200 + /* This extension keeps hand-written arginfo, so sensitive attributes are registered here. */ + zend_add_parameter_attribute( + zend_hash_str_find_ptr(CG(function_table), "ssh2_auth_password", sizeof("ssh2_auth_password") - 1), + 2, ZSTR_KNOWN(ZEND_STR_SENSITIVEPARAMETER), 0); + zend_add_parameter_attribute( + zend_hash_str_find_ptr(CG(function_table), "ssh2_auth_pubkey_file", sizeof("ssh2_auth_pubkey_file") - 1), + 4, ZSTR_KNOWN(ZEND_STR_SENSITIVEPARAMETER), 0); + zend_add_parameter_attribute( + zend_hash_str_find_ptr(CG(function_table), "ssh2_auth_pubkey", sizeof("ssh2_auth_pubkey") - 1), + 3, ZSTR_KNOWN(ZEND_STR_SENSITIVEPARAMETER), 0); + zend_add_parameter_attribute( + zend_hash_str_find_ptr(CG(function_table), "ssh2_auth_pubkey", sizeof("ssh2_auth_pubkey") - 1), + 4, ZSTR_KNOWN(ZEND_STR_SENSITIVEPARAMETER), 0); + zend_add_parameter_attribute( + zend_hash_str_find_ptr(CG(function_table), "ssh2_auth_hostbased_file", sizeof("ssh2_auth_hostbased_file") - 1), + 5, ZSTR_KNOWN(ZEND_STR_SENSITIVEPARAMETER), 0); +#endif + REGISTER_LONG_CONSTANT("SSH2_FINGERPRINT_MD5", PHP_SSH2_FINGERPRINT_MD5, CONST_CS | CONST_PERSISTENT); REGISTER_LONG_CONSTANT("SSH2_FINGERPRINT_SHA1", PHP_SSH2_FINGERPRINT_SHA1, CONST_CS | CONST_PERSISTENT); REGISTER_LONG_CONSTANT("SSH2_FINGERPRINT_HEX", PHP_SSH2_FINGERPRINT_HEX, CONST_CS | CONST_PERSISTENT); diff --git a/tests/ssh2_sensitive_auth_parameters.phpt b/tests/ssh2_sensitive_auth_parameters.phpt new file mode 100644 index 0000000..71461f2 --- /dev/null +++ b/tests/ssh2_sensitive_auth_parameters.phpt @@ -0,0 +1,76 @@ +--TEST-- +SSH2 authentication parameters publish sensitive metadata and redact traces +--SKIPIF-- +<?php +if (PHP_VERSION_ID < 80200) { + echo 'skip SensitiveParameter requires PHP 8.2 or newer'; +} +if (!extension_loaded('ssh2')) { + echo 'skip extension not loaded'; +} +?> +--INI-- +zend.exception_ignore_args=0 +--FILE-- +<?php +$sensitiveParameters = [ + ['ssh2_auth_password', 'password'], + ['ssh2_auth_pubkey_file', 'passphrase'], + ['ssh2_auth_pubkey', 'privkey'], + ['ssh2_auth_pubkey', 'passphrase'], + ['ssh2_auth_hostbased_file', 'passphrase'], +]; + +foreach ($sensitiveParameters as [$function, $parameter]) { + $reflectionParameter = new ReflectionParameter($function, $parameter); + + printf( + "%s:%s sensitive: %s\n", + $function, + $parameter, + count($reflectionParameter->getAttributes(SensitiveParameter::class)) === 1 ? 'true' : 'false', + ); +} + +$ordinaryParameters = [ + ['ssh2_auth_password', 'username'], + ['ssh2_auth_pubkey', 'pubkey'], + ['ssh2_auth_pubkey_file', 'privkeyfile'], + ['ssh2_auth_hostbased_file', 'privkeyfile'], +]; + +foreach ($ordinaryParameters as [$function, $parameter]) { + $reflectionParameter = new ReflectionParameter($function, $parameter); + + printf( + "%s:%s sensitive: %s\n", + $function, + $parameter, + $reflectionParameter->getAttributes(SensitiveParameter::class) === [] ? 'false' : 'true', + ); +} + +try { + ssh2_auth_password(null, 'trace-user', 'trace-secret'); +} catch (Throwable $exception) { + $arguments = $exception->getTrace()[0]['args']; + $trace = print_r($exception->getTrace(), true); + + printf("trace username: %s\n", $arguments[1]); + printf("trace password wrapped: %s\n", $arguments[2] instanceof SensitiveParameterValue ? 'true' : 'false'); + printf("trace contains secret: %s\n", str_contains($trace, 'trace-secret') ? 'true' : 'false'); +} +?> +--EXPECT-- +ssh2_auth_password:password sensitive: true +ssh2_auth_pubkey_file:passphrase sensitive: true +ssh2_auth_pubkey:privkey sensitive: true +ssh2_auth_pubkey:passphrase sensitive: true +ssh2_auth_hostbased_file:passphrase sensitive: true +ssh2_auth_password:username sensitive: false +ssh2_auth_pubkey:pubkey sensitive: false +ssh2_auth_pubkey_file:privkeyfile sensitive: false +ssh2_auth_hostbased_file:privkeyfile sensitive: false +trace username: trace-user +trace password wrapped: true +trace contains secret: false