From f12c5c99981551b41f45b66233503da7706f8162 Mon Sep 17 00:00:00 2001 From: Raj Siva-Rajah Date: Tue, 21 Jul 2026 16:44:07 +0000 Subject: [PATCH] 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. --- php_ssh2.h | 4 ++ ssh2.c | 61 +++++++++++++++--- tests/ssh2_sensitive_auth_parameters.phpt | 76 +++++++++++++++++++++++ 3 files changed, 132 insertions(+), 9 deletions(-) create mode 100644 tests/ssh2_sensitive_auth_parameters.phpt 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 0bb7ce6..a62489c 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-- + +--INI-- +zend.exception_ignore_args=0 +--FILE-- +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