Fix X509 test failures on Android

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,

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);

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);

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 <pthread.h>
 
 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,13 +571,34 @@ 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;
     (*gJvm)->GetEnv(gJvm, (void**)&env, JNI_VERSION_1_6);
     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;

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:

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 (!(*env)->IsSameObject(env, certs[i-1], certs[i]))
+    {
+        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);

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
 {

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,20 +142,22 @@ 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;
         }
 
         // 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;

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())
             {

src/libraries/System.Security.Cryptography.X509Certificates/tests/ChainTests.cs

@@ -210,10 +210,13 @@ public static void TestResetMethod()
 
         /// <summary>
         /// 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.
         /// </summary>
         [Fact]
         [OuterLoop]
+        [PlatformSpecific(~TestPlatforms.Android)]
         public static void BuildChainExtraStoreUntrustedRoot()
         {
             using (var testCert = new X509Certificate2(TestFiles.ChainPfxFile, TestData.ChainPfxPassword))

src/libraries/System.Security.Cryptography.X509Certificates/tests/PublicKeyTests.cs

@@ -12,6 +12,21 @@ namespace System.Security.Cryptography.X509Certificates.Tests
 {
     public static class PublicKeyTests
     {
+        public static bool SupportsBrainpool { get; } = IsCurveSupported(ECCurve.NamedCurves.brainpoolP160r1.Oid);
+
+        private static bool IsCurveSupported(Oid oid)
+        {
+            try
+            {
+                using ECDsa ecdsa = ECDsa.Create(ECCurve.CreateFromOid(oid));
+            }
+            catch (Exception ex) when (ex is CryptographicException || ex is PlatformNotSupportedException)
+            {
+                return false;
+            }
+            return true;
+        }
+
         private static PublicKey GetTestRsaKey()
         {
             using (var cert = new X509Certificate2(TestData.MsCertificate))
@@ -472,7 +487,7 @@ public static void TestECDHPublicKey_DeriveSecret()
             }
         }
 
-        [Theory, MemberData(nameof(BrainpoolCurves))]
+        [ConditionalTheory(nameof(SupportsBrainpool)), MemberData(nameof(BrainpoolCurves))]
         public static void TestECDsaPublicKey_BrainpoolP160r1_ValidatesSignature(byte[] curveData, byte[] existingSignature)
         {
             byte[] helloBytes = Encoding.ASCII.GetBytes("Hello");

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();
+    }
+}

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;
+    }
+}

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);

src/libraries/System.Security.Cryptography.X509Certificates/tests/System.Security.Cryptography.X509Certificates.Tests.csproj

@@ -12,7 +12,6 @@
   </PropertyGroup>
   <Import Project="$(CommonPath)System\Security\Cryptography\Asn1Reader\System.Security.Cryptography.Asn1Reader.Shared.projitems" />
   <ItemGroup>
-    <Compile Include="AssemblyInfo.cs" />
     <Compile Include="$(CommonPath)System\Memory\PointerMemoryManager.cs"
              Link="Common\System\Memory\PointerMemoryManager.cs" />
     <Compile Include="$(CommonPath)System\Security\Cryptography\CryptoPool.cs"
@@ -99,8 +98,22 @@
   <ItemGroup Condition="'$(UseAndroidCrypto)' == 'true' or '$(UseAppleCrypto)' == 'true'">
     <Compile Include="X509StoreMutableTests.cs" />
   </ItemGroup>
+  <ItemGroup Condition="'$(UseAndroidCrypto)' != 'true'">
+    <Compile Include="RevocationTests\DynamicRevocationTests.Default.cs" />
+  </ItemGroup>
   <ItemGroup Condition="'$(UseAndroidCrypto)' == 'true'">
     <Compile Include="X509StoreMutableTests.Android.cs" />
+    <Compile Include="RevocationTests\DynamicRevocationTests.Android.cs" />
+    <Compile Include="$(CommonPath)Interop\Android\System.Security.Cryptography.Native.Android\Interop.X509.cs"
+             Link="Common\Interop\Android\System.Security.Cryptography.Native.Android\Interop.X509.cs" />
+    <Compile Include="$(CommonPath)Interop\Android\System.Security.Cryptography.Native.Android\Interop.X509Chain.cs"
+             Link="Common\Interop\Android\System.Security.Cryptography.Native.Android\Interop.X509Chain.cs" />
+    <Compile Include="$(CommonPath)Interop\Android\Interop.JObjectLifetime.cs"
+             Link="Common\Interop\Android\Interop.JObjectLifetime.cs" />
+    <Compile Include="$(CommonPath)Interop\Android\Interop.Libraries.cs"
+             Link="Common\Interop\Android\Interop.Libraries.cs" />
+    <Compile Include="$(CommonPath)Interop\Unix\Interop.Libraries.cs"
+             Link="Common\Interop\Unix\Interop.Libraries.cs" />
   </ItemGroup>
   <ItemGroup Condition="'$(UseAppleCrypto)' == 'true'">
     <Compile Include="X509StoreMutableTests.OSX.cs" />

src/libraries/tests.proj

@@ -165,6 +165,9 @@
 
     <ProjectExclusions Include="$(MSBuildThisFileDirectory)System.Linq.Parallel\tests\System.Linq.Parallel.Tests.csproj" />
     <ProjectExclusions Include="$(MSBuildThisFileDirectory)System.Net.Http\tests\FunctionalTests\System.Net.Http.Functional.Tests.csproj" />
+
+    <!-- https://github.com/dotnet/runtime/issues/37669 -->
+    <ProjectExclusions Include="$(MSBuildThisFileDirectory)System.Security.Cryptography.X509Certificates\tests\System.Security.Cryptography.X509Certificates.csproj" />
   </ItemGroup>
 
   <ItemGroup Condition="'$(TargetOS)' == 'Browser' and '$(RunAOTCompilation)' == 'true' and '$(RunDisabledWasmTests)' != 'true'">