Stop using sp_hexadecimal & sp_help_revlogin

Andy

3 years ago

Caterpillars transform into beautiful butterflies to realize their full self, much in the same way `CONVERT()` changes binary into beautiful hexadecimal strings.

Since I started at Stack Overflow, I’ve had the chance to see some of the interesting challenges with running our databases at our scale.

My coworker Taryn Pratt (blog|twitter) recently blogged about fighting with deadlocks related to Stack Overflow for Teams.

Lots of logins

Taryn mentions in her post that each team gets their own login, user, schema, etc. This means we have a lot of logins. Additionally, we use Availability Groups for our High Availability, which means we need to keep those logins in sync across AG Replicas.

We recently ran into some performance problems with our login sync, which is based on sp_hexadecimal and sp_help_revlogin, the documented & recommended approach by Microsoft.

I’ve been installing & using these two procedures since I started working with SQL Server, back at the turn of the century. In the nearly two decades since, I’ve blindly installed & used these procedures, first on SQL Server 2000, and then on every version since… just because that’s the way I’ve always done it. But our recent performance problems made me rethink that, and dive in to take a look at the two procedures to see if I could do better, which made me realize, OHBOY! WE CAN DO BETTER!!

sp_hexadecimal

I started by looking at the code for sp_hexadecimal, which is a pretty short stored procedure. After just a few minutes, I realized that it was simply converting the binary value to a hexadecimal string (which is how it’s normally displayed to us in SSMS anyway). It’s just doing a data type conversion. It’s just doing a CONVERT().

Since SQL Server 2008, SQL Server has allowed the conversion of varbinary() to varchar() using the CONVERT() function, and specifying a style–this follows the same pattern we are familiar with using to convert between datetime and varchar.

sp_hexadecimal converts the binary value to varchar(514), so let’s try to use CONVERT() to do the same thing.

The docs explain the differences in style, which tells me I want to use style 1. I confirmed this by running a quick query to check the output matched my understanding of the docs:

SELECT Sid_OrigBinary   = sid,
       Sid_Style0       = CONVERT(varchar(514),sid,0),
       Sid_Style1       = CONVERT(varchar(514),sid,1),
       Sid_Style2       = CONVERT(varchar(514),sid,2)
FROM sys.server_principals;

And because I don’t trust myself, I then did a head-to-head matchup to compare the results of CONVERT() and sp_hexadecimal, just in case there was a minor detail I was missing. Sure enough, they produce equivalent results.

DECLARE hex_cur CURSOR LOCAL FAST_FORWARD FOR
    SELECT Sid_OrigBinary   = sid,
           Sid_Style1       = CONVERT(varchar(514),sid,1)
    FROM sys.server_principals;

DECLARE @Sid_OrigBinary varbinary(85);
DECLARE @Sid_Convert_Style1 varchar(514);
DECLARE @Sid_sp_hexadecimal varchar(514);

OPEN hex_cur;
FETCH hex_cur INTO @Sid_OrigBinary, @Sid_Convert_Style1;
 
WHILE (@@FETCH_STATUS = 0)
BEGIN
    EXEC sp_hexadecimal @binvalue = @Sid_OrigBinary, @hexvalue = @Sid_sp_hexadecimal OUTPUT;
    
    IF (@Sid_Convert_Style1 = @Sid_sp_hexadecimal)
    BEGIN
        PRINT CONCAT('MATCHED!!! - ', @Sid_sp_hexadecimal, ' - ', @Sid_Convert_Style1);
    END
    ELSE 
    BEGIN
        PRINT CONCAT('OH NO!!!! NOT MATCHED!!! - ', @Sid_sp_hexadecimal, ' - ', @Sid_Convert_Style1);
    END

    FETCH hex_cur INTO @Sid_OrigBinary, @Sid_Convert_Style1;
END
 
CLOSE hex_cur;
DEALLOCATE hex_cur;

Stop using sp_hexadecimal!

Mind blown. 🤯 It has been 13 years since sp_hexadecimal became obsolete, and it’s still being used heavily & continues to be recommended in the official Microsoft docs. Why continue to use this quasi-official procedure that you need to manually install when you’ve got a built-in function that does the same work for you?

Advantages of sp_hexadecimal:

That’s the way I’ve always done it
sp_help_revlogin uses it, so it’s easier

Advantages of CONVERT():

It’s built-in to the T-SQL language
It’s faster
It can be used in set-based operations without looping

Other than inertia and the dependency from sp_help_revlogin, there’s no upside to sp_hexadecimal. Next step is to see if we need sp_help_revlogin, or if we can get rid of that, too.

sp_help_revlogin

Let’s look at the code for sp_help_revlogin next.

A big chunk of sp_help_revlogin is actually just a cursor definition and the accompanying looping logic, all to accommodate the fact that sp_hexadecimal is a procedure. This is the spot where it actually builds the output inisde that cursor, creating a string as @tmpstr:

DECLARE @tmpstr varchar (1024)
--
SET @tmpstr = 'CREATE LOGIN ' + QUOTENAME( @name ) + ' WITH PASSWORD = ' + @PWD_string + ' HASHED, SID = ' + @SID_string + ', DEFAULT_DATABASE =  [' + @defaultdb + ']'

IF ( @is_policy_checked IS NOT NULL )
BEGIN
SET @tmpstr = @tmpstr + ', CHECK_POLICY = ' + @is_policy_checked
END
IF ( @is_expiration_checked IS NOT NULL )
BEGIN
SET @tmpstr = @tmpstr + ', CHECK_EXPIRATION = ' + @is_expiration_checked
END
END
IF (@denylogin = 1)
BEGIN -- login is denied access
SET @tmpstr = @tmpstr + '; DENY CONNECT SQL TO ' + QUOTENAME( @name )
END
ELSE IF (@hasaccess = 0)
BEGIN -- login exists but does not have access
SET @tmpstr = @tmpstr + '; REVOKE CONNECT SQL TO ' + QUOTENAME( @name )
END
IF (@is_disabled = 1)
BEGIN -- login is disabled
SET @tmpstr = @tmpstr + '; ALTER LOGIN ' + QUOTENAME( @name ) + ' DISABLE'
END
PRINT @tmpstr

When I looked at this with a critical eye, I came across a few problems with the code:

DECLARE @tmpstr varchar (1024) – The string is declared as varchar, not nvarchar. Login names & database names both allow unicode characters. If you have a login or default database that uses a unicode character, the output from sp_help_revlogin will be invalid. This is one of those cases where it might not affect you now, but when the scenario does come up, it will break your code, and you’ll have to troubleshoot. Fix it now, and you don’t have to worry later.
DEFAULT_DATABASE = [' + @defaultdb + ']' – Manually quoting object names like this is an opportunity for SQL Injection vulnerabilities, and also for bugs. There’s probably a low likelihood of hitting problems here since it’s been this way for two decades, but possible. A database name with a ] character will break the script output. Someone could even do something malicious, like creating a database named ];GO; SHUTDOWN WITH NOWAIT;. Instead, the code should use QUOTENAME(). My PR to fix this has been merged, which corrects the “official” version, but you may want to update your installed version, since this will not be pushed out in a CU.
sp_help_revlogin includes permissions – If the login is disabled, or has CONNECT SQL permission denied or revoked, sp_help_revlogin will include that in the same output. I can appreciate from a security perspective that this is a good move to preserve these attributes, but it can get in the way, too. I’ve seen many cases where companies juggle permissions so that a login is disabled on Primary, but enabled on read-only Secondary. Having the ALTER LOGIN...DISABLE or DENY CONNECT SQL in the output makes using sp_help_revlogin to copy logins between servers not possible in this scenario. Additionally, it ignores other server-level permissions like VIEW SERVER STATE, etc. That means you’ll still need to sync server-level permissions separately.
The output is just printed as text – This isn’t something new, but it’s certainly something that prevents using sp_help_revlogin in automation. You’d need to modify it to return a data set, which you could then do something with.

There’s so much going on here that by the time I remove the cursor, replace sp_hexadecimal with CONVERT(), and take care of these other things, I might as well just start from scratch. So I did.

Stop using sp_help_revlogin!

Before you tell me to use PowerShell, let me come out and say that I know I can use PowerShell. There are still plenty of folks who prefer to do these things in T-SQL without the extra context switch, and that’s how we’re going to do it today.

I’ve added two new views to my DBA database, available on GitHub.

In dbo.ServerLogins, I recreate the logic of sp_help_revlogin, and fix my concerns to make it unicode-friendly, set-based, and useful in automation. To get a list of recently-modified logins that can log in (have CONNECT SQL permission & are enabled), along with the necessary CREATE LOGIN statement, I’d simply do something like this. Note that the CREATE and ENABLE statements are in separate columns so they can be handled separately in automation:

SELECT LoginName, DateModified, CreateSql, EnableSql
FROM DBA.dbo.ServerLogins
WHERE CanLogIn = 1
AND DateModified >= DATEADD(DAY, -7, GETUTCDATE());

In the second view, dbo.ServerLoginPermissions, I provide all the GRANT or DENY statements for permissions. To get the ones appropriate for the CONNECT SQL permissions you would get from sp_help_revlogin, you can simply do this:

SELECT LoginName, PermissionType, PermissionSql
FROM DBA.dbo.ServerLoginPermissions
WHERE PermissionType = 'COSQ';

And obviously, I can combine the two views to get everything sp_help_revlogin gave us, in a more usable format that supports unicode and solves my other complaints:

SELECT l.LoginName, l.DateModified, l.CreateSql, l.EnableSql, p.PermissionSql
FROM DBA.dbo.ServerLogins AS l
JOIN DBA.dbo.ServerLoginPermissions AS p 
    ON l.LoginSid = p.LoginSid
    AND p.PermissionType = 'COSQ'
WHERE l.CanLogIn = 1;

And as a bonus, we can get the server-level permissions that sp_help_revlogin missed:

SELECT  l.LoginName, 
        l.DateModified, 
        l.CreateSql, 
        l.EnableSql, 
        PermissionsSql = STRING_AGG(p.PermissionSql, CHAR(10))
FROM DBA.dbo.ServerLogins AS l
JOIN DBA.dbo.ServerLoginPermissions AS p 
    ON l.LoginSid = p.LoginSid
WHERE l.CanLogIn = 1
--AND l.DateModified >= DATEADD(DAY, -7, GETUTCDATE())
GROUP BY l.LoginName, l.DateModified, l.CreateSql, l.EnableSql;

Go grab the code

Head over to my GitHub repo, clone the repo, install my DBA database, and update your automation, scripts, and internal documentation to use this view. Then update your server build scripts/documentation so you can stop using sp_hexadecimal and sp_help_revlogin.