Repository navigation
Conversation
Contributor
|
Tagging subscribers to 'arch-android': @vitek-karas, @simonrozsival, @steveisok, @akoeplinger |
|
Azure Pipelines: Successfully started running 3 pipeline(s). 13 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
Contributor
|
Tagging subscribers to this area: @bartonjs, @vcsjones, @dotnet/area-system-security |
Contributor
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
The native cryptography changes are significant, and approval-readiness assessments are mixed; final human review is warranted.
Review effort: Lite
Findings: None
What changed in this PR
Enables Android to import EC keys containing only the private scalar D by recovering the public point through PKCS#8 export.
Changes:
- Adds Android EC private-key PKCS#8 export and public-key recovery.
- Supports constructing key pairs without an initial public key.
- Removes obsolete EC public-key derivation capability gating and updates tests.
| File | Description |
|---|---|
src/native/libs/System.Security.Cryptography.Native.Android/pal_misc.h |
Allows nullable key components. |
src/native/libs/System.Security.Cryptography.Native.Android/pal_ecc_import_export.h |
Declares PKCS#8 export support. |
src/native/libs/System.Security.Cryptography.Native.Android/pal_ecc_import_export.c |
Implements D-only key creation and private-key export. |
src/libraries/System.Security.Cryptography/tests/ECDsaTestRegistration.OpenSsl.cs |
Removes obsolete capability gating. |
src/libraries/System.Security.Cryptography/tests/ECDsaTestRegistration.Default.cs |
Removes obsolete capability gating. |
src/libraries/System.Security.Cryptography/tests/ECDsaTestRegistration.Cng.cs |
Removes obsolete capability gating. |
src/libraries/System.Security.Cryptography/tests/EcDiffieHellmanOpenSslProvider.cs |
Removes obsolete capability gating. |
src/libraries/System.Security.Cryptography/tests/ECDiffieHellmanCngProvider.cs |
Removes obsolete capability gating. |
src/libraries/System.Security.Cryptography/tests/DefaultECDiffieHellmanProvider.Windows.cs |
Removes obsolete capability gating. |
src/libraries/System.Security.Cryptography/tests/DefaultECDiffieHellmanProvider.Unix.cs |
Removes obsolete capability gating. |
src/libraries/System.Security.Cryptography/tests/DefaultECDiffieHellmanProvider.Browser.cs |
Updates provider test configuration. |
src/libraries/System.Security.Cryptography/tests/DefaultECDiffieHellmanProvider.Android.cs |
Updates Android provider test configuration. |
src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/ECDsa/ECDsaImportExport.cs |
Expands EC import/export coverage. |
src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/ECDiffieHellman/ECDiffieHellmanTests.ImportExport.cs |
Expands D-only import/export coverage. |
src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/ECDiffieHellman/ECDiffieHellmanProvider.cs |
Removes obsolete capability plumbing. |
src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/ECDiffieHellman/ECDhKeyFileTests.cs |
Updates key-file tests. |
src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/EC/ECKeyFileTests.LimitedPrivate.cs |
Updates limited-private-key tests. |
src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/EC/ECKeyFileTests.cs |
Removes obsolete derivation gating. |
src/libraries/Common/src/System/Security/Cryptography/ECAndroid.ImportExport.cs |
Recovers public parameters for D-only imports. |
src/libraries/Common/src/Interop/Android/System.Security.Cryptography.Native.Android/Interop.EcDsa.ImportExport.cs |
Adds managed PKCS#8 export interop. |
Member
Author
|
/azp run runtime-android |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Android was our last platform that did not support importing only the private scalar
dfor EC keys withoutQ.This change allows Android to import
ECParametersthat contain onlyD. Android already supported this - what it did not do was make it straight forward to ask the private key "what is your public key". This meant that importing a D-only key would fail to export the public key.This makes it work by importing the private key D, then performing a PKCS#8 export on it, which gives us back Q.
This also removes
CanDeriveNewPublicKeysince every platform now has it astrue, except forbrowser, which don't run the EC tests anyway.