Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for Managed Identity for Azure resources #875

Merged
merged 24 commits into from
Jan 12, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
8f9ecde
Merge pull request #787 from Microsoft/dev
yitam May 30, 2018
9654020
Dev (#820)
yitam Jul 21, 2018
ff3c6a4
Merge branch 'master' into dev
yitam Sep 24, 2018
b47cec2
Dev to Master - 5.4.0-preview (#853)
yitam Sep 24, 2018
0fa3a43
Merge branch 'master' of https://github.com/Microsoft/msphpsql
yitam Sep 24, 2018
80442d4
Change readme links to https
BackEndTea Oct 1, 2018
14acf00
Merge pull request #857 from BackEndTea/patch-1
david-puglielli Oct 1, 2018
097c06f
Merge branch 'master' of https://github.com/Microsoft/msphpsql
yitam Oct 3, 2018
eb679ea
Update survey image link
Nov 14, 2018
2e13851
Merge branch 'master' of https://github.com/Microsoft/msphpsql
yitam Nov 14, 2018
f831c9e
Merge branch 'master' into dev
yitam Nov 14, 2018
b69c824
Merge branch 'dev' of https://github.com/Microsoft/msphpsql into dev
yitam Nov 16, 2018
3919628
Merge branch 'dev' of https://github.com/Microsoft/msphpsql into dev
yitam Nov 22, 2018
1bdbf86
Merge branch 'dev' of https://github.com/Microsoft/msphpsql into dev
yitam Nov 23, 2018
e3d897f
Merge branch 'dev' of https://github.com/Microsoft/msphpsql into dev
yitam Nov 28, 2018
313913a
Merge branch 'dev' of https://github.com/Microsoft/msphpsql into dev
yitam Nov 30, 2018
2b3b7da
Merge branch 'dev' of https://github.com/Microsoft/msphpsql into dev
yitam Dec 3, 2018
7d45079
Merge branch 'dev' of https://github.com/Microsoft/msphpsql into dev
yitam Dec 6, 2018
4d2ebb9
Merge branch 'dev' of https://github.com/Microsoft/msphpsql into dev
yitam Dec 7, 2018
28404e8
Merge branch 'dev' of https://github.com/Microsoft/msphpsql into dev
yitam Dec 18, 2018
d65f6a3
Merge branch 'dev' of https://github.com/Microsoft/msphpsql into dev
yitam Jan 3, 2019
7653e96
Merge branch 'dev' of https://github.com/Microsoft/msphpsql into dev
yitam Jan 4, 2019
2281bc2
Merge branch 'dev' of https://github.com/Microsoft/msphpsql into dev
yitam Jan 7, 2019
bbc58c5
Support for Managed Identity for Azure resources
yitam Nov 8, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion source/pdo_sqlsrv/pdo_util.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -383,7 +383,7 @@ pdo_error PDO_ERRORS[] = {
},
{
PDO_SQLSRV_ERROR_INVALID_AUTHENTICATION_OPTION,
{ IMSSP, (SQLCHAR*) "Invalid option for the Authentication keyword. Only SqlPassword or ActiveDirectoryPassword is supported.", -73, false }
{ IMSSP, (SQLCHAR*) "Invalid option for the Authentication keyword. Only SqlPassword, ActiveDirectoryPassword, or ActiveDirectoryMsi is supported.", -73, false }
},
{
SQLSRV_ERROR_CE_DRIVER_REQUIRED,
Expand Down Expand Up @@ -445,6 +445,10 @@ 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}
},

{ UINT_MAX, {} }
};
Expand Down
47 changes: 41 additions & 6 deletions source/shared/core_conn.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -730,7 +730,7 @@ bool core_is_authentication_option_valid( _In_z_ const char* value, _In_ size_t
if (value_len <= 0)
return false;

if( ! stricmp( value, AzureADOptions::AZURE_AUTH_SQL_PASSWORD ) || ! stricmp( value, AzureADOptions::AZURE_AUTH_AD_PASSWORD ) ) {
if (!stricmp(value, AzureADOptions::AZURE_AUTH_SQL_PASSWORD) || !stricmp(value, AzureADOptions::AZURE_AUTH_AD_PASSWORD) || !stricmp(value, AzureADOptions::AZURE_AUTH_AD_MSI)) {
return true;
}

Expand Down Expand Up @@ -769,16 +769,18 @@ void build_connection_string_and_set_conn_attr( _Inout_ sqlsrv_conn* conn, _Inou
bool mars_mentioned = false;
connection_option const* conn_opt;
bool access_token_used = false;
bool authentication_option_used = zend_hash_index_exists(options, SQLSRV_CONN_OPTION_AUTHENTICATION);

try {
// First of all, check if access token is specified. If so, check if UID,�PWD,�Authentication exist
// Since connection options access token and authentication cannot coexist, check if both of them are used.
// If access token is specified, check UID and�PWD as well.
// No need to check the keyword Trusted_Connection�because it is not among the acceptable options for SQLSRV drivers
if (zend_hash_index_exists(options, SQLSRV_CONN_OPTION_ACCESS_TOKEN)) {
bool invalidOptions = false;

// UID and PWD have to be NULLs... throw an exception as long as the user has specified any of them in the connection string,
// even if they may be empty strings. Likewise if the keyword Authentication exists
if (uid != NULL || pwd != NULL || zend_hash_index_exists(options, SQLSRV_CONN_OPTION_AUTHENTICATION)) {
if (uid != NULL || pwd != NULL || authentication_option_used) {
invalidOptions = true;
}

Expand All @@ -789,11 +791,44 @@ void build_connection_string_and_set_conn_attr( _Inout_ sqlsrv_conn* conn, _Inou
access_token_used = true;
}

// Check if Authentication is ActiveDirectoryMSI
// https://docs.microsoft.com/en-ca/azure/active-directory/managed-identities-azure-resources/overview
bool activeDirectoryMSI = false;
if (authentication_option_used) {
zval* auth_option = NULL;
auth_option = zend_hash_index_find(options, SQLSRV_CONN_OPTION_AUTHENTICATION);

char* option = Z_STRVAL_P(auth_option);

if (!stricmp(option, AzureADOptions::AZURE_AUTH_AD_MSI)) {
activeDirectoryMSI = true;

// 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();
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if the user sets the keyword to something other than ActiveDirectoryMsi? Do we just pass that straight to the ODBC driver?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I understand the question. In terms of keywords check, please check the shared\core_conn.cpp line 733

}
}

// Add the server name
common_conn_str_append_func( ODBCConnOptions::SERVER, server, strnlen_s( server ), connection_string TSRMLS_CC );

// if uid is not present then we use trusted connection -- but not when access token is used, because they are incompatible
if (!access_token_used) {

// If uid is not present then we use trusted connection -- but not when access token or ActiveDirectoryMSI is used,
// because they are incompatible
if (!access_token_used && !activeDirectoryMSI) {
if (uid == NULL || strnlen_s(uid) == 0) {
connection_string += CONNECTION_OPTION_NO_CREDENTIALS; // "Trusted_Connection={Yes};"
}
Expand Down
2 changes: 2 additions & 0 deletions source/shared/core_sqlsrv.h
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,7 @@ const int SQL_SERVER_2008_DEFAULT_DATETIME_SCALE = 7;
namespace AzureADOptions {
const char AZURE_AUTH_SQL_PASSWORD[] = "SqlPassword";
const char AZURE_AUTH_AD_PASSWORD[] = "ActiveDirectoryPassword";
const char AZURE_AUTH_AD_MSI[] = "ActiveDirectoryMsi";
}

// the message returned by ODBC Driver for SQL Server
Expand Down Expand Up @@ -1777,6 +1778,7 @@ 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,

// Driver specific error codes starts from here.
SQLSRV_ERROR_DRIVER_SPECIFIC = 1000,
Expand Down
6 changes: 5 additions & 1 deletion source/sqlsrv/util.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,7 @@ ss_error SS_ERRORS[] = {
},
{
SS_SQLSRV_ERROR_INVALID_AUTHENTICATION_OPTION,
{ IMSSP, (SQLCHAR*)"Invalid option for the Authentication keyword. Only SqlPassword or ActiveDirectoryPassword is supported.", -62, false }
{ IMSSP, (SQLCHAR*)"Invalid option for the Authentication keyword. Only SqlPassword, ActiveDirectoryPassword, or ActiveDirectoryMsi is supported.", -62, false }
},
{
SS_SQLSRV_ERROR_AE_QUERY_SQLTYPE_REQUIRED,
Expand Down Expand Up @@ -436,6 +436,10 @@ 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}
},

// terminate the list of errors/warnings
{ UINT_MAX, {} }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,5 +78,5 @@ if ($azureServer != 'TARGET_AD_SERVER') {
Connected successfully with Authentication=SqlPassword.
string(1) "%d"
Could not connect with Authentication=ActiveDirectoryIntegrated.
SQLSTATE[IMSSP]: Invalid option for the Authentication keyword. Only SqlPassword or ActiveDirectoryPassword is supported.
SQLSTATE[IMSSP]: Invalid option for the Authentication keyword. Only SqlPassword, ActiveDirectoryPassword, or ActiveDirectoryMsi is supported.
%s with Authentication=ActiveDirectoryPassword.
114 changes: 114 additions & 0 deletions test/functional/pdo_sqlsrv/pdo_azure_ad_managed_identity.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
--TEST--
Test some error conditions of Azure AD Managed Identity support
--DESCRIPTION--
This test expects certain exceptions to be thrown under some conditions.
--SKIPIF--
<?php require('skipif.inc');?>
--FILE--
<?php
require_once("MsCommon_mid-refactor.inc");

function verifyErrorMessage($exception, $expectedError, $msg)
{
if (strpos($exception->getMessage(), $expectedError) === false) {
echo "AzureAD Managed Identity test: expected to fail with $msg\n";

print_r($exception->getMessage());
echo "\n";
}
}

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;

try {
$conn = new PDO("sqlsrv:server = $server; driver=$driver;", $uid, $pwd);

$msodbcsqlVer = $conn->getAttribute(PDO::ATTR_CLIENT_VERSION)["DriverVer"];
$version = explode(".", $msodbcsqlVer);

if ($version[0] < 17 || $version[1] < 3) {
//skip the rest of this test, which requires ODBC driver 17.3 or above
return;
}
unset($conn);

// Try connecting to an invalid server, should get an exception from ODBC
$connectionInfo = "Authentication = ActiveDirectoryMsi;";
$testCase = 'invalidServer';
try {
$conn = new PDO("sqlsrv:server = invalidServer; $connectionInfo", null, null);
echo $message . $testCase . PHP_EOL;
} catch(PDOException $e) {
// TODO: check the exception message here
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This part is missing? For the sqlsrv test too.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For this we have to wait for ODBC 17.3 official release

}
} catch(PDOException $e) {
print_r($e->getMessage());
}
}

require_once('MsSetup.inc');

// Test some error conditions
connectWithInvalidOptions();

// Make a connection to an invalid server
connectInvalidServer();

echo "Done\n";
?>
--EXPECT--
Done
4 changes: 2 additions & 2 deletions test/functional/sqlsrv/sqlsrv_azure_ad_authentication.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ Array
[SQLSTATE] => IMSSP
[1] => -62
[code] => -62
[2] => Invalid option for the Authentication keyword. Only SqlPassword or ActiveDirectoryPassword is supported.
[message] => Invalid option for the Authentication keyword. Only SqlPassword or ActiveDirectoryPassword is supported.
[2] => Invalid option for the Authentication keyword. Only SqlPassword, ActiveDirectoryPassword, or ActiveDirectoryMsi is supported.
[message] => Invalid option for the Authentication keyword. Only SqlPassword, ActiveDirectoryPassword, or ActiveDirectoryMsi is supported.
)
%s with Authentication=ActiveDirectoryPassword.
88 changes: 88 additions & 0 deletions test/functional/sqlsrv/sqlsrv_azure_ad_managed_identity.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
--TEST--
Test some error conditions of Azure AD Managed Identity support
--DESCRIPTION--
This test expects certain exceptions to be thrown under some conditions.
--SKIPIF--
<?php require('skipif.inc');?>
--FILE--
<?php
require_once("MsCommon.inc");

function verifyErrorMessage($conn, $expectedError, $msg)
{
if ($conn === false) {
if (strpos(sqlsrv_errors($conn)[0]['message'], $expectedError) === false) {
print_r(sqlsrv_errors());
}
} else {
fatalError("AzureAD Managed Identity test: expected to fail with $msg\n");
}
}

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;

$connectionInfo = array("UID"=>$userName, "PWD"=>$userPassword, "Driver" => $driver);
$conn = sqlsrv_connect($server, $connectionInfo);
if ($conn === false) {
fatalError("Failed to connect in connectInvalidServer.");
}

$msodbcsqlVer = sqlsrv_client_info($conn)['DriverVer'];
$version = explode(".", $msodbcsqlVer);

if ($version[0] < 17 || $version[1] < 3) {
//skip the rest of this test, which requires ODBC driver 17.3 or above
return;
}
sqlsrv_close($conn);

// Try connecting to an invalid server, should get an exception from ODBC
$connectionInfo = array("Authentication"=>"ActiveDirectoryMsi");
$conn = sqlsrv_connect('invalidServer', $connectionInfo);
if ($conn) {
fatalError("AzureAD Managed Identity test: expected to fail with invalidServer\n");
} else {
// TODO: check the exception message here, using verifyErrorMessage()
}
}

// Test some error conditions
connectWithInvalidOptions($server);

// Make a connection to an invalid server
connectInvalidServer();

echo "Done\n";
?>
--EXPECT--
Done