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

Named Parameters in Sub Queries #716

Open
theparker opened this issue Mar 13, 2018 · 10 comments
Open

Named Parameters in Sub Queries #716

theparker opened this issue Mar 13, 2018 · 10 comments

Comments

@theparker
Copy link

Driver version or file name

5.2.0.0

SQL Server version

Microsoft SQL Server 2016 Standard (64-bit)
Version 13.0.4001.0

Client operating system

Windows 10

PHP version

PHP 7.1

Microsoft ODBC Driver version

ODBC Driver 17 for SQL Server

Table schema

Users (UserID, Username)
BannedUsers(UserID, BannedType, Date)

Problem description

With ColumnEncryption set to Enabled, I get the error

[Microsoft][ODBC Driver 17 for SQL Server]Syntax error, permission violation, or other nonspecific error

When I run queries with named parameters in sub query. The same code runs perfectly when ColumnEncryption is exempted or set to Disabled.

Expected behavior and actual behavior

This works

select * from Users where Username =:username

but this doesn't

select * from Users where Username=:username and UserID not in (select UserID from BannedUsers where BannedType = :type )

Repro code

$conn = new PDO('sqlsrv:server=srv;database=db;ColumnEncryption=Enabled', 'user', 'password', []);
$tSql = "select * from Users where Username=:username and UserID not in (select UserID from BannedUsers where BannedType = :type )";
$stmt = $conn->prepare($tSql);
$stmt->bindValue(':username', 'u', PDO::PARAM_STR);
$stmt->bindValue(':type', 2, PDO::PARAM_INT);
$stmt->execute();
@yitam
Copy link
Contributor

yitam commented Mar 13, 2018

hi @theparker , first, please make sure you have the right permissions.

Here are the links that might be helpful:

Database permissions
Enabling Always Encrypted in an ODBC application

Hope this helps.

@theparker
Copy link
Author

@yitam, I have granted all four database permissions to the user. Still get error when I execute the statement with ColumnEncryption=Enabled.

GRANT VIEW ANY COLUMN MASTER KEY DEFINITION TO dbuser
GRANT VIEW ANY COLUMN ENCRYPTION KEY DEFINITION TO dbuser
GRANT ALTER ANY COLUMN ENCRYPTION KEY TO dbuser
GRANT ALTER ANY COLUMN MASTER KEY TO dbuser

PS: No column in the 2 tables is encrypted yet as I want to be sure there are no breaking changes (backward compatible) before encrypting columns.

@david-puglielli
Copy link
Contributor

@theparker I have been able to reproduce the behaviour you see - it suffices to have only the second parameter (:type) to trigger the error. We will investigate and keep you updated.

@yitam
Copy link
Contributor

yitam commented Mar 29, 2018

It's a known limitation, @theparker

In a nutshell, in order to support the Always Encrypted feature, PHP drivers must invoke the ODBC API SQLDescribeParam() in order to get the exact metadata concerning various query parameters. Without Always Encrypted, binding parameters isn't as strict, so SQLDescribeParam() is not required.

Please read the doc for some other explanation.

Unfortunately, SQLDescribeParam() does have its limitations. While I agree the error message is misleading, the one you have reported is one of them -- it does not support describing parameters in a subquery.

Alternatively, you might consider using the EXCEPT operator.

For example, this query works:
$tSql = "select UserID from Users where username=:username EXCEPT select UserID from BannedUsers where bannedtype=:type ";

@yitam
Copy link
Contributor

yitam commented Apr 11, 2018

Closing this issue due to inactivity, @theparker . Please feel free to reopen the issue should you have any question.

@yitam yitam closed this as completed Apr 11, 2018
@theparker
Copy link
Author

@yitam, I understand the EXCEPT operator can be used in your example, however, you are restricted by the number of columns you can list using the EXCEPT operator (for instance, you had to modify my select list to get my example query to work). This is doable in a one-off situation or in a small application where rewriting queries without parameterised subqueries would not be much of an issue.

I am working on a huge enterprise application with so many complex queries. Rewriting all the queries to avoid the use of parameterised subqueries is sadly not an option (especially when most of the parameters in the subqueries are in no way related to the encrypted columns).

My question is, now that you have acknowledged this is an issue, do you plan on releasing a fix in a future or ideally next release?

Is it an idea to have a new function for processing query parameters that goes about it the old way i.e. without reference to SQLDescribeParam() and its attending limitations? This way, we can have a function for processing parameters on encrypted columns and one on plain old columns. This could be easier for developers to work around than rewriting queries.

CC: @david-puglielli

@yitam
Copy link
Contributor

yitam commented Apr 13, 2018

I understand your concerns, @theparker , but it's not a bug but rather a design decision to use SQLDescribeParam() behind the scene; the main reason is binding columns with Always Encrypted is a lot stricter with column data types.

That being said, ColumnEncryption is not enabled by default, in which case SQLDescribeParam() will not be used at all. Do the majority of your queries involve subqueries as well as encrypted columns?

@yitam yitam reopened this Apr 13, 2018
@theparker
Copy link
Author

Thanks for the reply @yitam. We use subqueries throughout our application but only few queries include encrypted columns. Barely any of our queries reference encrypted columns in subqueries.

@yitam
Copy link
Contributor

yitam commented Apr 16, 2018

Thanks @theparker , so in this case, would it work for you that you enable ColumnEncryption only when necessary?

@theparker
Copy link
Author

@yitam that sounds like the way to go though there could be issues when running queries in a transaction across multiple connections but I guess we will cross that bridge when we get there.

Thanks for your help once again and I do hope a future release would look at making it easier to go about this without the need to maintain multiple connections.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants