search

dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.36k stars 4.74k forks source link

[Android][x64] Java errors in System.Security.Cryptography.Tests #62081

Open MaximLipnin opened 2 years ago

MaximLipnin commented 2 years ago

There are java errors caught by the mobile targets rolling build. Some examples:

11-26 08:17:46.066  4907  4928 W System.err: javax.crypto.IllegalBlockSizeException: error:04000072:RSA routines:OPENSSL_internal:DATA_TOO_LARGE_FOR_KEY_SIZE
11-26 08:17:46.066  4907  4928 W System.err:    at com.android.org.conscrypt.NativeCrypto.EVP_PKEY_encrypt(Native Method)
11-26 08:17:46.066  4907  4928 W System.err:    at com.android.org.conscrypt.OpenSSLCipherRSA$OAEP.doCryptoOperation(OpenSSLCipherRSA.java:608)
11-26 08:17:46.066  4907  4928 W System.err:    at com.android.org.conscrypt.OpenSSLCipherRSA.engineDoFinal(OpenSSLCipherRSA.java:314)
11-26 08:17:46.066  4907  4928 W System.err:    at javax.crypto.Cipher.doFinal(Cipher.java:2055)

cc @steveisok @akoeplinger

ghost commented 2 years ago

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

Issue Details
There are java errors caught by the mobile targets rolling build. Some examples: - in System.Security.Cryptography.Dsa.Tests.DSASignVerify_Stream ``` 11-26 08:17:45.342 4907 4928 W System.err: java.security.SignatureException: error decoding signature bytes. 11-26 08:17:45.342 4907 4928 W System.err: at com.android.org.bouncycastle.jcajce.provider.asymmetric.dsa.DSASigner.engineVerify(DSASigner.java:141) 11-26 08:17:45.342 4907 4928 W System.err: at java.security.Signature$Delegate.engineVerify(Signature.java:1430) 11-26 08:17:45.342 4907 4928 W System.err: at java.security.Signature.verify(Signature.java:812) ``` - in System.Security.Cryptography.Rsa.Tests.EncryptDecrypt_Array ``` 11-26 08:17:45.850 4907 4928 W System.err: javax.crypto.BadPaddingException: error:04000085:RSA routines:OPENSSL_internal:OAEP_DECODING_ERROR 11-26 08:17:45.850 4907 4928 W System.err: at com.android.org.conscrypt.NativeCrypto.EVP_PKEY_decrypt(Native Method) 11-26 08:17:45.850 4907 4928 W System.err: at com.android.org.conscrypt.OpenSSLCipherRSA$OAEP.doCryptoOperation(OpenSSLCipherRSA.java:610) 11-26 08:17:45.850 4907 4928 W System.err: at com.android.org.conscrypt.OpenSSLCipherRSA.engineDoFinal(OpenSSLCipherRSA.java:314) 11-26 08:17:45.851 4907 4928 W System.err: at javax.crypto.Cipher.doFinal(Cipher.java:2055) ``` ``` 11-26 08:17:46.066 4907 4928 W System.err: javax.crypto.IllegalBlockSizeException: error:04000072:RSA routines:OPENSSL_internal:DATA_TOO_LARGE_FOR_KEY_SIZE 11-26 08:17:46.066 4907 4928 W System.err: at com.android.org.conscrypt.NativeCrypto.EVP_PKEY_encrypt(Native Method) 11-26 08:17:46.066 4907 4928 W System.err: at com.android.org.conscrypt.OpenSSLCipherRSA$OAEP.doCryptoOperation(OpenSSLCipherRSA.java:608) 11-26 08:17:46.066 4907 4928 W System.err: at com.android.org.conscrypt.OpenSSLCipherRSA.engineDoFinal(OpenSSLCipherRSA.java:314) 11-26 08:17:46.066 4907 4928 W System.err: at javax.crypto.Cipher.doFinal(Cipher.java:2055) ``` - in System.Security.Cryptography.EcDsa.Tests.ECDsaTests_Span ``` 11-26 08:17:54.249 4907 4928 W System.err: java.security.InvalidAlgorithmParameterException: unknown curve name: 1.3.36.3.3.2.8.1.1.1 11-26 08:17:54.249 4907 4928 W System.err: at com.android.org.conscrypt.OpenSSLECKeyPairGenerator.initialize(OpenSSLECKeyPairGenerator.java:115) 11-26 08:17:54.249 4907 4928 W System.err: at java.security.KeyPairGenerator.initialize(KeyPairGenerator.java:438) ``` - https://helixre8s23ayyeko0k025g8.blob.core.windows.net/dotnet-runtime-refs-heads-main-53c95a386c22482b9f/System.Security.Cryptography.Tests/1/adb-logcat-net.dot.System.Security.Cryptography.Tests-net.dot.MonoRunner.log?sv=2019-07-07&se=2021-12-16T08%3A07%3A09Z&sr=c&sp=rl&sig=rThva9mvZMGmfVQSuQwvJU67r3Ps%2BSZCkh%2Fi2m75ZWg%3D - https://helixre8s23ayyeko0k025g8.blob.core.windows.net/dotnet-runtime-refs-heads-main-5b9433c49c364b64bf/System.Security.Cryptography.Tests/1/adb-logcat-net.dot.System.Security.Cryptography.Tests-net.dot.MonoRunner.log?sv=2019-07-07&se=2021-12-15T20%3A11%3A30Z&sr=c&sp=rl&sig=KHTHllXLEJnN0%2FlqN98W0gvNqx5pTTaC1SHUJNQHuKE%3D cc @steveisok @akoeplinger
Author: MaximLipnin
Assignees: -
Labels: `area-System.Security`, `untriaged`
Milestone: -
vcsjones commented 2 years ago

I can take a look at these soon.

Do we know what API Level the unit tests run on? Just asking out of curiosity since the last test failure is a brainpool curve, and I'm guessing Android doesn't support those curves at all or only very recently.

MaximLipnin commented 2 years ago

Do we know what API Level the unit tests run on?

AFAIR currently we use API Level 29.

bartonjs commented 2 years ago

To take one example:

System.Security.Cryptography.Dsa.Tests.DSASignVerify_Stream.Verify2048WithSha1
11-26 08:17:45.318  4907  4928 W System.err: java.security.SignatureException: error decoding signature bytes.
11-26 08:17:45.330  1901  1920 I DropBoxManagerService: add tag=system_server_strictmode isTagEnabled=true flags=0x2
11-26 08:17:45.331  2041  2041 I wpa_supplicant: wlan0: CTRL-EVENT-BEACON-LOSS 
11-26 08:17:45.341  4907  4928 W System.err:    at com.android.org.bouncycastle.jcajce.provider.asymmetric.dsa.DSASigner.engineVerify(DSASigner.java:141)
11-26 08:17:45.341  4907  4928 W System.err:    at java.security.Signature$Delegate.engineVerify(Signature.java:1430)
11-26 08:17:45.341  4907  4928 W System.err:    at java.security.Signature.verify(Signature.java:812)
11-26 08:17:45.342  4907  4928 W System.err: java.security.SignatureException: error decoding signature bytes.
11-26 08:17:45.342  4907  4928 W System.err:    at com.android.org.bouncycastle.jcajce.provider.asymmetric.dsa.DSASigner.engineVerify(DSASigner.java:141)
11-26 08:17:45.342  4907  4928 W System.err:    at java.security.Signature$Delegate.engineVerify(Signature.java:1430)
11-26 08:17:45.342  4907  4928 W System.err:    at java.security.Signature.verify(Signature.java:812)

Those look reasonable for what the test is doing:

https://github.com/dotnet/runtime/blob/39b4f2661f7ea5abbffcb1cb15e78363af9a81c7/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/DSA/DSASignVerify.cs#L370-L392

Perhaps the "problem" here is that the implementation prints the exception to the log before deciding it's ignorable and that the function should just return false?

https://github.com/dotnet/runtime/blob/7414af2a5f6d8d99efc27d3f5ef7a394e0b23c42/src/native/libs/System.Security.Cryptography.Native.Android/pal_signature.c#L66-L70

vcsjones commented 2 years ago

Perhaps the "problem" here is that the implementation prints the exception to the log before deciding it's ignorable and that the function should just return false?

[ConditionalFact(nameof(SupportsFips186_3))]

If I had to guess, Android does not support FIPS 186-3 and SupportsFips186_3 is returning true when it shouldn't be.

@MaximLipnin All of these tests pass for me though after a few dozen runs. How can I determine from the logs which Git commit these failures came from?

MaximLipnin commented 2 years ago

@vcsjones @bartonjs Thanks for taking a look!

The latest rolling build still has those errors in the log. It seems that the closest commit with those failures is https://github.com/dotnet/runtime/commit/752d396519446c13c40738d1c3adfe7fef27a7fa but it doesn't look related. Perhaps, there is also some infra issue with the device where the test app is being run.

vcsjones commented 2 years ago

Okay, I think I completely misunderstood the problem. The tests are passing, we're just printing errors. But we are intentionally hitting an error path. So nothing is broken, we're just printing stuff we probably shouldn't be.

I understand what @bartonjs was saying now šŸ˜„.

MaximLipnin commented 2 years ago

Great! Please teach me now šŸ˜„ Am I right that the printing the exception should be under condition?

MaximLipnin commented 2 years ago

Is it same for unknown curve name error - printing the error/exception doesn't depend on a result of a key creation

https://github.com/dotnet/runtime/blob/main/src/libraries/System.Security.Cryptography/tests/DefaultECDsaProvider.Android.cs#L40-L46

vcsjones commented 2 years ago

Am I right that the printing the exception should be under condition?

To be honest I don't know what the expectations are for the Java runtime printing debug messages.

Using EcKeyCreateByOid as an example, it accepts arbitrary curve name/OIDs as input. If it fails, it returns NULL.

https://github.com/dotnet/runtime/blob/7414af2a5f6d8d99efc27d3f5ef7a394e0b23c42/src/native/libs/System.Security.Cryptography.Native.Android/pal_eckey.c#L111-L117

CheckJNIExceptions calls ExceptionDescribe, so anywhere we get a java exception, it's going to be written to STDERR:

https://github.com/dotnet/runtime/blob/7414af2a5f6d8d99efc27d3f5ef7a394e0b23c42/src/native/libs/System.Security.Cryptography.Native.Android/pal_jni.c#L520-L524

So in this particular case:

  1. We are feeding Android input. We don't know what Android supports which EC curves. The only way we can know for sure is to try. The 1.3.36.3.3.2.8.1.1.1 OID is for brainbool. Our tests detect brainbool support by "Can we successfully generate a key pair with a brain pool OID?"

  2. On Android, the answer is no. So the Java runtime throws an exception and we detect it that with CheckJNIExceptions. Since we get an exception, EcKeyCreateByOid returns NULL.

  3. We check for a NULL result here:

    https://github.com/dotnet/runtime/blob/985f47768dcac01683677ba394100b07b38cd5e3/src/libraries/System.Security.Cryptography/tests/DefaultECDsaProvider.Android.cs#L40-L46

  4. And we can conclude that brainpool is not supported, so IsValueOrFriendlyNameValid returns false.

Everything is working as intended here, other than the fact that CheckJNIExceptions prints to STDERR. If we want to print less exceptions to the log when we "expect those exceptions", that's possible. We would need to write some code to make it easier to detect "What type of jthrowable was thrown?". Have a little working proof-of-concept for that if we want to start going down the path of having less Java exceptions.

steveisok commented 2 years ago

Everything is working as intended here, other than the fact that CheckJNIExceptions prints to STDERR. If we want to print less exceptions to the log when we "expect those exceptions", that's possible. We would need to write some code to make it easier to detect "What type of jthrowable was thrown?". Have a little working proof-of-concept for that if we want to start going down the path of having less Java exceptions.

Correct me if I'm wrong, but it seems there is no other good alternative than detecting "expected" exceptions.

vcsjones commented 2 years ago

Correct me if I'm wrong, but it seems there is no other good alternative than detecting "expected" exceptions.

I don't think so.

Given that, I will start opening a few PRs so that we stop writing to stderr during Java exceptions if it is an exception we actually expect.

ghost commented 2 years ago

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

Issue Details
There are java errors caught by the mobile targets rolling build. Some examples: - in System.Security.Cryptography.Dsa.Tests.DSASignVerify_Stream ``` 11-26 08:17:45.342 4907 4928 W System.err: java.security.SignatureException: error decoding signature bytes. 11-26 08:17:45.342 4907 4928 W System.err: at com.android.org.bouncycastle.jcajce.provider.asymmetric.dsa.DSASigner.engineVerify(DSASigner.java:141) 11-26 08:17:45.342 4907 4928 W System.err: at java.security.Signature$Delegate.engineVerify(Signature.java:1430) 11-26 08:17:45.342 4907 4928 W System.err: at java.security.Signature.verify(Signature.java:812) ``` - in System.Security.Cryptography.Rsa.Tests.EncryptDecrypt_Array ``` 11-26 08:17:45.850 4907 4928 W System.err: javax.crypto.BadPaddingException: error:04000085:RSA routines:OPENSSL_internal:OAEP_DECODING_ERROR 11-26 08:17:45.850 4907 4928 W System.err: at com.android.org.conscrypt.NativeCrypto.EVP_PKEY_decrypt(Native Method) 11-26 08:17:45.850 4907 4928 W System.err: at com.android.org.conscrypt.OpenSSLCipherRSA$OAEP.doCryptoOperation(OpenSSLCipherRSA.java:610) 11-26 08:17:45.850 4907 4928 W System.err: at com.android.org.conscrypt.OpenSSLCipherRSA.engineDoFinal(OpenSSLCipherRSA.java:314) 11-26 08:17:45.851 4907 4928 W System.err: at javax.crypto.Cipher.doFinal(Cipher.java:2055) ``` ``` 11-26 08:17:46.066 4907 4928 W System.err: javax.crypto.IllegalBlockSizeException: error:04000072:RSA routines:OPENSSL_internal:DATA_TOO_LARGE_FOR_KEY_SIZE 11-26 08:17:46.066 4907 4928 W System.err: at com.android.org.conscrypt.NativeCrypto.EVP_PKEY_encrypt(Native Method) 11-26 08:17:46.066 4907 4928 W System.err: at com.android.org.conscrypt.OpenSSLCipherRSA$OAEP.doCryptoOperation(OpenSSLCipherRSA.java:608) 11-26 08:17:46.066 4907 4928 W System.err: at com.android.org.conscrypt.OpenSSLCipherRSA.engineDoFinal(OpenSSLCipherRSA.java:314) 11-26 08:17:46.066 4907 4928 W System.err: at javax.crypto.Cipher.doFinal(Cipher.java:2055) ``` - in System.Security.Cryptography.EcDsa.Tests.ECDsaTests_Span ``` 11-26 08:17:54.249 4907 4928 W System.err: java.security.InvalidAlgorithmParameterException: unknown curve name: 1.3.36.3.3.2.8.1.1.1 11-26 08:17:54.249 4907 4928 W System.err: at com.android.org.conscrypt.OpenSSLECKeyPairGenerator.initialize(OpenSSLECKeyPairGenerator.java:115) 11-26 08:17:54.249 4907 4928 W System.err: at java.security.KeyPairGenerator.initialize(KeyPairGenerator.java:438) ``` - https://helixre8s23ayyeko0k025g8.blob.core.windows.net/dotnet-runtime-refs-heads-main-53c95a386c22482b9f/System.Security.Cryptography.Tests/1/adb-logcat-net.dot.System.Security.Cryptography.Tests-net.dot.MonoRunner.log?sv=2019-07-07&se=2021-12-16T08%3A07%3A09Z&sr=c&sp=rl&sig=rThva9mvZMGmfVQSuQwvJU67r3Ps%2BSZCkh%2Fi2m75ZWg%3D - https://helixre8s23ayyeko0k025g8.blob.core.windows.net/dotnet-runtime-refs-heads-main-5b9433c49c364b64bf/System.Security.Cryptography.Tests/1/adb-logcat-net.dot.System.Security.Cryptography.Tests-net.dot.MonoRunner.log?sv=2019-07-07&se=2021-12-15T20%3A11%3A30Z&sr=c&sp=rl&sig=KHTHllXLEJnN0%2FlqN98W0gvNqx5pTTaC1SHUJNQHuKE%3D cc @steveisok @akoeplinger
Author: MaximLipnin
Assignees: -
Labels: `area-System.Security`, `os-android`
Milestone: -