From 28794a2c264ff74536c93cd4a7614dd31ea61527 Mon Sep 17 00:00:00 2001 From: Elinor Fung Date: Tue, 9 Mar 2021 20:00:20 -0800 Subject: [PATCH 01/19] Basic chain building TODO: - set custom trust root - handle revocation checking - propagate errors/status where possible --- .../Interop.X509Chain.cs | 85 +++++ .../CMakeLists.txt | 1 + .../pal_jni.c | 136 +++++++- .../pal_jni.h | 62 +++- .../pal_x509.c | 2 +- .../pal_x509chain.c | 328 ++++++++++++++++++ .../pal_x509chain.h | 78 +++++ .../Cryptography/Pal.Android/ChainPal.cs | 226 +++++++++++- .../src/Resources/Strings.resx | 3 + ...urity.Cryptography.X509Certificates.csproj | 2 + 10 files changed, 909 insertions(+), 14 deletions(-) create mode 100644 src/libraries/Common/src/Interop/Android/System.Security.Cryptography.Native.Android/Interop.X509Chain.cs create mode 100644 src/libraries/Native/Unix/System.Security.Cryptography.Native.Android/pal_x509chain.c create mode 100644 src/libraries/Native/Unix/System.Security.Cryptography.Native.Android/pal_x509chain.h 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 new file mode 100644 index 0000000000000..d88827f36b26e --- /dev/null +++ b/src/libraries/Common/src/Interop/Android/System.Security.Cryptography.Native.Android/Interop.X509Chain.cs @@ -0,0 +1,85 @@ +// 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 System.Runtime.InteropServices; +using System.Security.Cryptography; +using System.Security.Cryptography.X509Certificates; + +internal static partial class Interop +{ + internal static partial class AndroidCrypto + { + [DllImport(Libraries.CryptoNative, EntryPoint = "AndroidCryptoNative_X509ChainCreateContext")] + internal static extern SafeX509ChainContextHandle X509ChainCreateContext( + SafeX509Handle cert, + IntPtr[] extraStore, + int extraStoreLen); + + [DllImport(Libraries.CryptoNative, EntryPoint = "AndroidCryptoNative_X509ChainDestroyContext")] + internal static extern void X509ChainDestroyContext(IntPtr ctx); + + [DllImport(Libraries.CryptoNative, EntryPoint = "AndroidCryptoNative_X509ChainEvaluate")] + [return: MarshalAs(UnmanagedType.U1)] + internal static extern bool X509ChainEvaluate( + SafeX509ChainContextHandle ctx, + long timeInMsFromUnixEpoch); + + [DllImport(Libraries.CryptoNative, EntryPoint = "AndroidCryptoNative_X509ChainGetCertificateCount")] + private static extern int X509ChainGetCertificateCount(SafeX509ChainContextHandle ctx); + + [DllImport(Libraries.CryptoNative, EntryPoint = "AndroidCryptoNative_X509ChainGetCertificates")] + private static extern int X509ChainGetCertificates( + SafeX509ChainContextHandle ctx, + IntPtr[] certs, + int certsLen); + + internal static IntPtr[] X509ChainGetCertificates(SafeX509ChainContextHandle ctx) + { + int count = Interop.AndroidCrypto.X509ChainGetCertificateCount(ctx); + var certPtrs = new IntPtr[count]; + + int res = Interop.AndroidCrypto.X509ChainGetCertificates(ctx, certPtrs, certPtrs.Length); + if (res != SUCCESS) + throw new CryptographicException(); + + return certPtrs; + } + + [DllImport(Libraries.CryptoNative, EntryPoint = "AndroidCryptoNative_X509ChainSetCustomTrustStore")] + internal static extern int X509ChainSetCustomTrustStore( + SafeX509ChainContextHandle ctx, + 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, + X509RevocationMode revocationMode, + X509RevocationFlag revocationFlag); + } +} + +namespace System.Security.Cryptography.X509Certificates +{ + internal sealed class SafeX509ChainContextHandle : SafeHandle + { + public SafeX509ChainContextHandle() + : base(IntPtr.Zero, ownsHandle: true) + { + } + + protected override bool ReleaseHandle() + { + Interop.AndroidCrypto.X509ChainDestroyContext(handle); + SetHandle(IntPtr.Zero); + return true; + } + + public override bool IsInvalid => handle == IntPtr.Zero; + } +} diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native.Android/CMakeLists.txt b/src/libraries/Native/Unix/System.Security.Cryptography.Native.Android/CMakeLists.txt index 51b1466554dc1..bf7473bd5a3f9 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native.Android/CMakeLists.txt +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native.Android/CMakeLists.txt @@ -23,6 +23,7 @@ set(NATIVECRYPTO_SOURCES pal_ssl.c pal_sslstream.c pal_x509.c + pal_x509chain.c ) add_library(System.Security.Cryptography.Native.Android 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 5721eedc37b8b..52632a29a2e65 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 @@ -106,6 +106,11 @@ jmethodID g_keyPairGenInitializeWithParamsMethod; jmethodID g_keyPairGenInitializeMethod; jmethodID g_keyPairGenGenKeyPairMethod; +// java/security/KeyStore +jclass g_KeyStoreClass; +jmethodID g_KeyStoreGetInstance; +jmethodID g_KeyStoreLoad; + // java/security/Signature jclass g_SignatureClass; jmethodID g_SignatureGetInstance; @@ -127,12 +132,59 @@ jmethodID g_CertFactoryGenerateCRL; // java/security/cert/CertPath jclass g_CertPathClass; jmethodID g_CertPathGetEncoded; +jmethodID g_CertPathGetCertificates; + +// java/security/cert/CertPathBuilder +jclass g_CertPathBuilderClass; +jmethodID g_CertPathBuilderGetInstance; +jmethodID g_CertPathBuilderBuild; + +// java/security/cert/CertPathValidator +jclass g_CertPathValidatorClass; +jmethodID g_CertPathValidatorGetInstance; +jmethodID g_CertPathValidatorValidate; +jmethodID g_CertPathValidatorGetRevocationChecker; // only in API level 24+ + +// java/security/cert/CertStore +jclass g_CertStoreClass; +jmethodID g_CertStoreGetInstance; + +// java/security/cert/CollectionCertStoreParameters +jclass g_CollectionCertStoreParametersClass; +jmethodID g_CollectionCertStoreParametersCtor; + +// java/security/cert/PKIXBuilderParameters +jclass g_PKIXBuilderParametersClass; +jmethodID g_PKIXBuilderParametersCtor; +jmethodID g_PKIXBuilderParametersAddCertStore; +jmethodID g_PKIXBuilderParametersAddCertPathChecker; +jmethodID g_PKIXBuilderParametersSetDate; +jmethodID g_PKIXBuilderParametersSetRevocationEnabled; +jmethodID g_PKIXBuilderParametersSetTrustAnchors; + +// java/security/cert/PKIXCertPathBuilderResult +jclass g_PKIXCertPathBuilderResultClass; +jmethodID g_PKIXCertPathBuilderResultGetCertPath; +jmethodID g_PKIXCertPathBuilderResultGetTrustAnchor; + +// java/security/cert/PKIXRevocationChecker - only in API level 24+ +jclass g_PKIXRevocationCheckerClass; +jmethodID g_PKIXRevocationCheckerSetOptions; + +// java/security/cert/TrustAnchor +jclass g_TrustAnchorClass; +jmethodID g_TrustAnchorGetTrustedCert; // java/security/cert/X509Certificate jclass g_X509CertClass; jmethodID g_X509CertGetEncoded; jmethodID g_X509CertGetPublicKey; +// java/security/cert/X509CertSelector +jclass g_X509CertSelectorClass; +jmethodID g_X509CertSelectorCtor; +jmethodID g_X509CertSelectorSetCertificate; + // java/security/interfaces/RSAPrivateCrtKey jclass g_RSAPrivateCrtKeyClass; jmethodID g_RSAPrivateCrtKeyPubExpField; @@ -242,7 +294,8 @@ jmethodID g_destroy; // java/util/ArrayList jclass g_ArrayListClass; -jmethodID g_ArrayListCtor; +jmethodID g_ArrayListCtorWithCapacity; +jmethodID g_ArrayListCtorWithCollection; jmethodID g_ArrayListAdd; // java/util/Collection @@ -252,6 +305,7 @@ jmethodID g_CollectionSize; // java/util/Date jclass g_DateClass; +jmethodID g_DateCtor; jmethodID g_DateGetTime; // java/util/Iterator @@ -371,6 +425,21 @@ bool CheckJNIExceptions(JNIEnv* env) return false; } +bool TryGetJNIException(JNIEnv* env, jthrowable *ex, bool printException) +{ + if (!(*env)->ExceptionCheck(env)) + return false; + + if (printException) + { + (*env)->ExceptionDescribe(env); + } + + *ex = (*env)->ExceptionOccurred(env); + (*env)->ExceptionClear(env); + return true; +} + void AssertOnJNIExceptions(JNIEnv* env) { assert(!CheckJNIExceptions(env)); @@ -523,12 +592,53 @@ JNI_OnLoad(JavaVM *vm, void *reserved) g_CertFactoryGenerateCertPathFromStream = GetMethod(env, false, g_CertFactoryClass, "generateCertPath", "(Ljava/io/InputStream;Ljava/lang/String;)Ljava/security/cert/CertPath;"); g_CertFactoryGenerateCRL = GetMethod(env, false, g_CertFactoryClass, "generateCRL", "(Ljava/io/InputStream;)Ljava/security/cert/CRL;"); - g_CertPathClass = GetClassGRef(env, "java/security/cert/CertPath"); - g_CertPathGetEncoded = GetMethod(env, false, g_CertPathClass, "getEncoded", "(Ljava/lang/String;)[B"); + g_CertPathClass = GetClassGRef(env, "java/security/cert/CertPath"); + g_CertPathGetEncoded = GetMethod(env, false, g_CertPathClass, "getEncoded", "(Ljava/lang/String;)[B"); + g_CertPathGetCertificates = GetMethod(env, false, g_CertPathClass, "getCertificates", "()Ljava/util/List;"); + + g_CertPathBuilderClass = GetClassGRef(env, "java/security/cert/CertPathBuilder"); + g_CertPathBuilderGetInstance = GetMethod(env, true, g_CertPathBuilderClass, "getInstance", "(Ljava/lang/String;)Ljava/security/cert/CertPathBuilder;"); + g_CertPathBuilderBuild = GetMethod(env, false, g_CertPathBuilderClass, "build", "(Ljava/security/cert/CertPathParameters;)Ljava/security/cert/CertPathBuilderResult;"); - g_X509CertClass = GetClassGRef(env, "java/security/cert/X509Certificate"); - g_X509CertGetEncoded = GetMethod(env, false, g_X509CertClass, "getEncoded", "()[B"); - g_X509CertGetPublicKey = GetMethod(env, false, g_X509CertClass, "getPublicKey", "()Ljava/security/PublicKey;"); + g_CertPathValidatorClass = GetClassGRef(env, "java/security/cert/CertPathValidator"); + g_CertPathValidatorGetInstance = GetMethod(env, true, g_CertPathValidatorClass, "getInstance", "(Ljava/lang/String;)Ljava/security/cert/CertPathValidator;"); + g_CertPathValidatorValidate = GetMethod(env, false, g_CertPathValidatorClass, "validate", "(Ljava/security/cert/CertPath;Ljava/security/cert/CertPathParameters;)Ljava/security/cert/CertPathValidatorResult;"); + g_CertPathValidatorGetRevocationChecker = GetOptionalMethod(env, false, g_CertPathValidatorClass, "getRevocationChecker", "()Ljava/security/cert/CertPathChecker;"); + + g_CertStoreClass = GetClassGRef(env, "java/security/cert/CertStore"); + g_CertStoreGetInstance = GetMethod(env, true, g_CertStoreClass, "getInstance", "(Ljava/lang/String;Ljava/security/cert/CertStoreParameters;)Ljava/security/cert/CertStore;"); + + g_CollectionCertStoreParametersClass = GetClassGRef(env, "java/security/cert/CollectionCertStoreParameters"); + g_CollectionCertStoreParametersCtor = GetMethod(env, false, g_CollectionCertStoreParametersClass, "", "(Ljava/util/Collection;)V"); + + g_PKIXBuilderParametersClass = GetClassGRef(env, "java/security/cert/PKIXBuilderParameters"); + g_PKIXBuilderParametersCtor = GetMethod(env, false, g_PKIXBuilderParametersClass, "", "(Ljava/security/KeyStore;Ljava/security/cert/CertSelector;)V"); + g_PKIXBuilderParametersAddCertStore = GetMethod(env, false, g_PKIXBuilderParametersClass, "addCertStore", "(Ljava/security/cert/CertStore;)V"); + g_PKIXBuilderParametersAddCertPathChecker = GetMethod(env, false, g_PKIXBuilderParametersClass, "addCertPathChecker", "(Ljava/security/cert/PKIXCertPathChecker;)V"); + g_PKIXBuilderParametersSetDate = GetMethod(env, false, g_PKIXBuilderParametersClass, "setDate", "(Ljava/util/Date;)V"); + g_PKIXBuilderParametersSetRevocationEnabled = GetMethod(env, false, g_PKIXBuilderParametersClass, "setRevocationEnabled", "(Z)V"); + g_PKIXBuilderParametersSetTrustAnchors = GetMethod(env, false, g_PKIXBuilderParametersClass, "setTrustAnchors", "(Ljava/util/Set;)V"); + + g_PKIXCertPathBuilderResultClass = GetClassGRef(env, "java/security/cert/PKIXCertPathBuilderResult"); + g_PKIXCertPathBuilderResultGetCertPath = GetMethod(env, false, g_PKIXCertPathBuilderResultClass, "getCertPath", "()Ljava/security/cert/CertPath;"); + g_PKIXCertPathBuilderResultGetTrustAnchor = GetMethod(env, false, g_PKIXCertPathBuilderResultClass, "getTrustAnchor", "()Ljava/security/cert/TrustAnchor;"); + + if (g_CertPathValidatorGetRevocationChecker != NULL) + { + g_PKIXRevocationCheckerClass = GetClassGRef(env, "java/security/cert/PKIXRevocationChecker"); + g_PKIXRevocationCheckerSetOptions = GetMethod(env, false, g_PKIXRevocationCheckerClass, "setOptions", "(Ljava/util/Set;)V"); + } + + g_TrustAnchorClass = GetClassGRef(env, "java/security/cert/TrustAnchor"); + g_TrustAnchorGetTrustedCert = GetMethod(env, false, g_TrustAnchorClass, "getTrustedCert", "()Ljava/security/cert/X509Certificate;"); + + g_X509CertClass = GetClassGRef(env, "java/security/cert/X509Certificate"); + g_X509CertGetEncoded = GetMethod(env, false, g_X509CertClass, "getEncoded", "()[B"); + g_X509CertGetPublicKey = GetMethod(env, false, g_X509CertClass, "getPublicKey", "()Ljava/security/PublicKey;"); + + g_X509CertSelectorClass = GetClassGRef(env, "java/security/cert/X509CertSelector"); + g_X509CertSelectorCtor = GetMethod(env, false, g_X509CertSelectorClass, "", "()V"); + g_X509CertSelectorSetCertificate = GetMethod(env, false, g_X509CertSelectorClass, "setCertificate", "(Ljava/security/cert/X509Certificate;)V"); g_RSAKeyClass = GetClassGRef(env, "java/security/interfaces/RSAKey"); g_RSAKeyGetModulus = GetMethod(env, false, g_RSAKeyClass, "getModulus", "()Ljava/math/BigInteger;"); @@ -547,6 +657,10 @@ JNI_OnLoad(JavaVM *vm, void *reserved) g_keyPairGenInitializeWithParamsMethod = GetMethod(env, false, g_keyPairGenClass, "initialize", "(Ljava/security/spec/AlgorithmParameterSpec;)V"); g_keyPairGenGenKeyPairMethod = GetMethod(env, false, g_keyPairGenClass, "genKeyPair", "()Ljava/security/KeyPair;"); + g_KeyStoreClass = GetClassGRef(env, "java/security/KeyStore"); + g_KeyStoreGetInstance = GetMethod(env, true, g_KeyStoreClass, "getInstance", "(Ljava/lang/String;)Ljava/security/KeyStore;"); + g_KeyStoreLoad = GetMethod(env, false, g_KeyStoreClass, "load", "(Ljava/io/InputStream;[C)V"); + g_SignatureClass = GetClassGRef(env, "java/security/Signature"); g_SignatureGetInstance = GetMethod(env, true, g_SignatureClass, "getInstance", "(Ljava/lang/String;)Ljava/security/Signature;"); g_SignatureInitSign = GetMethod(env, false, g_SignatureClass, "initSign", "(Ljava/security/PrivateKey;)V"); @@ -580,7 +694,7 @@ JNI_OnLoad(JavaVM *vm, void *reserved) g_DSAPublicKeySpecClass = GetClassGRef(env, "java/security/spec/DSAPublicKeySpec"); g_DSAPublicKeySpecCtor = GetMethod(env, false, g_DSAPublicKeySpecClass, "", "(Ljava/math/BigInteger;Ljava/math/BigInteger;Ljava/math/BigInteger;Ljava/math/BigInteger;)V"); g_DSAPublicKeySpecGetY = GetMethod(env, false, g_DSAPublicKeySpecClass, "getY", "()Ljava/math/BigInteger;"); - g_DSAPublicKeySpecGetP = GetMethod(env, false, g_DSAPublicKeySpecClass, "getP", "()Ljava/math/BigInteger;"); + g_DSAPublicKeySpecGetP = GetMethod(env, false, g_DSAPublicKeySpecClass, "getP", "()Ljava/math/BigInteger;"); g_DSAPublicKeySpecGetQ = GetMethod(env, false, g_DSAPublicKeySpecClass, "getQ", "()Ljava/math/BigInteger;"); g_DSAPublicKeySpecGetG = GetMethod(env, false, g_DSAPublicKeySpecClass, "getG", "()Ljava/math/BigInteger;"); @@ -643,15 +757,17 @@ JNI_OnLoad(JavaVM *vm, void *reserved) g_DestroyableClass = GetClassGRef(env, "javax/security/auth/Destroyable"); g_destroy = GetMethod(env, false, g_DestroyableClass, "destroy", "()V"); - g_ArrayListClass = GetClassGRef(env, "java/util/ArrayList"); - g_ArrayListCtor = GetMethod(env, false, g_ArrayListClass, "", "(I)V"); - g_ArrayListAdd = GetMethod(env, false, g_ArrayListClass, "add", "(Ljava/lang/Object;)Z"); + g_ArrayListClass = GetClassGRef(env, "java/util/ArrayList"); + g_ArrayListCtorWithCapacity = GetMethod(env, false, g_ArrayListClass, "", "(I)V"); + g_ArrayListCtorWithCollection = GetMethod(env, false, g_ArrayListClass, "", "(Ljava/util/Collection;)V"); + g_ArrayListAdd = GetMethod(env, false, g_ArrayListClass, "add", "(Ljava/lang/Object;)Z"); g_CollectionClass = GetClassGRef(env, "java/util/Collection"); g_CollectionIterator = GetMethod(env, false, g_CollectionClass, "iterator", "()Ljava/util/Iterator;"); g_CollectionSize = GetMethod(env, false, g_CollectionClass, "size", "()I"); g_DateClass = GetClassGRef(env, "java/util/Date"); + g_DateCtor = GetMethod(env, false, g_DateClass, "", "(J)V"); g_DateGetTime = GetMethod(env, false, g_DateClass, "getTime", "()J"); g_IteratorClass = GetClassGRef(env, "java/util/Iterator"); diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native.Android/pal_jni.h b/src/libraries/Native/Unix/System.Security.Cryptography.Native.Android/pal_jni.h index 252565fb0c934..731f9b555cc14 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native.Android/pal_jni.h +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native.Android/pal_jni.h @@ -106,12 +106,59 @@ extern jmethodID g_CertFactoryGenerateCRL; // java/security/cert/CertPath extern jclass g_CertPathClass; extern jmethodID g_CertPathGetEncoded; +extern jmethodID g_CertPathGetCertificates; + +// java/security/cert/CertPathBuilder +extern jclass g_CertPathBuilderClass; +extern jmethodID g_CertPathBuilderGetInstance; +extern jmethodID g_CertPathBuilderBuild; + +// java/security/cert/CertPathValidator +extern jclass g_CertPathValidatorClass; +extern jmethodID g_CertPathValidatorGetInstance; +extern jmethodID g_CertPathValidatorValidate; +extern jmethodID g_CertPathValidatorGetRevocationChecker; // only in API level 24+ + +// java/security/cert/CertStore +extern jclass g_CertStoreClass; +extern jmethodID g_CertStoreGetInstance; + +// java/security/cert/CollectionCertStoreParameters +extern jclass g_CollectionCertStoreParametersClass; +extern jmethodID g_CollectionCertStoreParametersCtor; + +// java/security/cert/PKIXBuilderParameters +extern jclass g_PKIXBuilderParametersClass; +extern jmethodID g_PKIXBuilderParametersCtor; +extern jmethodID g_PKIXBuilderParametersAddCertStore; +extern jmethodID g_PKIXBuilderParametersAddCertPathChecker; +extern jmethodID g_PKIXBuilderParametersSetDate; +extern jmethodID g_PKIXBuilderParametersSetRevocationEnabled; +extern jmethodID g_PKIXBuilderParametersSetTrustAnchors; + +// java/security/cert/PKIXCertPathBuilderResult +extern jclass g_PKIXCertPathBuilderResultClass; +extern jmethodID g_PKIXCertPathBuilderResultGetCertPath; +extern jmethodID g_PKIXCertPathBuilderResultGetTrustAnchor; + +// java/security/cert/PKIXRevocationChecker - only in API level 24+ +extern jclass g_PKIXRevocationCheckerClass; +extern jmethodID g_PKIXRevocationCheckerSetOptions; + +// java/security/cert/TrustAnchor +extern jclass g_TrustAnchorClass; +extern jmethodID g_TrustAnchorGetTrustedCert; // java/security/cert/X509Certificate extern jclass g_X509CertClass; extern jmethodID g_X509CertGetEncoded; extern jmethodID g_X509CertGetPublicKey; +// java/security/cert/X509CertSelector +extern jclass g_X509CertSelectorClass; +extern jmethodID g_X509CertSelectorCtor; +extern jmethodID g_X509CertSelectorSetCertificate; + // java/security/interfaces/RSAKey extern jclass g_RSAKeyClass; extern jmethodID g_RSAKeyGetModulus; @@ -133,6 +180,11 @@ extern jmethodID g_keyPairGenInitializeMethod; extern jmethodID g_keyPairGenInitializeWithParamsMethod; extern jmethodID g_keyPairGenGenKeyPairMethod; +// java/security/KeyStore +extern jclass g_KeyStoreClass; +extern jmethodID g_KeyStoreGetInstance; +extern jmethodID g_KeyStoreLoad; + // java/security/Signature extern jclass g_SignatureClass; extern jmethodID g_SignatureGetInstance; @@ -249,9 +301,10 @@ extern jmethodID g_X509EncodedKeySpecCtor; extern jclass g_DestroyableClass; extern jmethodID g_destroy; -// java/util/Collection +// java/util/ArrayList extern jclass g_ArrayListClass; -extern jmethodID g_ArrayListCtor; +extern jmethodID g_ArrayListCtorWithCapacity; +extern jmethodID g_ArrayListCtorWithCollection; extern jmethodID g_ArrayListAdd; // java/util/Collection @@ -261,6 +314,7 @@ extern jmethodID g_CollectionSize; // java/util/Date extern jclass g_DateClass; +extern jmethodID g_DateCtor; extern jmethodID g_DateGetTime; // java/util/Iterator @@ -342,6 +396,10 @@ void ReleaseGRef(JNIEnv *env, jobject gref); void ReleaseLRef(JNIEnv *env, jobject lref); jclass GetClassGRef(JNIEnv *env, const char* name); bool CheckJNIExceptions(JNIEnv* env); + +// Get any pending JNI exception. Returns true if there was an exception, false otherwise. +bool TryGetJNIException(JNIEnv* env, jthrowable *ex, bool printException); + void AssertOnJNIExceptions(JNIEnv* env); jmethodID GetMethod(JNIEnv *env, bool isStatic, jclass klass, const char* name, const char* sig); jmethodID GetOptionalMethod(JNIEnv *env, bool isStatic, jclass klass, const char* name, const char* sig); 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 7c31b18c1e3ec..d1679912bf086 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 @@ -169,7 +169,7 @@ int32_t AndroidCryptoNative_X509ExportPkcs7(jobject* /*X509Certificate[]*/ certs // ArrayList certList = new ArrayList(); // foreach (Certificate cert in certs) // certList.add(cert); - loc[certList] = (*env)->NewObject(env, g_ArrayListClass, g_ArrayListCtor, certsLen); + loc[certList] = (*env)->NewObject(env, g_ArrayListClass, g_ArrayListCtorWithCapacity, certsLen); for (int i = 0; i < certsLen; ++i) { (*env)->CallBooleanMethod(env, loc[certList], g_ArrayListAdd, certs[i]); 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 new file mode 100644 index 0000000000000..a2f3448516090 --- /dev/null +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native.Android/pal_x509chain.c @@ -0,0 +1,328 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +#include "pal_x509chain.h" + +#include +#include +#include + +#define INIT_LOCALS(name, ...) \ + enum { __VA_ARGS__, count_##name }; \ + jobject name[count_##name] = { 0 }; \ + +#define RELEASE_LOCALS(name, env) \ +{ \ + for (int i_##name = 0; i_##name < count_##name; ++i_##name) \ + { \ + jobject local = name[i_##name]; \ + if (local != NULL) \ + (*env)->DeleteLocalRef(env, local); \ + } \ +} \ + +struct X509ChainContext_t +{ + jobject /*PKIXBuilderParameters*/ params; + jobject /*CertPath*/ certPath; + jobject /*TrustAnchor*/ trustAnchor; +}; + +X509ChainContext* AndroidCryptoNative_X509ChainCreateContext(jobject /*X509Certificate*/ cert, + jobject* /*X509Certificate[]*/ extraStore, + int32_t extraStoreLen) +{ + assert(cert != NULL); + assert(extraStore != NULL || extraStoreLen == 0); + JNIEnv* env = GetJNIEnv(); + + X509ChainContext* ret = NULL; + INIT_LOCALS(loc, keyStoreType, keyStore, targetSel, params, certList, certStoreType, certStoreParams, certStore) + + // String keyStoreType = "AndroidCAStore"; + // KeyStore keyStore = KeyStore.getInstance(keyStoreType); + // keyStore.load(null, null); + loc[keyStoreType] = JSTRING("AndroidCAStore"); + loc[keyStore] = (*env)->CallStaticObjectMethod(env, g_KeyStoreClass, g_KeyStoreGetInstance, loc[keyStoreType]); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); + (*env)->CallVoidMethod(env, loc[keyStore], g_KeyStoreLoad, NULL, NULL); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); + + // X509CertSelector targetSel = new X509CertSelector(); + // targetSel.setCertificate(cert); + loc[targetSel] = (*env)->NewObject(env, g_X509CertSelectorClass, g_X509CertSelectorCtor); + (*env)->CallVoidMethod(env, loc[targetSel], g_X509CertSelectorSetCertificate, cert); + + // PKIXBuilderParameters params = new PKIXBuilderParameters(keyStore, targetSelector); + loc[params] = (*env)->NewObject( + env, g_PKIXBuilderParametersClass, g_PKIXBuilderParametersCtor, loc[keyStore], loc[targetSel]); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); + + // ArrayList certList = new ArrayList(); + // certList.add(cert); + // for (int i = 0; i < extraStoreLen; i++) { + // certList.add(extraStore[i]); + // } + loc[certList] = (*env)->NewObject(env, g_ArrayListClass, g_ArrayListCtorWithCapacity, extraStoreLen); + (*env)->CallBooleanMethod(env, loc[certList], g_ArrayListAdd, cert); + for (int i = 0; i < extraStoreLen; ++i) + { + (*env)->CallBooleanMethod(env, loc[certList], g_ArrayListAdd, extraStore[i]); + } + + // String certStoreType = "Collection"; + // CollectionCertStoreParameters certStoreParams = new CollectionCertStoreParameters(certList); + // CertStore certStore = CertStore.getInstance(certStoreType, certStoreParams); + loc[certStoreType] = JSTRING("Collection"); + loc[certStoreParams] = (*env)->NewObject( + env, g_CollectionCertStoreParametersClass, g_CollectionCertStoreParametersCtor, loc[certList]); + loc[certStore] = (*env)->CallStaticObjectMethod( + env, g_CertStoreClass, g_CertStoreGetInstance, loc[certStoreType], loc[certStoreParams]); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); + + // params.addCertStore(certStore); + (*env)->CallVoidMethod(env, loc[params], g_PKIXBuilderParametersAddCertStore, loc[certStore]); + + ret = malloc(sizeof(X509ChainContext)); + memset(ret, 0, sizeof(X509ChainContext)); + ret->params = AddGRef(env, loc[params]); + +cleanup: + RELEASE_LOCALS(loc, env) + return ret; +} + +void AndroidCryptoNative_X509ChainDestroyContext(X509ChainContext* ctx) +{ + if (ctx == NULL) + return; + + JNIEnv* env = GetJNIEnv(); + ReleaseGRef(env, ctx->params); + ReleaseGRef(env, ctx->certPath); + ReleaseGRef(env, ctx->trustAnchor); + free(ctx); +} + +int32_t AndroidCryptoNative_X509ChainEvaluate(X509ChainContext* ctx, int64_t timeInMsFromUnixEpoch) +{ + assert(ctx != NULL); + JNIEnv* env = GetJNIEnv(); + + int32_t ret = FAIL; + INIT_LOCALS(loc, date, builderType, builder, result, ex, certPath, trustAnchor) + + jobject params = ctx->params; + + // Date date = new Date(timeInMsFromUnixEpoch); + // params.setDate(date); + loc[date] = (*env)->NewObject(env, g_DateClass, g_DateCtor, timeInMsFromUnixEpoch); + (*env)->CallVoidMethod(env, params, g_PKIXBuilderParametersSetDate, loc[date]); + + // Disable revocation checking when building the cert path. It will be handled in a validation pass if the path is + // successfully built. + // params.setRevocationEnabled(false); + (*env)->CallVoidMethod(env, params, g_PKIXBuilderParametersSetRevocationEnabled, false); + + // String builderType = "PKIX"; + // CertPathBuilder builder = CertPathBuilder.getInstance(builderType); + // PKIXCertPathBuilderResult result = (PKIXCertPathBuilderResult)builder.build(params); + loc[builderType] = JSTRING("PKIX"); + loc[builder] = + (*env)->CallStaticObjectMethod(env, g_CertPathBuilderClass, g_CertPathBuilderGetInstance, loc[builderType]); + loc[result] = (*env)->CallObjectMethod(env, loc[builder], g_CertPathBuilderBuild, params); + if (TryGetJNIException(env, &loc[ex], true /*printException*/)) + { + // TODO: [AndroidCrypto] Get/propagate the exception message to managed + goto cleanup; + } + + loc[certPath] = (*env)->CallObjectMethod(env, loc[result], g_PKIXCertPathBuilderResultGetCertPath); + loc[trustAnchor] = (*env)->CallObjectMethod(env, loc[result], g_PKIXCertPathBuilderResultGetTrustAnchor); + + ctx->certPath = AddGRef(env, loc[certPath]); + ctx->trustAnchor = AddGRef(env, loc[trustAnchor]); + ret = SUCCESS; + +cleanup: + RELEASE_LOCALS(loc, env) + return ret; +} + +int32_t AndroidCryptoNative_X509ChainGetCertificateCount(X509ChainContext* ctx) +{ + assert(ctx != NULL); + JNIEnv* env = GetJNIEnv(); + + // List certPathList = certPath.getCertificates(); + jobject certPathList = (*env)->CallObjectMethod(env, ctx->certPath, g_CertPathGetCertificates); + int certCount = (int)(*env)->CallIntMethod(env, certPathList, g_CollectionSize); + + (*env)->DeleteLocalRef(env, certPathList); + return certCount + 1; // +1 for the trust anchor +} + +int32_t AndroidCryptoNative_X509ChainGetCertificates(X509ChainContext* ctx, + jobject* /*X509Certificate[]*/ certs, + int32_t certsLen) +{ + assert(ctx != NULL); + JNIEnv* env = GetJNIEnv(); + + int32_t ret = FAIL; + INIT_LOCALS(loc, certPathList, iter) + + // List certPathList = certPath.getCertificates(); + loc[certPathList] = (*env)->CallObjectMethod(env, ctx->certPath, g_CertPathGetCertificates); + int certCount = (int)(*env)->CallIntMethod(env, loc[certPathList], g_CollectionSize); + + if (certsLen < certCount + 1) + goto cleanup; + + // int i = 0; + // Iterator iter = certs.iterator(); + // while (iter.hasNext()) { + // Certificate cert = iter.next(); + // out[i] = cert; + // i++; + // } + int32_t i = 0; + loc[iter] = (*env)->CallObjectMethod(env, loc[certPathList], g_CollectionIterator); + jboolean hasNext = (*env)->CallBooleanMethod(env, loc[iter], g_IteratorHasNext); + while (hasNext) + { + jobject cert = (*env)->CallObjectMethod(env, loc[iter], g_IteratorNext); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); + + certs[i] = ToGRef(env, cert); + i++; + + hasNext = (*env)->CallBooleanMethod(env, loc[iter], g_IteratorHasNext); + } + + // Certificate trustedCert = trustAnchor.getTrustedCert(); + // certs[i] = trustedCert; + jobject trustedCert = (*env)->CallObjectMethod(env, ctx->trustAnchor, g_TrustAnchorGetTrustedCert); + certs[i] = ToGRef(env, trustedCert); + + ret = SUCCESS; + +cleanup: + RELEASE_LOCALS(loc, env) + return ret; +} + +int32_t AndroidCryptoNative_X509ChainSetCustomTrustStore(X509ChainContext* ctx, + jobject* /*X509Certificate*/ customTrustStore, + int32_t customTrustStoreLen) +{ + // HashSet trustAnchors = new HashSet(); + // for (Certificate cert : customTrustStore) { + // TrustAnchor anchor = new TrustAnchor(cert, null); + // trustAnchors.Add(anchor); + // } + // params.setTrustAnchors(trustAnchors); + return FAIL; +} + +bool AndroidCryptoNative_X509ChainSupportsRevocationOptions(void) +{ + return g_CertPathValidatorGetRevocationChecker != NULL && g_PKIXRevocationCheckerClass != NULL; +} + +static jobject /*CertPath*/ +GetCertPathFromBuilderResult(JNIEnv* env, jobject /*CertPath*/ certPath, jobject /*TrustAnchor*/ trustAnchor) +{ + jobject ret = NULL; + INIT_LOCALS(loc, certPathList, certList, trustedCert, certFactoryType, certFactory) + + // List certPathList = certPath.getCertificates(); + loc[certPathList] = (*env)->CallObjectMethod(env, certPath, g_CertPathGetCertificates); + + // The result cert path does not include the trust anchor. Create a list combining the path and anchor. + // ArrayList certList = new ArrayList(certPathList); + loc[certList] = (*env)->NewObject(env, g_ArrayListClass, g_ArrayListCtorWithCollection, loc[certPathList]); + + // Certificate trustedCert = trustAnchor.getTrustedCert(); + // certList.add(trustedCert); + loc[trustedCert] = (*env)->CallObjectMethod(env, trustAnchor, g_TrustAnchorGetTrustedCert); + (*env)->CallBooleanMethod(env, loc[certList], g_ArrayListAdd, loc[trustedCert]); + + // CertificateFactory certFactory = CertificateFactory.getInstance("X.509"); + loc[certFactoryType] = JSTRING("X.509"); + loc[certFactory] = + (*env)->CallStaticObjectMethod(env, g_CertFactoryClass, g_CertFactoryGetInstance, loc[certFactoryType]); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); + + // CertPath certPathWithAnchor = certFactory.generateCertPath(certList); + jobject certPathWithAnchor = + (*env)->CallObjectMethod(env, loc[certFactory], g_CertFactoryGenerateCertPathFromList, loc[certList]); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); + + ret = certPathWithAnchor; + +cleanup: + RELEASE_LOCALS(loc, env) + return ret; +} + +int32_t AndroidCryptoNative_X509ChainValidate(X509ChainContext* ctx, + PAL_X509RevocationMode revocationMode, + PAL_X509RevocationFlag revocationFlag) +{ + assert(ctx != NULL); + JNIEnv* env = GetJNIEnv(); + + int32_t ret = FAIL; + INIT_LOCALS(loc, validatorType, validator, result, ex) + + bool checkRevocation = revocationMode != X509RevocationMode_NoCheck; + if (checkRevocation) + { + // bool isOnline = revocationMode == X509RevocationMode_Online; + if (AndroidCryptoNative_X509ChainSupportsRevocationOptions()) + { + // TODO: [AndroidCrypto] Deal with revocation options + goto cleanup; + } + else + { + if (revocationFlag == X509RevocationFlag_EndCertificateOnly) + { + goto cleanup; + } + + // Security.setProperty("oscp.enable", isOnline); + } + } + + // bool entireChain = checkRevocation && revocationFlag == X509RevocationFlag_EntireChain; + + jobject params = ctx->params; + jobject certPath = ctx->certPath; + + // params.setRevocationEnabled(checkRevocation); + (*env)->CallVoidMethod(env, params, g_PKIXBuilderParametersSetRevocationEnabled, checkRevocation); + + // String validatorType = "PKIX"; + // CertPathValidator validator = CertPathValidator.getInstance(validatorType); + // PKIXCertPathValidatorResult result = (PKIXCertPathValidatorResult)validator.validate(certPath, params); + loc[validatorType] = JSTRING("PKIX"); + loc[validator] = (*env)->CallStaticObjectMethod( + env, g_CertPathValidatorClass, g_CertPathValidatorGetInstance, loc[validatorType]); + loc[result] = (*env)->CallObjectMethod(env, loc[validator], g_CertPathValidatorValidate, certPath, params); + if (TryGetJNIException(env, &loc[ex], true /*printException*/)) + { + // TODO: [AndroidCrypto] Get/propagate the exception message to managed + // Exception should be CertPathValidatorException, which has: + // - getIndex() : index failed cert + // - getReason() - added in 24+ : reason for failure + goto cleanup; + } + + ret = SUCCESS; + +cleanup: + RELEASE_LOCALS(loc, env) + return ret; +} 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 new file mode 100644 index 0000000000000..841ab0b0ca591 --- /dev/null +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native.Android/pal_x509chain.h @@ -0,0 +1,78 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +#pragma once + +#include "pal_jni.h" + +typedef struct X509ChainContext_t X509ChainContext; + +/* +Create a context for building a certificate chain +*/ +PALEXPORT X509ChainContext* AndroidCryptoNative_X509ChainCreateContext(jobject /*X509Certificate*/ cert, + jobject* /*X509Certificate*/ extraStore, + int32_t extraStoreLen); + +/* +Destroy the context +*/ +PALEXPORT void AndroidCryptoNative_X509ChainDestroyContext(X509ChainContext* ctx); + +/* +Build a certificate path + +Always validates time and trust root. +*/ +PALEXPORT int32_t AndroidCryptoNative_X509ChainEvaluate(X509ChainContext* ctx, int64_t timeInMsFromUnixEpoch); + +/* +Return the number of certificates in the path +*/ +PALEXPORT int32_t AndroidCryptoNative_X509ChainGetCertificateCount(X509ChainContext* ctx); + +/* +Get the certificates in the path + +Returns 1 on success, 0 otherwise +*/ +PALEXPORT int32_t AndroidCryptoNative_X509ChainGetCertificates(X509ChainContext* ctx, + jobject* /*X509Certificate[]*/ certs, + int32_t certsLen); + +/* +Set the custom trust store +*/ +PALEXPORT int32_t AndroidCryptoNative_X509ChainSetCustomTrustStore(X509ChainContext* ctx, + jobject* /*X509Certificate[]*/ customTrustStore, + int32_t customTrustStoreLen); + +/* +Return whether or not revocation options are supported. +*/ +PALEXPORT bool AndroidCryptoNative_X509ChainSupportsRevocationOptions(void); + +// Matches managed X509RevocationMode enum +enum +{ + X509RevocationMode_NoCheck = 0, + X509RevocationMode_Online = 1, + X509RevocationMode_Offline = 2, +}; +typedef uint32_t PAL_X509RevocationMode; + +// Matches managed X509RevocationFlag enum +enum +{ + X509RevocationFlag_EndCertificateOnly = 0, + X509RevocationFlag_EntireChain = 1, + X509RevocationFlag_ExcludeRoot = 2, +}; +typedef uint32_t PAL_X509RevocationFlag; + +/* +Validate a certificate path. +*/ +PALEXPORT int32_t AndroidCryptoNative_X509ChainValidate(X509ChainContext* chain, + PAL_X509RevocationMode revocationMode, + PAL_X509RevocationFlag revocationFlag); diff --git a/src/libraries/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Android/ChainPal.cs b/src/libraries/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Android/ChainPal.cs index 00bfde848bd47..b56815dd19aac 100644 --- a/src/libraries/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Android/ChainPal.cs +++ b/src/libraries/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Android/ChainPal.cs @@ -7,6 +7,8 @@ using System.Security.Cryptography; using System.Security.Cryptography.X509Certificates; +using Microsoft.Win32.SafeHandles; + namespace Internal.Cryptography.Pal { internal sealed partial class ChainPal @@ -35,7 +37,229 @@ public static IChainPal BuildChain( TimeSpan timeout, bool disableAia) { - throw new NotImplementedException(nameof(BuildChain)); + var chainPal = new AndroidCertPath(); + try + { + chainPal.Initialize(cert, extraStore, customTrustStore, trustMode); + chainPal.Evaluate(verificationTime, applicationPolicy, certificatePolicy, revocationMode, revocationFlag); + } + catch + { + chainPal.Dispose(); + throw; + } + + return chainPal; + } + + private sealed class AndroidCertPath : IChainPal + { + public X509ChainElement[]? ChainElements { get; private set; } + public X509ChainStatus[]? ChainStatus { get; private set; } + + public SafeX509ChainHandle? SafeHandle => null; + + private SafeX509ChainContextHandle? _chainContext; + private bool _isValid; + + public void Dispose() + { + if (_chainContext != null) + { + _chainContext.Dispose(); + } + } + + public bool? Verify(X509VerificationFlags flags, out Exception? exception) + { + Debug.Assert(_chainContext != null); + exception = null; + + if (!_isValid) + { + if ((flags & X509VerificationFlags.IgnoreNotTimeValid) == X509VerificationFlags.IgnoreNotTimeValid) + { + // There is no way to bypass time validation on Android. + // It will not build any chain without a valid time. + exception = new PlatformNotSupportedException( + SR.Format(SR.Cryptography_VerificationFlagNotSupported, X509VerificationFlags.IgnoreNotTimeValid)); + return default(bool?); + } + + if ((flags & X509VerificationFlags.AllowUnknownCertificateAuthority) == X509VerificationFlags.AllowUnknownCertificateAuthority) + { + // There is no way to allow an untrusted root on Android. + // It will not build any chain without a trusted root. + exception = new PlatformNotSupportedException( + SR.Format(SR.Cryptography_VerificationFlagNotSupported, X509VerificationFlags.AllowUnknownCertificateAuthority)); + return default(bool?); + } + + return false; + } + + return ChainVerifier.Verify(ChainElements!, flags); + } + + internal void Initialize( + ICertificatePal cert, + X509Certificate2Collection? extraStore, + X509Certificate2Collection customTrustStore, + X509ChainTrustMode trustMode) + { + List extraCerts = new List() { cert.Handle }; + if (extraStore != null) + { + foreach (X509Certificate2 extraCert in extraStore) + { + extraCerts.Add(extraCert.Pal.Handle); + } + } + + List customTrustCerts = new List(); + bool useCustomRootTrust = trustMode == X509ChainTrustMode.CustomRootTrust; + if (useCustomRootTrust && customTrustStore != null) + { + foreach (X509Certificate2 custom in customTrustStore) + { + IntPtr certHandle = custom.Pal.Handle; + if (custom.SubjectName.RawData.ContentsEqual(custom.IssuerName.RawData)) + { + // Add self-issued certs to custom root trust cert + customTrustCerts.Add(certHandle); + } + else + { + // Add non-self-issued certs to extra certs + extraCerts.Add(certHandle); + } + } + } + + IntPtr[] extraArray = extraCerts.ToArray(); + _chainContext = Interop.AndroidCrypto.X509ChainCreateContext( + ((AndroidCertificatePal)cert).SafeHandle, + extraArray, + extraArray.Length); + + if (useCustomRootTrust) + { + IntPtr[] customTrustArray = customTrustCerts.ToArray(); + Interop.AndroidCrypto.X509ChainSetCustomTrustStore(_chainContext, customTrustArray, customTrustArray.Length); + } + } + + internal void Evaluate( + DateTime verificationTime, + OidCollection applicationPolicy, + OidCollection certificatePolicy, + X509RevocationMode revocationMode, + X509RevocationFlag revocationFlag) + { + Debug.Assert(_chainContext != null); + + long timeInMsFromUnixEpoch = new DateTimeOffset(verificationTime).ToUnixTimeMilliseconds(); + _isValid = Interop.AndroidCrypto.X509ChainEvaluate(_chainContext, timeInMsFromUnixEpoch); + if (!_isValid) + { + ChainElements = Array.Empty(); + var status = new X509ChainStatus() + { + Status = X509ChainStatusFlags.PartialChain + }; + ChainStatus = new X509ChainStatus[] { status }; + return; + } + + (X509Certificate2, List)[] results = GetValidationResults(_chainContext, applicationPolicy, certificatePolicy, revocationMode, revocationFlag); + + if (!IsPolicyMatch(results, applicationPolicy, certificatePolicy)) + { + X509ChainStatus policyFailStatus = new X509ChainStatus + { + Status = X509ChainStatusFlags.NotValidForUsage, + StatusInformation = SR.Chain_NoPolicyMatch, + }; + + for (int i = 0; i < results.Length; i++) + { + results[i].Item2.Add(policyFailStatus); + } + } + + X509ChainElement[] elements = new X509ChainElement[results.Length]; + for (int i = 0; i < results.Length; i++) + { + X509Certificate2 cert = results[i].Item1; + elements[i] = new X509ChainElement(cert, results[i].Item2.ToArray(), string.Empty); + } + + ChainElements = elements; + } + + private static (X509Certificate2, List)[] GetValidationResults( + SafeX509ChainContextHandle ctx, + OidCollection applicationPolicy, + OidCollection certificatePolicy, + X509RevocationMode revocationMode, + X509RevocationFlag revocationFlag) + { + IntPtr[] certPtrs = Interop.AndroidCrypto.X509ChainGetCertificates(ctx); + var results = new (X509Certificate2, List)[certPtrs.Length]; + for (int i = 0; i < results.Length; i++) + { + results[i].Item1 = new X509Certificate2(certPtrs[i]); + results[i].Item2 = new List(); + } + + int success = Interop.AndroidCrypto.X509ChainValidate(ctx, revocationMode, revocationFlag); + if (success != 1) + { + // TODO: [AndroidCrypto] Get actual validation errors + X509ChainStatus status = new X509ChainStatus + { + Status = X509ChainStatusFlags.PartialChain, + }; + + for (int i = 0; i < results.Length; i++) + { + results[i].Item2.Add(status); + } + } + + return results; + } + + private static bool IsPolicyMatch( + (X509Certificate2, List)[] results, + OidCollection? applicationPolicy, + OidCollection? certificatePolicy) + { + bool hasApplicationPolicy = applicationPolicy != null && applicationPolicy.Count > 0; + bool hasCertificatePolicy = certificatePolicy != null && certificatePolicy.Count > 0; + + if (!hasApplicationPolicy && !hasCertificatePolicy) + return true; + + List certsToRead = new List(results.Length); + for (int i = 0; i < results.Length; i++) + { + certsToRead.Add(results[i].Item1); + } + + CertificatePolicyChain policyChain = new CertificatePolicyChain(certsToRead); + if (hasCertificatePolicy && !policyChain.MatchesCertificatePolicies(certificatePolicy!)) + { + return false; + } + + if (hasApplicationPolicy && !policyChain.MatchesApplicationPolicies(applicationPolicy!)) + { + return false; + } + + return true; + } } } } diff --git a/src/libraries/System.Security.Cryptography.X509Certificates/src/Resources/Strings.resx b/src/libraries/System.Security.Cryptography.X509Certificates/src/Resources/Strings.resx index 6d619e428aa0b..29d32ea044b3e 100644 --- a/src/libraries/System.Security.Cryptography.X509Certificates/src/Resources/Strings.resx +++ b/src/libraries/System.Security.Cryptography.X509Certificates/src/Resources/Strings.resx @@ -448,4 +448,7 @@ The certificate content type could not be determined. + + The verification flag '{0}' is not supported on this platform. + diff --git a/src/libraries/System.Security.Cryptography.X509Certificates/src/System.Security.Cryptography.X509Certificates.csproj b/src/libraries/System.Security.Cryptography.X509Certificates/src/System.Security.Cryptography.X509Certificates.csproj index d654599322d2b..5d962f9dff329 100644 --- a/src/libraries/System.Security.Cryptography.X509Certificates/src/System.Security.Cryptography.X509Certificates.csproj +++ b/src/libraries/System.Security.Cryptography.X509Certificates/src/System.Security.Cryptography.X509Certificates.csproj @@ -422,6 +422,8 @@ Link="Common\Interop\Android\System.Security.Cryptography.Native.Android\Interop.Rsa.cs" /> + Date: Wed, 10 Mar 2021 09:49:24 -0800 Subject: [PATCH 02/19] Propagate error messages and set custom trust root certs --- .../Interop.X509Chain.cs | 46 ++++- .../pal_jni.c | 31 ++++ .../pal_jni.h | 17 ++ .../pal_x509chain.c | 157 ++++++++++++++++-- .../pal_x509chain.h | 17 +- .../Cryptography/Pal.Android/ChainPal.cs | 144 +++++++++++----- .../src/Resources/Strings.resx | 6 + 7 files changed, 358 insertions(+), 60 deletions(-) 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 d88827f36b26e..b95e0bdeeaccb 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 @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; +using System.Collections.Generic; using System.Runtime.InteropServices; using System.Security.Cryptography; using System.Security.Cryptography.X509Certificates; @@ -19,9 +20,9 @@ internal static extern SafeX509ChainContextHandle X509ChainCreateContext( [DllImport(Libraries.CryptoNative, EntryPoint = "AndroidCryptoNative_X509ChainDestroyContext")] internal static extern void X509ChainDestroyContext(IntPtr ctx); - [DllImport(Libraries.CryptoNative, EntryPoint = "AndroidCryptoNative_X509ChainEvaluate")] + [DllImport(Libraries.CryptoNative, EntryPoint = "AndroidCryptoNative_X509ChainBuild")] [return: MarshalAs(UnmanagedType.U1)] - internal static extern bool X509ChainEvaluate( + internal static extern bool X509ChainBuild( SafeX509ChainContextHandle ctx, long timeInMsFromUnixEpoch); @@ -34,7 +35,7 @@ private static extern int X509ChainGetCertificates( IntPtr[] certs, int certsLen); - internal static IntPtr[] X509ChainGetCertificates(SafeX509ChainContextHandle ctx) + internal static X509Certificate2[] X509ChainGetCertificates(SafeX509ChainContextHandle ctx) { int count = Interop.AndroidCrypto.X509ChainGetCertificateCount(ctx); var certPtrs = new IntPtr[count]; @@ -43,7 +44,44 @@ internal static IntPtr[] X509ChainGetCertificates(SafeX509ChainContextHandle ctx if (res != SUCCESS) throw new CryptographicException(); - return certPtrs; + var certs = new X509Certificate2[certPtrs.Length]; + for (int i = 0; i < certs.Length; i++) + { + certs[i] = new X509Certificate2(certPtrs[i]); + } + + return certs; + } + + [StructLayout(LayoutKind.Sequential)] + internal struct ValidationError + { + public IntPtr Message; // UTF-16 string + public int Index; + public int Status; + } + + [DllImport(Libraries.CryptoNative, EntryPoint = "AndroidCryptoNative_X509ChainGetErrorCount")] + private static extern int X509ChainGetErrorCount(SafeX509ChainContextHandle ctx); + + [DllImport(Libraries.CryptoNative, EntryPoint = "AndroidCryptoNative_X509ChainGetErrors")] + private static unsafe extern int X509ChainGetErrors( + SafeX509ChainContextHandle ctx, + [Out] ValidationError[] errors, + int errorsLen); + + internal static ValidationError[] X509ChainGetErrors(SafeX509ChainContextHandle ctx) + { + int count = Interop.AndroidCrypto.X509ChainGetErrorCount(ctx); + if (count == 0) + return Array.Empty(); + + var errors = new ValidationError[count]; + int res = Interop.AndroidCrypto.X509ChainGetErrors(ctx, errors, errors.Length); + if (res != SUCCESS) + throw new CryptographicException(); + + return errors; } [DllImport(Libraries.CryptoNative, EntryPoint = "AndroidCryptoNative_X509ChainSetCustomTrustStore")] 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 52632a29a2e65..927d842c2761e 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 @@ -14,6 +14,11 @@ jmethodID g_ByteArrayInputStreamReset; jclass g_Enum; jmethodID g_EnumOrdinal; +// java/lang/Throwable +jclass g_ThrowableClass; +jmethodID g_ThrowableGetCause; +jmethodID g_ThrowableGetMessage; + // java/security/Key jclass g_KeyClass; jmethodID g_KeyGetAlgorithm; @@ -145,6 +150,11 @@ jmethodID g_CertPathValidatorGetInstance; jmethodID g_CertPathValidatorValidate; jmethodID g_CertPathValidatorGetRevocationChecker; // only in API level 24+ +// java/security/cert/CertPathValidatorException +jclass g_CertPathValidatorExceptionClass; +jmethodID g_CertPathValidatorExceptionGetIndex; +jmethodID g_CertPathValidatorExceptionGetReason; // only in API level 24+ + // java/security/cert/CertStore jclass g_CertStoreClass; jmethodID g_CertStoreGetInstance; @@ -173,6 +183,7 @@ jmethodID g_PKIXRevocationCheckerSetOptions; // java/security/cert/TrustAnchor jclass g_TrustAnchorClass; +jclass g_TrustAnchorCtor; jmethodID g_TrustAnchorGetTrustedCert; // java/security/cert/X509Certificate @@ -294,6 +305,7 @@ jmethodID g_destroy; // java/util/ArrayList jclass g_ArrayListClass; +jmethodID g_ArrayListCtor; jmethodID g_ArrayListCtorWithCapacity; jmethodID g_ArrayListCtorWithCollection; jmethodID g_ArrayListAdd; @@ -308,6 +320,11 @@ jclass g_DateClass; jmethodID g_DateCtor; jmethodID g_DateGetTime; +// java/util/HashSet +jclass g_HashSetClass; +jmethodID g_HashSetCtorWithCapacity; +jmethodID g_HashSetAdd; + // java/util/Iterator jclass g_IteratorClass; jmethodID g_IteratorHasNext; @@ -524,6 +541,10 @@ JNI_OnLoad(JavaVM *vm, void *reserved) g_Enum = GetClassGRef(env, "java/lang/Enum"); g_EnumOrdinal = GetMethod(env, false, g_Enum, "ordinal", "()I"); + g_ThrowableClass = GetClassGRef(env, "java/lang/Throwable"); + g_ThrowableGetCause = GetMethod(env, false, g_ThrowableClass, "getCause", "()Ljava/lang/Throwable;"); + g_ThrowableGetMessage = GetMethod(env, false, g_ThrowableClass, "getMessage", "()Ljava/lang/String;"); + g_KeyClass = GetClassGRef(env, "java/security/Key"); g_KeyGetAlgorithm = GetMethod(env, false, g_KeyClass, "getAlgorithm", "()Ljava/lang/String;"); g_KeyGetEncoded = GetMethod(env, false, g_KeyClass, "getEncoded", "()[B"); @@ -605,6 +626,10 @@ JNI_OnLoad(JavaVM *vm, void *reserved) g_CertPathValidatorValidate = GetMethod(env, false, g_CertPathValidatorClass, "validate", "(Ljava/security/cert/CertPath;Ljava/security/cert/CertPathParameters;)Ljava/security/cert/CertPathValidatorResult;"); g_CertPathValidatorGetRevocationChecker = GetOptionalMethod(env, false, g_CertPathValidatorClass, "getRevocationChecker", "()Ljava/security/cert/CertPathChecker;"); + g_CertPathValidatorExceptionClass = GetClassGRef(env, "java/security/cert/CertPathValidatorException"); + g_CertPathValidatorExceptionGetIndex = GetMethod(env, false, g_CertPathValidatorExceptionClass, "getIndex", "()I"); + g_CertPathValidatorExceptionGetReason = GetOptionalMethod(env, false, g_CertPathValidatorExceptionClass, "getReason", "()Ljava/security/cert/CertPathValidatorException$Reason;"); + g_CertStoreClass = GetClassGRef(env, "java/security/cert/CertStore"); g_CertStoreGetInstance = GetMethod(env, true, g_CertStoreClass, "getInstance", "(Ljava/lang/String;Ljava/security/cert/CertStoreParameters;)Ljava/security/cert/CertStore;"); @@ -630,6 +655,7 @@ JNI_OnLoad(JavaVM *vm, void *reserved) } g_TrustAnchorClass = GetClassGRef(env, "java/security/cert/TrustAnchor"); + g_TrustAnchorCtor = GetMethod(env, false, g_TrustAnchorClass, "", "(Ljava/security/cert/X509Certificate;[B)V"); g_TrustAnchorGetTrustedCert = GetMethod(env, false, g_TrustAnchorClass, "getTrustedCert", "()Ljava/security/cert/X509Certificate;"); g_X509CertClass = GetClassGRef(env, "java/security/cert/X509Certificate"); @@ -758,6 +784,7 @@ JNI_OnLoad(JavaVM *vm, void *reserved) g_destroy = GetMethod(env, false, g_DestroyableClass, "destroy", "()V"); g_ArrayListClass = GetClassGRef(env, "java/util/ArrayList"); + g_ArrayListCtor = GetMethod(env, false, g_ArrayListClass, "", "()V"); g_ArrayListCtorWithCapacity = GetMethod(env, false, g_ArrayListClass, "", "(I)V"); g_ArrayListCtorWithCollection = GetMethod(env, false, g_ArrayListClass, "", "(Ljava/util/Collection;)V"); g_ArrayListAdd = GetMethod(env, false, g_ArrayListClass, "add", "(Ljava/lang/Object;)Z"); @@ -770,6 +797,10 @@ JNI_OnLoad(JavaVM *vm, void *reserved) g_DateCtor = GetMethod(env, false, g_DateClass, "", "(J)V"); g_DateGetTime = GetMethod(env, false, g_DateClass, "getTime", "()J"); + g_HashSetClass = GetClassGRef(env, "java/util/HashSet"); + g_HashSetCtorWithCapacity = GetMethod(env, false, g_HashSetClass, "", "(I)V"); + g_HashSetAdd = GetMethod(env, false, g_HashSetClass, "add", "(Ljava/lang/Object;)Z"); + g_IteratorClass = GetClassGRef(env, "java/util/Iterator"); g_IteratorHasNext = GetMethod(env, false, g_IteratorClass, "hasNext", "()Z"); g_IteratorNext = GetMethod(env, false, g_IteratorClass, "next", "()Ljava/lang/Object;"); diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native.Android/pal_jni.h b/src/libraries/Native/Unix/System.Security.Cryptography.Native.Android/pal_jni.h index 731f9b555cc14..0557bccfe075e 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native.Android/pal_jni.h +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native.Android/pal_jni.h @@ -23,6 +23,11 @@ extern jmethodID g_ByteArrayInputStreamReset; extern jclass g_Enum; extern jmethodID g_EnumOrdinal; +// java/lang/Throwable +extern jclass g_ThrowableClass; +extern jmethodID g_ThrowableGetCause; +extern jmethodID g_ThrowableGetMessage; + // java/security/Key extern jclass g_KeyClass; extern jmethodID g_KeyGetAlgorithm; @@ -119,6 +124,11 @@ extern jmethodID g_CertPathValidatorGetInstance; extern jmethodID g_CertPathValidatorValidate; extern jmethodID g_CertPathValidatorGetRevocationChecker; // only in API level 24+ +// java/security/cert/CertPathValidatorException +extern jclass g_CertPathValidatorExceptionClass; +extern jmethodID g_CertPathValidatorExceptionGetIndex; +extern jmethodID g_CertPathValidatorExceptionGetReason; + // java/security/cert/CertStore extern jclass g_CertStoreClass; extern jmethodID g_CertStoreGetInstance; @@ -147,6 +157,7 @@ extern jmethodID g_PKIXRevocationCheckerSetOptions; // java/security/cert/TrustAnchor extern jclass g_TrustAnchorClass; +extern jclass g_TrustAnchorCtor; extern jmethodID g_TrustAnchorGetTrustedCert; // java/security/cert/X509Certificate @@ -303,6 +314,7 @@ extern jmethodID g_destroy; // java/util/ArrayList extern jclass g_ArrayListClass; +extern jmethodID g_ArrayListCtor; extern jmethodID g_ArrayListCtorWithCapacity; extern jmethodID g_ArrayListCtorWithCollection; extern jmethodID g_ArrayListAdd; @@ -317,6 +329,11 @@ extern jclass g_DateClass; extern jmethodID g_DateCtor; extern jmethodID g_DateGetTime; +// java/util/HashSet +extern jclass g_HashSetClass; +extern jmethodID g_HashSetCtorWithCapacity; +extern jmethodID g_HashSetAdd; + // java/util/Iterator extern jclass g_IteratorClass; extern jmethodID g_IteratorHasNext; 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 a2f3448516090..2cb032104abf1 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 @@ -26,6 +26,46 @@ struct X509ChainContext_t jobject /*PKIXBuilderParameters*/ params; jobject /*CertPath*/ certPath; jobject /*TrustAnchor*/ trustAnchor; + + jobject /*ArrayList*/ errorList; +}; + +enum +{ + PAL_X509ChainNoError = 0, + PAL_X509ChainNotTimeValid = 0x00000001, + PAL_X509ChainNotTimeNested = 0x00000002, + PAL_X509ChainRevoked = 0x00000004, + PAL_X509ChainNotSignatureValid = 0x00000008, + PAL_X509ChainNotValidForUsage = 0x00000010, + PAL_X509ChainUntrustedRoot = 0x00000020, + PAL_X509ChainRevocationStatusUnknown = 0x00000040, + PAL_X509ChainCyclic = 0x00000080, + PAL_X509ChainInvalidExtension = 0x00000100, + PAL_X509ChainInvalidPolicyConstraints = 0x00000200, + PAL_X509ChainInvalidBasicConstraints = 0x00000400, + PAL_X509ChainInvalidNameConstraints = 0x00000800, + PAL_X509ChainHasNotSupportedNameConstraint = 0x00001000, + PAL_X509ChainHasNotDefinedNameConstraint = 0x00002000, + PAL_X509ChainHasNotPermittedNameConstraint = 0x00004000, + PAL_X509ChainHasExcludedNameConstraint = 0x00008000, + PAL_X509ChainPartialChain = 0x00010000, + PAL_X509ChainCtlNotTimeValid = 0x00020000, + PAL_X509ChainCtlNotSignatureValid = 0x00040000, + PAL_X509ChainCtlNotValidForUsage = 0x00080000, + PAL_X509ChainOfflineRevocation = 0x01000000, + PAL_X509ChainNoIssuanceChainPolicy = 0x02000000, + PAL_X509ChainExplicitDistrust = 0x04000000, + PAL_X509ChainHasNotSupportedCriticalExtension = 0x08000000, + PAL_X509ChainHasWeakSignature = 0x00100000, +}; +typedef uint32_t PAL_X509ChainStatusFlags; + +struct ValidationError_t +{ + uint16_t* message; + int index; + PAL_X509ChainStatusFlags chainStatus; }; X509ChainContext* AndroidCryptoNative_X509ChainCreateContext(jobject /*X509Certificate*/ cert, @@ -86,6 +126,7 @@ X509ChainContext* AndroidCryptoNative_X509ChainCreateContext(jobject /*X509Certi ret = malloc(sizeof(X509ChainContext)); memset(ret, 0, sizeof(X509ChainContext)); ret->params = AddGRef(env, loc[params]); + ret->errorList = ToGRef(env, (*env)->NewObject(env, g_ArrayListClass, g_ArrayListCtor)); cleanup: RELEASE_LOCALS(loc, env) @@ -101,10 +142,11 @@ void AndroidCryptoNative_X509ChainDestroyContext(X509ChainContext* ctx) ReleaseGRef(env, ctx->params); ReleaseGRef(env, ctx->certPath); ReleaseGRef(env, ctx->trustAnchor); + ReleaseGRef(env, ctx->errorList); free(ctx); } -int32_t AndroidCryptoNative_X509ChainEvaluate(X509ChainContext* ctx, int64_t timeInMsFromUnixEpoch) +int32_t AndroidCryptoNative_X509ChainBuild(X509ChainContext* ctx, int64_t timeInMsFromUnixEpoch) { assert(ctx != NULL); JNIEnv* env = GetJNIEnv(); @@ -133,7 +175,7 @@ int32_t AndroidCryptoNative_X509ChainEvaluate(X509ChainContext* ctx, int64_t tim loc[result] = (*env)->CallObjectMethod(env, loc[builder], g_CertPathBuilderBuild, params); if (TryGetJNIException(env, &loc[ex], true /*printException*/)) { - // TODO: [AndroidCrypto] Get/propagate the exception message to managed + (*env)->CallBooleanMethod(env, ctx->errorList, g_ArrayListAdd, loc[ex]); goto cleanup; } @@ -212,17 +254,115 @@ int32_t AndroidCryptoNative_X509ChainGetCertificates(X509ChainContext* ctx, return ret; } +int32_t AndroidCryptoNative_X509ChainGetErrorCount(X509ChainContext* ctx) +{ + assert(ctx != NULL); + JNIEnv* env = GetJNIEnv(); + int32_t count = (*env)->CallIntMethod(env, ctx->errorList, g_CollectionSize); + return count; +} + +static PAL_X509ChainStatusFlags ChainStatusFromValidatorExceptionReason(jobject reason) +{ + // TODO: [AndroidCrypto] Convert reason to chain status + return PAL_X509ChainNoError; +} + +static void PopulateValidationError(JNIEnv* env, jobject error, ValidationError* out) +{ + int index = -1; + PAL_X509ChainStatusFlags chainStatus = PAL_X509ChainNoError; + if ((*env)->IsInstanceOf(env, error, g_CertPathValidatorExceptionClass)) + { + index = (*env)->CallIntMethod(env, error, g_CertPathValidatorExceptionGetIndex); + + // Get the reason (if the API is available) and convert it to a chain status flag + if (g_CertPathValidatorExceptionGetReason != NULL) + { + jobject reason = (*env)->CallObjectMethod(env, error, g_CertPathValidatorExceptionGetReason); + chainStatus = ChainStatusFromValidatorExceptionReason(reason); + (*env)->DeleteLocalRef(env, reason); + } + } + + jobject message = (*env)->CallObjectMethod(env, error, g_ThrowableGetMessage); + jsize messageLen = (*env)->GetStringLength(env, message); + + // +1 for null terminator + uint16_t* messagePtr = malloc(sizeof(uint16_t) * (size_t)(messageLen + 1)); + messagePtr[messageLen] = '\0'; + (*env)->GetStringRegion(env, message, 0, messageLen, (jchar*)messagePtr); + + out->message = messagePtr; + out->index = index; + out->chainStatus = chainStatus; + + (*env)->DeleteLocalRef(env, message); +} + +int32_t AndroidCryptoNative_X509ChainGetErrors(X509ChainContext* ctx, + ValidationError* errors, + int32_t errorsLen) +{ + assert(ctx != NULL); + JNIEnv* env = GetJNIEnv(); + + int32_t ret = FAIL; + + // int i = 0; + // Iterator iter = errorList.iterator(); + // while (iter.hasNext()) { + // Throwable error = iter.next(); + // << populate errors[i] >> + // i++; + // } + int32_t i = 0; + jobject iter = (*env)->CallObjectMethod(env, ctx->errorList, g_CollectionIterator); + jboolean hasNext = (*env)->CallBooleanMethod(env, iter, g_IteratorHasNext); + while (hasNext) + { + jobject error = (*env)->CallObjectMethod(env, iter, g_IteratorNext); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); + + PopulateValidationError(env, error, &errors[i]); + i++; + + hasNext = (*env)->CallBooleanMethod(env, iter, g_IteratorHasNext); + (*env)->DeleteLocalRef(env, error); + } + + ret = SUCCESS; + +cleanup: + (*env)->DeleteLocalRef(env, iter); + return ret; +} + int32_t AndroidCryptoNative_X509ChainSetCustomTrustStore(X509ChainContext* ctx, jobject* /*X509Certificate*/ customTrustStore, int32_t customTrustStoreLen) { - // HashSet trustAnchors = new HashSet(); + assert(ctx != NULL); + JNIEnv* env = GetJNIEnv(); + + // HashSet anchors = new HashSet(customTrustStoreLen); // for (Certificate cert : customTrustStore) { // TrustAnchor anchor = new TrustAnchor(cert, null); - // trustAnchors.Add(anchor); + // anchors.Add(anchor); // } - // params.setTrustAnchors(trustAnchors); - return FAIL; + jobject anchors = (*env)->NewObject(env, g_HashSetClass, g_HashSetCtorWithCapacity, customTrustStoreLen); + for (int i = 0; i < customTrustStoreLen; ++i) + { + jobject anchor = (*env)->NewObject(env, g_TrustAnchorClass, g_TrustAnchorCtor, customTrustStore[i], NULL); + (*env)->CallBooleanMethod(env, anchors, g_HashSetAdd, anchor); + (*env)->DeleteLocalRef(env, anchor); + } + + // params.setTrustAnchors(anchors); + (*env)->CallVoidMethod(env, ctx->params, g_PKIXBuilderParametersSetTrustAnchors, anchors); + + (*env)->DeleteLocalRef(env, anchors); + return CheckJNIExceptions(env) ? FAIL : SUCCESS; } bool AndroidCryptoNative_X509ChainSupportsRevocationOptions(void) @@ -313,10 +453,7 @@ int32_t AndroidCryptoNative_X509ChainValidate(X509ChainContext* ctx, loc[result] = (*env)->CallObjectMethod(env, loc[validator], g_CertPathValidatorValidate, certPath, params); if (TryGetJNIException(env, &loc[ex], true /*printException*/)) { - // TODO: [AndroidCrypto] Get/propagate the exception message to managed - // Exception should be CertPathValidatorException, which has: - // - getIndex() : index failed cert - // - getReason() - added in 24+ : reason for failure + (*env)->CallBooleanMethod(env, ctx->errorList, g_ArrayListAdd, loc[ex]); goto cleanup; } 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 841ab0b0ca591..613f5355b7d76 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 @@ -6,6 +6,7 @@ #include "pal_jni.h" typedef struct X509ChainContext_t X509ChainContext; +typedef struct ValidationError_t ValidationError; /* Create a context for building a certificate chain @@ -24,7 +25,7 @@ Build a certificate path Always validates time and trust root. */ -PALEXPORT int32_t AndroidCryptoNative_X509ChainEvaluate(X509ChainContext* ctx, int64_t timeInMsFromUnixEpoch); +PALEXPORT int32_t AndroidCryptoNative_X509ChainBuild(X509ChainContext* ctx, int64_t timeInMsFromUnixEpoch); /* Return the number of certificates in the path @@ -40,6 +41,20 @@ PALEXPORT int32_t AndroidCryptoNative_X509ChainGetCertificates(X509ChainContext* jobject* /*X509Certificate[]*/ certs, int32_t certsLen); +/* +Return the number of errors encountered when building and validating the certificate path +*/ +PALEXPORT int32_t AndroidCryptoNative_X509ChainGetErrorCount(X509ChainContext* ctx); + +/* +Get the errors + +Returns 1 on success, 0 otherwise +*/ +PALEXPORT int32_t AndroidCryptoNative_X509ChainGetErrors(X509ChainContext* ctx, + ValidationError* errors, + int32_t errorsLen); + /* Set the custom trust store */ diff --git a/src/libraries/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Android/ChainPal.cs b/src/libraries/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Android/ChainPal.cs index b56815dd19aac..38d11af686a4b 100644 --- a/src/libraries/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Android/ChainPal.cs +++ b/src/libraries/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Android/ChainPal.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using System.Diagnostics; +using System.Runtime.InteropServices; using System.Security.Cryptography; using System.Security.Cryptography.X509Certificates; @@ -144,8 +145,18 @@ internal void Initialize( if (useCustomRootTrust) { + // Android does not support an empty set of trust anchors + if (customTrustCerts.Count == 0) + { + throw new PlatformNotSupportedException(SR.Cryptography_EmptyCustomTrustNotSupported); + } + IntPtr[] customTrustArray = customTrustCerts.ToArray(); - Interop.AndroidCrypto.X509ChainSetCustomTrustStore(_chainContext, customTrustArray, customTrustArray.Length); + int res = Interop.AndroidCrypto.X509ChainSetCustomTrustStore(_chainContext, customTrustArray, customTrustArray.Length); + if (res != 1) + { + throw new CryptographicException(); + } } } @@ -159,21 +170,56 @@ internal void Evaluate( Debug.Assert(_chainContext != null); long timeInMsFromUnixEpoch = new DateTimeOffset(verificationTime).ToUnixTimeMilliseconds(); - _isValid = Interop.AndroidCrypto.X509ChainEvaluate(_chainContext, timeInMsFromUnixEpoch); + _isValid = Interop.AndroidCrypto.X509ChainBuild(_chainContext, timeInMsFromUnixEpoch); if (!_isValid) { + // Android always validates time, signature, and trusted root. + // There is no way bypass that validation and build a path. ChainElements = Array.Empty(); - var status = new X509ChainStatus() + + Interop.AndroidCrypto.ValidationError[] errors = Interop.AndroidCrypto.X509ChainGetErrors(_chainContext); + var chainStatus = new X509ChainStatus[errors.Length]; + for (int i = 0; i < errors.Length; i++) { - Status = X509ChainStatusFlags.PartialChain - }; - ChainStatus = new X509ChainStatus[] { status }; + Interop.AndroidCrypto.ValidationError error = errors[i]; + chainStatus[i] = ValidationErrorToChainStatus(error); + Marshal.FreeHGlobal(error.Message); + } + + ChainStatus = chainStatus; return; } - (X509Certificate2, List)[] results = GetValidationResults(_chainContext, applicationPolicy, certificatePolicy, revocationMode, revocationFlag); + if (revocationMode != X509RevocationMode.NoCheck && !Interop.AndroidCrypto.X509ChainSupportsRevocationOptions()) + { + if (revocationFlag == X509RevocationFlag.EndCertificateOnly) + { + throw new PlatformNotSupportedException(SR.Format(SR.Cryptography_ChainSettingNotSupported, $"{nameof(X509RevocationFlag)}.{nameof(X509RevocationFlag.EndCertificateOnly)}")); + } + } + + int res = Interop.AndroidCrypto.X509ChainValidate(_chainContext, revocationMode, revocationFlag); + if (res != 1) + throw new CryptographicException(); - if (!IsPolicyMatch(results, applicationPolicy, certificatePolicy)) + X509Certificate2[] certs = Interop.AndroidCrypto.X509ChainGetCertificates(_chainContext); + List overallStatus = new List(); + List[] statuses = new List[certs.Length]; + Dictionary> errorsByIndex = GetStatusByIndex(_chainContext); + foreach (int index in errorsByIndex.Keys) + { + if (index == -1) + { + // Error is not tied to a specific index + overallStatus.AddRange(errorsByIndex[index]); + } + else + { + statuses[index] = errorsByIndex[index]; + } + } + + if (!IsPolicyMatch(certs, applicationPolicy, certificatePolicy)) { X509ChainStatus policyFailStatus = new X509ChainStatus { @@ -181,57 +227,70 @@ internal void Evaluate( StatusInformation = SR.Chain_NoPolicyMatch, }; - for (int i = 0; i < results.Length; i++) + for (int i = 0; i < statuses.Length; i++) { - results[i].Item2.Add(policyFailStatus); + if (statuses[i] == null) + { + statuses[i] = new List(); + } + + statuses[i].Add(policyFailStatus); } + + overallStatus.Add(policyFailStatus); } - X509ChainElement[] elements = new X509ChainElement[results.Length]; - for (int i = 0; i < results.Length; i++) + X509ChainElement[] elements = new X509ChainElement[certs.Length]; + for (int i = 0; i < certs.Length; i++) { - X509Certificate2 cert = results[i].Item1; - elements[i] = new X509ChainElement(cert, results[i].Item2.ToArray(), string.Empty); + X509ChainStatus[] elementStatus = statuses[i] == null ? Array.Empty() : statuses[i].ToArray(); + elements[i] = new X509ChainElement(certs[i], elementStatus, string.Empty); } ChainElements = elements; + ChainStatus = overallStatus.ToArray(); } - private static (X509Certificate2, List)[] GetValidationResults( - SafeX509ChainContextHandle ctx, - OidCollection applicationPolicy, - OidCollection certificatePolicy, - X509RevocationMode revocationMode, - X509RevocationFlag revocationFlag) + private static Dictionary> GetStatusByIndex(SafeX509ChainContextHandle ctx) { - IntPtr[] certPtrs = Interop.AndroidCrypto.X509ChainGetCertificates(ctx); - var results = new (X509Certificate2, List)[certPtrs.Length]; - for (int i = 0; i < results.Length; i++) + var statusByIndex = new Dictionary>(); + Interop.AndroidCrypto.ValidationError[] errors = Interop.AndroidCrypto.X509ChainGetErrors(ctx); + for (int i = 0; i < errors.Length; i++) { - results[i].Item1 = new X509Certificate2(certPtrs[i]); - results[i].Item2 = new List(); - } - - int success = Interop.AndroidCrypto.X509ChainValidate(ctx, revocationMode, revocationFlag); - if (success != 1) - { - // TODO: [AndroidCrypto] Get actual validation errors - X509ChainStatus status = new X509ChainStatus - { - Status = X509ChainStatusFlags.PartialChain, - }; + Interop.AndroidCrypto.ValidationError error = errors[i]; + X509ChainStatus chainStatus = ValidationErrorToChainStatus(error); + Marshal.FreeHGlobal(error.Message); - for (int i = 0; i < results.Length; i++) + if (!statusByIndex.ContainsKey(error.Index)) { - results[i].Item2.Add(status); + statusByIndex.Add(error.Index, new List()); } + + statusByIndex[error.Index].Add(chainStatus); } - return results; + return statusByIndex; + } + + private static X509ChainStatus ValidationErrorToChainStatus(Interop.AndroidCrypto.ValidationError error) + { + X509ChainStatusFlags statusFlags = (X509ChainStatusFlags)error.Status; + if (statusFlags == X509ChainStatusFlags.NoError) + { + // Android returns NoError as the error status when it cannot determine the status + // We just map that to partial chain. + statusFlags = X509ChainStatusFlags.PartialChain; + } + + return new X509ChainStatus + { + Status = statusFlags, + StatusInformation = Marshal.PtrToStringUni(error.Message) + }; } private static bool IsPolicyMatch( - (X509Certificate2, List)[] results, + X509Certificate2[] certs, OidCollection? applicationPolicy, OidCollection? certificatePolicy) { @@ -241,12 +300,7 @@ private static bool IsPolicyMatch( if (!hasApplicationPolicy && !hasCertificatePolicy) return true; - List certsToRead = new List(results.Length); - for (int i = 0; i < results.Length; i++) - { - certsToRead.Add(results[i].Item1); - } - + List certsToRead = new List(certs); CertificatePolicyChain policyChain = new CertificatePolicyChain(certsToRead); if (hasCertificatePolicy && !policyChain.MatchesCertificatePolicies(certificatePolicy!)) { diff --git a/src/libraries/System.Security.Cryptography.X509Certificates/src/Resources/Strings.resx b/src/libraries/System.Security.Cryptography.X509Certificates/src/Resources/Strings.resx index 29d32ea044b3e..d477de60b9ed5 100644 --- a/src/libraries/System.Security.Cryptography.X509Certificates/src/Resources/Strings.resx +++ b/src/libraries/System.Security.Cryptography.X509Certificates/src/Resources/Strings.resx @@ -448,6 +448,12 @@ The certificate content type could not be determined. + + The certificate chain setting '{0}' is not supported on this platform. + + + An empty custom trust store is not supported on this platform. + The verification flag '{0}' is not supported on this platform. From 16751c44e3abad82caa98bccc4828d70b755305b Mon Sep 17 00:00:00 2001 From: Elinor Fung Date: Thu, 11 Mar 2021 10:21:19 -0800 Subject: [PATCH 03/19] Share x509 enums --- .../Native/Unix/Common/pal_x509_types.h | 49 +++++++++++++++++++ .../pal_x509.c | 2 +- .../pal_x509.h | 13 +---- .../pal_x509chain.c | 32 +----------- .../pal_x509.h | 13 +---- .../pal_x509chain.h | 32 +----------- 6 files changed, 54 insertions(+), 87 deletions(-) create mode 100644 src/libraries/Native/Unix/Common/pal_x509_types.h diff --git a/src/libraries/Native/Unix/Common/pal_x509_types.h b/src/libraries/Native/Unix/Common/pal_x509_types.h new file mode 100644 index 0000000000000..47602dbbc1207 --- /dev/null +++ b/src/libraries/Native/Unix/Common/pal_x509_types.h @@ -0,0 +1,49 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +#pragma once + +// Matches managed X509ChainStatusFlags enum +enum +{ + PAL_X509ChainNoError = 0, + PAL_X509ChainNotTimeValid = 0x00000001, + PAL_X509ChainNotTimeNested = 0x00000002, + PAL_X509ChainRevoked = 0x00000004, + PAL_X509ChainNotSignatureValid = 0x00000008, + PAL_X509ChainNotValidForUsage = 0x00000010, + PAL_X509ChainUntrustedRoot = 0x00000020, + PAL_X509ChainRevocationStatusUnknown = 0x00000040, + PAL_X509ChainCyclic = 0x00000080, + PAL_X509ChainInvalidExtension = 0x00000100, + PAL_X509ChainInvalidPolicyConstraints = 0x00000200, + PAL_X509ChainInvalidBasicConstraints = 0x00000400, + PAL_X509ChainInvalidNameConstraints = 0x00000800, + PAL_X509ChainHasNotSupportedNameConstraint = 0x00001000, + PAL_X509ChainHasNotDefinedNameConstraint = 0x00002000, + PAL_X509ChainHasNotPermittedNameConstraint = 0x00004000, + PAL_X509ChainHasExcludedNameConstraint = 0x00008000, + PAL_X509ChainPartialChain = 0x00010000, + PAL_X509ChainCtlNotTimeValid = 0x00020000, + PAL_X509ChainCtlNotSignatureValid = 0x00040000, + PAL_X509ChainCtlNotValidForUsage = 0x00080000, + PAL_X509ChainOfflineRevocation = 0x01000000, + PAL_X509ChainNoIssuanceChainPolicy = 0x02000000, + PAL_X509ChainExplicitDistrust = 0x04000000, + PAL_X509ChainHasNotSupportedCriticalExtension = 0x08000000, + PAL_X509ChainHasWeakSignature = 0x00100000, +}; +typedef uint32_t PAL_X509ChainStatusFlags; + +// Matches managed X509ContentType enum +enum +{ + PAL_X509Unknown = 0, + PAL_Certificate = 1, + PAL_SerializedCert = 2, + PAL_Pkcs12 = 3, + PAL_SerializedStore = 4, + PAL_Pkcs7 = 5, + PAL_Authenticode = 6, +}; +typedef uint32_t PAL_X509ContentType; 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 d1679912bf086..17120c5883f62 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 @@ -204,7 +204,7 @@ PAL_X509ContentType AndroidCryptoNative_X509GetContentType(const uint8_t* buf, i PAL_X509ContentType ret = PAL_X509Unknown; INIT_LOCALS(loc, bytes, stream, certType, certFactory, pkcs7Type, certPath, cert) - // This functin checks: + // This function checks: // - PKCS7 DER/PEM // - X509 DER/PEM // The generateCertificate method used for the X509 DER/PEM check will succeed for some diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native.Android/pal_x509.h b/src/libraries/Native/Unix/System.Security.Cryptography.Native.Android/pal_x509.h index 476d0dedeaf35..53b267d4a694b 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native.Android/pal_x509.h +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native.Android/pal_x509.h @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. #include "pal_jni.h" +#include // Creation and lifetime PALEXPORT jobject /*X509Certificate*/ AndroidCryptoNative_X509Decode(const uint8_t* buf, int32_t len); @@ -36,18 +37,6 @@ PALEXPORT int32_t AndroidCryptoNative_X509ExportPkcs7(jobject* /*X509Certificate uint8_t* out, int32_t* outLen); -// Matches managed X509ContentType enum -enum -{ - PAL_X509Unknown = 0, - PAL_Certificate = 1, - PAL_SerializedCert = 2, - PAL_Pkcs12 = 3, - PAL_SerializedStore = 4, - PAL_Pkcs7 = 5, - PAL_Authenticode = 6, -}; -typedef uint32_t PAL_X509ContentType; PALEXPORT PAL_X509ContentType AndroidCryptoNative_X509GetContentType(const uint8_t* buf, int32_t len); // Matches managed PAL_KeyAlgorithm enum 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 2cb032104abf1..80236c230b108 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 @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. #include "pal_x509chain.h" +#include #include #include @@ -30,37 +31,6 @@ struct X509ChainContext_t jobject /*ArrayList*/ errorList; }; -enum -{ - PAL_X509ChainNoError = 0, - PAL_X509ChainNotTimeValid = 0x00000001, - PAL_X509ChainNotTimeNested = 0x00000002, - PAL_X509ChainRevoked = 0x00000004, - PAL_X509ChainNotSignatureValid = 0x00000008, - PAL_X509ChainNotValidForUsage = 0x00000010, - PAL_X509ChainUntrustedRoot = 0x00000020, - PAL_X509ChainRevocationStatusUnknown = 0x00000040, - PAL_X509ChainCyclic = 0x00000080, - PAL_X509ChainInvalidExtension = 0x00000100, - PAL_X509ChainInvalidPolicyConstraints = 0x00000200, - PAL_X509ChainInvalidBasicConstraints = 0x00000400, - PAL_X509ChainInvalidNameConstraints = 0x00000800, - PAL_X509ChainHasNotSupportedNameConstraint = 0x00001000, - PAL_X509ChainHasNotDefinedNameConstraint = 0x00002000, - PAL_X509ChainHasNotPermittedNameConstraint = 0x00004000, - PAL_X509ChainHasExcludedNameConstraint = 0x00008000, - PAL_X509ChainPartialChain = 0x00010000, - PAL_X509ChainCtlNotTimeValid = 0x00020000, - PAL_X509ChainCtlNotSignatureValid = 0x00040000, - PAL_X509ChainCtlNotValidForUsage = 0x00080000, - PAL_X509ChainOfflineRevocation = 0x01000000, - PAL_X509ChainNoIssuanceChainPolicy = 0x02000000, - PAL_X509ChainExplicitDistrust = 0x04000000, - PAL_X509ChainHasNotSupportedCriticalExtension = 0x08000000, - PAL_X509ChainHasWeakSignature = 0x00100000, -}; -typedef uint32_t PAL_X509ChainStatusFlags; - struct ValidationError_t { uint16_t* message; diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/pal_x509.h b/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/pal_x509.h index 067f54b099269..295688c0004d9 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/pal_x509.h +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/pal_x509.h @@ -6,21 +6,10 @@ #include "pal_digest.h" #include "pal_seckey.h" #include "pal_compiler.h" +#include #include -enum -{ - PAL_X509Unknown = 0, - PAL_Certificate = 1, - PAL_SerializedCert = 2, - PAL_Pkcs12 = 3, - PAL_SerializedStore = 4, - PAL_Pkcs7 = 5, - PAL_Authenticode = 6, -}; -typedef uint32_t PAL_X509ContentType; - /* Given a handle, determine if it represents a SecCertificateRef, SecIdentityRef, or other. If the handle is a certificate or identity it is CFRetain()ed (and must later be CFRelease()d). diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/pal_x509chain.h b/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/pal_x509chain.h index 6151db60e671b..e11bf3a821132 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/pal_x509chain.h +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/pal_x509chain.h @@ -6,40 +6,10 @@ #include "pal_digest.h" #include "pal_seckey.h" #include "pal_compiler.h" +#include #include -enum -{ - PAL_X509ChainNoError = 0, - PAL_X509ChainNotTimeValid = 0x00000001, - PAL_X509ChainNotTimeNested = 0x00000002, - PAL_X509ChainRevoked = 0x00000004, - PAL_X509ChainNotSignatureValid = 0x00000008, - PAL_X509ChainNotValidForUsage = 0x00000010, - PAL_X509ChainUntrustedRoot = 0x00000020, - PAL_X509ChainRevocationStatusUnknown = 0x00000040, - PAL_X509ChainCyclic = 0x00000080, - PAL_X509ChainInvalidExtension = 0x00000100, - PAL_X509ChainInvalidPolicyConstraints = 0x00000200, - PAL_X509ChainInvalidBasicConstraints = 0x00000400, - PAL_X509ChainInvalidNameConstraints = 0x00000800, - PAL_X509ChainHasNotSupportedNameConstraint = 0x00001000, - PAL_X509ChainHasNotDefinedNameConstraint = 0x00002000, - PAL_X509ChainHasNotPermittedNameConstraint = 0x00004000, - PAL_X509ChainHasExcludedNameConstraint = 0x00008000, - PAL_X509ChainPartialChain = 0x00010000, - PAL_X509ChainCtlNotTimeValid = 0x00020000, - PAL_X509ChainCtlNotSignatureValid = 0x00040000, - PAL_X509ChainCtlNotValidForUsage = 0x00080000, - PAL_X509ChainOfflineRevocation = 0x01000000, - PAL_X509ChainNoIssuanceChainPolicy = 0x02000000, - PAL_X509ChainExplicitDistrust = 0x04000000, - PAL_X509ChainHasNotSupportedCriticalExtension = 0x08000000, - PAL_X509ChainHasWeakSignature = 0x00100000, -}; -typedef uint32_t PAL_X509ChainStatusFlags; - #define PAL_X509ChainErrorNone 0 #define PAL_X509ChainErrorUnknownValueType (((uint64_t)0x0001L) << 32) #define PAL_X509ChainErrorUnknownValue (((uint64_t)0x0002L) << 32) From ddb048d5a9507f69f61b75952a47ee1ac3a7e5d7 Mon Sep 17 00:00:00 2001 From: Elinor Fung Date: Thu, 11 Mar 2021 04:14:25 -0800 Subject: [PATCH 04/19] Map failure reason to chain status when possible --- .../pal_jni.c | 42 ++++++++- .../pal_jni.h | 6 ++ .../pal_x509chain.c | 86 ++++++++++++++--- .../Cryptography/Pal.Android/ChainPal.cs | 94 ++++++++++++++----- .../src/Resources/Strings.resx | 21 +++-- 5 files changed, 199 insertions(+), 50 deletions(-) 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 927d842c2761e..dfcac956e88b9 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 @@ -155,6 +155,9 @@ jclass g_CertPathValidatorExceptionClass; jmethodID g_CertPathValidatorExceptionGetIndex; jmethodID g_CertPathValidatorExceptionGetReason; // only in API level 24+ +// java/security/cert/CertPathValidatorException$BasicReason - only in API level 24+ +jclass g_CertPathExceptionBasicReasonClass; + // java/security/cert/CertStore jclass g_CertStoreClass; jmethodID g_CertStoreGetInstance; @@ -177,6 +180,9 @@ jclass g_PKIXCertPathBuilderResultClass; jmethodID g_PKIXCertPathBuilderResultGetCertPath; jmethodID g_PKIXCertPathBuilderResultGetTrustAnchor; +// java/security/cert/PKIXReason - only in API level 24+ +jclass g_PKIXReasonClass; + // java/security/cert/PKIXRevocationChecker - only in API level 24+ jclass g_PKIXRevocationCheckerClass; jmethodID g_PKIXRevocationCheckerSetOptions; @@ -420,14 +426,38 @@ void ReleaseLRef(JNIEnv *env, jobject lref) (*env)->DeleteLocalRef(env, lref); } -jclass GetClassGRef(JNIEnv *env, const char* name) +static bool TryGetClassGRef(JNIEnv *env, const char* name, jclass* out) { + *out = NULL; LOG_DEBUG("Finding %s class", name); - jclass klass = ToGRef(env, (*env)->FindClass (env, name)); - if (!klass) { + jclass klass = (*env)->FindClass (env, name); + if (klass == NULL) + return false; + + *out = ToGRef(env, klass); + return true; +} + +jclass GetClassGRef(JNIEnv *env, const char* name) +{ + jclass klass = NULL; + if (!TryGetClassGRef(env, name, &klass)) + { LOG_ERROR("class %s was not found", name); - assert(klass); } + + assert(klass); + return klass; +} + +static jclass GetOptionalClassGRef(JNIEnv *env, const char* name) +{ + jclass klass = NULL; + if (!TryGetClassGRef(env, name, &klass)) + { + LOG_ERROR("optional class %s was not found", name); + } + return klass; } @@ -630,6 +660,8 @@ JNI_OnLoad(JavaVM *vm, void *reserved) g_CertPathValidatorExceptionGetIndex = GetMethod(env, false, g_CertPathValidatorExceptionClass, "getIndex", "()I"); g_CertPathValidatorExceptionGetReason = GetOptionalMethod(env, false, g_CertPathValidatorExceptionClass, "getReason", "()Ljava/security/cert/CertPathValidatorException$Reason;"); + g_CertPathExceptionBasicReasonClass = GetOptionalClassGRef(env, "java/security/cert/CertPathValidatorException$BasicReason"); + g_CertStoreClass = GetClassGRef(env, "java/security/cert/CertStore"); g_CertStoreGetInstance = GetMethod(env, true, g_CertStoreClass, "getInstance", "(Ljava/lang/String;Ljava/security/cert/CertStoreParameters;)Ljava/security/cert/CertStore;"); @@ -648,6 +680,8 @@ JNI_OnLoad(JavaVM *vm, void *reserved) g_PKIXCertPathBuilderResultGetCertPath = GetMethod(env, false, g_PKIXCertPathBuilderResultClass, "getCertPath", "()Ljava/security/cert/CertPath;"); g_PKIXCertPathBuilderResultGetTrustAnchor = GetMethod(env, false, g_PKIXCertPathBuilderResultClass, "getTrustAnchor", "()Ljava/security/cert/TrustAnchor;"); + g_PKIXReasonClass = GetOptionalClassGRef(env, "java/security/cert/PKIXReason"); + if (g_CertPathValidatorGetRevocationChecker != NULL) { g_PKIXRevocationCheckerClass = GetClassGRef(env, "java/security/cert/PKIXRevocationChecker"); diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native.Android/pal_jni.h b/src/libraries/Native/Unix/System.Security.Cryptography.Native.Android/pal_jni.h index 0557bccfe075e..1aa2e19397d4a 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native.Android/pal_jni.h +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native.Android/pal_jni.h @@ -129,6 +129,9 @@ extern jclass g_CertPathValidatorExceptionClass; extern jmethodID g_CertPathValidatorExceptionGetIndex; extern jmethodID g_CertPathValidatorExceptionGetReason; +// java/security/cert/CertPathValidatorException$BasicReason - only in API level 24+ +extern jclass g_CertPathExceptionBasicReasonClass; + // java/security/cert/CertStore extern jclass g_CertStoreClass; extern jmethodID g_CertStoreGetInstance; @@ -151,6 +154,9 @@ extern jclass g_PKIXCertPathBuilderResultClass; extern jmethodID g_PKIXCertPathBuilderResultGetCertPath; extern jmethodID g_PKIXCertPathBuilderResultGetTrustAnchor; +// java/security/cert/PKIXReason - only in API level 24+ +extern jclass g_PKIXReasonClass; + // java/security/cert/PKIXRevocationChecker - only in API level 24+ extern jclass g_PKIXRevocationCheckerClass; extern jmethodID g_PKIXRevocationCheckerSetOptions; 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 80236c230b108..78c0753d02671 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 @@ -143,7 +143,7 @@ int32_t AndroidCryptoNative_X509ChainBuild(X509ChainContext* ctx, int64_t timeIn loc[builder] = (*env)->CallStaticObjectMethod(env, g_CertPathBuilderClass, g_CertPathBuilderGetInstance, loc[builderType]); loc[result] = (*env)->CallObjectMethod(env, loc[builder], g_CertPathBuilderBuild, params); - if (TryGetJNIException(env, &loc[ex], true /*printException*/)) + if (TryGetJNIException(env, &loc[ex], false /*printException*/)) { (*env)->CallBooleanMethod(env, ctx->errorList, g_ArrayListAdd, loc[ex]); goto cleanup; @@ -232,10 +232,75 @@ int32_t AndroidCryptoNative_X509ChainGetErrorCount(X509ChainContext* ctx) return count; } -static PAL_X509ChainStatusFlags ChainStatusFromValidatorExceptionReason(jobject reason) +enum { - // TODO: [AndroidCrypto] Convert reason to chain status - return PAL_X509ChainNoError; + PKIXREASON_NAME_CHAINING, + PKIXREASON_INVALID_KEY_USAGE, + PKIXREASON_INVALID_POLICY, + PKIXREASON_NO_TRUST_ANCHOR, + PKIXREASON_UNRECOGNIZED_CRIT_EXT, + PKIXREASON_NOT_CA_CERT, + PKIXREASON_PATH_TOO_LONG, + PKIXREASON_INVALID_NAME, +}; + +enum +{ + BASICREASON_UNSPECIFIED, + BASICREASON_EXPIRED, + BASICREASON_NOT_YET_VALID, + BASICREASON_REVOKED, + BASICREASON_UNDETERMINED_REVOCATION_STATUS, + BASICREASON_INVALID_SIGNATURE, + BASICREASON_ALGORITHM_CONSTRAINED, +}; + +static PAL_X509ChainStatusFlags ChainStatusFromValidatorExceptionReason(JNIEnv* env, jobject reason) +{ + int value = (*env)->CallIntMethod(env, reason, g_EnumOrdinal); + if (g_CertPathExceptionBasicReasonClass != NULL && (*env)->IsInstanceOf(env, reason, g_CertPathExceptionBasicReasonClass)) + { + switch (value) + { + case BASICREASON_UNSPECIFIED: + return PAL_X509ChainPartialChain; + case BASICREASON_EXPIRED: + case BASICREASON_NOT_YET_VALID: + return PAL_X509ChainNotTimeValid; + case BASICREASON_REVOKED: + return PAL_X509ChainRevoked; + case BASICREASON_UNDETERMINED_REVOCATION_STATUS: + return PAL_X509ChainRevocationStatusUnknown; + case BASICREASON_INVALID_SIGNATURE: + return PAL_X509ChainCtlNotSignatureValid; + case BASICREASON_ALGORITHM_CONSTRAINED: + return PAL_X509ChainPartialChain; + } + } + else if (g_PKIXReasonClass != NULL && (*env)->IsInstanceOf(env, reason, g_PKIXReasonClass)) + { + switch (value) + { + case PKIXREASON_NAME_CHAINING: + return PAL_X509ChainPartialChain; + case PKIXREASON_INVALID_KEY_USAGE: + return PAL_X509ChainNotValidForUsage; + case PKIXREASON_INVALID_POLICY: + return PAL_X509ChainInvalidPolicyConstraints; + case PKIXREASON_NO_TRUST_ANCHOR: + return PAL_X509ChainPartialChain; + case PKIXREASON_UNRECOGNIZED_CRIT_EXT: + return PAL_X509ChainHasNotSupportedCriticalExtension; + case PKIXREASON_NOT_CA_CERT: + return PAL_X509ChainUntrustedRoot; + case PKIXREASON_PATH_TOO_LONG: + return PAL_X509ChainInvalidBasicConstraints; + case PKIXREASON_INVALID_NAME: + return PAL_X509ChainInvalidNameConstraints; + } + } + + return PAL_X509ChainPartialChain; } static void PopulateValidationError(JNIEnv* env, jobject error, ValidationError* out) @@ -250,7 +315,7 @@ static void PopulateValidationError(JNIEnv* env, jobject error, ValidationError* if (g_CertPathValidatorExceptionGetReason != NULL) { jobject reason = (*env)->CallObjectMethod(env, error, g_CertPathValidatorExceptionGetReason); - chainStatus = ChainStatusFromValidatorExceptionReason(reason); + chainStatus = ChainStatusFromValidatorExceptionReason(env, reason); (*env)->DeleteLocalRef(env, reason); } } @@ -393,15 +458,9 @@ int32_t AndroidCryptoNative_X509ChainValidate(X509ChainContext* ctx, if (AndroidCryptoNative_X509ChainSupportsRevocationOptions()) { // TODO: [AndroidCrypto] Deal with revocation options - goto cleanup; } else { - if (revocationFlag == X509RevocationFlag_EndCertificateOnly) - { - goto cleanup; - } - // Security.setProperty("oscp.enable", isOnline); } } @@ -420,11 +479,12 @@ int32_t AndroidCryptoNative_X509ChainValidate(X509ChainContext* ctx, loc[validatorType] = JSTRING("PKIX"); loc[validator] = (*env)->CallStaticObjectMethod( env, g_CertPathValidatorClass, g_CertPathValidatorGetInstance, loc[validatorType]); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); + loc[result] = (*env)->CallObjectMethod(env, loc[validator], g_CertPathValidatorValidate, certPath, params); - if (TryGetJNIException(env, &loc[ex], true /*printException*/)) + if (TryGetJNIException(env, &loc[ex], false /*printException*/)) { (*env)->CallBooleanMethod(env, ctx->errorList, g_ArrayListAdd, loc[ex]); - goto cleanup; } ret = SUCCESS; diff --git a/src/libraries/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Android/ChainPal.cs b/src/libraries/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Android/ChainPal.cs index 38d11af686a4b..11b1686668a73 100644 --- a/src/libraries/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Android/ChainPal.cs +++ b/src/libraries/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Android/ChainPal.cs @@ -78,22 +78,23 @@ public void Dispose() if (!_isValid) { - if ((flags & X509VerificationFlags.IgnoreNotTimeValid) == X509VerificationFlags.IgnoreNotTimeValid) + // There is no way to bypass time, trusted root, name, or policy constraint + // validation on Android. It will not build any chain without these all + // being valid. + X509VerificationFlags[] unsupportedFlags = new X509VerificationFlags[] { - // There is no way to bypass time validation on Android. - // It will not build any chain without a valid time. - exception = new PlatformNotSupportedException( - SR.Format(SR.Cryptography_VerificationFlagNotSupported, X509VerificationFlags.IgnoreNotTimeValid)); - return default(bool?); - } - - if ((flags & X509VerificationFlags.AllowUnknownCertificateAuthority) == X509VerificationFlags.AllowUnknownCertificateAuthority) + X509VerificationFlags.IgnoreNotTimeValid, + X509VerificationFlags.AllowUnknownCertificateAuthority, + X509VerificationFlags.IgnoreInvalidName, + X509VerificationFlags.IgnoreInvalidPolicy, + }; + foreach (X509VerificationFlags unsupported in unsupportedFlags) { - // There is no way to allow an untrusted root on Android. - // It will not build any chain without a trusted root. - exception = new PlatformNotSupportedException( - SR.Format(SR.Cryptography_VerificationFlagNotSupported, X509VerificationFlags.AllowUnknownCertificateAuthority)); - return default(bool?); + if ((flags & unsupported) == unsupported) + { + exception = new PlatformNotSupportedException(SR.Format(SR.Chain_VerificationFlagNotSupported, unsupported)); + return default(bool?); + } } return false; @@ -148,7 +149,7 @@ internal void Initialize( // Android does not support an empty set of trust anchors if (customTrustCerts.Count == 0) { - throw new PlatformNotSupportedException(SR.Cryptography_EmptyCustomTrustNotSupported); + throw new PlatformNotSupportedException(SR.Chain_EmptyCustomTrustNotSupported); } IntPtr[] customTrustArray = customTrustCerts.ToArray(); @@ -173,7 +174,7 @@ internal void Evaluate( _isValid = Interop.AndroidCrypto.X509ChainBuild(_chainContext, timeInMsFromUnixEpoch); if (!_isValid) { - // Android always validates time, signature, and trusted root. + // Android always validates name, time, signature, and trusted root. // There is no way bypass that validation and build a path. ChainElements = Array.Empty(); @@ -190,11 +191,35 @@ internal void Evaluate( return; } - if (revocationMode != X509RevocationMode.NoCheck && !Interop.AndroidCrypto.X509ChainSupportsRevocationOptions()) + if (revocationMode != X509RevocationMode.NoCheck) { - if (revocationFlag == X509RevocationFlag.EndCertificateOnly) + // TODO: [AndroidCrypto] Handle revocation modes/flags + if (revocationFlag == X509RevocationFlag.EntireChain) { - throw new PlatformNotSupportedException(SR.Format(SR.Cryptography_ChainSettingNotSupported, $"{nameof(X509RevocationFlag)}.{nameof(X509RevocationFlag.EndCertificateOnly)}")); + throw new NotImplementedException($"{nameof(Evaluate)} (X509RevocationFlag.{revocationFlag})"); + } + + if (Interop.AndroidCrypto.X509ChainSupportsRevocationOptions()) + { + // Defaults to online when revocation options are available + if (revocationMode == X509RevocationMode.Offline) + { + throw new NotImplementedException($"{nameof(Evaluate)} (X509RevocationMode.{revocationMode})"); + } + } + else + { + if (revocationFlag == X509RevocationFlag.EndCertificateOnly) + { + // No way to specfiy end certificate only if revocation options are not available + throw new PlatformNotSupportedException(SR.Format(SR.Chain_SettingNotSupported, $"{nameof(X509RevocationFlag)}.{nameof(X509RevocationFlag.EndCertificateOnly)}")); + } + + // Defaults to offline when revocation options are not available + if (revocationMode == X509RevocationMode.Online) + { + throw new NotImplementedException($"{nameof(Evaluate)} (X509RevocationMode.{revocationMode})"); + } } } @@ -205,17 +230,38 @@ internal void Evaluate( X509Certificate2[] certs = Interop.AndroidCrypto.X509ChainGetCertificates(_chainContext); List overallStatus = new List(); List[] statuses = new List[certs.Length]; + + int firstErrorIndex = -1; Dictionary> errorsByIndex = GetStatusByIndex(_chainContext); foreach (int index in errorsByIndex.Keys) { - if (index == -1) + overallStatus.AddRange(errorsByIndex[index]); + + // -1 indicates that error is not tied to a specific index + if (index != -1) { - // Error is not tied to a specific index - overallStatus.AddRange(errorsByIndex[index]); + statuses[index] = errorsByIndex[index]; + firstErrorIndex = Math.Max(index, firstErrorIndex); } - else + } + + // Android will stop checking after the first error it hits, so we explicitly + // assign PartialChain to everything from the first error to the end certificate + if (firstErrorIndex > 0) + { + X509ChainStatus partialChainStatus = new X509ChainStatus { - statuses[index] = errorsByIndex[index]; + Status = X509ChainStatusFlags.PartialChain, + StatusInformation = SR.Chain_PartialChain, + }; + for (int i = firstErrorIndex - 1; i >= 0; i--) + { + if (statuses[i] == null) + { + statuses[i] = new List(); + } + + statuses[i].Add(partialChainStatus); } } diff --git a/src/libraries/System.Security.Cryptography.X509Certificates/src/Resources/Strings.resx b/src/libraries/System.Security.Cryptography.X509Certificates/src/Resources/Strings.resx index d477de60b9ed5..eb14630f320de 100644 --- a/src/libraries/System.Security.Cryptography.X509Certificates/src/Resources/Strings.resx +++ b/src/libraries/System.Security.Cryptography.X509Certificates/src/Resources/Strings.resx @@ -103,9 +103,21 @@ Index was out of range. Must be non-negative and less than the size of the collection. + + An empty custom trust store is not supported on this platform. + The certificate has invalid policy. + + The certificate chain is incomplete. + + + The certificate chain setting '{0}' is not supported on this platform. + + + The verification flag '{0}' is not supported on this platform. + The provided value of {0} bytes does not match the expected size of {1} bytes for the algorithm ({2}). @@ -448,13 +460,4 @@ The certificate content type could not be determined. - - The certificate chain setting '{0}' is not supported on this platform. - - - An empty custom trust store is not supported on this platform. - - - The verification flag '{0}' is not supported on this platform. - From 14ac707053ec0767fa0a3ada0466fa85f350b0d8 Mon Sep 17 00:00:00 2001 From: Elinor Fung Date: Thu, 11 Mar 2021 15:56:49 -0800 Subject: [PATCH 05/19] Update tests to reflect limited support on Android --- .../CertificateRequestChainTests.cs | 7 +- .../tests/ChainTests.cs | 212 ++++++++++++++---- .../tests/DynamicChainTests.cs | 134 +++++++++-- .../tests/TestData.cs | 25 +++ 4 files changed, 310 insertions(+), 68 deletions(-) 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 4ea087b42ce44..16c41b6a68dca 100644 --- a/src/libraries/System.Security.Cryptography.X509Certificates/tests/CertificateCreation/CertificateRequestChainTests.cs +++ b/src/libraries/System.Security.Cryptography.X509Certificates/tests/CertificateCreation/CertificateRequestChainTests.cs @@ -176,9 +176,9 @@ public static void ChainCertRequirements(bool useIntermed, bool? isCA, X509KeyUs using (X509Chain chain = new X509Chain()) { chain.ChainPolicy.RevocationMode = X509RevocationMode.NoCheck; - chain.ChainPolicy.VerificationFlags = X509VerificationFlags.AllowUnknownCertificateAuthority; chain.ChainPolicy.ExtraStore.Add(rootCert); chain.ChainPolicy.VerificationTime = start.ToLocalTime().DateTime; + chain.AllowUnknownAuthorityOrAddSelfSignedToCustomTrust(rootCert); if (useIntermed) { @@ -418,12 +418,13 @@ private static void CreateAndTestChain( using (X509Chain chain = new X509Chain()) { chain.ChainPolicy.RevocationMode = X509RevocationMode.NoCheck; - chain.ChainPolicy.VerificationFlags = X509VerificationFlags.AllowUnknownCertificateAuthority; chain.ChainPolicy.ExtraStore.Add(intermed1CertWithKey); chain.ChainPolicy.ExtraStore.Add(intermed2CertWithKey); chain.ChainPolicy.ExtraStore.Add(rootCertWithKey); chain.ChainPolicy.VerificationTime = now.ToLocalTime().DateTime; + chain.AllowUnknownAuthorityOrAddSelfSignedToCustomTrust(rootCertWithKey); + RunChain(chain, leafCert, true, "Initial chain build"); try @@ -505,7 +506,7 @@ public static void CreateChain_RSAPSS() using (X509Chain chain = new X509Chain()) { chain.ChainPolicy.RevocationMode = X509RevocationMode.NoCheck; - chain.ChainPolicy.VerificationFlags = X509VerificationFlags.AllowUnknownCertificateAuthority; + chain.AllowUnknownAuthorityOrAddSelfSignedToCustomTrust(rootCertWithKey); chain.ChainPolicy.ExtraStore.Add(intermedCertWithKey); chain.ChainPolicy.ExtraStore.Add(rootCertWithKey); chain.ChainPolicy.VerificationTime = notBefore.ToLocalTime().DateTime; diff --git a/src/libraries/System.Security.Cryptography.X509Certificates/tests/ChainTests.cs b/src/libraries/System.Security.Cryptography.X509Certificates/tests/ChainTests.cs index ec552a08fd45d..b50216fdf3a17 100644 --- a/src/libraries/System.Security.Cryptography.X509Certificates/tests/ChainTests.cs +++ b/src/libraries/System.Security.Cryptography.X509Certificates/tests/ChainTests.cs @@ -126,10 +126,16 @@ public static void VerifyChainFromHandle() public static void VerifyChainFromHandle_Unix() { using (var microsoftDotCom = new X509Certificate2(TestData.MicrosoftDotComSslCertBytes)) + using (var microsoftDotComIssuer = new X509Certificate2(TestData.MicrosoftDotComIssuerBytes)) + using (var microsoftDotComRoot = new X509Certificate2(TestData.MicrosoftDotComRootBytes)) using (var chainHolder = new ChainHolder()) { X509Chain chain = chainHolder.Chain; + + chain.ChainPolicy.ExtraStore.Add(microsoftDotComRoot); + chain.ChainPolicy.ExtraStore.Add(microsoftDotComIssuer); chain.ChainPolicy.VerificationFlags = X509VerificationFlags.AllowUnknownCertificateAuthority; + chain.ChainPolicy.VerificationTime = new DateTime(2021, 02, 26, 12, 01, 01, DateTimeKind.Local); chain.ChainPolicy.RevocationMode = X509RevocationMode.NoCheck; @@ -175,7 +181,10 @@ public static void TestResetMethod() Assert.False(valid); chainHolder.DisposeChainElements(); + // Make sure AllowUnknownCertificateAuthority is set, as this test we verifies it does not get reset chain.ChainPolicy.VerificationFlags = X509VerificationFlags.AllowUnknownCertificateAuthority; + chain.AllowUnknownAuthorityOrAddSelfSignedToCustomTrust(sampleCert); + chain.ChainPolicy.VerificationTime = new DateTime(2015, 10, 15, 12, 01, 01, DateTimeKind.Local); chain.ChainPolicy.RevocationMode = X509RevocationMode.NoCheck; @@ -245,18 +254,28 @@ public static void SystemTrustCertificateWithCustomRootTrust(bool addCertificate chain.ChainPolicy.CustomTrustStore.Add(testCert); } - Assert.False(chain.Build(microsoftDotCom)); - - // Linux and Windows do not search the default system root stores when CustomRootTrust is enabled - if (OperatingSystem.IsMacOS()) + if (PlatformDetection.IsAndroid) { - Assert.Equal(3, chain.ChainElements.Count); - Assert.Equal(X509ChainStatusFlags.UntrustedRoot, chain.AllStatusFlags()); + // Android does not support an empty custom root trust + // Only self-issued certs are treated as trusted anchors, so building the chain + // should through PNSE regardless of whether or not testCert is added to the store + Assert.Throws(() => chain.Build(microsoftDotCom)); } else { - Assert.Equal(2, chain.ChainElements.Count); - Assert.Equal(X509ChainStatusFlags.PartialChain, chain.AllStatusFlags()); + Assert.False(chain.Build(microsoftDotCom)); + + // Linux and Windows do not search the default system root stores when CustomRootTrust is enabled + if (OperatingSystem.IsMacOS()) + { + Assert.Equal(3, chain.ChainElements.Count); + Assert.Equal(X509ChainStatusFlags.UntrustedRoot, chain.AllStatusFlags()); + } + else + { + Assert.Equal(2, chain.ChainElements.Count); + Assert.Equal(X509ChainStatusFlags.PartialChain, chain.AllStatusFlags()); + } } } } @@ -287,6 +306,8 @@ public static void BuildChainCustomTrustStore( chainPrep.ChainPolicy.VerificationTime = microsoftDotCom.NotBefore.AddSeconds(1); chainPrep.Build(microsoftDotCom); + + Assert.True(chainPrep.ChainElements.Count >= 3, "Expected chain to build with root cert as third element"); X509Certificate2 rootCert = chainPrep.ChainElements[2].Certificate; using (var chainHolderTest = new ChainHolder()) @@ -326,7 +347,7 @@ public static void BuildChainCustomTrustStore( } [Fact] - public static void BuildChainWithSystemTrustAndCustomTrustCertificates() + public static void BuildChainWithSBystemTrustAndCustomTrustCertificates() { using (var testCert = new X509Certificate2(TestFiles.ChainPfxFile, TestData.ChainPfxPassword)) using (var chainHolder = new ChainHolder()) @@ -441,6 +462,16 @@ public static void VerifyExpiration_LocalTime(DateTime verificationTime, bool sh // Ignore anything except NotTimeValid chain.ChainPolicy.VerificationFlags = X509VerificationFlags.AllFlags & ~X509VerificationFlags.IgnoreNotTimeValid; + + // Android doesn't support certain flags and will error if the chain is + // invalid and one of those flags is set. + if (PlatformDetection.IsAndroid) + { + chain.ChainPolicy.VerificationFlags &= ~X509VerificationFlags.AllowUnknownCertificateAuthority; + chain.ChainPolicy.VerificationFlags &= ~X509VerificationFlags.IgnoreInvalidName; + chain.ChainPolicy.VerificationFlags &= ~X509VerificationFlags.IgnoreInvalidPolicy; + } + chain.ChainPolicy.RevocationMode = X509RevocationMode.NoCheck; chain.ChainPolicy.VerificationTime = verificationTime; @@ -448,10 +479,21 @@ public static void VerifyExpiration_LocalTime(DateTime verificationTime, bool sh Assert.Equal(shouldBeValid, builtSuccessfully); - // If we failed to build the chain, ensure that NotTimeValid is one of the reasons. + // If we failed to build the chain, validate the chain status if (!shouldBeValid) { - Assert.Contains(chain.ChainStatus, s => s.Status == X509ChainStatusFlags.NotTimeValid); + if (PlatformDetection.IsAndroid) + { + // Android always validates timestamp as part of building a path, + // so invalid time comes back as PartialChain with no elements + Assert.Equal(X509ChainStatusFlags.PartialChain, chain.AllStatusFlags()); + Assert.Equal(0, chain.ChainElements.Count); + } + else + { + // Ensure that NotTimeValid is one of the reasons. + Assert.Contains(chain.ChainStatus, s => s.Status == X509ChainStatusFlags.NotTimeValid); + } } } } @@ -459,20 +501,19 @@ public static void VerifyExpiration_LocalTime(DateTime verificationTime, bool sh [Fact] public static void BuildChain_WithApplicationPolicy_Match() { - using (var msCer = new X509Certificate2(TestData.MsCertificate)) + using (var cert = new X509Certificate2(TestData.CertWithEnhancedKeyUsage)) using (var chainHolder = new ChainHolder()) { X509Chain chain = chainHolder.Chain; // Code Signing chain.ChainPolicy.ApplicationPolicy.Add(new Oid("1.3.6.1.5.5.7.3.3")); - chain.ChainPolicy.VerificationTime = msCer.NotBefore.AddHours(2); - chain.ChainPolicy.VerificationFlags = - X509VerificationFlags.AllowUnknownCertificateAuthority; + chain.ChainPolicy.VerificationTime = cert.NotBefore.AddHours(2); + chain.AllowUnknownAuthorityOrAddSelfSignedToCustomTrust(cert); chain.ChainPolicy.RevocationMode = X509RevocationMode.NoCheck; - bool valid = chain.Build(msCer); + bool valid = chain.Build(cert); Assert.True(valid, "Chain built validly"); } } @@ -480,15 +521,14 @@ public static void BuildChain_WithApplicationPolicy_Match() [Fact] public static void BuildChain_WithApplicationPolicy_NoMatch() { - using (var cert = new X509Certificate2(TestData.MsCertificate)) + using (var cert = new X509Certificate2(TestData.CertWithEnhancedKeyUsage)) using (var chainHolder = new ChainHolder()) { X509Chain chain = chainHolder.Chain; // Gibberish. (Code Signing + ".1") chain.ChainPolicy.ApplicationPolicy.Add(new Oid("1.3.6.1.5.5.7.3.3.1")); - chain.ChainPolicy.VerificationFlags = - X509VerificationFlags.AllowUnknownCertificateAuthority; + chain.AllowUnknownAuthorityOrAddSelfSignedToCustomTrust(cert); chain.ChainPolicy.RevocationMode = X509RevocationMode.NoCheck; chain.ChainPolicy.VerificationTime = cert.NotBefore.AddHours(2); @@ -517,9 +557,8 @@ public static void BuildChain_WithCertificatePolicy_Match() // Code Signing chain.ChainPolicy.CertificatePolicy.Add(new Oid("2.18.19")); - chain.ChainPolicy.VerificationFlags = - X509VerificationFlags.AllowUnknownCertificateAuthority; chain.ChainPolicy.VerificationTime = cert.NotBefore.AddHours(2); + chain.AllowUnknownAuthorityOrAddSelfSignedToCustomTrust(cert); chain.ChainPolicy.RevocationMode = X509RevocationMode.NoCheck; @@ -537,11 +576,10 @@ public static void BuildChain_WithCertificatePolicy_NoMatch() X509Chain chain = chainHolder.Chain; chain.ChainPolicy.CertificatePolicy.Add(new Oid("2.999")); - chain.ChainPolicy.VerificationFlags = - X509VerificationFlags.AllowUnknownCertificateAuthority; chain.ChainPolicy.RevocationMode = X509RevocationMode.NoCheck; chain.ChainPolicy.VerificationTime = cert.NotBefore.AddHours(2); + chain.AllowUnknownAuthorityOrAddSelfSignedToCustomTrust(cert); bool valid = chain.Build(cert); Assert.False(valid, "Chain built validly"); @@ -744,6 +782,12 @@ public static void InvalidSelfSignedSignature() X509ChainStatusFlags.UntrustedRoot | X509ChainStatusFlags.PartialChain; } + else if (OperatingSystem.IsAndroid()) + { + // Android always validates signature as part of building a path, + // so invalid signature comes back as PartialChain with no elements + expectedFlags = X509ChainStatusFlags.PartialChain; + } else { expectedFlags = @@ -784,6 +828,9 @@ public static void InvalidSelfSignedSignature() } [Fact] + // 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. + [PlatformSpecific(~TestPlatforms.Android)] public static void ChainErrorsAtMultipleLayers() { // These certificates were generated for this test using CertificateRequest @@ -792,8 +839,8 @@ public static void ChainErrorsAtMultipleLayers() // // These certificates have been hard-coded to enable the scenario on // netstandard. - byte[] endEntityBytes = Encoding.ASCII.GetBytes(@" ------BEGIN CERTIFICATE----- + byte[] endEntityBytes = Encoding.ASCII.GetBytes( +@"-----BEGIN CERTIFICATE----- MIIC6DCCAdCgAwIBAgIQAKjmD7+TWUwQN2ucajn9kTANBgkqhkiG9w0BAQsFADAXMRUwEwYDVQQD EwxJbnRlcm1lZGlhdGUwHhcNMTkwMzAzMjM1NzA3WhcNMTkwNjAzMjM1NzA3WjAVMRMwEQYDVQQD EwpFbmQtRW50aXR5MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAxTybBkpMdQ8IeL1C @@ -809,8 +856,8 @@ public static void ChainErrorsAtMultipleLayers() psHHsU9xg0o7L2WXD5qYhD2JCQIVWNRmRZCf1luWlKqUaqWWONMJ44hk8Md+ohxpyCRmbtLRZPzd wlkQzPsc9A== -----END CERTIFICATE-----"); - byte[] intermediateBytes = Encoding.ASCII.GetBytes(@" ------BEGIN CERTIFICATE----- + byte[] intermediateBytes = Encoding.ASCII.GetBytes( +@"-----BEGIN CERTIFICATE----- MIIC1DCCAbygAwIBAgIPRoY1rB2tMVJeYB4GILkNMA0GCSqGSIb3DQEBCwUAMBQxEjAQBgNVBAMT CVRlc3QgUm9vdDAeFw0xOTAyMTgyMzU3MDdaFw0yMDAyMTgyMzU3MDdaMBcxFTATBgNVBAMTDElu dGVybWVkaWF0ZTCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBALxYzEN6nYvQ0TOg/jOF @@ -825,8 +872,8 @@ public static void ChainErrorsAtMultipleLayers() ueuTl2qTtbBh015GuEld61EBXSBLIUqwOAeFYrNJbC4J2mXgnLTWC380cBf5KWeSdjLYgk2sZ1V4 FKKQecZIhxdlDGzMAbbmEV+2EqS+As2C7+y4dkpG4nnbQe/4AFr8vekHdrI= -----END CERTIFICATE-----"); - byte[] rootBytes = Encoding.ASCII.GetBytes(@" ------BEGIN CERTIFICATE----- + byte[] rootBytes = Encoding.ASCII.GetBytes( +@"-----BEGIN CERTIFICATE----- MIICyjCCAbKgAwIBAgIIKKt3K3rRbvQwDQYJKoZIhvcNAQELBQAwFDESMBAGA1UEAxMJVGVzdCBS b290MB4XDTE5MDIwNDIzNTcwN1oXDTIxMDIwNDIzNTcwN1owFDESMBAGA1UEAxMJVGVzdCBSb290 MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAiM7tv4YvqmWYGF1vbeM2cQWV1NVBxKU4 @@ -876,7 +923,7 @@ public static void ChainWithEmptySubject() { X509Chain chain = chainHolder.Chain; chain.ChainPolicy.RevocationMode = X509RevocationMode.NoCheck; - chain.ChainPolicy.VerificationFlags |= X509VerificationFlags.AllowUnknownCertificateAuthority; + chain.AllowUnknownAuthorityOrAddSelfSignedToCustomTrust(issuer); chain.ChainPolicy.ExtraStore.Add(issuer); Assert.True(chain.Build(cert), "chain.Build(cert)"); @@ -888,6 +935,7 @@ public static void ChainWithEmptySubject() } [Fact] + [PlatformSpecific(~TestPlatforms.Android)] public static void BuildInvalidSignatureTwice() { byte[] bytes = (byte[])TestData.MsCertificate.Clone(); @@ -950,6 +998,43 @@ void CheckChain() } } + [Fact] + [PlatformSpecific(TestPlatforms.Android)] + public static void BuildInvalidSignatureTwice_Android() + { + byte[] bytes = (byte[])TestData.MicrosoftDotComSslCertBytes.Clone(); + bytes[bytes.Length - 1] ^= 0xFF; + + using (X509Certificate2 microsoftDotComIssuer = new X509Certificate2(TestData.MicrosoftDotComIssuerBytes)) + using (X509Certificate2 microsoftDotComRoot = new X509Certificate2(TestData.MicrosoftDotComRootBytes)) + using (X509Certificate2 cert = new X509Certificate2(bytes)) + using (ChainHolder chainHolder = new ChainHolder()) + { + X509Chain chain = chainHolder.Chain; + chain.ChainPolicy.VerificationTime = cert.NotBefore.AddHours(2); + + chain.ChainPolicy.ExtraStore.Add(microsoftDotComRoot); + chain.ChainPolicy.ExtraStore.Add(microsoftDotComIssuer); + chain.ChainPolicy.RevocationMode = X509RevocationMode.NoCheck; + + CheckChain(); + CheckChain(); + + void CheckChain() + { + bool valid = chain.Build(cert); + X509ChainStatusFlags allFlags = chain.AllStatusFlags(); + + // Android always validates signature as part of building a path, + // so invalid signature comes back as PartialChain with no elements + Assert.Equal(X509ChainStatusFlags.PartialChain, allFlags); + Assert.Equal(0, chain.ChainElements.Count); + Assert.False(valid, $"Chain should not be valid"); + chainHolder.DisposeChainElements(); + } + } + } + [Fact] [PlatformSpecific(~TestPlatforms.Linux)] public static void BuildChainForFraudulentCertificate() @@ -1002,13 +1087,23 @@ public static void BuildChainForFraudulentCertificate() chain.ChainPolicy.VerificationTime = cert.NotBefore.AddHours(2); Assert.False(chain.Build(cert)); - X509ChainElement certElement = chain.ChainElements - .OfType() - .Single(e => e.Certificate.Subject == cert.Subject); + if (PlatformDetection.IsAndroid) + { + // Android always validates trust as part of building a path, + // so violations comes back as PartialChain with no elements + Assert.Equal(X509ChainStatusFlags.PartialChain, chain.AllStatusFlags()); + Assert.Equal(0, chain.ChainElements.Count); + } + else + { + X509ChainElement certElement = chain.ChainElements + .OfType() + .Single(e => e.Certificate.Subject == cert.Subject); - const X509ChainStatusFlags ExpectedFlag = X509ChainStatusFlags.ExplicitDistrust; - X509ChainStatusFlags actualFlags = certElement.AllStatusFlags(); - Assert.True((actualFlags & ExpectedFlag) == ExpectedFlag, $"Has expected flag {ExpectedFlag} but was {actualFlags}"); + const X509ChainStatusFlags ExpectedFlag = X509ChainStatusFlags.ExplicitDistrust; + X509ChainStatusFlags actualFlags = certElement.AllStatusFlags(); + Assert.True((actualFlags & ExpectedFlag) == ExpectedFlag, $"Has expected flag {ExpectedFlag} but was {actualFlags}"); + } } } @@ -1079,13 +1174,23 @@ public static void BuildChainForCertificateSignedWithDisallowedKey() chain.ChainPolicy.ExtraStore.Add(intermediateCert); Assert.False(chain.Build(cert)); - X509ChainElement certElement = chain.ChainElements - .OfType() - .Single(e => e.Certificate.Subject == intermediateCert.Subject); + if (PlatformDetection.IsAndroid) + { + // Android always validates trust as part of building a path, + // so violations comes back as PartialChain with no elements + Assert.Equal(X509ChainStatusFlags.PartialChain, chain.AllStatusFlags()); + Assert.Equal(0, chain.ChainElements.Count); + } + else + { + X509ChainElement certElement = chain.ChainElements + .OfType() + .Single(e => e.Certificate.Subject == intermediateCert.Subject); - const X509ChainStatusFlags ExpectedFlag = X509ChainStatusFlags.ExplicitDistrust; - X509ChainStatusFlags actualFlags = certElement.AllStatusFlags(); - Assert.True((actualFlags & ExpectedFlag) == ExpectedFlag, $"Has expected flag {ExpectedFlag} but was {actualFlags}"); + const X509ChainStatusFlags ExpectedFlag = X509ChainStatusFlags.ExplicitDistrust; + X509ChainStatusFlags actualFlags = certElement.AllStatusFlags(); + Assert.True((actualFlags & ExpectedFlag) == ExpectedFlag, $"Has expected flag {ExpectedFlag} but was {actualFlags}"); + } } } @@ -1166,5 +1271,28 @@ internal static X509ChainStatusFlags AllStatusFlags(this X509ChainElement chainE X509ChainStatusFlags.NoError, (f, s) => f | s.Status); } + + internal static void AllowUnknownAuthorityOrAddSelfSignedToCustomTrust(this X509Chain chain, X509Certificate2 cert) + { + if (!PlatformDetection.IsAndroid) + { + chain.ChainPolicy.VerificationFlags |= X509VerificationFlags.AllowUnknownCertificateAuthority; + return; + } + + // Many tests set AllowUnknownCertificateAuthority in order to build a valid chain for testing the + // validation of other properties. + // Android does not support building a path that does not lead to a trusted root. Using a custom + // root trust with a self-signed cert allows for building a valid chain with the cert. + if (cert.SubjectName.RawData.SequenceEqual(cert.IssuerName.RawData)) + { + chain.ChainPolicy.CustomTrustStore.Add(cert); + chain.ChainPolicy.TrustMode = X509ChainTrustMode.CustomRootTrust; + } + else + { + Assert.True(false, "Could not configure chain policy to handle unknown certificate authority"); + } + } } } diff --git a/src/libraries/System.Security.Cryptography.X509Certificates/tests/DynamicChainTests.cs b/src/libraries/System.Security.Cryptography.X509Certificates/tests/DynamicChainTests.cs index 7c1cec9f1e37c..02c0bdc117f2a 100644 --- a/src/libraries/System.Security.Cryptography.X509Certificates/tests/DynamicChainTests.cs +++ b/src/libraries/System.Security.Cryptography.X509Certificates/tests/DynamicChainTests.cs @@ -142,6 +142,15 @@ DateTime RewindIfNeeded(DateTime input, X509Certificate2 cert, X509ChainStatusFl } } } + else if (OperatingSystem.IsAndroid()) + { + // Android always validates signature as part of building a path, + // so invalid signature comes back as PartialChain with no elements + expectedCount = 0; + endEntityErrors = X509ChainStatusFlags.PartialChain; + intermediateErrors = X509ChainStatusFlags.PartialChain; + rootErrors = X509ChainStatusFlags.PartialChain; + } else if (OperatingSystem.IsWindows()) { // Windows only reports NotTimeValid on the start-of-chain (end-entity in this case) @@ -153,11 +162,21 @@ DateTime RewindIfNeeded(DateTime input, X509Certificate2 cert, X509ChainStatusFl X509ChainStatusFlags expectedAllErrors = endEntityErrors | intermediateErrors | rootErrors; - // If PartialChain or UntrustedRoot are the only remaining errors, the chain will succeed. - const X509ChainStatusFlags SuccessCodes = - X509ChainStatusFlags.UntrustedRoot | X509ChainStatusFlags.PartialChain; + bool expectSuccess; + if (PlatformDetection.IsAndroid) + { + // Android always validates signature as part of building a path, so chain + // building is expected to fail + expectSuccess = false; + } + else + { + // If PartialChain or UntrustedRoot are the only remaining errors, the chain will succeed. + const X509ChainStatusFlags SuccessCodes = + X509ChainStatusFlags.UntrustedRoot | X509ChainStatusFlags.PartialChain; - bool expectSuccess = (expectedAllErrors & ~SuccessCodes) == 0; + expectSuccess = (expectedAllErrors & ~SuccessCodes) == 0; + } using (endEntityCert) using (intermediateCert) @@ -170,8 +189,12 @@ DateTime RewindIfNeeded(DateTime input, X509Certificate2 cert, X509ChainStatusFl chain.ChainPolicy.ExtraStore.Add(intermediateCert); chain.ChainPolicy.ExtraStore.Add(rootCert); - chain.ChainPolicy.VerificationFlags |= - X509VerificationFlags.AllowUnknownCertificateAuthority; + // Android doesn't respect AllowUnknownCertificateAuthority + if (!PlatformDetection.IsAndroid) + { + chain.ChainPolicy.VerificationFlags |= + X509VerificationFlags.AllowUnknownCertificateAuthority; + } int i = 0; @@ -193,7 +216,10 @@ void CheckChain() Assert.Equal(expectedCount, chain.ChainElements.Count); Assert.Equal(expectedAllErrors, chain.AllStatusFlags()); - Assert.Equal(endEntityErrors, chain.ChainElements[0].AllStatusFlags()); + if (expectedCount > 0) + { + Assert.Equal(endEntityErrors, chain.ChainElements[0].AllStatusFlags()); + } if (expectedCount > 2) { @@ -257,7 +283,7 @@ public static void BasicConstraints_ExceedMaximumPathLength() chain.ChainPolicy.ExtraStore.Add(intermediateCert2); Assert.False(chain.Build(endEntityCert)); - Assert.Equal(X509ChainStatusFlags.InvalidBasicConstraints, chain.AllStatusFlags()); + Assert.Equal(PlatformBasicConstraints(X509ChainStatusFlags.InvalidBasicConstraints), chain.AllStatusFlags()); } } @@ -287,7 +313,7 @@ public static void BasicConstraints_ViolatesCaFalse() rootCert, intermediateCert, endEntityCert, - expectedFlags: X509ChainStatusFlags.InvalidBasicConstraints); + expectedFlags: PlatformBasicConstraints(X509ChainStatusFlags.InvalidBasicConstraints)); } } @@ -319,10 +345,20 @@ public static void TestLeafCertificateWithUnknownCriticalExtension() chain.ChainPolicy.RevocationMode = X509RevocationMode.NoCheck; Assert.False(chain.Build(cert)); - X509ChainElement certElement = chain.ChainElements.OfType().Single(); - const X509ChainStatusFlags ExpectedFlag = X509ChainStatusFlags.HasNotSupportedCriticalExtension; - X509ChainStatusFlags actualFlags = certElement.AllStatusFlags(); - Assert.True((actualFlags & ExpectedFlag) == ExpectedFlag, $"Has expected flag {ExpectedFlag} but was {actualFlags}"); + if (PlatformDetection.IsAndroid) + { + // Android always unsupported critical extensions as part of building a path, + // so errors comes back as PartialChain with no elements + Assert.Equal(X509ChainStatusFlags.PartialChain, chain.AllStatusFlags()); + Assert.Equal(0, chain.ChainElements.Count); + } + else + { + X509ChainElement certElement = chain.ChainElements.OfType().Single(); + const X509ChainStatusFlags ExpectedFlag = X509ChainStatusFlags.HasNotSupportedCriticalExtension; + X509ChainStatusFlags actualFlags = certElement.AllStatusFlags(); + Assert.True((actualFlags & ExpectedFlag) == ExpectedFlag, $"Has expected flag {ExpectedFlag} but was {actualFlags}"); + } } } } @@ -430,9 +466,19 @@ public static void CustomRootTrustDoesNotTrustIntermediates( chain.ChainPolicy.ExtraStore.Add(rootCert); } - Assert.Equal(saveAllInCustomTrustStore, chain.Build(endEntityCert)); - Assert.Equal(3, chain.ChainElements.Count); - Assert.Equal(chainFlags, chain.AllStatusFlags()); + if (PlatformDetection.IsAndroid && !saveAllInCustomTrustStore) + { + // Android does not support an empty custom root trust + // Only self-issued certs are treated as trusted anchors, so building the chain + // should through PNSE even though the intermediate cert is added to the store + Assert.Throws(() => chain.Build(endEntityCert)); + } + else + { + Assert.Equal(saveAllInCustomTrustStore, chain.Build(endEntityCert)); + Assert.Equal(3, chain.ChainElements.Count); + Assert.Equal(chainFlags, chain.AllStatusFlags()); + } } } @@ -454,9 +500,17 @@ public static void CustomTrustModeWithNoCustomTrustCerts() chain.ChainPolicy.VerificationTime = endEntityCert.NotBefore.AddSeconds(1); chain.ChainPolicy.TrustMode = X509ChainTrustMode.CustomRootTrust; - Assert.False(chain.Build(endEntityCert)); - Assert.Equal(1, chain.ChainElements.Count); - Assert.Equal(X509ChainStatusFlags.PartialChain, chain.AllStatusFlags()); + if (PlatformDetection.IsAndroid) + { + // Android does not support an empty custom root trust + Assert.Throws(() => chain.Build(endEntityCert)); + } + else + { + Assert.False(chain.Build(endEntityCert)); + Assert.Equal(1, chain.ChainElements.Count); + Assert.Equal(X509ChainStatusFlags.PartialChain, chain.AllStatusFlags()); + } } } @@ -481,8 +535,16 @@ public static void NameConstraintViolation_ExcludedTree_Dns() SubjectAlternativeNameBuilder builder = new SubjectAlternativeNameBuilder(); builder.AddDnsName("www.example.com"); - // excluded DNS name constraint for example.com. + // excluded DNS name constraint for .example.com. string nameConstraints = "3012A110300E820C2E6578616D706C652E636F6D"; + if (PlatformDetection.IsAndroid) + { + // Android does not consider the constraint as being violated when it has + // the leading period. It checks expects the period as part of the left-side + // labels and not the constraint when doing validation. + // Use an excluded DNS name constraint without the period: example.com + nameConstraints = "3012A110300E820C6578616D706C652E636F6D"; + } TestNameConstrainedChain(nameConstraints, builder, (bool result, X509Chain chain) => { Assert.False(result, "chain.Build"); @@ -507,7 +569,9 @@ public static void NameConstraintViolation_PermittedTree_HasMin() } [Fact] - [PlatformSpecific(~TestPlatforms.Windows)] // Windows seems to skip over nonsense GeneralNames. + // Windows seems to skip over nonsense GeneralNames. + // Android will check for a match. Since the permitted name does match the subject alt name, it succeeds. + [PlatformSpecific(~(TestPlatforms.Windows | TestPlatforms.Android))] public static void NameConstraintViolation_InvalidGeneralNames() { SubjectAlternativeNameBuilder builder = new SubjectAlternativeNameBuilder(); @@ -761,9 +825,21 @@ public static void PolicyConstraints_Mapped() } } + private static X509ChainStatusFlags PlatformBasicConstraints(X509ChainStatusFlags flags) + { + if (OperatingSystem.IsAndroid()) + { + // Android always validates basic constraints as part of building a path + // so violations comes back as PartialChain with no elements. + flags = X509ChainStatusFlags.PartialChain; + } + + return flags; + } + private static X509ChainStatusFlags PlatformNameConstraints(X509ChainStatusFlags flags) { - if (RuntimeInformation.IsOSPlatform(OSPlatform.OSX)) + if (OperatingSystem.IsMacOS()) { const X509ChainStatusFlags AnyNameConstraintFlags = X509ChainStatusFlags.HasExcludedNameConstraint | @@ -778,13 +854,19 @@ private static X509ChainStatusFlags PlatformNameConstraints(X509ChainStatusFlags flags |= X509ChainStatusFlags.InvalidNameConstraints; } } + else if (OperatingSystem.IsAndroid()) + { + // Android always validates name constraints as part of building a path + // so violations comes back as PartialChain with no elements. + flags = X509ChainStatusFlags.PartialChain; + } return flags; } private static X509ChainStatusFlags PlatformPolicyConstraints(X509ChainStatusFlags flags) { - if (RuntimeInformation.IsOSPlatform(OSPlatform.OSX)) + if (OperatingSystem.IsMacOS()) { const X509ChainStatusFlags AnyPolicyConstraintFlags = X509ChainStatusFlags.NoIssuanceChainPolicy; @@ -795,6 +877,12 @@ private static X509ChainStatusFlags PlatformPolicyConstraints(X509ChainStatusFla flags |= X509ChainStatusFlags.InvalidPolicyConstraints; } } + else if (OperatingSystem.IsAndroid()) + { + // Android always validates policy constraints as part of building a path + // so violations comes back as PartialChain with no elements. + flags = X509ChainStatusFlags.PartialChain; + } return flags; } diff --git a/src/libraries/System.Security.Cryptography.X509Certificates/tests/TestData.cs b/src/libraries/System.Security.Cryptography.X509Certificates/tests/TestData.cs index ad689987d3aec..606232e892d83 100644 --- a/src/libraries/System.Security.Cryptography.X509Certificates/tests/TestData.cs +++ b/src/libraries/System.Security.Cryptography.X509Certificates/tests/TestData.cs @@ -636,6 +636,31 @@ public static SecureString CreatePfxDataPasswordSecureString() "3000302D021500B9316CC7E05C9F79197E0B41F6FD4E3FCEB72A8A0214075505" + "CCAECB18B7EF4C00F9C069FA3BC78014DE").HexToByteArray(); + public static byte[] CertWithEnhancedKeyUsage = ( + "308202BD308201A5A00302010202106144CAE1C8BF3049BB9BF62C08B74F1330" + + "0D06092A864886F70D01010B0500300E310C300A06035504031303466F6F301E" + + "170D3135303330313232343735385A170D3136303330313034343735385A300E" + + "310C300A06035504031303466F6F30820122300D06092A864886F70D01010105" + + "000382010F003082010A0282010100C2B02AED5493FDACE7F0D91118D3433F56" + + "A1320202CCABA9C4A78C8F6B60BC896F6E579E39453C2392C9C12AFC57C0D81E" + + "ACAA92186921A52FE3EC36769780EE60776CB3EEF1327197C05C1D539C105DBF" + + "DD3762C941E892F838DDFA87D0DD5642A1AC526E46CF4FF6F450ABD970189DC6" + + "90ABE886DB65F3B34F7F2F2C60A59A2F85E531C54D4B182878D14D79A9B88EB3" + + "29F14FFCEBBFA0DFDC97D3609C55F0A0D1D242E53B7143ACC64ABB7D985A1E43" + + "9EF3EF0A0F8074574CE48069E0499F9C23F4E496373C0F18EE7AFCA08BCE5993" + + "78637AAE51D2C57793FA899047BEA820D7E3FF1B935A32023BA13234F7D725C7" + + "A8F3C9C16757E19F4710DEE2C0B2E10203010001A317301530130603551D2504" + + "0C300A06082B06010505070303300D06092A864886F70D01010B050003820101" + + "0062A51D36C1E2945D4A7BC80385961084C2A600391D8A8D4F14572A0E994062" + + "6C4071B7533C2FB8894B55AFCEEE6C59E697244D43D136336DF04A595BB7C09F" + + "A2885C43926AA61BECA482EA5A4F62279C123206C8908F299DC41A62C80ECE98" + + "A36D1F12FEC80919E8DEB209C5B02EBD6D98BE00A84CB61781DC3C1585D79C91" + + "0490365DEB5B96899B1469B930CB1CE6874E256B45573F43D7DD16BD4F590B9A" + + "EEAA2E64987B64C6A96DDA5E6CA2EF96357B78ABFAF815DF4D021A15980EC7D9" + + "ECE244B1ADDF8878FC15236101BFA8E804ADA7C36DE088A16E3BEAABB65D1A91" + + "BA7010C7E3AA701A03B843A93AC1A291A791B3DBEEB511CBF73E2252691CBA33" + + "E2").HexToByteArray(); + public static byte[] CertWithPolicies = ( "308201f33082015ca0030201020210134fb7082cf69bbb4930bfc8e1ca446130" + "0d06092a864886f70d0101050500300e310c300a06035504031303466f6f301e" + From aa473153dd179b8e993c93f670f2a82698e97934 Mon Sep 17 00:00:00 2001 From: Elinor Fung Date: Thu, 11 Mar 2021 23:11:23 -0800 Subject: [PATCH 06/19] Handle revocation when newer APIs are available --- .../pal_jni.c | 28 ++- .../pal_jni.h | 15 ++ .../pal_x509chain.c | 167 ++++++++---------- .../Cryptography/Pal.Android/ChainPal.cs | 11 +- 4 files changed, 120 insertions(+), 101 deletions(-) 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 dfcac956e88b9..f0ee49fb3839c 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 @@ -116,6 +116,10 @@ jclass g_KeyStoreClass; jmethodID g_KeyStoreGetInstance; jmethodID g_KeyStoreLoad; +// java/security/Security +jclass g_Security; +jmethodID g_SecuritySetProperty; + // java/security/Signature jclass g_SignatureClass; jmethodID g_SignatureGetInstance; @@ -187,6 +191,13 @@ jclass g_PKIXReasonClass; jclass g_PKIXRevocationCheckerClass; jmethodID g_PKIXRevocationCheckerSetOptions; +// java/security/cert/PKIXRevocationChecker$Option - only in API level 24+ +jclass g_PKIXRevocationCheckerOptionClass; +jfieldID g_PKIXRevocationCheckerOptionNoFallback; +jfieldID g_PKIXRevocationCheckerOptionOnlyEndEntity; +jfieldID g_PKIXRevocationCheckerOptionPreferCrls; +jfieldID g_PKIXRevocationCheckerOptionSoftFail; + // java/security/cert/TrustAnchor jclass g_TrustAnchorClass; jclass g_TrustAnchorCtor; @@ -336,6 +347,10 @@ jclass g_IteratorClass; jmethodID g_IteratorHasNext; jmethodID g_IteratorNext; +// java/util/List +jclass g_ListClass; +jmethodID g_ListGet; + // java/util/Set jclass g_SetClass; jmethodID g_SetIterator; @@ -684,8 +699,14 @@ JNI_OnLoad(JavaVM *vm, void *reserved) if (g_CertPathValidatorGetRevocationChecker != NULL) { - g_PKIXRevocationCheckerClass = GetClassGRef(env, "java/security/cert/PKIXRevocationChecker"); - g_PKIXRevocationCheckerSetOptions = GetMethod(env, false, g_PKIXRevocationCheckerClass, "setOptions", "(Ljava/util/Set;)V"); + g_PKIXRevocationCheckerClass = GetClassGRef(env, "java/security/cert/PKIXRevocationChecker"); + g_PKIXRevocationCheckerSetOptions = GetMethod(env, false, g_PKIXRevocationCheckerClass, "setOptions", "(Ljava/util/Set;)V"); + + g_PKIXRevocationCheckerOptionClass = GetClassGRef(env, "java/security/cert/PKIXRevocationChecker$Option"); + g_PKIXRevocationCheckerOptionNoFallback = GetField(env, true, g_PKIXRevocationCheckerOptionClass, "NO_FALLBACK", "Ljava/security/cert/PKIXRevocationChecker$Option;"); + g_PKIXRevocationCheckerOptionOnlyEndEntity = GetField(env, true, g_PKIXRevocationCheckerOptionClass, "ONLY_END_ENTITY", "Ljava/security/cert/PKIXRevocationChecker$Option;"); + g_PKIXRevocationCheckerOptionPreferCrls = GetField(env, true, g_PKIXRevocationCheckerOptionClass, "PREFER_CRLS", "Ljava/security/cert/PKIXRevocationChecker$Option;"); + g_PKIXRevocationCheckerOptionSoftFail = GetField(env, true, g_PKIXRevocationCheckerOptionClass, "SOFT_FAIL", "Ljava/security/cert/PKIXRevocationChecker$Option;"); } g_TrustAnchorClass = GetClassGRef(env, "java/security/cert/TrustAnchor"); @@ -839,6 +860,9 @@ JNI_OnLoad(JavaVM *vm, void *reserved) g_IteratorHasNext = GetMethod(env, false, g_IteratorClass, "hasNext", "()Z"); g_IteratorNext = GetMethod(env, false, g_IteratorClass, "next", "()Ljava/lang/Object;"); + g_ListClass = GetClassGRef(env, "java/util/List"); + g_ListGet = GetMethod(env, false, g_ListClass, "get", "(I)Ljava/lang/Object;"); + g_SetClass = GetClassGRef(env, "java/util/Set"); g_SetIterator = GetMethod(env, false, g_SetClass, "iterator", "()Ljava/util/Iterator;"); diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native.Android/pal_jni.h b/src/libraries/Native/Unix/System.Security.Cryptography.Native.Android/pal_jni.h index 1aa2e19397d4a..d6936e3caedd9 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native.Android/pal_jni.h +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native.Android/pal_jni.h @@ -161,6 +161,13 @@ extern jclass g_PKIXReasonClass; extern jclass g_PKIXRevocationCheckerClass; extern jmethodID g_PKIXRevocationCheckerSetOptions; +// java/security/cert/PKIXRevocationChecker$Option - only in API level 24+ +extern jclass g_PKIXRevocationCheckerOptionClass; +extern jfieldID g_PKIXRevocationCheckerOptionNoFallback; +extern jfieldID g_PKIXRevocationCheckerOptionOnlyEndEntity; +extern jfieldID g_PKIXRevocationCheckerOptionPreferCrls; +extern jfieldID g_PKIXRevocationCheckerOptionSoftFail; + // java/security/cert/TrustAnchor extern jclass g_TrustAnchorClass; extern jclass g_TrustAnchorCtor; @@ -202,6 +209,10 @@ extern jclass g_KeyStoreClass; extern jmethodID g_KeyStoreGetInstance; extern jmethodID g_KeyStoreLoad; +// java/security/Security +extern jclass g_Security; +extern jmethodID g_SecuritySetProperty; + // java/security/Signature extern jclass g_SignatureClass; extern jmethodID g_SignatureGetInstance; @@ -345,6 +356,10 @@ extern jclass g_IteratorClass; extern jmethodID g_IteratorHasNext; extern jmethodID g_IteratorNext; +// java/util/List +extern jclass g_ListClass; +extern jmethodID g_ListGet; + // java/util/Set extern jclass g_SetClass; extern jmethodID g_SetIterator; 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 78c0753d02671..c59112a48cabd 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 @@ -182,34 +182,23 @@ int32_t AndroidCryptoNative_X509ChainGetCertificates(X509ChainContext* ctx, JNIEnv* env = GetJNIEnv(); int32_t ret = FAIL; - INIT_LOCALS(loc, certPathList, iter) // List certPathList = certPath.getCertificates(); - loc[certPathList] = (*env)->CallObjectMethod(env, ctx->certPath, g_CertPathGetCertificates); - int certCount = (int)(*env)->CallIntMethod(env, loc[certPathList], g_CollectionSize); - + jobject certPathList = (*env)->CallObjectMethod(env, ctx->certPath, g_CertPathGetCertificates); + int certCount = (int)(*env)->CallIntMethod(env, certPathList, g_CollectionSize); if (certsLen < certCount + 1) goto cleanup; - // int i = 0; - // Iterator iter = certs.iterator(); - // while (iter.hasNext()) { - // Certificate cert = iter.next(); - // out[i] = cert; - // i++; + // for (int i = 0; i < certPathList.size(); ++i) { + // Certificate cert = certPathList.get(i); + // certs[i] = cert; // } - int32_t i = 0; - loc[iter] = (*env)->CallObjectMethod(env, loc[certPathList], g_CollectionIterator); - jboolean hasNext = (*env)->CallBooleanMethod(env, loc[iter], g_IteratorHasNext); - while (hasNext) + int32_t i; + for (i = 0; i < certCount; ++i) { - jobject cert = (*env)->CallObjectMethod(env, loc[iter], g_IteratorNext); + jobject cert = (*env)->CallObjectMethod(env, certPathList, g_ListGet, i); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); - certs[i] = ToGRef(env, cert); - i++; - - hasNext = (*env)->CallBooleanMethod(env, loc[iter], g_IteratorHasNext); } // Certificate trustedCert = trustAnchor.getTrustedCert(); @@ -220,7 +209,7 @@ int32_t AndroidCryptoNative_X509ChainGetCertificates(X509ChainContext* ctx, ret = SUCCESS; cleanup: - RELEASE_LOCALS(loc, env) + (*env)->DeleteLocalRef(env, certPathList); return ret; } @@ -258,7 +247,8 @@ enum static PAL_X509ChainStatusFlags ChainStatusFromValidatorExceptionReason(JNIEnv* env, jobject reason) { int value = (*env)->CallIntMethod(env, reason, g_EnumOrdinal); - if (g_CertPathExceptionBasicReasonClass != NULL && (*env)->IsInstanceOf(env, reason, g_CertPathExceptionBasicReasonClass)) + if (g_CertPathExceptionBasicReasonClass != NULL + && (*env)->IsInstanceOf(env, reason, g_CertPathExceptionBasicReasonClass)) { switch (value) { @@ -335,41 +325,32 @@ static void PopulateValidationError(JNIEnv* env, jobject error, ValidationError* (*env)->DeleteLocalRef(env, message); } -int32_t AndroidCryptoNative_X509ChainGetErrors(X509ChainContext* ctx, - ValidationError* errors, - int32_t errorsLen) +int32_t AndroidCryptoNative_X509ChainGetErrors(X509ChainContext* ctx, ValidationError* errors, int32_t errorsLen) { assert(ctx != NULL); JNIEnv* env = GetJNIEnv(); int32_t ret = FAIL; - // int i = 0; - // Iterator iter = errorList.iterator(); - // while (iter.hasNext()) { - // Throwable error = iter.next(); + int32_t count = (*env)->CallIntMethod(env, ctx->errorList, g_CollectionSize); + if (errorsLen < count) + goto exit; + + // for (int i = 0; i < erroList.size(); ++i) { + // Throwable error = erroList.get(i); // << populate errors[i] >> - // i++; // } - int32_t i = 0; - jobject iter = (*env)->CallObjectMethod(env, ctx->errorList, g_CollectionIterator); - jboolean hasNext = (*env)->CallBooleanMethod(env, iter, g_IteratorHasNext); - while (hasNext) + for (int32_t i = 0; i < count; ++i) { - jobject error = (*env)->CallObjectMethod(env, iter, g_IteratorNext); - ON_EXCEPTION_PRINT_AND_GOTO(cleanup); - + jobject error = (*env)->CallObjectMethod(env, ctx->errorList, g_ListGet, i); + ON_EXCEPTION_PRINT_AND_GOTO(exit); PopulateValidationError(env, error, &errors[i]); - i++; - - hasNext = (*env)->CallBooleanMethod(env, iter, g_IteratorHasNext); (*env)->DeleteLocalRef(env, error); } ret = SUCCESS; -cleanup: - (*env)->DeleteLocalRef(env, iter); +exit: return ret; } @@ -405,40 +386,39 @@ bool AndroidCryptoNative_X509ChainSupportsRevocationOptions(void) return g_CertPathValidatorGetRevocationChecker != NULL && g_PKIXRevocationCheckerClass != NULL; } -static jobject /*CertPath*/ -GetCertPathFromBuilderResult(JNIEnv* env, jobject /*CertPath*/ certPath, jobject /*TrustAnchor*/ trustAnchor) +static jobject /*HashSet*/ +GetRevocationCheckerOptions(JNIEnv* env, PAL_X509RevocationMode revocationMode, PAL_X509RevocationFlag revocationFlag) { - jobject ret = NULL; - INIT_LOCALS(loc, certPathList, certList, trustedCert, certFactoryType, certFactory) - - // List certPathList = certPath.getCertificates(); - loc[certPathList] = (*env)->CallObjectMethod(env, certPath, g_CertPathGetCertificates); - - // The result cert path does not include the trust anchor. Create a list combining the path and anchor. - // ArrayList certList = new ArrayList(certPathList); - loc[certList] = (*env)->NewObject(env, g_ArrayListClass, g_ArrayListCtorWithCollection, loc[certPathList]); + assert(AndroidCryptoNative_X509ChainSupportsRevocationOptions()); - // Certificate trustedCert = trustAnchor.getTrustedCert(); - // certList.add(trustedCert); - loc[trustedCert] = (*env)->CallObjectMethod(env, trustAnchor, g_TrustAnchorGetTrustedCert); - (*env)->CallBooleanMethod(env, loc[certList], g_ArrayListAdd, loc[trustedCert]); - - // CertificateFactory certFactory = CertificateFactory.getInstance("X.509"); - loc[certFactoryType] = JSTRING("X.509"); - loc[certFactory] = - (*env)->CallStaticObjectMethod(env, g_CertFactoryClass, g_CertFactoryGetInstance, loc[certFactoryType]); - ON_EXCEPTION_PRINT_AND_GOTO(cleanup); + // HashSet options = new HashSet(3); + jobject options = (*env)->NewObject(env, g_HashSetClass, g_HashSetCtorWithCapacity, 3); - // CertPath certPathWithAnchor = certFactory.generateCertPath(certList); - jobject certPathWithAnchor = - (*env)->CallObjectMethod(env, loc[certFactory], g_CertFactoryGenerateCertPathFromList, loc[certList]); - ON_EXCEPTION_PRINT_AND_GOTO(cleanup); + if (revocationMode == X509RevocationMode_Offline) + { + // options.add(PKIXRevocationChecker.Option.PREFER_CRLS); + jobject preferCrls = (*env)->GetStaticObjectField( + env, g_PKIXRevocationCheckerOptionClass, g_PKIXRevocationCheckerOptionPreferCrls); + (*env)->CallBooleanMethod(env, options, g_HashSetAdd, preferCrls); + (*env)->DeleteLocalRef(env, preferCrls); + + // options.add(PKIXRevocationChecker.Option.NO_FALLBACK); + jobject noFallback = (*env)->GetStaticObjectField( + env, g_PKIXRevocationCheckerOptionClass, g_PKIXRevocationCheckerOptionNoFallback); + (*env)->CallBooleanMethod(env, options, g_HashSetAdd, noFallback); + (*env)->DeleteLocalRef(env, noFallback); + } - ret = certPathWithAnchor; + if (revocationFlag == X509RevocationFlag_EndCertificateOnly) + { + // options.add(PKIXRevocationChecker.Option.ONLY_END_ENTITY); + jobject endOnly = (*env)->GetStaticObjectField( + env, g_PKIXRevocationCheckerOptionClass, g_PKIXRevocationCheckerOptionOnlyEndEntity); + (*env)->CallBooleanMethod(env, options, g_HashSetAdd, endOnly); + (*env)->DeleteLocalRef(env, endOnly); + } -cleanup: - RELEASE_LOCALS(loc, env) - return ret; + return options; } int32_t AndroidCryptoNative_X509ChainValidate(X509ChainContext* ctx, @@ -449,38 +429,47 @@ int32_t AndroidCryptoNative_X509ChainValidate(X509ChainContext* ctx, JNIEnv* env = GetJNIEnv(); int32_t ret = FAIL; - INIT_LOCALS(loc, validatorType, validator, result, ex) + INIT_LOCALS(loc, validatorType, validator, checker, result, ex) + + jobject params = ctx->params; + jobject certPath = ctx->certPath; + + // String validatorType = "PKIX"; + // CertPathValidator validator = CertPathValidator.getInstance(validatorType); + // PKIXCertPathValidatorResult result = (PKIXCertPathValidatorResult)validator.validate(certPath, params); + loc[validatorType] = JSTRING("PKIX"); + loc[validator] = (*env)->CallStaticObjectMethod( + env, g_CertPathValidatorClass, g_CertPathValidatorGetInstance, loc[validatorType]); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); bool checkRevocation = revocationMode != X509RevocationMode_NoCheck; + + // params.setRevocationEnabled(checkRevocation); + (*env)->CallVoidMethod(env, params, g_PKIXBuilderParametersSetRevocationEnabled, checkRevocation); if (checkRevocation) { - // bool isOnline = revocationMode == X509RevocationMode_Online; if (AndroidCryptoNative_X509ChainSupportsRevocationOptions()) { - // TODO: [AndroidCrypto] Deal with revocation options + // PKIXRevocationChecker checker = validator.getRevocationChecker(); + jobject checker = (*env)->CallObjectMethod(env, loc[validator], g_CertPathValidatorGetRevocationChecker); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); + + // checker.setOptions(options); + // params.addCertPathChecker(checker); + jobject options = GetRevocationCheckerOptions(env, revocationMode, revocationFlag); + (*env)->CallVoidMethod(env, checker, g_PKIXRevocationCheckerSetOptions, options); + (*env)->CallVoidMethod(env, params, g_PKIXBuilderParametersAddCertPathChecker, checker); + + (*env)->DeleteLocalRef(env, options); + (*env)->DeleteLocalRef(env, checker); } else { + // TODO: [AndroidCrypto] Handle options // Security.setProperty("oscp.enable", isOnline); } } - // bool entireChain = checkRevocation && revocationFlag == X509RevocationFlag_EntireChain; - - jobject params = ctx->params; - jobject certPath = ctx->certPath; - - // params.setRevocationEnabled(checkRevocation); - (*env)->CallVoidMethod(env, params, g_PKIXBuilderParametersSetRevocationEnabled, checkRevocation); - - // String validatorType = "PKIX"; - // CertPathValidator validator = CertPathValidator.getInstance(validatorType); - // PKIXCertPathValidatorResult result = (PKIXCertPathValidatorResult)validator.validate(certPath, params); - loc[validatorType] = JSTRING("PKIX"); - loc[validator] = (*env)->CallStaticObjectMethod( - env, g_CertPathValidatorClass, g_CertPathValidatorGetInstance, loc[validatorType]); - ON_EXCEPTION_PRINT_AND_GOTO(cleanup); - loc[result] = (*env)->CallObjectMethod(env, loc[validator], g_CertPathValidatorValidate, certPath, params); if (TryGetJNIException(env, &loc[ex], false /*printException*/)) { diff --git a/src/libraries/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Android/ChainPal.cs b/src/libraries/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Android/ChainPal.cs index 11b1686668a73..2c4e71e464105 100644 --- a/src/libraries/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Android/ChainPal.cs +++ b/src/libraries/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Android/ChainPal.cs @@ -193,21 +193,12 @@ internal void Evaluate( if (revocationMode != X509RevocationMode.NoCheck) { - // TODO: [AndroidCrypto] Handle revocation modes/flags if (revocationFlag == X509RevocationFlag.EntireChain) { throw new NotImplementedException($"{nameof(Evaluate)} (X509RevocationFlag.{revocationFlag})"); } - if (Interop.AndroidCrypto.X509ChainSupportsRevocationOptions()) - { - // Defaults to online when revocation options are available - if (revocationMode == X509RevocationMode.Offline) - { - throw new NotImplementedException($"{nameof(Evaluate)} (X509RevocationMode.{revocationMode})"); - } - } - else + if (!Interop.AndroidCrypto.X509ChainSupportsRevocationOptions()) { if (revocationFlag == X509RevocationFlag.EndCertificateOnly) { From ff5f4bdb2bae59a49dfac12fa5817f1484f2e870 Mon Sep 17 00:00:00 2001 From: Elinor Fung Date: Mon, 15 Mar 2021 11:37:33 -0700 Subject: [PATCH 07/19] Apply suggestions from code review Co-authored-by: Jeremy Koritzinsky --- .../Unix/System.Security.Cryptography.Native.Android/pal_jni.c | 2 +- .../tests/ChainTests.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) 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 f0ee49fb3839c..f41274bb9bede 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 @@ -470,7 +470,7 @@ static jclass GetOptionalClassGRef(JNIEnv *env, const char* name) jclass klass = NULL; if (!TryGetClassGRef(env, name, &klass)) { - LOG_ERROR("optional class %s was not found", name); + LOG_DEBUG("optional class %s was not found", name); } return klass; diff --git a/src/libraries/System.Security.Cryptography.X509Certificates/tests/ChainTests.cs b/src/libraries/System.Security.Cryptography.X509Certificates/tests/ChainTests.cs index b50216fdf3a17..9d80bf91fd32d 100644 --- a/src/libraries/System.Security.Cryptography.X509Certificates/tests/ChainTests.cs +++ b/src/libraries/System.Security.Cryptography.X509Certificates/tests/ChainTests.cs @@ -258,7 +258,7 @@ public static void SystemTrustCertificateWithCustomRootTrust(bool addCertificate { // Android does not support an empty custom root trust // Only self-issued certs are treated as trusted anchors, so building the chain - // should through PNSE regardless of whether or not testCert is added to the store + // should throw PNSE regardless of whether or not testCert is added to the store Assert.Throws(() => chain.Build(microsoftDotCom)); } else From ecb6936cb690d35ddece67ef5e57d7e3c89480ca Mon Sep 17 00:00:00 2001 From: Elinor Fung Date: Mon, 15 Mar 2021 11:41:37 -0700 Subject: [PATCH 08/19] Move INIT_LOCALS, RELEASE_LOCALS, RELEASE_LOCALS_ENV macros to pal_jni --- .../pal_dsa.c | 16 ++-------- .../pal_ecc_import_export.c | 13 -------- .../pal_jni.h | 22 ++++++++++++++ .../pal_x509.c | 30 +++++-------------- .../pal_x509chain.c | 26 ++++------------ .../tests/ChainTests.cs | 3 +- 6 files changed, 40 insertions(+), 70 deletions(-) diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native.Android/pal_dsa.c b/src/libraries/Native/Unix/System.Security.Cryptography.Native.Android/pal_dsa.c index 7fcf57658bf54..68bcc91786f25 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native.Android/pal_dsa.c +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native.Android/pal_dsa.c @@ -6,18 +6,6 @@ #include "pal_signature.h" #include "pal_bignum.h" -#define INIT_LOCALS(name, ...) \ - enum { __VA_ARGS__, count_##name }; \ - jobject name[count_##name] = { 0 } \ - -#define RELEASE_LOCALS_ENV(name, releaseFn) \ -do { \ - for (int i = 0; i < count_##name; ++i) \ - { \ - releaseFn(env, name[i]); \ - } \ -} while(0) - int32_t AndroidCryptoNative_DsaGenerateKey(jobject* dsa, int32_t bits) { assert(dsa); @@ -225,7 +213,7 @@ int32_t AndroidCryptoNative_GetDsaParameters( assert(gLength); assert(yLength); assert(xLength); - + JNIEnv* env = GetJNIEnv(); INIT_LOCALS(loc, algName, keyFactory, publicKey, publicKeySpec, privateKey, privateKeySpec); @@ -282,7 +270,7 @@ int32_t AndroidCryptoNative_DsaKeyCreateByExplicitParameters( assert(false); return 0; } - + JNIEnv* env = GetJNIEnv(); INIT_LOCALS(bn, P, Q, G, Y, X); 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 6bce31af2df42..db0edce61dc9f 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 @@ -7,19 +7,6 @@ #include "pal_jni.h" #include "pal_utilities.h" - -#define INIT_LOCALS(name, ...) \ - enum { __VA_ARGS__, count_##name }; \ - jobject name[count_##name] = { 0 } \ - -#define RELEASE_LOCALS_ENV(name, releaseFn) \ -do { \ - for (int i = 0; i < count_##name; ++i) \ - { \ - releaseFn(env, name[i]); \ - } \ -} while(0) - int32_t AndroidCryptoNative_GetECKeyParameters(const EC_KEY* key, int32_t includePrivate, jobject* qx, diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native.Android/pal_jni.h b/src/libraries/Native/Unix/System.Security.Cryptography.Native.Android/pal_jni.h index d6936e3caedd9..e2077ab510701 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native.Android/pal_jni.h +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native.Android/pal_jni.h @@ -427,6 +427,28 @@ extern jmethodID g_KeyAgreementGenerateSecret; #define JSTRING(str) ((jstring)(*env)->NewStringUTF(env, str)) #define ON_EXCEPTION_PRINT_AND_GOTO(label) if (CheckJNIExceptions(env)) goto label +#define INIT_LOCALS(name, ...) \ + enum { __VA_ARGS__, count_##name }; \ + jobject name[count_##name] = { 0 } \ + +#define RELEASE_LOCALS(name, env) \ +do { \ + for (int i_##name = 0; i_##name < count_##name; ++i_##name) \ + { \ + jobject local = name[i_##name]; \ + if (local != NULL) \ + (*env)->DeleteLocalRef(env, local); \ + } \ +} while(0) + +#define RELEASE_LOCALS_ENV(name, releaseFn) \ +do { \ + for (int i = 0; i < count_##name; ++i) \ + { \ + releaseFn(env, name[i]); \ + } \ +} while(0) + void SaveTo(uint8_t* src, uint8_t** dst, size_t len, bool overwrite); jobject ToGRef(JNIEnv *env, jobject lref); jobject AddGRef(JNIEnv *env, jobject gref); 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 17120c5883f62..cb74fa0116609 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 @@ -10,20 +10,6 @@ #include #include -#define INIT_LOCALS(name, ...) \ - enum { __VA_ARGS__, count_##name }; \ - jobject name[count_##name] = { 0 }; \ - -#define RELEASE_LOCALS(name, env) \ -{ \ - for (int i_##name = 0; i_##name < count_##name; ++i_##name) \ - { \ - jobject local = name[i_##name]; \ - if (local != NULL) \ - (*env)->DeleteLocalRef(env, local); \ - } \ -} \ - #define INSUFFICIENT_BUFFER -1 static int32_t PopulateByteArray(JNIEnv* env, jbyteArray source, uint8_t* dest, int32_t* len); @@ -35,7 +21,7 @@ jobject /*X509Certificate*/ AndroidCryptoNative_X509Decode(const uint8_t* buf, i JNIEnv* env = GetJNIEnv(); jobject ret = NULL; - INIT_LOCALS(loc, bytes, stream, certType, certFactory) + INIT_LOCALS(loc, bytes, stream, certType, certFactory); // byte[] bytes = new byte[] { ... } // InputStream stream = new ByteArrayInputStream(bytes); @@ -57,7 +43,7 @@ jobject /*X509Certificate*/ AndroidCryptoNative_X509Decode(const uint8_t* buf, i ret = ToGRef(env, ret); cleanup: - RELEASE_LOCALS(loc, env) + RELEASE_LOCALS(loc, env); return ret; } @@ -88,7 +74,7 @@ int32_t AndroidCryptoNative_X509DecodeCollection(const uint8_t* buf, JNIEnv* env = GetJNIEnv(); int32_t ret = FAIL; - INIT_LOCALS(loc, bytes, stream, certType, certFactory, certs, iter) + INIT_LOCALS(loc, bytes, stream, certType, certFactory, certs, iter); // byte[] bytes = new byte[] { ... } // InputStream stream = new ByteArrayInputStream(bytes); @@ -150,7 +136,7 @@ int32_t AndroidCryptoNative_X509DecodeCollection(const uint8_t* buf, ret = SUCCESS; cleanup: - RELEASE_LOCALS(loc, env) + RELEASE_LOCALS(loc, env); return ret; } @@ -164,7 +150,7 @@ int32_t AndroidCryptoNative_X509ExportPkcs7(jobject* /*X509Certificate[]*/ certs JNIEnv* env = GetJNIEnv(); int32_t ret = FAIL; - INIT_LOCALS(loc, certList, certType, certFactory, certPath, pkcs7Type, encoded) + INIT_LOCALS(loc, certList, certType, certFactory, certPath, pkcs7Type, encoded); // ArrayList certList = new ArrayList(); // foreach (Certificate cert in certs) @@ -192,7 +178,7 @@ int32_t AndroidCryptoNative_X509ExportPkcs7(jobject* /*X509Certificate[]*/ certs ret = PopulateByteArray(env, loc[encoded], out, outLen); cleanup: - RELEASE_LOCALS(loc, env) + RELEASE_LOCALS(loc, env); return ret; } @@ -202,7 +188,7 @@ PAL_X509ContentType AndroidCryptoNative_X509GetContentType(const uint8_t* buf, i JNIEnv* env = GetJNIEnv(); PAL_X509ContentType ret = PAL_X509Unknown; - INIT_LOCALS(loc, bytes, stream, certType, certFactory, pkcs7Type, certPath, cert) + INIT_LOCALS(loc, bytes, stream, certType, certFactory, pkcs7Type, certPath, cert); // This function checks: // - PKCS7 DER/PEM @@ -242,7 +228,7 @@ PAL_X509ContentType AndroidCryptoNative_X509GetContentType(const uint8_t* buf, i } cleanup: - RELEASE_LOCALS(loc, env) + RELEASE_LOCALS(loc, env); return ret; } 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 c59112a48cabd..285f21ce68005 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 @@ -8,20 +8,6 @@ #include #include -#define INIT_LOCALS(name, ...) \ - enum { __VA_ARGS__, count_##name }; \ - jobject name[count_##name] = { 0 }; \ - -#define RELEASE_LOCALS(name, env) \ -{ \ - for (int i_##name = 0; i_##name < count_##name; ++i_##name) \ - { \ - jobject local = name[i_##name]; \ - if (local != NULL) \ - (*env)->DeleteLocalRef(env, local); \ - } \ -} \ - struct X509ChainContext_t { jobject /*PKIXBuilderParameters*/ params; @@ -47,7 +33,7 @@ X509ChainContext* AndroidCryptoNative_X509ChainCreateContext(jobject /*X509Certi JNIEnv* env = GetJNIEnv(); X509ChainContext* ret = NULL; - INIT_LOCALS(loc, keyStoreType, keyStore, targetSel, params, certList, certStoreType, certStoreParams, certStore) + INIT_LOCALS(loc, keyStoreType, keyStore, targetSel, params, certList, certStoreType, certStoreParams, certStore); // String keyStoreType = "AndroidCAStore"; // KeyStore keyStore = KeyStore.getInstance(keyStoreType); @@ -99,7 +85,7 @@ X509ChainContext* AndroidCryptoNative_X509ChainCreateContext(jobject /*X509Certi ret->errorList = ToGRef(env, (*env)->NewObject(env, g_ArrayListClass, g_ArrayListCtor)); cleanup: - RELEASE_LOCALS(loc, env) + RELEASE_LOCALS(loc, env); return ret; } @@ -122,7 +108,7 @@ int32_t AndroidCryptoNative_X509ChainBuild(X509ChainContext* ctx, int64_t timeIn JNIEnv* env = GetJNIEnv(); int32_t ret = FAIL; - INIT_LOCALS(loc, date, builderType, builder, result, ex, certPath, trustAnchor) + INIT_LOCALS(loc, date, builderType, builder, result, ex, certPath, trustAnchor); jobject params = ctx->params; @@ -157,7 +143,7 @@ int32_t AndroidCryptoNative_X509ChainBuild(X509ChainContext* ctx, int64_t timeIn ret = SUCCESS; cleanup: - RELEASE_LOCALS(loc, env) + RELEASE_LOCALS(loc, env); return ret; } @@ -429,7 +415,7 @@ int32_t AndroidCryptoNative_X509ChainValidate(X509ChainContext* ctx, JNIEnv* env = GetJNIEnv(); int32_t ret = FAIL; - INIT_LOCALS(loc, validatorType, validator, checker, result, ex) + INIT_LOCALS(loc, validatorType, validator, checker, result, ex); jobject params = ctx->params; jobject certPath = ctx->certPath; @@ -479,6 +465,6 @@ int32_t AndroidCryptoNative_X509ChainValidate(X509ChainContext* ctx, ret = SUCCESS; cleanup: - RELEASE_LOCALS(loc, env) + RELEASE_LOCALS(loc, env); return ret; } diff --git a/src/libraries/System.Security.Cryptography.X509Certificates/tests/ChainTests.cs b/src/libraries/System.Security.Cryptography.X509Certificates/tests/ChainTests.cs index 9d80bf91fd32d..c20df132805d1 100644 --- a/src/libraries/System.Security.Cryptography.X509Certificates/tests/ChainTests.cs +++ b/src/libraries/System.Security.Cryptography.X509Certificates/tests/ChainTests.cs @@ -181,7 +181,8 @@ public static void TestResetMethod() Assert.False(valid); chainHolder.DisposeChainElements(); - // Make sure AllowUnknownCertificateAuthority is set, as this test we verifies it does not get reset + // This test checks that the verification flags do not get reset when the chain is reset, + // so we set AllowUnknownCertificateAuthority even on platforms that don't respect it. chain.ChainPolicy.VerificationFlags = X509VerificationFlags.AllowUnknownCertificateAuthority; chain.AllowUnknownAuthorityOrAddSelfSignedToCustomTrust(sampleCert); From 319113046cfe43fa575f33a6ffffa72604538d4e Mon Sep 17 00:00:00 2001 From: Elinor Fung Date: Wed, 17 Mar 2021 16:23:21 -0700 Subject: [PATCH 09/19] Apply suggestions from code review Co-authored-by: Jeremy Barton --- .../tests/ChainTests.cs | 2 +- .../tests/DynamicChainTests.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Security.Cryptography.X509Certificates/tests/ChainTests.cs b/src/libraries/System.Security.Cryptography.X509Certificates/tests/ChainTests.cs index c20df132805d1..52a275ad34a5f 100644 --- a/src/libraries/System.Security.Cryptography.X509Certificates/tests/ChainTests.cs +++ b/src/libraries/System.Security.Cryptography.X509Certificates/tests/ChainTests.cs @@ -348,7 +348,7 @@ public static void BuildChainCustomTrustStore( } [Fact] - public static void BuildChainWithSBystemTrustAndCustomTrustCertificates() + public static void BuildChainWithSystemTrustAndCustomTrustCertificates() { using (var testCert = new X509Certificate2(TestFiles.ChainPfxFile, TestData.ChainPfxPassword)) using (var chainHolder = new ChainHolder()) diff --git a/src/libraries/System.Security.Cryptography.X509Certificates/tests/DynamicChainTests.cs b/src/libraries/System.Security.Cryptography.X509Certificates/tests/DynamicChainTests.cs index 02c0bdc117f2a..a82c958c3b2c6 100644 --- a/src/libraries/System.Security.Cryptography.X509Certificates/tests/DynamicChainTests.cs +++ b/src/libraries/System.Security.Cryptography.X509Certificates/tests/DynamicChainTests.cs @@ -543,7 +543,7 @@ public static void NameConstraintViolation_ExcludedTree_Dns() // the leading period. It checks expects the period as part of the left-side // labels and not the constraint when doing validation. // Use an excluded DNS name constraint without the period: example.com - nameConstraints = "3012A110300E820C6578616D706C652E636F6D"; + nameConstraints = "3011A10F300D820B6578616D706C652E636F6D"; } TestNameConstrainedChain(nameConstraints, builder, (bool result, X509Chain chain) => { From 8a16ced573f5d38675f83e146504ca9d7346590e Mon Sep 17 00:00:00 2001 From: Elinor Fung Date: Fri, 19 Mar 2021 21:16:59 -0700 Subject: [PATCH 10/19] Stop throwing PNSE on verification/revocation flags that aren't respected Treat X509RevocationFlag EntireChain as ExcludeRoot If revocation options aren't available, treat X509RevocationFlag EndCertificateOnly as ExcludeRoot --- .../pal_x509chain.c | 11 +++++- .../Cryptography/Pal.Android/ChainPal.cs | 39 +++++-------------- .../tests/ChainTests.cs | 12 +++--- 3 files changed, 24 insertions(+), 38 deletions(-) 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 285f21ce68005..764a99daab6f5 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 @@ -434,6 +434,11 @@ int32_t AndroidCryptoNative_X509ChainValidate(X509ChainContext* ctx, (*env)->CallVoidMethod(env, params, g_PKIXBuilderParametersSetRevocationEnabled, checkRevocation); if (checkRevocation) { + if (revocationFlag == X509RevocationFlag_EntireChain) + { + LOG_INFO("Treating revocation flag 'EntireChain' as 'ExcludeRoot'. Revocation will not be checked for the root certificate."); + } + if (AndroidCryptoNative_X509ChainSupportsRevocationOptions()) { // PKIXRevocationChecker checker = validator.getRevocationChecker(); @@ -451,8 +456,10 @@ int32_t AndroidCryptoNative_X509ChainValidate(X509ChainContext* ctx, } else { - // TODO: [AndroidCrypto] Handle options - // Security.setProperty("oscp.enable", isOnline); + if (revocationFlag == X509RevocationFlag_EndCertificateOnly) + { + LOG_INFO("Treating revocation flag 'EndCertificateOnly' as 'ExcludeRoot'. Revocation will be checked for non-end certificates."); + } } } diff --git a/src/libraries/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Android/ChainPal.cs b/src/libraries/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Android/ChainPal.cs index 2c4e71e464105..2c7c16bafb6b4 100644 --- a/src/libraries/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Android/ChainPal.cs +++ b/src/libraries/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Android/ChainPal.cs @@ -78,25 +78,11 @@ public void Dispose() if (!_isValid) { - // There is no way to bypass time, trusted root, name, or policy constraint - // validation on Android. It will not build any chain without these all - // being valid. - X509VerificationFlags[] unsupportedFlags = new X509VerificationFlags[] - { - X509VerificationFlags.IgnoreNotTimeValid, - X509VerificationFlags.AllowUnknownCertificateAuthority, - X509VerificationFlags.IgnoreInvalidName, - X509VerificationFlags.IgnoreInvalidPolicy, - }; - foreach (X509VerificationFlags unsupported in unsupportedFlags) - { - if ((flags & unsupported) == unsupported) - { - exception = new PlatformNotSupportedException(SR.Format(SR.Chain_VerificationFlagNotSupported, unsupported)); - return default(bool?); - } - } - + // There is no way to bypass certain validation - time, trusted root, name, + // policy constraint - on Android. It will not build any chain without these + // all being valid. This will be an empty chain with PartialChain status. + Debug.Assert(ChainElements!.Length == 0); + Debug.Assert(ChainStatus!.Length > 0 && (ChainStatus[0].Status & X509ChainStatusFlags.PartialChain) == X509ChainStatusFlags.PartialChain); return false; } @@ -118,6 +104,10 @@ internal void Initialize( } } + Debug.Assert( + trustMode == X509ChainTrustMode.System || trustMode == X509ChainTrustMode.CustomRootTrust, + "Unsupported trust mode. Only System and CustomRootTrust are currently handled"); + List customTrustCerts = new List(); bool useCustomRootTrust = trustMode == X509ChainTrustMode.CustomRootTrust; if (useCustomRootTrust && customTrustStore != null) @@ -193,19 +183,8 @@ internal void Evaluate( if (revocationMode != X509RevocationMode.NoCheck) { - if (revocationFlag == X509RevocationFlag.EntireChain) - { - throw new NotImplementedException($"{nameof(Evaluate)} (X509RevocationFlag.{revocationFlag})"); - } - if (!Interop.AndroidCrypto.X509ChainSupportsRevocationOptions()) { - if (revocationFlag == X509RevocationFlag.EndCertificateOnly) - { - // No way to specfiy end certificate only if revocation options are not available - throw new PlatformNotSupportedException(SR.Format(SR.Chain_SettingNotSupported, $"{nameof(X509RevocationFlag)}.{nameof(X509RevocationFlag.EndCertificateOnly)}")); - } - // Defaults to offline when revocation options are not available if (revocationMode == X509RevocationMode.Online) { diff --git a/src/libraries/System.Security.Cryptography.X509Certificates/tests/ChainTests.cs b/src/libraries/System.Security.Cryptography.X509Certificates/tests/ChainTests.cs index 52a275ad34a5f..cce865f2c40b7 100644 --- a/src/libraries/System.Security.Cryptography.X509Certificates/tests/ChainTests.cs +++ b/src/libraries/System.Security.Cryptography.X509Certificates/tests/ChainTests.cs @@ -840,8 +840,8 @@ public static void ChainErrorsAtMultipleLayers() // // These certificates have been hard-coded to enable the scenario on // netstandard. - byte[] endEntityBytes = Encoding.ASCII.GetBytes( -@"-----BEGIN CERTIFICATE----- + byte[] endEntityBytes = Encoding.ASCII.GetBytes(@" +-----BEGIN CERTIFICATE----- MIIC6DCCAdCgAwIBAgIQAKjmD7+TWUwQN2ucajn9kTANBgkqhkiG9w0BAQsFADAXMRUwEwYDVQQD EwxJbnRlcm1lZGlhdGUwHhcNMTkwMzAzMjM1NzA3WhcNMTkwNjAzMjM1NzA3WjAVMRMwEQYDVQQD EwpFbmQtRW50aXR5MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAxTybBkpMdQ8IeL1C @@ -857,8 +857,8 @@ public static void ChainErrorsAtMultipleLayers() psHHsU9xg0o7L2WXD5qYhD2JCQIVWNRmRZCf1luWlKqUaqWWONMJ44hk8Md+ohxpyCRmbtLRZPzd wlkQzPsc9A== -----END CERTIFICATE-----"); - byte[] intermediateBytes = Encoding.ASCII.GetBytes( -@"-----BEGIN CERTIFICATE----- + byte[] intermediateBytes = Encoding.ASCII.GetBytes(@" +-----BEGIN CERTIFICATE----- MIIC1DCCAbygAwIBAgIPRoY1rB2tMVJeYB4GILkNMA0GCSqGSIb3DQEBCwUAMBQxEjAQBgNVBAMT CVRlc3QgUm9vdDAeFw0xOTAyMTgyMzU3MDdaFw0yMDAyMTgyMzU3MDdaMBcxFTATBgNVBAMTDElu dGVybWVkaWF0ZTCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBALxYzEN6nYvQ0TOg/jOF @@ -873,8 +873,8 @@ public static void ChainErrorsAtMultipleLayers() ueuTl2qTtbBh015GuEld61EBXSBLIUqwOAeFYrNJbC4J2mXgnLTWC380cBf5KWeSdjLYgk2sZ1V4 FKKQecZIhxdlDGzMAbbmEV+2EqS+As2C7+y4dkpG4nnbQe/4AFr8vekHdrI= -----END CERTIFICATE-----"); - byte[] rootBytes = Encoding.ASCII.GetBytes( -@"-----BEGIN CERTIFICATE----- + byte[] rootBytes = Encoding.ASCII.GetBytes(@" +-----BEGIN CERTIFICATE----- MIICyjCCAbKgAwIBAgIIKKt3K3rRbvQwDQYJKoZIhvcNAQELBQAwFDESMBAGA1UEAxMJVGVzdCBS b290MB4XDTE5MDIwNDIzNTcwN1oXDTIxMDIwNDIzNTcwN1owFDESMBAGA1UEAxMJVGVzdCBSb290 MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAiM7tv4YvqmWYGF1vbeM2cQWV1NVBxKU4 From f08663f225d4e33258a26c07f5986497b5f46cb5 Mon Sep 17 00:00:00 2001 From: Elinor Fung Date: Sat, 20 Mar 2021 03:12:05 -0700 Subject: [PATCH 11/19] Update tests based on lack of AIA fetching support --- .../tests/ChainTests.cs | 108 ++++++++++++------ .../tests/DynamicChainTests.cs | 1 + .../tests/RevocationTests/AiaTests.cs | 1 + 3 files changed, 77 insertions(+), 33 deletions(-) diff --git a/src/libraries/System.Security.Cryptography.X509Certificates/tests/ChainTests.cs b/src/libraries/System.Security.Cryptography.X509Certificates/tests/ChainTests.cs index cce865f2c40b7..0ff97c6cbe489 100644 --- a/src/libraries/System.Security.Cryptography.X509Certificates/tests/ChainTests.cs +++ b/src/libraries/System.Security.Cryptography.X509Certificates/tests/ChainTests.cs @@ -289,11 +289,22 @@ public enum BuildChainCustomTrustStoreTestArguments : int MultipleCalls } + public static IEnumerable BuildChainCustomTrustStoreData() + { + if (!PlatformDetection.IsAndroid) + { + // Android doesn't support an empty custom root + yield return new object[] { false, X509ChainStatusFlags.UntrustedRoot, BuildChainCustomTrustStoreTestArguments.TrustedIntermediateUntrustedRoot }; + } + + yield return new object[] { true, X509ChainStatusFlags.NoError, BuildChainCustomTrustStoreTestArguments.UntrustedIntermediateTrustedRoot }; + yield return new object[] { true, X509ChainStatusFlags.NoError, BuildChainCustomTrustStoreTestArguments.TrustedIntermediateTrustedRoot }; + yield return new object[] { true, X509ChainStatusFlags.NoError, BuildChainCustomTrustStoreTestArguments.MultipleCalls }; + } + [Theory] - [InlineData(false, X509ChainStatusFlags.UntrustedRoot, BuildChainCustomTrustStoreTestArguments.TrustedIntermediateUntrustedRoot)] - [InlineData(true, X509ChainStatusFlags.NoError, BuildChainCustomTrustStoreTestArguments.UntrustedIntermediateTrustedRoot)] - [InlineData(true, X509ChainStatusFlags.NoError, BuildChainCustomTrustStoreTestArguments.TrustedIntermediateTrustedRoot)] - [InlineData(true, X509ChainStatusFlags.NoError, BuildChainCustomTrustStoreTestArguments.MultipleCalls)] + [PlatformSpecific(~TestPlatforms.Android)] // Test relies on AIA fetching, which Android does not support + [MemberData(nameof(BuildChainCustomTrustStoreData))] public static void BuildChainCustomTrustStore( bool chainBuildsSuccessfully, X509ChainStatusFlags chainFlags, @@ -313,38 +324,68 @@ public static void BuildChainCustomTrustStore( using (var chainHolderTest = new ChainHolder()) { - X509Chain chainTest = chainHolderTest.Chain; - chainTest.ChainPolicy.RevocationMode = X509RevocationMode.NoCheck; - chainTest.ChainPolicy.VerificationTime = microsoftDotCom.NotBefore.AddSeconds(1); - chainTest.ChainPolicy.TrustMode = X509ChainTrustMode.CustomRootTrust; + BuildChainCustomTrustStoreImpl(chainHolderTest, rootCert, microsoftDotCom, chainBuildsSuccessfully, chainFlags, testArguments); + } + } + } - switch (testArguments) - { - case BuildChainCustomTrustStoreTestArguments.TrustedIntermediateUntrustedRoot: - chainTest.ChainPolicy.ExtraStore.Add(rootCert); - break; - case BuildChainCustomTrustStoreTestArguments.UntrustedIntermediateTrustedRoot: - chainTest.ChainPolicy.CustomTrustStore.Add(rootCert); - break; - case BuildChainCustomTrustStoreTestArguments.TrustedIntermediateTrustedRoot: - chainTest.ChainPolicy.CustomTrustStore.Add(rootCert); - break; - case BuildChainCustomTrustStoreTestArguments.MultipleCalls: - chainTest.ChainPolicy.CustomTrustStore.Add(rootCert); - chainTest.Build(microsoftDotCom); - chainHolderTest.DisposeChainElements(); - chainTest.ChainPolicy.CustomTrustStore.Remove(rootCert); - chainTest.ChainPolicy.TrustMode = X509ChainTrustMode.System; - break; - default: - throw new InvalidDataException(); - } + [Theory] + [PlatformSpecific(TestPlatforms.Android)] + [MemberData(nameof(BuildChainCustomTrustStoreData))] + public static void BuildChainCustomTrustStore_Android( + bool chainBuildsSuccessfully, + X509ChainStatusFlags chainFlags, + BuildChainCustomTrustStoreTestArguments testArguments) + { + using (var microsoftDotCom = new X509Certificate2(TestData.MicrosoftDotComSslCertBytes)) + using (var microsoftDotComIssuer = new X509Certificate2(TestData.MicrosoftDotComIssuerBytes)) + using (var microsoftDotComRoot = new X509Certificate2(TestData.MicrosoftDotComRootBytes)) + using (var chainHolder = new ChainHolder()) + { + // Fetching is not supported on Android, so we need to add intermediate to the extra store + chainHolder.Chain.ChainPolicy.ExtraStore.Add(microsoftDotComIssuer); + BuildChainCustomTrustStoreImpl(chainHolder, microsoftDotComRoot, microsoftDotCom, chainBuildsSuccessfully, chainFlags, testArguments); + } + } - Assert.Equal(chainBuildsSuccessfully, chainTest.Build(microsoftDotCom)); - Assert.Equal(3, chainTest.ChainElements.Count); - Assert.Equal(chainFlags, chainTest.AllStatusFlags()); - } + private static void BuildChainCustomTrustStoreImpl( + ChainHolder chainHolder, + X509Certificate2 rootCert, + X509Certificate2 endCert, + bool chainBuildsSuccessfully, + X509ChainStatusFlags chainFlags, + BuildChainCustomTrustStoreTestArguments testArguments) + { + X509Chain chainTest = chainHolder.Chain; + chainTest.ChainPolicy.RevocationMode = X509RevocationMode.NoCheck; + chainTest.ChainPolicy.VerificationTime = endCert.NotBefore.AddSeconds(1); + chainTest.ChainPolicy.TrustMode = X509ChainTrustMode.CustomRootTrust; + + switch (testArguments) + { + case BuildChainCustomTrustStoreTestArguments.TrustedIntermediateUntrustedRoot: + chainTest.ChainPolicy.ExtraStore.Add(rootCert); + break; + case BuildChainCustomTrustStoreTestArguments.UntrustedIntermediateTrustedRoot: + chainTest.ChainPolicy.CustomTrustStore.Add(rootCert); + break; + case BuildChainCustomTrustStoreTestArguments.TrustedIntermediateTrustedRoot: + chainTest.ChainPolicy.CustomTrustStore.Add(rootCert); + break; + case BuildChainCustomTrustStoreTestArguments.MultipleCalls: + chainTest.ChainPolicy.CustomTrustStore.Add(rootCert); + chainTest.Build(endCert); + chainHolder.DisposeChainElements(); + chainTest.ChainPolicy.CustomTrustStore.Remove(rootCert); + chainTest.ChainPolicy.TrustMode = X509ChainTrustMode.System; + break; + default: + throw new InvalidDataException(); } + + Assert.Equal(chainBuildsSuccessfully, chainTest.Build(endCert)); + Assert.Equal(3, chainTest.ChainElements.Count); + Assert.Equal(chainFlags, chainTest.AllStatusFlags()); } [Fact] @@ -916,6 +957,7 @@ public static void ChainErrorsAtMultipleLayers() } [Fact] + [PlatformSpecific(~TestPlatforms.Android)] // Chain building on Android fails with an empty subject public static void ChainWithEmptySubject() { using (var cert = new X509Certificate2(TestData.EmptySubjectCertificate)) diff --git a/src/libraries/System.Security.Cryptography.X509Certificates/tests/DynamicChainTests.cs b/src/libraries/System.Security.Cryptography.X509Certificates/tests/DynamicChainTests.cs index a82c958c3b2c6..7b27fe535b8d8 100644 --- a/src/libraries/System.Security.Cryptography.X509Certificates/tests/DynamicChainTests.cs +++ b/src/libraries/System.Security.Cryptography.X509Certificates/tests/DynamicChainTests.cs @@ -364,6 +364,7 @@ public static void TestLeafCertificateWithUnknownCriticalExtension() } [Fact] + [PlatformSpecific(~TestPlatforms.Android)] // Android does not support AIA fetching public static void TestInvalidAia() { using (RSA key = RSA.Create()) diff --git a/src/libraries/System.Security.Cryptography.X509Certificates/tests/RevocationTests/AiaTests.cs b/src/libraries/System.Security.Cryptography.X509Certificates/tests/RevocationTests/AiaTests.cs index aa6d7af75e798..c3729b97b2532 100644 --- a/src/libraries/System.Security.Cryptography.X509Certificates/tests/RevocationTests/AiaTests.cs +++ b/src/libraries/System.Security.Cryptography.X509Certificates/tests/RevocationTests/AiaTests.cs @@ -7,6 +7,7 @@ namespace System.Security.Cryptography.X509Certificates.Tests.RevocationTests { + [PlatformSpecific(~TestPlatforms.Android)] // Android does not support AIA fetching public static class AiaTests { [Fact] From ccfa398e7fa2795868cf5ea4a9736cd578871d3e Mon Sep 17 00:00:00 2001 From: Elinor Fung Date: Sat, 20 Mar 2021 20:01:43 -0700 Subject: [PATCH 12/19] Treat X509RevocationMode Offline as Online Try to get better revocation status when possible by retrying validation without revocation --- .../pal_jni.c | 2 + .../pal_jni.h | 1 + .../pal_x509chain.c | 162 ++++++++++++------ .../Cryptography/Pal.Android/ChainPal.cs | 60 ++++--- .../src/Resources/Strings.resx | 3 + 5 files changed, 154 insertions(+), 74 deletions(-) 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 14359ae1e860d..c2fa8e233e12a 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 @@ -193,6 +193,7 @@ jclass g_PKIXBuilderParametersClass; jmethodID g_PKIXBuilderParametersCtor; jmethodID g_PKIXBuilderParametersAddCertStore; jmethodID g_PKIXBuilderParametersAddCertPathChecker; +jmethodID g_PKIXBuilderParametersSetCertPathCheckers; jmethodID g_PKIXBuilderParametersSetDate; jmethodID g_PKIXBuilderParametersSetRevocationEnabled; jmethodID g_PKIXBuilderParametersSetTrustAnchors; @@ -722,6 +723,7 @@ JNI_OnLoad(JavaVM *vm, void *reserved) g_PKIXBuilderParametersCtor = GetMethod(env, false, g_PKIXBuilderParametersClass, "", "(Ljava/security/KeyStore;Ljava/security/cert/CertSelector;)V"); g_PKIXBuilderParametersAddCertStore = GetMethod(env, false, g_PKIXBuilderParametersClass, "addCertStore", "(Ljava/security/cert/CertStore;)V"); g_PKIXBuilderParametersAddCertPathChecker = GetMethod(env, false, g_PKIXBuilderParametersClass, "addCertPathChecker", "(Ljava/security/cert/PKIXCertPathChecker;)V"); + g_PKIXBuilderParametersSetCertPathCheckers = GetMethod(env, false, g_PKIXBuilderParametersClass, "setCertPathCheckers", "(Ljava/util/List;)V"); g_PKIXBuilderParametersSetDate = GetMethod(env, false, g_PKIXBuilderParametersClass, "setDate", "(Ljava/util/Date;)V"); g_PKIXBuilderParametersSetRevocationEnabled = GetMethod(env, false, g_PKIXBuilderParametersClass, "setRevocationEnabled", "(Z)V"); g_PKIXBuilderParametersSetTrustAnchors = GetMethod(env, false, g_PKIXBuilderParametersClass, "setTrustAnchors", "(Ljava/util/Set;)V"); diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native.Android/pal_jni.h b/src/libraries/Native/Unix/System.Security.Cryptography.Native.Android/pal_jni.h index 767f46f08a090..c8135f6d4e552 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native.Android/pal_jni.h +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native.Android/pal_jni.h @@ -145,6 +145,7 @@ extern jclass g_PKIXBuilderParametersClass; extern jmethodID g_PKIXBuilderParametersCtor; extern jmethodID g_PKIXBuilderParametersAddCertStore; extern jmethodID g_PKIXBuilderParametersAddCertPathChecker; +extern jmethodID g_PKIXBuilderParametersSetCertPathCheckers; extern jmethodID g_PKIXBuilderParametersSetDate; extern jmethodID g_PKIXBuilderParametersSetRevocationEnabled; extern jmethodID g_PKIXBuilderParametersSetTrustAnchors; 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 764a99daab6f5..8ef1f607b2062 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 @@ -15,6 +15,7 @@ struct X509ChainContext_t jobject /*TrustAnchor*/ trustAnchor; jobject /*ArrayList*/ errorList; + jobject /*ArrayList*/ revocationErrorList; }; struct ValidationError_t @@ -99,6 +100,7 @@ void AndroidCryptoNative_X509ChainDestroyContext(X509ChainContext* ctx) ReleaseGRef(env, ctx->certPath); ReleaseGRef(env, ctx->trustAnchor); ReleaseGRef(env, ctx->errorList); + ReleaseGRef(env, ctx->revocationErrorList); free(ctx); } @@ -204,6 +206,11 @@ int32_t AndroidCryptoNative_X509ChainGetErrorCount(X509ChainContext* ctx) assert(ctx != NULL); JNIEnv* env = GetJNIEnv(); int32_t count = (*env)->CallIntMethod(env, ctx->errorList, g_CollectionSize); + if (ctx->revocationErrorList != NULL) + { + count += (*env)->CallIntMethod(env, ctx->revocationErrorList, g_CollectionSize); + } + return count; } @@ -279,7 +286,7 @@ static PAL_X509ChainStatusFlags ChainStatusFromValidatorExceptionReason(JNIEnv* return PAL_X509ChainPartialChain; } -static void PopulateValidationError(JNIEnv* env, jobject error, ValidationError* out) +static void PopulateValidationError(JNIEnv* env, jobject error, bool isRevocationError, ValidationError* out) { int index = -1; PAL_X509ChainStatusFlags chainStatus = PAL_X509ChainNoError; @@ -297,12 +304,23 @@ static void PopulateValidationError(JNIEnv* env, jobject error, ValidationError* } jobject message = (*env)->CallObjectMethod(env, error, g_ThrowableGetMessage); - jsize messageLen = (*env)->GetStringLength(env, message); + uint16_t* messagePtr = NULL; + if (message != NULL) + { + jsize messageLen = message == NULL ? 0 : (*env)->GetStringLength(env, message); - // +1 for null terminator - uint16_t* messagePtr = malloc(sizeof(uint16_t) * (size_t)(messageLen + 1)); - messagePtr[messageLen] = '\0'; - (*env)->GetStringRegion(env, message, 0, messageLen, (jchar*)messagePtr); + // +1 for null terminator + messagePtr = malloc(sizeof(uint16_t) * (size_t)(messageLen + 1)); + messagePtr[messageLen] = '\0'; + (*env)->GetStringRegion(env, message, 0, messageLen, (jchar*)messagePtr); + } + + // If the error is known to be from revocation checking, but couldn't be mapped to a revocation status, + // report it as RevocationStatusUnknown + if (isRevocationError && chainStatus != PAL_X509ChainRevocationStatusUnknown && chainStatus != PAL_X509ChainRevoked) + { + chainStatus = PAL_X509ChainRevocationStatusUnknown; + } out->message = messagePtr; out->index = index; @@ -318,19 +336,34 @@ int32_t AndroidCryptoNative_X509ChainGetErrors(X509ChainContext* ctx, Validation int32_t ret = FAIL; - int32_t count = (*env)->CallIntMethod(env, ctx->errorList, g_CollectionSize); - if (errorsLen < count) + int32_t errorCount = (*env)->CallIntMethod(env, ctx->errorList, g_CollectionSize); + int32_t revocationErrorCount = + ctx->revocationErrorList == NULL ? 0 : (*env)->CallIntMethod(env, ctx->revocationErrorList, g_CollectionSize); + + if (errorsLen < errorCount + revocationErrorCount) goto exit; - // for (int i = 0; i < erroList.size(); ++i) { - // Throwable error = erroList.get(i); + // for (int i = 0; i < errorList.size(); ++i) { + // Throwable error = errorList.get(i); // << populate errors[i] >> // } - for (int32_t i = 0; i < count; ++i) + for (int32_t i = 0; i < errorCount; ++i) { jobject error = (*env)->CallObjectMethod(env, ctx->errorList, g_ListGet, i); ON_EXCEPTION_PRINT_AND_GOTO(exit); - PopulateValidationError(env, error, &errors[i]); + PopulateValidationError(env, error, false /*isRevocationError*/, &errors[i]); + (*env)->DeleteLocalRef(env, error); + } + + // for (int i = 0; i < revocationErrorList.size(); ++i) { + // Throwable error = revocationErrorList.get(i); + // << populate errors[i] >> + // } + for (int32_t i = 0; i < revocationErrorCount; ++i) + { + jobject error = (*env)->CallObjectMethod(env, ctx->revocationErrorList, g_ListGet, i); + ON_EXCEPTION_PRINT_AND_GOTO(exit); + PopulateValidationError(env, error, true /*isRevocationError*/, &errors[errorCount + i]); (*env)->DeleteLocalRef(env, error); } @@ -372,41 +405,6 @@ bool AndroidCryptoNative_X509ChainSupportsRevocationOptions(void) return g_CertPathValidatorGetRevocationChecker != NULL && g_PKIXRevocationCheckerClass != NULL; } -static jobject /*HashSet*/ -GetRevocationCheckerOptions(JNIEnv* env, PAL_X509RevocationMode revocationMode, PAL_X509RevocationFlag revocationFlag) -{ - assert(AndroidCryptoNative_X509ChainSupportsRevocationOptions()); - - // HashSet options = new HashSet(3); - jobject options = (*env)->NewObject(env, g_HashSetClass, g_HashSetCtorWithCapacity, 3); - - if (revocationMode == X509RevocationMode_Offline) - { - // options.add(PKIXRevocationChecker.Option.PREFER_CRLS); - jobject preferCrls = (*env)->GetStaticObjectField( - env, g_PKIXRevocationCheckerOptionClass, g_PKIXRevocationCheckerOptionPreferCrls); - (*env)->CallBooleanMethod(env, options, g_HashSetAdd, preferCrls); - (*env)->DeleteLocalRef(env, preferCrls); - - // options.add(PKIXRevocationChecker.Option.NO_FALLBACK); - jobject noFallback = (*env)->GetStaticObjectField( - env, g_PKIXRevocationCheckerOptionClass, g_PKIXRevocationCheckerOptionNoFallback); - (*env)->CallBooleanMethod(env, options, g_HashSetAdd, noFallback); - (*env)->DeleteLocalRef(env, noFallback); - } - - if (revocationFlag == X509RevocationFlag_EndCertificateOnly) - { - // options.add(PKIXRevocationChecker.Option.ONLY_END_ENTITY); - jobject endOnly = (*env)->GetStaticObjectField( - env, g_PKIXRevocationCheckerOptionClass, g_PKIXRevocationCheckerOptionOnlyEndEntity); - (*env)->CallBooleanMethod(env, options, g_HashSetAdd, endOnly); - (*env)->DeleteLocalRef(env, endOnly); - } - - return options; -} - int32_t AndroidCryptoNative_X509ChainValidate(X509ChainContext* ctx, PAL_X509RevocationMode revocationMode, PAL_X509RevocationFlag revocationFlag) @@ -436,29 +434,47 @@ int32_t AndroidCryptoNative_X509ChainValidate(X509ChainContext* ctx, { if (revocationFlag == X509RevocationFlag_EntireChain) { - LOG_INFO("Treating revocation flag 'EntireChain' as 'ExcludeRoot'. Revocation will not be checked for the root certificate."); + LOG_INFO("Treating revocation flag 'EntireChain' as 'ExcludeRoot'. " + "Revocation will not be checked for the root certificate."); } if (AndroidCryptoNative_X509ChainSupportsRevocationOptions()) { + if (revocationMode == X509RevocationMode_Offline) + { + // Android does not supply a way to disable OCSP/CRL fetching + LOG_INFO("Treating revocation mode 'Offline' as 'Online'."); + } + // PKIXRevocationChecker checker = validator.getRevocationChecker(); jobject checker = (*env)->CallObjectMethod(env, loc[validator], g_CertPathValidatorGetRevocationChecker); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); - // checker.setOptions(options); + if (revocationFlag == X509RevocationFlag_EndCertificateOnly) + { + // HashSet options = new HashSet(3); + // options.add(PKIXRevocationChecker.Option.ONLY_END_ENTITY); + // checker.setOptions(options); + jobject options = (*env)->NewObject(env, g_HashSetClass, g_HashSetCtorWithCapacity, 3); + jobject endOnly = (*env)->GetStaticObjectField(env, g_PKIXRevocationCheckerOptionClass, g_PKIXRevocationCheckerOptionOnlyEndEntity); + (*env)->CallBooleanMethod(env, options, g_HashSetAdd, endOnly); + (*env)->CallVoidMethod(env, checker, g_PKIXRevocationCheckerSetOptions, options); + + (*env)->DeleteLocalRef(env, options); + (*env)->DeleteLocalRef(env, endOnly); + } + // params.addCertPathChecker(checker); - jobject options = GetRevocationCheckerOptions(env, revocationMode, revocationFlag); - (*env)->CallVoidMethod(env, checker, g_PKIXRevocationCheckerSetOptions, options); (*env)->CallVoidMethod(env, params, g_PKIXBuilderParametersAddCertPathChecker, checker); - (*env)->DeleteLocalRef(env, options); (*env)->DeleteLocalRef(env, checker); } else { if (revocationFlag == X509RevocationFlag_EndCertificateOnly) { - LOG_INFO("Treating revocation flag 'EndCertificateOnly' as 'ExcludeRoot'. Revocation will be checked for non-end certificates."); + LOG_INFO("Treating revocation flag 'EndCertificateOnly' as 'ExcludeRoot'. " + "Revocation will be checked for non-end certificates."); } } } @@ -466,7 +482,45 @@ int32_t AndroidCryptoNative_X509ChainValidate(X509ChainContext* ctx, loc[result] = (*env)->CallObjectMethod(env, loc[validator], g_CertPathValidatorValidate, certPath, params); if (TryGetJNIException(env, &loc[ex], false /*printException*/)) { - (*env)->CallBooleanMethod(env, ctx->errorList, g_ArrayListAdd, loc[ex]); + // If revocation checking was on, we can re-run validation without revocation checking in an attempt to get + // better error status + if (checkRevocation) + { + // params.setRevocationEnabled(false); + (*env)->CallVoidMethod(env, params, g_PKIXBuilderParametersSetRevocationEnabled, false); + if (AndroidCryptoNative_X509ChainSupportsRevocationOptions()) + { + // If we added a revocation checker, we also need to clear it. We don't add any other checkers, + // so we can just clear the list of checkers instead of getting the existing list, copying it, + // removing the revocation checker, and setting the list to the modified copy. + // params.setCertPathCheckers(null); + (*env)->CallVoidMethod(env, params, g_PKIXBuilderParametersSetCertPathCheckers, NULL); + } + + jobject noRevocationResult = + (*env)->CallObjectMethod(env, loc[validator], g_CertPathValidatorValidate, certPath, params); + if (TryClearJNIExceptions(env)) + { + // Failed even without revocation checking + (*env)->CallBooleanMethod(env, ctx->errorList, g_ArrayListAdd, loc[ex]); + } + else + { + if (ctx->revocationErrorList == NULL) + { + ctx->revocationErrorList = ToGRef(env, (*env)->NewObject(env, g_ArrayListClass, g_ArrayListCtor)); + } + + // Succeeded without revocation checking - errors must be from revocation checking + (*env)->CallBooleanMethod(env, ctx->revocationErrorList, g_ArrayListAdd, loc[ex]); + } + + (*env)->DeleteLocalRef(env, noRevocationResult); + } + else + { + (*env)->CallBooleanMethod(env, ctx->errorList, g_ArrayListAdd, loc[ex]); + } } ret = SUCCESS; diff --git a/src/libraries/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Android/ChainPal.cs b/src/libraries/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Android/ChainPal.cs index 2c7c16bafb6b4..d4bd74cb56160 100644 --- a/src/libraries/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Android/ChainPal.cs +++ b/src/libraries/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Android/ChainPal.cs @@ -202,6 +202,7 @@ internal void Evaluate( List[] statuses = new List[certs.Length]; int firstErrorIndex = -1; + int firstRevocationErrorIndex = -1; Dictionary> errorsByIndex = GetStatusByIndex(_chainContext); foreach (int index in errorsByIndex.Keys) { @@ -212,6 +213,10 @@ internal void Evaluate( { statuses[index] = errorsByIndex[index]; firstErrorIndex = Math.Max(index, firstErrorIndex); + if (errorsByIndex[index].Exists(s => s.Status == X509ChainStatusFlags.Revoked || s.Status == X509ChainStatusFlags.RevocationStatusUnknown)) + { + firstRevocationErrorIndex = Math.Max(index, firstRevocationErrorIndex); + } } } @@ -224,36 +229,29 @@ internal void Evaluate( Status = X509ChainStatusFlags.PartialChain, StatusInformation = SR.Chain_PartialChain, }; - for (int i = firstErrorIndex - 1; i >= 0; i--) - { - if (statuses[i] == null) - { - statuses[i] = new List(); - } + AddStatusFromIndexToEndCertificate(firstErrorIndex - 1, partialChainStatus, statuses, overallStatus); + } - statuses[i].Add(partialChainStatus); - } + if (firstRevocationErrorIndex > 0) + { + // Assign RevocationStatusUnknown to everything from the first revocation error to the end certificate + X509ChainStatus revocationUnknownStatus = new X509ChainStatus + { + Status = X509ChainStatusFlags.RevocationStatusUnknown, + StatusInformation = SR.Chain_RevocationStatusUnknown, + }; + AddStatusFromIndexToEndCertificate(firstRevocationErrorIndex - 1, revocationUnknownStatus, statuses, overallStatus); } if (!IsPolicyMatch(certs, applicationPolicy, certificatePolicy)) { + // Assign NotValidForUsage to everything X509ChainStatus policyFailStatus = new X509ChainStatus { Status = X509ChainStatusFlags.NotValidForUsage, StatusInformation = SR.Chain_NoPolicyMatch, }; - - for (int i = 0; i < statuses.Length; i++) - { - if (statuses[i] == null) - { - statuses[i] = new List(); - } - - statuses[i].Add(policyFailStatus); - } - - overallStatus.Add(policyFailStatus); + AddStatusFromIndexToEndCertificate(statuses.Length - 1, policyFailStatus, statuses, overallStatus); } X509ChainElement[] elements = new X509ChainElement[certs.Length]; @@ -267,6 +265,28 @@ internal void Evaluate( ChainStatus = overallStatus.ToArray(); } + private static void AddStatusFromIndexToEndCertificate( + int index, + X509ChainStatus statusToSet, + List[] statuses, + List overallStatus) + { + if (!overallStatus.Exists(s => s.Status == statusToSet.Status)) + { + overallStatus.Add(statusToSet); + } + + for (int i = index; i >= 0; i--) + { + if (statuses[i] == null) + { + statuses[i] = new List(); + } + + statuses[i].Add(statusToSet); + } + } + private static Dictionary> GetStatusByIndex(SafeX509ChainContextHandle ctx) { var statusByIndex = new Dictionary>(); diff --git a/src/libraries/System.Security.Cryptography.X509Certificates/src/Resources/Strings.resx b/src/libraries/System.Security.Cryptography.X509Certificates/src/Resources/Strings.resx index a71d0795dfd5e..97eefea5eff5b 100644 --- a/src/libraries/System.Security.Cryptography.X509Certificates/src/Resources/Strings.resx +++ b/src/libraries/System.Security.Cryptography.X509Certificates/src/Resources/Strings.resx @@ -112,6 +112,9 @@ The certificate chain is incomplete. + + The certificate's revocation status could not be determined. + The certificate chain setting '{0}' is not supported on this platform. From a84f4ae1d94ce3ec1c5dc77d614fba19cecca9e5 Mon Sep 17 00:00:00 2001 From: Elinor Fung Date: Sun, 21 Mar 2021 10:45:06 -0700 Subject: [PATCH 13/19] Make test revocation responder handle POST requests for OCSP --- .../X509Certificates/CertificateAuthority.cs | 3 +- .../X509Certificates/RevocationResponder.cs | 52 +++++++++++++++---- 2 files changed, 45 insertions(+), 10 deletions(-) diff --git a/src/libraries/Common/tests/System/Security/Cryptography/X509Certificates/CertificateAuthority.cs b/src/libraries/Common/tests/System/Security/Cryptography/X509Certificates/CertificateAuthority.cs index 0ff2bde7f066d..3685c0d6f03a7 100644 --- a/src/libraries/Common/tests/System/Security/Cryptography/X509Certificates/CertificateAuthority.cs +++ b/src/libraries/Common/tests/System/Security/Cryptography/X509Certificates/CertificateAuthority.cs @@ -517,8 +517,9 @@ singleExtensions [1] EXPLICIT Extensions OPTIONAL } } else if (status == CertStatus.Revoked) { + // Android does not support all precisions for seconds - just omit fractional seconds for testing on Android writer.PushSequence(s_context1); - writer.WriteGeneralizedTime(revokedTime); + writer.WriteGeneralizedTime(revokedTime, omitFractionalSeconds: OperatingSystem.IsAndroid()); writer.PopSequence(s_context1); } else diff --git a/src/libraries/Common/tests/System/Security/Cryptography/X509Certificates/RevocationResponder.cs b/src/libraries/Common/tests/System/Security/Cryptography/X509Certificates/RevocationResponder.cs index 67b5ad4a2a455..4e9e5e20612a9 100644 --- a/src/libraries/Common/tests/System/Security/Cryptography/X509Certificates/RevocationResponder.cs +++ b/src/libraries/Common/tests/System/Security/Cryptography/X509Certificates/RevocationResponder.cs @@ -48,17 +48,23 @@ internal void AddCertificateAuthority(CertificateAuthority authority) { if (authority.AiaHttpUri != null && authority.AiaHttpUri.StartsWith(UriPrefix)) { - _aiaPaths.Add(authority.AiaHttpUri.Substring(UriPrefix.Length - 1), authority); + string path = authority.AiaHttpUri.Substring(UriPrefix.Length - 1); + Trace($"Adding AIA path : {path}"); + _aiaPaths.Add(path, authority); } if (authority.CdpUri != null && authority.CdpUri.StartsWith(UriPrefix)) { - _crlPaths.Add(authority.CdpUri.Substring(UriPrefix.Length - 1), authority); + string path = authority.CdpUri.Substring(UriPrefix.Length - 1); + Trace($"Adding CRL path : {path}"); + _crlPaths.Add(path, authority); } if (authority.OcspUri != null && authority.OcspUri.StartsWith(UriPrefix)) { - _ocspAuthorities.Add((authority.OcspUri.Substring(UriPrefix.Length - 1), authority)); + string path = authority.OcspUri.Substring(UriPrefix.Length - 1); + Trace($"Adding OCSP path : {path}"); + _ocspAuthorities.Add((path, authority)); } } @@ -211,20 +217,18 @@ private void HandleRequest(HttpListenerContext context, ref bool responded) if (url.StartsWith(prefix)) { - if (context.Request.HttpMethod == "GET") + byte[] reqBytes; + if (TryGetOcspRequestBytes(context.Request, prefix, out reqBytes)) { ReadOnlyMemory certId; ReadOnlyMemory nonce; - try { - string base64 = HttpUtility.UrlDecode(url.Substring(prefix.Length + 1)); - byte[] reqBytes = Convert.FromBase64String(base64); DecodeOcspRequest(reqBytes, out certId, out nonce); } - catch (Exception) + catch (Exception e) { - Trace($"OcspRequest Decode failed ({url})"); + Trace($"OcspRequest Decode failed ({url}) - {e}"); context.Response.StatusCode = 400; context.Response.Close(); return; @@ -291,6 +295,36 @@ private static HttpListener OpenListener(out string uriPrefix) } } + private static bool TryGetOcspRequestBytes(HttpListenerRequest request, string prefix, out byte[] requestBytes) + { + requestBytes = null; + try + { + if (request.HttpMethod == "GET") + { + string base64 = HttpUtility.UrlDecode(request.RawUrl.Substring(prefix.Length + 1)); + requestBytes = Convert.FromBase64String(base64); + return true; + } + else if (request.HttpMethod == "POST" && request.ContentType == "application/ocsp-request") + { + using (System.IO.Stream stream = request.InputStream) + { + requestBytes = new byte[request.ContentLength64]; + int read = stream.Read(requestBytes, 0, requestBytes.Length); + System.Diagnostics.Debug.Assert(read == requestBytes.Length); + return true; + } + } + } + catch (Exception e) + { + Trace($"Failed to get OCSP request bytes ({request.RawUrl}) - {e}"); + } + + return false; + } + private static void DecodeOcspRequest( byte[] requestBytes, out ReadOnlyMemory certId, From 88db1e0ca3c760dc07fa82614b84140d4ae0a74a Mon Sep 17 00:00:00 2001 From: Elinor Fung Date: Sun, 21 Mar 2021 23:58:35 -0700 Subject: [PATCH 14/19] AddRef cert handles --- .../Cryptography/Pal.Android/ChainPal.cs | 104 +++++++++++++----- 1 file changed, 74 insertions(+), 30 deletions(-) diff --git a/src/libraries/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Android/ChainPal.cs b/src/libraries/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Android/ChainPal.cs index d4bd74cb56160..9433621a55c66 100644 --- a/src/libraries/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Android/ChainPal.cs +++ b/src/libraries/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Android/ChainPal.cs @@ -95,12 +95,12 @@ internal void Initialize( X509Certificate2Collection customTrustStore, X509ChainTrustMode trustMode) { - List extraCerts = new List() { cert.Handle }; + List extraCertHandles = new List() { ((AndroidCertificatePal)cert).SafeHandle }; if (extraStore != null) { foreach (X509Certificate2 extraCert in extraStore) { - extraCerts.Add(extraCert.Pal.Handle); + extraCertHandles.Add(((AndroidCertificatePal)extraCert.Pal).SafeHandle); } } @@ -108,45 +108,78 @@ internal void Initialize( trustMode == X509ChainTrustMode.System || trustMode == X509ChainTrustMode.CustomRootTrust, "Unsupported trust mode. Only System and CustomRootTrust are currently handled"); - List customTrustCerts = new List(); + List customTrustCertHandles = new List(); bool useCustomRootTrust = trustMode == X509ChainTrustMode.CustomRootTrust; if (useCustomRootTrust && customTrustStore != null) { foreach (X509Certificate2 custom in customTrustStore) { - IntPtr certHandle = custom.Pal.Handle; + SafeHandle certHandle = ((AndroidCertificatePal)custom.Pal).SafeHandle; if (custom.SubjectName.RawData.ContentsEqual(custom.IssuerName.RawData)) { // Add self-issued certs to custom root trust cert - customTrustCerts.Add(certHandle); + customTrustCertHandles.Add(certHandle); } else { // Add non-self-issued certs to extra certs - extraCerts.Add(certHandle); + extraCertHandles.Add(certHandle); } } } - IntPtr[] extraArray = extraCerts.ToArray(); - _chainContext = Interop.AndroidCrypto.X509ChainCreateContext( - ((AndroidCertificatePal)cert).SafeHandle, - extraArray, - extraArray.Length); + int extraIdx = 0; + int customIdx = 0; + try + { + IntPtr[] extraCerts = new IntPtr[extraCertHandles.Count]; + for (extraIdx = 0; extraIdx < extraCertHandles.Count; extraIdx++) + { + SafeHandle handle = extraCertHandles[extraIdx]; + bool addedRef = false; + handle.DangerousAddRef(ref addedRef); + extraCerts[extraIdx] = handle.DangerousGetHandle(); + } + + _chainContext = Interop.AndroidCrypto.X509ChainCreateContext( + ((AndroidCertificatePal)cert).SafeHandle, + extraCerts, + extraCerts.Length); - if (useCustomRootTrust) + if (useCustomRootTrust) + { + // Android does not support an empty set of trust anchors + if (customTrustCertHandles.Count == 0) + { + throw new PlatformNotSupportedException(SR.Chain_EmptyCustomTrustNotSupported); + } + + IntPtr[] customTrustCerts = new IntPtr[customTrustCertHandles.Count]; + for (customIdx = 0; customIdx < customTrustCertHandles.Count; customIdx++) + { + SafeHandle handle = customTrustCertHandles[customIdx]; + bool addedRef = false; + handle.DangerousAddRef(ref addedRef); + customTrustCerts[customIdx] = handle.DangerousGetHandle(); + } + + int res = Interop.AndroidCrypto.X509ChainSetCustomTrustStore(_chainContext, customTrustCerts, customTrustCerts.Length); + if (res != 1) + { + throw new CryptographicException(); + } + } + } + finally { - // Android does not support an empty set of trust anchors - if (customTrustCerts.Count == 0) + for (extraIdx -= 1; extraIdx >= 0; extraIdx--) { - throw new PlatformNotSupportedException(SR.Chain_EmptyCustomTrustNotSupported); + extraCertHandles[extraIdx].DangerousRelease(); } - IntPtr[] customTrustArray = customTrustCerts.ToArray(); - int res = Interop.AndroidCrypto.X509ChainSetCustomTrustStore(_chainContext, customTrustArray, customTrustArray.Length); - if (res != 1) + for (customIdx -= 1; customIdx >= 0; customIdx--) { - throw new CryptographicException(); + customTrustCertHandles[customIdx].DangerousRelease(); } } } @@ -206,7 +239,12 @@ internal void Evaluate( Dictionary> errorsByIndex = GetStatusByIndex(_chainContext); foreach (int index in errorsByIndex.Keys) { - overallStatus.AddRange(errorsByIndex[index]); + List errors = errorsByIndex[index]; + for (int i = 0; i < errors.Count; i++) + { + X509ChainStatus status = errors[i]; + AddUniqueStatus(overallStatus, ref status); + } // -1 indicates that error is not tied to a specific index if (index != -1) @@ -229,7 +267,7 @@ internal void Evaluate( Status = X509ChainStatusFlags.PartialChain, StatusInformation = SR.Chain_PartialChain, }; - AddStatusFromIndexToEndCertificate(firstErrorIndex - 1, partialChainStatus, statuses, overallStatus); + AddStatusFromIndexToEndCertificate(firstErrorIndex - 1, ref partialChainStatus, statuses, overallStatus); } if (firstRevocationErrorIndex > 0) @@ -240,7 +278,7 @@ internal void Evaluate( Status = X509ChainStatusFlags.RevocationStatusUnknown, StatusInformation = SR.Chain_RevocationStatusUnknown, }; - AddStatusFromIndexToEndCertificate(firstRevocationErrorIndex - 1, revocationUnknownStatus, statuses, overallStatus); + AddStatusFromIndexToEndCertificate(firstRevocationErrorIndex - 1, ref revocationUnknownStatus, statuses, overallStatus); } if (!IsPolicyMatch(certs, applicationPolicy, certificatePolicy)) @@ -251,7 +289,7 @@ internal void Evaluate( Status = X509ChainStatusFlags.NotValidForUsage, StatusInformation = SR.Chain_NoPolicyMatch, }; - AddStatusFromIndexToEndCertificate(statuses.Length - 1, policyFailStatus, statuses, overallStatus); + AddStatusFromIndexToEndCertificate(statuses.Length - 1, ref policyFailStatus, statuses, overallStatus); } X509ChainElement[] elements = new X509ChainElement[certs.Length]; @@ -267,15 +305,11 @@ internal void Evaluate( private static void AddStatusFromIndexToEndCertificate( int index, - X509ChainStatus statusToSet, + ref X509ChainStatus statusToSet, List[] statuses, List overallStatus) { - if (!overallStatus.Exists(s => s.Status == statusToSet.Status)) - { - overallStatus.Add(statusToSet); - } - + AddUniqueStatus(overallStatus, ref statusToSet); for (int i = index; i >= 0; i--) { if (statuses[i] == null) @@ -283,7 +317,17 @@ private static void AddStatusFromIndexToEndCertificate( statuses[i] = new List(); } - statuses[i].Add(statusToSet); + AddUniqueStatus(statuses[i], ref statusToSet); + } + } + + private static void AddUniqueStatus(List list, ref X509ChainStatus status) + { + X509ChainStatusFlags statusFlags = status.Status; + string statusInfo = status.StatusInformation; + if (!list.Exists(s => s.Status == statusFlags && s.StatusInformation == statusInfo)) + { + list.Add(status); } } From 95321087ba00a29ec947f10977e8534db91a87d1 Mon Sep 17 00:00:00 2001 From: Elinor Fung Date: Sun, 21 Mar 2021 19:24:42 -0700 Subject: [PATCH 15/19] Handle EndCertificateOnly revocation check with root-only chain Validation in two passes - with / without revocation for better status --- .../pal_x509chain.c | 237 +++++++++++------- 1 file changed, 152 insertions(+), 85 deletions(-) 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 8ef1f607b2062..0c14bdaf80053 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 @@ -255,7 +255,7 @@ static PAL_X509ChainStatusFlags ChainStatusFromValidatorExceptionReason(JNIEnv* case BASICREASON_UNDETERMINED_REVOCATION_STATUS: return PAL_X509ChainRevocationStatusUnknown; case BASICREASON_INVALID_SIGNATURE: - return PAL_X509ChainCtlNotSignatureValid; + return PAL_X509ChainNotSignatureValid; case BASICREASON_ALGORITHM_CONSTRAINED: return PAL_X509ChainPartialChain; } @@ -405,121 +405,147 @@ bool AndroidCryptoNative_X509ChainSupportsRevocationOptions(void) return g_CertPathValidatorGetRevocationChecker != NULL && g_PKIXRevocationCheckerClass != NULL; } -int32_t AndroidCryptoNative_X509ChainValidate(X509ChainContext* ctx, - PAL_X509RevocationMode revocationMode, - PAL_X509RevocationFlag revocationFlag) +static jobject /*CertPath*/ CreateCertPathFromAnchor(JNIEnv* env, jobject /*TrustAnchor*/ trustAnchor) { - assert(ctx != NULL); - JNIEnv* env = GetJNIEnv(); + jobject ret = NULL; + INIT_LOCALS(loc, certList, trustedCert, certFactoryType, certFactory); - int32_t ret = FAIL; - INIT_LOCALS(loc, validatorType, validator, checker, result, ex); + // ArrayList certList = new ArrayList(1); + loc[certList] = (*env)->NewObject(env, g_ArrayListClass, g_ArrayListCtorWithCapacity, 1); - jobject params = ctx->params; - jobject certPath = ctx->certPath; + // Certificate trustedCert = trustAnchor.getTrustedCert(); + // certList.add(trustedCert); + loc[trustedCert] = (*env)->CallObjectMethod(env, trustAnchor, g_TrustAnchorGetTrustedCert); + (*env)->CallBooleanMethod(env, loc[certList], g_ArrayListAdd, loc[trustedCert]); + + // CertificateFactory certFactory = CertificateFactory.getInstance("X.509"); + loc[certFactoryType] = JSTRING("X.509"); + loc[certFactory] = + (*env)->CallStaticObjectMethod(env, g_CertFactoryClass, g_CertFactoryGetInstance, loc[certFactoryType]); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); - // String validatorType = "PKIX"; - // CertPathValidator validator = CertPathValidator.getInstance(validatorType); - // PKIXCertPathValidatorResult result = (PKIXCertPathValidatorResult)validator.validate(certPath, params); - loc[validatorType] = JSTRING("PKIX"); - loc[validator] = (*env)->CallStaticObjectMethod( - env, g_CertPathValidatorClass, g_CertPathValidatorGetInstance, loc[validatorType]); + // CertPath certPath = certFactory.generateCertPath(certList); + jobject certPath = + (*env)->CallObjectMethod(env, loc[certFactory], g_CertFactoryGenerateCertPathFromList, loc[certList]); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); - bool checkRevocation = revocationMode != X509RevocationMode_NoCheck; + ret = certPath; + +cleanup: + RELEASE_LOCALS(loc, env); + return ret; +} - // params.setRevocationEnabled(checkRevocation); - (*env)->CallVoidMethod(env, params, g_PKIXBuilderParametersSetRevocationEnabled, checkRevocation); - if (checkRevocation) +static int32_t ValidateWithRevocation(JNIEnv* env, + X509ChainContext* ctx, + jobject /*CertPathValidator*/ validator, + PAL_X509RevocationMode revocationMode, + PAL_X509RevocationFlag revocationFlag, + bool hadErrorsWithoutRevocation) +{ + assert(ctx != NULL); + assert(validator != NULL); + + int32_t ret = FAIL; + INIT_LOCALS(loc, certPathFromAnchor, options, checker, result, ex); + + if (revocationMode == X509RevocationMode_Offline) { - if (revocationFlag == X509RevocationFlag_EntireChain) + // Android does not supply a way to disable OCSP/CRL fetching + LOG_INFO("Treating revocation mode 'Offline' as 'Online'."); + } + + jobject certPathToUse = NULL; + if (revocationFlag == X509RevocationFlag_EntireChain) + { + LOG_INFO("Treating revocation flag 'EntireChain' as 'ExcludeRoot'. " + "Revocation will be not be checked for root certificate."); + + certPathToUse = ctx->certPath; + } + else if (revocationFlag == X509RevocationFlag_EndCertificateOnly) + { + // List certPathList = certPath.getCertificates(); + // int certCount = certPathList.size(); + jobject certPathList = (*env)->CallObjectMethod(env, ctx->certPath, g_CertPathGetCertificates); + int certCount = (int)(*env)->CallIntMethod(env, certPathList, g_CollectionSize); + if (certCount == 0) { - LOG_INFO("Treating revocation flag 'EntireChain' as 'ExcludeRoot'. " - "Revocation will not be checked for the root certificate."); + // If the chain only consists only of the trust anchor, create a path with just + // the trust anchor for revocation checking for end certificate only. This should + // still pass normal (non-revocation) validation when it is the only certificate. + loc[certPathFromAnchor] = CreateCertPathFromAnchor(env, ctx->trustAnchor); + certPathToUse = loc[certPathFromAnchor]; } - - if (AndroidCryptoNative_X509ChainSupportsRevocationOptions()) + else { - if (revocationMode == X509RevocationMode_Offline) + certPathToUse = ctx->certPath; + if (AndroidCryptoNative_X509ChainSupportsRevocationOptions()) { - // Android does not supply a way to disable OCSP/CRL fetching - LOG_INFO("Treating revocation mode 'Offline' as 'Online'."); - } - - // PKIXRevocationChecker checker = validator.getRevocationChecker(); - jobject checker = (*env)->CallObjectMethod(env, loc[validator], g_CertPathValidatorGetRevocationChecker); - ON_EXCEPTION_PRINT_AND_GOTO(cleanup); + // 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. - if (revocationFlag == X509RevocationFlag_EndCertificateOnly) - { // HashSet options = new HashSet(3); // options.add(PKIXRevocationChecker.Option.ONLY_END_ENTITY); - // checker.setOptions(options); - jobject options = (*env)->NewObject(env, g_HashSetClass, g_HashSetCtorWithCapacity, 3); - jobject endOnly = (*env)->GetStaticObjectField(env, g_PKIXRevocationCheckerOptionClass, g_PKIXRevocationCheckerOptionOnlyEndEntity); - (*env)->CallBooleanMethod(env, options, g_HashSetAdd, endOnly); - (*env)->CallVoidMethod(env, checker, g_PKIXRevocationCheckerSetOptions, options); - - (*env)->DeleteLocalRef(env, options); + loc[options] = (*env)->NewObject(env, g_HashSetClass, g_HashSetCtorWithCapacity, 3); + jobject endOnly = (*env)->GetStaticObjectField( + env, g_PKIXRevocationCheckerOptionClass, g_PKIXRevocationCheckerOptionOnlyEndEntity); + (*env)->CallBooleanMethod(env, loc[options], g_HashSetAdd, endOnly); (*env)->DeleteLocalRef(env, endOnly); } - - // params.addCertPathChecker(checker); - (*env)->CallVoidMethod(env, params, g_PKIXBuilderParametersAddCertPathChecker, checker); - - (*env)->DeleteLocalRef(env, checker); - } - else - { - if (revocationFlag == X509RevocationFlag_EndCertificateOnly) + else { LOG_INFO("Treating revocation flag 'EndCertificateOnly' as 'ExcludeRoot'. " "Revocation will be checked for non-end certificates."); } } + + (*env)->DeleteLocalRef(env, certPathList); + } + else + { + assert(revocationFlag == X509RevocationFlag_ExcludeRoot); + certPathToUse = ctx->certPath; } - loc[result] = (*env)->CallObjectMethod(env, loc[validator], g_CertPathValidatorValidate, certPath, params); - if (TryGetJNIException(env, &loc[ex], false /*printException*/)) + jobject params = ctx->params; + if (AndroidCryptoNative_X509ChainSupportsRevocationOptions()) { - // If revocation checking was on, we can re-run validation without revocation checking in an attempt to get - // better error status - if (checkRevocation) - { - // params.setRevocationEnabled(false); - (*env)->CallVoidMethod(env, params, g_PKIXBuilderParametersSetRevocationEnabled, false); - if (AndroidCryptoNative_X509ChainSupportsRevocationOptions()) - { - // If we added a revocation checker, we also need to clear it. We don't add any other checkers, - // so we can just clear the list of checkers instead of getting the existing list, copying it, - // removing the revocation checker, and setting the list to the modified copy. - // params.setCertPathCheckers(null); - (*env)->CallVoidMethod(env, params, g_PKIXBuilderParametersSetCertPathCheckers, NULL); - } + // PKIXRevocationChecker checker = validator.getRevocationChecker(); + loc[checker] = (*env)->CallObjectMethod(env, validator, g_CertPathValidatorGetRevocationChecker); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); - jobject noRevocationResult = - (*env)->CallObjectMethod(env, loc[validator], g_CertPathValidatorValidate, certPath, params); - if (TryClearJNIExceptions(env)) - { - // Failed even without revocation checking - (*env)->CallBooleanMethod(env, ctx->errorList, g_ArrayListAdd, loc[ex]); - } - else - { - if (ctx->revocationErrorList == NULL) - { - ctx->revocationErrorList = ToGRef(env, (*env)->NewObject(env, g_ArrayListClass, g_ArrayListCtor)); - } + // Set any specific options + if (loc[options] != NULL) + { + // checker.setOptions(options); + (*env)->CallVoidMethod(env, loc[checker], g_PKIXRevocationCheckerSetOptions, loc[options]); + } - // Succeeded without revocation checking - errors must be from revocation checking - (*env)->CallBooleanMethod(env, ctx->revocationErrorList, g_ArrayListAdd, loc[ex]); - } + // params.addCertPathChecker(checker); + (*env)->CallVoidMethod(env, params, g_PKIXBuilderParametersAddCertPathChecker, loc[checker]); + } - (*env)->DeleteLocalRef(env, noRevocationResult); + // params.setRevocationEnabled(true); + // PKIXCertPathValidatorResult result = (PKIXCertPathValidatorResult)validator.validate(certPathToUse, params); + (*env)->CallVoidMethod(env, params, g_PKIXBuilderParametersSetRevocationEnabled, true); + loc[result] = (*env)->CallObjectMethod(env, validator, g_CertPathValidatorValidate, certPathToUse, params); + if (TryGetJNIException(env, &loc[ex], false /*printException*/)) + { + if (hadErrorsWithoutRevocation) + { + // Failed without revocation checking - we don't know for sure if these errors were from revocation checking + (*env)->CallBooleanMethod(env, ctx->errorList, g_ArrayListAdd, loc[ex]); } else { - (*env)->CallBooleanMethod(env, ctx->errorList, g_ArrayListAdd, loc[ex]); + // Succeeded without revocation checking - errors must be from revocation checking + if (ctx->revocationErrorList == NULL) + { + ctx->revocationErrorList = ToGRef(env, (*env)->NewObject(env, g_ArrayListClass, g_ArrayListCtor)); + } + + (*env)->CallBooleanMethod(env, ctx->revocationErrorList, g_ArrayListAdd, loc[ex]); } } @@ -529,3 +555,44 @@ int32_t AndroidCryptoNative_X509ChainValidate(X509ChainContext* ctx, RELEASE_LOCALS(loc, env); return ret; } + +int32_t AndroidCryptoNative_X509ChainValidate(X509ChainContext* ctx, + PAL_X509RevocationMode revocationMode, + PAL_X509RevocationFlag revocationFlag) +{ + assert(ctx != NULL); + JNIEnv* env = GetJNIEnv(); + + int32_t ret = FAIL; + INIT_LOCALS(loc, validatorType, validator, result, ex); + + // String validatorType = "PKIX"; + // CertPathValidator validator = CertPathValidator.getInstance(validatorType); + loc[validatorType] = JSTRING("PKIX"); + loc[validator] = (*env)->CallStaticObjectMethod( + env, g_CertPathValidatorClass, g_CertPathValidatorGetInstance, loc[validatorType]); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); + + // PKIXCertPathValidatorResult result = (PKIXCertPathValidatorResult)validator.validate(certPath, params); + loc[result] = + (*env)->CallObjectMethod(env, loc[validator], g_CertPathValidatorValidate, ctx->certPath, ctx->params); + bool hadErrorsWithoutRevocation = TryGetJNIException(env, &loc[ex], false /*printException*/); + if (hadErrorsWithoutRevocation) + { + (*env)->CallBooleanMethod(env, ctx->errorList, g_ArrayListAdd, loc[ex]); + } + + if (revocationMode != X509RevocationMode_NoCheck) + { + ret = ValidateWithRevocation( + env, ctx, loc[validator], revocationMode, revocationFlag, hadErrorsWithoutRevocation); + } + else + { + ret = SUCCESS; + } + +cleanup: + RELEASE_LOCALS(loc, env); + return ret; +} From 7d9f79b35fef42a94ad48a8bb33802810f31412b Mon Sep 17 00:00:00 2001 From: Elinor Fung Date: Mon, 22 Mar 2021 11:03:56 -0700 Subject: [PATCH 16/19] Update DynamicRevocationTests for Android behaviour --- .../RevocationTests/DynamicRevocationTests.cs | 254 ++++++++++++++---- 1 file changed, 196 insertions(+), 58 deletions(-) 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 d832cb91e33c3..2671660cc7930 100644 --- a/src/libraries/System.Security.Cryptography.X509Certificates/tests/RevocationTests/DynamicRevocationTests.cs +++ b/src/libraries/System.Security.Cryptography.X509Certificates/tests/RevocationTests/DynamicRevocationTests.cs @@ -17,11 +17,33 @@ public static class DynamicRevocationTests private static readonly Oid s_tlsServerOid = new Oid("1.3.6.1.5.5.7.3.1", null); + private static bool SupportsEntireChainCheck => !PlatformDetection.IsAndroid; + private static readonly X509ChainStatusFlags ThisOsRevocationStatusUnknown = - PlatformDetection.IsOSX ? + PlatformDetection.IsOSX || PlatformDetection.IsAndroid ? X509ChainStatusFlags.RevocationStatusUnknown : X509ChainStatusFlags.RevocationStatusUnknown | X509ChainStatusFlags.OfflineRevocation; + // Android will stop checking after the first revocation error, so any revoked certificates + // after will have PartialChain and RevocationStatusUnknown instead of Revoked + private static readonly X509ChainStatusFlags ThisOsRevokedWithPreviousRevocationError = + PlatformDetection.IsAndroid ? + X509ChainStatusFlags.PartialChain | X509ChainStatusFlags.RevocationStatusUnknown : + X509ChainStatusFlags.Revoked; + + // Android will stop checking after the first revocation error, so any revoked certificates + // after will have PartialChain and RevocationStatusUnknown instead of RevocationStatusUnknown + private static readonly X509ChainStatusFlags ThisOsRevocationStatusUnknownWithPreviousRevocationError = + PlatformDetection.IsAndroid ? + X509ChainStatusFlags.PartialChain | X509ChainStatusFlags.RevocationStatusUnknown : + ThisOsRevocationStatusUnknown; + + // Android will stop checking after the first revocation error, so any non-revoked certificates + // after will have PartialChain and RevocationStatusUnknown instead of NoError + private static readonly X509ChainStatusFlags ThisOsNoErrorWithPreviousRevocationError = + PlatformDetection.IsAndroid ? + X509ChainStatusFlags.PartialChain | X509ChainStatusFlags.RevocationStatusUnknown : + X509ChainStatusFlags.NoError; private delegate void RunSimpleTest( CertificateAuthority root, @@ -79,10 +101,22 @@ public static IEnumerable AllViableRevocation [MemberData(nameof(AllViableRevocation))] public static void NothingRevoked(PkiOptions pkiOptions) { + bool usingCrl = pkiOptions.HasFlag(PkiOptions.IssuerRevocationViaCrl) || pkiOptions.HasFlag(PkiOptions.EndEntityRevocationViaCrl);// && !pkiOptions.HasFlag(PkiOptions.EndEntityRevocationViaOcsp); SimpleTest( pkiOptions, (root, intermediate, endEntity, holder, responder) => { + if (PlatformDetection.IsAndroid && usingCrl) + { + // Android uses the verification time when determining if a CRL is relevant. If there + // are no relevant CRLs based on that time, the revocation status will be unknown. + // SimpleTest sets the verification time to the end entity's NotBefore + 1 minute, + // while the revocation responder uses the current time to set thisUpdate/nextUpdate. + // If using CRLs, set the verification time to the current time so that fetched CRLs + // will be considered relevant. + holder.Chain.ChainPolicy.VerificationTime = DateTime.UtcNow; + } + SimpleRevocationBody( holder, endEntity, @@ -489,7 +523,7 @@ public static void RevokeEndEntity_RootUnrelatedOcsp(PkiOptions pkiOptions) chain, rootStatus: X509ChainStatusFlags.NoError, issrStatus: ThisOsRevocationStatusUnknown, - leafStatus: X509ChainStatusFlags.NoError); + leafStatus: ThisOsNoErrorWithPreviousRevocationError); Assert.False(chainBuilt, "Chain built with ExcludeRoot."); holder.DisposeChainElements(); @@ -508,10 +542,24 @@ public static void RevokeEndEntity_RootUnrelatedOcsp(PkiOptions pkiOptions) }); } + public static IEnumerable PolicyErrorsNotTimeValidData + { + get + { + // Values are { policyErrors, notTimeValid } + yield return new object[] { true, false}; + + // Android always validates timestamp as part of building a path, + // so we don't include test cases with invalid time here + if (!PlatformDetection.IsAndroid) + { + yield return new object[] { false, true}; + yield return new object[] { true, true}; + } + } + } [Theory] - [InlineData(false, true)] - [InlineData(true, false)] - [InlineData(true, true)] + [MemberData(nameof(PolicyErrorsNotTimeValidData))] [ActiveIssue("https://github.com/dotnet/runtime/issues/31249", TestPlatforms.OSX)] public static void RevokeIntermediate_PolicyErrors_NotTimeValid(bool policyErrors, bool notTimeValid) { @@ -560,7 +608,7 @@ public static void RevokeIntermediate_PolicyErrors_NotTimeValid(bool policyError chain, rootStatus: issuerExtraProblems, issrStatus: issuerExtraProblems | X509ChainStatusFlags.Revoked, - leafStatus: leafProblems | ThisOsRevocationStatusUnknown); + leafStatus: leafProblems | ThisOsRevocationStatusUnknownWithPreviousRevocationError); Assert.False(chainBuilt, "Chain built with ExcludeRoot."); holder.DisposeChainElements(); @@ -596,9 +644,7 @@ public static void RevokeIntermediate_PolicyErrors_NotTimeValid(bool policyError } [Theory] - [InlineData(false, true)] - [InlineData(true, false)] - [InlineData(true, true)] + [MemberData(nameof(PolicyErrorsNotTimeValidData))] [ActiveIssue("https://github.com/dotnet/runtime/issues/31249", TestPlatforms.OSX)] public static void RevokeEndEntity_PolicyErrors_NotTimeValid(bool policyErrors, bool notTimeValid) { @@ -719,7 +765,7 @@ public static void RevokeEndEntity_RootRevocationOffline(PkiOptions pkiOptions) chain, rootStatus: X509ChainStatusFlags.NoError, issrStatus: ThisOsRevocationStatusUnknown, - leafStatus: X509ChainStatusFlags.Revoked); + leafStatus: ThisOsRevokedWithPreviousRevocationError); Assert.False(chainBuilt, "Chain built with ExcludeRoot."); holder.DisposeChainElements(); @@ -737,19 +783,22 @@ public static void RevokeEndEntity_RootRevocationOffline(PkiOptions pkiOptions) Assert.False(chainBuilt, "Chain built with EndCertificateOnly"); holder.DisposeChainElements(); - chain.ChainPolicy.RevocationFlag = X509RevocationFlag.EntireChain; + if (SupportsEntireChainCheck) + { + chain.ChainPolicy.RevocationFlag = X509RevocationFlag.EntireChain; - chainBuilt = chain.Build(endEntity); + chainBuilt = chain.Build(endEntity); - // Potentially surprising result: Even in EntireChain mode, - // root revocation is NoError, not RevocationStatusUnknown. - AssertChainStatus( - chain, - rootStatus: X509ChainStatusFlags.NoError, - issrStatus: ThisOsRevocationStatusUnknown, - leafStatus: X509ChainStatusFlags.Revoked); + // Potentially surprising result: Even in EntireChain mode, + // root revocation is NoError, not RevocationStatusUnknown. + AssertChainStatus( + chain, + rootStatus: X509ChainStatusFlags.NoError, + issrStatus: ThisOsRevocationStatusUnknown, + leafStatus: X509ChainStatusFlags.Revoked); - Assert.False(chainBuilt, "Chain built with EntireChain"); + Assert.False(chainBuilt, "Chain built with EntireChain"); + } } } @@ -792,7 +841,7 @@ public static void NothingRevoked_RootRevocationOffline(PkiOptions pkiOptions) chain, rootStatus: X509ChainStatusFlags.NoError, issrStatus: ThisOsRevocationStatusUnknown, - leafStatus: X509ChainStatusFlags.NoError); + leafStatus: ThisOsNoErrorWithPreviousRevocationError); Assert.False(chainBuilt, "Chain built with ExcludeRoot."); holder.DisposeChainElements(); @@ -810,19 +859,22 @@ public static void NothingRevoked_RootRevocationOffline(PkiOptions pkiOptions) Assert.True(chainBuilt, "Chain built with EndCertificateOnly"); holder.DisposeChainElements(); - chain.ChainPolicy.RevocationFlag = X509RevocationFlag.EntireChain; + if (SupportsEntireChainCheck) + { + chain.ChainPolicy.RevocationFlag = X509RevocationFlag.EntireChain; - chainBuilt = chain.Build(endEntity); + chainBuilt = chain.Build(endEntity); - // Potentially surprising result: Even in EntireChain mode, - // root revocation is NoError, not RevocationStatusUnknown. - AssertChainStatus( - chain, - rootStatus: X509ChainStatusFlags.NoError, - issrStatus: ThisOsRevocationStatusUnknown, - leafStatus: X509ChainStatusFlags.NoError); + // Potentially surprising result: Even in EntireChain mode, + // root revocation is NoError, not RevocationStatusUnknown. + AssertChainStatus( + chain, + rootStatus: X509ChainStatusFlags.NoError, + issrStatus: ThisOsRevocationStatusUnknown, + leafStatus: X509ChainStatusFlags.NoError); - Assert.False(chainBuilt, "Chain built with EntireChain"); + Assert.False(chainBuilt, "Chain built with EntireChain"); + } } } @@ -893,6 +945,19 @@ public static void RevokeEndEntityWithExpiredRevocation(PkiOptions pkiOptions) (root, intermediate, endEntity, holder, responder) => { DateTime revocationTime = endEntity.NotBefore; + if (PlatformDetection.IsAndroid) + { + // Android seems to use different times (+/- some buffer) to determine whether or not + // to use the revocation data it fetches. + // CRL : verification time + // OCSP : current time + // This test dynamically build the certs such that the current time falls within their + // period of validity (with more than a one second range), so we should be able to use + // the current time as revocation time and one second past that as verification time. + revocationTime = DateTime.UtcNow; + Assert.True(revocationTime >= endEntity.NotBefore && revocationTime < endEntity.NotAfter); + } + holder.Chain.ChainPolicy.VerificationTime = revocationTime.AddSeconds(1); intermediate.RevocationExpiration = revocationTime; @@ -917,6 +982,20 @@ public static void RevokeIntermediateWithExpiredRevocation(PkiOptions pkiOptions (root, intermediate, endEntity, holder, responder) => { DateTime revocationTime = endEntity.NotBefore; + if (PlatformDetection.IsAndroid) + { + // Android seems to use different times (+/- some buffer) to determine whether or not + // to use the revocation data it fetches. + // CRL : verification time + // OCSP : current time + // This test dynamically build the certs such that the current time falls within their + // period of validity (with more than a one second range), so we should be able to use + // the current time as revocation time and one second past that as verification time. + // This should allow the fetched data from both CRL and OCSP to be considered relevant. + revocationTime = DateTime.UtcNow; + Assert.True(revocationTime >= endEntity.NotBefore && revocationTime < endEntity.NotAfter); + } + holder.Chain.ChainPolicy.VerificationTime = revocationTime.AddSeconds(1); using (X509Certificate2 intermediatePub = intermediate.CloneIssuerCert()) @@ -938,11 +1017,23 @@ public static void RevokeIntermediateWithExpiredRevocation(PkiOptions pkiOptions [MemberData(nameof(AllViableRevocation))] public static void CheckEndEntityWithExpiredRevocation(PkiOptions pkiOptions) { + bool usingCrl = pkiOptions.HasFlag(PkiOptions.IssuerRevocationViaCrl) || pkiOptions.HasFlag(PkiOptions.EndEntityRevocationViaCrl); SimpleTest( pkiOptions, (root, intermediate, endEntity, holder, responder) => { intermediate.RevocationExpiration = endEntity.NotBefore; + if (PlatformDetection.IsAndroid && usingCrl) + { + // Android seems to use different times (+/- some buffer) to determine whether or not + // to use the revocation data it fetches. + // CRL : verification time + // OCSP : current time + // If using CRL, set the verification time to the current time. This should result in + // the fetched CRL for checking the issuer being considered relevant and that for the + // end entity considered irrelevant. + holder.Chain.ChainPolicy.VerificationTime = DateTime.UtcNow; + } RunWithInconclusiveEndEntityRevocation(holder, endEntity); }); @@ -953,11 +1044,23 @@ public static void CheckEndEntityWithExpiredRevocation(PkiOptions pkiOptions) [ActiveIssue("https://github.com/dotnet/runtime/issues/31249", TestPlatforms.OSX)] public static void CheckIntermediateWithExpiredRevocation(PkiOptions pkiOptions) { + bool usingCrl = pkiOptions.HasFlag(PkiOptions.IssuerRevocationViaCrl) || pkiOptions.HasFlag(PkiOptions.EndEntityRevocationViaCrl); SimpleTest( pkiOptions, (root, intermediate, endEntity, holder, responder) => { root.RevocationExpiration = endEntity.NotBefore; + if (PlatformDetection.IsAndroid && usingCrl) + { + // Android seems to use different times (+/- some buffer) to determine whether or not + // to use the revocation data it fetches. + // CRL : verification time + // OCSP : current time + // If using CRL, set the verification time to the current time. This should result in + // the fetched CRL for checking the issuer being considered irrelevant and that for the + // end entity considered relevant. + holder.Chain.ChainPolicy.VerificationTime = DateTime.UtcNow; + } RunWithInconclusiveIntermediateRevocation(holder, endEntity); }); @@ -977,12 +1080,26 @@ public static void TestRevocationWithNoNextUpdate_NotRevoked() // cache. We don't care about the build result. holder.Chain.Build(endEntity); - SimpleRevocationBody( - holder, - endEntity, - rootRevoked: false, - issrRevoked: false, - leafRevoked: false); + if (PlatformDetection.IsAndroid) + { + // Android uses the verification time when determining if a CRL is relevant. + // Set the verification time to the current time so that fetched CRLs will + // be considered relevant. + holder.Chain.ChainPolicy.VerificationTime = DateTime.UtcNow; + + // Android treats not having NextUpdate as invalid for determining revocation status, + // so the revocation status will be unknown + RunWithInconclusiveEndEntityRevocation(holder, endEntity); + } + else + { + SimpleRevocationBody( + holder, + endEntity, + rootRevoked: false, + issrRevoked: false, + leafRevoked: false); + } }); } @@ -1004,17 +1121,26 @@ public static void TestRevocationWithNoNextUpdate_Revoked() // cache. We don't care about the build result. holder.Chain.Build(endEntity); - SimpleRevocationBody( - holder, - endEntity, - rootRevoked: false, - issrRevoked: false, - leafRevoked: true); + if (PlatformDetection.IsAndroid) + { + // Android treats not having NextUpdate as invalid for determining revocation status, + // so the revocation status will be unknown + RunWithInconclusiveEndEntityRevocation(holder, endEntity); + } + else + { + SimpleRevocationBody( + holder, + endEntity, + rootRevoked: false, + issrRevoked: false, + leafRevoked: true); + } }); } [Fact] - [PlatformSpecific(~TestPlatforms.OSX)] //macOS does not support offline chain building. + [PlatformSpecific(~(TestPlatforms.Android | TestPlatforms.OSX))] // Android and macOS do not support offline revocation chain building. public static void TestRevocation_Offline_NotRevoked() { SimpleTest( @@ -1058,7 +1184,7 @@ public static void TestRevocation_Offline_NotRevoked() } [Fact] - [PlatformSpecific(~TestPlatforms.OSX)] //macOS does not support offline chain building. + [PlatformSpecific(~(TestPlatforms.Android | TestPlatforms.OSX))] // Android and macOS do not support offline revocation chain building. public static void TestRevocation_Offline_Revoked() { SimpleTest( @@ -1147,15 +1273,20 @@ private static void CheckRevokedRootDirectly( holder.DisposeChainElements(); X509Chain chain = holder.Chain; - chain.ChainPolicy.RevocationFlag = X509RevocationFlag.EntireChain; - bool chainBuilt = chain.Build(rootCert); + bool chainBuilt; + if (SupportsEntireChainCheck) + { + chain.ChainPolicy.RevocationFlag = X509RevocationFlag.EntireChain; + chainBuilt = chain.Build(rootCert); - Assert.Equal(1, chain.ChainElements.Count); - Assert.Equal(X509ChainStatusFlags.Revoked, chain.ChainElements[0].AllStatusFlags()); - Assert.Equal(X509ChainStatusFlags.Revoked, chain.AllStatusFlags()); - Assert.False(chainBuilt, "Chain validated with revoked root self-test, EntireChain"); + Assert.Equal(1, chain.ChainElements.Count); + Assert.Equal(X509ChainStatusFlags.Revoked, chain.ChainElements[0].AllStatusFlags()); + Assert.Equal(X509ChainStatusFlags.Revoked, chain.AllStatusFlags()); + Assert.False(chainBuilt, "Chain validated with revoked root self-test, EntireChain"); + + holder.DisposeChainElements(); + } - holder.DisposeChainElements(); chain.ChainPolicy.RevocationFlag = X509RevocationFlag.ExcludeRoot; chainBuilt = chain.Build(rootCert); @@ -1227,7 +1358,7 @@ private static void RunWithInconclusiveIntermediateRevocation( chain, rootStatus: X509ChainStatusFlags.NoError, issrStatus: ThisOsRevocationStatusUnknown, - leafStatus: X509ChainStatusFlags.NoError); + leafStatus: ThisOsNoErrorWithPreviousRevocationError); Assert.False(chainBuilt, "Chain built with ExcludeRoot (without flags)"); holder.DisposeChainElements(); @@ -1248,6 +1379,13 @@ private static void RunWithInconclusiveIntermediateRevocation( chain.ChainPolicy.RevocationFlag = X509RevocationFlag.ExcludeRoot; chain.ChainPolicy.VerificationFlags |= X509VerificationFlags.IgnoreCertificateAuthorityRevocationUnknown; + if (PlatformDetection.IsAndroid) + { + // Android stops validation at the first failure, so the end certificate would + // end up marked with PartialChain and RevocationStatusUnknown + chain.ChainPolicy.VerificationFlags |= X509VerificationFlags.AllowUnknownCertificateAuthority; + chain.ChainPolicy.VerificationFlags |= X509VerificationFlags.IgnoreEndRevocationUnknown; + } chainBuilt = chain.Build(endEntity); @@ -1255,7 +1393,7 @@ private static void RunWithInconclusiveIntermediateRevocation( chain, rootStatus: X509ChainStatusFlags.NoError, issrStatus: ThisOsRevocationStatusUnknown, - leafStatus: X509ChainStatusFlags.NoError); + leafStatus: ThisOsNoErrorWithPreviousRevocationError); Assert.True(chainBuilt, "Chain built with ExcludeRoot (with ignore flags)"); } @@ -1281,7 +1419,7 @@ private static void SimpleRevocationBody( AssertRevocationLevel(chain, endEntityCert, false, false, leafRevoked); - if (testWithRootRevocation) + if (testWithRootRevocation && SupportsEntireChainCheck) { holder.DisposeChainElements(); @@ -1308,8 +1446,8 @@ private static void AssertRevocationLevel( AssertChainStatus( chain, rootStatus: X509ChainStatusFlags.Revoked, - issrStatus: ThisOsRevocationStatusUnknown, - leafStatus: ThisOsRevocationStatusUnknown); + issrStatus: ThisOsRevocationStatusUnknownWithPreviousRevocationError, + leafStatus: ThisOsRevocationStatusUnknownWithPreviousRevocationError); Assert.False(chainBuilt, $"Chain built under {chain.ChainPolicy.RevocationFlag}"); } @@ -1321,7 +1459,7 @@ private static void AssertRevocationLevel( chain, rootStatus: X509ChainStatusFlags.NoError, issrStatus: X509ChainStatusFlags.Revoked, - leafStatus: ThisOsRevocationStatusUnknown); + leafStatus: ThisOsRevocationStatusUnknownWithPreviousRevocationError); Assert.False(chainBuilt, $"Chain built under {chain.ChainPolicy.RevocationFlag}"); } From 8626ff6f05aa04d3144f3ddb38b0aa281850c105 Mon Sep 17 00:00:00 2001 From: Elinor Fung Date: Mon, 22 Mar 2021 14:29:02 -0700 Subject: [PATCH 17/19] Skip validation with revocation if without revocation failed --- .../Interop.X509Chain.cs | 7 +-- .../pal_x509chain.c | 48 ++++++++--------- .../pal_x509chain.h | 8 +-- .../Cryptography/Pal.Android/ChainPal.cs | 51 ++++++++++--------- .../RevocationTests/DynamicRevocationTests.cs | 23 +++------ 5 files changed, 60 insertions(+), 77 deletions(-) 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 b95e0bdeeaccb..70b3f60bf3512 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 @@ -90,15 +90,12 @@ 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, X509RevocationMode revocationMode, - X509RevocationFlag revocationFlag); + X509RevocationFlag revocationFlag, + out byte checkedRevocation); } } 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 0c14bdaf80053..f57753fa799a8 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 @@ -240,8 +240,8 @@ enum static PAL_X509ChainStatusFlags ChainStatusFromValidatorExceptionReason(JNIEnv* env, jobject reason) { int value = (*env)->CallIntMethod(env, reason, g_EnumOrdinal); - if (g_CertPathExceptionBasicReasonClass != NULL - && (*env)->IsInstanceOf(env, reason, g_CertPathExceptionBasicReasonClass)) + if (g_CertPathExceptionBasicReasonClass != NULL && + (*env)->IsInstanceOf(env, reason, g_CertPathExceptionBasicReasonClass)) { switch (value) { @@ -302,6 +302,10 @@ static void PopulateValidationError(JNIEnv* env, jobject error, bool isRevocatio (*env)->DeleteLocalRef(env, reason); } } + else + { + chainStatus = isRevocationError ? PAL_X509ChainRevocationStatusUnknown : PAL_X509ChainPartialChain; + } jobject message = (*env)->CallObjectMethod(env, error, g_ThrowableGetMessage); uint16_t* messagePtr = NULL; @@ -400,7 +404,7 @@ int32_t AndroidCryptoNative_X509ChainSetCustomTrustStore(X509ChainContext* ctx, return CheckJNIExceptions(env) ? FAIL : SUCCESS; } -bool AndroidCryptoNative_X509ChainSupportsRevocationOptions(void) +static bool X509ChainSupportsRevocationOptions(void) { return g_CertPathValidatorGetRevocationChecker != NULL && g_PKIXRevocationCheckerClass != NULL; } @@ -440,8 +444,7 @@ static int32_t ValidateWithRevocation(JNIEnv* env, X509ChainContext* ctx, jobject /*CertPathValidator*/ validator, PAL_X509RevocationMode revocationMode, - PAL_X509RevocationFlag revocationFlag, - bool hadErrorsWithoutRevocation) + PAL_X509RevocationFlag revocationFlag) { assert(ctx != NULL); assert(validator != NULL); @@ -480,7 +483,7 @@ static int32_t ValidateWithRevocation(JNIEnv* env, else { certPathToUse = ctx->certPath; - if (AndroidCryptoNative_X509ChainSupportsRevocationOptions()) + if (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. @@ -509,7 +512,7 @@ static int32_t ValidateWithRevocation(JNIEnv* env, } jobject params = ctx->params; - if (AndroidCryptoNative_X509ChainSupportsRevocationOptions()) + if (X509ChainSupportsRevocationOptions()) { // PKIXRevocationChecker checker = validator.getRevocationChecker(); loc[checker] = (*env)->CallObjectMethod(env, validator, g_CertPathValidatorGetRevocationChecker); @@ -532,21 +535,12 @@ static int32_t ValidateWithRevocation(JNIEnv* env, loc[result] = (*env)->CallObjectMethod(env, validator, g_CertPathValidatorValidate, certPathToUse, params); if (TryGetJNIException(env, &loc[ex], false /*printException*/)) { - if (hadErrorsWithoutRevocation) + if (ctx->revocationErrorList == NULL) { - // Failed without revocation checking - we don't know for sure if these errors were from revocation checking - (*env)->CallBooleanMethod(env, ctx->errorList, g_ArrayListAdd, loc[ex]); + ctx->revocationErrorList = ToGRef(env, (*env)->NewObject(env, g_ArrayListClass, g_ArrayListCtor)); } - else - { - // Succeeded without revocation checking - errors must be from revocation checking - if (ctx->revocationErrorList == NULL) - { - ctx->revocationErrorList = ToGRef(env, (*env)->NewObject(env, g_ArrayListClass, g_ArrayListCtor)); - } - (*env)->CallBooleanMethod(env, ctx->revocationErrorList, g_ArrayListAdd, loc[ex]); - } + (*env)->CallBooleanMethod(env, ctx->revocationErrorList, g_ArrayListAdd, loc[ex]); } ret = SUCCESS; @@ -558,11 +552,14 @@ static int32_t ValidateWithRevocation(JNIEnv* env, int32_t AndroidCryptoNative_X509ChainValidate(X509ChainContext* ctx, PAL_X509RevocationMode revocationMode, - PAL_X509RevocationFlag revocationFlag) + PAL_X509RevocationFlag revocationFlag, + bool* checkedRevocation) { assert(ctx != NULL); + assert(checkedRevocation != NULL); JNIEnv* env = GetJNIEnv(); + *checkedRevocation = false; int32_t ret = FAIL; INIT_LOCALS(loc, validatorType, validator, result, ex); @@ -576,16 +573,15 @@ int32_t AndroidCryptoNative_X509ChainValidate(X509ChainContext* ctx, // PKIXCertPathValidatorResult result = (PKIXCertPathValidatorResult)validator.validate(certPath, params); loc[result] = (*env)->CallObjectMethod(env, loc[validator], g_CertPathValidatorValidate, ctx->certPath, ctx->params); - bool hadErrorsWithoutRevocation = TryGetJNIException(env, &loc[ex], false /*printException*/); - if (hadErrorsWithoutRevocation) + if (TryGetJNIException(env, &loc[ex], false /*printException*/)) { (*env)->CallBooleanMethod(env, ctx->errorList, g_ArrayListAdd, loc[ex]); + ret = SUCCESS; } - - if (revocationMode != X509RevocationMode_NoCheck) + else if (revocationMode != X509RevocationMode_NoCheck) { - ret = ValidateWithRevocation( - env, ctx, loc[validator], revocationMode, revocationFlag, hadErrorsWithoutRevocation); + ret = ValidateWithRevocation(env, ctx, loc[validator], revocationMode, revocationFlag); + *checkedRevocation = true; } else { 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 613f5355b7d76..3e2dce658711c 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 @@ -62,11 +62,6 @@ PALEXPORT int32_t AndroidCryptoNative_X509ChainSetCustomTrustStore(X509ChainCont jobject* /*X509Certificate[]*/ customTrustStore, int32_t customTrustStoreLen); -/* -Return whether or not revocation options are supported. -*/ -PALEXPORT bool AndroidCryptoNative_X509ChainSupportsRevocationOptions(void); - // Matches managed X509RevocationMode enum enum { @@ -90,4 +85,5 @@ Validate a certificate path. */ PALEXPORT int32_t AndroidCryptoNative_X509ChainValidate(X509ChainContext* chain, PAL_X509RevocationMode revocationMode, - PAL_X509RevocationFlag revocationFlag); + PAL_X509RevocationFlag revocationFlag, + bool* checkedRevocation); diff --git a/src/libraries/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Android/ChainPal.cs b/src/libraries/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Android/ChainPal.cs index 9433621a55c66..1af847e0defe9 100644 --- a/src/libraries/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Android/ChainPal.cs +++ b/src/libraries/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Android/ChainPal.cs @@ -214,19 +214,8 @@ internal void Evaluate( return; } - if (revocationMode != X509RevocationMode.NoCheck) - { - if (!Interop.AndroidCrypto.X509ChainSupportsRevocationOptions()) - { - // Defaults to offline when revocation options are not available - if (revocationMode == X509RevocationMode.Online) - { - throw new NotImplementedException($"{nameof(Evaluate)} (X509RevocationMode.{revocationMode})"); - } - } - } - - int res = Interop.AndroidCrypto.X509ChainValidate(_chainContext, revocationMode, revocationFlag); + byte checkedRevocation; + int res = Interop.AndroidCrypto.X509ChainValidate(_chainContext, revocationMode, revocationFlag, out checkedRevocation); if (res != 1) throw new CryptographicException(); @@ -234,7 +223,10 @@ internal void Evaluate( List overallStatus = new List(); List[] statuses = new List[certs.Length]; - int firstErrorIndex = -1; + // Android will stop checking after the first error it hits, so we track the first + // instances of revocation and non-revocation errors to fix-up the status of elements + // beyond the first error + int firstNonRevocationErrorIndex = -1; int firstRevocationErrorIndex = -1; Dictionary> errorsByIndex = GetStatusByIndex(_chainContext); foreach (int index in errorsByIndex.Keys) @@ -250,24 +242,26 @@ internal void Evaluate( if (index != -1) { statuses[index] = errorsByIndex[index]; - firstErrorIndex = Math.Max(index, firstErrorIndex); if (errorsByIndex[index].Exists(s => s.Status == X509ChainStatusFlags.Revoked || s.Status == X509ChainStatusFlags.RevocationStatusUnknown)) { firstRevocationErrorIndex = Math.Max(index, firstRevocationErrorIndex); } + else + { + firstNonRevocationErrorIndex = Math.Max(index, firstNonRevocationErrorIndex); + } } } - // Android will stop checking after the first error it hits, so we explicitly - // assign PartialChain to everything from the first error to the end certificate - if (firstErrorIndex > 0) + if (firstNonRevocationErrorIndex > 0) { + // Assign PartialChain to everything from the first non-revocation error to the end certificate X509ChainStatus partialChainStatus = new X509ChainStatus { Status = X509ChainStatusFlags.PartialChain, StatusInformation = SR.Chain_PartialChain, }; - AddStatusFromIndexToEndCertificate(firstErrorIndex - 1, ref partialChainStatus, statuses, overallStatus); + AddStatusFromIndexToEndCertificate(firstNonRevocationErrorIndex - 1, ref partialChainStatus, statuses, overallStatus); } if (firstRevocationErrorIndex > 0) @@ -281,6 +275,18 @@ internal void Evaluate( AddStatusFromIndexToEndCertificate(firstRevocationErrorIndex - 1, ref revocationUnknownStatus, statuses, overallStatus); } + if (revocationMode != X509RevocationMode.NoCheck && checkedRevocation == 0) + { + // Revocation checking was requested, but not performed (due to basic validation failing) + // Assign RevocationStatusUnknown to everything + X509ChainStatus revocationUnknownStatus = new X509ChainStatus + { + Status = X509ChainStatusFlags.RevocationStatusUnknown, + StatusInformation = SR.Chain_RevocationStatusUnknown, + }; + AddStatusFromIndexToEndCertificate(statuses.Length - 1, ref revocationUnknownStatus, statuses, overallStatus); + } + if (!IsPolicyMatch(certs, applicationPolicy, certificatePolicy)) { // Assign NotValidForUsage to everything @@ -355,12 +361,7 @@ private static Dictionary> GetStatusByIndex(SafeX509C private static X509ChainStatus ValidationErrorToChainStatus(Interop.AndroidCrypto.ValidationError error) { X509ChainStatusFlags statusFlags = (X509ChainStatusFlags)error.Status; - if (statusFlags == X509ChainStatusFlags.NoError) - { - // Android returns NoError as the error status when it cannot determine the status - // We just map that to partial chain. - statusFlags = X509ChainStatusFlags.PartialChain; - } + Debug.Assert(statusFlags != X509ChainStatusFlags.NoError); return new X509ChainStatus { 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 2671660cc7930..209d1912c8d85 100644 --- a/src/libraries/System.Security.Cryptography.X509Certificates/tests/RevocationTests/DynamicRevocationTests.cs +++ b/src/libraries/System.Security.Cryptography.X509Certificates/tests/RevocationTests/DynamicRevocationTests.cs @@ -25,24 +25,17 @@ public static class DynamicRevocationTests X509ChainStatusFlags.RevocationStatusUnknown | X509ChainStatusFlags.OfflineRevocation; // Android will stop checking after the first revocation error, so any revoked certificates - // after will have PartialChain and RevocationStatusUnknown instead of Revoked + // after will have RevocationStatusUnknown instead of Revoked private static readonly X509ChainStatusFlags ThisOsRevokedWithPreviousRevocationError = PlatformDetection.IsAndroid ? - X509ChainStatusFlags.PartialChain | X509ChainStatusFlags.RevocationStatusUnknown : + X509ChainStatusFlags.RevocationStatusUnknown : X509ChainStatusFlags.Revoked; - // Android will stop checking after the first revocation error, so any revoked certificates - // after will have PartialChain and RevocationStatusUnknown instead of RevocationStatusUnknown - private static readonly X509ChainStatusFlags ThisOsRevocationStatusUnknownWithPreviousRevocationError = - PlatformDetection.IsAndroid ? - X509ChainStatusFlags.PartialChain | X509ChainStatusFlags.RevocationStatusUnknown : - ThisOsRevocationStatusUnknown; - // Android will stop checking after the first revocation error, so any non-revoked certificates - // after will have PartialChain and RevocationStatusUnknown instead of NoError + // after will have RevocationStatusUnknown instead of NoError private static readonly X509ChainStatusFlags ThisOsNoErrorWithPreviousRevocationError = PlatformDetection.IsAndroid ? - X509ChainStatusFlags.PartialChain | X509ChainStatusFlags.RevocationStatusUnknown : + X509ChainStatusFlags.RevocationStatusUnknown : X509ChainStatusFlags.NoError; private delegate void RunSimpleTest( @@ -608,7 +601,7 @@ public static void RevokeIntermediate_PolicyErrors_NotTimeValid(bool policyError chain, rootStatus: issuerExtraProblems, issrStatus: issuerExtraProblems | X509ChainStatusFlags.Revoked, - leafStatus: leafProblems | ThisOsRevocationStatusUnknownWithPreviousRevocationError); + leafStatus: leafProblems | ThisOsRevocationStatusUnknown); Assert.False(chainBuilt, "Chain built with ExcludeRoot."); holder.DisposeChainElements(); @@ -1446,8 +1439,8 @@ private static void AssertRevocationLevel( AssertChainStatus( chain, rootStatus: X509ChainStatusFlags.Revoked, - issrStatus: ThisOsRevocationStatusUnknownWithPreviousRevocationError, - leafStatus: ThisOsRevocationStatusUnknownWithPreviousRevocationError); + issrStatus: ThisOsRevocationStatusUnknown, + leafStatus: ThisOsRevocationStatusUnknown); Assert.False(chainBuilt, $"Chain built under {chain.ChainPolicy.RevocationFlag}"); } @@ -1459,7 +1452,7 @@ private static void AssertRevocationLevel( chain, rootStatus: X509ChainStatusFlags.NoError, issrStatus: X509ChainStatusFlags.Revoked, - leafStatus: ThisOsRevocationStatusUnknownWithPreviousRevocationError); + leafStatus: ThisOsRevocationStatusUnknown); Assert.False(chainBuilt, $"Chain built under {chain.ChainPolicy.RevocationFlag}"); } From 3628031eb6752ecc992e40ffa446b9af81671fab Mon Sep 17 00:00:00 2001 From: Elinor Fung Date: Mon, 22 Mar 2021 15:31:59 -0700 Subject: [PATCH 18/19] Update tests --- .../tests/ChainTests.cs | 166 +++++------------- .../RevocationTests/DynamicRevocationTests.cs | 5 +- 2 files changed, 47 insertions(+), 124 deletions(-) diff --git a/src/libraries/System.Security.Cryptography.X509Certificates/tests/ChainTests.cs b/src/libraries/System.Security.Cryptography.X509Certificates/tests/ChainTests.cs index 0ff97c6cbe489..60e328287bb05 100644 --- a/src/libraries/System.Security.Cryptography.X509Certificates/tests/ChainTests.cs +++ b/src/libraries/System.Security.Cryptography.X509Certificates/tests/ChainTests.cs @@ -303,89 +303,49 @@ public static IEnumerable BuildChainCustomTrustStoreData() } [Theory] - [PlatformSpecific(~TestPlatforms.Android)] // Test relies on AIA fetching, which Android does not support [MemberData(nameof(BuildChainCustomTrustStoreData))] public static void BuildChainCustomTrustStore( bool chainBuildsSuccessfully, X509ChainStatusFlags chainFlags, BuildChainCustomTrustStoreTestArguments testArguments) { - using (var microsoftDotCom = new X509Certificate2(TestData.MicrosoftDotComSslCertBytes)) - using (var chainHolderPrep = new ChainHolder()) + using (var endCert = new X509Certificate2(TestData.MicrosoftDotComSslCertBytes)) + using (var issuerCert = new X509Certificate2(TestData.MicrosoftDotComIssuerBytes)) + using (var rootCert = new X509Certificate2(TestData.MicrosoftDotComRootBytes)) + using (var chainHolder = new ChainHolder()) { - X509Chain chainPrep = chainHolderPrep.Chain; - chainPrep.ChainPolicy.RevocationMode = X509RevocationMode.NoCheck; - chainPrep.ChainPolicy.VerificationTime = microsoftDotCom.NotBefore.AddSeconds(1); - - chainPrep.Build(microsoftDotCom); + X509Chain chainTest = chainHolder.Chain; + chainTest.ChainPolicy.RevocationMode = X509RevocationMode.NoCheck; + chainTest.ChainPolicy.VerificationTime = endCert.NotBefore.AddSeconds(1); + chainTest.ChainPolicy.TrustMode = X509ChainTrustMode.CustomRootTrust; + chainTest.ChainPolicy.ExtraStore.Add(issuerCert); - Assert.True(chainPrep.ChainElements.Count >= 3, "Expected chain to build with root cert as third element"); - X509Certificate2 rootCert = chainPrep.ChainElements[2].Certificate; - - using (var chainHolderTest = new ChainHolder()) + switch (testArguments) { - BuildChainCustomTrustStoreImpl(chainHolderTest, rootCert, microsoftDotCom, chainBuildsSuccessfully, chainFlags, testArguments); + case BuildChainCustomTrustStoreTestArguments.TrustedIntermediateUntrustedRoot: + chainTest.ChainPolicy.ExtraStore.Add(rootCert); + break; + case BuildChainCustomTrustStoreTestArguments.UntrustedIntermediateTrustedRoot: + chainTest.ChainPolicy.CustomTrustStore.Add(rootCert); + break; + case BuildChainCustomTrustStoreTestArguments.TrustedIntermediateTrustedRoot: + chainTest.ChainPolicy.CustomTrustStore.Add(rootCert); + break; + case BuildChainCustomTrustStoreTestArguments.MultipleCalls: + chainTest.ChainPolicy.CustomTrustStore.Add(rootCert); + chainTest.Build(endCert); + chainHolder.DisposeChainElements(); + chainTest.ChainPolicy.CustomTrustStore.Remove(rootCert); + chainTest.ChainPolicy.TrustMode = X509ChainTrustMode.System; + break; + default: + throw new InvalidDataException(); } - } - } - - [Theory] - [PlatformSpecific(TestPlatforms.Android)] - [MemberData(nameof(BuildChainCustomTrustStoreData))] - public static void BuildChainCustomTrustStore_Android( - bool chainBuildsSuccessfully, - X509ChainStatusFlags chainFlags, - BuildChainCustomTrustStoreTestArguments testArguments) - { - using (var microsoftDotCom = new X509Certificate2(TestData.MicrosoftDotComSslCertBytes)) - using (var microsoftDotComIssuer = new X509Certificate2(TestData.MicrosoftDotComIssuerBytes)) - using (var microsoftDotComRoot = new X509Certificate2(TestData.MicrosoftDotComRootBytes)) - using (var chainHolder = new ChainHolder()) - { - // Fetching is not supported on Android, so we need to add intermediate to the extra store - chainHolder.Chain.ChainPolicy.ExtraStore.Add(microsoftDotComIssuer); - BuildChainCustomTrustStoreImpl(chainHolder, microsoftDotComRoot, microsoftDotCom, chainBuildsSuccessfully, chainFlags, testArguments); - } - } - private static void BuildChainCustomTrustStoreImpl( - ChainHolder chainHolder, - X509Certificate2 rootCert, - X509Certificate2 endCert, - bool chainBuildsSuccessfully, - X509ChainStatusFlags chainFlags, - BuildChainCustomTrustStoreTestArguments testArguments) - { - X509Chain chainTest = chainHolder.Chain; - chainTest.ChainPolicy.RevocationMode = X509RevocationMode.NoCheck; - chainTest.ChainPolicy.VerificationTime = endCert.NotBefore.AddSeconds(1); - chainTest.ChainPolicy.TrustMode = X509ChainTrustMode.CustomRootTrust; - - switch (testArguments) - { - case BuildChainCustomTrustStoreTestArguments.TrustedIntermediateUntrustedRoot: - chainTest.ChainPolicy.ExtraStore.Add(rootCert); - break; - case BuildChainCustomTrustStoreTestArguments.UntrustedIntermediateTrustedRoot: - chainTest.ChainPolicy.CustomTrustStore.Add(rootCert); - break; - case BuildChainCustomTrustStoreTestArguments.TrustedIntermediateTrustedRoot: - chainTest.ChainPolicy.CustomTrustStore.Add(rootCert); - break; - case BuildChainCustomTrustStoreTestArguments.MultipleCalls: - chainTest.ChainPolicy.CustomTrustStore.Add(rootCert); - chainTest.Build(endCert); - chainHolder.DisposeChainElements(); - chainTest.ChainPolicy.CustomTrustStore.Remove(rootCert); - chainTest.ChainPolicy.TrustMode = X509ChainTrustMode.System; - break; - default: - throw new InvalidDataException(); + Assert.Equal(chainBuildsSuccessfully, chainTest.Build(endCert)); + Assert.Equal(3, chainTest.ChainElements.Count); + Assert.Equal(chainFlags, chainTest.AllStatusFlags()); } - - Assert.Equal(chainBuildsSuccessfully, chainTest.Build(endCert)); - Assert.Equal(3, chainTest.ChainElements.Count); - Assert.Equal(chainFlags, chainTest.AllStatusFlags()); } [Fact] @@ -504,16 +464,6 @@ public static void VerifyExpiration_LocalTime(DateTime verificationTime, bool sh // Ignore anything except NotTimeValid chain.ChainPolicy.VerificationFlags = X509VerificationFlags.AllFlags & ~X509VerificationFlags.IgnoreNotTimeValid; - - // Android doesn't support certain flags and will error if the chain is - // invalid and one of those flags is set. - if (PlatformDetection.IsAndroid) - { - chain.ChainPolicy.VerificationFlags &= ~X509VerificationFlags.AllowUnknownCertificateAuthority; - chain.ChainPolicy.VerificationFlags &= ~X509VerificationFlags.IgnoreInvalidName; - chain.ChainPolicy.VerificationFlags &= ~X509VerificationFlags.IgnoreInvalidPolicy; - } - chain.ChainPolicy.RevocationMode = X509RevocationMode.NoCheck; chain.ChainPolicy.VerificationTime = verificationTime; @@ -978,21 +928,23 @@ public static void ChainWithEmptySubject() } [Fact] - [PlatformSpecific(~TestPlatforms.Android)] public static void BuildInvalidSignatureTwice() { byte[] bytes = (byte[])TestData.MsCertificate.Clone(); bytes[bytes.Length - 1] ^= 0xFF; + using (X509Certificate2 microsoftDotComIssuer = new X509Certificate2(TestData.MicrosoftDotComIssuerBytes)) + using (X509Certificate2 microsoftDotComRoot = new X509Certificate2(TestData.MicrosoftDotComRootBytes)) using (X509Certificate2 cert = new X509Certificate2(bytes)) using (ChainHolder chainHolder = new ChainHolder()) { X509Chain chain = chainHolder.Chain; chain.ChainPolicy.VerificationTime = cert.NotBefore.AddHours(2); - chain.ChainPolicy.VerificationFlags = - X509VerificationFlags.AllowUnknownCertificateAuthority; + chain.AllowUnknownAuthorityOrAddSelfSignedToCustomTrust(microsoftDotComRoot); chain.ChainPolicy.RevocationMode = X509RevocationMode.NoCheck; + chain.ChainPolicy.ExtraStore.Add(microsoftDotComRoot); + chain.ChainPolicy.ExtraStore.Add(microsoftDotComIssuer); int iter = 0; @@ -1015,6 +967,15 @@ void CheckChain() X509ChainStatusFlags.PartialChain, allFlags); } + else if (OperatingSystem.IsAndroid()) + { + // Android always validates signature as part of building a path, + // so invalid signature comes back as PartialChain with no elements + Assert.Equal(X509ChainStatusFlags.PartialChain, allFlags); + Assert.Equal(0, chain.ChainElements.Count); + Assert.False(valid, $"Chain should not be valid"); + chainHolder.DisposeChainElements(); + } else { // These asserts are "most informative first". @@ -1041,43 +1002,6 @@ void CheckChain() } } - [Fact] - [PlatformSpecific(TestPlatforms.Android)] - public static void BuildInvalidSignatureTwice_Android() - { - byte[] bytes = (byte[])TestData.MicrosoftDotComSslCertBytes.Clone(); - bytes[bytes.Length - 1] ^= 0xFF; - - using (X509Certificate2 microsoftDotComIssuer = new X509Certificate2(TestData.MicrosoftDotComIssuerBytes)) - using (X509Certificate2 microsoftDotComRoot = new X509Certificate2(TestData.MicrosoftDotComRootBytes)) - using (X509Certificate2 cert = new X509Certificate2(bytes)) - using (ChainHolder chainHolder = new ChainHolder()) - { - X509Chain chain = chainHolder.Chain; - chain.ChainPolicy.VerificationTime = cert.NotBefore.AddHours(2); - - chain.ChainPolicy.ExtraStore.Add(microsoftDotComRoot); - chain.ChainPolicy.ExtraStore.Add(microsoftDotComIssuer); - chain.ChainPolicy.RevocationMode = X509RevocationMode.NoCheck; - - CheckChain(); - CheckChain(); - - void CheckChain() - { - bool valid = chain.Build(cert); - X509ChainStatusFlags allFlags = chain.AllStatusFlags(); - - // Android always validates signature as part of building a path, - // so invalid signature comes back as PartialChain with no elements - Assert.Equal(X509ChainStatusFlags.PartialChain, allFlags); - Assert.Equal(0, chain.ChainElements.Count); - Assert.False(valid, $"Chain should not be valid"); - chainHolder.DisposeChainElements(); - } - } - } - [Fact] [PlatformSpecific(~TestPlatforms.Linux)] public static void BuildChainForFraudulentCertificate() 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 209d1912c8d85..7b311230b0d5d 100644 --- a/src/libraries/System.Security.Cryptography.X509Certificates/tests/RevocationTests/DynamicRevocationTests.cs +++ b/src/libraries/System.Security.Cryptography.X509Certificates/tests/RevocationTests/DynamicRevocationTests.cs @@ -94,7 +94,7 @@ public static IEnumerable AllViableRevocation [MemberData(nameof(AllViableRevocation))] public static void NothingRevoked(PkiOptions pkiOptions) { - bool usingCrl = pkiOptions.HasFlag(PkiOptions.IssuerRevocationViaCrl) || pkiOptions.HasFlag(PkiOptions.EndEntityRevocationViaCrl);// && !pkiOptions.HasFlag(PkiOptions.EndEntityRevocationViaOcsp); + bool usingCrl = pkiOptions.HasFlag(PkiOptions.IssuerRevocationViaCrl) || pkiOptions.HasFlag(PkiOptions.EndEntityRevocationViaCrl); SimpleTest( pkiOptions, (root, intermediate, endEntity, holder, responder) => @@ -1375,8 +1375,7 @@ private static void RunWithInconclusiveIntermediateRevocation( if (PlatformDetection.IsAndroid) { // Android stops validation at the first failure, so the end certificate would - // end up marked with PartialChain and RevocationStatusUnknown - chain.ChainPolicy.VerificationFlags |= X509VerificationFlags.AllowUnknownCertificateAuthority; + // end up marked with RevocationStatusUnknown chain.ChainPolicy.VerificationFlags |= X509VerificationFlags.IgnoreEndRevocationUnknown; } From d279986b2d8aa21db72022d16032f6e1970e4c2c Mon Sep 17 00:00:00 2001 From: Elinor Fung Date: Mon, 22 Mar 2021 15:37:50 -0700 Subject: [PATCH 19/19] Remove unnecessary dispose --- .../tests/ChainTests.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/libraries/System.Security.Cryptography.X509Certificates/tests/ChainTests.cs b/src/libraries/System.Security.Cryptography.X509Certificates/tests/ChainTests.cs index 60e328287bb05..9e5dbb03606e6 100644 --- a/src/libraries/System.Security.Cryptography.X509Certificates/tests/ChainTests.cs +++ b/src/libraries/System.Security.Cryptography.X509Certificates/tests/ChainTests.cs @@ -974,7 +974,6 @@ void CheckChain() Assert.Equal(X509ChainStatusFlags.PartialChain, allFlags); Assert.Equal(0, chain.ChainElements.Count); Assert.False(valid, $"Chain should not be valid"); - chainHolder.DisposeChainElements(); } else {