[Android](CryptoNative_HmacUpdate): Parameter 'data' must be a valid pointer'

[MaximLipnin]

System.Security.Cryptography.Tests failed on Android due to a crash with the following message:

Abort message: '/__w/1/s/src/libraries/Native/Unix/System.Security.Cryptography.Native.Android/pal_hmac.c:88 (CryptoNative_HmacUpdate): Parameter 'data' must be a valid pointer'

https://helixre8s23ayyeko0k025g8.blob.core.windows.net/dotnet-runtime-refs-pull-61670-merge-3d266bcf337a4ffe92/System.Security.Cryptography.Tests/1/adb-logcat-net.dot.System.Security.Cryptography.Tests-net.dot.MonoRunner.log?sv=2019-07-07&se=2021-12-08T15%3A07%3A31Z&sr=c&sp=rl&sig=McQh8mbdRtBm%2FUFGKVsx7BoxJQ6bKhUkvsFldClN2iI%3D

cc @steveisok @akoeplinger

[ghost]

Tagging subscribers to this area: @bartonjs, @vcsjones, @krwq, @GrabYourPitchforks
See info in area-owners.md if you want to be subscribed.

Issue Details

---

System.Security.Cryptography.Tests failed on Android due to a crash with the following message:

Abort message: '/__w/1/s/src/libraries/Native/Unix/System.Security.Cryptography.Native.Android/pal_hmac.c:88 (CryptoNative_HmacUpdate): Parameter 'data' must be a valid pointer'

https://helixre8s23ayyeko0k025g8.blob.core.windows.net/dotnet-runtime-refs-pull-61670-merge-3d266bcf337a4ffe92/System.Security.Cryptography.Tests/1/adb-logcat-net.dot.System.Security.Cryptography.Tests-net.dot.MonoRunner.log?sv=2019-07-07&se=2021-12-08T15%3A07%3A31Z&sr=c&sp=rl&sig=McQh8mbdRtBm%2FUFGKVsx7BoxJQ6bKhUkvsFldClN2iI%3D

cc @steveisok @akoeplinger

Author:	MaximLipnin
Assignees:	-
Labels:	area-System.Security, untriaged
Milestone:	-

[MaximLipnin]

It might be related to #61742 @AaronRobinsonMSFT

[marek-safar]

I'm curious to know we got into this state

[ghost]

Tagging subscribers to 'arch-android': @steveisok, @akoeplinger
See info in area-owners.md if you want to be subscribed.

Issue Details

---

System.Security.Cryptography.Tests failed on Android due to a crash with the following message:

Abort message: '/__w/1/s/src/libraries/Native/Unix/System.Security.Cryptography.Native.Android/pal_hmac.c:88 (CryptoNative_HmacUpdate): Parameter 'data' must be a valid pointer'

https://helixre8s23ayyeko0k025g8.blob.core.windows.net/dotnet-runtime-refs-pull-61670-merge-3d266bcf337a4ffe92/System.Security.Cryptography.Tests/1/adb-logcat-net.dot.System.Security.Cryptography.Tests-net.dot.MonoRunner.log?sv=2019-07-07&se=2021-12-08T15%3A07%3A31Z&sr=c&sp=rl&sig=McQh8mbdRtBm%2FUFGKVsx7BoxJQ6bKhUkvsFldClN2iI%3D

cc @steveisok @akoeplinger

Author:	MaximLipnin
Assignees:	-
Labels:	area-System.Security, os-android, untriaged
Milestone:	-

[steveisok]

I confirmed it started happening at least from the last rolling build.

[AaronRobinsonMSFT]

I'll take a look.

[AaronRobinsonMSFT]

/cc @jkoritzinsky @elinor-fung

[marek-safar]

@steveisok could we ensure if there is CI hole that it's closed?

[AaronRobinsonMSFT]

I think this is a slight semantic change that is handled properly but validates earlier than needed on Android.

runtime/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_evp.c

Lines 96 to 101 in 6ff57f1

	int32_t CryptoNative_EvpDigestOneShot(const EVP_MD* type, const void* source, int32_t sourceSize, uint8_t* md, uint32_t* mdSize)
	{
	if (type == NULL || sourceSize < 0 || md == NULL || mdSize == NULL)
	return 0;
	EVP_MD_CTX* ctx = CryptoNative_EvpMdCtxCreate(type);

My plan is to move the NULL check after the case where length is 0.

runtime/src/libraries/Native/Unix/System.Security.Cryptography.Native.Android/pal_evp.c

Lines 51 to 56 in 6ff57f1

	int32_t CryptoNative_EvpDigestOneShot(intptr_t type, void* source, int32_t sourceSize, uint8_t* md, uint32_t* mdSize)
	{
	abort_if_invalid_pointer_argument (source);
	if (!type || !md || !mdSize || sourceSize < 0)
	return FAIL;

[elinor-fung]

The signature of EvpDigentOneShot is blittable, so the semantic change should not be in how we are handling arguments for that function though.

runtime/src/libraries/Common/src/Interop/Android/System.Security.Cryptography.Native.Android/Interop.Evp.cs

Lines 34 to 35 in f6e2377

	[GeneratedDllImport(Libraries.AndroidCryptoNative, EntryPoint = "CryptoNative_EvpDigestOneShot")]
	internal static unsafe partial int EvpDigestOneShot(IntPtr type, byte* source, int sourceSize, byte* md, uint* mdSize);

Something must have caused inputs to change so that it is being passed null.

[AaronRobinsonMSFT]

@elinor-fung Agreed. I confirmed we are simply creating a DllImport anyways so I'm unclear how we got here. I am putting up a commit reverting only that change to confirm in the CI.