[Android] Put Java JNI function in a separate static library - #108513

Open
grendello wants to merge 9 commits into
dotnet:mainfrom
grendello:dev/grendel/android-crypto-public-symbols
Open

[Android] Put Java JNI function in a separate static library#108513
grendello wants to merge 9 commits into
dotnet:mainfrom
grendello:dev/grendel/android-crypto-public-symbols

Conversation

@grendello

Copy link
Copy Markdown
Contributor

In dotnet/android#9006 we are working on linking the .NET for Android
runtime dynamically at the application build time.

Linking includes all the relevant BCL native libraries, and one of the goals is to
hide all the exported symbols used as p/invokes by the managed BCL
libraries. This is because p/invoke calls are handled internally and,
with dynamic linking of the runtime, there is no longer any reason to
use dlopen and dlsym to look them up, they are all resolved
internally, at the link time.

Symbol hiding works fine thanks to the --exclude-libsclang flag,
which makes all the exported symbols in the indicated .a archives to
not be exported by the linker. However, System.Security.Cryptography.Native.Android
is special in the sense that it contains one symbol which must not be hidden,
Java_net_dot_android_crypto_DotnetProxyTrustManager_verifyRemoteCertificate.

The above function is a Java native method implementation, and it
requires that not only its name follows the Java JNI naming rules, but
that it is also available for the JVM to look up using dlsym.

I tried using the --export-dynamic-symbol clang flag to
export just this function, but it doesn't appear to work no matter
where I put the flag in relation to reference to the .a archives.

Instead, the problem can be dealt with by putting the JNI function in a
separate static library, so that I can link it without changing symbol
visibility, while making all the System.Security.Cryptography.Native.Android
symbols invisible.

@dotnet-policy-servicedotnet-policy-serviceBot added the community-contribution Indicates that the PR has been added by a community member label Oct 3, 2024
@grendello
grendelloforce-pushed the dev/grendel/android-crypto-public-symbols branch from aa4fac2 to 7e67d99CompareOctober 3, 2024 11:59
@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.

@bartonjs

Copy link
Copy Markdown
Member

@grendello I'm not sure what state this is in. I've been assuming since October that someone else from .NET Android was going to sign off... it's outside of my domain. But since it's in my area and I'm trying to get things tidied up... "do we want this, or should we close the PR?"

@grendello
grendelloforce-pushed the dev/grendel/android-crypto-public-symbols branch from 9521f00 to d8003f8CompareAugust 5, 2025 09:32
CopilotAI review requested due to automatic review settings August 5, 2025 09:32

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

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 refactors the Android cryptography native library to support dynamic linking in .NET for Android runtime by separating a Java JNI function into its own static library. The key motivation is to enable symbol hiding for all BCL native library exports while keeping the JNI function visible to the Java Virtual Machine.

  • Move JNI function to separate static library to control symbol visibility during dynamic linking
  • Refactor callback storage mechanism to support the new architecture
  • Update build configuration to include the new static library in all relevant targets

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
FileDescription
pal_trust_manager_jni_export.cNew file containing the JNI function and atomic callback storage
pal_trust_manager.hAdd function declaration for the new callback storage function
pal_trust_manager.cRemove JNI function and atomic storage, delegate to new storage function
CMakeLists.txtAdd new static library target for JNI exports with detailed comments
apphost/static/CMakeLists.txtInclude new static library in native libs list
Directory.Build.propsAdd new static library to platform manifest
Comments suppressed due to low confidence (1)

src/native/libs/System.Security.Cryptography.Native.Android/pal_trust_manager_jni_export.c:6

  • The function name 'StoreRemoteVerificationCallback' doesn't follow the established naming convention. Based on the existing code pattern, it should be prefixed with 'AndroidCryptoNative_' like other public functions in this module.
void StoreRemoteVerificationCallback (RemoteCertificateValidationCallback callback)

@grendello
grendelloforce-pushed the dev/grendel/android-crypto-public-symbols branch from d8003f8 to 436d952CompareAugust 5, 2025 09:35
@filipnavara

filipnavara commented Aug 6, 2025

Copy link
Copy Markdown
Member

I am not necessarily opposed to it but this feels like a pretty heavy solution. In case of NativeAOT it's possible to fix this with just including this in MSBuild item group:

<IlcArgInclude="--export-dynamic-symbol:Java_net_dot_android_crypto_DotnetProxyTrustManager_verifyRemoteCertificate" />

The build process generates an exports file which is then fed to the linker. I wonder if the CoreCLR/Android build process uses something similar because the linker script takes a precedence over --export-dynamic-symbol argument to clang/lld.

For reference, the NativeAOT exports file looks like this:

V1.0 {
global:
Java_net_dot_android_crypto_DotnetProxyTrustManager_verifyRemoteCertificate;
JNI_OnLoad;
JNI_OnUnload;
Java_net_dot_jni_nativeaot_JavaInteropRuntime_init;
local: *;
};

@grendello

Copy link
Copy Markdown
ContributorAuthor

@filipnavara I played with the option you mention, with varying success. The goal I have in mind is to hide symbols by default with few exceptions, and for static linking this is easier than playing with --export-dynamic-symbol for X symbols.

@grendello

Copy link
Copy Markdown
ContributorAuthor

When linking statically a NativeAOT app, does it really need to export the symbol though?

Yes, it does. It's a JNI interface and the JavaVM/Dalvik looks for it.

Yeah, you're right. I forgot it is actually called from Java.

@grendello

Copy link
Copy Markdown
ContributorAuthor

Eventually, I think the runtime pack should provide a manifest of libraries that are to be used for linking as well as the init functions to call (if any) and what symbols to export.

For now, I'd say responsibility to export this (and potentially others) symbol rests on the consumer (NativeAOT or Android in this case)

@grendello
grendelloforce-pushed the dev/grendel/android-crypto-public-symbols branch from cadc93e to 4dc1629CompareAugust 6, 2025 14:00
@jkotas

Copy link
Copy Markdown
Member

I'd say responsibility to export this (and potentially others) symbol rests on the consumer (NativeAOT or Android in this case)

I agree - the export should be next to logic that references the library. Add --export-dynamic-symbol:... next to the line that references the library in the NAOT buildintegration in this PR?

Comment threadsrc/native/corehost/apphost/static/CMakeLists.txt Outdated
Comment threadsrc/native/libs/System.Security.Cryptography.Native.Android/CMakeLists.txt Outdated
grendelloand others added 8 commits August 22, 2025 12:29
In dotnet/android#9006 we are working on linking
the .NET for Android runtime dynamically at the application build time.
Linking involves all the BCL native libraries, including
`System.Security.Cryptography.Native.Android` and one of the goals is to
hide all the exported symbols used as p/invokes by the managed BCL
libraries. This is because p/invoke calls are handled internally and,
with dynamic linking of the runtime, there is no longer any reason to
use `dlopen` and `dlsym` to look them up, they are all resolved
internally, at the link time.
Symbol hiding works fine thanks to the `--exclude-libs` `clang` flag,
which makes all the exported symbols in the indicated `.a` archives to
not be exported by the linker. However,
`System.Security.Cryptography.Native.Android` is special in the sense
that it contains one symbol which must not be hidden,
`Java_net_dot_android_crypto_DotnetProxyTrustManager_verifyRemoteCertificate`.
The above function is a Java `native` method implementation, and it
requires that not only its name follows the Java JNI naming rules, but
that is also available for the JVM to look up using `dlsym`.
I tried using the `--export-dynamic-symbol` clang flag to
export **just** this function, but it doesn't appear to work no matter
where I put the flag in relation to reference to the `.a` archives.
Instead, the problem can be dealt with by putting the JNI function in a
separate static library, so that I can link it without changing symbol
visibility, while making all the
`System.Security.Cryptography.Native.Android` symbols invisible.
…e.Unix.targets
Co-authored-by: Filip Navara <filip.navara@gmail.com>
…e.Unix.targets
Co-authored-by: Jan Kotas <jkotas@microsoft.com>
@grendello
grendelloforce-pushed the dev/grendel/android-crypto-public-symbols branch from a17572a to 90dbd76CompareAugust 22, 2025 10:29
@github-actions

github-actionsBot commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Workflow state for the Holistic Review Orchestrator.

{
"version": 5,
"last_dispatched_commit": "f573e20268e1d94196a6cf4eb2bc623de843e2f7",
"last_dispatched_base_ref": "main",
"last_dispatched_base_sha": "bd94b146570c5f3057f29e40de901ca5154b7a92",
"last_reviewed_commit": "f573e20268e1d94196a6cf4eb2bc623de843e2f7",
"last_reviewed_base_ref": "main",
"last_reviewed_base_sha": "bd94b146570c5f3057f29e40de901ca5154b7a92",
"last_recorded_worker_run_id": "29672719152",
"review_attempt_commit": "",
"review_attempt_base_ref": "",
"review_attempt_count": 0,
"max_review_attempts": 5,
"review_history_format": "holistic-review-disclosure-v1",
"review_history": [
{
"commit": "f573e20268e1d94196a6cf4eb2bc623de843e2f7",
"review_id": 4729976780
}
]
}

@github-actionsgithub-actionsBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Holistic Review

Motivation: For .NET for Android dynamic runtime linking (dotnet/android#9006), all BCL native libraries are linked into a single .so and their exported symbols are hidden via clang's --exclude-libs applied to the .a archives. System.Security.Cryptography.Native.Android contains one symbol that must stay exported: the JNI native method Java_net_dot_android_crypto_DotnetProxyTrustManager_verifyRemoteCertificate, which the JVM resolves via dlsym. Attempts to selectively re-export it with --export-dynamic-symbol did not work because a linker version script takes precedence.

Approach: The JNI entry point plus its backing static _Atomic callback state and a new StoreRemoteVerificationCallback accessor are moved into a new translation unit pal_trust_manager_jni_export.c, compiled into a dedicated static archive System.Security.Cryptography.Native.Android.JNIExport-Static (output name ...JNIExport). This lets the Android build exclude the main crypto archive from symbol hiding while keeping the single JNI symbol exported through the separate archive. The shared library still compiles the JNI source directly, so its behavior is unchanged. Supporting plumbing is added: the new archive is registered in the static apphost NATIVE_LIBS, in the shared-framework platform manifest, and (for NativeAOT) in Microsoft.NETCore.Native.Unix.targets alongside an --export-dynamic-symbolIlcArg. The refactor also adds the standard MIT license headers to pal_trust_manager.{c,h}.

Summary: This is a focused, correct build/packaging refactor. The atomic callback variable and its sole consumer (the JNI function) remain in the same translation unit, so the store/load pairing and abort_unless guard semantics are preserved; the register function in pal_trust_manager.c now delegates to StoreRemoteVerificationCallback in that same unit. Both the shared and static link paths are updated consistently, and the NativeAOT path is covered separately. Only minor style nits (a space before the parenthesis in StoreRemoteVerificationCallback (...) in the header and implementation) deviate from the surrounding code; these are non-blocking. Verdict: LGTM. Note: functional validation depends on Android CI build/link of the split archives, which I did not run.

Detailed Findings

  • Minor style: void StoreRemoteVerificationCallback (RemoteCertificateValidationCallback callback) (in pal_trust_manager.h and pal_trust_manager_jni_export.c) has a space before the opening parenthesis, inconsistent with the rest of the file. Non-blocking.

Note

This review was generated by this repository's Holistic Review agentic workflow to complement the built-in Copilot review.

Generated by Holistic Review · 51 AIC · ⌖ 10.5 AIC · ⊞ 10K

Sign up for freeto 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.

8 participants

@grendello@bartonjs@filipnavara@jkotas@simonrozsival@akoeplinger@teo-tsirpanis
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

[Android] Put Java JNI function in a separate static library - #108513

Open
grendello wants to merge 9 commits into
dotnet:mainfrom
grendello:dev/grendel/android-crypto-public-symbols
Open

[Android] Put Java JNI function in a separate static library#108513
grendello wants to merge 9 commits into
dotnet:mainfrom
grendello:dev/grendel/android-crypto-public-symbols

Conversation

@grendello

Copy link
Copy Markdown
Contributor

In dotnet/android#9006 we are working on linking the .NET for Android
runtime dynamically at the application build time.

Linking includes all the relevant BCL native libraries, and one of the goals is to
hide all the exported symbols used as p/invokes by the managed BCL
libraries. This is because p/invoke calls are handled internally and,
with dynamic linking of the runtime, there is no longer any reason to
use dlopen and dlsym to look them up, they are all resolved
internally, at the link time.

Symbol hiding works fine thanks to the --exclude-libsclang flag,
which makes all the exported symbols in the indicated .a archives to
not be exported by the linker. However, System.Security.Cryptography.Native.Android
is special in the sense that it contains one symbol which must not be hidden,
Java_net_dot_android_crypto_DotnetProxyTrustManager_verifyRemoteCertificate.

The above function is a Java native method implementation, and it
requires that not only its name follows the Java JNI naming rules, but
that it is also available for the JVM to look up using dlsym.

I tried using the --export-dynamic-symbol clang flag to
export just this function, but it doesn't appear to work no matter
where I put the flag in relation to reference to the .a archives.

Instead, the problem can be dealt with by putting the JNI function in a
separate static library, so that I can link it without changing symbol
visibility, while making all the System.Security.Cryptography.Native.Android
symbols invisible.

@dotnet-policy-servicedotnet-policy-serviceBot added the community-contribution Indicates that the PR has been added by a community member label Oct 3, 2024
@grendello
grendelloforce-pushed the dev/grendel/android-crypto-public-symbols branch from aa4fac2 to 7e67d99CompareOctober 3, 2024 11:59
@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.

@bartonjs

Copy link
Copy Markdown
Member

@grendello I'm not sure what state this is in. I've been assuming since October that someone else from .NET Android was going to sign off... it's outside of my domain. But since it's in my area and I'm trying to get things tidied up... "do we want this, or should we close the PR?"

@grendello
grendelloforce-pushed the dev/grendel/android-crypto-public-symbols branch from 9521f00 to d8003f8CompareAugust 5, 2025 09:32
CopilotAI review requested due to automatic review settings August 5, 2025 09:32

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

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 refactors the Android cryptography native library to support dynamic linking in .NET for Android runtime by separating a Java JNI function into its own static library. The key motivation is to enable symbol hiding for all BCL native library exports while keeping the JNI function visible to the Java Virtual Machine.

  • Move JNI function to separate static library to control symbol visibility during dynamic linking
  • Refactor callback storage mechanism to support the new architecture
  • Update build configuration to include the new static library in all relevant targets

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
FileDescription
pal_trust_manager_jni_export.cNew file containing the JNI function and atomic callback storage
pal_trust_manager.hAdd function declaration for the new callback storage function
pal_trust_manager.cRemove JNI function and atomic storage, delegate to new storage function
CMakeLists.txtAdd new static library target for JNI exports with detailed comments
apphost/static/CMakeLists.txtInclude new static library in native libs list
Directory.Build.propsAdd new static library to platform manifest
Comments suppressed due to low confidence (1)

src/native/libs/System.Security.Cryptography.Native.Android/pal_trust_manager_jni_export.c:6

  • The function name 'StoreRemoteVerificationCallback' doesn't follow the established naming convention. Based on the existing code pattern, it should be prefixed with 'AndroidCryptoNative_' like other public functions in this module.
void StoreRemoteVerificationCallback (RemoteCertificateValidationCallback callback)

@grendello
grendelloforce-pushed the dev/grendel/android-crypto-public-symbols branch from d8003f8 to 436d952CompareAugust 5, 2025 09:35
@filipnavara

filipnavara commented Aug 6, 2025

Copy link
Copy Markdown
Member

I am not necessarily opposed to it but this feels like a pretty heavy solution. In case of NativeAOT it's possible to fix this with just including this in MSBuild item group:

<IlcArgInclude="--export-dynamic-symbol:Java_net_dot_android_crypto_DotnetProxyTrustManager_verifyRemoteCertificate" />

The build process generates an exports file which is then fed to the linker. I wonder if the CoreCLR/Android build process uses something similar because the linker script takes a precedence over --export-dynamic-symbol argument to clang/lld.

For reference, the NativeAOT exports file looks like this:

V1.0 {
global:
Java_net_dot_android_crypto_DotnetProxyTrustManager_verifyRemoteCertificate;
JNI_OnLoad;
JNI_OnUnload;
Java_net_dot_jni_nativeaot_JavaInteropRuntime_init;
local: *;
};

@grendello

Copy link
Copy Markdown
ContributorAuthor

@filipnavara I played with the option you mention, with varying success. The goal I have in mind is to hide symbols by default with few exceptions, and for static linking this is easier than playing with --export-dynamic-symbol for X symbols.

@grendello

Copy link
Copy Markdown
ContributorAuthor

When linking statically a NativeAOT app, does it really need to export the symbol though?

Yes, it does. It's a JNI interface and the JavaVM/Dalvik looks for it.

Yeah, you're right. I forgot it is actually called from Java.

@grendello

Copy link
Copy Markdown
ContributorAuthor

Eventually, I think the runtime pack should provide a manifest of libraries that are to be used for linking as well as the init functions to call (if any) and what symbols to export.

For now, I'd say responsibility to export this (and potentially others) symbol rests on the consumer (NativeAOT or Android in this case)

@grendello
grendelloforce-pushed the dev/grendel/android-crypto-public-symbols branch from cadc93e to 4dc1629CompareAugust 6, 2025 14:00
@jkotas

Copy link
Copy Markdown
Member

I'd say responsibility to export this (and potentially others) symbol rests on the consumer (NativeAOT or Android in this case)

I agree - the export should be next to logic that references the library. Add --export-dynamic-symbol:... next to the line that references the library in the NAOT buildintegration in this PR?

Comment threadsrc/native/corehost/apphost/static/CMakeLists.txt Outdated
Comment threadsrc/native/libs/System.Security.Cryptography.Native.Android/CMakeLists.txt Outdated
grendelloand others added 8 commits August 22, 2025 12:29
In dotnet/android#9006 we are working on linking
the .NET for Android runtime dynamically at the application build time.
Linking involves all the BCL native libraries, including
`System.Security.Cryptography.Native.Android` and one of the goals is to
hide all the exported symbols used as p/invokes by the managed BCL
libraries. This is because p/invoke calls are handled internally and,
with dynamic linking of the runtime, there is no longer any reason to
use `dlopen` and `dlsym` to look them up, they are all resolved
internally, at the link time.
Symbol hiding works fine thanks to the `--exclude-libs` `clang` flag,
which makes all the exported symbols in the indicated `.a` archives to
not be exported by the linker. However,
`System.Security.Cryptography.Native.Android` is special in the sense
that it contains one symbol which must not be hidden,
`Java_net_dot_android_crypto_DotnetProxyTrustManager_verifyRemoteCertificate`.
The above function is a Java `native` method implementation, and it
requires that not only its name follows the Java JNI naming rules, but
that is also available for the JVM to look up using `dlsym`.
I tried using the `--export-dynamic-symbol` clang flag to
export **just** this function, but it doesn't appear to work no matter
where I put the flag in relation to reference to the `.a` archives.
Instead, the problem can be dealt with by putting the JNI function in a
separate static library, so that I can link it without changing symbol
visibility, while making all the
`System.Security.Cryptography.Native.Android` symbols invisible.
…e.Unix.targets
Co-authored-by: Filip Navara <filip.navara@gmail.com>
…e.Unix.targets
Co-authored-by: Jan Kotas <jkotas@microsoft.com>
@grendello
grendelloforce-pushed the dev/grendel/android-crypto-public-symbols branch from a17572a to 90dbd76CompareAugust 22, 2025 10:29
@github-actions

github-actionsBot commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Workflow state for the Holistic Review Orchestrator.

{
"version": 5,
"last_dispatched_commit": "f573e20268e1d94196a6cf4eb2bc623de843e2f7",
"last_dispatched_base_ref": "main",
"last_dispatched_base_sha": "bd94b146570c5f3057f29e40de901ca5154b7a92",
"last_reviewed_commit": "f573e20268e1d94196a6cf4eb2bc623de843e2f7",
"last_reviewed_base_ref": "main",
"last_reviewed_base_sha": "bd94b146570c5f3057f29e40de901ca5154b7a92",
"last_recorded_worker_run_id": "29672719152",
"review_attempt_commit": "",
"review_attempt_base_ref": "",
"review_attempt_count": 0,
"max_review_attempts": 5,
"review_history_format": "holistic-review-disclosure-v1",
"review_history": [
{
"commit": "f573e20268e1d94196a6cf4eb2bc623de843e2f7",
"review_id": 4729976780
}
]
}

@github-actionsgithub-actionsBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Holistic Review

Motivation: For .NET for Android dynamic runtime linking (dotnet/android#9006), all BCL native libraries are linked into a single .so and their exported symbols are hidden via clang's --exclude-libs applied to the .a archives. System.Security.Cryptography.Native.Android contains one symbol that must stay exported: the JNI native method Java_net_dot_android_crypto_DotnetProxyTrustManager_verifyRemoteCertificate, which the JVM resolves via dlsym. Attempts to selectively re-export it with --export-dynamic-symbol did not work because a linker version script takes precedence.

Approach: The JNI entry point plus its backing static _Atomic callback state and a new StoreRemoteVerificationCallback accessor are moved into a new translation unit pal_trust_manager_jni_export.c, compiled into a dedicated static archive System.Security.Cryptography.Native.Android.JNIExport-Static (output name ...JNIExport). This lets the Android build exclude the main crypto archive from symbol hiding while keeping the single JNI symbol exported through the separate archive. The shared library still compiles the JNI source directly, so its behavior is unchanged. Supporting plumbing is added: the new archive is registered in the static apphost NATIVE_LIBS, in the shared-framework platform manifest, and (for NativeAOT) in Microsoft.NETCore.Native.Unix.targets alongside an --export-dynamic-symbolIlcArg. The refactor also adds the standard MIT license headers to pal_trust_manager.{c,h}.

Summary: This is a focused, correct build/packaging refactor. The atomic callback variable and its sole consumer (the JNI function) remain in the same translation unit, so the store/load pairing and abort_unless guard semantics are preserved; the register function in pal_trust_manager.c now delegates to StoreRemoteVerificationCallback in that same unit. Both the shared and static link paths are updated consistently, and the NativeAOT path is covered separately. Only minor style nits (a space before the parenthesis in StoreRemoteVerificationCallback (...) in the header and implementation) deviate from the surrounding code; these are non-blocking. Verdict: LGTM. Note: functional validation depends on Android CI build/link of the split archives, which I did not run.

Detailed Findings

  • Minor style: void StoreRemoteVerificationCallback (RemoteCertificateValidationCallback callback) (in pal_trust_manager.h and pal_trust_manager_jni_export.c) has a space before the opening parenthesis, inconsistent with the rest of the file. Non-blocking.

Note

This review was generated by this repository's Holistic Review agentic workflow to complement the built-in Copilot review.

Generated by Holistic Review · 51 AIC · ⌖ 10.5 AIC · ⊞ 10K

Sign up for freeto 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.

8 participants

@grendello@bartonjs@filipnavara@jkotas@simonrozsival@akoeplinger@teo-tsirpanis
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

[Android] Put Java JNI function in a separate static library - #108513

Open
grendello wants to merge 9 commits into
dotnet:mainfrom
grendello:dev/grendel/android-crypto-public-symbols
Open

[Android] Put Java JNI function in a separate static library#108513
grendello wants to merge 9 commits into
dotnet:mainfrom
grendello:dev/grendel/android-crypto-public-symbols

Conversation

@grendello

Copy link
Copy Markdown
Contributor

In dotnet/android#9006 we are working on linking the .NET for Android
runtime dynamically at the application build time.

Linking includes all the relevant BCL native libraries, and one of the goals is to
hide all the exported symbols used as p/invokes by the managed BCL
libraries. This is because p/invoke calls are handled internally and,
with dynamic linking of the runtime, there is no longer any reason to
use dlopen and dlsym to look them up, they are all resolved
internally, at the link time.

Symbol hiding works fine thanks to the --exclude-libsclang flag,
which makes all the exported symbols in the indicated .a archives to
not be exported by the linker. However, System.Security.Cryptography.Native.Android
is special in the sense that it contains one symbol which must not be hidden,
Java_net_dot_android_crypto_DotnetProxyTrustManager_verifyRemoteCertificate.

The above function is a Java native method implementation, and it
requires that not only its name follows the Java JNI naming rules, but
that it is also available for the JVM to look up using dlsym.

I tried using the --export-dynamic-symbol clang flag to
export just this function, but it doesn't appear to work no matter
where I put the flag in relation to reference to the .a archives.

Instead, the problem can be dealt with by putting the JNI function in a
separate static library, so that I can link it without changing symbol
visibility, while making all the System.Security.Cryptography.Native.Android
symbols invisible.

@dotnet-policy-servicedotnet-policy-serviceBot added the community-contribution Indicates that the PR has been added by a community member label Oct 3, 2024
@grendello
grendelloforce-pushed the dev/grendel/android-crypto-public-symbols branch from aa4fac2 to 7e67d99CompareOctober 3, 2024 11:59
@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.

@bartonjs

Copy link
Copy Markdown
Member

@grendello I'm not sure what state this is in. I've been assuming since October that someone else from .NET Android was going to sign off... it's outside of my domain. But since it's in my area and I'm trying to get things tidied up... "do we want this, or should we close the PR?"

@grendello
grendelloforce-pushed the dev/grendel/android-crypto-public-symbols branch from 9521f00 to d8003f8CompareAugust 5, 2025 09:32
CopilotAI review requested due to automatic review settings August 5, 2025 09:32

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

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 refactors the Android cryptography native library to support dynamic linking in .NET for Android runtime by separating a Java JNI function into its own static library. The key motivation is to enable symbol hiding for all BCL native library exports while keeping the JNI function visible to the Java Virtual Machine.

  • Move JNI function to separate static library to control symbol visibility during dynamic linking
  • Refactor callback storage mechanism to support the new architecture
  • Update build configuration to include the new static library in all relevant targets

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
FileDescription
pal_trust_manager_jni_export.cNew file containing the JNI function and atomic callback storage
pal_trust_manager.hAdd function declaration for the new callback storage function
pal_trust_manager.cRemove JNI function and atomic storage, delegate to new storage function
CMakeLists.txtAdd new static library target for JNI exports with detailed comments
apphost/static/CMakeLists.txtInclude new static library in native libs list
Directory.Build.propsAdd new static library to platform manifest
Comments suppressed due to low confidence (1)

src/native/libs/System.Security.Cryptography.Native.Android/pal_trust_manager_jni_export.c:6

  • The function name 'StoreRemoteVerificationCallback' doesn't follow the established naming convention. Based on the existing code pattern, it should be prefixed with 'AndroidCryptoNative_' like other public functions in this module.
void StoreRemoteVerificationCallback (RemoteCertificateValidationCallback callback)

@grendello
grendelloforce-pushed the dev/grendel/android-crypto-public-symbols branch from d8003f8 to 436d952CompareAugust 5, 2025 09:35
@filipnavara

filipnavara commented Aug 6, 2025

Copy link
Copy Markdown
Member

I am not necessarily opposed to it but this feels like a pretty heavy solution. In case of NativeAOT it's possible to fix this with just including this in MSBuild item group:

<IlcArgInclude="--export-dynamic-symbol:Java_net_dot_android_crypto_DotnetProxyTrustManager_verifyRemoteCertificate" />

The build process generates an exports file which is then fed to the linker. I wonder if the CoreCLR/Android build process uses something similar because the linker script takes a precedence over --export-dynamic-symbol argument to clang/lld.

For reference, the NativeAOT exports file looks like this:

V1.0 {
global:
Java_net_dot_android_crypto_DotnetProxyTrustManager_verifyRemoteCertificate;
JNI_OnLoad;
JNI_OnUnload;
Java_net_dot_jni_nativeaot_JavaInteropRuntime_init;
local: *;
};

@grendello

Copy link
Copy Markdown
ContributorAuthor

@filipnavara I played with the option you mention, with varying success. The goal I have in mind is to hide symbols by default with few exceptions, and for static linking this is easier than playing with --export-dynamic-symbol for X symbols.

@grendello

Copy link
Copy Markdown
ContributorAuthor

When linking statically a NativeAOT app, does it really need to export the symbol though?

Yes, it does. It's a JNI interface and the JavaVM/Dalvik looks for it.

Yeah, you're right. I forgot it is actually called from Java.

@grendello

Copy link
Copy Markdown
ContributorAuthor

Eventually, I think the runtime pack should provide a manifest of libraries that are to be used for linking as well as the init functions to call (if any) and what symbols to export.

For now, I'd say responsibility to export this (and potentially others) symbol rests on the consumer (NativeAOT or Android in this case)

@grendello
grendelloforce-pushed the dev/grendel/android-crypto-public-symbols branch from cadc93e to 4dc1629CompareAugust 6, 2025 14:00
@jkotas

Copy link
Copy Markdown
Member

I'd say responsibility to export this (and potentially others) symbol rests on the consumer (NativeAOT or Android in this case)

I agree - the export should be next to logic that references the library. Add --export-dynamic-symbol:... next to the line that references the library in the NAOT buildintegration in this PR?

Comment threadsrc/native/corehost/apphost/static/CMakeLists.txt Outdated
Comment threadsrc/native/libs/System.Security.Cryptography.Native.Android/CMakeLists.txt Outdated
grendelloand others added 8 commits August 22, 2025 12:29
In dotnet/android#9006 we are working on linking
the .NET for Android runtime dynamically at the application build time.
Linking involves all the BCL native libraries, including
`System.Security.Cryptography.Native.Android` and one of the goals is to
hide all the exported symbols used as p/invokes by the managed BCL
libraries. This is because p/invoke calls are handled internally and,
with dynamic linking of the runtime, there is no longer any reason to
use `dlopen` and `dlsym` to look them up, they are all resolved
internally, at the link time.
Symbol hiding works fine thanks to the `--exclude-libs` `clang` flag,
which makes all the exported symbols in the indicated `.a` archives to
not be exported by the linker. However,
`System.Security.Cryptography.Native.Android` is special in the sense
that it contains one symbol which must not be hidden,
`Java_net_dot_android_crypto_DotnetProxyTrustManager_verifyRemoteCertificate`.
The above function is a Java `native` method implementation, and it
requires that not only its name follows the Java JNI naming rules, but
that is also available for the JVM to look up using `dlsym`.
I tried using the `--export-dynamic-symbol` clang flag to
export **just** this function, but it doesn't appear to work no matter
where I put the flag in relation to reference to the `.a` archives.
Instead, the problem can be dealt with by putting the JNI function in a
separate static library, so that I can link it without changing symbol
visibility, while making all the
`System.Security.Cryptography.Native.Android` symbols invisible.
…e.Unix.targets
Co-authored-by: Filip Navara <filip.navara@gmail.com>
…e.Unix.targets
Co-authored-by: Jan Kotas <jkotas@microsoft.com>
@grendello
grendelloforce-pushed the dev/grendel/android-crypto-public-symbols branch from a17572a to 90dbd76CompareAugust 22, 2025 10:29
@github-actions

github-actionsBot commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Workflow state for the Holistic Review Orchestrator.

{
"version": 5,
"last_dispatched_commit": "f573e20268e1d94196a6cf4eb2bc623de843e2f7",
"last_dispatched_base_ref": "main",
"last_dispatched_base_sha": "bd94b146570c5f3057f29e40de901ca5154b7a92",
"last_reviewed_commit": "f573e20268e1d94196a6cf4eb2bc623de843e2f7",
"last_reviewed_base_ref": "main",
"last_reviewed_base_sha": "bd94b146570c5f3057f29e40de901ca5154b7a92",
"last_recorded_worker_run_id": "29672719152",
"review_attempt_commit": "",
"review_attempt_base_ref": "",
"review_attempt_count": 0,
"max_review_attempts": 5,
"review_history_format": "holistic-review-disclosure-v1",
"review_history": [
{
"commit": "f573e20268e1d94196a6cf4eb2bc623de843e2f7",
"review_id": 4729976780
}
]
}

@github-actionsgithub-actionsBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Holistic Review

Motivation: For .NET for Android dynamic runtime linking (dotnet/android#9006), all BCL native libraries are linked into a single .so and their exported symbols are hidden via clang's --exclude-libs applied to the .a archives. System.Security.Cryptography.Native.Android contains one symbol that must stay exported: the JNI native method Java_net_dot_android_crypto_DotnetProxyTrustManager_verifyRemoteCertificate, which the JVM resolves via dlsym. Attempts to selectively re-export it with --export-dynamic-symbol did not work because a linker version script takes precedence.

Approach: The JNI entry point plus its backing static _Atomic callback state and a new StoreRemoteVerificationCallback accessor are moved into a new translation unit pal_trust_manager_jni_export.c, compiled into a dedicated static archive System.Security.Cryptography.Native.Android.JNIExport-Static (output name ...JNIExport). This lets the Android build exclude the main crypto archive from symbol hiding while keeping the single JNI symbol exported through the separate archive. The shared library still compiles the JNI source directly, so its behavior is unchanged. Supporting plumbing is added: the new archive is registered in the static apphost NATIVE_LIBS, in the shared-framework platform manifest, and (for NativeAOT) in Microsoft.NETCore.Native.Unix.targets alongside an --export-dynamic-symbolIlcArg. The refactor also adds the standard MIT license headers to pal_trust_manager.{c,h}.

Summary: This is a focused, correct build/packaging refactor. The atomic callback variable and its sole consumer (the JNI function) remain in the same translation unit, so the store/load pairing and abort_unless guard semantics are preserved; the register function in pal_trust_manager.c now delegates to StoreRemoteVerificationCallback in that same unit. Both the shared and static link paths are updated consistently, and the NativeAOT path is covered separately. Only minor style nits (a space before the parenthesis in StoreRemoteVerificationCallback (...) in the header and implementation) deviate from the surrounding code; these are non-blocking. Verdict: LGTM. Note: functional validation depends on Android CI build/link of the split archives, which I did not run.

Detailed Findings

  • Minor style: void StoreRemoteVerificationCallback (RemoteCertificateValidationCallback callback) (in pal_trust_manager.h and pal_trust_manager_jni_export.c) has a space before the opening parenthesis, inconsistent with the rest of the file. Non-blocking.

Note

This review was generated by this repository's Holistic Review agentic workflow to complement the built-in Copilot review.

Generated by Holistic Review · 51 AIC · ⌖ 10.5 AIC · ⊞ 10K

Sign up for freeto 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.

8 participants

@grendello@bartonjs@filipnavara@jkotas@simonrozsival@akoeplinger@teo-tsirpanis
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

[Android] Put Java JNI function in a separate static library - #108513

Open
grendello wants to merge 9 commits into
dotnet:mainfrom
grendello:dev/grendel/android-crypto-public-symbols
Open

[Android] Put Java JNI function in a separate static library#108513
grendello wants to merge 9 commits into
dotnet:mainfrom
grendello:dev/grendel/android-crypto-public-symbols

Conversation

@grendello

Copy link
Copy Markdown
Contributor

In dotnet/android#9006 we are working on linking the .NET for Android
runtime dynamically at the application build time.

Linking includes all the relevant BCL native libraries, and one of the goals is to
hide all the exported symbols used as p/invokes by the managed BCL
libraries. This is because p/invoke calls are handled internally and,
with dynamic linking of the runtime, there is no longer any reason to
use dlopen and dlsym to look them up, they are all resolved
internally, at the link time.

Symbol hiding works fine thanks to the --exclude-libsclang flag,
which makes all the exported symbols in the indicated .a archives to
not be exported by the linker. However, System.Security.Cryptography.Native.Android
is special in the sense that it contains one symbol which must not be hidden,
Java_net_dot_android_crypto_DotnetProxyTrustManager_verifyRemoteCertificate.

The above function is a Java native method implementation, and it
requires that not only its name follows the Java JNI naming rules, but
that it is also available for the JVM to look up using dlsym.

I tried using the --export-dynamic-symbol clang flag to
export just this function, but it doesn't appear to work no matter
where I put the flag in relation to reference to the .a archives.

Instead, the problem can be dealt with by putting the JNI function in a
separate static library, so that I can link it without changing symbol
visibility, while making all the System.Security.Cryptography.Native.Android
symbols invisible.

@dotnet-policy-servicedotnet-policy-serviceBot added the community-contribution Indicates that the PR has been added by a community member label Oct 3, 2024
@grendello
grendelloforce-pushed the dev/grendel/android-crypto-public-symbols branch from aa4fac2 to 7e67d99CompareOctober 3, 2024 11:59
@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.

@bartonjs

Copy link
Copy Markdown
Member

@grendello I'm not sure what state this is in. I've been assuming since October that someone else from .NET Android was going to sign off... it's outside of my domain. But since it's in my area and I'm trying to get things tidied up... "do we want this, or should we close the PR?"

@grendello
grendelloforce-pushed the dev/grendel/android-crypto-public-symbols branch from 9521f00 to d8003f8CompareAugust 5, 2025 09:32
CopilotAI review requested due to automatic review settings August 5, 2025 09:32

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

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 refactors the Android cryptography native library to support dynamic linking in .NET for Android runtime by separating a Java JNI function into its own static library. The key motivation is to enable symbol hiding for all BCL native library exports while keeping the JNI function visible to the Java Virtual Machine.

  • Move JNI function to separate static library to control symbol visibility during dynamic linking
  • Refactor callback storage mechanism to support the new architecture
  • Update build configuration to include the new static library in all relevant targets

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
FileDescription
pal_trust_manager_jni_export.cNew file containing the JNI function and atomic callback storage
pal_trust_manager.hAdd function declaration for the new callback storage function
pal_trust_manager.cRemove JNI function and atomic storage, delegate to new storage function
CMakeLists.txtAdd new static library target for JNI exports with detailed comments
apphost/static/CMakeLists.txtInclude new static library in native libs list
Directory.Build.propsAdd new static library to platform manifest
Comments suppressed due to low confidence (1)

src/native/libs/System.Security.Cryptography.Native.Android/pal_trust_manager_jni_export.c:6

  • The function name 'StoreRemoteVerificationCallback' doesn't follow the established naming convention. Based on the existing code pattern, it should be prefixed with 'AndroidCryptoNative_' like other public functions in this module.
void StoreRemoteVerificationCallback (RemoteCertificateValidationCallback callback)

@grendello
grendelloforce-pushed the dev/grendel/android-crypto-public-symbols branch from d8003f8 to 436d952CompareAugust 5, 2025 09:35
@filipnavara

filipnavara commented Aug 6, 2025

Copy link
Copy Markdown
Member

I am not necessarily opposed to it but this feels like a pretty heavy solution. In case of NativeAOT it's possible to fix this with just including this in MSBuild item group:

<IlcArgInclude="--export-dynamic-symbol:Java_net_dot_android_crypto_DotnetProxyTrustManager_verifyRemoteCertificate" />

The build process generates an exports file which is then fed to the linker. I wonder if the CoreCLR/Android build process uses something similar because the linker script takes a precedence over --export-dynamic-symbol argument to clang/lld.

For reference, the NativeAOT exports file looks like this:

V1.0 {
global:
Java_net_dot_android_crypto_DotnetProxyTrustManager_verifyRemoteCertificate;
JNI_OnLoad;
JNI_OnUnload;
Java_net_dot_jni_nativeaot_JavaInteropRuntime_init;
local: *;
};

@grendello

Copy link
Copy Markdown
ContributorAuthor

@filipnavara I played with the option you mention, with varying success. The goal I have in mind is to hide symbols by default with few exceptions, and for static linking this is easier than playing with --export-dynamic-symbol for X symbols.

@grendello

Copy link
Copy Markdown
ContributorAuthor

When linking statically a NativeAOT app, does it really need to export the symbol though?

Yes, it does. It's a JNI interface and the JavaVM/Dalvik looks for it.

Yeah, you're right. I forgot it is actually called from Java.

@grendello

Copy link
Copy Markdown
ContributorAuthor

Eventually, I think the runtime pack should provide a manifest of libraries that are to be used for linking as well as the init functions to call (if any) and what symbols to export.

For now, I'd say responsibility to export this (and potentially others) symbol rests on the consumer (NativeAOT or Android in this case)

@grendello
grendelloforce-pushed the dev/grendel/android-crypto-public-symbols branch from cadc93e to 4dc1629CompareAugust 6, 2025 14:00
@jkotas

Copy link
Copy Markdown
Member

I'd say responsibility to export this (and potentially others) symbol rests on the consumer (NativeAOT or Android in this case)

I agree - the export should be next to logic that references the library. Add --export-dynamic-symbol:... next to the line that references the library in the NAOT buildintegration in this PR?

Comment threadsrc/native/corehost/apphost/static/CMakeLists.txt Outdated
Comment threadsrc/native/libs/System.Security.Cryptography.Native.Android/CMakeLists.txt Outdated
grendelloand others added 8 commits August 22, 2025 12:29
In dotnet/android#9006 we are working on linking
the .NET for Android runtime dynamically at the application build time.
Linking involves all the BCL native libraries, including
`System.Security.Cryptography.Native.Android` and one of the goals is to
hide all the exported symbols used as p/invokes by the managed BCL
libraries. This is because p/invoke calls are handled internally and,
with dynamic linking of the runtime, there is no longer any reason to
use `dlopen` and `dlsym` to look them up, they are all resolved
internally, at the link time.
Symbol hiding works fine thanks to the `--exclude-libs` `clang` flag,
which makes all the exported symbols in the indicated `.a` archives to
not be exported by the linker. However,
`System.Security.Cryptography.Native.Android` is special in the sense
that it contains one symbol which must not be hidden,
`Java_net_dot_android_crypto_DotnetProxyTrustManager_verifyRemoteCertificate`.
The above function is a Java `native` method implementation, and it
requires that not only its name follows the Java JNI naming rules, but
that is also available for the JVM to look up using `dlsym`.
I tried using the `--export-dynamic-symbol` clang flag to
export **just** this function, but it doesn't appear to work no matter
where I put the flag in relation to reference to the `.a` archives.
Instead, the problem can be dealt with by putting the JNI function in a
separate static library, so that I can link it without changing symbol
visibility, while making all the
`System.Security.Cryptography.Native.Android` symbols invisible.
…e.Unix.targets
Co-authored-by: Filip Navara <filip.navara@gmail.com>
…e.Unix.targets
Co-authored-by: Jan Kotas <jkotas@microsoft.com>
@grendello
grendelloforce-pushed the dev/grendel/android-crypto-public-symbols branch from a17572a to 90dbd76CompareAugust 22, 2025 10:29
@github-actions

github-actionsBot commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Workflow state for the Holistic Review Orchestrator.

{
"version": 5,
"last_dispatched_commit": "f573e20268e1d94196a6cf4eb2bc623de843e2f7",
"last_dispatched_base_ref": "main",
"last_dispatched_base_sha": "bd94b146570c5f3057f29e40de901ca5154b7a92",
"last_reviewed_commit": "f573e20268e1d94196a6cf4eb2bc623de843e2f7",
"last_reviewed_base_ref": "main",
"last_reviewed_base_sha": "bd94b146570c5f3057f29e40de901ca5154b7a92",
"last_recorded_worker_run_id": "29672719152",
"review_attempt_commit": "",
"review_attempt_base_ref": "",
"review_attempt_count": 0,
"max_review_attempts": 5,
"review_history_format": "holistic-review-disclosure-v1",
"review_history": [
{
"commit": "f573e20268e1d94196a6cf4eb2bc623de843e2f7",
"review_id": 4729976780
}
]
}

@github-actionsgithub-actionsBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Holistic Review

Motivation: For .NET for Android dynamic runtime linking (dotnet/android#9006), all BCL native libraries are linked into a single .so and their exported symbols are hidden via clang's --exclude-libs applied to the .a archives. System.Security.Cryptography.Native.Android contains one symbol that must stay exported: the JNI native method Java_net_dot_android_crypto_DotnetProxyTrustManager_verifyRemoteCertificate, which the JVM resolves via dlsym. Attempts to selectively re-export it with --export-dynamic-symbol did not work because a linker version script takes precedence.

Approach: The JNI entry point plus its backing static _Atomic callback state and a new StoreRemoteVerificationCallback accessor are moved into a new translation unit pal_trust_manager_jni_export.c, compiled into a dedicated static archive System.Security.Cryptography.Native.Android.JNIExport-Static (output name ...JNIExport). This lets the Android build exclude the main crypto archive from symbol hiding while keeping the single JNI symbol exported through the separate archive. The shared library still compiles the JNI source directly, so its behavior is unchanged. Supporting plumbing is added: the new archive is registered in the static apphost NATIVE_LIBS, in the shared-framework platform manifest, and (for NativeAOT) in Microsoft.NETCore.Native.Unix.targets alongside an --export-dynamic-symbolIlcArg. The refactor also adds the standard MIT license headers to pal_trust_manager.{c,h}.

Summary: This is a focused, correct build/packaging refactor. The atomic callback variable and its sole consumer (the JNI function) remain in the same translation unit, so the store/load pairing and abort_unless guard semantics are preserved; the register function in pal_trust_manager.c now delegates to StoreRemoteVerificationCallback in that same unit. Both the shared and static link paths are updated consistently, and the NativeAOT path is covered separately. Only minor style nits (a space before the parenthesis in StoreRemoteVerificationCallback (...) in the header and implementation) deviate from the surrounding code; these are non-blocking. Verdict: LGTM. Note: functional validation depends on Android CI build/link of the split archives, which I did not run.

Detailed Findings

  • Minor style: void StoreRemoteVerificationCallback (RemoteCertificateValidationCallback callback) (in pal_trust_manager.h and pal_trust_manager_jni_export.c) has a space before the opening parenthesis, inconsistent with the rest of the file. Non-blocking.

Note

This review was generated by this repository's Holistic Review agentic workflow to complement the built-in Copilot review.

Generated by Holistic Review · 51 AIC · ⌖ 10.5 AIC · ⊞ 10K

Sign up for freeto 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.

8 participants

@grendello@bartonjs@filipnavara@jkotas@simonrozsival@akoeplinger@teo-tsirpanis
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

[Android] Put Java JNI function in a separate static library - #108513

Open
grendello wants to merge 9 commits into
dotnet:mainfrom
grendello:dev/grendel/android-crypto-public-symbols
Open

[Android] Put Java JNI function in a separate static library#108513
grendello wants to merge 9 commits into
dotnet:mainfrom
grendello:dev/grendel/android-crypto-public-symbols

Conversation

@grendello

Copy link
Copy Markdown
Contributor

In dotnet/android#9006 we are working on linking the .NET for Android
runtime dynamically at the application build time.

Linking includes all the relevant BCL native libraries, and one of the goals is to
hide all the exported symbols used as p/invokes by the managed BCL
libraries. This is because p/invoke calls are handled internally and,
with dynamic linking of the runtime, there is no longer any reason to
use dlopen and dlsym to look them up, they are all resolved
internally, at the link time.

Symbol hiding works fine thanks to the --exclude-libsclang flag,
which makes all the exported symbols in the indicated .a archives to
not be exported by the linker. However, System.Security.Cryptography.Native.Android
is special in the sense that it contains one symbol which must not be hidden,
Java_net_dot_android_crypto_DotnetProxyTrustManager_verifyRemoteCertificate.

The above function is a Java native method implementation, and it
requires that not only its name follows the Java JNI naming rules, but
that it is also available for the JVM to look up using dlsym.

I tried using the --export-dynamic-symbol clang flag to
export just this function, but it doesn't appear to work no matter
where I put the flag in relation to reference to the .a archives.

Instead, the problem can be dealt with by putting the JNI function in a
separate static library, so that I can link it without changing symbol
visibility, while making all the System.Security.Cryptography.Native.Android
symbols invisible.

@dotnet-policy-servicedotnet-policy-serviceBot added the community-contribution Indicates that the PR has been added by a community member label Oct 3, 2024
@grendello
grendelloforce-pushed the dev/grendel/android-crypto-public-symbols branch from aa4fac2 to 7e67d99CompareOctober 3, 2024 11:59
@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.

@bartonjs

Copy link
Copy Markdown
Member

@grendello I'm not sure what state this is in. I've been assuming since October that someone else from .NET Android was going to sign off... it's outside of my domain. But since it's in my area and I'm trying to get things tidied up... "do we want this, or should we close the PR?"

@grendello
grendelloforce-pushed the dev/grendel/android-crypto-public-symbols branch from 9521f00 to d8003f8CompareAugust 5, 2025 09:32
CopilotAI review requested due to automatic review settings August 5, 2025 09:32

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

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 refactors the Android cryptography native library to support dynamic linking in .NET for Android runtime by separating a Java JNI function into its own static library. The key motivation is to enable symbol hiding for all BCL native library exports while keeping the JNI function visible to the Java Virtual Machine.

  • Move JNI function to separate static library to control symbol visibility during dynamic linking
  • Refactor callback storage mechanism to support the new architecture
  • Update build configuration to include the new static library in all relevant targets

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
FileDescription
pal_trust_manager_jni_export.cNew file containing the JNI function and atomic callback storage
pal_trust_manager.hAdd function declaration for the new callback storage function
pal_trust_manager.cRemove JNI function and atomic storage, delegate to new storage function
CMakeLists.txtAdd new static library target for JNI exports with detailed comments
apphost/static/CMakeLists.txtInclude new static library in native libs list
Directory.Build.propsAdd new static library to platform manifest
Comments suppressed due to low confidence (1)

src/native/libs/System.Security.Cryptography.Native.Android/pal_trust_manager_jni_export.c:6

  • The function name 'StoreRemoteVerificationCallback' doesn't follow the established naming convention. Based on the existing code pattern, it should be prefixed with 'AndroidCryptoNative_' like other public functions in this module.
void StoreRemoteVerificationCallback (RemoteCertificateValidationCallback callback)

@grendello
grendelloforce-pushed the dev/grendel/android-crypto-public-symbols branch from d8003f8 to 436d952CompareAugust 5, 2025 09:35
@filipnavara

filipnavara commented Aug 6, 2025

Copy link
Copy Markdown
Member

I am not necessarily opposed to it but this feels like a pretty heavy solution. In case of NativeAOT it's possible to fix this with just including this in MSBuild item group:

<IlcArgInclude="--export-dynamic-symbol:Java_net_dot_android_crypto_DotnetProxyTrustManager_verifyRemoteCertificate" />

The build process generates an exports file which is then fed to the linker. I wonder if the CoreCLR/Android build process uses something similar because the linker script takes a precedence over --export-dynamic-symbol argument to clang/lld.

For reference, the NativeAOT exports file looks like this:

V1.0 {
global:
Java_net_dot_android_crypto_DotnetProxyTrustManager_verifyRemoteCertificate;
JNI_OnLoad;
JNI_OnUnload;
Java_net_dot_jni_nativeaot_JavaInteropRuntime_init;
local: *;
};

@grendello

Copy link
Copy Markdown
ContributorAuthor

@filipnavara I played with the option you mention, with varying success. The goal I have in mind is to hide symbols by default with few exceptions, and for static linking this is easier than playing with --export-dynamic-symbol for X symbols.

@grendello

Copy link
Copy Markdown
ContributorAuthor

When linking statically a NativeAOT app, does it really need to export the symbol though?

Yes, it does. It's a JNI interface and the JavaVM/Dalvik looks for it.

Yeah, you're right. I forgot it is actually called from Java.

@grendello

Copy link
Copy Markdown
ContributorAuthor

Eventually, I think the runtime pack should provide a manifest of libraries that are to be used for linking as well as the init functions to call (if any) and what symbols to export.

For now, I'd say responsibility to export this (and potentially others) symbol rests on the consumer (NativeAOT or Android in this case)

@grendello
grendelloforce-pushed the dev/grendel/android-crypto-public-symbols branch from cadc93e to 4dc1629CompareAugust 6, 2025 14:00
@jkotas

Copy link
Copy Markdown
Member

I'd say responsibility to export this (and potentially others) symbol rests on the consumer (NativeAOT or Android in this case)

I agree - the export should be next to logic that references the library. Add --export-dynamic-symbol:... next to the line that references the library in the NAOT buildintegration in this PR?

Comment threadsrc/native/corehost/apphost/static/CMakeLists.txt Outdated
Comment threadsrc/native/libs/System.Security.Cryptography.Native.Android/CMakeLists.txt Outdated
grendelloand others added 8 commits August 22, 2025 12:29
In dotnet/android#9006 we are working on linking
the .NET for Android runtime dynamically at the application build time.
Linking involves all the BCL native libraries, including
`System.Security.Cryptography.Native.Android` and one of the goals is to
hide all the exported symbols used as p/invokes by the managed BCL
libraries. This is because p/invoke calls are handled internally and,
with dynamic linking of the runtime, there is no longer any reason to
use `dlopen` and `dlsym` to look them up, they are all resolved
internally, at the link time.
Symbol hiding works fine thanks to the `--exclude-libs` `clang` flag,
which makes all the exported symbols in the indicated `.a` archives to
not be exported by the linker. However,
`System.Security.Cryptography.Native.Android` is special in the sense
that it contains one symbol which must not be hidden,
`Java_net_dot_android_crypto_DotnetProxyTrustManager_verifyRemoteCertificate`.
The above function is a Java `native` method implementation, and it
requires that not only its name follows the Java JNI naming rules, but
that is also available for the JVM to look up using `dlsym`.
I tried using the `--export-dynamic-symbol` clang flag to
export **just** this function, but it doesn't appear to work no matter
where I put the flag in relation to reference to the `.a` archives.
Instead, the problem can be dealt with by putting the JNI function in a
separate static library, so that I can link it without changing symbol
visibility, while making all the
`System.Security.Cryptography.Native.Android` symbols invisible.
…e.Unix.targets
Co-authored-by: Filip Navara <filip.navara@gmail.com>
…e.Unix.targets
Co-authored-by: Jan Kotas <jkotas@microsoft.com>
@grendello
grendelloforce-pushed the dev/grendel/android-crypto-public-symbols branch from a17572a to 90dbd76CompareAugust 22, 2025 10:29
@github-actions

github-actionsBot commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Workflow state for the Holistic Review Orchestrator.

{
"version": 5,
"last_dispatched_commit": "f573e20268e1d94196a6cf4eb2bc623de843e2f7",
"last_dispatched_base_ref": "main",
"last_dispatched_base_sha": "bd94b146570c5f3057f29e40de901ca5154b7a92",
"last_reviewed_commit": "f573e20268e1d94196a6cf4eb2bc623de843e2f7",
"last_reviewed_base_ref": "main",
"last_reviewed_base_sha": "bd94b146570c5f3057f29e40de901ca5154b7a92",
"last_recorded_worker_run_id": "29672719152",
"review_attempt_commit": "",
"review_attempt_base_ref": "",
"review_attempt_count": 0,
"max_review_attempts": 5,
"review_history_format": "holistic-review-disclosure-v1",
"review_history": [
{
"commit": "f573e20268e1d94196a6cf4eb2bc623de843e2f7",
"review_id": 4729976780
}
]
}

@github-actionsgithub-actionsBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Holistic Review

Motivation: For .NET for Android dynamic runtime linking (dotnet/android#9006), all BCL native libraries are linked into a single .so and their exported symbols are hidden via clang's --exclude-libs applied to the .a archives. System.Security.Cryptography.Native.Android contains one symbol that must stay exported: the JNI native method Java_net_dot_android_crypto_DotnetProxyTrustManager_verifyRemoteCertificate, which the JVM resolves via dlsym. Attempts to selectively re-export it with --export-dynamic-symbol did not work because a linker version script takes precedence.

Approach: The JNI entry point plus its backing static _Atomic callback state and a new StoreRemoteVerificationCallback accessor are moved into a new translation unit pal_trust_manager_jni_export.c, compiled into a dedicated static archive System.Security.Cryptography.Native.Android.JNIExport-Static (output name ...JNIExport). This lets the Android build exclude the main crypto archive from symbol hiding while keeping the single JNI symbol exported through the separate archive. The shared library still compiles the JNI source directly, so its behavior is unchanged. Supporting plumbing is added: the new archive is registered in the static apphost NATIVE_LIBS, in the shared-framework platform manifest, and (for NativeAOT) in Microsoft.NETCore.Native.Unix.targets alongside an --export-dynamic-symbolIlcArg. The refactor also adds the standard MIT license headers to pal_trust_manager.{c,h}.

Summary: This is a focused, correct build/packaging refactor. The atomic callback variable and its sole consumer (the JNI function) remain in the same translation unit, so the store/load pairing and abort_unless guard semantics are preserved; the register function in pal_trust_manager.c now delegates to StoreRemoteVerificationCallback in that same unit. Both the shared and static link paths are updated consistently, and the NativeAOT path is covered separately. Only minor style nits (a space before the parenthesis in StoreRemoteVerificationCallback (...) in the header and implementation) deviate from the surrounding code; these are non-blocking. Verdict: LGTM. Note: functional validation depends on Android CI build/link of the split archives, which I did not run.

Detailed Findings

  • Minor style: void StoreRemoteVerificationCallback (RemoteCertificateValidationCallback callback) (in pal_trust_manager.h and pal_trust_manager_jni_export.c) has a space before the opening parenthesis, inconsistent with the rest of the file. Non-blocking.

Note

This review was generated by this repository's Holistic Review agentic workflow to complement the built-in Copilot review.

Generated by Holistic Review · 51 AIC · ⌖ 10.5 AIC · ⊞ 10K

Sign up for freeto 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.

8 participants

@grendello@bartonjs@filipnavara@jkotas@simonrozsival@akoeplinger@teo-tsirpanis
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

[Android] Put Java JNI function in a separate static library - #108513

Open
grendello wants to merge 9 commits into
dotnet:mainfrom
grendello:dev/grendel/android-crypto-public-symbols
Open

[Android] Put Java JNI function in a separate static library#108513
grendello wants to merge 9 commits into
dotnet:mainfrom
grendello:dev/grendel/android-crypto-public-symbols

Conversation

@grendello

Copy link
Copy Markdown
Contributor

In dotnet/android#9006 we are working on linking the .NET for Android
runtime dynamically at the application build time.

Linking includes all the relevant BCL native libraries, and one of the goals is to
hide all the exported symbols used as p/invokes by the managed BCL
libraries. This is because p/invoke calls are handled internally and,
with dynamic linking of the runtime, there is no longer any reason to
use dlopen and dlsym to look them up, they are all resolved
internally, at the link time.

Symbol hiding works fine thanks to the --exclude-libsclang flag,
which makes all the exported symbols in the indicated .a archives to
not be exported by the linker. However, System.Security.Cryptography.Native.Android
is special in the sense that it contains one symbol which must not be hidden,
Java_net_dot_android_crypto_DotnetProxyTrustManager_verifyRemoteCertificate.

The above function is a Java native method implementation, and it
requires that not only its name follows the Java JNI naming rules, but
that it is also available for the JVM to look up using dlsym.

I tried using the --export-dynamic-symbol clang flag to
export just this function, but it doesn't appear to work no matter
where I put the flag in relation to reference to the .a archives.

Instead, the problem can be dealt with by putting the JNI function in a
separate static library, so that I can link it without changing symbol
visibility, while making all the System.Security.Cryptography.Native.Android
symbols invisible.

@dotnet-policy-servicedotnet-policy-serviceBot added the community-contribution Indicates that the PR has been added by a community member label Oct 3, 2024
@grendello
grendelloforce-pushed the dev/grendel/android-crypto-public-symbols branch from aa4fac2 to 7e67d99CompareOctober 3, 2024 11:59
@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.

@bartonjs

Copy link
Copy Markdown
Member

@grendello I'm not sure what state this is in. I've been assuming since October that someone else from .NET Android was going to sign off... it's outside of my domain. But since it's in my area and I'm trying to get things tidied up... "do we want this, or should we close the PR?"

@grendello
grendelloforce-pushed the dev/grendel/android-crypto-public-symbols branch from 9521f00 to d8003f8CompareAugust 5, 2025 09:32
CopilotAI review requested due to automatic review settings August 5, 2025 09:32

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

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 refactors the Android cryptography native library to support dynamic linking in .NET for Android runtime by separating a Java JNI function into its own static library. The key motivation is to enable symbol hiding for all BCL native library exports while keeping the JNI function visible to the Java Virtual Machine.

  • Move JNI function to separate static library to control symbol visibility during dynamic linking
  • Refactor callback storage mechanism to support the new architecture
  • Update build configuration to include the new static library in all relevant targets

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
FileDescription
pal_trust_manager_jni_export.cNew file containing the JNI function and atomic callback storage
pal_trust_manager.hAdd function declaration for the new callback storage function
pal_trust_manager.cRemove JNI function and atomic storage, delegate to new storage function
CMakeLists.txtAdd new static library target for JNI exports with detailed comments
apphost/static/CMakeLists.txtInclude new static library in native libs list
Directory.Build.propsAdd new static library to platform manifest
Comments suppressed due to low confidence (1)

src/native/libs/System.Security.Cryptography.Native.Android/pal_trust_manager_jni_export.c:6

  • The function name 'StoreRemoteVerificationCallback' doesn't follow the established naming convention. Based on the existing code pattern, it should be prefixed with 'AndroidCryptoNative_' like other public functions in this module.
void StoreRemoteVerificationCallback (RemoteCertificateValidationCallback callback)

@grendello
grendelloforce-pushed the dev/grendel/android-crypto-public-symbols branch from d8003f8 to 436d952CompareAugust 5, 2025 09:35
@filipnavara

filipnavara commented Aug 6, 2025

Copy link
Copy Markdown
Member

I am not necessarily opposed to it but this feels like a pretty heavy solution. In case of NativeAOT it's possible to fix this with just including this in MSBuild item group:

<IlcArgInclude="--export-dynamic-symbol:Java_net_dot_android_crypto_DotnetProxyTrustManager_verifyRemoteCertificate" />

The build process generates an exports file which is then fed to the linker. I wonder if the CoreCLR/Android build process uses something similar because the linker script takes a precedence over --export-dynamic-symbol argument to clang/lld.

For reference, the NativeAOT exports file looks like this:

V1.0 {
global:
Java_net_dot_android_crypto_DotnetProxyTrustManager_verifyRemoteCertificate;
JNI_OnLoad;
JNI_OnUnload;
Java_net_dot_jni_nativeaot_JavaInteropRuntime_init;
local: *;
};

@grendello

Copy link
Copy Markdown
ContributorAuthor

@filipnavara I played with the option you mention, with varying success. The goal I have in mind is to hide symbols by default with few exceptions, and for static linking this is easier than playing with --export-dynamic-symbol for X symbols.

@grendello

Copy link
Copy Markdown
ContributorAuthor

When linking statically a NativeAOT app, does it really need to export the symbol though?

Yes, it does. It's a JNI interface and the JavaVM/Dalvik looks for it.

Yeah, you're right. I forgot it is actually called from Java.

@grendello

Copy link
Copy Markdown
ContributorAuthor

Eventually, I think the runtime pack should provide a manifest of libraries that are to be used for linking as well as the init functions to call (if any) and what symbols to export.

For now, I'd say responsibility to export this (and potentially others) symbol rests on the consumer (NativeAOT or Android in this case)

@grendello
grendelloforce-pushed the dev/grendel/android-crypto-public-symbols branch from cadc93e to 4dc1629CompareAugust 6, 2025 14:00
@jkotas

Copy link
Copy Markdown
Member

I'd say responsibility to export this (and potentially others) symbol rests on the consumer (NativeAOT or Android in this case)

I agree - the export should be next to logic that references the library. Add --export-dynamic-symbol:... next to the line that references the library in the NAOT buildintegration in this PR?

Comment threadsrc/native/corehost/apphost/static/CMakeLists.txt Outdated
Comment threadsrc/native/libs/System.Security.Cryptography.Native.Android/CMakeLists.txt Outdated
grendelloand others added 8 commits August 22, 2025 12:29
In dotnet/android#9006 we are working on linking
the .NET for Android runtime dynamically at the application build time.
Linking involves all the BCL native libraries, including
`System.Security.Cryptography.Native.Android` and one of the goals is to
hide all the exported symbols used as p/invokes by the managed BCL
libraries. This is because p/invoke calls are handled internally and,
with dynamic linking of the runtime, there is no longer any reason to
use `dlopen` and `dlsym` to look them up, they are all resolved
internally, at the link time.
Symbol hiding works fine thanks to the `--exclude-libs` `clang` flag,
which makes all the exported symbols in the indicated `.a` archives to
not be exported by the linker. However,
`System.Security.Cryptography.Native.Android` is special in the sense
that it contains one symbol which must not be hidden,
`Java_net_dot_android_crypto_DotnetProxyTrustManager_verifyRemoteCertificate`.
The above function is a Java `native` method implementation, and it
requires that not only its name follows the Java JNI naming rules, but
that is also available for the JVM to look up using `dlsym`.
I tried using the `--export-dynamic-symbol` clang flag to
export **just** this function, but it doesn't appear to work no matter
where I put the flag in relation to reference to the `.a` archives.
Instead, the problem can be dealt with by putting the JNI function in a
separate static library, so that I can link it without changing symbol
visibility, while making all the
`System.Security.Cryptography.Native.Android` symbols invisible.
…e.Unix.targets
Co-authored-by: Filip Navara <filip.navara@gmail.com>
…e.Unix.targets
Co-authored-by: Jan Kotas <jkotas@microsoft.com>
@grendello
grendelloforce-pushed the dev/grendel/android-crypto-public-symbols branch from a17572a to 90dbd76CompareAugust 22, 2025 10:29
@github-actions

github-actionsBot commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Workflow state for the Holistic Review Orchestrator.

{
"version": 5,
"last_dispatched_commit": "f573e20268e1d94196a6cf4eb2bc623de843e2f7",
"last_dispatched_base_ref": "main",
"last_dispatched_base_sha": "bd94b146570c5f3057f29e40de901ca5154b7a92",
"last_reviewed_commit": "f573e20268e1d94196a6cf4eb2bc623de843e2f7",
"last_reviewed_base_ref": "main",
"last_reviewed_base_sha": "bd94b146570c5f3057f29e40de901ca5154b7a92",
"last_recorded_worker_run_id": "29672719152",
"review_attempt_commit": "",
"review_attempt_base_ref": "",
"review_attempt_count": 0,
"max_review_attempts": 5,
"review_history_format": "holistic-review-disclosure-v1",
"review_history": [
{
"commit": "f573e20268e1d94196a6cf4eb2bc623de843e2f7",
"review_id": 4729976780
}
]
}

@github-actionsgithub-actionsBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Holistic Review

Motivation: For .NET for Android dynamic runtime linking (dotnet/android#9006), all BCL native libraries are linked into a single .so and their exported symbols are hidden via clang's --exclude-libs applied to the .a archives. System.Security.Cryptography.Native.Android contains one symbol that must stay exported: the JNI native method Java_net_dot_android_crypto_DotnetProxyTrustManager_verifyRemoteCertificate, which the JVM resolves via dlsym. Attempts to selectively re-export it with --export-dynamic-symbol did not work because a linker version script takes precedence.

Approach: The JNI entry point plus its backing static _Atomic callback state and a new StoreRemoteVerificationCallback accessor are moved into a new translation unit pal_trust_manager_jni_export.c, compiled into a dedicated static archive System.Security.Cryptography.Native.Android.JNIExport-Static (output name ...JNIExport). This lets the Android build exclude the main crypto archive from symbol hiding while keeping the single JNI symbol exported through the separate archive. The shared library still compiles the JNI source directly, so its behavior is unchanged. Supporting plumbing is added: the new archive is registered in the static apphost NATIVE_LIBS, in the shared-framework platform manifest, and (for NativeAOT) in Microsoft.NETCore.Native.Unix.targets alongside an --export-dynamic-symbolIlcArg. The refactor also adds the standard MIT license headers to pal_trust_manager.{c,h}.

Summary: This is a focused, correct build/packaging refactor. The atomic callback variable and its sole consumer (the JNI function) remain in the same translation unit, so the store/load pairing and abort_unless guard semantics are preserved; the register function in pal_trust_manager.c now delegates to StoreRemoteVerificationCallback in that same unit. Both the shared and static link paths are updated consistently, and the NativeAOT path is covered separately. Only minor style nits (a space before the parenthesis in StoreRemoteVerificationCallback (...) in the header and implementation) deviate from the surrounding code; these are non-blocking. Verdict: LGTM. Note: functional validation depends on Android CI build/link of the split archives, which I did not run.

Detailed Findings

  • Minor style: void StoreRemoteVerificationCallback (RemoteCertificateValidationCallback callback) (in pal_trust_manager.h and pal_trust_manager_jni_export.c) has a space before the opening parenthesis, inconsistent with the rest of the file. Non-blocking.

Note

This review was generated by this repository's Holistic Review agentic workflow to complement the built-in Copilot review.

Generated by Holistic Review · 51 AIC · ⌖ 10.5 AIC · ⊞ 10K

Sign up for freeto 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.

8 participants

@grendello@bartonjs@filipnavara@jkotas@simonrozsival@akoeplinger@teo-tsirpanis
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

[Android] Put Java JNI function in a separate static library - #108513

Open
grendello wants to merge 9 commits into
dotnet:mainfrom
grendello:dev/grendel/android-crypto-public-symbols
Open

[Android] Put Java JNI function in a separate static library#108513
grendello wants to merge 9 commits into
dotnet:mainfrom
grendello:dev/grendel/android-crypto-public-symbols

Conversation

@grendello

Copy link
Copy Markdown
Contributor

In dotnet/android#9006 we are working on linking the .NET for Android
runtime dynamically at the application build time.

Linking includes all the relevant BCL native libraries, and one of the goals is to
hide all the exported symbols used as p/invokes by the managed BCL
libraries. This is because p/invoke calls are handled internally and,
with dynamic linking of the runtime, there is no longer any reason to
use dlopen and dlsym to look them up, they are all resolved
internally, at the link time.

Symbol hiding works fine thanks to the --exclude-libsclang flag,
which makes all the exported symbols in the indicated .a archives to
not be exported by the linker. However, System.Security.Cryptography.Native.Android
is special in the sense that it contains one symbol which must not be hidden,
Java_net_dot_android_crypto_DotnetProxyTrustManager_verifyRemoteCertificate.

The above function is a Java native method implementation, and it
requires that not only its name follows the Java JNI naming rules, but
that it is also available for the JVM to look up using dlsym.

I tried using the --export-dynamic-symbol clang flag to
export just this function, but it doesn't appear to work no matter
where I put the flag in relation to reference to the .a archives.

Instead, the problem can be dealt with by putting the JNI function in a
separate static library, so that I can link it without changing symbol
visibility, while making all the System.Security.Cryptography.Native.Android
symbols invisible.

@dotnet-policy-servicedotnet-policy-serviceBot added the community-contribution Indicates that the PR has been added by a community member label Oct 3, 2024
@grendello
grendelloforce-pushed the dev/grendel/android-crypto-public-symbols branch from aa4fac2 to 7e67d99CompareOctober 3, 2024 11:59
@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.

@bartonjs

Copy link
Copy Markdown
Member

@grendello I'm not sure what state this is in. I've been assuming since October that someone else from .NET Android was going to sign off... it's outside of my domain. But since it's in my area and I'm trying to get things tidied up... "do we want this, or should we close the PR?"

@grendello
grendelloforce-pushed the dev/grendel/android-crypto-public-symbols branch from 9521f00 to d8003f8CompareAugust 5, 2025 09:32
CopilotAI review requested due to automatic review settings August 5, 2025 09:32

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

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 refactors the Android cryptography native library to support dynamic linking in .NET for Android runtime by separating a Java JNI function into its own static library. The key motivation is to enable symbol hiding for all BCL native library exports while keeping the JNI function visible to the Java Virtual Machine.

  • Move JNI function to separate static library to control symbol visibility during dynamic linking
  • Refactor callback storage mechanism to support the new architecture
  • Update build configuration to include the new static library in all relevant targets

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
FileDescription
pal_trust_manager_jni_export.cNew file containing the JNI function and atomic callback storage
pal_trust_manager.hAdd function declaration for the new callback storage function
pal_trust_manager.cRemove JNI function and atomic storage, delegate to new storage function
CMakeLists.txtAdd new static library target for JNI exports with detailed comments
apphost/static/CMakeLists.txtInclude new static library in native libs list
Directory.Build.propsAdd new static library to platform manifest
Comments suppressed due to low confidence (1)

src/native/libs/System.Security.Cryptography.Native.Android/pal_trust_manager_jni_export.c:6

  • The function name 'StoreRemoteVerificationCallback' doesn't follow the established naming convention. Based on the existing code pattern, it should be prefixed with 'AndroidCryptoNative_' like other public functions in this module.
void StoreRemoteVerificationCallback (RemoteCertificateValidationCallback callback)

@grendello
grendelloforce-pushed the dev/grendel/android-crypto-public-symbols branch from d8003f8 to 436d952CompareAugust 5, 2025 09:35
@filipnavara

filipnavara commented Aug 6, 2025

Copy link
Copy Markdown
Member

I am not necessarily opposed to it but this feels like a pretty heavy solution. In case of NativeAOT it's possible to fix this with just including this in MSBuild item group:

<IlcArgInclude="--export-dynamic-symbol:Java_net_dot_android_crypto_DotnetProxyTrustManager_verifyRemoteCertificate" />

The build process generates an exports file which is then fed to the linker. I wonder if the CoreCLR/Android build process uses something similar because the linker script takes a precedence over --export-dynamic-symbol argument to clang/lld.

For reference, the NativeAOT exports file looks like this:

V1.0 {
global:
Java_net_dot_android_crypto_DotnetProxyTrustManager_verifyRemoteCertificate;
JNI_OnLoad;
JNI_OnUnload;
Java_net_dot_jni_nativeaot_JavaInteropRuntime_init;
local: *;
};

@grendello

Copy link
Copy Markdown
ContributorAuthor

@filipnavara I played with the option you mention, with varying success. The goal I have in mind is to hide symbols by default with few exceptions, and for static linking this is easier than playing with --export-dynamic-symbol for X symbols.

@grendello

Copy link
Copy Markdown
ContributorAuthor

When linking statically a NativeAOT app, does it really need to export the symbol though?

Yes, it does. It's a JNI interface and the JavaVM/Dalvik looks for it.

Yeah, you're right. I forgot it is actually called from Java.

@grendello

Copy link
Copy Markdown
ContributorAuthor

Eventually, I think the runtime pack should provide a manifest of libraries that are to be used for linking as well as the init functions to call (if any) and what symbols to export.

For now, I'd say responsibility to export this (and potentially others) symbol rests on the consumer (NativeAOT or Android in this case)

@grendello
grendelloforce-pushed the dev/grendel/android-crypto-public-symbols branch from cadc93e to 4dc1629CompareAugust 6, 2025 14:00
@jkotas

Copy link
Copy Markdown
Member

I'd say responsibility to export this (and potentially others) symbol rests on the consumer (NativeAOT or Android in this case)

I agree - the export should be next to logic that references the library. Add --export-dynamic-symbol:... next to the line that references the library in the NAOT buildintegration in this PR?

Comment threadsrc/native/corehost/apphost/static/CMakeLists.txt Outdated
Comment threadsrc/native/libs/System.Security.Cryptography.Native.Android/CMakeLists.txt Outdated
grendelloand others added 8 commits August 22, 2025 12:29
In dotnet/android#9006 we are working on linking
the .NET for Android runtime dynamically at the application build time.
Linking involves all the BCL native libraries, including
`System.Security.Cryptography.Native.Android` and one of the goals is to
hide all the exported symbols used as p/invokes by the managed BCL
libraries. This is because p/invoke calls are handled internally and,
with dynamic linking of the runtime, there is no longer any reason to
use `dlopen` and `dlsym` to look them up, they are all resolved
internally, at the link time.
Symbol hiding works fine thanks to the `--exclude-libs` `clang` flag,
which makes all the exported symbols in the indicated `.a` archives to
not be exported by the linker. However,
`System.Security.Cryptography.Native.Android` is special in the sense
that it contains one symbol which must not be hidden,
`Java_net_dot_android_crypto_DotnetProxyTrustManager_verifyRemoteCertificate`.
The above function is a Java `native` method implementation, and it
requires that not only its name follows the Java JNI naming rules, but
that is also available for the JVM to look up using `dlsym`.
I tried using the `--export-dynamic-symbol` clang flag to
export **just** this function, but it doesn't appear to work no matter
where I put the flag in relation to reference to the `.a` archives.
Instead, the problem can be dealt with by putting the JNI function in a
separate static library, so that I can link it without changing symbol
visibility, while making all the
`System.Security.Cryptography.Native.Android` symbols invisible.
…e.Unix.targets
Co-authored-by: Filip Navara <filip.navara@gmail.com>
…e.Unix.targets
Co-authored-by: Jan Kotas <jkotas@microsoft.com>
@grendello
grendelloforce-pushed the dev/grendel/android-crypto-public-symbols branch from a17572a to 90dbd76CompareAugust 22, 2025 10:29
@github-actions

github-actionsBot commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Workflow state for the Holistic Review Orchestrator.

{
"version": 5,
"last_dispatched_commit": "f573e20268e1d94196a6cf4eb2bc623de843e2f7",
"last_dispatched_base_ref": "main",
"last_dispatched_base_sha": "bd94b146570c5f3057f29e40de901ca5154b7a92",
"last_reviewed_commit": "f573e20268e1d94196a6cf4eb2bc623de843e2f7",
"last_reviewed_base_ref": "main",
"last_reviewed_base_sha": "bd94b146570c5f3057f29e40de901ca5154b7a92",
"last_recorded_worker_run_id": "29672719152",
"review_attempt_commit": "",
"review_attempt_base_ref": "",
"review_attempt_count": 0,
"max_review_attempts": 5,
"review_history_format": "holistic-review-disclosure-v1",
"review_history": [
{
"commit": "f573e20268e1d94196a6cf4eb2bc623de843e2f7",
"review_id": 4729976780
}
]
}

@github-actionsgithub-actionsBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Holistic Review

Motivation: For .NET for Android dynamic runtime linking (dotnet/android#9006), all BCL native libraries are linked into a single .so and their exported symbols are hidden via clang's --exclude-libs applied to the .a archives. System.Security.Cryptography.Native.Android contains one symbol that must stay exported: the JNI native method Java_net_dot_android_crypto_DotnetProxyTrustManager_verifyRemoteCertificate, which the JVM resolves via dlsym. Attempts to selectively re-export it with --export-dynamic-symbol did not work because a linker version script takes precedence.

Approach: The JNI entry point plus its backing static _Atomic callback state and a new StoreRemoteVerificationCallback accessor are moved into a new translation unit pal_trust_manager_jni_export.c, compiled into a dedicated static archive System.Security.Cryptography.Native.Android.JNIExport-Static (output name ...JNIExport). This lets the Android build exclude the main crypto archive from symbol hiding while keeping the single JNI symbol exported through the separate archive. The shared library still compiles the JNI source directly, so its behavior is unchanged. Supporting plumbing is added: the new archive is registered in the static apphost NATIVE_LIBS, in the shared-framework platform manifest, and (for NativeAOT) in Microsoft.NETCore.Native.Unix.targets alongside an --export-dynamic-symbolIlcArg. The refactor also adds the standard MIT license headers to pal_trust_manager.{c,h}.

Summary: This is a focused, correct build/packaging refactor. The atomic callback variable and its sole consumer (the JNI function) remain in the same translation unit, so the store/load pairing and abort_unless guard semantics are preserved; the register function in pal_trust_manager.c now delegates to StoreRemoteVerificationCallback in that same unit. Both the shared and static link paths are updated consistently, and the NativeAOT path is covered separately. Only minor style nits (a space before the parenthesis in StoreRemoteVerificationCallback (...) in the header and implementation) deviate from the surrounding code; these are non-blocking. Verdict: LGTM. Note: functional validation depends on Android CI build/link of the split archives, which I did not run.

Detailed Findings

  • Minor style: void StoreRemoteVerificationCallback (RemoteCertificateValidationCallback callback) (in pal_trust_manager.h and pal_trust_manager_jni_export.c) has a space before the opening parenthesis, inconsistent with the rest of the file. Non-blocking.

Note

This review was generated by this repository's Holistic Review agentic workflow to complement the built-in Copilot review.

Generated by Holistic Review · 51 AIC · ⌖ 10.5 AIC · ⊞ 10K

Sign up for freeto 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.

8 participants

@grendello@bartonjs@filipnavara@jkotas@simonrozsival@akoeplinger@teo-tsirpanis
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

[Android] Put Java JNI function in a separate static library - #108513

Open
grendello wants to merge 9 commits into
dotnet:mainfrom
grendello:dev/grendel/android-crypto-public-symbols
Open

[Android] Put Java JNI function in a separate static library#108513
grendello wants to merge 9 commits into
dotnet:mainfrom
grendello:dev/grendel/android-crypto-public-symbols

Conversation

@grendello

Copy link
Copy Markdown
Contributor

In dotnet/android#9006 we are working on linking the .NET for Android
runtime dynamically at the application build time.

Linking includes all the relevant BCL native libraries, and one of the goals is to
hide all the exported symbols used as p/invokes by the managed BCL
libraries. This is because p/invoke calls are handled internally and,
with dynamic linking of the runtime, there is no longer any reason to
use dlopen and dlsym to look them up, they are all resolved
internally, at the link time.

Symbol hiding works fine thanks to the --exclude-libsclang flag,
which makes all the exported symbols in the indicated .a archives to
not be exported by the linker. However, System.Security.Cryptography.Native.Android
is special in the sense that it contains one symbol which must not be hidden,
Java_net_dot_android_crypto_DotnetProxyTrustManager_verifyRemoteCertificate.

The above function is a Java native method implementation, and it
requires that not only its name follows the Java JNI naming rules, but
that it is also available for the JVM to look up using dlsym.

I tried using the --export-dynamic-symbol clang flag to
export just this function, but it doesn't appear to work no matter
where I put the flag in relation to reference to the .a archives.

Instead, the problem can be dealt with by putting the JNI function in a
separate static library, so that I can link it without changing symbol
visibility, while making all the System.Security.Cryptography.Native.Android
symbols invisible.

@dotnet-policy-servicedotnet-policy-serviceBot added the community-contribution Indicates that the PR has been added by a community member label Oct 3, 2024
@grendello
grendelloforce-pushed the dev/grendel/android-crypto-public-symbols branch from aa4fac2 to 7e67d99CompareOctober 3, 2024 11:59
@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.

@bartonjs

Copy link
Copy Markdown
Member

@grendello I'm not sure what state this is in. I've been assuming since October that someone else from .NET Android was going to sign off... it's outside of my domain. But since it's in my area and I'm trying to get things tidied up... "do we want this, or should we close the PR?"

@grendello
grendelloforce-pushed the dev/grendel/android-crypto-public-symbols branch from 9521f00 to d8003f8CompareAugust 5, 2025 09:32
CopilotAI review requested due to automatic review settings August 5, 2025 09:32

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

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 refactors the Android cryptography native library to support dynamic linking in .NET for Android runtime by separating a Java JNI function into its own static library. The key motivation is to enable symbol hiding for all BCL native library exports while keeping the JNI function visible to the Java Virtual Machine.

  • Move JNI function to separate static library to control symbol visibility during dynamic linking
  • Refactor callback storage mechanism to support the new architecture
  • Update build configuration to include the new static library in all relevant targets

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
FileDescription
pal_trust_manager_jni_export.cNew file containing the JNI function and atomic callback storage
pal_trust_manager.hAdd function declaration for the new callback storage function
pal_trust_manager.cRemove JNI function and atomic storage, delegate to new storage function
CMakeLists.txtAdd new static library target for JNI exports with detailed comments
apphost/static/CMakeLists.txtInclude new static library in native libs list
Directory.Build.propsAdd new static library to platform manifest
Comments suppressed due to low confidence (1)

src/native/libs/System.Security.Cryptography.Native.Android/pal_trust_manager_jni_export.c:6

  • The function name 'StoreRemoteVerificationCallback' doesn't follow the established naming convention. Based on the existing code pattern, it should be prefixed with 'AndroidCryptoNative_' like other public functions in this module.
void StoreRemoteVerificationCallback (RemoteCertificateValidationCallback callback)

@grendello
grendelloforce-pushed the dev/grendel/android-crypto-public-symbols branch from d8003f8 to 436d952CompareAugust 5, 2025 09:35
@filipnavara

filipnavara commented Aug 6, 2025

Copy link
Copy Markdown
Member

I am not necessarily opposed to it but this feels like a pretty heavy solution. In case of NativeAOT it's possible to fix this with just including this in MSBuild item group:

<IlcArgInclude="--export-dynamic-symbol:Java_net_dot_android_crypto_DotnetProxyTrustManager_verifyRemoteCertificate" />

The build process generates an exports file which is then fed to the linker. I wonder if the CoreCLR/Android build process uses something similar because the linker script takes a precedence over --export-dynamic-symbol argument to clang/lld.

For reference, the NativeAOT exports file looks like this:

V1.0 {
global:
Java_net_dot_android_crypto_DotnetProxyTrustManager_verifyRemoteCertificate;
JNI_OnLoad;
JNI_OnUnload;
Java_net_dot_jni_nativeaot_JavaInteropRuntime_init;
local: *;
};

@grendello

Copy link
Copy Markdown
ContributorAuthor

@filipnavara I played with the option you mention, with varying success. The goal I have in mind is to hide symbols by default with few exceptions, and for static linking this is easier than playing with --export-dynamic-symbol for X symbols.

@grendello

Copy link
Copy Markdown
ContributorAuthor

When linking statically a NativeAOT app, does it really need to export the symbol though?

Yes, it does. It's a JNI interface and the JavaVM/Dalvik looks for it.

Yeah, you're right. I forgot it is actually called from Java.

@grendello

Copy link
Copy Markdown
ContributorAuthor

Eventually, I think the runtime pack should provide a manifest of libraries that are to be used for linking as well as the init functions to call (if any) and what symbols to export.

For now, I'd say responsibility to export this (and potentially others) symbol rests on the consumer (NativeAOT or Android in this case)

@grendello
grendelloforce-pushed the dev/grendel/android-crypto-public-symbols branch from cadc93e to 4dc1629CompareAugust 6, 2025 14:00
@jkotas

Copy link
Copy Markdown
Member

I'd say responsibility to export this (and potentially others) symbol rests on the consumer (NativeAOT or Android in this case)

I agree - the export should be next to logic that references the library. Add --export-dynamic-symbol:... next to the line that references the library in the NAOT buildintegration in this PR?

Comment threadsrc/native/corehost/apphost/static/CMakeLists.txt Outdated
Comment threadsrc/native/libs/System.Security.Cryptography.Native.Android/CMakeLists.txt Outdated
grendelloand others added 8 commits August 22, 2025 12:29
In dotnet/android#9006 we are working on linking
the .NET for Android runtime dynamically at the application build time.
Linking involves all the BCL native libraries, including
`System.Security.Cryptography.Native.Android` and one of the goals is to
hide all the exported symbols used as p/invokes by the managed BCL
libraries. This is because p/invoke calls are handled internally and,
with dynamic linking of the runtime, there is no longer any reason to
use `dlopen` and `dlsym` to look them up, they are all resolved
internally, at the link time.
Symbol hiding works fine thanks to the `--exclude-libs` `clang` flag,
which makes all the exported symbols in the indicated `.a` archives to
not be exported by the linker. However,
`System.Security.Cryptography.Native.Android` is special in the sense
that it contains one symbol which must not be hidden,
`Java_net_dot_android_crypto_DotnetProxyTrustManager_verifyRemoteCertificate`.
The above function is a Java `native` method implementation, and it
requires that not only its name follows the Java JNI naming rules, but
that is also available for the JVM to look up using `dlsym`.
I tried using the `--export-dynamic-symbol` clang flag to
export **just** this function, but it doesn't appear to work no matter
where I put the flag in relation to reference to the `.a` archives.
Instead, the problem can be dealt with by putting the JNI function in a
separate static library, so that I can link it without changing symbol
visibility, while making all the
`System.Security.Cryptography.Native.Android` symbols invisible.
…e.Unix.targets
Co-authored-by: Filip Navara <filip.navara@gmail.com>
…e.Unix.targets
Co-authored-by: Jan Kotas <jkotas@microsoft.com>
@grendello
grendelloforce-pushed the dev/grendel/android-crypto-public-symbols branch from a17572a to 90dbd76CompareAugust 22, 2025 10:29
@github-actions

github-actionsBot commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Workflow state for the Holistic Review Orchestrator.

{
"version": 5,
"last_dispatched_commit": "f573e20268e1d94196a6cf4eb2bc623de843e2f7",
"last_dispatched_base_ref": "main",
"last_dispatched_base_sha": "bd94b146570c5f3057f29e40de901ca5154b7a92",
"last_reviewed_commit": "f573e20268e1d94196a6cf4eb2bc623de843e2f7",
"last_reviewed_base_ref": "main",
"last_reviewed_base_sha": "bd94b146570c5f3057f29e40de901ca5154b7a92",
"last_recorded_worker_run_id": "29672719152",
"review_attempt_commit": "",
"review_attempt_base_ref": "",
"review_attempt_count": 0,
"max_review_attempts": 5,
"review_history_format": "holistic-review-disclosure-v1",
"review_history": [
{
"commit": "f573e20268e1d94196a6cf4eb2bc623de843e2f7",
"review_id": 4729976780
}
]
}

@github-actionsgithub-actionsBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Holistic Review

Motivation: For .NET for Android dynamic runtime linking (dotnet/android#9006), all BCL native libraries are linked into a single .so and their exported symbols are hidden via clang's --exclude-libs applied to the .a archives. System.Security.Cryptography.Native.Android contains one symbol that must stay exported: the JNI native method Java_net_dot_android_crypto_DotnetProxyTrustManager_verifyRemoteCertificate, which the JVM resolves via dlsym. Attempts to selectively re-export it with --export-dynamic-symbol did not work because a linker version script takes precedence.

Approach: The JNI entry point plus its backing static _Atomic callback state and a new StoreRemoteVerificationCallback accessor are moved into a new translation unit pal_trust_manager_jni_export.c, compiled into a dedicated static archive System.Security.Cryptography.Native.Android.JNIExport-Static (output name ...JNIExport). This lets the Android build exclude the main crypto archive from symbol hiding while keeping the single JNI symbol exported through the separate archive. The shared library still compiles the JNI source directly, so its behavior is unchanged. Supporting plumbing is added: the new archive is registered in the static apphost NATIVE_LIBS, in the shared-framework platform manifest, and (for NativeAOT) in Microsoft.NETCore.Native.Unix.targets alongside an --export-dynamic-symbolIlcArg. The refactor also adds the standard MIT license headers to pal_trust_manager.{c,h}.

Summary: This is a focused, correct build/packaging refactor. The atomic callback variable and its sole consumer (the JNI function) remain in the same translation unit, so the store/load pairing and abort_unless guard semantics are preserved; the register function in pal_trust_manager.c now delegates to StoreRemoteVerificationCallback in that same unit. Both the shared and static link paths are updated consistently, and the NativeAOT path is covered separately. Only minor style nits (a space before the parenthesis in StoreRemoteVerificationCallback (...) in the header and implementation) deviate from the surrounding code; these are non-blocking. Verdict: LGTM. Note: functional validation depends on Android CI build/link of the split archives, which I did not run.

Detailed Findings

  • Minor style: void StoreRemoteVerificationCallback (RemoteCertificateValidationCallback callback) (in pal_trust_manager.h and pal_trust_manager_jni_export.c) has a space before the opening parenthesis, inconsistent with the rest of the file. Non-blocking.

Note

This review was generated by this repository's Holistic Review agentic workflow to complement the built-in Copilot review.

Generated by Holistic Review · 51 AIC · ⌖ 10.5 AIC · ⊞ 10K

Sign up for freeto 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.

8 participants

@grendello@bartonjs@filipnavara@jkotas@simonrozsival@akoeplinger@teo-tsirpanis