Creating a procedure to dynamically build an update statement

  • Disclaimer, I am new to sql and new to these forums so bear with me on this one. Below I have shown my source code for two tables and a nested cursor that traverses tables and columns within tables specified by a user i.e. whoever is running the procedure. Basically I am trying to allow the user to specify which specific column to run the function dbo.character_scramble(@column_name) on and also to dynamically build an update statement from the col_name/tab_name supplied by the user. I believe that I am headed in the right direction, but I have kind of got myself stuck and rather than beating my head against a wall trying to figure it out I thought I would post it and try to get some help on the issue.

    Ideally I would want the procedure call to go something like this exec phi_scrub(tab_name, col_name), but I'm not really sure how to accomplish this. Notice that in the procedure call I passed the tab_name and the col_name as parameters. I have been trying a number of different things and none of them seem to work. Please I want to reiterate that I am new to SQL and new to these forums so bear with me if my problem seems somewhat elementary or the explanation of it seems incomplete. Thanks!

    if exists (select * from sysobjects where type = 'P' AND

    name = 'phi_scrub')

    drop procedure phi_scrub

    GO

    create procedure phi_scrub(@tbl_id int, @col_name varchar, @tab_name varchar)

    create table encrypt_table

    (

    table_id int identity,

    sch_name varchar(50),

    table_name varchar(50),

    active BIT

    )

    create table encrypt_table_columns

    (

    table_id int,

    column_name varchar(50),

    active BIT

    )

    --truncate table encrypt_table;

    --truncate table encrypt_table_columns;

    declare @id int

    insert into encrypt_table values('dbo','Person')

    set @id = @@identity

    insert into encrypt_table_columns values(@id,'SSN','10000');

    insert into encrypt_table_columns values(@id,'fname','11000');

    insert into encrypt_table_columns values(@id,'lname','12000');

    select et.sch_name, et.table_name, tc.column_name, tc.seed from encrypt_table et,

    encrypt_table_columns tc where et.table_id = tc.table_id;

    drop table encrypt_table;

    drop table encrypt_table_columns;

    GO

    DECLARE @table_id int

    DECLARE @schema_name varchar(128)

    DECLARE @table_name varchar(128)

    DECLARE @STR varchar(4000)

    DECLARE table_cur CURSOR READ_ONLY FOR

    SELECT table_id, sch_name, table_name

    FROM dba.dbo.encrypt_table

    OPEN table_cur

    FETCH NEXT FROM table_cur INTO @table_id, @schema_name, @table_name

    WHILE @@fetch_status = 0

    BEGIN

    --start the update statement

    SET @STR = 'Update ' + @schema_name + '.' + @table_name + ' SET '

    DECLARE @column_name varchar(50)

    DECLARE @ctr int

    DECLARE col_cur CURSOR READ_ONLY FOR

    SELECT column_name

    FROM dba.dbo.encrypt_table_columns

    WHERE table_id = @table_id

    --init ctr

    SET @ctr = 1

    OPEN col_cur

    FETCH NEXT FROM col_cur INTO @column_name

    WHILE @@fetch_status = 0

    BEGIN

    SET @STR = @STR + @column_name + ' = brett_test.dbo.character_scramble(' + @column_name + '), '

    FETCH NEXT FROM col_cur INTO @column_name

    END

    CLOSE col_cur

    DEALLOCATE col_Cur

    --get rid of the last comma to fix syntax issue

    SET @STR = substring(@str,1,Len(@str)-1)

    --include a cr/lf to make print statement readable

    SET @STR = @STR + char(13)

    Print @STR

    --EXEC (@str)

    FETCH NEXT FROM table_cur INTO @table_id, @schema_name, @table_name

    END

    CLOSE table_cur

    DEALLOCATE table_Cur

    select * from person;

  • I personally do not trust users to know what a table or a column is let alone what they are called in the database.

    I am also not a fan of dynamic SQL so my approach would be to create a "calling" procedure that accepts the two parameters ('table_name', column_name') and based on values passed calls another procedure designed specifically to handle the update of that column (yes, I would create a procedure for each column if necessary). also, I would avoid any cursors. They muck with the set-based magic that SQL server does so well.

  • blampe (6/4/2012)


    Disclaimer, I am new to sql and new to these forums so bear with me on this one.

    Not a problem, particularly since you're looking to help yourself and not have us do it for you. You've already gotten past the worst hurdle we usually have to deal with for new people. That said, welcome to the forums. 🙂

    Below I have shown my source code for two tables and a nested cursor that traverses tables and columns within tables specified by a user i.e. whoever is running the procedure.

    I'll be honest, I'm already scared.

    Basically I am trying to allow the user to specify which specific column to run the function dbo.character_scramble(@column_name) on and also to dynamically build an update statement from the col_name/tab_name supplied by the user.

    First, Character_Scramble sounds like an encryptor. I'm assuming you have a reversal of the encryption ready to go as well. There's a few concerns with this. The primary of which is you are reinventing the wheel. The second of this is allowing an end user to dictate data-design.

    Database and column encryption are huge topics, I know, but well worth your time if you're going to need it. By implementing the existing standards and methods not only will you get a skill you can transport to a new location if needed but anyone who needs to support your work will already have familiarity. However, and I'm not sure I can stress this enough: Encrypting at the database level for anything but communications is usually a bad idea. The application tier can usually encrypt at twice the speed or better than the DB Engine can. The App is designed for string manipulation, the database is design for set based data manipulation.

    Now, lets add to this concern, you're allowing users to self-encrypt large portions of the database. That's a nice empowerment but you're going to run into some problems. What happens if someone scrambles the scramble? How many times do you unscramble to 'clear' the data. There's no seeding in your scrambler, and the proc is locally stored. Anyone who gets their hands on the database (which is the assumption that you're protecting against by scrambling) also gets their hands on the descrambler procs. This to me is usually a recipe for disaster. Encryption keys stored off server and proper design with the users sending requests as to what data needs to stay encrypted is your best bet.

    So, with that all said, yes, you're in the right general direction, but you need to protect yourself from SQL Injections as well. Direct interface fields transferred directly into your script are a security hole a mile wide.

    I believe that I am headed in the right direction, but I have kind of got myself stuck and rather than beating my head against a wall trying to figure it out I thought I would post it and try to get some help on the issue.

    Sounds fair to me. 🙂

    Ideally I would want the procedure call to go something like this exec phi_scrub(tab_name, col_name), but I'm not really sure how to accomplish this. Notice that in the procedure call I passed the tab_name and the col_name as parameters. I have been trying a number of different things and none of them seem to work.

    Let's take a look at what you've got so far, maybe I can help with where you're stuck instead of handing you a script from scratch that doesn't help you figure out the jam up in the process.

    if exists (select * from sysobjects where type = 'P' AND

    name = 'phi_scrub')

    drop procedure phi_scrub

    GO

    Just a side note, this tends to be easier, but bother are equally valid:

    IF OBJECT_ID('phi_scrub', 'p') IS NOT NULL

    DROP PROCEDURE phi_scrub

    create procedure phi_scrub(@tbl_id int, @col_name varchar, @tab_name varchar)

    create table encrypt_table

    (

    table_id int identity,

    sch_name varchar(50),

    table_name varchar(50),

    active BIT

    )

    Brakes!

    First problem is that you're creating a permanent table in a procedure. You're not checking for existance and if two people do a concurrent call of the procedure you're going to have all SORTS of errors. However, this is precisely what temporary tables were created for. They're basically tables that only exist for the run of that instance of the procedure. Check out BOL (Books Online, the help file) under CREATE TABLE for temporary tables. They're indicated by using the # sign in front of the name, like so: CREATE TABLE #temp (...definition like any normal table...).

    Temporary tables makes sure that two concurrent calls of the same proc don't step on each other's processing space.

    declare @id int

    insert into encrypt_table values('dbo','Person')

    set @id = @@identity

    Be VERY careful with @@IDENTITY. Look into the function Scope_Identity(). It'll keep you from accidentally picking up triggered Identity values and the like.

    insert into encrypt_table_columns values(@id,'SSN','10000');

    insert into encrypt_table_columns values(@id,'fname','11000');

    insert into encrypt_table_columns values(@id,'lname','12000');

    select et.sch_name, et.table_name, tc.column_name, tc.seed from encrypt_table et,

    encrypt_table_columns tc where et.table_id = tc.table_id;

    drop table encrypt_table;

    drop table encrypt_table_columns;

    GO

    So, if I read this right, you're setting your cursor to look for Table Person and the three columns SSN, fname, and lname. SSN/FName/LName should be passed in as a parameter. Hm, we'll get back to that.

    Assuming the below is a second procedure to be called:

    DECLARE @table_id int

    DECLARE @schema_name varchar(128)

    DECLARE @table_name varchar(128)

    DECLARE @STR varchar(4000)

    DECLARE table_cur CURSOR READ_ONLY FOR

    SELECT table_id, sch_name, table_name

    FROM dba.dbo.encrypt_table

    Just so you know, most people who work in SQL Server will have a painful reaction to seeing the word CURSOR pretty much anywhere. However in this case I believe it's warranted.

    OPEN table_cur

    FETCH NEXT FROM table_cur INTO @table_id, @schema_name, @table_name

    WHILE @@fetch_status = 0

    BEGIN

    --start the update statement

    SET @STR = 'Update ' + @schema_name + '.' + @table_name + ' SET '

    DECLARE @column_name varchar(50)

    DECLARE @ctr int

    DECLARE col_cur CURSOR READ_ONLY FOR

    SELECT column_name

    FROM dba.dbo.encrypt_table_columns

    WHERE table_id = @table_id

    --init ctr

    SET @ctr = 1

    OPEN col_cur

    FETCH NEXT FROM col_cur INTO @column_name

    WHILE @@fetch_status = 0

    BEGIN

    SET @STR = @STR + @column_name + ' = brett_test.dbo.character_scramble(' + @column_name + '), '

    FETCH NEXT FROM col_cur INTO @column_name

    Very nice, instead of hitting each of the tables multiple times you merely affect all the columns in each table once. Smart design.

    END

    CLOSE col_cur

    DEALLOCATE col_Cur

    --get rid of the last comma to fix syntax issue

    SET @STR = substring(@str,1,Len(@str)-1)

    --include a cr/lf to make print statement readable

    SET @STR = @STR + char(13)

    Print @STR

    --EXEC (@str)

    FETCH NEXT FROM table_cur INTO @table_id, @schema_name, @table_name

    END

    CLOSE table_cur

    DEALLOCATE table_Cur

    select * from person;

    You've actually got a relatively sound script here. A few things. sp_executesql parameterization won't really work here because you have to affect the structure of the script itself, so unfortunately you can't protect yourself from injections. This is bad, and should never be allowed near an end user because of that risk. --'; DROP TABLE master.dbo.databases; -- is bad.

    However, it looks like your largest problem is figuring out how to get a multiple entry column list into a parameter. For that, I recommend you use the Delimited Split 8k function, which will allow you to pass a comma delimited list into a proc and it will unpivot it for you into a usable tableset.

    You'll find more on it here: http://www.sqlservercentral.com/articles/Tally+Table/72993/

    So, with all of that said, if you can help us understand the final goal of doing all this, we might be able to help you avoid the hazard you're building yourself into and could perhaps help you find a more effective way of reaching the final desired result.


    - Craig Farrell

    Never stop learning, even if it hurts. Ego bruises are practically mandatory as you learn unless you've never risked enough to make a mistake.

    For better assistance in answering your questions[/url] | Forum Netiquette
    For index/tuning help, follow these directions.[/url] |Tally Tables[/url]

    Twitter: @AnyWayDBA

  • As for defining the purpose of this script a little more clearly, this whole thing is being used in an incredibly simple manner. I know that giving users the power to encrypt data is probably a bit of an overstep, but this script is going to be run by DBA's in my department as a way to encrypt sensitive data for presentation to clients. That is, clients want to see our database setup and we deal with a lot of personal information from people so the character_scramble function is simply a means to mask this data from the eyes of our clients. Thank you very much for the comprehensive answer Craig!!!

  • I suggest looking into SQL's existing EncryptByKey and DecryptByKey functions.

    I disagree with the idea that encryption/decryption can be left to the app. Because you may want to work with data in a SQL statement, and app-based code obviously won't help with that.

    SQL DBA,SQL Server MVP(07, 08, 09) "It's a dog-eat-dog world, and I'm wearing Milk-Bone underwear." "Norm", on "Cheers". Also from "Cheers", from "Carla": "You need to know 3 things about Tortelli men: Tortelli men draw women like flies; Tortelli men treat women like flies; Tortelli men's brains are in their flies".

  • blampe (6/4/2012)


    As for defining the purpose of this script a little more clearly, this whole thing is being used in an incredibly simple manner. I know that giving users the power to encrypt data is probably a bit of an overstep, but this script is going to be run by DBA's in my department as a way to encrypt sensitive data for presentation to clients. That is, clients want to see our database setup and we deal with a lot of personal information from people so the character_scramble function is simply a means to mask this data from the eyes of our clients. Thank you very much for the comprehensive answer Craig!!!

    Ah! Then I agree with Scott, simply use the EncryptByKey function, it'll be faster (http://msdn.microsoft.com/en-us/library/ms174361(v=sql.100).aspx). It's also pretty simple to use if you're not worried about keeping the keys around for later usage and the like, since this is a one-way encryption.

    However, this also sounds like you're creating an overkill solution. If this is meant to be a one-off solution ran by the DBAs, simply hand them a script with the basic build and let them copy/paste and adjust as necessary for each unique database. This both keeps it OUT of the hands of users (who shouldn't be mucking with it) and makes it easier for later maintenance for both yourself and the others. After one or two iterations of making sure you encrypt everything that they might want to show an end user you'll have a script that you can blindly run against database copies that will NEVER be sitting available in your production environment that can hose you up to the need to restore the db.

    @scott: By best practice you never store your unencryption mechanisms and your data in the same place. Admittedly, there are times when you may need to work with the data and that's an exception to best practice, but by having the key nowhere near the storage a hacker now has to break into two locations to get everything he'll need. Encryption and security merely being forcing the hacker to take more time to try to catch them before they get at what they were trying to steal.

    If the database needs to work with the data you're usually better off simply doing TDE to protect the at rest data so you don't have the computational overhead of constant decryption.


    - Craig Farrell

    Never stop learning, even if it hurts. Ego bruises are practically mandatory as you learn unless you've never risked enough to make a mistake.

    For better assistance in answering your questions[/url] | Forum Netiquette
    For index/tuning help, follow these directions.[/url] |Tally Tables[/url]

    Twitter: @AnyWayDBA

  • @Scott: By best practice you never store your unencryption mechanisms and your data in the same place. Admittedly, there are times when you may need to work with the data and that's an exception to best practice, but by having the key nowhere near the storage a hacker now has to break into two locations to get everything he'll need.

    The SQL Server design takes all that into consideration, of course. Required keys are stored (encrypted) in the master db, etc.. So the hacker has to have info from both master and the user db to decrypt the data.

    If the database needs to work with the data you're usually better off simply doing TDE to protect the at rest data so you don't have the computational overhead of constant decryption.

    Not for just a very small number of columns. There is large overhead, and some restrictions, with TDE.

    SQL DBA,SQL Server MVP(07, 08, 09) "It's a dog-eat-dog world, and I'm wearing Milk-Bone underwear." "Norm", on "Cheers". Also from "Cheers", from "Carla": "You need to know 3 things about Tortelli men: Tortelli men draw women like flies; Tortelli men treat women like flies; Tortelli men's brains are in their flies".

  • ScottPletcher (6/4/2012)


    @Scott: By best practice you never store your unencryption mechanisms and your data in the same place. Admittedly, there are times when you may need to work with the data and that's an exception to best practice, but by having the key nowhere near the storage a hacker now has to break into two locations to get everything he'll need.

    The SQL Server design takes all that into consideration, of course. Required keys are stored (encrypted) in the master db, etc.. So the hacker has to have info from both master and the user db to decrypt the data.

    Agreed, but what can I say, I'm paranoid. I figure if they've gotten close enough to one to run decryption procs they're close enough to the rest of it to finish the job. I'm not saying I *never* use local encryption keys, I'm just parroting the best practice that makes the most sense to me unless I have an incredibly important exception not to do it.

    If the database needs to work with the data you're usually better off simply doing TDE to protect the at rest data so you don't have the computational overhead of constant decryption.

    Not for just a very small number of columns. There is large overhead, and some restrictions, with TDE.

    A good point but TDE in my experience when I've used it has not had unreasonable overhead. I've also designed multi-database apps so that the encrypted components are stored separately from the rest so I don't pay the pain in bulk. This is probably a really good discussion to have in another thread though, weighing the value of different encryption forms and where to farm off the pain to, as I fear the OP may end up lost in the shuffle. This tends to end up as a polarizing conversation with a lot of preferences.


    - Craig Farrell

    Never stop learning, even if it hurts. Ego bruises are practically mandatory as you learn unless you've never risked enough to make a mistake.

    For better assistance in answering your questions[/url] | Forum Netiquette
    For index/tuning help, follow these directions.[/url] |Tally Tables[/url]

    Twitter: @AnyWayDBA

Viewing 8 posts - 1 through 7 (of 7 total)

You must be logged in to reply to this topic. Login to reply