Skip to content

Commit 9db423a

Browse files
simonrozsivalCopilotvcsjones
authored
[Android] Handle X509ChainContext creation failures (dotnet#128651)
An Android production app reported a native abort while building an X.509 chain on arm64. The available tombstone snippet showed the process aborting in `AndroidCryptoNative_X509ChainBuild` from `pal_x509chain.c`, with the native guard reporting that parameter `ctx` was not a valid pointer. The report did not include a repro or full tombstone, but the observed failure mode means managed code reached the native build entry point with a null `X509ChainContext*`. ``` Thread /__w/1/s/src/native/libs/System.Security.Cryptography.Native.Android/pal_x509chain.c:113 (AndroidCryptoNative_X509ChainBuild): Parameter 'ctx' must be a valid pointer *** *** *** *** *** *** *** *** *** *** *** *** *** *** *** *** pid: 0, tid: 31609 >>> com.app.name <<< backtrace: #00 pc 0x000000000002232c /system/lib64/libc.so (abort+116) #1 pc 0x0000000000021fe8 [removed]-KwPZdoEumri00C7kBm3pQw==/lib/arm64/libSystem.Security.Cryptography.Native.Android.so #2 pc 0x00000000000220b0 [removed]-KwPZdoEumri00C7kBm3pQw==/lib/arm64/libSystem.Security.Cryptography.Native.Android.so (AndroidCryptoNative_X509ChainBuild+88) #3 pc 0x000000000000cfcc ``` `X509ChainContext` is created by `AndroidCryptoNative_X509ChainCreateContext`. That initialization can fail if Android certificate store setup or PKIX parameter construction throws, or if required JNI global references cannot be created. Previously, the managed Android chain path stored the returned `SafeHandle` without checking whether context creation failed, so a later build could pass a null native context to `AndroidCryptoNative_X509ChainBuild` and terminate the app process. This change makes context creation fail gracefully: - The native create path checks Java exceptions around object creation and method calls more consistently. - Partial native contexts are destroyed if global-reference creation fails. - The Android interop wrapper checks the returned chain context immediately, including a null safe-handle return, and throws `CryptographicException` if initialization failed. No regression test is included because the reliable failure modes depend on Android platform/provider state or artificial fault injection, and a test hook would be fragile and not representative. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Kevin Jones <kevin@vcsjones.com>
1 parent 890d67a commit 9db423a

4 files changed

Lines changed: 30 additions & 5 deletions

File tree

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

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,24 @@ internal static partial class Interop
1313
internal static partial class AndroidCrypto
1414
{
1515
[LibraryImport(Libraries.AndroidCryptoNative, EntryPoint = "AndroidCryptoNative_X509ChainCreateContext")]
16-
internal static partial SafeX509ChainContextHandle X509ChainCreateContext(
16+
private static partial SafeX509ChainContextHandle X509ChainCreateContext(
1717
SafeX509Handle cert,
1818
IntPtr[] extraStore,
1919
int extraStoreLen);
2020

21+
internal static SafeX509ChainContextHandle X509ChainCreateContext(SafeX509Handle cert, IntPtr[] extraStore)
22+
{
23+
SafeX509ChainContextHandle chainContext = X509ChainCreateContext(cert, extraStore, extraStore.Length);
24+
25+
if (chainContext.IsInvalid)
26+
{
27+
chainContext.Dispose();
28+
throw new CryptographicException(SR.Cryptography_AndroidX509ChainContextInitializationFailed);
29+
}
30+
31+
return chainContext;
32+
}
33+
2134
[LibraryImport(Libraries.AndroidCryptoNative, EntryPoint = "AndroidCryptoNative_X509ChainDestroyContext")]
2235
internal static partial void X509ChainDestroyContext(IntPtr ctx);
2336

src/libraries/System.Security.Cryptography/src/Resources/Strings.resx

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -237,6 +237,9 @@
237237
<data name="Cryptography_AlgorithmTypesMustBeVisible" xml:space="preserve">
238238
<value>Algorithms added to CryptoConfig must be accessible from outside their assembly.</value>
239239
</data>
240+
<data name="Cryptography_AndroidX509ChainContextInitializationFailed" xml:space="preserve">
241+
<value>The Android X.509 certificate chain could not be initialized.</value>
242+
</data>
240243
<data name="Cryptography_ArgDSARequiresDSAKey" xml:space="preserve">
241244
<value>Keys used with the DSACng algorithm must have an algorithm group of DSA.</value>
242245
</data>

src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/ChainPal.Android.cs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -141,8 +141,7 @@ internal void Initialize(
141141

142142
_chainContext = Interop.AndroidCrypto.X509ChainCreateContext(
143143
((AndroidCertificatePal)cert).SafeHandle,
144-
extraCerts,
145-
extraCerts.Length);
144+
extraCerts);
146145

147146
if (useCustomRootTrust)
148147
{

src/native/libs/System.Security.Cryptography.Native.Android/pal_x509chain.c

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ X509ChainContext* AndroidCryptoNative_X509ChainCreateContext(jobject /*X509Certi
3939
JNIEnv* env = GetJNIEnv();
4040

4141
X509ChainContext* ret = NULL;
42-
INIT_LOCALS(loc, keyStoreType, keyStore, targetSel, params, certList, certStoreType, certStoreParams, certStore);
42+
INIT_LOCALS(loc, keyStoreType, keyStore, targetSel, params, certList, certStoreType, certStoreParams, certStore, errorList);
4343

4444
// String keyStoreType = "AndroidCAStore";
4545
// KeyStore keyStore = KeyStore.getInstance(keyStoreType);
@@ -53,7 +53,9 @@ X509ChainContext* AndroidCryptoNative_X509ChainCreateContext(jobject /*X509Certi
5353
// X509CertSelector targetSel = new X509CertSelector();
5454
// targetSel.setCertificate(cert);
5555
loc[targetSel] = (*env)->NewObject(env, g_X509CertSelectorClass, g_X509CertSelectorCtor);
56+
ON_EXCEPTION_PRINT_AND_GOTO(cleanup);
5657
(*env)->CallVoidMethod(env, loc[targetSel], g_X509CertSelectorSetCertificate, cert);
58+
ON_EXCEPTION_PRINT_AND_GOTO(cleanup);
5759

5860
// PKIXBuilderParameters params = new PKIXBuilderParameters(keyStore, targetSelector);
5961
loc[params] = (*env)->NewObject(
@@ -66,10 +68,13 @@ X509ChainContext* AndroidCryptoNative_X509ChainCreateContext(jobject /*X509Certi
6668
// certList.add(extraStore[i]);
6769
// }
6870
loc[certList] = (*env)->NewObject(env, g_ArrayListClass, g_ArrayListCtorWithCapacity, extraStoreLen);
71+
ON_EXCEPTION_PRINT_AND_GOTO(cleanup);
6972
(*env)->CallBooleanMethod(env, loc[certList], g_ArrayListAdd, cert);
73+
ON_EXCEPTION_PRINT_AND_GOTO(cleanup);
7074
for (int i = 0; i < extraStoreLen; ++i)
7175
{
7276
(*env)->CallBooleanMethod(env, loc[certList], g_ArrayListAdd, extraStore[i]);
77+
ON_EXCEPTION_PRINT_AND_GOTO(cleanup);
7378
}
7479

7580
// String certStoreType = "Collection";
@@ -78,16 +83,21 @@ X509ChainContext* AndroidCryptoNative_X509ChainCreateContext(jobject /*X509Certi
7883
loc[certStoreType] = make_java_string(env, "Collection");
7984
loc[certStoreParams] = (*env)->NewObject(
8085
env, g_CollectionCertStoreParametersClass, g_CollectionCertStoreParametersCtor, loc[certList]);
86+
ON_EXCEPTION_PRINT_AND_GOTO(cleanup);
8187
loc[certStore] = (*env)->CallStaticObjectMethod(
8288
env, g_CertStoreClass, g_CertStoreGetInstance, loc[certStoreType], loc[certStoreParams]);
8389
ON_EXCEPTION_PRINT_AND_GOTO(cleanup);
8490

8591
// params.addCertStore(certStore);
8692
(*env)->CallVoidMethod(env, loc[params], g_PKIXBuilderParametersAddCertStore, loc[certStore]);
93+
ON_EXCEPTION_PRINT_AND_GOTO(cleanup);
94+
95+
loc[errorList] = (*env)->NewObject(env, g_ArrayListClass, g_ArrayListCtor);
96+
ON_EXCEPTION_PRINT_AND_GOTO(cleanup);
8797

8898
ret = xcalloc(1, sizeof(X509ChainContext));
8999
ret->params = AddGRef(env, loc[params]);
90-
ret->errorList = ToGRef(env, (*env)->NewObject(env, g_ArrayListClass, g_ArrayListCtor));
100+
ret->errorList = AddGRef(env, loc[errorList]);
91101

92102
cleanup:
93103
RELEASE_LOCALS(loc, env);

0 commit comments

Comments
 (0)