May 23, 2019 at 12:21 am
Hi All
I am requesting a stored procedure to encrypt a field in my database. The code so far looks like this.
USE [MyDB]
GO
/****** Object: StoredProcedure [dbo].[SP_EncryptField] Script Date: 05/23/2019 14:13:22 ******/
SET ANSI_NULLS ON
GO
SET QUOTED_IDENTIFIER ON
GO
CREATE PROCEDURE [dbo].[SP_EncryptField](
@strTableName [nvarchar](255),
@strFieldName [nvarchar](255),
@strStringToEncrypt [nvarchar](255),
@intID [int]
)
WITH EXECUTE AS Owner --Required to give permissions in order to allow the decryption
AS
BEGIN
DECLARE @SQLQuery AS NVARCHAR(500)
OPEN SYMMETRIC KEY MyKEY
DECRYPTION BY CERTIFICATE MyCert
SET @SQLQuery = N'UPDATE dbo.AAA_' + @strTableName + ' SET [' + @strFieldName + '] = EncryptByKey(Key_GUID(''EncryptPIIKEY''), ''' + @strStringToEncrypt + ''') Where [ID] = ' + @intID
EXECUTE @SQLQuery
CLOSE SYMMETRIC KEY MyKEY
END
GO
The issue appears to be with the condition in @sqlquery, which is throwing an error whenever I run it from VBA, e.g. Where [ID] = ' + intID
This is to dynamically provide these details at runtime.
The error in VBA is: Conversion failed when converting the nvarchar value 'Etc, etc' where [ID] = to data type int
I tried: where [ID] = ' + convert(nvarchar(255), @intID) but that didn't work. I tried banging my head against the wall, but that didn't either.
Can anybody offer an easy to understand solution?
Cheers
May 23, 2019 at 9:14 am
The root cause is that you want the table and column names to be dynamic. In a relational database a table is supposed to model a unique entity and a column is supposed to model a unique attribute of that entity. From that perspective, what you are trying to does not make sense, because each column requires its own set of rules. If you want to perform the same operation on a large number of columns in a large number of tables, chances are good that something is wrong in the data model.
As for the procedure, there are a number of problems with it and it starts early. You have EXECUTE AS OWNER, which means that the procedure runs with the power of dbo and can do anything in the database. And it is open to SQL injection so that if plain user that runs this procedure can inject code that causes the procedure to perform something completely different that you intended.
I have material on my web site from which how you can learn to this properly. The article Packaging Permissions in Stored Procedures discusses how to handle the permissions properly and the article The Curse and Blessings of Dynamic SQL gives advice how to work with dynamic SQL.
However, I'm thinking that you may be better of reconsidering the approach altogether. Wouldn't it be better to encrypt the data client-side? If you encrypt in the database, you store the encrypted data with the encryption keys, which is not overly secure. If you are on SQL 2016 or later, you could also consider using Always Encrypted.
[font="Times New Roman"]Erland Sommarskog, SQL Server MVP, www.sommarskog.se[/font]
May 23, 2019 at 3:20 pm
Quick fix:
In the line "SET @sqlquery", try casting your @intID to an NVARCHAR(255).
EDIT - re-read your post and I see you tried this already. Is it giving the same error when you do the convert? What about if you cast instead of convert?
EDIT2 - I just tried this which does basically what you are doing, but doesn't actually execute the dynamic SQL:
DECLARE @strTableName NVARCHAR(255) = 'TEST1',@strFieldName NVARCHAR(255)='TEST2', @strStringToEncrypt NVARCHAR(255)='TEST3', @intID INT=10
DECLARE @SQLQuery AS NVARCHAR(500)
SET @SQLQuery = N'UPDATE dbo.AAA_' + @strTableName + ' SET [' + @strFieldName + '] = EncryptByKey(Key_GUID(''EncryptPIIKEY''), ''' + @strStringToEncrypt + ''') Where [ID] = ' + CONVERT(NVARCHAR(255),@intID)
SELECT @sqlQuery
and it returns exactly what I am expecting.
Why I would spend a bit more time on building up a proper fix:
Now, I do agree 110% about the SQL injection comments that Erland mentioned. For example, lets say somebody figures out the SQL query that is being run when they call the stored procedure. Knowing that, it would be pretty easy to manipulate it to do whatever you wanted. For example, lets assume there is no syntax errors (column names, table names, etc); this could be a value in @strTableName and all the other input variables were "0" (zero):
Table1 SET col1=col1; DROP DATABASE testDB; --
So your dynamic SQL would be:
UPDATE dbo.AAA_Table1 SET col1=col1; DROP DATABASE testDB; -- SET [0] = EncryptByKey(Key_GUID(EncryptPIIKEY),'0') WHERE [ID] = 0
Reading that, it would update all rows in table AAA_Table1 with the same value (so you should have no errors from that), then drop the database. Everything else would be a comment.
I do agree that the encryption COULD be done client side, but since you say "VBA" it makes me think this is something embedded in an office document (Excel?). And in that case, I would prefer to encrypt on the database side. I would do some sort of validation on all input strings though either on the database side OR the application side. The problem with doing the data validation is that you may not think of every possible case where things could go sideways. For example, if you fail the SP on a space being found in any parameter, they just do commands without space or with newlines in there or tabs... there is almost always a way around it when you do dynamic SQL.
Fixing the syntax error is not hard as it is just the way SQL does the data type conversions. If there is an INT in the value, it tries to convert the whole thing to int. For example:
DECLARE @int INT = 10
DECLARE @char CHAR(2) = '10'
SELECT @int, @char
WHERE @int=@char
Will succeed, but it will be implicitly converting @char to the data type int. You can verify this by changing the value of @char to be '1A' and you will get the error about converting the VARCHAR value '1A' to data type int. Now, why SQL thinks it is a VARCHAR when it is clearly a CHAR, I am not certain, but is also outside the scope of your question.
The above is all just my opinion on what you should do.
As with all advice you find on a random internet forum - you shouldn't blindly follow it. Always test on a test server to see if there is negative side effects before making changes to live!
I recommend you NEVER run "random code" you found online on any system you care about UNLESS you understand and can verify the code OR you don't care if the code trashes your system.
May 23, 2019 at 10:45 pm
Thanks you both for your input.
I did a little reading and came up with this modification:
USE [MyDB]
GO
OPEN SYMMETRIC KEY EncryptPIIKEY
DECRYPTION BY CERTIFICATE EncryptPIICert
SET ANSI_NULLS ON
GO
SET QUOTED_IDENTIFIER ON
GO
CREATE PROCEDURE [dbo].[SP_EncryptField](
@strTableName [nvarchar](255),
@strFieldName [nvarchar](255),
@strStringToEncrypt [nvarchar](255),
@intID [int]
)
With EXECUTE AS Owner --Required to give permissions in order to allow the decryption
AS
BEGIN
OPEN SYMMETRIC KEY MyKEY
DECRYPTION BY CERTIFICATE MyCert
DECLARE @SQLQuery AS NVARCHAR(255)
SET @SQLQuery = N'UPDATE [dbo].[AAA_' + @strTableName + '] SET [' + @strFieldName + '] = EncryptByKey(Key_GUID(''MyKEY''), N' + @strStringToEncrypt + ') Where [ID] = ' + cast(@intID as nvarchar(20)) + ';'
EXEC sp_executesql @SQLQuery, N'@strStringToEncrypt varchar(255)', @strStringToEncrypt
END
GO
Firstly, you need to know that I do not care if someone intercepts a single field being encrypted. The data is only valuable as a whole. A single field is meaningless.
Secondly, I do not know what you mean with the Execute as Owner issue. As far as I can tell, the above script works. It sends the data to the field and encrypts it as the same time. So tell me what's wrong with that? I spent 2 days to get it to work and it does.
The premise is that I can run this for any field that I need to send encrypted data to. I don't need to encrypt the entire table, or all fields in the table, just some fields (a varchar field) in some tables where the record ID is supplied.
Happy to hear thoughts
Otherwise, cheers for helps.
May 24, 2019 at 8:17 am
The problem with EXECUTE AS OWNER is that the code runs with full powers of dbo, so if there is an SQL injection hole the possibilities for exploit is bigger. It can also distort auditing.
I will have to say that I get worried by the attitude "what's the problem it works?". This is exactly the sort of attitude that makes it possible to hack sites - people are just happy that it "works" and don't care about security.
How you would go about to perform the encryption from Access, but this is obviously not the forum to ask about that - you will need to find an Access forum or related for that purpose.
Here is an improvement of your dynamic SQL which should be safe for SQL injection. Quotename embeds the value in brackets - and doubles any closing bracket there may be. The string and the ID are sent as parameters, no inlining. And the query string is nvarchar(MAX) to avoid truncation indicents.
DECLARE @SQLQuery AS NVARCHAR(MAX)
SET @SQLQuery = N'UPDATE [dbo].' + quotename('AAA_' + @strTableName) +
' SET ' + quotename(@strFieldName) + ' = EncryptByKey(Key_GUID(''MyKEY''), @strStringToEncrypt)
Where [ID] = @intID ;'
EXEC sp_executesql @SQLQuery, N'@strStringToEncrypt varchar(255), @intID int', @strStringToEncrypt, @intID
[font="Times New Roman"]Erland Sommarskog, SQL Server MVP, www.sommarskog.se[/font]
Viewing 5 posts - 1 through 4 (of 4 total)
You must be logged in to reply to this topic. Login to reply