Let ODBC driver verify azure ad authentications (#1326)

This commit is contained in:
Jenny Tam 2021-11-23 12:52:08 -08:00 committed by GitHub
parent a570522863
commit cbfc7638e2
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
12 changed files with 6 additions and 250 deletions

View file

@ -173,31 +173,6 @@ void conn_string_parser::validate_key( _In_reads_(key_len) const char *key, _Ino
THROW_PDO_ERROR( this->ctx, PDO_SQLSRV_ERROR_INVALID_DSN_KEY, static_cast<char*>( key_name ) );
}
void conn_string_parser::add_key_value_pair( _In_reads_(len) const char* value, _In_ int len )
{
// if the keyword is 'Authentication', check whether the user specified option is supported
bool valid = true;
if ( stricmp( this->current_key_name, ODBCConnOptions::Authentication ) == 0 ) {
if (len <= 0)
valid = false;
else {
// extract option from the value by len
sqlsrv_malloc_auto_ptr<char> option;
option = static_cast<char*>( sqlsrv_malloc( len + 1 ) );
memcpy_s( option, len + 1, value, len );
option[len] = '\0';
valid = AzureADOptions::isAuthValid(option, len);
}
}
if( !valid ) {
THROW_PDO_ERROR( this->ctx, PDO_SQLSRV_ERROR_INVALID_AUTHENTICATION_OPTION, this->current_key_name );
}
string_parser::add_key_value_pair( value, len );
}
inline bool sql_string_parser::is_placeholder_char( char c )
{
// placeholder only accepts numbers, upper and lower case alphabets and underscore

View file

@ -377,10 +377,6 @@ pdo_error PDO_ERRORS[] = {
PDO_SQLSRV_ERROR_EMULATE_INOUT_UNSUPPORTED,
{ IMSSP, (SQLCHAR*) "Statement with emulate prepare on does not support output or input_output parameters.", -72, false }
},
{
PDO_SQLSRV_ERROR_INVALID_AUTHENTICATION_OPTION,
{ IMSSP, (SQLCHAR*) "Invalid option for the Authentication keyword. Only SqlPassword, ActiveDirectoryPassword, ActiveDirectoryMsi or ActiveDirectoryServicePrincipal is supported.", -73, false }
},
{
SQLSRV_ERROR_CE_DRIVER_REQUIRED,
{ IMSSP, (SQLCHAR*) "The Always Encrypted feature requires Microsoft ODBC Driver 17 for SQL Server (or above) for %1!s!.", -78, true }
@ -441,10 +437,6 @@ pdo_error PDO_ERRORS[] = {
SQLSRV_ERROR_INVALID_DECIMAL_PLACES,
{ IMSSP, (SQLCHAR*) "Expected an integer to specify number of decimals to format the output values of decimal data types.", -92, false}
},
{
SQLSRV_ERROR_AAD_MSI_UID_PWD_NOT_NULL,
{ IMSSP, (SQLCHAR*) "When using ActiveDirectoryMsi Authentication, PWD must be NULL. UID can be NULL, but if not, an empty string is not accepted.", -93, false}
},
{
SQLSRV_ERROR_DATA_CLASSIFICATION_PRE_EXECUTION,
{ IMSSP, (SQLCHAR*) "The statement must be executed to retrieve Data Classification Sensitivity Metadata.", -94, false}

View file

@ -149,9 +149,6 @@ class conn_string_parser : private string_parser
int discard_trailing_white_spaces( _In_reads_(len) const char* str, _Inout_ int len );
void validate_key( _In_reads_(key_len) const char *key, _Inout_ int key_len);
protected:
void add_key_value_pair( _In_reads_(len) const char* value, _In_ int len);
public:
conn_string_parser( _In_ sqlsrv_context& ctx, _In_ const char* dsn, _In_ int len, _In_ HashTable* conn_options_ht );
void parse_conn_string( void );
@ -391,7 +388,6 @@ enum PDO_ERROR_CODES {
PDO_SQLSRV_ERROR_INVALID_OUTPUT_PARAM_TYPE,
PDO_SQLSRV_ERROR_INVALID_CURSOR_WITH_SCROLL_TYPE,
PDO_SQLSRV_ERROR_EMULATE_INOUT_UNSUPPORTED,
PDO_SQLSRV_ERROR_INVALID_AUTHENTICATION_OPTION,
PDO_SQLSRV_ERROR_CE_DIRECT_QUERY_UNSUPPORTED,
PDO_SQLSRV_ERROR_CE_EMULATE_PREPARE_UNSUPPORTED,
PDO_SQLSRV_ERROR_EXTENDED_STRING_TYPE_INVALID

View file

@ -704,41 +704,6 @@ bool core_is_conn_opt_value_escaped( _Inout_ const char* value, _Inout_ size_t v
return true;
}
namespace AzureADOptions {
enum AAD_AUTH_TYPE {
MIN_AAD_AUTH_TYPE = 0,
SQL_PASSWORD = 0,
AAD_PASSWORD,
AAD_MSI,
AAD_SPA,
MAX_AAD_AUTH_TYPE
};
const char *AADAuths[] = { "SqlPassword", "ActiveDirectoryPassword", "ActiveDirectoryMsi", "ActiveDirectoryServicePrincipal" };
bool isAuthValid(_In_z_ const char* value, _In_ size_t value_len)
{
if (value_len <= 0)
return false;
bool isValid = false;
for (short i = MIN_AAD_AUTH_TYPE; i < MAX_AAD_AUTH_TYPE && !isValid; i++)
{
if (!stricmp(value, AADAuths[i])) {
isValid = true;
}
}
return isValid;
}
bool isAADMsi(_In_z_ const char* value)
{
return (value != NULL && !stricmp(value, AADAuths[AAD_MSI]));
}
}
// *** internal connection functions and classes ***
namespace {
@ -792,10 +757,11 @@ void build_connection_string_and_set_conn_attr( _Inout_ sqlsrv_conn* conn, _Inou
access_token_used = true;
}
// Check if Authentication is ActiveDirectoryMSI
// Check if Authentication is ActiveDirectoryMSI because we have to handle this case differently
// https://docs.microsoft.com/en-ca/azure/active-directory/managed-identities-azure-resources/overview
bool activeDirectoryMSI = false;
if (authentication_option_used) {
const char aadMSIoption[] = "ActiveDirectoryMSI";
zval* auth_option = NULL;
auth_option = zend_hash_index_find(options, SQLSRV_CONN_OPTION_AUTHENTICATION);
@ -804,34 +770,16 @@ void build_connection_string_and_set_conn_attr( _Inout_ sqlsrv_conn* conn, _Inou
option = Z_STRVAL_P(auth_option);
}
//if (option != NULL && !stricmp(option, AzureADOptions::AZURE_AUTH_AD_MSI)) {
activeDirectoryMSI = AzureADOptions::isAADMsi(option);
if (activeDirectoryMSI) {
// There are two types of managed identities:
// (1) A system-assigned managed identity: UID must be NULL
// (2) A user-assigned managed identity: UID defined but must not be an empty string
// In both cases, PWD must be NULL
bool invalid = false;
if (pwd != NULL) {
invalid = true;
} else {
if (uid != NULL && strnlen_s(uid) == 0) {
invalid = true;
}
}
CHECK_CUSTOM_ERROR(invalid, conn, SQLSRV_ERROR_AAD_MSI_UID_PWD_NOT_NULL ) {
throw core::CoreException();
}
if (option != NULL && !stricmp(option, aadMSIoption)) {
activeDirectoryMSI = true;
}
}
// Add the server name
common_conn_str_append_func( ODBCConnOptions::SERVER, server, strnlen_s( server ), connection_string );
// If uid is not present then we use trusted connection -- but not when access token or ActiveDirectoryMSI is used,
// because they are incompatible
// If uid is not present then we use trusted connection -- but not when connecting
// using the access token or Authentication is ActiveDirectoryMSI
if (!access_token_used && !activeDirectoryMSI) {
if (uid == NULL || strnlen_s(uid) == 0) {
connection_string += CONNECTION_OPTION_NO_CREDENTIALS; // "Trusted_Connection={Yes};"

View file

@ -190,11 +190,6 @@ const int SQL_SERVER_2005_DEFAULT_DATETIME_SCALE = 3;
const int SQL_SERVER_2008_DEFAULT_DATETIME_PRECISION = 34;
const int SQL_SERVER_2008_DEFAULT_DATETIME_SCALE = 7;
namespace AzureADOptions {
bool isAuthValid(_In_z_ const char* value, _In_ size_t value_len);
bool isAADMsi(_In_z_ const char* value);
}
// the message returned by ODBC Driver for SQL Server
const char ODBC_CONNECTION_BUSY_ERROR[] = "Connection is busy with results for another command";
@ -2011,7 +2006,6 @@ enum SQLSRV_ERROR_CODES {
SQLSRV_ERROR_INVALID_OPTION_WITH_ACCESS_TOKEN,
SQLSRV_ERROR_EMPTY_ACCESS_TOKEN,
SQLSRV_ERROR_INVALID_DECIMAL_PLACES,
SQLSRV_ERROR_AAD_MSI_UID_PWD_NOT_NULL,
SQLSRV_ERROR_DATA_CLASSIFICATION_PRE_EXECUTION,
SQLSRV_ERROR_DATA_CLASSIFICATION_NOT_AVAILABLE,
SQLSRV_ERROR_DATA_CLASSIFICATION_FAILED,

View file

@ -1362,16 +1362,6 @@ int get_conn_option_key( _Inout_ sqlsrv_context& ctx, _In_ zend_string* key, _In
throw ss::SSException();
}
bool valid = true;
if( stricmp( SS_CONN_OPTS[i].sqlsrv_name, SSConnOptionNames::Authentication ) == 0 ) {
valid = AzureADOptions::isAuthValid(value, value_len);
}
CHECK_CUSTOM_ERROR( !valid, ctx, SS_SQLSRV_ERROR_INVALID_AUTHENTICATION_OPTION, SS_CONN_OPTS[i].sqlsrv_name ) {
throw ss::SSException();
}
break;
}
case CONN_ATTR_INVALID:

View file

@ -206,7 +206,6 @@ enum SS_ERROR_CODES {
SS_SQLSRV_ERROR_CONNECT_BRACES_NOT_ESCAPED,
SS_SQLSRV_ERROR_INVALID_OUTPUT_PARAM_TYPE,
SS_SQLSRV_ERROR_PARAM_VAR_NOT_REF,
SS_SQLSRV_ERROR_INVALID_AUTHENTICATION_OPTION,
SS_SQLSRV_ERROR_AE_QUERY_SQLTYPE_REQUIRED
};

View file

@ -363,10 +363,6 @@ ss_error SS_ERRORS[] = {
"Output or bidirectional variable parameters (SQLSRV_PARAM_OUT and SQLSRV_PARAM_INOUT) passed to sqlsrv_prepare or sqlsrv_query should be passed by reference, not by value."
, -61, true }
},
{
SS_SQLSRV_ERROR_INVALID_AUTHENTICATION_OPTION,
{ IMSSP, (SQLCHAR*)"Invalid option for the Authentication keyword. Only SqlPassword, ActiveDirectoryPassword, ActiveDirectoryMsi or ActiveDirectoryServicePrincipal is supported.", -62, false }
},
{
SS_SQLSRV_ERROR_AE_QUERY_SQLTYPE_REQUIRED,
{ IMSSP, (SQLCHAR*)"Must specify the SQL type for each parameter in a parameterized query when using sqlsrv_query in a column encryption enabled connection.", -63, false }
@ -429,10 +425,6 @@ ss_error SS_ERRORS[] = {
SQLSRV_ERROR_INVALID_DECIMAL_PLACES,
{ IMSSP, (SQLCHAR*) "Expected an integer to specify number of decimals to format the output values of decimal data types.", -117, false}
},
{
SQLSRV_ERROR_AAD_MSI_UID_PWD_NOT_NULL,
{ IMSSP, (SQLCHAR*) "When using ActiveDirectoryMsi Authentication, PWD must be NULL. UID can be NULL, but if not, an empty string is not accepted.", -118, false}
},
{
SQLSRV_ERROR_DATA_CLASSIFICATION_PRE_EXECUTION,
{ IMSSP, (SQLCHAR*) "The statement must be executed to retrieve Data Classification Sensitivity Metadata.", -119, false}

View file

@ -34,22 +34,6 @@ if ($stmt === false) {
unset($conn);
///////////////////////////////////////////////////////////////////////////////////////////
// Test Azure AD with integrated authentication. This should fail because
// we don't support it.
//
$connectionInfo = "Authentication = ActiveDirectoryIntegrated; TrustServerCertificate = true;";
try {
$conn = new PDO("sqlsrv:server = $server ; $connectionInfo");
echo "Connected successfully with Authentication=ActiveDirectoryIntegrated.\n";
unset($conn);
} catch (PDOException $e) {
echo "Could not connect with Authentication=ActiveDirectoryIntegrated.\n";
print_r($e->getMessage());
echo "\n";
}
///////////////////////////////////////////////////////////////////////////////////////////
// Test Azure AD on an Azure database instance. Replace $azureServer, etc with
// your credentials to test, or this part is skipped.
@ -95,6 +79,4 @@ if ($azureServer != 'TARGET_AD_SERVER') {
--EXPECTF--
Connected successfully with Authentication=SqlPassword.
string(1) "%d"
Could not connect with Authentication=ActiveDirectoryIntegrated.
SQLSTATE[IMSSP]: Invalid option for the Authentication keyword. Only SqlPassword, ActiveDirectoryPassword, ActiveDirectoryMsi or ActiveDirectoryServicePrincipal is supported.
%s with Authentication=ActiveDirectoryPassword.

View file

@ -18,58 +18,6 @@ function verifyErrorMessage($exception, $expectedError, $msg)
}
}
function connectWithInvalidOptions()
{
global $server;
$message = 'AzureAD Managed Identity test: expected to fail with ';
$expectedError = 'When using ActiveDirectoryMsi Authentication, PWD must be NULL. UID can be NULL, but if not, an empty string is not accepted';
$uid = '';
$connectionInfo = "Authentication = ActiveDirectoryMsi;";
$testCase = 'empty UID provided';
try {
$conn = new PDO("sqlsrv:server = $server; $connectionInfo", $uid);
echo $message . $testCase . PHP_EOL;
} catch(PDOException $e) {
verifyErrorMessage($e, $expectedError, $testCase);
}
unset($connectionInfo);
$pwd = '';
$connectionInfo = "Authentication = ActiveDirectoryMsi;";
$testCase = 'empty PWD provided';
try {
$conn = new PDO("sqlsrv:server = $server; $connectionInfo", null, $pwd);
echo $message . $testCase . PHP_EOL;
} catch(PDOException $e) {
verifyErrorMessage($e, $expectedError, $testCase);
}
unset($connectionInfo);
$pwd = 'dummy';
$connectionInfo = "Authentication = ActiveDirectoryMsi;";
$testCase = 'PWD provided';
try {
$conn = new PDO("sqlsrv:server = $server; $connectionInfo", null, $pwd);
echo $message . $testCase . PHP_EOL;
} catch(PDOException $e) {
verifyErrorMessage($e, $expectedError, $testCase);
}
unset($connectionInfo);
$expectedError = 'When using Azure AD Access Token, the connection string must not contain UID, PWD, or Authentication keywords.';
$connectionInfo = "Authentication = ActiveDirectoryMsi; AccessToken = '123';";
$testCase = 'AccessToken option';
try {
$conn = new PDO("sqlsrv:server = $server; $connectionInfo");
echo $message . $testCase . PHP_EOL;
} catch(PDOException $e) {
verifyErrorMessage($e, $expectedError, $testCase);
}
unset($connectionInfo);
}
function connectInvalidServer()
{
global $server, $driver, $uid, $pwd;
@ -102,9 +50,6 @@ function connectInvalidServer()
require_once('MsSetup.inc');
// Test some error conditions
connectWithInvalidOptions();
// Make a connection to an invalid server
connectInvalidServer();

View file

@ -34,22 +34,6 @@ if (sqlsrv_fetch($stmt)) {
sqlsrv_free_stmt($stmt);
sqlsrv_close($conn);
///////////////////////////////////////////////////////////////////////////////////////////
// Test Azure AD with integrated authentication. This should fail because
// we don't support it.
//
$connectionInfo = array( "Authentication"=>"ActiveDirectoryIntegrated", "TrustServerCertificate"=>true );
$conn = sqlsrv_connect($server, $connectionInfo);
if ($conn === false) {
echo "Could not connect with Authentication=ActiveDirectoryIntegrated.\n";
$errors = sqlsrv_errors();
print_r($errors[0]);
} else {
echo "Connected successfully with Authentication=ActiveDirectoryIntegrated.\n";
sqlsrv_close($conn);
}
///////////////////////////////////////////////////////////////////////////////////////////
// Test Azure AD on an Azure database instance. Replace $azureServer, etc with
// your credentials to test, or this part is skipped.
@ -99,14 +83,4 @@ if ($azureServer != 'TARGET_AD_SERVER') {
--EXPECTF--
Connected successfully with Authentication=SqlPassword.
string(1) "%d"
Could not connect with Authentication=ActiveDirectoryIntegrated.
Array
(
[0] => IMSSP
[SQLSTATE] => IMSSP
[1] => -62
[code] => -62
[2] => Invalid option for the Authentication keyword. Only SqlPassword, ActiveDirectoryPassword, ActiveDirectoryMsi or ActiveDirectoryServicePrincipal is supported.
[message] => Invalid option for the Authentication keyword. Only SqlPassword, ActiveDirectoryPassword, ActiveDirectoryMsi or ActiveDirectoryServicePrincipal is supported.
)
%s with Authentication=ActiveDirectoryPassword.

View file

@ -19,34 +19,6 @@ function verifyErrorMessage($conn, $expectedError, $msg)
}
}
function connectWithInvalidOptions()
{
global $server;
$expectedError = 'When using ActiveDirectoryMsi Authentication, PWD must be NULL. UID can be NULL, but if not, an empty string is not accepted';
$connectionInfo = array("UID"=>"", "Authentication" => "ActiveDirectoryMsi");
$conn = sqlsrv_connect($server, $connectionInfo);
verifyErrorMessage($conn, $expectedError, 'empty UID provided');
unset($connectionInfo);
$connectionInfo = array("PWD"=>"", "Authentication" => "ActiveDirectoryMsi");
$conn = sqlsrv_connect($server, $connectionInfo);
verifyErrorMessage($conn, $expectedError, 'empty PWD provided');
unset($connectionInfo);
$connectionInfo = array("PWD"=>"pwd", "Authentication" => "ActiveDirectoryMsi");
$conn = sqlsrv_connect($server, $connectionInfo);
verifyErrorMessage($conn, $expectedError, 'PWD provided');
unset($connectionInfo);
$expectedError = 'When using Azure AD Access Token, the connection string must not contain UID, PWD, or Authentication keywords.';
$connectionInfo = array("Authentication"=>"ActiveDirectoryMsi", "AccessToken" => "123");
$conn = sqlsrv_connect($server, $connectionInfo);
verifyErrorMessage($conn, $expectedError, 'AccessToken option');
unset($connectionInfo);
}
function connectInvalidServer()
{
global $server, $driver, $userName, $userPassword;
@ -76,9 +48,6 @@ function connectInvalidServer()
}
}
// Test some error conditions
connectWithInvalidOptions($server);
// Make a connection to an invalid server
connectInvalidServer();