I cringe every time I see a recommendation for xp_cmdshell.

  • rmechaber (10/14/2011)


    In fact, when I searched on my machines for code or files containing "xp_cmdshell" I only turned up some demo scripts and this item: "8. Calls to xp_CmdShell will not be used without prior approval of the DBA’s". Source? Jeff Moden's Documentation Standards for MS-SQL.doc.

    Heh... wow! What "great" memories that document brought back. 😀

    That was from one of the companies that I helped do a lockdown on. When I first got to that company, they were experiencing an average of 640 deadlocks per day with spikes to 4000 on a regular basis, several jobs froze the production database for several minutes several times a day, had no documentation in their code, RBAR code called stored procs that were 8 levels deep, all applications were "wide open" with "SA" privs, all developers had "SA" privs in production as did many users, and most batch processing jobs took 8 to 10 hours to execute provided they didn't fail due to a deadlock along the way.

    When I left 4 years later, the 8 to 10 hours jobs were running in less than 3 minutes, the job "freezes" ran transparently in just a second or two, a 24 hour job which usually failed was running in 11 minutes without failure, there were usually less than 5 deadlocks in a week, the legacy code with xp_CmdShell was totally secured so that no one could run xp_CmdShell directly but they could call jobs that did use it, all code was fully documented, and change controls (code promotions to prod) dropped from an agonizing six hours with emergency patches to only 15 minutes with no patches or rollbacks.

    And that was using SQL Server 2000 before using xp_CmdShell could be done securely as easily as it can be done since SS 2005.

    The reason why folks needed to get permission from the DBA's to include calls to xp_CmdShell was because the old-regime was using BCP calls and home-grown code via xp_CmdShell instead of using BULK INSERT and creating file lists via xp_CmdShell instead of using the simplicity of xp_DirTree.

    --Jeff Moden


    RBAR is pronounced "ree-bar" and is a "Modenism" for Row-By-Agonizing-Row.
    First step towards the paradigm shift of writing Set Based code:
    ________Stop thinking about what you want to do to a ROW... think, instead, of what you want to do to a COLUMN.
    "Change is inevitable... change for the better is not".

    Helpful Links:
    How to post code problems
    How to Post Performance Problems
    Create a Tally Function (fnTally)
    Intro to Tally Tables and Functions

  • Thanks for the reply. Let me respond bit by bit:

    Why not spend that time coding a PowerShell script where you can move across the boundaries of the file system domain and the database T-SQL domain quite elegantly. There is so much more available in PowerShell for future needs, and more becoming available with each release of SQL Server. Why box yourself into T-SQL and the SQLCLR for Admin tasks?

    The syntax of PowerShell seems very odd to me, with its one-line-embedded-recursive logic. I've read enough to know how powerful and cool it is, but I haven't the time to learn yet another programming language b/c the old one isn't acceptable anymore.

    Resist the urge to turn your SQL Server into an application server, Windows does that far better than SQL Server. And system admin tasks inside SQL Server...no thank you.

    I think you misunderstood. When I referred to system maintenance, I meant SQL Server maintenance, not Windows server or domain-related maintenance. No arguments there.

    Yes, except that with your use of the SQLCLR you have to set the permission set for the assembly to EXTERNAL_ACCESS or UNSAFE. That is pretty much my guide on this topic. If the assembly cannot be marked SAFE then I need to take a step back and re-evaluate what I am doing. If I find there is an alternative, and am not being compelled to forge ahead by forces beyond my control then I move in a different direction.

    I'm no expert on this, but I haven't seen much written stating EXTERNAL_ACCESS and signed assemblies are unsafe. There's no need to set the assemblies to UNSAFE. Even so, there's a nice summary statement on this issue in SQL Server 2005 Administration by Wood, Leiter, and Turly: "Using the UNSAFE setting to enable access to external resources isn't necessarily a bad thing. This just means that the .NET CLR can't guarantee that it's safe -- you and your developers have to do that, just like many of us have been doing for the past 10 or 12 years."

    If you can point me to examples of security breaches caused by using signed assemblies with EXTERNAL_ACCESS, used to create SQL TVFs with access controlled by normal SQL GRANTs, I'd be interested in reading about them.

    Rich

  • Jeff Moden (10/14/2011)


    @opc.three,

    What's funny about all of this is that xp_DirTree, xp_CmdShell, xp_FileExists, etc, etc, etc all live in a single DLL called XPSTAR.DLL.

    XPSTAR.dll, not sure why that is amusing but it is 🙂

    Why they didn't take it just one step further and make a viable and secure "File Handler" suite is totally beyond me. As you pointed out, at least a "BULK EXPORT" would be handy.

    They did hand us a very powerful dev platform in the SQLCLR. I think they just left us to our own devices on file handling. Elliott took the bull by the horns there. Before I got into PowerShell I was looking into another very similar library, before I knew of yours Elliott. Not sure if you've seen it: http://filesystemhelper.codeplex.com. Needless to say I passed on it and went the PowerShell route for system admin.

    BTW, I did revisit that particular CONNECT item and zinged them a bit.

    Nice, they deserve every bit of it too. There has been a lack of support in that are for far too long. Not to prop up MySQL, because the product is a hot mess, but this is an area (one of extremely few) where MySQL does a better job than SQL Server IMO, especially when it comes to the traditional CSV format. Getting flat files into and out of MySQL from within an SQL context is far simpler than in SQL Server. It is beyond me why MS has not closed that loop.

    There are no special teachers of virtue, because virtue is taught by the whole community.
    --Plato

  • rmechaber (10/14/2011)


    Thanks for the reply. Let me respond bit by bit:

    Why not spend that time coding a PowerShell script where you can move across the boundaries of the file system domain and the database T-SQL domain quite elegantly. There is so much more available in PowerShell for future needs, and more becoming available with each release of SQL Server. Why box yourself into T-SQL and the SQLCLR for Admin tasks?

    The syntax of PowerShell seems very odd to me, with its one-line-embedded-recursive logic.

    You're alluding to "the pipeline". It would take you under an hour to wrap your head around it. Getting good at it, well, that comes with time, as with anything. I'm not great to be sure, but I'm hooked.

    I've read enough to know how powerful and cool it is, but I haven't the time to learn yet another programming language b/c the old one isn't acceptable anymore.

    I'm not sure that's the point. I see it as though the technology is settling. PowerShell is a complete replacement for CmdShell. It's built on top of .NET so you have access to not only the PowerShell scripting langauge and built-in CmdLets but you can instantiate and use any .NET object in the BCL or any custom assembly.

    Resist the urge to turn your SQL Server into an application server, Windows does that far better than SQL Server. And system admin tasks inside SQL Server...no thank you.

    I think you misunderstood. When I referred to system maintenance, I meant SQL Server maintenance, not Windows server or domain-related maintenance. No arguments there.

    The line is blurry...is deleting old backup files SQL Server admin or Windows Admin? hmmm...a PowerShell script that can get data from a SQL Server database and then access the file system to delete old files, or any combination thereof, makes the distinction irrelevant and the security context is consistent throughout so it is easy to configure and manage and straightforward to document.

    Yes, except that with your use of the SQLCLR you have to set the permission set for the assembly to EXTERNAL_ACCESS or UNSAFE. That is pretty much my guide on this topic. If the assembly cannot be marked SAFE then I need to take a step back and re-evaluate what I am doing. If I find there is an alternative, and am not being compelled to forge ahead by forces beyond my control then I move in a different direction.

    I'm no expert on this, but I haven't seen much written stating EXTERNAL_ACCESS and signed assemblies are unsafe.

    It's about reducing risk. Internal hacking is as relevant today as external hacking, if not more so.

    There's no need to set the assemblies to UNSAFE.

    We rarely need UNSAFE. The only time I have run into it as a developer is when making a Windows API call to access Crypto libs from a SQLCLR UDF. That project was a one-off to handle a reporting request...not advised for mainstream use and it never went to a prod environment.

    In a past thread on these forums UNSAFE was needed to load a "relative dll" (a.k.a. C-dll, a lib loaded to memory directly from disk) which also required a Windows API call. That dll was from a third-party IIRC so hands were tied. Elliott, I think you were on that thread too. Turned out to be a 64-bit SQL to 32-bit dll issue so the OP was stuck and could not use SQLCLR, but if they were on 32-bit SQL Server UNSAFE would have been required in their case.

    Even so, there's a nice summary statement on this issue in SQL Server 2005 Administration by Wood, Leiter, and Turly: "Using the UNSAFE setting to enable access to external resources isn't necessarily a bad thing. This just means that the .NET CLR can't guarantee that it's safe -- you and your developers have to do that, just like many of us have been doing for the past 10 or 12 years."

    If you can point me to examples of security breaches caused by using signed assemblies with EXTERNAL_ACCESS, used to create SQL TVFs with access controlled by normal SQL GRANTs, I'd be interested in reading about them.

    I am not saying that SQLCLR cannot be properly secured. Just as I am not saying xp_CmdShell cannot be properly secured. Again, it's about reducing risk and surface area. There are also hoops to jump through with impersonation and possibly having to elevate the rights of the SQL Server service account using SQLCLR with anything but a SAFE permission set. No thanks.

    There are no special teachers of virtue, because virtue is taught by the whole community.
    --Plato

  • From separate post..

    They did hand us a very powerful dev platform in the SQLCLR. I think they just left us to our own devices on file handling. Elliott took the bull by the horns there. Before I got into PowerShell I was looking into another very similar library, before I knew of yours Elliott. Not sure if you've seen it: http://filesystemhelper.codeplex.com. Needless to say I passed on it and went the PowerShell route for system admin.

    I had not seem that library before, it seems to contain everything mine does and a bit more.. I don't have a rename function. But doesn't seem to allow you to get the file image or save one. I had written mine back in late '08 but just recently "pretty'd" it up a bit.

    Yes, except that with your use of the SQLCLR you have to set the permission set for the assembly to EXTERNAL_ACCESS or UNSAFE. That is pretty much my guide on this topic. If the assembly cannot be marked SAFE then I need to take a step back and re-evaluate what I am doing. If I find there is an alternative, and am not being compelled to forge ahead by forces beyond my control then I move in a different direction.

    I'm no expert on this, but I haven't seen much written stating EXTERNAL_ACCESS and signed assemblies are unsafe.

    It's about reducing risk. Internal hacking is as relevant today as external hacking, if not more so.

    There's no need to set the assemblies to UNSAFE.

    We rarely need UNSAFE. The only time I have run into it as a developer is when making a Windows API call to access Crypto libs from a SQLCLR UDF. That project was a one-off to handle a reporting request...not advised for mainstream use and it never went to a prod environment.

    In a past thread on these forums UNSAFE was needed to load a "relative dll" (a.k.a. C-dll, a lib loaded to memory directly from disk) which also required a Windows API call. That dll was from a third-party IIRC so hands were tied. Elliott, I think you were on that thread too. Turned out to be a 64-bit SQL to 32-bit dll issue so the OP was stuck and could not use SQLCLR, but if they were on 32-bit SQL Server UNSAFE would have been required in their case.

    I seem to remember that thread.. I think that most people don't realize how little difference between EXTERNAL_ACCESS and UNSAFE there is. EXTERNAL_ACCESS is the same as UNSAFE except you can't use P/Invoke. Thats really about it. Personally I try NEVER to call outside DLLs, and only really use UNSAFE for when I have to call external EXE files. I believe it is a best practice to separate code into Assmblies based on their access level, meaning safe items in SAFE assemblies, items that require external access but NOT p/invoke in EXTERNAL_ACCESS assemblies, and items that require p/invoke in UNSAFE assemblies. I would be hard pressed to say that SQLCLR is unsafe to use. I think a lot of the resistance is because within the database it is a black box, you CAN'T look inside and that makes people uneasy. This is the reason why I demand that the code be provided to me upfront for review. Could someone slip some extra code in, yes, could I miss something small, sure. But anything short of me compiling it myself risks that.

    CEWII

  • This discussion is pretty good except that I'm not really understanding the actual attack surface issue itself. The closest I could find on the web is that an attacker could use an sql injection attack on a web server to elevate privileges to get to the OS, but in my opinion that just says sql injection attacks are perfectly acceptable which I really disagree with.

    So what am I missing?

  • patrickmcginnis59 (10/24/2011)


    This discussion is pretty good except that I'm not really understanding the actual attack surface issue itself. The closest I could find on the web is that an attacker could use an sql injection attack on a web server to elevate privileges to get to the OS, but in my opinion that just says sql injection attacks are perfectly acceptable which I really disagree with.

    So what am I missing?

    BWAA-HAAA!!!! Very well done Mr. McGinnis! 🙂 YOU have hit the proverbial nail on the head! THAT's part of the whole point that I've been trying to make!

    --Jeff Moden


    RBAR is pronounced "ree-bar" and is a "Modenism" for Row-By-Agonizing-Row.
    First step towards the paradigm shift of writing Set Based code:
    ________Stop thinking about what you want to do to a ROW... think, instead, of what you want to do to a COLUMN.
    "Change is inevitable... change for the better is not".

    Helpful Links:
    How to post code problems
    How to Post Performance Problems
    Create a Tally Function (fnTally)
    Intro to Tally Tables and Functions

  • I guess I see it differently Jeff. I certainly agree SQL injection is not acceptable but I am opposed to having xp_cmdshell enabled without a really good reason. All new development I deal with is not allowed to use xp_cmdshell, period. There are no exceptions in my current environment, as well as I have been pushing the re-writing of all objects that use xp_cmdshell. There are ways to do everything I used to use xp_cmdshell for much more securely. I will happily work with my developers to resolve the issue. I am not only worried about external threats but internal ones as well, where it be buggy code, sloppy code or malicious intent. I'd rather have the door shut on xp_cmdshell.

    CEWII

  • Elliott Whitlow (10/24/2011)


    I guess I see it differently Jeff. I certainly agree SQL injection is not acceptable but I am opposed to having xp_cmdshell enabled without a really good reason. All new development I deal with is not allowed to use xp_cmdshell, period. There are no exceptions in my current environment, as well as I have been pushing the re-writing of all objects that use xp_cmdshell. There are ways to do everything I used to use xp_cmdshell for much more securely. I will happily work with my developers to resolve the issue. I am not only worried about external threats but internal ones as well, where it be buggy code, sloppy code or malicious intent. I'd rather have the door shut on xp_cmdshell.

    CEWII

    Understood and appreciated. However, if your system is properly locked down, then xp_CmdShell is actually less of a threat than the "more secure" methods because no one (save the DBA's themselves) and no system login on the production box needs anything more than PUBLIC privs even if they need to execute procs that use xp_CmdShell and, no, they can't execute xp_CmdShell directly, either. That's what people don't understand in all of this... if you're compelled to use "more secure" methods, then your system simply isn't secure to begin with whether it be from external or internal threats. Period. 😉

    {Edit} Fixed a misspelled word.

    --Jeff Moden


    RBAR is pronounced "ree-bar" and is a "Modenism" for Row-By-Agonizing-Row.
    First step towards the paradigm shift of writing Set Based code:
    ________Stop thinking about what you want to do to a ROW... think, instead, of what you want to do to a COLUMN.
    "Change is inevitable... change for the better is not".

    Helpful Links:
    How to post code problems
    How to Post Performance Problems
    Create a Tally Function (fnTally)
    Intro to Tally Tables and Functions

  • Jeff Moden (10/24/2011)


    Elliott Whitlow (10/24/2011)


    I guess I see it differently Jeff. I certainly agree SQL injection is not acceptable but I am opposed to having xp_cmdshell enabled without a really good reason. All new development I deal with is not allowed to use xp_cmdshell, period. There are no exceptions in my current environment, as well as I have been pushing the re-writing of all objects that use xp_cmdshell. There are ways to do everything I used to use xp_cmdshell for much more securely. I will happily work with my developers to resolve the issue. I am not only worried about external threats but internal ones as well, where it be buggy code, sloppy code or malicious intent. I'd rather have the door shut on xp_cmdshell.

    CEWII

    Understood and appreciated. However, if you're system is properly locked down, then xp_CmdShell is actually less of a threat than the "more secure" methods because no one (save the DBA's themselves) and no system login on the production box needs anything more than PUBLIC privs even if they need to execute procs that use xp_CmdShell and, no, they can't execute xp_CmdShell directly, either. That's what people don't understand in all of this... if you're compelled to use "more secure" methods, then your system simply isn't secure to begin with whether it be from external or internal threats. Period. 😉

    I can go along with your point Jeff.

    CEWII

  • patrickmcginnis59 (10/24/2011)


    This discussion is pretty good except that I'm not really understanding the actual attack surface issue itself. The closest I could find on the web is that an attacker could use an sql injection attack on a web server to elevate privileges to get to the OS, but in my opinion that just says sql injection attacks are perfectly acceptable which I really disagree with.

    So what am I missing?

    Surface area refers to what functionality is exposed to a user, whether that be internal or external, not just known security-related exploits. Elliott alluded to it, but it is internal threats we need to be concerned with as well. Depending on the environment internal threats can be more of a concern or even the exclusive concern. If developers cannot use xp_CmdShell in their solutions because it is disabled or forbidden per their permissions then I have a smaller surface area to protect than if it were available to them.


    Changing gears a bit...

    There is a distinction to consider that is being lost in translation in this discussion. The relevant question that determines how I handle xp_CmdShell goes as follows:

    Will application developers (i.e. not considered a database professional) be empowered to write new T-SQL code that invokes xp_CmdShell? For me the answer is categorically 'no'.

    Since that answer is 'no', we know we have a captive audience. I believe that is the place Jeff is speaking from, a place where qualified database professional provide black-box stored procedures that may make use of xp_CmdShell, but whose internal use of it are hidden from the caller and whose security configuration prevents direct access to xp_CmdShell.

    Maintaining this position, at this point the question becomes Do I or my teammates require xp_CmdShell to perform database maintenance tasks, or does some data consumer have a requirement that compels us to use it in the black-box manner Jeff described? In new systems I am designing the answer is also categorically 'no'. I design away from having to use xp_CmdShell. However, for systems I inherit I look to lock it up in little black-boxes as Jeff has suggested and then follow what Elliott has suggested which is to continually work towards refactoring its use out of the system until I can disable xp_CmdShell.

    There are no special teachers of virtue, because virtue is taught by the whole community.
    --Plato

  • BTW, Orlando... thanks for the link to "How Standards Proliferate". Now THAT's funny because it's oh-so true! 😛

    --Jeff Moden


    RBAR is pronounced "ree-bar" and is a "Modenism" for Row-By-Agonizing-Row.
    First step towards the paradigm shift of writing Set Based code:
    ________Stop thinking about what you want to do to a ROW... think, instead, of what you want to do to a COLUMN.
    "Change is inevitable... change for the better is not".

    Helpful Links:
    How to post code problems
    How to Post Performance Problems
    Create a Tally Function (fnTally)
    Intro to Tally Tables and Functions

  • opc.three (10/24/2011)


    patrickmcginnis59 (10/24/2011)


    This discussion is pretty good except that I'm not really understanding the actual attack surface issue itself. The closest I could find on the web is that an attacker could use an sql injection attack on a web server to elevate privileges to get to the OS, but in my opinion that just says sql injection attacks are perfectly acceptable which I really disagree with.

    So what am I missing?

    Surface area refers to what functionality is exposed to a user, whether that be internal or external, not just known security-related exploits. Elliott alluded to it, but it is internal threats we need to be concerned with as well. Depending on the environment internal threats can be more of a concern or even the exclusive concern. If developers cannot use xp_CmdShell in their solutions because it is disabled or forbidden per their permissions then I have a smaller surface area to protect than if it were available to them.


    I think that giving xp_cmdshell out to everyone is a bad idea, but I also think its possible to use safely, or otherwise Microsoft wouldn't provide the functionality.

  • patrickmcginnis59 (10/24/2011)


    I think that giving xp_cmdshell out to everyone is a bad idea, but I also think its possible to use safely, or otherwise Microsoft wouldn't provide the functionality.

    Agreed and to that very point, I think giving anyone except the DBA's (and you can't actually stop them if they have SA privs) privs to run xp_CmdShell directly is a huge "Bozo-no-no." In fact, I feel the same way about giving anyone even something as low as "data reader". As of 2005, it's just too simple to have everyone do everything through a proc. No one (again, except DBA's) needs privs to do anything except to execute the procs they've been allowed to execute in a properly locked down system.

    --Jeff Moden


    RBAR is pronounced "ree-bar" and is a "Modenism" for Row-By-Agonizing-Row.
    First step towards the paradigm shift of writing Set Based code:
    ________Stop thinking about what you want to do to a ROW... think, instead, of what you want to do to a COLUMN.
    "Change is inevitable... change for the better is not".

    Helpful Links:
    How to post code problems
    How to Post Performance Problems
    Create a Tally Function (fnTally)
    Intro to Tally Tables and Functions

  • Jeff Moden (10/24/2011)


    BTW, Orlando... thanks for the link to "How Standards Proliferate". Now THAT's funny because it's oh-so true! 😛

    Haha, happy you liked it! It hit me dead center when I read it so I had to share it. I still look at it often. It helps humble me a bit when my soapbox starts getting too much use 😉

    There are no special teachers of virtue, because virtue is taught by the whole community.
    --Plato

Viewing 15 posts - 46 through 60 (of 76 total)

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