Logon trigger malfunctions

  • TL:DR: Logon trigger binning all inbound logins, not selective ones.

    Hi

    This is a tricky one. I've written an SP which is called by a logon trigger whenever a particular account, renamed here to AnonLogin in domain DOMAIN, logs in. The idea is to deny the login if certain conditions are met. I need it to deny the login for all non-application logins, i.e. any login not instigated by a human being in front of SSMS.

    So I wrote the following procedure:

    CREATE PROCEDURE sp_killAnonLogin AS

    BEGIN

    DECLARE @session_id int

    DECLARE @cmd VARCHAR(8)

    IF (SELECT COUNT(*)

    FROM sys.dm_exec_sessions

    WHERE program_name LIKE '%Microsoft%SQL%Server%Management%Studio%'

    AND nt_user_name='AnonLogin') = 1

    BEGIN

    SET @session_id=(SELECT session_id FROM sys.dm_exec_sessions WHERE program_name LIKE '%Microsoft%SQL%Server%Management%Studio%' AND nt_user_name='AnonLogin');

    SET @cmd='KILL ' + CAST(@session_id AS VARCHAR(3))

    RAISERROR('Anonymous logon detected and killed. Notification sent to DBA.',16,1) WITH LOG

    EXEC (@cmd)

    END

    IF (SELECT COUNT(*)

    FROM sys.dm_exec_sessions

    WHERE program_name LIKE '%Microsoft%SQL%Server%Management%Studio%'

    AND nt_user_name='AnonLogin') > 1

    BEGIN

    DECLARE iter CURSOR FOR

    SELECT session_id FROM sys.dm_exec_sessions WHERE program_name LIKE '%Microsoft%SQL%Server%Management%Studio%' AND nt_user_name='AnonLogin'

    DECLARE @tempspid SYSNAME

    OPEN iter

    FETCH iter INTO @tempspid

    DECLARE @foocmd NVARCHAR(15)

    WHILE @@FETCH_STATUS = 0

    BEGIN

    SET @foocmd = 'KILL ' + CAST(@tempspid AS NVARCHAR(3))

    exec sp_executesql @foocmd

    RAISERROR('Multiple anonymous logons detected and killed. Notification sent to DBA.',16,1)

    FETCH iter INTO @tempspid

    END

    CLOSE iter

    DEALLOCATE iter

    END

    END

    In English, it checks to see if any logins as reported in sys.dm_exec_sessions have the NT_User_Name = AnonLogin and also whether the program_name is like the string Microsoft SQL Server Management Studio. If so, it's killed. If there's just one, then one branch of the code executes, if more than one then a cursor iterates through each, killing the processes based on the session ID.

    Here's the trigger:

    CREATE TRIGGER killAnonLoginTrigger ON ALL SERVER WITH EXECUTE AS 'DOMAIN\AnonLogin' FOR LOGON AS

    BEGIN

    EXEC sp_killAnonLogin

    END

    Now here's my problem - it works TOO well. It's not just denying login to DOMAIN\AnonLogin, but to any domain account - so it's breaching both conditions. It's ignoring the program_name and nt_user_name conditions! This caused widespread problems for a few minutes (including denying my own login, even in SQLCMD) until I issued DROP TRIGGER killAnonLoginTrigger FOR ALL SERVER and removed the SP. I had to use the DAC via SQLCMD to get access, even!

    I can't test it because I can't simulate the load (around 15-20 logins / minute).

    For those of you thinking 'why didn't he create a login trigger which simply issues ROLLBACK if the conditions are met', i.e. as per BOL - I can't because crucially, I need it to differentiate between application logins and user logins. When I tried using the EVENTDATA() XML schema, I found all results returned were NULL. This might just be because they will always be null when directly queried during testing and only return a result during the event itself but I'm hesitant to deploy this with the same effect.

    Has anyone got any ideas why this behaviour happens? Or a better way of implementing it?

    Thanks

    ---

    Note to developers:
    CAST(SUBSTRING(CAST(FLOOR(NULLIF(ISNULL(COALESCE(1,NULL),NULL),NULL)) AS CHAR(1)),1,1) AS INT) == 1
    So why complicate your code AND MAKE MY JOB HARDER??!:crazy:

    Want to get the best help? Click here https://www.sqlservercentral.com/articles/forum-etiquette-how-to-post-datacode-on-a-forum-to-get-the-best-help (Jeff Moden)
    My blog: http://uksqldba.blogspot.com
    Visit http://www.DerekColley.co.uk to find out more about me.

  • That's slightly strange.

    The below is using the same logic as your code, so can't see that it would work any better.

    DECLARE @sql NVARCHAR(MAX)

    IF (SELECT COUNT(*)

    FROM sys.dm_exec_sessions

    WHERE program_name LIKE '%Microsoft%SQL%Server%Management%Studio%' AND nt_user_name = 'AnonLogin') > 1

    BEGIN

    SELECT @sql = STUFF(REPLACE(CAST((

    SELECT ';' + CHAR(13) + CHAR(10) + 'KILL ' + CAST(session_id AS VARCHAR(3))

    FROM sys.dm_exec_sessions

    WHERE program_name LIKE '%Microsoft%SQL%Server%Management%Studio%' AND nt_user_name='AnonLogin'

    FOR XML PATH(''),TYPE) AS NVARCHAR(MAX)),'&#x0D',CHAR(13) + CHAR(10)),1,5,'')

    PRINT @sql

    --EXECUTE sp_executesql @sql

    RAISERROR('Multiple anonymous logons detected and killed. Notification sent to DBA.',16,1)

    END

    ELSE

    BEGIN

    SELECT @sql = 'KILL ' + CAST(session_id AS VARCHAR(3))

    FROM sys.dm_exec_sessions

    WHERE program_name LIKE '%Microsoft%SQL%Server%Management%Studio%' AND nt_user_name='AnonLogin'

    PRINT @sql

    --EXECUTE sp_executesql @sql

    END

    Can you stick the below in an agent job, have it run every 30 mins and append to a log file so you can review later?

    SELECT REPLICATE('-',80)

    UNION ALL

    SELECT CONVERT(VARCHAR(80),GETDATE())

    UNION ALL

    SELECT REPLICATE('-',80)

    UNION ALL

    SELECT login_name

    FROM sys.dm_exec_sessions

    WHERE program_name LIKE '%Microsoft%SQL%Server%Management%Studio%' AND nt_user_name = 'AnonLogin'

    UNION ALL

    SELECT REPLICATE('-',80)

    UNION ALL

    SELECT CHAR(13)+CHAR(10)


    --Edit

    In my first piece of code, there should be a "& # x 0 D" (no spaces) as part of the REPLACE. The forums have deleted it, but you should be able to see where it goes.


    Forever trying to learn
    My blog - http://www.cadavre.co.uk/
    For better, quicker answers on T-SQL questions, click on the following...http://www.sqlservercentral.com/articles/Best+Practices/61537/
    For better, quicker answers on SQL Server performance related questions, click on the following...http://www.sqlservercentral.com/articles/SQLServerCentral/66909/

  • Cadavre

    Thank you for your detailed reply, I've been over your code and agree that it follows the same logic. I've not been able to test it so far since I need to set up a reasonable testing environment and our local CAB have binned the idea of using production to do it. Given the large impact that a misconfigured trigger can have (and the havoc it can wreak, like today!) this is going to have to be put to one side for now.

    I was thinking perhaps it's down to the quality of the inbound connections? If a connection would come in as e.g. DOMAIN\thisUser but somehow impersonate DOMAIN\AnonLogin would the login trigger activate?

    I've mapped the program logic over to VBScript and can't find a fault, and dry-run it on paper with no ideas.

    Thanks again for your input on this.

    ---

    Note to developers:
    CAST(SUBSTRING(CAST(FLOOR(NULLIF(ISNULL(COALESCE(1,NULL),NULL),NULL)) AS CHAR(1)),1,1) AS INT) == 1
    So why complicate your code AND MAKE MY JOB HARDER??!:crazy:

    Want to get the best help? Click here https://www.sqlservercentral.com/articles/forum-etiquette-how-to-post-datacode-on-a-forum-to-get-the-best-help (Jeff Moden)
    My blog: http://uksqldba.blogspot.com
    Visit http://www.DerekColley.co.uk to find out more about me.

  • You did give SQL this instruction when creating the trigger:

    WITH EXECUTE AS 'DOMAIN\AnonLogin'

    Maybe you can use ORIGINAL_LOGIN( ) to still get the actual original login name.

    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".

  • is that logic right?

    i don't think you need to go to the dmvs, first of all...

    second, this pretty much says "if you find any of these conditions, even if it's not me, rollback the login process.

    IF (SELECT COUNT(*)

    FROM sys.dm_exec_sessions

    WHERE program_name LIKE '%Microsoft%SQL%Server%Management%Studio%'

    AND nt_user_name='AnonLogin') = 1

    BEGIN

    SET @session_id=(SELECT session_id FROM sys.dm_exec_sessions WHERE program_name LIKE '%Microsoft%SQL%Server%Management%Studio%' AND nt_user_name='AnonLogin');

    SET @cmd='KILL ' + CAST(@session_id AS VARCHAR(3))

    RAISERROR('Anonymous logon detected and killed. Notification sent to DBA.',16,1) WITH LOG

    EXEC (@cmd)

    END

    why not check the session that is login in instead?

    IF APP_NAME() LIKE '%Microsoft SQL Server%' AND suser_name() IN ('mydomain\Lowell','AnonLogin')

    ROLLBACK;

    Lowell


    --help us help you! If you post a question, make sure you include a CREATE TABLE... statement and INSERT INTO... statement into that table to give the volunteers here representative data. with your description of the problem, we can provide a tested, verifiable solution to your question! asking the question the right way gets you a tested answer the fastest way possible!

  • The problem looks like it's the Execute As.

    The login trigger proc is running as AnonLogin. Thus ANY login calling that proc qualifies as being a SPID that should be killed. The trigger kills itself, and thus kills the login attempt.

    Change that line, test in dev/QA, see if that solves it.

    - Gus "GSquared", RSVP, OODA, MAP, NMVP, FAQ, SAT, SQL, DNA, RNA, UOI, IOU, AM, PM, AD, BC, BCE, USA, UN, CF, ROFL, LOL, ETC
    Property of The Thread

    "Nobody knows the age of the human race, but everyone agrees it's old enough to know better." - Anon

  • I dug into login triggers a little bit (since I don't write those every day) to confirm a few things.

    What you want to do is get the SPID from the XML in the EventData, and use Original_Login() to get the login name. Query the specific SPID of the connection for the data you need, and kill just that SPID.

    There's no reason to carry on execution of the trigger proc if the Original_Login() function returns something other than the anonymous login you're trying to block. So check that, and Return if it's not what you're looking for.

    - Gus "GSquared", RSVP, OODA, MAP, NMVP, FAQ, SAT, SQL, DNA, RNA, UOI, IOU, AM, PM, AD, BC, BCE, USA, UN, CF, ROFL, LOL, ETC
    Property of The Thread

    "Nobody knows the age of the human race, but everyone agrees it's old enough to know better." - Anon

  • The problem looks like it's the Execute As.

    The login trigger proc is running as AnonLogin. Thus ANY login calling that proc qualifies as being a SPID that should be killed. The trigger kills itself, and thus kills the login attempt.

    I think you might have hit the nail on the head, I've been examining the logic of the procedure when it's fine, but simply being called in the wrong circumstances.

    I did look at EVENTDATA() but test runs, where I used a PRINT or RAISERROR to dump the EVENTDATA fields, returned NULL. Thinking about it though, it would - since these fields will be NULL after the event (login) has occurred, so doing it in a query window will achieve nothing.

    I think I will use EVENTDATA but I won't be able to confirm working or not in this thread as it'll have to go on my pile of things to do. Dev/QA is not fit for purpose at the moment.

    Thanks to everyone for your replies.

    ---

    Note to developers:
    CAST(SUBSTRING(CAST(FLOOR(NULLIF(ISNULL(COALESCE(1,NULL),NULL),NULL)) AS CHAR(1)),1,1) AS INT) == 1
    So why complicate your code AND MAKE MY JOB HARDER??!:crazy:

    Want to get the best help? Click here https://www.sqlservercentral.com/articles/forum-etiquette-how-to-post-datacode-on-a-forum-to-get-the-best-help (Jeff Moden)
    My blog: http://uksqldba.blogspot.com
    Visit http://www.DerekColley.co.uk to find out more about me.

  • derek.colley (2/16/2012)


    The problem looks like it's the Execute As.

    The login trigger proc is running as AnonLogin. Thus ANY login calling that proc qualifies as being a SPID that should be killed. The trigger kills itself, and thus kills the login attempt.

    I think you might have hit the nail on the head, I've been examining the logic of the procedure when it's fine, but simply being called in the wrong circumstances.

    I did look at EVENTDATA() but test runs, where I used a PRINT or RAISERROR to dump the EVENTDATA fields, returned NULL. Thinking about it though, it would - since these fields will be NULL after the event (login) has occurred, so doing it in a query window will achieve nothing.

    I think I will use EVENTDATA but I won't be able to confirm working or not in this thread as it'll have to go on my pile of things to do. Dev/QA is not fit for purpose at the moment.

    Thanks to everyone for your replies.

    The EventData() values aren't available outside the scope of the trigger itself, per documentation. Even a proc called by the trigger isn't in-scope for that data, per MSDN.

    Also, per the documentation, login triggers will return data to the logs, not the client, since a valid connection hasn't been established yet when they are run.

    So I tried this:

    CREATE DATABASE DBA;

    GO

    USE DBA;

    GO

    CREATE TABLE dbo.LogonLog (

    LogData XML);

    GO

    USE master;

    go

    CREATE TRIGGER LoginTest

    ON ALL SERVER

    FOR LOGON

    AS

    SET NOCOUNT ON;

    INSERT INTO DBA.dbo.LogonLog (LogData)

    SELECT EVENTDATA();

    GO

    SELECT *

    FROM DBA.dbo.LogonLog;

    I tested it, logged on a separate connection, and there was plenty of data in the table after the logon attempt.

    It did have a problem writing to the log table when I tried to log on with an account that doesn't have datawriter privileges in the DBA database, so an Execute As statement that has the right privileges will be needed. That was as expected. Alternatively, a database that allows guest/public logins to write to a log table might work. I haven't tried that, but it sounds good in theory.

    - Gus "GSquared", RSVP, OODA, MAP, NMVP, FAQ, SAT, SQL, DNA, RNA, UOI, IOU, AM, PM, AD, BC, BCE, USA, UN, CF, ROFL, LOL, ETC
    Property of The Thread

    "Nobody knows the age of the human race, but everyone agrees it's old enough to know better." - Anon

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

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