Search code examples
t-sqlhashbytes

how to avoid the "Type conversion in expression" when using HASHBYTES


I have this query:

declare @password nvarchar(254) ='R0nald0$123456.', 
@username nvarchar(50)='Ronaldo',
@playerId uniqueidentifier,
@userType int;

declare @UserUsername_CS int =CHECKSUM(@username);

    SELECT Top 1 @playerId = [UserId]
      FROM [dbo].[UsersPrivate]
     WHERE [UserUsername_CS] =  @UserUsername_CS    
       And [UserUsername]   = @username                
       AND [UserPassword]   = HASHBYTES('SHA2_256', CONCAT([Salt], @password));

select @playerId

It generates the right result; however, I am not sure I am using the "best way" to do this when comparing the user's password with the provided password via the hash.

As [Salt] is part of the table, I can't pre-glue them to get a Binary(32) variable. I have the following index:

CREATE UNIQUE INDEX [IX_UsersPrivate_UserName_UserPassword] ON [dbo].[UsersPrivate]
(
    [UserUsername_CS] ASC,
    [UserUsername] ASC,
    [UserPassword] ASC  
)

, which is ignored by the execution plan.

Type conversion in expression (CONVERT_IMPLICIT(nvarchar(8), [ClubStat].[dbo].[UsersPrivate].[Salt],0)) may affect "CardinalityEstimate" in query plan choice

Here is the execution plan: Past the plan

This is the table definition:

CREATE TABLE [dbo].[UsersPrivate](
    [UserId] [uniqueidentifier] NOT NULL,
    [UserPassword] [binary](32) NOT NULL,
    [UserUsername] [nvarchar](50) NOT NULL,
    [UserUsername_CS]  AS (checksum([UserUsername])) PERSISTED,
    [Salt] [binary](16) NOT NULL,
 CONSTRAINT [PK_UsersPrivate] PRIMARY KEY CLUSTERED 
(
    [UserId] ASC
)

I use this to generate the PW and Salt:

DECLARE @salt BINARY(16) = CAST(CRYPT_GEN_RANDOM(16) AS BINARY(16));

-- Compute the hash
DECLARE @hashedPassword BINARY(32);
SET @hashedPassword = HASHBYTES('SHA2_256', CONCAT(@salt, CAST(@password AS VARBINARY(510))));

Solution

  • CONCAT returns a string, so it's implicitly converting the @salt and @password to varchar. Instead, use + to concat varbinary values.

    SET @hashedPassword = HASHBYTES('SHA2_256', @salt + CAST(@password AS VARBINARY(510)));
    

    Note that in general you should hash the password on the client-side, not the server. The salt is not private and can be passed to the client. A commmon design is to use the username itself as the salt.

    Also, your unique index should be over UserUsername, and Password should be an INCLUDE.

    CREATE UNIQUE INDEX [IX_UsersPrivate_UserName_UserPassword] ON dbo.UsersPrivate
    (UserUsername) INCLUDE (UserPassword, Salt)
    WITH (DROP_EXISTING = ON);
    

    The username hash is unnecessary as I mentioned in comments, because the column it's hashing isn't wide enough to gain anything, you should just index that directly.