diff --git a/CHANGELOG.md b/CHANGELOG.md index 46fce34cd..6c6d404db 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,14 @@ 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. +- 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..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 @@ -104,23 +105,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" @@ -145,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!" @@ -271,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 @@ -294,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)" @@ -356,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()); @@ -493,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..." @@ -591,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: | @@ -621,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 @@ -662,18 +664,30 @@ jobs: php --ri pdo_sqlsrv displayName: 'Build and install drivers' + - script: | + set -e + bash "$(Build.SourcesDirectory)/test/tools/test_pdo_password_cleanup.sh" + displayName: 'Verify native PDO password erasure' + env: + MSSQL_SERVER: $(server) + MSSQL_USER: $(sqlUser) + 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 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" @@ -682,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' @@ -694,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' @@ -861,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' @@ -874,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 @@ -885,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 @@ -896,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 @@ -904,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' @@ -984,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 @@ -1010,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 @@ -1020,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 diff --git a/source/pdo_sqlsrv/pdo_dbh.cpp b/source/pdo_sqlsrv/pdo_dbh.cpp index 63755aafe..5d9670d6f 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,17 +71,12 @@ 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"; } -enum PDO_CONN_OPTIONS { - - PDO_CONN_OPTION_SERVER = SQLSRV_CONN_OPTION_DRIVER_SPECIFIC, - -}; - enum PDO_STMT_OPTIONS { PDO_STMT_OPTION_ENCODING = SQLSRV_STMT_OPTION_DRIVER_SPECIFIC, @@ -223,6 +222,25 @@ const connection_option PDO_CONN_OPTS[] = { CONN_ATTR_STRING, conn_str_append_func::func }, + // Passwords use the parser's separate secure buffer, not 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 +367,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 +412,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 +502,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), @@ -631,9 +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 ); + pdo_secure_password dsn_password; try { @@ -660,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 ); @@ -678,8 +723,22 @@ 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; + 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 = dsn_password.get(); + } + } + 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_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/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/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 9fe39087a..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); } @@ -834,24 +822,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/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/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..ab2b03445 --- /dev/null +++ b/test/functional/pdo_sqlsrv/pdo_connection_alias_values.phpt @@ -0,0 +1,68 @@ +--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; + if (isset($e->errorInfo[0], $e->errorInfo[1])) { + echo 'SQLSTATE: ', $e->errorInfo[0], ', code: ', $e->errorInfo[1], 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 . '}'; +} +// 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=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'); +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=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 +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..5212f8410 --- /dev/null +++ b/test/functional/sqlsrv/sqlsrv_connection_alias_values.phpt @@ -0,0 +1,65 @@ +--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; + // 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()'); + $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); +} + +// 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' => 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'); +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' => 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 +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 diff --git a/test/tools/pdo_password_cleanup.php b/test/tools/pdo_password_cleanup.php new file mode 100644 index 000000000..e9211c6a1 --- /dev/null +++ b/test/tools/pdo_password_cleanup.php @@ -0,0 +1,99 @@ +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'; +// 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), + 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; + // 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(); +exit($passed ? 0 : 1); \ No newline at end of file diff --git a/test/tools/pdo_password_cleanup_observer.cpp b/test/tools/pdo_password_cleanup_observer.cpp new file mode 100644 index 000000000..919d1b8ac --- /dev/null +++ b/test/tools/pdo_password_cleanup_observer.cpp @@ -0,0 +1,112 @@ +// 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 +#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 (value == NULL || expected_count == MAX_EXPECTED || length >= MAX_VALUE) { + ++failures; + return; + } + expected_release& entry = expected[expected_count++]; + std::copy_n(value, length, entry.value); + 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/tools/test_pdo_password_cleanup.sh b/test/tools/test_pdo_password_cleanup.sh new file mode 100644 index 000000000..c8cd705a5 --- /dev/null +++ b/test/tools/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/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 +# 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/tools/pdo_password_cleanup.php" \ No newline at end of file