Implicit conversion from data type error

[G12Consult]

Hi,

I have a SQL database on shared hosting and the following line in a stored procedure is failing and gives the following error

'Implicit conversion from data type nvarchar to binary is not allowed. Use the CONVERT function to run this query.'

It works fine locally on my machine

How do i correct? Basically I am trying to compare the password and salt in a database to authenticate a user

DECLARE @PASSWORD_CHECK BINARY(64)
SET @PASSWORD_CHECK=(SELECT [PASSWORD] FROM [AUTH].[GLOBAL_USERS] WHERE USERNAME=@pUSERNAME AND PASSWORD=HASHBYTES('SHA2_512', @pPASSWORD+CAST(SALT AS NVARCHAR(36))))

falls over on the second line: SET @PASSWORD_CHECK....

 

[bborissov]

What type is password field in GLOBAL_USERS table?

Borislav Borissov
VFP9 SP2, SQL Server

 

[Olaf Doschke]

The question is whther the error is raised by

USERNAME = @USERNAME

or

CAST(SALT as NVARCHAR(3&))

Casting the SALT is more likely to cause the trouble, Assuming SALT is varbinary, how about converting the input parameter @PASSWORD to binary instead? If not, make SALT a varbinary value.

I guess whether conversion from SALT to NVARCHAR works depends on collation setting, which Unicode encoding the server uses. If you tested in SQL Server before 2012, SQL Server used UCS-2, allowing all 2-byte combination 000.ffff, From 2012 on UTF-16 not all byte combinations are allowed so not all binary SALT could be valid Unicode, you couldn't even convert it.

HASHBYTES hashes varbinary input and so use that, it will not depend on any server settings, especially about Unicode and collections. So use

HASHBYTES('SHA2_512', CAST(@pPASSWORD AS VARBINARY(MAX))+SALT

Bye, Olaf.

Olaf Doschke Software Engineering

https://www.doschke.name

 

[G12Consult]

Nvarchar

Should it be something else?

 

[G12Consult]

This is my procedure for adding a new user

ALTER PROCEDURE [AUTHENTICATION].[spADDNEWUSER]
		@pTENANT_ID int
		,@pUSERNAME NVARCHAR(50)
		,@pFIRSTNAME NVARCHAR(50) 
		,@pSURNAME NVARCHAR(50)
		,@pEMAIL_ADDRESS NVARCHAR(50)
		,@pCOUNTRY NVARCHAR(50)
		,@pPASSWORD NVARCHAR(50)
		,@pUSERTYPE int = NULL
		,@RESPONSEMESSAGE NVARCHAR(250) OUTPUT
AS
BEGIN
    SET NOCOUNT ON

    DECLARE @salt UNIQUEIDENTIFIER=NEWID()
    BEGIN TRY

        INSERT INTO AUTHENTICATION.[USERS] (
											TENANT_ID
											, USERNAME
											, FIRSTNAME
											, SURNAME
											, EMAIL_ADDRESS
											, COUNTRY
											, PASSWORD
											, SALT
											, USER_TYPE_ID
											)
        VALUES(
					@pTENANT_ID
					,@pUSERNAME
					,@pFIRSTNAME
					,@pSURNAME
					,@pEMAIL_ADDRESS
					,@pCOUNTRY 
					, HASHBYTES('SHA2_512', @pPASSWORD+CAST(@SALT AS NVARCHAR(36)))
					, @SALT
					, @pUSERTYPE
				)

       SET @RESPONSEMESSAGE='Success'

    END TRY
    BEGIN CATCH
        SET @RESPONSEMESSAGE=ERROR_MESSAGE() 
    END CATCH

END

In my table

Password = nvarchar
Salt = uniqueidentifier

I will try you advice

Thanks

 

[Olaf Doschke]

Password has to be the result of HASHBYTES, which is binary, isn't it? If you define the password field as varchar again you have the problem depending on how SQL Servre uses Unicode, store a binary thing as binary thing, and a hash is, don't be fooled by the human readable display as hexadecimal, same goes for uniqueidentifier, those are by nature binary things.

You can generate a random salt by NewID(), I see that, you can also stay storing it as uiniqueiedentifier, but use it as binary in the HASBYTES passed in argument.

So my recommendation changes a bit, but stays with the same principles, keep binary things in their binary type domain. One thing that becomes important then is how the password comes in from the login. If this has a web frontend it will likely be UTF-8, If this is converted ti ICS-2 or ITF-16 that works as long as it stays with the conversion, I'd convert it to a binary bytearray before even passing it on to SQL Server, so the frontend or business logic already ensures the user input of password does always get to this password check as the same format.

Therefore I'd also change the parameter from @pPASSWORD NVARCHAR(50) to @pPASSWORD VARBINARY(MAX), perhaps limited length, but think of users using password managers, they'd like to be able to pass in longer passwords, too. Maybe limit it to 1000, but not 50. Yes, that's far higher than the entropy of SHA could ever give you, but that's the state of the art nowadays regarding password managers.

Bye, Olaf.

Olaf Doschke Software Engineering

https://www.doschke.name

 

[G12Consult]

I will try to understand your advice and implement.

Why would it work locally? Would this be a security setting on the shared server as to why it would not work.

The original code works fine and authenticates the user successfully locally.

Thanks again

 

[Olaf Doschke]

I guess it could work based on different versions, as I said, and different settings regarding Unicode. But no matter why it worked locally, isn't it preferable to act on binary where it's sensible? And also to care for transporting the same input to the whole password check process and not just passing on whatever encoding the password is to resolve that step as the last one?

So, actually I'd not care why it worked, the way this is defined is instable by depending on some previous step to pass in the same password no matter whether it comes from a UTF-8 Website, a UTF-16 desktop form or whatever that is and may have gone through some implicit conversions before on the transport layer of a web application. Take care of that early on and then you know you better have that binary as soon as possible and so even locally where it already worked I'd change it simply because of that instable outset. Just any change in some collation and also the storage of the password hash as varchar isn't safe to stay as it needs to be.

Bye, Olaf.

Olaf Doschke Software Engineering

https://www.doschke.name

 

[Olaf Doschke]

Just a little addendum:

I tested this a bit and I remembered CAST(newid() as NVarchar(36)) as lowercase not uppercase. I can't confirm this without current access to as many SQL Server Versions as may be necessary and there may be a setting. But I found this:

https://henbo.wordpress.com/2007/12/02/the-mystery-of-upper-case-and-lower-case-guid-values/

Just one more reason to stay with my recommendation of converting the NewID to binary(16) (It's always 128bit or 16 bytes) and define password as varbainry input. Then also store the final hash corresponding to its length. Maybe use Varbinary to be flexible to using other hashes and don't define it exactly as the SHA-512 result length of 512 bits=64 bytes. But newid() will not change from 16 bytes, otherwise the uniqueidentifier data type itself would also need to change.

Jave you managed to apply this? If you have a detail question, just come forward with it.

Bye, Olaf.

Olaf Doschke Software Engineering

https://www.doschke.name

 

[G12Consult]

This is still giving me the same error when I execute the stored procedure.

ALTER PROCEDURE [AUTH].[spADDNEWUSER]
		@pUSER_ID int
		,@pUSERNAME NVARCHAR(50)
		,@pPASSWORD VARBINARY(MAX)
		,@COUNTRY_CODE NCHAR(2)
		,@RESPONSEMESSAGE NVARCHAR(250) OUTPUT
AS
BEGIN
    SET NOCOUNT ON

    DECLARE @SALT VARBINARY(4) = CRYPT_GEN_RANDOM(4)
    BEGIN TRY

        INSERT INTO [AUTH].[GLOBAL_USERS] (
											[USER_ID]
											, USERNAME
											, [PASSWORD]
											, COUNTRY_CODE
											
											)
        VALUES(
					 '1'
					,@pUSERNAME
					, HASHBYTES('SHA2_512', CONVERT(VARBINARY(MAX), @pPASSWORD) + @SALT )
					, 'AU'

					)

       SET @RESPONSEMESSAGE='Success'

    END TRY
    BEGIN CATCH
        SET @RESPONSEMESSAGE=ERROR_MESSAGE() 
    END CATCH

The hashbytes row is throwing the error.

My password field in the table is VARBINARY (MAX)

No idea where I am going wrong. Maybe I am not understanding you correctly?

I am giving the value of @pPassword as 'Password' as a test

 

[fredericofonseca]

SET @PASSWORD_CHECK=(SELECT [PASSWORD]
FROM [AUTH].[GLOBAL_USERS]
WHERE USERNAME=@pUSERNAME
AND PASSWORD=HASHBYTES('SHA2_512', @pPASSWORD+CAST(SALT AS NVARCHAR(36))))

That cast of SALT as nvarchar is most likely what is causing your issue


Regards

Frederico Fonseca
SysSoft Integrated Ltd

http://www.syssoft-int.com

FAQ219-2884
FAQ181-2886

 

[Olaf Doschke]

Because you don't do what I suggested:

@pPASSWORD VARBINARY(MAX)
...
DECLARE @SALT VARBINARY(4) = CRYPT_GEN_RANDOM(4)

That's ok.

HASHBYTES('SHA2_512', CONVERT(VARBINARY(MAX), @pPASSWORD) + @SALT )

@PPassword already is Varbinary(MAX), @SALT also needs no conversion the way you redefined it now. Just 4 bytes is a little short, also you could convert or cast a uniqueidentifier to varbinary, too, but indeed cryptographic random numbers are the best salt, just take a few more bytes. A uniqueidentifier is 16 bytes, so you could take CRYPT_GEN_RANDOM(16). Uniqueidentifier avoids doublettes, but indeed that's not as valuable as a good cryptographic random salt and there's no need to go beyond the entropy of the SHA512, but it would not hurt using CRYPT_GEN_RANDOM(64), so even just the salt itself has the same entropy as the hash.

Good implementations of password hashing will do 1000s of rounds hashing the hated value in a loop, by the way, either with a varying number of rounds or a time complexity, which adapts to the servers capabilities. On a site with thousands of logins per hour, you'd likely not be able to dedicate a second of CPU time to each login, the server has other things to do, but take a look into oWASP or other security related websites and blogs to know more about this.

As I said HASHBYTES can take in a binary message as a parameter and compute the binary hash. Hasbytes does not need a varchar parameter, so simply do this:

HASHBYTES('SHA2_512', @pPASSWORD + @SALT)

Just checking, yes, this works:

DECLARE @pPASSWORD VARBINARY(MAX) = CONVERT(Varbinary(MAX), N'Hello World')
DECLARE @SALT VARBINARY(4) = CRYPT_GEN_RANDOM(4)

SELECT @pPASSWORD, @SALT, HASHBYTES('SHA2_512', @pPASSWORD + @SALT )

On my SQL Server instance, the CONVERT() does also not trigger an error. But it's really not necessary. If this doesn't solve the issue: What is the data type of the [PASSWORD} column in the [AUTH].[spADDNEWUSER]? Is it varbinary?

Bye, Olaf.

Olaf Doschke Software Engineering

https://www.doschke.name

 

[G12Consult]

This is still throwing an error.

Here is my addnewuser code

ALTER PROCEDURE [AUTH].[spADDNEWUSER]
		@pUSER_ID int
		,@pUSERNAME NVARCHAR(50)
		,@pPASSWORD VARBINARY(MAX)
		,@COUNTRY_CODE NCHAR(2)
		,@RESPONSEMESSAGE NVARCHAR(250) OUTPUT
AS
BEGIN
    SET NOCOUNT ON

    DECLARE @SALT VARBINARY(16) = CRYPT_GEN_RANDOM(16)
    BEGIN TRY

        INSERT INTO [AUTH].[GLOBAL_USERS] (
											[USER_ID]
											, USERNAME
											, [PASSWORD]
											, SALT
											, COUNTRY_CODE
											
											)
        VALUES(
					 '1'
					,@pUSERNAME
					, HASHBYTES('SHA2_512', @pPASSWORD + @SALT )
					,@SALT
					, 'AU'

					)

       SET @RESPONSEMESSAGE='Success'

    END TRY
    BEGIN CATCH
        SET @RESPONSEMESSAGE=ERROR_MESSAGE() 
    END CATCH

END

The datatype of password field in the table is varbinary(max)

Here is the code for wen I execute the above code:

DECLARE	@return_value int,
		@RESPONSEMESSAGE nvarchar(250)

EXEC	@return_value = [AUTH].[spADDNEWUSER]
		@pUSER_ID = 1,
		@pUSERNAME = N'ap@xxxx.com.au',
		@pPASSWORD = N'Test123',
		@COUNTRY_CODE = N'AU',
		@RESPONSEMESSAGE = @RESPONSEMESSAGE OUTPUT

SELECT	@RESPONSEMESSAGE as N'@RESPONSEMESSAGE'

SELECT	'Return Value' = @return_value

Error is:

Implicit conversion from data type nvarchar to varbinary(max) is not allowed. Use the CONVERT function to run this query.

I am using this code

HASHBYTES('SHA2_512', @pPASSWORD + @SALT )

to populate the password field. Maybe this is wrong?

Should I have a seperate column for password, salt and hash?

 

[Olaf Doschke]

@pPASSWORD = N'Test123'

Well, you didn't pay attention to the change of the password parameter type. The whole philosophy and idea and reasoning was to get away with the conversion problem to binary inside the [AUTHENTICATION].[spADDNEWUSER] procedure.

You can decide whether that even extends to the EXEC getting this passed in from any outside system. For testing purposes you can't do just what I did:

DECLARE @pPASSWORD VARBINARY(MAX) = [b]CONVERT(Varbinary(MAX), N'Test123')[/b]

Either you read what I already wrote, or here it's once more: Even if you define the password to be varchar throughout your code, the password input from any frontend can change in time and can be any kind of encoding from ASIII, ANSI, Unicode and several variants like UTF-8, UTF-16, UCS-2. Just think by the rule of encapsulation, you don't need to know this when the input is binary and the outside world ensures the user input password is already converted to binary before it enters your routine. The forntend knows best and has to know what encoding the password is and if that changes has to ensure you still get the same input. So on top of encapsulation think of separation of concerns. The encoding of the password should NOT be made your concern, it's a frontend topic only. Frontend and Backend usually should also be separated by a third middle tier. So the encoding of the password as text input should at maximum arrive in the middle tier but surely not be hardwired into the backend layer,

These are OOP programming basics.

Say what you like, say any outside module will also use the same encoding all the time, say divers used to connect will ensure the password text arrives as SQL Server N'Unicodestring' Unicode variant.

Do you remember the initial statement about your problem? That this all worked already but not on this server? And the only pointer still is differences in the Unicode encoding, something related to that. And the solution is to push this problem outside your scope because it is not your scope.

You only make it your problem, if you'd insist to be able to let this parameter be NVARCHAR. You could then say this moves the problem to the developer using your database, providing user and password in the correct encoding. But you see what even already happened while this isn't already involving any outside client, just your testing.

Where does this end? Well, at the frontend layer, obviously, where the knowledge about the encoding is. If a web application developer decides to change a website from UTF-8 to Latin-1 then he has to deal with the problem of users not being able to enter Unicode characters in their passwords anymore. And it is his responsibility, clearly. If he decides to change from UTF-8 to UCS-2 or UTF-16 it again becomes his responsibility the binary password he passes forward to you stays. It's hard enough even for the programmers responsible for their tier to take this into account, but it surely would just lead to pointing fingers, if you make this a thing extending into your tier, the backend.

Bye, Olaf.

Olaf Doschke Software Engineering

https://www.doschke.name

 

[Olaf Doschke]

And once more, separate:

In the end the frontend will need to ensure the password entered as it was the first time arrives at your tier as the same bytes it was then throughout the whole lifetime of passwords. Planning a change of character/text encoding in the front OR middle tier then requires to be able to convert it back to what it was. You have to be strict with the interface between tiers of an application. Poiint this out in documentation. Only demanding the password parameter to be binary ensures you get the same bytes with every login, not just the same text and even less just same looking text. The way login work with hashing means the password already is binary data, even though the input mechanism is keyboard only allowing a subset of all 256 bytes. Password length and other rules ensure its information content is high enough. But the only one who can really ensure the same bytes are forwarded to you is already the frontend tier.

It will seem unusual to frontend designers or application developers, but actually it's the only way it can be applying programming principles strictly. The world is used very much to implicit conversions, ANSI applications using ODBC drivers that convert the ANSI input to MSSQL Unicode, for example.

Bye, Olaf.

Olaf Doschke Software Engineering

https://www.doschke.name