From 600020e1e4ed31e83fa6d6a949d95b7e2ef58338 Mon Sep 17 00:00:00 2001 From: Jahnvi Thakkar Date: Fri, 11 Sep 2026 16:06:56 +0530 Subject: [PATCH 1/4] Support connection keyword aliases and PDO DSN passwords --- CHANGELOG.md | 7 ++ source/pdo_sqlsrv/pdo_dbh.cpp | 76 +++++++++++- source/pdo_sqlsrv/pdo_util.cpp | 2 +- source/shared/core_conn.cpp | 22 ++-- source/sqlsrv/conn.cpp | 53 ++++++++- source/sqlsrv/util.cpp | 2 +- .../pdo_sqlsrv/pdo_azure_ad_access_token.phpt | 2 +- .../pdo_connection_alias_values.phpt | 64 ++++++++++ .../pdo_connection_brace_validation.phpt | 47 ++++++++ .../pdo_connection_keyword_aliases.phpt | 106 +++++++++++++++++ .../sqlsrv/sqlsrv_azure_ad_access_token.phpt | 2 +- .../sqlsrv_connection_alias_values.phpt | 59 +++++++++ .../sqlsrv_connection_brace_validation.phpt | 37 ++++++ .../sqlsrv_connection_keyword_aliases.phpt | 112 ++++++++++++++++++ 14 files changed, 569 insertions(+), 22 deletions(-) create mode 100644 test/functional/pdo_sqlsrv/pdo_connection_alias_values.phpt create mode 100644 test/functional/pdo_sqlsrv/pdo_connection_brace_validation.phpt create mode 100644 test/functional/pdo_sqlsrv/pdo_connection_keyword_aliases.phpt create mode 100644 test/functional/sqlsrv/sqlsrv_connection_alias_values.phpt create mode 100644 test/functional/sqlsrv/sqlsrv_connection_brace_validation.phpt create mode 100644 test/functional/sqlsrv/sqlsrv_connection_keyword_aliases.phpt diff --git a/CHANGELOG.md b/CHANGELOG.md index 46fce34cd..611ae8c1a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,13 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/) ## [Unreleased] +### Added +- Connection option aliases `ConnectTimeout` for `LoginTimeout`, `FailoverPartner` for `Failover_Partner`, and `WorkstationID` for `WSID` in both drivers. Existing ODBC keywords and attributes are retained for compatibility with older supported ODBC drivers. Names are case-insensitive, and the last occurrence of an option or its alias wins. +- The `Password` alias for `PWD` in SQLSRV connection options, and `PWD`/`Password` in PDO_SQLSRV DSNs. A non-null PDO constructor password takes precedence (including an empty string); otherwise the DSN password is used. The username still comes from the PDO constructor. Password aliases retain existing authentication restrictions and brace-escaping rules. Prefer constructor credentials when a DSN might be logged or shared. + +### Fixed +- Bounded connection-option brace validation to the supplied value length, avoiding reads past the terminator of brace-quoted values. + ## 5.13.3 - 2026-08-07 Updated PECL release packages. Here is the list of updates: diff --git a/source/pdo_sqlsrv/pdo_dbh.cpp b/source/pdo_sqlsrv/pdo_dbh.cpp index 63755aafe..5f5527b32 100644 --- a/source/pdo_sqlsrv/pdo_dbh.cpp +++ b/source/pdo_sqlsrv/pdo_dbh.cpp @@ -50,16 +50,20 @@ const char ConnectionPooling[] = "ConnectionPooling"; const char Language[] = "Language"; const char ConnectRetryCount[] = "ConnectRetryCount"; const char ConnectRetryInterval[] = "ConnectRetryInterval"; +const char ConnectTimeout[] = "ConnectTimeout"; const char Database[] = "Database"; const char Driver[] = "Driver"; const char Encrypt[] = "Encrypt"; const char Failover_Partner[] = "Failover_Partner"; +const char FailoverPartner[] = "FailoverPartner"; const char KeyStoreAuthentication[] = "KeyStoreAuthentication"; const char KeyStorePrincipalId[] = "KeyStorePrincipalId"; const char KeyStoreSecret[] = "KeyStoreSecret"; const char LoginTimeout[] = "LoginTimeout"; const char MARS_Option[] = "MultipleActiveResultSets"; const char MultiSubnetFailover[] = "MultiSubnetFailover"; +const char PWD[] = "PWD"; +const char Password[] = "Password"; const char QuotedId[] = "QuotedId"; const char TraceFile[] = "TraceFile"; const char TraceOn[] = "TraceOn"; @@ -67,6 +71,7 @@ const char TrustServerCertificate[] = "TrustServerCertificate"; const char TransactionIsolation[] = "TransactionIsolation"; const char TransparentNetworkIPResolution[] = "TransparentNetworkIPResolution"; const char WSID[] = "WSID"; +const char WorkstationID[] = "WorkstationID"; const char ComputePool[] = "ComputePool"; const char HostNameInCertificate[] = "HostNameInCertificate"; @@ -75,6 +80,7 @@ const char HostNameInCertificate[] = "HostNameInCertificate"; enum PDO_CONN_OPTIONS { PDO_CONN_OPTION_SERVER = SQLSRV_CONN_OPTION_DRIVER_SPECIFIC, + PDO_CONN_OPTION_PASSWORD, }; @@ -223,6 +229,25 @@ const connection_option PDO_CONN_OPTS[] = { CONN_ATTR_STRING, conn_str_append_func::func }, + // Like Server, credentials are extracted by the factory before core option dispatch. + { + PDOConnOptionNames::PWD, + sizeof( PDOConnOptionNames::PWD ), + PDO_CONN_OPTION_PASSWORD, + NULL, + 0, + CONN_ATTR_STRING, + conn_null_func::func + }, + { + PDOConnOptionNames::Password, + sizeof( PDOConnOptionNames::Password ), + PDO_CONN_OPTION_PASSWORD, + NULL, + 0, + CONN_ATTR_STRING, + conn_null_func::func + }, { PDOConnOptionNames::APP, sizeof( PDOConnOptionNames::APP ), @@ -349,6 +374,15 @@ const connection_option PDO_CONN_OPTS[] = { CONN_ATTR_STRING, conn_str_append_func::func }, + { + PDOConnOptionNames::FailoverPartner, + sizeof( PDOConnOptionNames::FailoverPartner ), + SQLSRV_CONN_OPTION_FAILOVER_PARTNER, + ODBCConnOptions::Failover_Partner, + sizeof( ODBCConnOptions::Failover_Partner ), + CONN_ATTR_STRING, + conn_str_append_func::func + }, { PDOConnOptionNames::KeyStoreAuthentication, sizeof( PDOConnOptionNames::KeyStoreAuthentication ), @@ -385,6 +419,15 @@ const connection_option PDO_CONN_OPTS[] = { CONN_ATTR_INT, pdo_int_conn_attr_func::func }, + { + PDOConnOptionNames::ConnectTimeout, + sizeof( PDOConnOptionNames::ConnectTimeout ), + SQLSRV_CONN_OPTION_LOGIN_TIMEOUT, + ODBCConnOptions::LoginTimeout, + sizeof( ODBCConnOptions::LoginTimeout ), + CONN_ATTR_INT, + pdo_int_conn_attr_func::func + }, { PDOConnOptionNames::MARS_Option, sizeof( PDOConnOptionNames::MARS_Option ), @@ -466,6 +509,15 @@ const connection_option PDO_CONN_OPTS[] = { CONN_ATTR_STRING, conn_str_append_func::func }, + { + PDOConnOptionNames::WorkstationID, + sizeof( PDOConnOptionNames::WorkstationID ), + SQLSRV_CONN_OPTION_WSID, + ODBCConnOptions::WSID, + sizeof( ODBCConnOptions::WSID ), + CONN_ATTR_STRING, + conn_str_append_func::func + }, { PDOConnOptionNames::ComputePool, sizeof(PDOConnOptionNames::ComputePool), @@ -634,6 +686,10 @@ int pdo_sqlsrv_db_handle_factory( _Inout_ pdo_dbh_t *dbh, _In_opt_ zval *driver_ sqlsrv_malloc_auto_ptr dsn_parser; zval server_z; ZVAL_UNDEF( &server_z ); + zval dsn_password_z; + ZVAL_UNDEF( &dsn_password_z ); + zval_auto_ptr dsn_password_guard; + dsn_password_guard = &dsn_password_z; try { @@ -678,8 +734,26 @@ int pdo_sqlsrv_db_handle_factory( _Inout_ pdo_dbh_t *dbh, _In_opt_ zval *driver_ zval_add_ref( &server_z ); zend_hash_index_del( pdo_conn_options_ht, PDO_CONN_OPTION_SERVER ); + // Preserve constructor credentials, including an explicitly empty password. + // The DSN is a fallback only when the constructor password is NULL. + const char* password = dbh->password; + zval* temp_password_z = zend_hash_index_find( pdo_conn_options_ht, PDO_CONN_OPTION_PASSWORD ); + if (temp_password_z != NULL) { + ZVAL_COPY( &dsn_password_z, temp_password_z ); + zend_hash_index_del( pdo_conn_options_ht, PDO_CONN_OPTION_PASSWORD ); + + CHECK_CUSTOM_ERROR( memchr( Z_STRVAL( dsn_password_z ), '\0', Z_STRLEN( dsn_password_z )) != NULL, + g_pdo_henv_cp, PDO_SQLSRV_ERROR_INVALID_DSN_VALUE, PDOConnOptionNames::PWD, NULL ) { + throw pdo::PDOException(); + } + + if (password == NULL) { + password = Z_STRVAL( dsn_password_z ); + } + } + sqlsrv_conn* conn = core_sqlsrv_connect( *g_pdo_henv_cp, *g_pdo_henv_ncp, core::allocate_conn, Z_STRVAL( server_z ), - dbh->username, dbh->password, pdo_conn_options_ht, pdo_sqlsrv_handle_dbh_error, + dbh->username, password, pdo_conn_options_ht, pdo_sqlsrv_handle_dbh_error, PDO_CONN_OPTS, dbh, "pdo_sqlsrv_db_handle_factory" ); // Free the string in server_z after being used diff --git a/source/pdo_sqlsrv/pdo_util.cpp b/source/pdo_sqlsrv/pdo_util.cpp index e6c71f3c8..6ff994de0 100644 --- a/source/pdo_sqlsrv/pdo_util.cpp +++ b/source/pdo_sqlsrv/pdo_util.cpp @@ -427,7 +427,7 @@ pdo_error PDO_ERRORS[] = { }, { SQLSRV_ERROR_INVALID_OPTION_WITH_ACCESS_TOKEN, - { IMSSP, (SQLCHAR*) "When using Azure AD Access Token, the connection string must not contain UID, PWD, or Authentication keywords.", -90, false} + { IMSSP, (SQLCHAR*) "When using Azure AD Access Token, the connection string must not contain UID, PWD, Password, or Authentication keywords.", -90, false} }, { SQLSRV_ERROR_EMPTY_ACCESS_TOKEN, diff --git a/source/shared/core_conn.cpp b/source/shared/core_conn.cpp index 9fe39087a..3f6fb7ed4 100644 --- a/source/shared/core_conn.cpp +++ b/source/shared/core_conn.cpp @@ -834,24 +834,20 @@ bool core_is_conn_opt_value_escaped( _Inout_ const char* value, _Inout_ size_t v return (value[0] != '}'); } - const char *pstr = value; if (value_len > 0 && value[0] == '{' && value[value_len - 1] == '}') { - pstr = ++value; + ++value; value_len -= 2; } - const char *pch = strchr(pstr, '}'); - size_t i = 0; - - while (pch != NULL && i < value_len) { - i = pch - pstr + 1; - - if (i == value_len || (i < value_len && pstr[i] != '}')) { - return false; + // Search only the value, not the closing wrapper or bytes past its terminator. + // A credential parsed from a PDO DSN can retain its enclosing braces. + for (size_t i = 0; i < value_len; ++i) { + if (value[i] == '}') { + if (i + 1 == value_len || value[i + 1] != '}') { + return false; + } + ++i; // skip the escaped brace } - - i++; // skip the brace - pch = strchr(pch + 2, '}'); // continue searching } return true; diff --git a/source/sqlsrv/conn.cpp b/source/sqlsrv/conn.cpp index 35ffd37c0..57ba17434 100644 --- a/source/sqlsrv/conn.cpp +++ b/source/sqlsrv/conn.cpp @@ -221,8 +221,7 @@ namespace SSStmtOptionNames { namespace SSConnOptionNames { -// most of these strings are the same for both the sqlsrv_connect connection option -// and the name put into the connection string. MARS is the only one that's different. +// PHP-facing aliases share an internal option key and the existing ODBC mapping. const char APP[] = "APP"; const char AccessToken[] = "AccessToken"; const char ApplicationIntent[] = "ApplicationIntent"; @@ -234,6 +233,7 @@ const char ConnectionPooling[] = "ConnectionPooling"; const char Language[] = "Language"; const char ConnectRetryCount[] = "ConnectRetryCount"; const char ConnectRetryInterval[] = "ConnectRetryInterval"; +const char ConnectTimeout[] = "ConnectTimeout"; const char Database[] = "Database"; const char DecimalPlaces[] = "DecimalPlaces"; const char FormatDecimals[] = "FormatDecimals"; @@ -242,6 +242,7 @@ const char Driver[] = "Driver"; const char BatchErrorContinue[] = "BatchErrorContinue"; const char Encrypt[] = "Encrypt"; const char Failover_Partner[] = "Failover_Partner"; +const char FailoverPartner[] = "FailoverPartner"; const char KeyStoreAuthentication[] = "KeyStoreAuthentication"; const char KeyStorePrincipalId[] = "KeyStorePrincipalId"; const char KeyStoreSecret[] = "KeyStoreSecret"; @@ -249,6 +250,7 @@ const char LoginTimeout[] = "LoginTimeout"; const char MARS_Option[] = "MultipleActiveResultSets"; const char MultiSubnetFailover[] = "MultiSubnetFailover"; const char PWD[] = "PWD"; +const char Password[] = "Password"; const char QuotedId[] = "QuotedId"; const char TraceFile[] = "TraceFile"; const char TraceOn[] = "TraceOn"; @@ -257,6 +259,7 @@ const char TransactionIsolation[] = "TransactionIsolation"; const char TransparentNetworkIPResolution[] = "TransparentNetworkIPResolution"; const char UID[] = "UID"; const char WSID[] = "WSID"; +const char WorkstationID[] = "WorkstationID"; const char ComputePool[] = "ComputePool"; const char HostNameInCertificate[] = "HostNameInCertificate"; @@ -461,6 +464,15 @@ const connection_option SS_CONN_OPTS[] = { CONN_ATTR_STRING, conn_str_append_func::func }, + { + SSConnOptionNames::FailoverPartner, + sizeof( SSConnOptionNames::FailoverPartner ), + SQLSRV_CONN_OPTION_FAILOVER_PARTNER, + ODBCConnOptions::Failover_Partner, + sizeof( ODBCConnOptions::Failover_Partner ), + CONN_ATTR_STRING, + conn_str_append_func::func + }, { SSConnOptionNames::KeyStoreAuthentication, sizeof( SSConnOptionNames::KeyStoreAuthentication ), @@ -497,6 +509,15 @@ const connection_option SS_CONN_OPTS[] = { CONN_ATTR_INT, int_conn_attr_func::func }, + { + SSConnOptionNames::ConnectTimeout, + sizeof( SSConnOptionNames::ConnectTimeout ), + SQLSRV_CONN_OPTION_LOGIN_TIMEOUT, + ODBCConnOptions::LoginTimeout, + sizeof( ODBCConnOptions::LoginTimeout ), + CONN_ATTR_INT, + int_conn_attr_func::func + }, { SSConnOptionNames::MARS_Option, sizeof( SSConnOptionNames::MARS_Option ), @@ -578,6 +599,15 @@ const connection_option SS_CONN_OPTS[] = { CONN_ATTR_STRING, conn_str_append_func::func }, + { + SSConnOptionNames::WorkstationID, + sizeof( SSConnOptionNames::WorkstationID ), + SQLSRV_CONN_OPTION_WSID, + ODBCConnOptions::WSID, + sizeof( ODBCConnOptions::WSID ), + CONN_ATTR_STRING, + conn_str_append_func::func + }, { SSConnOptionNames::DateAsString, sizeof( SSConnOptionNames::DateAsString ), @@ -1544,7 +1574,8 @@ void validate_conn_options( _Inout_ sqlsrv_context& ctx, _In_ zval* user_options int type = HASH_KEY_NON_EXISTENT; type = key ? HASH_KEY_IS_STRING : HASH_KEY_IS_LONG; - CHECK_CUSTOM_ERROR(( Z_TYPE_P( data ) == IS_NULL || Z_TYPE_P( data ) == IS_UNDEF ), ctx, SS_SQLSRV_ERROR_INVALID_OPTION, key, NULL) { + CHECK_CUSTOM_ERROR(( Z_TYPE_P( data ) == IS_NULL || Z_TYPE_P( data ) == IS_UNDEF ), ctx, SS_SQLSRV_ERROR_INVALID_OPTION, + key ? ZSTR_VAL( key ) : "", NULL) { throw ss::SSException(); } @@ -1559,7 +1590,21 @@ void validate_conn_options( _Inout_ sqlsrv_context& ctx, _In_ zval* user_options *uid = Z_STRVAL_P( data ); } - else if ( key_len == sizeof( SSConnOptionNames::PWD ) && !stricmp( ZSTR_VAL( key ), SSConnOptionNames::PWD )) { + else if (( key_len == sizeof( SSConnOptionNames::PWD ) && !stricmp( ZSTR_VAL( key ), SSConnOptionNames::PWD )) || + ( key_len == sizeof( SSConnOptionNames::Password ) && !stricmp( ZSTR_VAL( key ), SSConnOptionNames::Password ))) { + + ZVAL_DEREF( data ); + CHECK_CUSTOM_ERROR( Z_TYPE_P( data ) != IS_STRING, ctx, SQLSRV_ERROR_INVALID_OPTION_TYPE_STRING, + ZSTR_VAL( key ), NULL ) { + throw ss::SSException(); + } + + // Credentials are passed to the core as NUL-terminated strings. + // Reject embedded NULs rather than silently using a truncated password. + CHECK_CUSTOM_ERROR( memchr( Z_STRVAL_P( data ), '\0', Z_STRLEN_P( data )) != NULL, + ctx, SS_SQLSRV_ERROR_INVALID_OPTION, ZSTR_VAL( key ), NULL ) { + throw ss::SSException(); + } *pwd = Z_STRVAL_P( data ); } diff --git a/source/sqlsrv/util.cpp b/source/sqlsrv/util.cpp index 23ffe73a5..fe5373f61 100644 --- a/source/sqlsrv/util.cpp +++ b/source/sqlsrv/util.cpp @@ -415,7 +415,7 @@ ss_error SS_ERRORS[] = { }, { SQLSRV_ERROR_INVALID_OPTION_WITH_ACCESS_TOKEN, - { IMSSP, (SQLCHAR*) "When using Azure AD Access Token, the connection string must not contain UID, PWD, or Authentication keywords.", -115, false} + { IMSSP, (SQLCHAR*) "When using Azure AD Access Token, the connection string must not contain UID, PWD, Password, or Authentication keywords.", -115, false} }, { SQLSRV_ERROR_EMPTY_ACCESS_TOKEN, diff --git a/test/functional/pdo_sqlsrv/pdo_azure_ad_access_token.phpt b/test/functional/pdo_sqlsrv/pdo_azure_ad_access_token.phpt index 56f271a42..bdbc92b0e 100644 --- a/test/functional/pdo_sqlsrv/pdo_azure_ad_access_token.phpt +++ b/test/functional/pdo_sqlsrv/pdo_azure_ad_access_token.phpt @@ -38,7 +38,7 @@ function connectWithEmptyAccessToken($server) function connectWithInvalidOptions($server) { $dummyToken = 'abcde'; - $expectedError = 'When using Azure AD Access Token, the connection string must not contain UID, PWD, or Authentication keywords.'; + $expectedError = 'When using Azure AD Access Token, the connection string must not contain UID, PWD, Password, or Authentication keywords.'; $message = 'AzureAD access token test: expected to fail with '; $uid = ''; diff --git a/test/functional/pdo_sqlsrv/pdo_connection_alias_values.phpt b/test/functional/pdo_sqlsrv/pdo_connection_alias_values.phpt new file mode 100644 index 000000000..dc44234a1 --- /dev/null +++ b/test/functional/pdo_sqlsrv/pdo_connection_alias_values.phpt @@ -0,0 +1,64 @@ +--TEST-- +PDO DSN passwords and workstation aliases reach the server without replacing constructor credentials +--ENV-- +PHPT_EXEC=true +--SKIPIF-- + +--FILE-- +query('SELECT HOST_NAME()'); + $host = $stmt->fetchColumn(); + $stmt->closeCursor(); + unset($stmt, $conn); + echo $label, ': ', $host === $expectedHost ? 'OK' : 'FAIL (host)', PHP_EOL; + } catch (PDOException $e) { + // A failure must not dump the DSN or constructor arguments. + echo $label, ': FAIL (connect)', PHP_EOL; + } +} + +// Match the driver's existing credential quoting, preserving already escaped braces. +$dsnPassword = $pwd; +if (strlen($dsnPassword) < 2 || $dsnPassword[0] !== '{' || substr($dsnPassword, -1) !== '}') { + $dsnPassword = '{' . $dsnPassword . '}'; +} +foreach (array('PWD', 'Password', 'pAsSwOrD') as $key) { + verifyAliasConnection("$key=$dsnPassword;WorkstationID=alias-test;ConnectTimeout=5", null, 'alias-test', $key); +} +verifyAliasConnection('Password=bad};WorkstationID=alias-test', $pwd, 'alias-test', 'constructor wins'); +verifyAliasConnection("PWD=bad};Password=$dsnPassword;WSID=alias-test", null, 'alias-test', 'Password last'); +verifyAliasConnection("Password=bad};PWD=$dsnPassword;WSID=alias-test", null, 'alias-test', 'PWD last'); +verifyAliasConnection('WSID=old;WorkstationID=new', $pwd, 'new', 'WorkstationID last'); +verifyAliasConnection('WorkstationID=old;WSID=new', $pwd, 'new', 'WSID last'); +verifyAliasConnection('WorkstationID={alias;=}}test}', $pwd, 'alias;=}test', 'workstation delimiters'); +// These check acceptance in both orders, not elapsed-time precedence. +verifyAliasConnection('LoginTimeout=2;ConnectTimeout=5;WSID=alias-test', $pwd, 'alias-test', 'ConnectTimeout last'); +verifyAliasConnection('ConnectTimeout=2;LoginTimeout=5;WSID=alias-test', $pwd, 'alias-test', 'LoginTimeout last'); +?> +--EXPECT-- +PWD: OK +Password: OK +pAsSwOrD: OK +constructor wins: OK +Password last: OK +PWD last: OK +WorkstationID last: OK +WSID last: OK +workstation delimiters: OK +ConnectTimeout last: OK +LoginTimeout last: OK \ No newline at end of file diff --git a/test/functional/pdo_sqlsrv/pdo_connection_brace_validation.phpt b/test/functional/pdo_sqlsrv/pdo_connection_brace_validation.phpt new file mode 100644 index 000000000..75b40475f --- /dev/null +++ b/test/functional/pdo_sqlsrv/pdo_connection_brace_validation.phpt @@ -0,0 +1,47 @@ +--TEST-- +PDO bounds credential brace validation for constructor and DSN passwords +--DESCRIPTION-- +An invalid Driver stops valid values before SQLDriverConnect. Run with Valgrind +to detect reads beyond the terminator of brace-quoted values. +--SKIPIF-- + +--FILE-- + $case) { + try { + new PDO($dsn, 'alias-test', $case[0]); + echo "constructor case $index: FAIL\n"; + } catch (PDOException $e) { + if (($e->errorInfo[1] ?? null) !== ($case[1] ? -79 : -21)) { + echo "constructor case $index: FAIL\n"; + } + } +} +foreach (array('PWD', 'Password') as $key) { + foreach (array('{}', '{t}', '{}}}', '{test}}}', '{te}}st}', '{test;=value}') as $index => $value) { + try { + new PDO($dsn . "$key=$value", 'alias-test', null); + echo "$key case $index: FAIL\n"; + } catch (PDOException $e) { + if (($e->errorInfo[1] ?? null) !== -79) { + echo "$key case $index: FAIL\n"; + } + } + } +} +echo "Done\n"; +?> +--EXPECT-- +Done \ No newline at end of file diff --git a/test/functional/pdo_sqlsrv/pdo_connection_keyword_aliases.phpt b/test/functional/pdo_sqlsrv/pdo_connection_keyword_aliases.phpt new file mode 100644 index 000000000..5dc12bba8 --- /dev/null +++ b/test/functional/pdo_sqlsrv/pdo_connection_keyword_aliases.phpt @@ -0,0 +1,106 @@ +--TEST-- +PDO connection aliases and DSN passwords preserve constructor precedence +--DESCRIPTION-- +An invalid option, invalid Driver, or credential validation stops each case +before SQLDriverConnect. Constructor passwords override DSN passwords whenever +non-null, including an empty string. No SQL Server is required. +--SKIPIF-- + +--FILE-- +errorInfo[1]) && $e->errorInfo[0] === 'IMSSP' + && $e->errorInfo[1] === $code && strpos($e->getMessage(), $fragment) !== false; + // Never print the DSN, password, or exception trace on failure. + echo $label, ': ', $passed ? 'OK' : 'FAIL', PHP_EOL; + } +} + +$sentinel = '__AliasTestMustNotConnect'; +$values = array( + 'LoginTimeout' => '3', + 'ConnectTimeout' => '3', + 'Failover_Partner' => 'unused', + 'FailoverPartner' => 'unused', + 'WSID' => 'alias-test', + 'WorkstationID' => 'alias-test', + 'PWD' => 'dummy', + 'Password' => 'dummy' +); +foreach ($values as $key => $value) { + foreach (array($key, strtolower($key), strtoupper($key)) as $spelling) { + expectAliasError(" $spelling = $value;$sentinel=1", null, null, -42, $sentinel, $spelling); + } +} + +$driver = 'Driver=AliasTestInvalidDriver;'; +expectAliasError($driver . 'Password=bad}', 'alias-test', 'dummy', -79, 'Driver option', 'constructor wins'); +expectAliasError($driver . 'PWD=bad}', 'alias-test', '', -79, 'Driver option', 'empty constructor wins'); +expectAliasError($driver . 'Password=dummy', 'alias-test', 'bad}', -21, 'right brace', 'constructor validated'); +expectAliasError($driver . 'PWD=bad}', 'alias-test', null, -21, 'right brace', 'null constructor uses DSN'); +expectAliasError($driver . 'PWD=bad};Password=dummy', 'alias-test', null, -79, 'Driver option', 'Password last'); +expectAliasError($driver . 'Password=bad};PWD=dummy', 'alias-test', null, -79, 'Driver option', 'PWD last'); +expectAliasError($driver . 'PWD=dummy;Password=bad}', 'alias-test', null, -21, 'right brace', 'last password validated'); +expectAliasError($driver . 'Password={dummy;=}}value}', 'alias-test', null, -79, 'Driver option', 'password delimiters'); +expectAliasError($driver . 'Password=', 'alias-test', null, -79, 'Driver option', 'empty DSN password'); + +foreach (array('PWD', 'Password') as $key) { + foreach (array('', 'dummy') as $value) { + expectAliasError("$key=$value;AccessToken=dummy", null, null, -90, 'Access Token', "$key token conflict"); + } + expectAliasError("$key={dummy", null, null, -67, 'right brace', "$key missing brace"); + expectAliasError("$key={dummy}junk", null, null, -63, 'invalid value', "$key trailing junk"); +} +expectAliasError('AccessToken=', null, null, -91, 'Access Token is empty', 'absent password'); +expectAliasError('Passwrod=dummy', null, null, -42, 'Passwrod', 'unknown spelling'); +?> +--EXPECT-- +LoginTimeout: OK +logintimeout: OK +LOGINTIMEOUT: OK +ConnectTimeout: OK +connecttimeout: OK +CONNECTTIMEOUT: OK +Failover_Partner: OK +failover_partner: OK +FAILOVER_PARTNER: OK +FailoverPartner: OK +failoverpartner: OK +FAILOVERPARTNER: OK +WSID: OK +wsid: OK +WSID: OK +WorkstationID: OK +workstationid: OK +WORKSTATIONID: OK +PWD: OK +pwd: OK +PWD: OK +Password: OK +password: OK +PASSWORD: OK +constructor wins: OK +empty constructor wins: OK +constructor validated: OK +null constructor uses DSN: OK +Password last: OK +PWD last: OK +last password validated: OK +password delimiters: OK +empty DSN password: OK +PWD token conflict: OK +PWD token conflict: OK +PWD missing brace: OK +PWD trailing junk: OK +Password token conflict: OK +Password token conflict: OK +Password missing brace: OK +Password trailing junk: OK +absent password: OK +unknown spelling: OK \ No newline at end of file diff --git a/test/functional/sqlsrv/sqlsrv_azure_ad_access_token.phpt b/test/functional/sqlsrv/sqlsrv_azure_ad_access_token.phpt index 62c1e3d15..54ae6c7d5 100644 --- a/test/functional/sqlsrv/sqlsrv_azure_ad_access_token.phpt +++ b/test/functional/sqlsrv/sqlsrv_azure_ad_access_token.phpt @@ -34,7 +34,7 @@ function connectWithEmptyAccessToken($server) function connectWithInvalidOptions($server) { $dummyToken = 'abcde'; - $expectedError = 'When using Azure AD Access Token, the connection string must not contain UID, PWD, or Authentication keywords.'; + $expectedError = 'When using Azure AD Access Token, the connection string must not contain UID, PWD, Password, or Authentication keywords.'; $connectionInfo = array("UID"=>"", "AccessToken" => "$dummyToken"); $conn = sqlsrv_connect($server, $connectionInfo); diff --git a/test/functional/sqlsrv/sqlsrv_connection_alias_values.phpt b/test/functional/sqlsrv/sqlsrv_connection_alias_values.phpt new file mode 100644 index 000000000..cd57b3916 --- /dev/null +++ b/test/functional/sqlsrv/sqlsrv_connection_alias_values.phpt @@ -0,0 +1,59 @@ +--TEST-- +SQLSRV password and workstation aliases reach the server with last-value precedence +--ENV-- +PHPT_EXEC=true +--SKIPIF-- + +--FILE-- + $uid, 'Database' => $databaseName, 'Driver' => $driver, 'Encrypt' => $encrypt); + $conn = sqlsrv_connect($server, $base + $options); + if ($conn === false) { + echo $label, ': FAIL (connect)', PHP_EOL; + return; + } + $stmt = sqlsrv_query($conn, 'SELECT HOST_NAME()'); + $host = null; + if ($stmt !== false) { + $row = sqlsrv_fetch_array($stmt, SQLSRV_FETCH_NUMERIC); + $host = $row[0] ?? null; + sqlsrv_free_stmt($stmt); + } + echo $label, ': ', $host === $expectedHost ? 'OK' : 'FAIL (host)', PHP_EOL; + sqlsrv_close($conn); +} + +foreach (array('PWD', 'Password', 'pAsSwOrD') as $key) { + verifyAliasConnection(array($key => $pwd, 'WorkstationID' => 'alias-test', 'ConnectTimeout' => 5), 'alias-test', $key); +} +verifyAliasConnection(array('PWD' => $pwd, 'WSID' => 'old', 'WorkstationID' => 'new'), 'new', 'WorkstationID last'); +verifyAliasConnection(array('PWD' => $pwd, 'WorkstationID' => 'old', 'WSID' => 'new'), 'new', 'WSID last'); +verifyAliasConnection(array('PWD' => $pwd, 'WorkstationID' => '{alias;=}}test}'), 'alias;=}test', 'workstation delimiters'); +verifyAliasConnection(array('PWD' => 'bad}', 'Password' => $pwd, 'WSID' => 'alias-test'), 'alias-test', 'Password last'); +verifyAliasConnection(array('Password' => 'bad}', 'PWD' => $pwd, 'WSID' => 'alias-test'), 'alias-test', 'PWD last'); +// These check acceptance in both orders, not elapsed-time precedence. +verifyAliasConnection(array('PWD' => $pwd, 'LoginTimeout' => 2, 'ConnectTimeout' => 5, 'WSID' => 'alias-test'), 'alias-test', 'ConnectTimeout last'); +verifyAliasConnection(array('PWD' => $pwd, 'ConnectTimeout' => 2, 'LoginTimeout' => 5, 'WSID' => 'alias-test'), 'alias-test', 'LoginTimeout last'); +?> +--EXPECT-- +PWD: OK +Password: OK +pAsSwOrD: OK +WorkstationID last: OK +WSID last: OK +workstation delimiters: OK +Password last: OK +PWD last: OK +ConnectTimeout last: OK +LoginTimeout last: OK \ No newline at end of file diff --git a/test/functional/sqlsrv/sqlsrv_connection_brace_validation.phpt b/test/functional/sqlsrv/sqlsrv_connection_brace_validation.phpt new file mode 100644 index 000000000..986628e02 --- /dev/null +++ b/test/functional/sqlsrv/sqlsrv_connection_brace_validation.phpt @@ -0,0 +1,37 @@ +--TEST-- +SQLSRV bounds credential brace validation for empty, quoted and escaped values +--DESCRIPTION-- +An invalid Driver stops valid values before SQLDriverConnect. Run with Valgrind +to detect reads beyond the terminator of brace-quoted values. +--SKIPIF-- + +--FILE-- + $case) { + $conn = sqlsrv_connect('127.0.0.1', array('UID' => 'alias-test', $key => $case[0], 'Driver' => 'AliasTestInvalidDriver')); + $errors = sqlsrv_errors(SQLSRV_ERR_ERRORS); + $expected = $case[1] ? -106 : -4; + if ($conn !== false || !isset($errors[0]) || $errors[0]['code'] !== $expected) { + echo "$key case $index: FAIL\n"; + } + if ($conn !== false) { + sqlsrv_close($conn); + } + } +} +echo "Done\n"; +?> +--EXPECT-- +Done \ No newline at end of file diff --git a/test/functional/sqlsrv/sqlsrv_connection_keyword_aliases.phpt b/test/functional/sqlsrv/sqlsrv_connection_keyword_aliases.phpt new file mode 100644 index 000000000..b6d9447e9 --- /dev/null +++ b/test/functional/sqlsrv/sqlsrv_connection_keyword_aliases.phpt @@ -0,0 +1,112 @@ +--TEST-- +SQLSRV connection keyword aliases preserve validation and credential handling +--DESCRIPTION-- +All cases fail before SQLDriverConnect: an unknown option, invalid Driver, or +credential validation stops the connection. No SQL Server is required. +--SKIPIF-- + +--FILE-- + 3, + 'ConnectTimeout' => 3, + 'Failover_Partner' => 'unused', + 'FailoverPartner' => 'unused', + 'WSID' => 'alias-test', + 'WorkstationID' => 'alias-test', + 'PWD' => 'dummy', + 'Password' => 'dummy' +); +foreach ($values as $key => $value) { + foreach (array($key, strtolower($key), strtoupper($key)) as $spelling) { + expectAliasError(array($spelling => $value, $sentinel => true), -1, $sentinel, $spelling); + } +} + +foreach (array('LoginTimeout', 'ConnectTimeout') as $key) { + expectAliasError(array($key => '3', $sentinel => true), -33, 'Integer type was expected', "$key type"); +} +foreach (array('Failover_Partner', 'FailoverPartner', 'WSID', 'WorkstationID', 'PWD', 'Password') as $key) { + expectAliasError(array($key => 3, $sentinel => true), -33, 'String type was expected', "$key type"); +} +foreach (array('PWD', 'Password') as $key) { + expectAliasError(array($key => null, $sentinel => true), -1, $key, "$key null"); + $password = 'dummy'; + $referenced = array($key => &$password, $sentinel => true); + expectAliasError($referenced, -1, $sentinel, "$key reference"); + expectAliasError(array($key => "dummy\0suffix", $sentinel => true), -1, $key, "$key NUL"); + foreach (array('', 'dummy') as $value) { + expectAliasError(array($key => $value, 'AccessToken' => 'dummy'), -115, 'Access Token', "$key token conflict"); + } +} + +$base = array('UID' => 'alias-test', 'Driver' => 'AliasTestInvalidDriver'); +expectAliasError($base + array('PWD' => 'bad}', 'Password' => 'dummy'), -106, 'Driver option', 'Password last'); +expectAliasError($base + array('Password' => 'bad}', 'PWD' => 'dummy'), -106, 'Driver option', 'PWD last'); +expectAliasError($base + array('PWD' => 'dummy', 'Password' => 'bad}'), -4, 'right brace', 'last password validated'); +expectAliasError($base + array('Password' => '{dummy;=}}value}'), -106, 'Driver option', 'password delimiters'); +expectAliasError(array('Passwrod' => 'dummy'), -1, 'Passwrod', 'unknown spelling'); +?> +--EXPECT-- +LoginTimeout: OK +logintimeout: OK +LOGINTIMEOUT: OK +ConnectTimeout: OK +connecttimeout: OK +CONNECTTIMEOUT: OK +Failover_Partner: OK +failover_partner: OK +FAILOVER_PARTNER: OK +FailoverPartner: OK +failoverpartner: OK +FAILOVERPARTNER: OK +WSID: OK +wsid: OK +WSID: OK +WorkstationID: OK +workstationid: OK +WORKSTATIONID: OK +PWD: OK +pwd: OK +PWD: OK +Password: OK +password: OK +PASSWORD: OK +LoginTimeout type: OK +ConnectTimeout type: OK +Failover_Partner type: OK +FailoverPartner type: OK +WSID type: OK +WorkstationID type: OK +PWD type: OK +Password type: OK +PWD null: OK +PWD reference: OK +PWD NUL: OK +PWD token conflict: OK +PWD token conflict: OK +Password null: OK +Password reference: OK +Password NUL: OK +Password token conflict: OK +Password token conflict: OK +Password last: OK +PWD last: OK +last password validated: OK +password delimiters: OK +unknown spelling: OK \ No newline at end of file From e3b2908672bed5f353740a2049298c0bc878b2ef Mon Sep 17 00:00:00 2001 From: Jahnvi Thakkar Date: Sat, 12 Sep 2026 16:06:14 +0530 Subject: [PATCH 2/4] Securely erase parsed PDO passwords and fix macOS CI setup --- CHANGELOG.md | 1 + azure-pipelines.yml | 27 +- source/pdo_sqlsrv/pdo_dbh.cpp | 31 +-- source/pdo_sqlsrv/pdo_parser.cpp | 26 +- source/pdo_sqlsrv/php_pdo_sqlsrv_int.h | 55 +++- source/shared/core_conn.cpp | 16 +- source/shared/core_sqlsrv.h | 3 + source/shared/core_util.cpp | 15 ++ test/native/pdo_password_cleanup.php | 97 +++++++ test/native/pdo_password_cleanup_observer.cpp | 111 ++++++++ test/native/test_pdo_password_cleanup.sh | 64 +++++ test/tools/requirements.txt | 2 + test/tools/test_macos_odbc_install.py | 240 ++++++++++++++++++ 13 files changed, 637 insertions(+), 51 deletions(-) create mode 100644 test/native/pdo_password_cleanup.php create mode 100644 test/native/pdo_password_cleanup_observer.cpp create mode 100644 test/native/test_pdo_password_cleanup.sh create mode 100644 test/tools/requirements.txt create mode 100644 test/tools/test_macos_odbc_install.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 611ae8c1a..6c6d404db 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/) ### Fixed - Bounded connection-option brace validation to the supplied value length, avoiding reads past the terminator of brace-quoted values. +- Securely erase driver-owned PDO DSN password copies before releasing or replacing them, including parser errors and connection failures. Original PDO/caller-owned strings and exception trace behavior are unchanged. ## 5.13.3 - 2026-08-07 Updated PECL release packages. Here is the list of updates: diff --git a/azure-pipelines.yml b/azure-pipelines.yml index 0fa5db982..39df57697 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -104,23 +104,24 @@ jobs: set -e echo "Installing Microsoft ODBC Driver 18 for SQL Server..." - brew tap microsoft/mssql-release https://github.com/Microsoft/homebrew-mssql-release - # Homebrew now requires explicit trust for third-party taps/formulae in CI. + # Tapping evaluates formulae, so trust this specific tap before adding it. if brew help trust >/dev/null 2>&1; then - brew trust microsoft/mssql-release || \ - brew trust --formula microsoft/mssql-release/msodbcsql18 + brew trust --tap microsoft/mssql-release fi - HOMEBREW_ACCEPT_EULA=Y brew install msodbcsql18 mssql-tools18 + brew tap microsoft/mssql-release https://github.com/Microsoft/homebrew-mssql-release + HOMEBREW_ACCEPT_EULA=Y brew install \ + microsoft/mssql-release/msodbcsql18 \ + microsoft/mssql-release/mssql-tools18 # Verify installation - if [[ $(brew list --verbose msodbcsql18) ]]; then + if driver_files=$(brew list --verbose msodbcsql18) && [[ -n "$driver_files" ]]; then echo "ODBC driver is installed successfully" else echo "ODBC driver did not install properly" exit 1 fi - if [[ $(brew list --verbose mssql-tools18) ]]; then + if tool_files=$(brew list --verbose mssql-tools18) && [[ -n "$tool_files" ]]; then echo "TOOLS installed successfully" else echo "TOOLS did not install properly" @@ -662,6 +663,18 @@ jobs: php --ri pdo_sqlsrv displayName: 'Build and install drivers' + - script: | + set -e + bash "$(Build.SourcesDirectory)/test/native/test_pdo_password_cleanup.sh" + displayName: 'Verify native PDO password erasure' + env: + MSSQL_SERVER: $(server) + MSSQL_USER: $(uid) + MSSQL_PASSWORD: $(pwd) + MSSQL_DRIVER_NAME: 'ODBC Driver 18 for SQL Server' + LANG: 'en_US.UTF-8' + LC_ALL: 'en_US.UTF-8' + - script: | echo "Updating MsSetup.inc files..." cd $(Build.SourcesDirectory)/test/functional/sqlsrv diff --git a/source/pdo_sqlsrv/pdo_dbh.cpp b/source/pdo_sqlsrv/pdo_dbh.cpp index 5f5527b32..5d9670d6f 100644 --- a/source/pdo_sqlsrv/pdo_dbh.cpp +++ b/source/pdo_sqlsrv/pdo_dbh.cpp @@ -77,13 +77,6 @@ const char HostNameInCertificate[] = "HostNameInCertificate"; } -enum PDO_CONN_OPTIONS { - - PDO_CONN_OPTION_SERVER = SQLSRV_CONN_OPTION_DRIVER_SPECIFIC, - PDO_CONN_OPTION_PASSWORD, - -}; - enum PDO_STMT_OPTIONS { PDO_STMT_OPTION_ENCODING = SQLSRV_STMT_OPTION_DRIVER_SPECIFIC, @@ -229,7 +222,7 @@ const connection_option PDO_CONN_OPTS[] = { CONN_ATTR_STRING, conn_str_append_func::func }, - // Like Server, credentials are extracted by the factory before core option dispatch. + // Passwords use the parser's separate secure buffer, not core option dispatch. { PDOConnOptionNames::PWD, sizeof( PDOConnOptionNames::PWD ), @@ -683,13 +676,9 @@ int pdo_sqlsrv_db_handle_factory( _Inout_ pdo_dbh_t *dbh, _In_opt_ zval *driver_ dbh->methods = &pdo_sqlsrv_dbh_methods; dbh->driver_data = NULL; zval* temp_server_z = NULL; - sqlsrv_malloc_auto_ptr dsn_parser; zval server_z; ZVAL_UNDEF( &server_z ); - zval dsn_password_z; - ZVAL_UNDEF( &dsn_password_z ); - zval_auto_ptr dsn_password_guard; - dsn_password_guard = &dsn_password_z; + pdo_secure_password dsn_password; try { @@ -716,9 +705,9 @@ int pdo_sqlsrv_db_handle_factory( _Inout_ pdo_dbh_t *dbh, _In_opt_ zval *driver_ ZVAL_PTR_DTOR, 0 /*persistent*/ ); // Either of g_pdo_henv_cp or g_pdo_henv_ncp can be used to propogate the error. - dsn_parser = new ( sqlsrv_malloc( sizeof( conn_string_parser ))) conn_string_parser( *g_pdo_henv_cp, dbh->data_source, - static_cast( dbh->data_source_len ), pdo_conn_options_ht ); - dsn_parser->parse_conn_string(); + conn_string_parser dsn_parser( *g_pdo_henv_cp, dbh->data_source, + static_cast( dbh->data_source_len ), pdo_conn_options_ht, dsn_password ); + dsn_parser.parse_conn_string(); // Extract the server name temp_server_z = zend_hash_index_find( pdo_conn_options_ht, PDO_CONN_OPTION_SERVER ); @@ -737,18 +726,14 @@ int pdo_sqlsrv_db_handle_factory( _Inout_ pdo_dbh_t *dbh, _In_opt_ zval *driver_ // Preserve constructor credentials, including an explicitly empty password. // The DSN is a fallback only when the constructor password is NULL. const char* password = dbh->password; - zval* temp_password_z = zend_hash_index_find( pdo_conn_options_ht, PDO_CONN_OPTION_PASSWORD ); - if (temp_password_z != NULL) { - ZVAL_COPY( &dsn_password_z, temp_password_z ); - zend_hash_index_del( pdo_conn_options_ht, PDO_CONN_OPTION_PASSWORD ); - - CHECK_CUSTOM_ERROR( memchr( Z_STRVAL( dsn_password_z ), '\0', Z_STRLEN( dsn_password_z )) != NULL, + if (dsn_password.get() != NULL) { + CHECK_CUSTOM_ERROR( memchr( dsn_password.get(), '\0', dsn_password.size() ) != NULL, g_pdo_henv_cp, PDO_SQLSRV_ERROR_INVALID_DSN_VALUE, PDOConnOptionNames::PWD, NULL ) { throw pdo::PDOException(); } if (password == NULL) { - password = Z_STRVAL( dsn_password_z ); + password = dsn_password.get(); } } diff --git a/source/pdo_sqlsrv/pdo_parser.cpp b/source/pdo_sqlsrv/pdo_parser.cpp index 720c20777..282a9b1d9 100644 --- a/source/pdo_sqlsrv/pdo_parser.cpp +++ b/source/pdo_sqlsrv/pdo_parser.cpp @@ -26,7 +26,9 @@ extern "C" { #include "php_pdo_sqlsrv_int.h" // Constructor -conn_string_parser:: conn_string_parser( _In_ sqlsrv_context& ctx, _In_ const char* dsn, _In_ int len, _In_ HashTable* conn_options_ht ) +conn_string_parser:: conn_string_parser( _In_ sqlsrv_context& ctx, _In_ const char* dsn, _In_ int len, + _In_ HashTable* conn_options_ht, _Inout_ pdo_secure_password& dsn_password ) : + password(dsn_password) { this->orig_str = dsn; this->len = len; @@ -139,6 +141,18 @@ void string_parser::add_key_value_pair( _In_reads_(val_len) const char* value, _ core::sqlsrv_zend_hash_index_update( *ctx, this->element_ht, this->current_key, &value_z ); } +// Intercept password values before the ordinary helper makes a Zend string copy. +// The factory's secure owner also covers parser failures and overwritten aliases. +void conn_string_parser::add_conn_option( _In_reads_(val_len) const char* value, _In_ int val_len ) +{ + if (this->current_key == PDO_CONN_OPTION_PASSWORD) { + this->password.assign(value, static_cast(val_len)); + } + else { + add_key_value_pair(value, val_len); + } +} + // Add a key-value pair to the hashtable with int value void sql_string_parser::add_key_int_value_pair( _In_ unsigned int value ) { zval value_z; @@ -236,7 +250,7 @@ void conn_string_parser:: parse_conn_string( void ) // if EOS encountered after 0 or more spaces OR semi-colon encountered. if( !discard_white_spaces() || this->orig_str[pos] == ';' ) { - add_key_value_pair( NULL, 0 ); + add_conn_option( NULL, 0 ); if( this->is_eos() ) { @@ -298,7 +312,7 @@ void conn_string_parser:: parse_conn_string( void ) state = NextKeyValuePair; } - add_key_value_pair( &( this->orig_str[start_pos] ), this->pos - start_pos ); + add_conn_option( &( this->orig_str[start_pos] ), this->pos - start_pos ); SQLSRV_ASSERT((( state == NextKeyValuePair ) || ( this->is_eos() )), "conn_string_parser::parse_conn_string: Invalid state encountered " ); @@ -313,7 +327,7 @@ void conn_string_parser:: parse_conn_string( void ) if( !next() ) { // EOS - add_key_value_pair( &( this->orig_str[start_pos] ), this->pos - start_pos ); + add_conn_option( &( this->orig_str[start_pos] ), this->pos - start_pos ); break; } @@ -340,7 +354,7 @@ void conn_string_parser:: parse_conn_string( void ) if( ! this->discard_white_spaces() ) { //EOS - add_key_value_pair( &( this->orig_str[start_pos] ), end_pos - start_pos ); + add_conn_option( &( this->orig_str[start_pos] ), end_pos - start_pos ); break; } } @@ -348,7 +362,7 @@ void conn_string_parser:: parse_conn_string( void ) // if semi-colon than go to next key-value pair if ( this->orig_str[pos] == ';' ) { - add_key_value_pair( &( this->orig_str[start_pos] ), end_pos - start_pos ); + add_conn_option( &( this->orig_str[start_pos] ), end_pos - start_pos ); state = NextKeyValuePair; break; } diff --git a/source/pdo_sqlsrv/php_pdo_sqlsrv_int.h b/source/pdo_sqlsrv/php_pdo_sqlsrv_int.h index d98456f0b..ecfdbc61b 100644 --- a/source/pdo_sqlsrv/php_pdo_sqlsrv_int.h +++ b/source/pdo_sqlsrv/php_pdo_sqlsrv_int.h @@ -131,6 +131,56 @@ class string_parser // PDO DSN Parser //********************************************************************************************************************************* +enum PDO_CONN_OPTIONS { + PDO_CONN_OPTION_SERVER = SQLSRV_CONN_OPTION_DRIVER_SPECIFIC, + PDO_CONN_OPTION_PASSWORD, +}; + +// The factory owns this buffer throughout parsing and connection establishment. +// Never retain password copies in Zend strings (including interned empty strings) +// or the ordinary options hash, whose destructor does not erase secret bytes. +class pdo_secure_password +{ + private: + char* value; + size_t length; + + public: + pdo_secure_password() : value(NULL), length(0) {} + pdo_secure_password(const pdo_secure_password&) = delete; + pdo_secure_password& operator=(const pdo_secure_password&) = delete; + + ~pdo_secure_password() + { + reset(); + } + + void reset() + { + if (value != NULL) { + core_sqlsrv_secure_zero(value, length + 1); + sqlsrv_free(value); + value = NULL; + length = 0; + } + } + + // Input is a borrowed slice of the original DSN, never this buffer. + void assign(_In_reads_bytes_(len) const char* input, _In_ size_t len) + { + reset(); // wipe superseded aliases before allocating their replacement + value = static_cast(sqlsrv_malloc(len, sizeof(char), 1)); + length = len; + if (len != 0) { + memcpy(value, input, len); + } + value[len] = '\0'; + } + + const char* get() const { return value; } + size_t size() const { return length; } +}; + // Parser class used to parse DSN connection string. class conn_string_parser : private string_parser { @@ -147,11 +197,14 @@ class conn_string_parser : private string_parser private: const char* current_key_name; + pdo_secure_password& password; + void add_conn_option( _In_reads_(val_len) const char* value, _In_ int val_len ); int discard_trailing_white_spaces( _In_reads_(buf_len) const char* str, _Inout_ int buf_len ); void validate_key( _In_reads_(key_len) const char *key, _Inout_ int key_len); public: - conn_string_parser( _In_ sqlsrv_context& ctx, _In_ const char* dsn, _In_ int len, _In_ HashTable* conn_options_ht ); + conn_string_parser( _In_ sqlsrv_context& ctx, _In_ const char* dsn, _In_ int len, + _In_ HashTable* conn_options_ht, _Inout_ pdo_secure_password& dsn_password ); void parse_conn_string( void ); }; diff --git a/source/shared/core_conn.cpp b/source/shared/core_conn.cpp index 3f6fb7ed4..263eb5e03 100644 --- a/source/shared/core_conn.cpp +++ b/source/shared/core_conn.cpp @@ -120,23 +120,11 @@ static std::mutex s_token_cache_mutex; // tokens alive for at least this long to account for in-flight recoveries. static const time_t TOKEN_CACHE_TTL_FLOOR = 120; -// Securely zero memory before freeing to scrub token secrets. -// Plain memset can be optimized away by the compiler when the buffer -// is not read afterward; these platform calls are guaranteed to persist. -static void secure_zero(_Out_writes_bytes_(len) void* ptr, size_t len) -{ -#ifdef _WIN32 - SecureZeroMemory(ptr, len); -#else - explicit_bzero(ptr, len); -#endif -} - static void token_cache_free_entry(TokenCacheEntry* e) { - secure_zero(e->token->data, e->token->dataSize); + core_sqlsrv_secure_zero(e->token->data, e->token->dataSize); free(e->token); - secure_zero(e->raw_content, e->raw_len); + core_sqlsrv_secure_zero(e->raw_content, e->raw_len); free(e->raw_content); free(e); } diff --git a/source/shared/core_sqlsrv.h b/source/shared/core_sqlsrv.h index 320bf2e1c..8f712f2e9 100644 --- a/source/shared/core_sqlsrv.h +++ b/source/shared/core_sqlsrv.h @@ -437,6 +437,9 @@ inline void sqlsrv_free( _Inout_ void* ptr ) #endif +// Wipe owned secret storage with a platform primitive that cannot be optimized away. +void core_sqlsrv_secure_zero( _Out_writes_bytes_(len) void* ptr, _In_ size_t len ); + // trait class that allows us to assign const types to an auto_ptr template struct remove_const { diff --git a/source/shared/core_util.cpp b/source/shared/core_util.cpp index 1ac592e04..b9279df06 100644 --- a/source/shared/core_util.cpp +++ b/source/shared/core_util.cpp @@ -21,6 +21,21 @@ #include "core_sqlsrv.h" +#ifdef _WIN32 +#include +#else +#include +#endif + +void core_sqlsrv_secure_zero( _Out_writes_bytes_(len) void* ptr, _In_ size_t len ) +{ +#ifdef _WIN32 + SecureZeroMemory(ptr, len); +#else + explicit_bzero(ptr, len); +#endif +} + namespace { severity_callback g_driver_severity; diff --git a/test/native/pdo_password_cleanup.php b/test/native/pdo_password_cleanup.php new file mode 100644 index 000000000..6d3cbeca2 --- /dev/null +++ b/test/native/pdo_password_cleanup.php @@ -0,0 +1,97 @@ +cleanup_probe_reset(); + foreach ($values as $value) { + $probe->cleanup_probe_expect($value, strlen($value)); + } + $code = null; + try { + $conn = new PDO($dsn, $username, $password); + $result = $conn->query('SELECT 1'); + if ((int) $result->fetchColumn() !== 1) { + throw new RuntimeException('Unexpected connection result'); + } + unset($result, $conn); + } catch (PDOException $e) { + $code = $e->errorInfo[1] ?? $e->getCode(); + } + $unchanged = hash('sha256', $dsn) === $originalDsnHash + && ($password === null ? null : hash('sha256', $password)) === $originalPasswordHash; + $released = $probe->cleanup_probe_released(); + $passed = $code === $expectedCode && $unchanged + && $probe->cleanup_probe_failures() === 0 && $released === count($values); + // Never print a DSN, credentials, or exception arguments on failure. + if (!$passed) { + echo "$label: FAIL (observed $released of ", count($values), " secure releases)\n"; + return false; + } + echo "$label: PASS\n"; + return true; +} + +$first = 'cleanup-first-value-17'; +$second = 'cleanup-second-value-18'; +$prefix = 'sqlsrv:Server=127.0.0.1;Driver=CleanupInvalidDriver;'; +$cases = array( + array('factory failure', $prefix . "PWD=$first", 'cleanup-user', null, array($first), -79), + array('parser error', $prefix . "Password=$first;UnknownKeyword=1", null, null, array($first), -42), + array('missing server', "sqlsrv:Password=$first", null, null, array($first), -64), + array('replacement', $prefix . "PWD=$first;Password=$second", 'cleanup-user', null, array($first, $second), -79), + array('reverse replacement', $prefix . "Password=$first;pWd=$second", 'cleanup-user', null, array($first, $second), -79), + array('replacement then error', $prefix . "PWD=$first;Password=$second;UnknownKeyword=1", null, null, array($first, $second), -42), + array('unused DSN password', $prefix . "Password=$first", 'cleanup-user', 'constructor-only', array($first), -79), + array('empty constructor wins', $prefix . "PWD=$first", 'cleanup-user', '', array($first), -79), + array('empty password', $prefix . 'Password=', 'cleanup-user', null, array(''), -79), + array('one-byte password', $prefix . 'Password=x', 'cleanup-user', null, array('x'), -79), + array('empty replacement', $prefix . "PWD=$first;Password=", 'cleanup-user', null, array($first, ''), -79), + array('replace empty password', $prefix . "PWD=;Password=$second", 'cleanup-user', null, array('', $second), -79), + array('identical replacements', $prefix . "PWD=$first;Password=$first", 'cleanup-user', null, array($first, $first), -79), + array('absent password', $prefix, 'cleanup-user', null, array(), -79), + array('incomplete first password', $prefix . 'Password={' . $first, null, null, array(), -67), + array('incomplete replacement', $prefix . "PWD=$first;Password={" . $second, null, null, array($first), -67), + array('credential validation error', $prefix . "Password=$first}", 'cleanup-user', null, array($first . '}'), -21), + array('access token conflict', $prefix . "Password=$first;AccessToken=dummy", null, null, array($first), -90), + array('quoted delimiters', $prefix . 'Password={cleanup;=}}value}', 'cleanup-user', null, array('{cleanup;=}}value}'), -79) +); +$passed = true; +foreach ($cases as $case) { + $passed = checkCleanup(...$case) && $passed; +} + +if (getenv('MSPHPSQL_CLEANUP_NO_SERVER') === '1') { + echo "Live factory-success tests not requested (no-server mode)\n"; +} else { + $server = getenv('MSSQL_SERVER'); + $username = getenv('MSSQL_USER'); + $password = getenv('MSSQL_PASSWORD'); + $driver = getenv('MSSQL_DRIVER_NAME'); + if (!$server || !$username || $password === false || !$driver) { + throw new RuntimeException('Set MSSQL_SERVER, MSSQL_USER, MSSQL_PASSWORD, and MSSQL_DRIVER_NAME for live cleanup tests'); + } + $dsn = 'sqlsrv:Server={' . str_replace('}', '}}', $server) . '};Driver={' . $driver . '};Encrypt=no;'; + $quoted = '{' . str_replace('}', '}}', $password) . '}'; + $passed = checkCleanup('factory success', $dsn . "Password=$quoted", $username, null, array($quoted), null) && $passed; + $passed = checkCleanup('success with constructor override', $dsn . "PWD=$first", $username, $password, array($first), null) && $passed; + $passed = checkCleanup('ODBC authentication failure', $dsn . "Password=$first", 'cleanup-nonexistent-user', null, array($first), 18456) && $passed; +} +$probe->cleanup_probe_reset(); +exit($passed ? 0 : 1); \ No newline at end of file diff --git a/test/native/pdo_password_cleanup_observer.cpp b/test/native/pdo_password_cleanup_observer.cpp new file mode 100644 index 000000000..c99026e9a --- /dev/null +++ b/test/native/pdo_password_cleanup_observer.cpp @@ -0,0 +1,111 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. +// Linked only into the disposable Linux test module. Never ship this observer. +// GNU ld --wrap intercepts erasure and Zend release in the real driver objects. +#include +#include + +namespace { +const size_t MAX_EXPECTED = 16; +const size_t MAX_VALUE = 1024; +struct expected_release { + unsigned char value[MAX_VALUE]; + size_t length; + void* pointer; + bool wiped; + bool released; +}; +expected_release expected[MAX_EXPECTED]; +size_t expected_count = 0; +unsigned int failures = 0; + +void observe_wipe(void* pointer, size_t length) +{ + for (size_t i = 0; i < expected_count; ++i) { + expected_release& entry = expected[i]; + if (!entry.wiped && length == entry.length && + std::memcmp(pointer, entry.value, length) == 0) { + entry.pointer = pointer; + entry.wiped = true; + return; + } + } +} + +void observe_release(void* pointer) +{ + for (size_t i = 0; i < expected_count; ++i) { + expected_release& entry = expected[i]; + if (entry.wiped && !entry.released && entry.pointer == pointer) { + // This inspection is BEFORE the allocator is called. No freed memory + // is read, and neither expected values nor memory contents are logged. + const unsigned char* bytes = static_cast(pointer); + for (size_t j = 0; j < entry.length; ++j) { + if (bytes[j] != 0) { + ++failures; + break; + } + } + entry.released = true; + return; + } + } +} +} + +extern "C" { +void __real_explicit_bzero(void*, size_t); +void __real___explicit_bzero_chk(void*, size_t, size_t); +void __real__efree(void*); + +void __wrap_explicit_bzero(void* pointer, size_t length) +{ + observe_wipe(pointer, length); + __real_explicit_bzero(pointer, length); +} + +void __wrap___explicit_bzero_chk(void* pointer, size_t length, size_t object_size) +{ + observe_wipe(pointer, length); + __real___explicit_bzero_chk(pointer, length, object_size); +} + +void __wrap__efree(void* pointer) +{ + observe_release(pointer); + __real__efree(pointer); +} + +__attribute__((visibility("default"))) void cleanup_probe_reset() +{ + std::memset(expected, 0, sizeof(expected)); + expected_count = 0; + failures = 0; +} + +__attribute__((visibility("default"))) void cleanup_probe_expect(const char* value, size_t length) +{ + if (expected_count == MAX_EXPECTED || length >= MAX_VALUE) { + ++failures; + return; + } + expected_release& entry = expected[expected_count++]; + std::memcpy(entry.value, value, length); + entry.value[length] = '\0'; + entry.length = length + 1; // include the owned buffer's NUL terminator +} + +__attribute__((visibility("default"))) unsigned int cleanup_probe_failures() +{ + return failures; +} + +__attribute__((visibility("default"))) size_t cleanup_probe_released() +{ + size_t released = 0; + for (size_t i = 0; i < expected_count; ++i) { + released += expected[i].released ? 1 : 0; + } + return released; +} +} \ No newline at end of file diff --git a/test/native/test_pdo_password_cleanup.sh b/test/native/test_pdo_password_cleanup.sh new file mode 100644 index 000000000..12dc7288c --- /dev/null +++ b/test/native/test_pdo_password_cleanup.sh @@ -0,0 +1,64 @@ +#!/usr/bin/env bash +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. +# Linux native regression. Requires matching php/phpize/php-config, PHP FFI, +# a C++ compiler, make and unixODBC development headers. No production test API. +# By default run live success/failure cases using MSSQL_SERVER, MSSQL_USER, +# MSSQL_PASSWORD and MSSQL_DRIVER_NAME; pass --no-server for parser-only cases. +set -euo pipefail + +if [[ "$(uname -s)" != Linux ]]; then + echo 'This GNU linker instrumentation test requires Linux.' >&2 + exit 1 +fi +if [[ $# -gt 1 || ( $# -eq 1 && "$1" != --no-server ) ]]; then + echo 'Usage: test_pdo_password_cleanup.sh [--no-server]' >&2 + exit 1 +fi +if [[ ${1:-} == --no-server ]]; then + export MSPHPSQL_CLEANUP_NO_SERVER=1 +else + export MSPHPSQL_CLEANUP_NO_SERVER=0 + : "${MSSQL_SERVER:?Set MSSQL_SERVER for live cleanup tests}" + : "${MSSQL_USER:?Set MSSQL_USER for live cleanup tests}" + : "${MSSQL_PASSWORD?Set MSSQL_PASSWORD for live cleanup tests}" + : "${MSSQL_DRIVER_NAME:?Set MSSQL_DRIVER_NAME for live cleanup tests}" +fi + +if [[ "$(php -n -r 'echo PHP_VERSION_ID;')" != "$(php-config --vernum)" ]]; then + echo 'php and php-config must refer to the same PHP version.' >&2 + exit 1 +fi +# The observer uses the release-build _efree ABI; debug builds add arguments. +if [[ "$(php -n -r 'echo (int) PHP_DEBUG;')" != 0 ]]; then + echo 'This observer requires a non-debug PHP build.' >&2 + exit 1 +fi +php -n -d extension=ffi -r 'exit(extension_loaded("FFI") ? 0 : 1);' + +root=$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")/../.." && pwd) +work=$(mktemp -d /tmp/msphpsql-cleanup.XXXXXX) +trap 'rm -rf -- "$work"' EXIT +mkdir -p "$work/pdo_sqlsrv/shared" +cp "$root"/source/pdo_sqlsrv/*.{cpp,h,m4} "$work/pdo_sqlsrv/" +cp "$root"/source/shared/*.{cpp,h,hpp} "$work/pdo_sqlsrv/shared/" +cp "$root/test/native/pdo_password_cleanup_observer.cpp" "$work/observer.cpp" + +# This observer wraps only calls originating in the disposable test module. +# The production secure erase is still called; zeroed bytes are then inspected +# in __wrap__efree immediately BEFORE Zend actually releases the allocation. +c++ -std=c++11 -O2 -fPIC -Wall -Wextra -Werror -c "$work/observer.cpp" -o "$work/observer.o" +cd "$work/pdo_sqlsrv" +if ! { phpize > "$work/build.log" 2>&1 && ./configure --with-pdo_sqlsrv >> "$work/build.log" 2>&1; }; then + tail -n 60 "$work/build.log" >&2 + exit 1 +fi +if ! make -j2 LDFLAGS="$work/observer.o -Wl,--wrap=explicit_bzero,--wrap=__explicit_bzero_chk,--wrap=_efree" >> "$work/build.log" 2>&1; then + tail -n 60 "$work/build.log" >&2 + exit 1 +fi +export MSPHPSQL_CLEANUP_MODULE="$work/pdo_sqlsrv/modules/pdo_sqlsrv.so" +php -n -d extension=pdo -d extension=ffi -d ffi.enable=1 \ + -d zend.exception_ignore_args=1 \ + -d "extension=$MSPHPSQL_CLEANUP_MODULE" \ + "$root/test/native/pdo_password_cleanup.php" \ No newline at end of file diff --git a/test/tools/requirements.txt b/test/tools/requirements.txt new file mode 100644 index 000000000..08248ef14 --- /dev/null +++ b/test/tools/requirements.txt @@ -0,0 +1,2 @@ +pytest>=7.0 +PyYAML>=6.0 diff --git a/test/tools/test_macos_odbc_install.py b/test/tools/test_macos_odbc_install.py new file mode 100644 index 000000000..4a439136c --- /dev/null +++ b/test/tools/test_macos_odbc_install.py @@ -0,0 +1,240 @@ +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. +"""Exercise the real macOS ODBC pipeline script without installing packages. + +Requires Bash (set TEST_BASH to Git Bash on Windows) and requirements.txt. +The brew function below models tap-time trust enforcement, not Homebrew's +package installation. Actual macOS installation remains a CI check. +""" + +import os +from pathlib import Path +import shutil +import subprocess + +import pytest +import yaml + +ROOT = Path(__file__).resolve().parents[2] +TAP = "microsoft/mssql-release" +INSTALL = f"install {TAP}/msodbcsql18 {TAP}/mssql-tools18" + +BREW_STUB = r""" +trusted=${TEST_BREW_PRETRUSTED:-0} +tapped=0 +installed=0 +brew() { + printf 'BREW:%s\n' "$*" >&2 + case "$1" in + help) + [[ "$*" == 'help trust' ]] || return 90 + [[ "$TEST_BREW_SUPPORTS_TRUST" == 1 ]] + ;; + trust) + [[ "$*" == 'trust --tap microsoft/mssql-release' ]] || return 91 + [[ "$TEST_BREW_FAILURE" != trust ]] || return 21 + trusted=1 + ;; + tap) + [[ "$2" == microsoft/mssql-release ]] || return 92 + remote=https://github.com/Microsoft/homebrew-mssql-release + [[ "$#" == 3 && "$3" == "$remote" ]] || return 92 + [[ "$TEST_BREW_FAILURE" != tap ]] || return 22 + if [[ "$TEST_BREW_REQUIRES_TRUST" == 1 && "$trusted" != 1 ]]; then + echo 'Refusing to load formula from untrusted tap' >&2 + return 23 + fi + tapped=1 + ;; + install) + [[ "$tapped" == 1 && "$HOMEBREW_ACCEPT_EULA" == Y ]] || return 93 + [[ "$#" == 3 ]] || return 94 + [[ "$2" == microsoft/mssql-release/msodbcsql18 ]] || return 94 + [[ "$3" == microsoft/mssql-release/mssql-tools18 ]] || return 94 + [[ "$TEST_BREW_FAILURE" != install ]] || return 24 + installed=1 + ;; + list) + [[ "$2" == --verbose && "$installed" == 1 ]] || return 95 + [[ "$TEST_BREW_FAILURE" != "list_$3" ]] || return 25 + if [[ "$TEST_BREW_FAILURE" == "list_empty_$3" ]]; then + return 0 + fi + printf '/test-cellar/%s/18.7.1.1\n' "$3" + [[ "$TEST_BREW_FAILURE" != "list_partial_$3" ]] || return 26 + ;; + reinstall) + [[ "$*" == 'reinstall openssl@1.1' ]] || return 96 + # The existing optional OpenSSL workaround is allowed to fail. + return 1 + ;; + *) return 97 ;; + esac +} +""" + + +@pytest.fixture(scope="module", name="install_script") +def fixture_install_script() -> str: + pipeline = yaml.safe_load( + (ROOT / "azure-pipelines.yml").read_text(encoding="utf-8") + ) + jobs = [job for job in pipeline["jobs"] if job.get("job") == "macOS"] + assert len(jobs) == 1 + steps = [ + step + for step in jobs[0]["steps"] + if step.get("displayName") == "Install ODBC Driver 18 and Tools" + ] + assert len(steps) == 1 + script = steps[0]["script"] + assert isinstance(script, str) + return script + + +@pytest.fixture(scope="module", name="bash") +def fixture_bash() -> str: + executable = os.environ.get("TEST_BASH") or shutil.which("bash") + assert executable, "Bash is required; set TEST_BASH to its executable" + return executable + + +def run_install( + script: str, + bash: str, + directory: Path, + *, + supports_trust: bool = True, + requires_trust: bool = True, + pretrusted: bool = False, + failure: str = "", +) -> subprocess.CompletedProcess[str]: + # Isolate inherited shell startup hooks and Homebrew settings. Never run + # real brew: the stub handles every invocation from the extracted step. + excluded = {"BASH_ENV", "ENV", "SHELLOPTS", "BASHOPTS"} + env = { + key: value + for key, value in os.environ.items() + if key not in excluded + and not key.startswith(("HOMEBREW_", "TEST_BREW_", "BASH_FUNC_")) + } + env.update( + TEST_BREW_SUPPORTS_TRUST=str(int(supports_trust)), + TEST_BREW_REQUIRES_TRUST=str(int(requires_trust)), + TEST_BREW_PRETRUSTED=str(int(pretrusted)), + TEST_BREW_FAILURE=failure, + ) + return subprocess.run( + [bash, "--noprofile", "--norc", "-s"], + input=BREW_STUB + "\n" + script, + text=True, + encoding="utf-8", + capture_output=True, + cwd=directory, + env=env, + timeout=15, + check=False, + ) + + +def calls(result: subprocess.CompletedProcess[str]) -> list[str]: + return [ + line.removeprefix("BREW:") + for line in result.stderr.splitlines() + if line.startswith("BREW:") + ] + + +@pytest.mark.parametrize("pretrusted", [False, True]) +def test_trust_precedes_tap( + install_script: str, bash: str, tmp_path: Path, pretrusted: bool +) -> None: + result = run_install(install_script, bash, tmp_path, pretrusted=pretrusted) + assert result.returncode == 0, result.stderr + commands = calls(result) + trust = f"trust --tap {TAP}" + tap = f"tap {TAP} https://github.com/Microsoft/homebrew-mssql-release" + assert commands.index(trust) < commands.index(tap) + assert commands.index(tap) < commands.index(INSTALL) + assert "list --verbose msodbcsql18" in commands + assert "list --verbose mssql-tools18" in commands + + +def test_older_homebrew_without_trust_command( + install_script: str, bash: str, tmp_path: Path +) -> None: + result = run_install( + install_script, + bash, + tmp_path, + supports_trust=False, + requires_trust=False, + ) + assert result.returncode == 0, result.stderr + assert INSTALL in calls(result) + assert not any(call.startswith("trust ") for call in calls(result)) + + +@pytest.mark.parametrize( + ("failure", "blocked_command"), + [ + ("trust", "tap "), + ("tap", "install "), + ("install", "list "), + ("list_msodbcsql18", "reinstall "), + ("list_mssql-tools18", "reinstall "), + ("list_empty_msodbcsql18", "reinstall "), + ("list_empty_mssql-tools18", "reinstall "), + ("list_partial_msodbcsql18", "reinstall "), + ("list_partial_mssql-tools18", "reinstall "), + ], +) +def test_setup_fails_closed( + install_script: str, + bash: str, + tmp_path: Path, + failure: str, + blocked_command: str, +) -> None: + result = run_install(install_script, bash, tmp_path, failure=failure) + assert result.returncode != 0 + assert not any( + command.startswith(blocked_command) for command in calls(result) + ) + assert "##vso[task.prependpath]" not in result.stdout + + +def test_missing_trust_capability_cannot_bypass_enforcement( + install_script: str, bash: str, tmp_path: Path +) -> None: + result = run_install(install_script, bash, tmp_path, supports_trust=False) + assert result.returncode != 0 + assert "untrusted tap" in result.stderr + assert INSTALL not in calls(result) + + +def test_no_global_trust_bypass(install_script: str) -> None: + assert "HOMEBREW_NO_REQUIRE_TAP_TRUST" not in install_script + assert "brew trust --formula" not in install_script + assert "trust --tap microsoft/mssql-release" in install_script + + +def test_path_with_spaces( + install_script: str, bash: str, tmp_path: Path +) -> None: + directory = tmp_path / "pipeline checkout with spaces" + directory.mkdir() + result = run_install(install_script, bash, directory) + assert result.returncode == 0, result.stderr + + +def test_bash_syntax(install_script: str, bash: str) -> None: + result = subprocess.run( + [bash, "--noprofile", "--norc", "-n"], + input=install_script, + text=True, + capture_output=True, + timeout=15, + check=False, + ) + assert result.returncode == 0, result.stderr From 60f2945613db68e63b1443aa9825dfa7b89193ad Mon Sep 17 00:00:00 2001 From: Jahnvi Thakkar Date: Mon, 14 Sep 2026 12:13:29 +0530 Subject: [PATCH 3/4] Fix Windows alias test startup timeout and trim validation tooling --- azure-pipelines.yml | 2 +- .../pdo_connection_alias_values.phpt | 10 +- .../sqlsrv_connection_alias_values.phpt | 12 +- .../pdo_password_cleanup.php | 6 +- .../pdo_password_cleanup_observer.cpp | 5 +- test/tools/requirements.txt | 2 - test/tools/test_macos_odbc_install.py | 240 ------------------ .../test_pdo_password_cleanup.sh | 4 +- 8 files changed, 26 insertions(+), 255 deletions(-) rename test/{native => tools}/pdo_password_cleanup.php (94%) rename test/{native => tools}/pdo_password_cleanup_observer.cpp (95%) delete mode 100644 test/tools/requirements.txt delete mode 100644 test/tools/test_macos_odbc_install.py rename test/{native => tools}/test_pdo_password_cleanup.sh (95%) diff --git a/azure-pipelines.yml b/azure-pipelines.yml index 39df57697..57d7e3246 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -665,7 +665,7 @@ jobs: - script: | set -e - bash "$(Build.SourcesDirectory)/test/native/test_pdo_password_cleanup.sh" + bash "$(Build.SourcesDirectory)/test/tools/test_pdo_password_cleanup.sh" displayName: 'Verify native PDO password erasure' env: MSSQL_SERVER: $(server) diff --git a/test/functional/pdo_sqlsrv/pdo_connection_alias_values.phpt b/test/functional/pdo_sqlsrv/pdo_connection_alias_values.phpt index dc44234a1..ab2b03445 100644 --- a/test/functional/pdo_sqlsrv/pdo_connection_alias_values.phpt +++ b/test/functional/pdo_sqlsrv/pdo_connection_alias_values.phpt @@ -29,6 +29,9 @@ function verifyAliasConnection($keywords, $password, $expectedHost, $label) } catch (PDOException $e) { // A failure must not dump the DSN or constructor arguments. echo $label, ': FAIL (connect)', PHP_EOL; + if (isset($e->errorInfo[0], $e->errorInfo[1])) { + echo 'SQLSTATE: ', $e->errorInfo[0], ', code: ', $e->errorInfo[1], PHP_EOL; + } } } @@ -37,8 +40,9 @@ $dsnPassword = $pwd; if (strlen($dsnPassword) < 2 || $dsnPassword[0] !== '{' || substr($dsnPassword, -1) !== '}') { $dsnPassword = '{' . $dsnPassword . '}'; } +// Allow LocalDB cold starts under coverage; these are not timing assertions. foreach (array('PWD', 'Password', 'pAsSwOrD') as $key) { - verifyAliasConnection("$key=$dsnPassword;WorkstationID=alias-test;ConnectTimeout=5", null, 'alias-test', $key); + verifyAliasConnection("$key=$dsnPassword;WorkstationID=alias-test;ConnectTimeout=30", null, 'alias-test', $key); } verifyAliasConnection('Password=bad};WorkstationID=alias-test', $pwd, 'alias-test', 'constructor wins'); verifyAliasConnection("PWD=bad};Password=$dsnPassword;WSID=alias-test", null, 'alias-test', 'Password last'); @@ -47,8 +51,8 @@ verifyAliasConnection('WSID=old;WorkstationID=new', $pwd, 'new', 'WorkstationID verifyAliasConnection('WorkstationID=old;WSID=new', $pwd, 'new', 'WSID last'); verifyAliasConnection('WorkstationID={alias;=}}test}', $pwd, 'alias;=}test', 'workstation delimiters'); // These check acceptance in both orders, not elapsed-time precedence. -verifyAliasConnection('LoginTimeout=2;ConnectTimeout=5;WSID=alias-test', $pwd, 'alias-test', 'ConnectTimeout last'); -verifyAliasConnection('ConnectTimeout=2;LoginTimeout=5;WSID=alias-test', $pwd, 'alias-test', 'LoginTimeout last'); +verifyAliasConnection('LoginTimeout=20;ConnectTimeout=30;WSID=alias-test', $pwd, 'alias-test', 'ConnectTimeout last'); +verifyAliasConnection('ConnectTimeout=20;LoginTimeout=30;WSID=alias-test', $pwd, 'alias-test', 'LoginTimeout last'); ?> --EXPECT-- PWD: OK diff --git a/test/functional/sqlsrv/sqlsrv_connection_alias_values.phpt b/test/functional/sqlsrv/sqlsrv_connection_alias_values.phpt index cd57b3916..5212f8410 100644 --- a/test/functional/sqlsrv/sqlsrv_connection_alias_values.phpt +++ b/test/functional/sqlsrv/sqlsrv_connection_alias_values.phpt @@ -21,6 +21,10 @@ function verifyAliasConnection($options, $expectedHost, $label) $conn = sqlsrv_connect($server, $base + $options); if ($conn === false) { echo $label, ': FAIL (connect)', PHP_EOL; + // Report only diagnostic codes, never connection strings or credentials. + foreach (sqlsrv_errors(SQLSRV_ERR_ERRORS) ?? array() as $error) { + echo 'SQLSTATE: ', $error['SQLSTATE'], ', code: ', $error['code'], PHP_EOL; + } return; } $stmt = sqlsrv_query($conn, 'SELECT HOST_NAME()'); @@ -34,8 +38,10 @@ function verifyAliasConnection($options, $expectedHost, $label) sqlsrv_close($conn); } +// Test alias behavior, not connection speed. LocalDB cold starts under coverage +// can exceed five seconds before the first connection is established. foreach (array('PWD', 'Password', 'pAsSwOrD') as $key) { - verifyAliasConnection(array($key => $pwd, 'WorkstationID' => 'alias-test', 'ConnectTimeout' => 5), 'alias-test', $key); + verifyAliasConnection(array($key => $pwd, 'WorkstationID' => 'alias-test', 'ConnectTimeout' => 30), 'alias-test', $key); } verifyAliasConnection(array('PWD' => $pwd, 'WSID' => 'old', 'WorkstationID' => 'new'), 'new', 'WorkstationID last'); verifyAliasConnection(array('PWD' => $pwd, 'WorkstationID' => 'old', 'WSID' => 'new'), 'new', 'WSID last'); @@ -43,8 +49,8 @@ verifyAliasConnection(array('PWD' => $pwd, 'WorkstationID' => '{alias;=}}test}') verifyAliasConnection(array('PWD' => 'bad}', 'Password' => $pwd, 'WSID' => 'alias-test'), 'alias-test', 'Password last'); verifyAliasConnection(array('Password' => 'bad}', 'PWD' => $pwd, 'WSID' => 'alias-test'), 'alias-test', 'PWD last'); // These check acceptance in both orders, not elapsed-time precedence. -verifyAliasConnection(array('PWD' => $pwd, 'LoginTimeout' => 2, 'ConnectTimeout' => 5, 'WSID' => 'alias-test'), 'alias-test', 'ConnectTimeout last'); -verifyAliasConnection(array('PWD' => $pwd, 'ConnectTimeout' => 2, 'LoginTimeout' => 5, 'WSID' => 'alias-test'), 'alias-test', 'LoginTimeout last'); +verifyAliasConnection(array('PWD' => $pwd, 'LoginTimeout' => 20, 'ConnectTimeout' => 30, 'WSID' => 'alias-test'), 'alias-test', 'ConnectTimeout last'); +verifyAliasConnection(array('PWD' => $pwd, 'ConnectTimeout' => 20, 'LoginTimeout' => 30, 'WSID' => 'alias-test'), 'alias-test', 'LoginTimeout last'); ?> --EXPECT-- PWD: OK diff --git a/test/native/pdo_password_cleanup.php b/test/tools/pdo_password_cleanup.php similarity index 94% rename from test/native/pdo_password_cleanup.php rename to test/tools/pdo_password_cleanup.php index 6d3cbeca2..e9211c6a1 100644 --- a/test/native/pdo_password_cleanup.php +++ b/test/tools/pdo_password_cleanup.php @@ -50,7 +50,8 @@ function checkCleanup($label, $dsn, $username, $password, $values, $expectedCode $first = 'cleanup-first-value-17'; $second = 'cleanup-second-value-18'; -$prefix = 'sqlsrv:Server=127.0.0.1;Driver=CleanupInvalidDriver;'; +// Invalid Driver rejects these cases before any network connection is attempted. +$prefix = 'sqlsrv:Server=unused.invalid;Driver=CleanupInvalidDriver;'; $cases = array( array('factory failure', $prefix . "PWD=$first", 'cleanup-user', null, array($first), -79), array('parser error', $prefix . "Password=$first;UnknownKeyword=1", null, null, array($first), -42), @@ -90,7 +91,8 @@ function checkCleanup($label, $dsn, $username, $password, $values, $expectedCode $dsn = 'sqlsrv:Server={' . str_replace('}', '}}', $server) . '};Driver={' . $driver . '};Encrypt=no;'; $quoted = '{' . str_replace('}', '}}', $password) . '}'; $passed = checkCleanup('factory success', $dsn . "Password=$quoted", $username, null, array($quoted), null) && $passed; - $passed = checkCleanup('success with constructor override', $dsn . "PWD=$first", $username, $password, array($first), null) && $passed; + // Constructor credentials use the driver's existing brace-escaping rules too. + $passed = checkCleanup('success with constructor override', $dsn . "PWD=$first", $username, $quoted, array($first), null) && $passed; $passed = checkCleanup('ODBC authentication failure', $dsn . "Password=$first", 'cleanup-nonexistent-user', null, array($first), 18456) && $passed; } $probe->cleanup_probe_reset(); diff --git a/test/native/pdo_password_cleanup_observer.cpp b/test/tools/pdo_password_cleanup_observer.cpp similarity index 95% rename from test/native/pdo_password_cleanup_observer.cpp rename to test/tools/pdo_password_cleanup_observer.cpp index c99026e9a..919d1b8ac 100644 --- a/test/native/pdo_password_cleanup_observer.cpp +++ b/test/tools/pdo_password_cleanup_observer.cpp @@ -2,6 +2,7 @@ // Licensed under the MIT License. // Linked only into the disposable Linux test module. Never ship this observer. // GNU ld --wrap intercepts erasure and Zend release in the real driver objects. +#include #include #include @@ -85,12 +86,12 @@ __attribute__((visibility("default"))) void cleanup_probe_reset() __attribute__((visibility("default"))) void cleanup_probe_expect(const char* value, size_t length) { - if (expected_count == MAX_EXPECTED || length >= MAX_VALUE) { + if (value == NULL || expected_count == MAX_EXPECTED || length >= MAX_VALUE) { ++failures; return; } expected_release& entry = expected[expected_count++]; - std::memcpy(entry.value, value, length); + std::copy_n(value, length, entry.value); entry.value[length] = '\0'; entry.length = length + 1; // include the owned buffer's NUL terminator } diff --git a/test/tools/requirements.txt b/test/tools/requirements.txt deleted file mode 100644 index 08248ef14..000000000 --- a/test/tools/requirements.txt +++ /dev/null @@ -1,2 +0,0 @@ -pytest>=7.0 -PyYAML>=6.0 diff --git a/test/tools/test_macos_odbc_install.py b/test/tools/test_macos_odbc_install.py deleted file mode 100644 index 4a439136c..000000000 --- a/test/tools/test_macos_odbc_install.py +++ /dev/null @@ -1,240 +0,0 @@ -# Copyright (c) Microsoft Corporation. All rights reserved. -# Licensed under the MIT License. -"""Exercise the real macOS ODBC pipeline script without installing packages. - -Requires Bash (set TEST_BASH to Git Bash on Windows) and requirements.txt. -The brew function below models tap-time trust enforcement, not Homebrew's -package installation. Actual macOS installation remains a CI check. -""" - -import os -from pathlib import Path -import shutil -import subprocess - -import pytest -import yaml - -ROOT = Path(__file__).resolve().parents[2] -TAP = "microsoft/mssql-release" -INSTALL = f"install {TAP}/msodbcsql18 {TAP}/mssql-tools18" - -BREW_STUB = r""" -trusted=${TEST_BREW_PRETRUSTED:-0} -tapped=0 -installed=0 -brew() { - printf 'BREW:%s\n' "$*" >&2 - case "$1" in - help) - [[ "$*" == 'help trust' ]] || return 90 - [[ "$TEST_BREW_SUPPORTS_TRUST" == 1 ]] - ;; - trust) - [[ "$*" == 'trust --tap microsoft/mssql-release' ]] || return 91 - [[ "$TEST_BREW_FAILURE" != trust ]] || return 21 - trusted=1 - ;; - tap) - [[ "$2" == microsoft/mssql-release ]] || return 92 - remote=https://github.com/Microsoft/homebrew-mssql-release - [[ "$#" == 3 && "$3" == "$remote" ]] || return 92 - [[ "$TEST_BREW_FAILURE" != tap ]] || return 22 - if [[ "$TEST_BREW_REQUIRES_TRUST" == 1 && "$trusted" != 1 ]]; then - echo 'Refusing to load formula from untrusted tap' >&2 - return 23 - fi - tapped=1 - ;; - install) - [[ "$tapped" == 1 && "$HOMEBREW_ACCEPT_EULA" == Y ]] || return 93 - [[ "$#" == 3 ]] || return 94 - [[ "$2" == microsoft/mssql-release/msodbcsql18 ]] || return 94 - [[ "$3" == microsoft/mssql-release/mssql-tools18 ]] || return 94 - [[ "$TEST_BREW_FAILURE" != install ]] || return 24 - installed=1 - ;; - list) - [[ "$2" == --verbose && "$installed" == 1 ]] || return 95 - [[ "$TEST_BREW_FAILURE" != "list_$3" ]] || return 25 - if [[ "$TEST_BREW_FAILURE" == "list_empty_$3" ]]; then - return 0 - fi - printf '/test-cellar/%s/18.7.1.1\n' "$3" - [[ "$TEST_BREW_FAILURE" != "list_partial_$3" ]] || return 26 - ;; - reinstall) - [[ "$*" == 'reinstall openssl@1.1' ]] || return 96 - # The existing optional OpenSSL workaround is allowed to fail. - return 1 - ;; - *) return 97 ;; - esac -} -""" - - -@pytest.fixture(scope="module", name="install_script") -def fixture_install_script() -> str: - pipeline = yaml.safe_load( - (ROOT / "azure-pipelines.yml").read_text(encoding="utf-8") - ) - jobs = [job for job in pipeline["jobs"] if job.get("job") == "macOS"] - assert len(jobs) == 1 - steps = [ - step - for step in jobs[0]["steps"] - if step.get("displayName") == "Install ODBC Driver 18 and Tools" - ] - assert len(steps) == 1 - script = steps[0]["script"] - assert isinstance(script, str) - return script - - -@pytest.fixture(scope="module", name="bash") -def fixture_bash() -> str: - executable = os.environ.get("TEST_BASH") or shutil.which("bash") - assert executable, "Bash is required; set TEST_BASH to its executable" - return executable - - -def run_install( - script: str, - bash: str, - directory: Path, - *, - supports_trust: bool = True, - requires_trust: bool = True, - pretrusted: bool = False, - failure: str = "", -) -> subprocess.CompletedProcess[str]: - # Isolate inherited shell startup hooks and Homebrew settings. Never run - # real brew: the stub handles every invocation from the extracted step. - excluded = {"BASH_ENV", "ENV", "SHELLOPTS", "BASHOPTS"} - env = { - key: value - for key, value in os.environ.items() - if key not in excluded - and not key.startswith(("HOMEBREW_", "TEST_BREW_", "BASH_FUNC_")) - } - env.update( - TEST_BREW_SUPPORTS_TRUST=str(int(supports_trust)), - TEST_BREW_REQUIRES_TRUST=str(int(requires_trust)), - TEST_BREW_PRETRUSTED=str(int(pretrusted)), - TEST_BREW_FAILURE=failure, - ) - return subprocess.run( - [bash, "--noprofile", "--norc", "-s"], - input=BREW_STUB + "\n" + script, - text=True, - encoding="utf-8", - capture_output=True, - cwd=directory, - env=env, - timeout=15, - check=False, - ) - - -def calls(result: subprocess.CompletedProcess[str]) -> list[str]: - return [ - line.removeprefix("BREW:") - for line in result.stderr.splitlines() - if line.startswith("BREW:") - ] - - -@pytest.mark.parametrize("pretrusted", [False, True]) -def test_trust_precedes_tap( - install_script: str, bash: str, tmp_path: Path, pretrusted: bool -) -> None: - result = run_install(install_script, bash, tmp_path, pretrusted=pretrusted) - assert result.returncode == 0, result.stderr - commands = calls(result) - trust = f"trust --tap {TAP}" - tap = f"tap {TAP} https://github.com/Microsoft/homebrew-mssql-release" - assert commands.index(trust) < commands.index(tap) - assert commands.index(tap) < commands.index(INSTALL) - assert "list --verbose msodbcsql18" in commands - assert "list --verbose mssql-tools18" in commands - - -def test_older_homebrew_without_trust_command( - install_script: str, bash: str, tmp_path: Path -) -> None: - result = run_install( - install_script, - bash, - tmp_path, - supports_trust=False, - requires_trust=False, - ) - assert result.returncode == 0, result.stderr - assert INSTALL in calls(result) - assert not any(call.startswith("trust ") for call in calls(result)) - - -@pytest.mark.parametrize( - ("failure", "blocked_command"), - [ - ("trust", "tap "), - ("tap", "install "), - ("install", "list "), - ("list_msodbcsql18", "reinstall "), - ("list_mssql-tools18", "reinstall "), - ("list_empty_msodbcsql18", "reinstall "), - ("list_empty_mssql-tools18", "reinstall "), - ("list_partial_msodbcsql18", "reinstall "), - ("list_partial_mssql-tools18", "reinstall "), - ], -) -def test_setup_fails_closed( - install_script: str, - bash: str, - tmp_path: Path, - failure: str, - blocked_command: str, -) -> None: - result = run_install(install_script, bash, tmp_path, failure=failure) - assert result.returncode != 0 - assert not any( - command.startswith(blocked_command) for command in calls(result) - ) - assert "##vso[task.prependpath]" not in result.stdout - - -def test_missing_trust_capability_cannot_bypass_enforcement( - install_script: str, bash: str, tmp_path: Path -) -> None: - result = run_install(install_script, bash, tmp_path, supports_trust=False) - assert result.returncode != 0 - assert "untrusted tap" in result.stderr - assert INSTALL not in calls(result) - - -def test_no_global_trust_bypass(install_script: str) -> None: - assert "HOMEBREW_NO_REQUIRE_TAP_TRUST" not in install_script - assert "brew trust --formula" not in install_script - assert "trust --tap microsoft/mssql-release" in install_script - - -def test_path_with_spaces( - install_script: str, bash: str, tmp_path: Path -) -> None: - directory = tmp_path / "pipeline checkout with spaces" - directory.mkdir() - result = run_install(install_script, bash, directory) - assert result.returncode == 0, result.stderr - - -def test_bash_syntax(install_script: str, bash: str) -> None: - result = subprocess.run( - [bash, "--noprofile", "--norc", "-n"], - input=install_script, - text=True, - capture_output=True, - timeout=15, - check=False, - ) - assert result.returncode == 0, result.stderr diff --git a/test/native/test_pdo_password_cleanup.sh b/test/tools/test_pdo_password_cleanup.sh similarity index 95% rename from test/native/test_pdo_password_cleanup.sh rename to test/tools/test_pdo_password_cleanup.sh index 12dc7288c..c8cd705a5 100644 --- a/test/native/test_pdo_password_cleanup.sh +++ b/test/tools/test_pdo_password_cleanup.sh @@ -42,7 +42,7 @@ trap 'rm -rf -- "$work"' EXIT mkdir -p "$work/pdo_sqlsrv/shared" cp "$root"/source/pdo_sqlsrv/*.{cpp,h,m4} "$work/pdo_sqlsrv/" cp "$root"/source/shared/*.{cpp,h,hpp} "$work/pdo_sqlsrv/shared/" -cp "$root/test/native/pdo_password_cleanup_observer.cpp" "$work/observer.cpp" +cp "$root/test/tools/pdo_password_cleanup_observer.cpp" "$work/observer.cpp" # This observer wraps only calls originating in the disposable test module. # The production secure erase is still called; zeroed bytes are then inspected @@ -61,4 +61,4 @@ export MSPHPSQL_CLEANUP_MODULE="$work/pdo_sqlsrv/modules/pdo_sqlsrv.so" php -n -d extension=pdo -d extension=ffi -d ffi.enable=1 \ -d zend.exception_ignore_args=1 \ -d "extension=$MSPHPSQL_CLEANUP_MODULE" \ - "$root/test/native/pdo_password_cleanup.php" \ No newline at end of file + "$root/test/tools/pdo_password_cleanup.php" \ No newline at end of file From 7566453a05e748ba180d736c724ccb8af66eea47 Mon Sep 17 00:00:00 2001 From: Jahnvi Thakkar Date: Tue, 15 Sep 2026 12:52:55 +0530 Subject: [PATCH 4/4] Avoid Bash UID collision in pipeline SQL username variable --- azure-pipelines.yml | 49 +++++++++++++++++++++++---------------------- 1 file changed, 25 insertions(+), 24 deletions(-) diff --git a/azure-pipelines.yml b/azure-pipelines.yml index 57d7e3246..4cd8a9c72 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -6,7 +6,8 @@ variables: host: 'sql1' sqlsrv_db: 'sqlsrv_testdb' pdo_sqlsrv_db: 'pdo_sqlsrv_testdb' - uid: 'sa' + # Azure uppercases environment variable names; UID would shadow Bash's user ID. + sqlUser: 'sa' trigger: - dev @@ -146,10 +147,10 @@ jobs: # Try to connect /opt/homebrew/opt/mssql-tools18/bin/sqlcmd -S 127.0.0.1 \ - -U $(uid) -P "$(macpwd)" -No -C \ + -U $(sqlUser) -P "$(macpwd)" -No -C \ -Q "SELECT @@Version" || \ /usr/local/opt/mssql-tools18/bin/sqlcmd -S 127.0.0.1 \ - -U $(uid) -P "$(macpwd)" -No -C \ + -U $(sqlUser) -P "$(macpwd)" -No -C \ -Q "SELECT @@Version" echo "SQL Server connection verified!" @@ -272,7 +273,7 @@ jobs: set -e cd $(REPO_ROOT)/test/functional/setup export TEST_PHP_SQL_SERVER='127.0.0.1' - export TEST_PHP_SQL_UID='$(uid)' + export TEST_PHP_SQL_UID='$(sqlUser)' export TEST_PHP_SQL_PWD='$(macpwd)' # Add sqlcmd and bcp tools to PATH @@ -295,7 +296,7 @@ jobs: cd $(REPO_ROOT)/test/functional SQL_SERVER="127.0.0.1" - SQL_USER="$(uid)" + SQL_USER="$(sqlUser)" SQL_PWD="$(macpwd)" SRV_DB="$(macOS_sqlsrv_db)" PDO_DB="$(macOS_pdo_sqlsrv_db)" @@ -357,19 +358,19 @@ jobs: # Connection environment variables export MSSQL_SERVER='127.0.0.1' - export MSSQL_USER='$(uid)' + export MSSQL_USER='$(sqlUser)' export MSSQL_PASSWORD='$(macpwd)' export MSSQL_DATABASE_NAME='$(macOS_sqlsrv_db)' export MSSQL_DRIVER_NAME='ODBC Driver 18 for SQL Server' export TEST_PHP_SQL_SERVER='127.0.0.1' - export TEST_PHP_SQL_UID='$(uid)' + export TEST_PHP_SQL_UID='$(sqlUser)' export TEST_PHP_SQL_PWD='$(macpwd)' # Verify connection before running tests echo "Verifying database connection before tests..." $PHP_BIN -c $PHP_INI -r " \$server = '127.0.0.1'; - \$options = array('Database' => '$(macOS_sqlsrv_db)', 'UID' => '$(uid)', 'PWD' => '$(macpwd)', 'Encrypt' => 'no'); + \$options = array('Database' => '$(macOS_sqlsrv_db)', 'UID' => '$(sqlUser)', 'PWD' => '$(macpwd)', 'Encrypt' => 'no'); \$conn = sqlsrv_connect(\$server, \$options); if (\$conn === false) { print_r(sqlsrv_errors()); @@ -494,7 +495,7 @@ jobs: - script: | export TEST_PHP_SQL_SERVER='127.0.0.1' - export TEST_PHP_SQL_UID='$(uid)' + export TEST_PHP_SQL_UID='$(sqlUser)' export TEST_PHP_SQL_PWD='$(macpwd)' echo "Dropping test databases..." @@ -592,7 +593,7 @@ jobs: sleep 10 # Verify SQL Server is running - docker exec -t $(host) /opt/mssql-tools18/bin/sqlcmd -S $(server) -C -U $(uid) -P $(pwd) -Q 'SELECT @@VERSION' + docker exec -t $(host) /opt/mssql-tools18/bin/sqlcmd -S $(server) -C -U $(sqlUser) -P $(pwd) -Q 'SELECT @@VERSION' displayName: 'Run SQL Server for Linux' - script: | @@ -622,7 +623,7 @@ jobs: - script: | echo "Setting up test databases..." export TEST_PHP_SQL_SERVER='$(server)' - export TEST_PHP_SQL_UID='$(uid)' + export TEST_PHP_SQL_UID='$(sqlUser)' export TEST_PHP_SQL_PWD='$(pwd)' cd $(Build.SourcesDirectory)/test/functional/setup @@ -669,7 +670,7 @@ jobs: displayName: 'Verify native PDO password erasure' env: MSSQL_SERVER: $(server) - MSSQL_USER: $(uid) + MSSQL_USER: $(sqlUser) MSSQL_PASSWORD: $(pwd) MSSQL_DRIVER_NAME: 'ODBC Driver 18 for SQL Server' LANG: 'en_US.UTF-8' @@ -680,13 +681,13 @@ jobs: cd $(Build.SourcesDirectory)/test/functional/sqlsrv sed -i -e 's/TARGET_SERVER/'"$(server)"'/g' MsSetup.inc sed -i -e 's/TARGET_DATABASE/'"$(sqlsrv_db)"'/g' MsSetup.inc - sed -i -e 's/TARGET_USERNAME/'"$(uid)"'/g' MsSetup.inc + sed -i -e 's/TARGET_USERNAME/'"$(sqlUser)"'/g' MsSetup.inc sed -i -e 's/TARGET_PASSWORD/'"$(pwd)"'/g' MsSetup.inc cd $(Build.SourcesDirectory)/test/functional/pdo_sqlsrv sed -i -e 's/TARGET_SERVER/'"$(server)"'/g' MsSetup.inc sed -i -e 's/TARGET_DATABASE/'"$(pdo_sqlsrv_db)"'/g' MsSetup.inc - sed -i -e 's/TARGET_USERNAME/'"$(uid)"'/g' MsSetup.inc + sed -i -e 's/TARGET_USERNAME/'"$(sqlUser)"'/g' MsSetup.inc sed -i -e 's/TARGET_PASSWORD/'"$(pwd)"'/g' MsSetup.inc echo "MsSetup.inc files updated" @@ -695,7 +696,7 @@ jobs: - script: | cd $(Build.SourcesDirectory)/test/functional/sqlsrv export MSSQL_SERVER='$(server)' - export MSSQL_USER='$(uid)' + export MSSQL_USER='$(sqlUser)' export MSSQL_PASSWORD='$(pwd)' export MSSQL_DATABASE_NAME='$(sqlsrv_db)' export MSSQL_DRIVER_NAME='ODBC Driver 18 for SQL Server' @@ -707,7 +708,7 @@ jobs: - script: | cd $(Build.SourcesDirectory)/test/functional/pdo_sqlsrv export MSSQL_SERVER='$(server)' - export MSSQL_USER='$(uid)' + export MSSQL_USER='$(sqlUser)' export MSSQL_PASSWORD='$(pwd)' export MSSQL_DATABASE_NAME='$(pdo_sqlsrv_db)' export MSSQL_DRIVER_NAME='ODBC Driver 18 for SQL Server' @@ -874,7 +875,7 @@ jobs: # Test connection with SQL Auth using named pipe Write-Host "Testing connection to LocalDB with SQL Authentication..." - sqlcmd -S "(localdb)\MSSQLLocalDB" -U $(uid) -P "$(pwd)" -Q "SELECT @@VERSION" + sqlcmd -S "(localdb)\MSSQLLocalDB" -U $(sqlUser) -P "$(pwd)" -Q "SELECT @@VERSION" Write-Host "SQL Server LocalDB is ready" displayName: 'Start SQL Server LocalDB' @@ -887,7 +888,7 @@ jobs: (Get-Content .\MsSetup.inc) | ForEach-Object { $_ -replace "TARGET_SERVER", $server ` -replace "TARGET_DATABASE", "$(sqlsrv_db)" ` - -replace "TARGET_USERNAME", "$(uid)" ` + -replace "TARGET_USERNAME", "$(sqlUser)" ` -replace "TARGET_PASSWORD", "$(pwd)" } | Set-Content .\MsSetup.inc @@ -898,7 +899,7 @@ jobs: (Get-Content .\MsSetup.inc) | ForEach-Object { $_ -replace "TARGET_SERVER", $server ` -replace "TARGET_DATABASE", "$(pdo_sqlsrv_db)" ` - -replace "TARGET_USERNAME", "$(uid)" ` + -replace "TARGET_USERNAME", "$(sqlUser)" ` -replace "TARGET_PASSWORD", "$(pwd)" } | Set-Content .\MsSetup.inc @@ -909,7 +910,7 @@ jobs: (Get-Content .\connect.inc) | ForEach-Object { $_ -replace '(\$server = )[^;]+;', "`$1'$server';" ` -replace '(\$databaseName = )[^;]+;', "`$1'$(sqlsrv_db)';" ` - -replace '(\$uid = )[^;]+;', "`$1'$(uid)';" ` + -replace '(\$uid = )[^;]+;', "`$1'$(sqlUser)';" ` -replace '(\$pwd = )[^;]+;', "`$1'$(pwd)';" } | Set-Content .\connect.inc @@ -917,7 +918,7 @@ jobs: (Get-Content .\connect.inc) | ForEach-Object { $_ -replace '(\$server = )[^;]+;', "`$1'$server';" ` -replace '(\$databaseName = )[^;]+;', "`$1'$(pdo_sqlsrv_db)';" ` - -replace '(\$uid = )[^;]+;', "`$1'$(uid)';" ` + -replace '(\$uid = )[^;]+;', "`$1'$(sqlUser)';" ` -replace '(\$pwd = )[^;]+;', "`$1'$(pwd)';" } | Set-Content .\connect.inc displayName: 'Update connection credentials' @@ -997,7 +998,7 @@ jobs: # Set environment variables for the Python script $env:TEST_PHP_SQL_SERVER = "(localdb)\MSSQLLocalDB" - $env:TEST_PHP_SQL_UID = "$(uid)" + $env:TEST_PHP_SQL_UID = "$(sqlUser)" $env:TEST_PHP_SQL_PWD = "$(pwd)" cd $(Build.SourcesDirectory)\test\functional\setup @@ -1023,7 +1024,7 @@ jobs: set PATH=C:\Program Files\OpenCppCoverage;%PATH% cd $(Build.SourcesDirectory)\test\functional\sqlsrv set MSSQL_SERVER=(localdb)\MSSQLLocalDB - set MSSQL_USER=$(uid) + set MSSQL_USER=$(sqlUser) set MSSQL_PASSWORD=$(pwd) set MSSQL_DATABASE_NAME=$(sqlsrv_db) OpenCppCoverage --sources $(Build.SourcesDirectory)\buildscripts --modules php_sqlsrv.dll --cover_children --export_type cobertura:$(Build.SourcesDirectory)\coverage-sqlsrv.xml -- php run-tests.php *.phpt --no-color --show-diff 2>&1 | tee ..\sqlsrv.log @@ -1033,7 +1034,7 @@ jobs: set PATH=C:\Program Files\OpenCppCoverage;%PATH% cd $(Build.SourcesDirectory)\test\functional\pdo_sqlsrv set MSSQL_SERVER=(localdb)\MSSQLLocalDB - set MSSQL_USER=$(uid) + set MSSQL_USER=$(sqlUser) set MSSQL_PASSWORD=$(pwd) set MSSQL_DATABASE_NAME=$(pdo_sqlsrv_db) OpenCppCoverage --sources $(Build.SourcesDirectory)\buildscripts --modules php_pdo_sqlsrv.dll --cover_children --export_type cobertura:$(Build.SourcesDirectory)\coverage-pdo_sqlsrv.xml -- php run-tests.php *.phpt --no-color --show-diff 2>&1 | tee ..\pdo_sqlsrv.log