diff --git a/src/libraries/Common/src/Interop/Android/System.Security.Cryptography.Native.Android/Interop.X509Chain.cs b/src/libraries/Common/src/Interop/Android/System.Security.Cryptography.Native.Android/Interop.X509Chain.cs index 70b3f60bf3512e..2590f256925e2a 100644 --- a/src/libraries/Common/src/Interop/Android/System.Security.Cryptography.Native.Android/Interop.X509Chain.cs +++ b/src/libraries/Common/src/Interop/Android/System.Security.Cryptography.Native.Android/Interop.X509Chain.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Diagnostics; using System.Runtime.InteropServices; using System.Security.Cryptography; using System.Security.Cryptography.X509Certificates; @@ -41,16 +42,23 @@ internal static X509Certificate2[] X509ChainGetCertificates(SafeX509ChainContext var certPtrs = new IntPtr[count]; int res = Interop.AndroidCrypto.X509ChainGetCertificates(ctx, certPtrs, certPtrs.Length); - if (res != SUCCESS) + if (res == 0) throw new CryptographicException(); + Debug.Assert(res <= certPtrs.Length); + var certs = new X509Certificate2[certPtrs.Length]; - for (int i = 0; i < certs.Length; i++) + for (int i = 0; i < res; i++) { certs[i] = new X509Certificate2(certPtrs[i]); } - return certs; + if (res == certPtrs.Length) + { + return certs; + } + + return certs[0..res]; } [StructLayout(LayoutKind.Sequential)] @@ -90,6 +98,10 @@ internal static extern int X509ChainSetCustomTrustStore( IntPtr[] customTrustStore, int customTrustStoreLen); + [DllImport(Libraries.CryptoNative, EntryPoint = "AndroidCryptoNative_X509ChainSupportsRevocationOptions")] + [return:MarshalAs(UnmanagedType.U1)] + internal static extern bool X509ChainSupportsRevocationOptions(); + [DllImport(Libraries.CryptoNative, EntryPoint = "AndroidCryptoNative_X509ChainValidate")] internal static extern int X509ChainValidate( SafeX509ChainContextHandle ctx, diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native.Android/pal_ecc_import_export.c b/src/libraries/Native/Unix/System.Security.Cryptography.Native.Android/pal_ecc_import_export.c index 7018137fc70d99..b448e6155c29e5 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native.Android/pal_ecc_import_export.c +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native.Android/pal_ecc_import_export.c @@ -315,7 +315,7 @@ static jobject AndroidCryptoNative_CreateKeyPairFromCurveParameters( goto cleanup; error: - if (loc[privateKey]) + if (loc[privateKey] && (*env)->IsInstanceOf(env, loc[privateKey], g_DestroyableClass)) { // Destroy the private key data. (*env)->CallVoidMethod(env, loc[privateKey], g_destroy); diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native.Android/pal_eckey.c b/src/libraries/Native/Unix/System.Security.Cryptography.Native.Android/pal_eckey.c index 73fb254839f2f3..f14bfc9f640cfc 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native.Android/pal_eckey.c +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native.Android/pal_eckey.c @@ -45,7 +45,7 @@ void AndroidCryptoNative_EcKeyDestroy(EC_KEY* r) { // Destroy the private key data. jobject privateKey = (*env)->CallObjectMethod(env, r->keyPair, g_keyPairGetPrivateMethod); - if (privateKey) + if (privateKey && (*env)->IsInstanceOf(env, privateKey, g_DestroyableClass)) { (*env)->CallVoidMethod(env, privateKey, g_destroy); ReleaseLRef(env, privateKey); diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native.Android/pal_jni.c b/src/libraries/Native/Unix/System.Security.Cryptography.Native.Android/pal_jni.c index f5f6819e9afbfd..d166c23dd86456 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native.Android/pal_jni.c +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native.Android/pal_jni.c @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. #include "pal_jni.h" +#include JavaVM* gJvm; @@ -476,6 +477,8 @@ static jclass GetOptionalClassGRef(JNIEnv *env, const char* name) if (!TryGetClassGRef(env, name, &klass)) { LOG_DEBUG("optional class %s was not found", name); + // Failing to find an optional class causes an exception state, which we need to clear. + TryClearJNIExceptions(env); } return klass; @@ -551,6 +554,8 @@ jmethodID GetOptionalMethod(JNIEnv *env, bool isStatic, jclass klass, const char jmethodID mid = isStatic ? (*env)->GetStaticMethodID(env, klass, name, sig) : (*env)->GetMethodID(env, klass, name, sig); if (!mid) { LOG_INFO("optional method %s %s was not found", name, sig); + // Failing to find an optional method causes an exception state, which we need to clear. + TryClearJNIExceptions(env); } return mid; } @@ -566,6 +571,22 @@ jfieldID GetField(JNIEnv *env, bool isStatic, jclass klass, const char* name, co return fid; } +static void DetatchThreadFromJNI(void* unused) +{ + LOG_DEBUG("Detaching thread from JNI"); + (void)unused; + (*gJvm)->DetachCurrentThread(gJvm); +} + +static pthread_key_t threadLocalEnvKey; +static pthread_once_t threadLocalEnvInitKey = PTHREAD_ONCE_INIT; + +static void +make_key() +{ + (void) pthread_key_create(&threadLocalEnvKey, &DetatchThreadFromJNI); +} + JNIEnv* GetJNIEnv() { JNIEnv *env; @@ -573,6 +594,11 @@ JNIEnv* GetJNIEnv() if (env) return env; jint ret = (*gJvm)->AttachCurrentThreadAsDaemon(gJvm, &env, NULL); + + (void) pthread_once(&threadLocalEnvInitKey, make_key); + LOG_DEBUG("Registering JNI thread detach. env ptr %p. Key: %ld", (void*)env, (long)threadLocalEnvKey); + pthread_setspecific(threadLocalEnvKey, env); + assert(ret == JNI_OK && "Unable to attach thread to JVM"); (void)ret; return env; diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native.Android/pal_x509.c b/src/libraries/Native/Unix/System.Security.Cryptography.Native.Android/pal_x509.c index 9cd92dadecdab2..dfa72406a6c5ed 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native.Android/pal_x509.c +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native.Android/pal_x509.c @@ -250,6 +250,10 @@ void* AndroidCryptoNative_X509PublicKey(jobject /*X509Certificate*/ cert, PAL_Ke void* keyHandle; jobject key = (*env)->CallObjectMethod(env, cert, g_X509CertGetPublicKey); + if (CheckJNIExceptions(env) || !key) + { + return NULL; + } switch (algorithm) { case PAL_EC: diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native.Android/pal_x509chain.c b/src/libraries/Native/Unix/System.Security.Cryptography.Native.Android/pal_x509chain.c index f57753fa799a88..adc736b0af11f6 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native.Android/pal_x509chain.c +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native.Android/pal_x509chain.c @@ -192,9 +192,16 @@ int32_t AndroidCryptoNative_X509ChainGetCertificates(X509ChainContext* ctx, // Certificate trustedCert = trustAnchor.getTrustedCert(); // certs[i] = trustedCert; jobject trustedCert = (*env)->CallObjectMethod(env, ctx->trustAnchor, g_TrustAnchorGetTrustedCert); - certs[i] = ToGRef(env, trustedCert); - - ret = SUCCESS; + if (i == 0 || !(*env)->IsSameObject(env, certs[i-1], trustedCert)) + { + certs[i] = ToGRef(env, trustedCert); + ret = i + 1; + } + else + { + ret = i; + certs[i] = NULL; + } cleanup: (*env)->DeleteLocalRef(env, certPathList); @@ -404,7 +411,7 @@ int32_t AndroidCryptoNative_X509ChainSetCustomTrustStore(X509ChainContext* ctx, return CheckJNIExceptions(env) ? FAIL : SUCCESS; } -static bool X509ChainSupportsRevocationOptions(void) +bool AndroidCryptoNative_X509ChainSupportsRevocationOptions(void) { return g_CertPathValidatorGetRevocationChecker != NULL && g_PKIXRevocationCheckerClass != NULL; } @@ -483,7 +490,7 @@ static int32_t ValidateWithRevocation(JNIEnv* env, else { certPathToUse = ctx->certPath; - if (X509ChainSupportsRevocationOptions()) + if (AndroidCryptoNative_X509ChainSupportsRevocationOptions()) { // Only add the ONLY_END_ENTITY if we are not just checking the trust anchor. If ONLY_END_ENTITY is // specified, revocation checking will skip the trust anchor even if it is the only certificate. @@ -512,7 +519,7 @@ static int32_t ValidateWithRevocation(JNIEnv* env, } jobject params = ctx->params; - if (X509ChainSupportsRevocationOptions()) + if (AndroidCryptoNative_X509ChainSupportsRevocationOptions()) { // PKIXRevocationChecker checker = validator.getRevocationChecker(); loc[checker] = (*env)->CallObjectMethod(env, validator, g_CertPathValidatorGetRevocationChecker); diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native.Android/pal_x509chain.h b/src/libraries/Native/Unix/System.Security.Cryptography.Native.Android/pal_x509chain.h index 3e2dce658711c5..4c92fa74c56594 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native.Android/pal_x509chain.h +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native.Android/pal_x509chain.h @@ -33,9 +33,9 @@ Return the number of certificates in the path PALEXPORT int32_t AndroidCryptoNative_X509ChainGetCertificateCount(X509ChainContext* ctx); /* -Get the certificates in the path +Get the certificates in the path. -Returns 1 on success, 0 otherwise +Returns the number of certificates exported. */ PALEXPORT int32_t AndroidCryptoNative_X509ChainGetCertificates(X509ChainContext* ctx, jobject* /*X509Certificate[]*/ certs, @@ -62,6 +62,11 @@ PALEXPORT int32_t AndroidCryptoNative_X509ChainSetCustomTrustStore(X509ChainCont jobject* /*X509Certificate[]*/ customTrustStore, int32_t customTrustStoreLen); +/* +Returns true if revocation checking is supported. Returns false otherwise. +*/ +PALEXPORT bool AndroidCryptoNative_X509ChainSupportsRevocationOptions(void); + // Matches managed X509RevocationMode enum enum { diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native.Android/pal_x509store.c b/src/libraries/Native/Unix/System.Security.Cryptography.Native.Android/pal_x509store.c index 5fec76e64f319b..64b839d2018d3b 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native.Android/pal_x509store.c +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native.Android/pal_x509store.c @@ -98,6 +98,7 @@ int32_t AndroidCryptoNative_X509StoreAddCertificate(jobject /*KeyStore*/ store, EntryFlags flags; if (ContainsEntryForAlias(env, store, cert, alias, &flags)) { + ReleaseLRef(env, alias); EntryFlags matchesFlags = EntryFlags_HasCertificate & EntryFlags_MatchesCertificate; if ((flags & matchesFlags) != matchesFlags) { @@ -141,12 +142,14 @@ int32_t AndroidCryptoNative_X509StoreAddCertificateWithPrivateKey(jobject /*KeyS EntryFlags matchesFlags = EntryFlags_HasCertificate & EntryFlags_MatchesCertificate; if ((flags & matchesFlags) != matchesFlags) { + RELEASE_LOCALS(loc, env); LOG_ERROR("Store already contains alias with entry that does not match the expected certificate"); return FAIL; } if ((flags & EntryFlags_HasPrivateKey) == EntryFlags_HasPrivateKey) { + RELEASE_LOCALS(loc, env); // Certificate with private key is already in store - nothing to do LOG_DEBUG("Store already contains certificate with private key"); return SUCCESS; @@ -154,7 +157,7 @@ int32_t AndroidCryptoNative_X509StoreAddCertificateWithPrivateKey(jobject /*KeyS // Delete existing entry. We will replace the existing cert with the cert + private key. // store.deleteEntry(alias); - (*env)->CallVoidMethod(env, store, g_KeyStoreDeleteEntry, alias); + (*env)->CallVoidMethod(env, store, g_KeyStoreDeleteEntry, loc[alias]); } bool releasePrivateKey = true; diff --git a/src/libraries/System.Security.Cryptography.X509Certificates/tests/AssemblyInfo.cs b/src/libraries/System.Security.Cryptography.X509Certificates/tests/AssemblyInfo.cs deleted file mode 100644 index 3a74f33d7aa32e..00000000000000 --- a/src/libraries/System.Security.Cryptography.X509Certificates/tests/AssemblyInfo.cs +++ /dev/null @@ -1,7 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using System; -using Xunit; - -[assembly: ActiveIssue("https://github.com/dotnet/runtime/issues/37669", TestPlatforms.Browser)] diff --git a/src/libraries/System.Security.Cryptography.X509Certificates/tests/CertificateCreation/CertificateRequestChainTests.cs b/src/libraries/System.Security.Cryptography.X509Certificates/tests/CertificateCreation/CertificateRequestChainTests.cs index 16c41b6a68dcab..10c33cacca14c7 100644 --- a/src/libraries/System.Security.Cryptography.X509Certificates/tests/CertificateCreation/CertificateRequestChainTests.cs +++ b/src/libraries/System.Security.Cryptography.X509Certificates/tests/CertificateCreation/CertificateRequestChainTests.cs @@ -526,6 +526,13 @@ public static void CreateChain_RSAPSS() private static bool DetectPssSupport() { + if (PlatformDetection.IsAndroid) + { + // Android supports PSS at the algorithms layer, but does not support it + // being used in cert chains. + return false; + } + using (X509Certificate2 cert = new X509Certificate2(TestData.PfxData, TestData.PfxDataPassword)) using (RSA rsa = cert.GetRSAPrivateKey()) { diff --git a/src/libraries/System.Security.Cryptography.X509Certificates/tests/ChainTests.cs b/src/libraries/System.Security.Cryptography.X509Certificates/tests/ChainTests.cs index 9e5dbb03606e65..5744a52030e7a9 100644 --- a/src/libraries/System.Security.Cryptography.X509Certificates/tests/ChainTests.cs +++ b/src/libraries/System.Security.Cryptography.X509Certificates/tests/ChainTests.cs @@ -210,10 +210,13 @@ public static void TestResetMethod() /// /// Tests that when a certificate chain has a root certification which is not trusted by the trust provider, - /// Build returns false and a ChainStatus returns UntrustedRoot + /// Build returns false and a ChainStatus returns UntrustedRoot. + /// Android does not support the detailed status in this test. It always validates time + /// and trusted root. It will fail to build any chain if those are not valid. /// [Fact] [OuterLoop] + [PlatformSpecific(~TestPlatforms.Android)] public static void BuildChainExtraStoreUntrustedRoot() { using (var testCert = new X509Certificate2(TestFiles.ChainPfxFile, TestData.ChainPfxPassword)) diff --git a/src/libraries/System.Security.Cryptography.X509Certificates/tests/PublicKeyTests.cs b/src/libraries/System.Security.Cryptography.X509Certificates/tests/PublicKeyTests.cs index 411ca757829584..346cf590026987 100644 --- a/src/libraries/System.Security.Cryptography.X509Certificates/tests/PublicKeyTests.cs +++ b/src/libraries/System.Security.Cryptography.X509Certificates/tests/PublicKeyTests.cs @@ -489,7 +489,12 @@ public static void TestECDsaPublicKey_BrainpoolP160r1_ValidatesSignature(byte[] Assert.Equal("1.2.840.10045.2.1", cert.PublicKey.Oid.Value); bool isSignatureValid = publicKey.VerifyData(helloBytes, existingSignature, HashAlgorithmName.SHA256); - Assert.True(isSignatureValid, "isSignatureValid"); + + if (!isSignatureValid) + { + Assert.True(PlatformDetection.IsAndroid, "signature invalid on Android only"); + return; + } unchecked { diff --git a/src/libraries/System.Security.Cryptography.X509Certificates/tests/RevocationTests/DynamicRevocationTests.Android.cs b/src/libraries/System.Security.Cryptography.X509Certificates/tests/RevocationTests/DynamicRevocationTests.Android.cs new file mode 100644 index 00000000000000..fd733341851fcd --- /dev/null +++ b/src/libraries/System.Security.Cryptography.X509Certificates/tests/RevocationTests/DynamicRevocationTests.Android.cs @@ -0,0 +1,16 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Collections.Generic; +using System.Linq; +using System.Runtime.CompilerServices; +using System.Security.Cryptography.X509Certificates.Tests.Common; +using Xunit; + +namespace System.Security.Cryptography.X509Certificates.Tests.RevocationTests +{ + public static partial class DynamicRevocationTests + { + public static bool SupportsDynamicRevocation { get; } = Interop.AndroidCrypto.X509ChainSupportsRevocationOptions(); + } +} diff --git a/src/libraries/System.Security.Cryptography.X509Certificates/tests/RevocationTests/DynamicRevocationTests.Default.cs b/src/libraries/System.Security.Cryptography.X509Certificates/tests/RevocationTests/DynamicRevocationTests.Default.cs new file mode 100644 index 00000000000000..34384d913b4f0e --- /dev/null +++ b/src/libraries/System.Security.Cryptography.X509Certificates/tests/RevocationTests/DynamicRevocationTests.Default.cs @@ -0,0 +1,16 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Collections.Generic; +using System.Linq; +using System.Runtime.CompilerServices; +using System.Security.Cryptography.X509Certificates.Tests.Common; +using Xunit; + +namespace System.Security.Cryptography.X509Certificates.Tests.RevocationTests +{ + public static partial class DynamicRevocationTests + { + public static bool SupportsDynamicRevocation => true; + } +} diff --git a/src/libraries/System.Security.Cryptography.X509Certificates/tests/RevocationTests/DynamicRevocationTests.cs b/src/libraries/System.Security.Cryptography.X509Certificates/tests/RevocationTests/DynamicRevocationTests.cs index 7b311230b0d5d5..ebf8910855825d 100644 --- a/src/libraries/System.Security.Cryptography.X509Certificates/tests/RevocationTests/DynamicRevocationTests.cs +++ b/src/libraries/System.Security.Cryptography.X509Certificates/tests/RevocationTests/DynamicRevocationTests.cs @@ -10,7 +10,8 @@ namespace System.Security.Cryptography.X509Certificates.Tests.RevocationTests { [OuterLoop("These tests run serially at about 1 second each, and the code shouldn't change that often.")] - public static class DynamicRevocationTests + [ConditionalClass(typeof(DynamicRevocationTests), nameof(SupportsDynamicRevocation))] + public static partial class DynamicRevocationTests { // The CI machines are doing an awful lot of things at once, be generous with the timeout; internal static readonly TimeSpan s_urlRetrievalLimit = TimeSpan.FromSeconds(15); diff --git a/src/libraries/System.Security.Cryptography.X509Certificates/tests/System.Security.Cryptography.X509Certificates.Tests.csproj b/src/libraries/System.Security.Cryptography.X509Certificates/tests/System.Security.Cryptography.X509Certificates.Tests.csproj index 53d0dd75a0baa1..af574bfff240d7 100644 --- a/src/libraries/System.Security.Cryptography.X509Certificates/tests/System.Security.Cryptography.X509Certificates.Tests.csproj +++ b/src/libraries/System.Security.Cryptography.X509Certificates/tests/System.Security.Cryptography.X509Certificates.Tests.csproj @@ -12,7 +12,6 @@ - + + + + + + + + + diff --git a/src/libraries/tests.proj b/src/libraries/tests.proj index 55fdbc13f9f1fb..2d5924c7ca45ee 100644 --- a/src/libraries/tests.proj +++ b/src/libraries/tests.proj @@ -165,6 +165,9 @@ + + +