Clear AKV data after setting the connection attribute or when exception is thrown (#854)

* Dev (#820)

* Fixed the potential error reported by Prefast code analysis

* Use SQLSRV_ASSERT for checking NULL ptrs

* For these AKV tests check env despite not AE connected

* Added the driver option to run functional tests

* Fixed connection pooling tests for more than one ODBC drivers

* added driver option to pdo isPooled.php

* Removed win32 ifdefs re connection resiliency (#802)

* Set the driver argument for getDSN to null by default (#798)

* Added the driver argument to getDSN

* Dropped the driver argument but set to null as default

* Removed the AE condition in locale support

* Modified the AE condition for locale support

* Changed int to SQLLEN to avoid infinite loop (#806)

* Version 5.3.0 (#803)

* Version 5.3.0

* Fixed the wrong replacements

* Added comments block to m4 files

* Use dnl for comments

*  Modified AE fetch phptypes test to insert only one row at a time and loop through php types (#801)

* Modified AE fetch phptypes test to insert only one row at a time and loop through php types

* Fixed formatting

* Streamlined two very similar large column name tests (#807)

* Streamlined two very similar large column name tests

* Changed the EOL

* Updates to change log and readme (#811)

* Updates to change log and readme

* Dropped support for Ubuntu 17

* Modified as per review comments

* Fixed connection resiliency tests for Unix, updated AppVeyor for ODBC 17.2

* Fixed expected output

* Fixed output and skipifs

* Fixed skipifs and output

* Fixed driver name

* Updated installation instructions and sample script (#813)

* Updated instructions and sample test for 5.3.0 RTW

* Fixed sample code to adhere to php coding standard

* Fixed cases and spaces

* Modified NOTE for UB 18.04 based on review comments

* Added 'exit'

* Modified change log and readme based on review to PR 811

* Applied review comments

* build output to debug appveyor failure

* removed debug output

* Streamlined two very similar large column name tests (#815)

* Streamlined two very similar large column name tests

* Added random number of test table names to avoid operand clash issues

* Replaced to with for based on review

* Changelog updated

* changelog updated, test skipif changed to run on unix platforms

* Fixed skipif typo

* Fixed typo in skipif for pdo

* Fixed some output for Travis

* Moved error checking inside pdo connres tests

* Added links back to changelog

* Fixed output for sqlsrv connres tests

* Fixed output

* Fixed output again

* Clear AKV data after connection or when exception is thrown

* Modified tests too to skip some AKV tests without real credentials

* Used assignment operator also free the existing memory
This commit is contained in:
Jenny Tam 2018-09-26 14:51:16 -07:00 committed by GitHub
parent 432901d7a0
commit 32732c885e
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 43 additions and 19 deletions

View file

@ -257,7 +257,9 @@ sqlsrv_conn* core_sqlsrv_connect( _In_ sqlsrv_context& henv_cp, _In_ sqlsrv_cont
throw core::CoreException();
}
load_azure_key_vault( conn );
// After load_azure_key_vault, reset AKV related variables regardless
load_azure_key_vault(conn);
conn->ce_option.akv_reset();
// determine the version of the server we're connected to. The server version is left in the
// connection upon return.
@ -292,6 +294,7 @@ sqlsrv_conn* core_sqlsrv_connect( _In_ sqlsrv_context& henv_cp, _In_ sqlsrv_cont
throw;
}
catch( core::CoreException& ) {
conn->ce_option.akv_reset();
conn_str.clear();
conn->invalidate();
throw;
@ -862,6 +865,7 @@ void build_connection_string_and_set_conn_attr( _Inout_ sqlsrv_conn* conn, _Inou
}
catch( core::CoreException& ) {
conn->ce_option.akv_reset();
throw;
}
}
@ -984,10 +988,10 @@ void load_azure_key_vault(_Inout_ sqlsrv_conn* conn TSRMLS_DC)
throw core::CoreException();
}
char *akv_id = Z_STRVAL_P(conn->ce_option.akv_id);
char *akv_secret = Z_STRVAL_P(conn->ce_option.akv_secret);
unsigned int id_len = static_cast<unsigned int>(Z_STRLEN_P(conn->ce_option.akv_id));
unsigned int key_size = static_cast<unsigned int>(Z_STRLEN_P(conn->ce_option.akv_secret));
char *akv_id = conn->ce_option.akv_id.get();
char *akv_secret = conn->ce_option.akv_secret.get();
unsigned int id_len = strnlen_s(akv_id);
unsigned int key_size = strnlen_s(akv_secret);
configure_azure_key_vault(conn, AKV_CONFIG_FLAGS, conn->ce_option.akv_mode, 0);
configure_azure_key_vault(conn, AKV_CONFIG_PRINCIPALID, akv_id, id_len);
@ -1120,6 +1124,7 @@ void ce_akv_str_set_func::func(_In_ connection_option const* option, _In_ zval*
{
SQLSRV_ASSERT(Z_TYPE_P(value) == IS_STRING, "Azure Key Vault keywords accept only strings.");
const char *value_str = Z_STRVAL_P(value);
size_t value_len = Z_STRLEN_P(value);
CHECK_CUSTOM_ERROR(value_len <= 0, conn, SQLSRV_ERROR_KEYSTORE_INVALID_VALUE) {
@ -1130,7 +1135,6 @@ void ce_akv_str_set_func::func(_In_ connection_option const* option, _In_ zval*
{
case SQLSRV_CONN_OPTION_KEYSTORE_AUTHENTICATION:
{
char *value_str = Z_STRVAL_P(value);
if (!stricmp(value_str, "KeyVaultPassword")) {
conn->ce_option.akv_mode = AKVCFG_AUTHMODE_PASSWORD;
} else if (!stricmp(value_str, "KeyVaultClientSecret")) {
@ -1145,14 +1149,19 @@ void ce_akv_str_set_func::func(_In_ connection_option const* option, _In_ zval*
break;
}
case SQLSRV_CONN_OPTION_KEYSTORE_PRINCIPAL_ID:
{
conn->ce_option.akv_id = value;
conn->ce_option.akv_required = true;
break;
}
case SQLSRV_CONN_OPTION_KEYSTORE_SECRET:
{
conn->ce_option.akv_secret = value;
// Create a new string to save a copy of the zvalue
char *pValue = static_cast<char*>(sqlsrv_malloc(value_len + 1));
memcpy_s(pValue, value_len + 1, value_str, value_len);
pValue[value_len] = '\0'; // this makes sure there will be no trailing garbage
// This will free the existing memory block before assigning the new pointer -- the user might set the value(s) more than once
if (option->conn_option_key == SQLSRV_CONN_OPTION_KEYSTORE_PRINCIPAL_ID) {
conn->ce_option.akv_id = pValue;
} else {
conn->ce_option.akv_secret = pValue;
}
conn->ce_option.akv_required = true;
break;
}

View file

@ -1055,15 +1055,23 @@ struct stmt_option;
// This holds the various details of column encryption.
struct col_encryption_option {
bool enabled; // column encryption enabled, false by default
SQLINTEGER akv_mode;
zval_auto_ptr akv_id;
zval_auto_ptr akv_secret;
bool akv_required;
bool enabled; // column encryption enabled, false by default
SQLINTEGER akv_mode;
sqlsrv_malloc_auto_ptr<char> akv_id;
sqlsrv_malloc_auto_ptr<char> akv_secret;
bool akv_required;
col_encryption_option() : enabled( false ), akv_mode(-1), akv_required( false )
{
}
void akv_reset()
{
akv_id.reset();
akv_secret.reset();
akv_required = false;
akv_mode = -1;
}
};
// *** connection resource structure ***

View file

@ -1,7 +1,7 @@
--TEST--
Test connection keywords for Azure Key Vault for Always Encrypted.
--SKIPIF--
<?php require('skipif_not_akv.inc'); ?>
<?php require('skipif_mid-refactor.inc'); ?>
--FILE--
<?php
require_once('pdo_ae_azure_key_vault_common.php');

View file

@ -4,6 +4,10 @@ if (!extension_loaded("pdo") || !extension_loaded('pdo_sqlsrv')) {
}
require_once("MsSetup.inc");
if ($keystore != 'akv')
die ( 'skip - the test requires valid Azure Key Vault credentials.' );
if ($driver != "ODBC Driver 17 for SQL Server") {
// the testing is not set to use ODBC 17
die("skip - AE feature not supported in the current environment.");

View file

@ -5,6 +5,9 @@ if (! extension_loaded("sqlsrv")) {
}
require_once("MsSetup.inc");
if ($keystore != 'akv')
die ( 'skip - the test requires valid Azure Key Vault credentials.' );
if ($driver != "ODBC Driver 17 for SQL Server") {
// the testing is not set to use ODBC 17
die("skip - AE feature not supported in the current environment.");

View file

@ -1,7 +1,7 @@
--TEST--
Test connection keywords for Azure Key Vault for Always Encrypted.
--SKIPIF--
<?php require('skipif_not_akv.inc'); ?>
<?php require('skipif_versions_old.inc'); ?>
--FILE--
<?php
require_once('sqlsrv_ae_azure_key_vault_common.php');