Skip to content

Fix exporting public key from RSA key from Android KeyStore#128057

Open
vcsjones wants to merge 1 commit into
dotnet:mainfrom
vcsjones:android-fix-private-key-export
Open

Fix exporting public key from RSA key from Android KeyStore#128057
vcsjones wants to merge 1 commit into
dotnet:mainfrom
vcsjones:android-fix-private-key-export

Conversation

@vcsjones
Copy link
Copy Markdown
Member

Android has multiple ways to represent a private key. Our RSAParameters exporter assumed that we would always export off an instance of java.security.interfaces.RSAPrivateCrtKey. But that is not true if the private key was retrieved from the key store - the private key is represented by android.security.keystore.AndroidKeyStoreRSAPrivateKey.

This would crash the Android runtime with something like:

Abort message: 'art/runtime/java_vm_ext.cc:470] JNI DETECTED ERROR IN APPLICATION: can't call java.math.BigInteger java.security.interfaces.RSAPrivateCrtKey.getPublicExponent() on instance of android.security.keystore.AndroidKeyStoreRSAPrivateKey'

AndroidKeyStoreRSAPrivateKey do not permit exporting the private key because they are likely in the TEE. We can only get the public parameters.

This does mean, unfortunately, that ExportParameters(includePrivateParameters: true) will fail on KeyStore keys because Android doesn't let us get the key material back. We can get the public key back with includePrivateParameters: false.

This at least fixes the hard android runtime crash and turns it in to a CryptographicException.

Fixes #119924

Copilot AI review requested due to automatic review settings May 11, 2026 23:10
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the Android RSA parameter export path to avoid calling RSAPrivateCrtKey APIs on KeyStore-backed private key instances that don’t implement that interface, and adds coverage for the KeyStore scenario. The native layer now gates CRT-only parameter extraction behind an explicit includePrivateParameters flag and type checks, enabling public-parameter export without triggering a JNI abort.

Changes:

  • Add an includePrivateParameters argument to AndroidCryptoNative_GetRsaParameters and thread it through the managed interop.
  • In the native implementation, only export CRT private parameters when explicitly requested and the private key is an RSAPrivateCrtKey; otherwise export public parameters from the public key.
  • Add an Android-specific X509 store regression test ensuring public export succeeds and private export throws.
Show a summary per file
File Description
src/native/libs/System.Security.Cryptography.Native.Android/pal_rsa.h Updates RSA key-type comments and adds includePrivateParameters to the exported parameters API signature.
src/native/libs/System.Security.Cryptography.Native.Android/pal_rsa.c Implements conditional RSA parameter export: CRT-only private export when requested; otherwise export via public key or fail safely.
src/libraries/Common/src/Interop/Android/System.Security.Cryptography.Native.Android/Interop.Rsa.cs Updates the P/Invoke signature and passes the new includePrivateParameters flag to native.
src/libraries/System.Security.Cryptography/tests/X509Certificates/X509StoreMutableTests.Android.cs Adds a regression test for KeyStore-backed RSA public export and private export failure behavior.

Copilot's findings

  • Files reviewed: 4/4 changed files
  • Comments generated: 1

Copy link
Copy Markdown
Member

@bartonjs bartonjs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't remember off the top of my head of FindByThumprint clones the cert instances or not. If Copilot's right, then, yeah, dispose storeCert. (If it's wrong, the extra dispose doesn't really hurt, but isn't needed)

@vcsjones
Copy link
Copy Markdown
Member Author

I don't remember off the top of my head of FindByThumprint clones the cert instances or not.

It doesn't.

@dotnet-policy-service
Copy link
Copy Markdown
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Android cryptography crash when using KeyStore key

3 participants