Skip to content

Conversation

@MSylvia
Copy link
Member

@MSylvia MSylvia commented Jul 18, 2017

Change branch to d15-3 branch of mono

Change branch to d15-3 branch of mono
@dnfclas
Copy link

dnfclas commented Jul 18, 2017

This seems like a small (but important) contribution, so no Contribution License Agreement is required at this point. We will now review your pull request.
Thanks,
.NET Foundation Pull Request Bot

@MSylvia MSylvia merged commit 5139ec7 into d15-3 Jul 21, 2017
jonpryor added a commit to jonpryor/xamarin-android that referenced this pull request Aug 26, 2020
Fixes: https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1139578

Context: dotnet/java-interop#691
Context: https://liquid.microsoft.com/Web/Object/Read/ms.security/Requirements/Microsoft.Security.SystemsADM.10039#guide

Changes: dotnet/java-interop@007b35b...dcaf794

  * dotnet/java-interop@dcaf794d: [generator] Fix error and warning codes values (dotnet#697)
  * dotnet/java-interop@08ff4db1: [java-interop, Java.Interop] Securely load native libs (dotnet#691)
  * dotnet/java-interop@09c68911: [generator] Clean up generated whitespace (dotnet#692)
  * dotnet/java-interop@d664e90e: [generator] Make warning and error messages localizable. (dotnet#689)

For improved security, the guidance is that when loading `.dll` files
on Windows, the default [`LoadLibrary()`][0] behavior is to be
*avoided*, as the [Dynamic-Link Library Search Order][1] includes the
"current working directory" and directories listed in `%PATH%`, both
of which are under control of an "attacker" and not the application.

Instead, [`LoadLibraryEx()`][2] should be used, providing a set of
`LOAD_LIBRARY_SEARCH_*` values which constrain the set of directories
that will be searched for the library (if a full path is not used)
and/or the library's dependencies.

Historically, Xamarin.Android hasn't directly called any
`LoadLibrary()` function.  Instead, xamarin-android called
[**dlopen**(3)][3], then used dlfcn-win32/dlfcn-win32@ef7e412d to
implement `dlopen()` in terms of `LoadLibraryEx()`.

Unfortunately, dlfcn-win32/dlfcn-win32@ef7e412d calls `LoadLibraryEx()`
with `LOAD_WITH_ALTERED_SEARCH_PATH`, which while a security
improvement, does not go far enough for our internal requirements.

Migrate away from dlfcn-win32 and instead use the new
`java-interop-dlfcn.h` family of functions added in
dotnet/java-interop@08ff4db1.  These new functions appropriately use
`LoadLibraryEx()` with a constrained set of `LOAD_LIBRARY_SEARCH_*`
flags.

*Non-obvious semantic note*: On Mono for all platforms, P/Invoking
`__Internal` is equivalent to trying to load the path `NULL`, e.g.
on macOS `dlopen(NULL, …)`.  However, only Android allows a custom
"`loader_func`" to intercept the attempted load of `NULL` and "do
something" with it, which xamarin-android *does* intercept, "remapping"
`NULL` to `libxa-internal-api.so`.  On all other platforms,
`loader_func` cannot intercept this invocation.

…which made for an "interesting" interaction: on macOS, if we used:

	api_dso_handle = java_interop_lib_load (dso_path.get (), JAVA_INTEROP_LIB_LOAD_LOCALLY, nullptr);

then the macOS Designer integration tests would fail!

	Renderer >> 4 [monodroid] Calling into managed runtime init
	Renderer (error) >>
	Renderer (error) >> Unhandled Exception:
	Renderer (error) >> System.EntryPointNotFoundException: java_interop_jnienv_get_java_vm assembly:<unknown assembly> type:<unknown type> member:(null)
	Renderer (error) >> at (wrapper managed-to-native) Java.Interop.NativeMethods.java_interop_jnienv_get_java_vm(intptr,intptr&)
	Renderer (error) >> at Java.Interop.JniEnvironment+References.GetJavaVM (System.IntPtr jnienv, System.IntPtr& vm) [0x00000] in <0f003a4904fd44d0a8cc6a63962ab40b>:0
	Renderer (error) >> at Java.Interop.JniEnvironmentInfo.set_EnvironmentPointer (System.IntPtr value) [0x00037] in <0f003a4904fd44d0a8cc6a63962ab40b>:0
	Renderer (error) >> at Java.Interop.JniEnvironmentInfo..ctor (System.IntPtr environmentPointer, Java.Interop.JniRuntime runtime) [0x00006] in <0f003a4904fd44d0a8cc6a63962ab40b>:0
	Renderer (error) >> at Java.Interop.JniRuntime..ctor (Java.Interop.JniRuntime+CreationOptions options) [0x0017b] in <0f003a4904fd44d0a8cc6a63962ab40b>:0
	Renderer (error) >> at Android.Runtime.AndroidRuntime..ctor (System.IntPtr jnienv, System.IntPtr vm, System.Boolean allocNewObjectSupported, System.IntPtr classLoader, System.IntPtr classLoader_loadClass, System.Boolean jniAddNativeMethodRegistrationAttributePresent) [0x00000] in /Users/builder/azdo/_work/4/s/xamarin-android/src/Mono.Android/Android.Runtime/AndroidRuntime.cs:25

In order for the macOS Designer integration tests to succeed,
`libxa-internal-api.dylib` *must* be loaded as `RTLD_GLOBAL`, which is
done via `JAVA_INTEROP_LIB_LOAD_GLOBALLY`.

[0]: https://docs.microsoft.com/en-us/windows/win32/api/libloaderapi/nf-libloaderapi-loadlibrarya
[1]: https://docs.microsoft.com/en-us/windows/win32/dlls/dynamic-link-library-search-order
[2]: https://docs.microsoft.com/en-us/windows/win32/api/libloaderapi/nf-libloaderapi-loadlibraryexa
[3]: https://linux.die.net/man/3/dlopen
jonpryor added a commit that referenced this pull request Aug 27, 2020
Fixes: https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1139578

Context: dotnet/java-interop#691
Context: https://liquid.microsoft.com/Web/Object/Read/ms.security/Requirements/Microsoft.Security.SystemsADM.10039#guide

Changes: dotnet/java-interop@007b35b...dcaf794

  * dotnet/java-interop@dcaf794d: [generator] Fix error and warning codes values (#697)
  * dotnet/java-interop@08ff4db1: [java-interop, Java.Interop] Securely load native libs (#691)
  * dotnet/java-interop@09c68911: [generator] Clean up generated whitespace (#692)
  * dotnet/java-interop@d664e90e: [generator] Make warning and error messages localizable. (#689)

For improved security, the guidance is that when loading `.dll` files
on Windows, the default [`LoadLibrary()`][0] behavior is to be
*avoided*, as the [Dynamic-Link Library Search Order][1] includes the
"current working directory" and directories listed in `%PATH%`, both
of which are under control of an "attacker" and not the application.

Instead, [`LoadLibraryEx()`][2] should be used, providing a set of
`LOAD_LIBRARY_SEARCH_*` values which constrain the set of directories
that will be searched for the library (if a full path is not used)
and/or the library's dependencies.

Historically, Xamarin.Android hasn't directly called any
`LoadLibrary()` function.  Instead, xamarin-android called
[**dlopen**(3)][3], then used dlfcn-win32/dlfcn-win32@ef7e412d to
implement `dlopen()` in terms of `LoadLibraryEx()`.

Unfortunately, dlfcn-win32/dlfcn-win32@ef7e412d calls `LoadLibraryEx()`
with `LOAD_WITH_ALTERED_SEARCH_PATH`, which while a security
improvement, does not go far enough for our internal requirements.

Migrate away from dlfcn-win32 and instead use the new
`java-interop-dlfcn.h` family of functions added in
dotnet/java-interop@08ff4db1.  These new functions appropriately use
`LoadLibraryEx()` with a constrained set of `LOAD_LIBRARY_SEARCH_*`
flags.

*Non-obvious semantic note*: On Mono for all platforms, P/Invoking
`__Internal` is equivalent to trying to load the path `NULL`, e.g.
on macOS `dlopen(NULL, …)`.  However, only Android allows a custom
"`loader_func`" to intercept the attempted load of `NULL` and "do
something" with it, which xamarin-android *does* intercept, "remapping"
`NULL` to `libxa-internal-api.so`.  On all other platforms,
`loader_func` cannot intercept this invocation.

…which made for an "interesting" interaction: on macOS, if we used:

	api_dso_handle = java_interop_lib_load (dso_path.get (), JAVA_INTEROP_LIB_LOAD_LOCALLY, nullptr);

then the macOS Designer integration tests would fail!

	Renderer >> 4 [monodroid] Calling into managed runtime init
	Renderer (error) >>
	Renderer (error) >> Unhandled Exception:
	Renderer (error) >> System.EntryPointNotFoundException: java_interop_jnienv_get_java_vm assembly:<unknown assembly> type:<unknown type> member:(null)
	Renderer (error) >> at (wrapper managed-to-native) Java.Interop.NativeMethods.java_interop_jnienv_get_java_vm(intptr,intptr&)
	Renderer (error) >> at Java.Interop.JniEnvironment+References.GetJavaVM (System.IntPtr jnienv, System.IntPtr& vm) [0x00000] in <0f003a4904fd44d0a8cc6a63962ab40b>:0
	Renderer (error) >> at Java.Interop.JniEnvironmentInfo.set_EnvironmentPointer (System.IntPtr value) [0x00037] in <0f003a4904fd44d0a8cc6a63962ab40b>:0
	Renderer (error) >> at Java.Interop.JniEnvironmentInfo..ctor (System.IntPtr environmentPointer, Java.Interop.JniRuntime runtime) [0x00006] in <0f003a4904fd44d0a8cc6a63962ab40b>:0
	Renderer (error) >> at Java.Interop.JniRuntime..ctor (Java.Interop.JniRuntime+CreationOptions options) [0x0017b] in <0f003a4904fd44d0a8cc6a63962ab40b>:0
	Renderer (error) >> at Android.Runtime.AndroidRuntime..ctor (System.IntPtr jnienv, System.IntPtr vm, System.Boolean allocNewObjectSupported, System.IntPtr classLoader, System.IntPtr classLoader_loadClass, System.Boolean jniAddNativeMethodRegistrationAttributePresent) [0x00000] in /Users/builder/azdo/_work/4/s/xamarin-android/src/Mono.Android/Android.Runtime/AndroidRuntime.cs:25

In order for the macOS Designer integration tests to succeed,
`libxa-internal-api.dylib` *must* be loaded as `RTLD_GLOBAL`, which is
done via `JAVA_INTEROP_LIB_LOAD_GLOBALLY`.

[0]: https://docs.microsoft.com/en-us/windows/win32/api/libloaderapi/nf-libloaderapi-loadlibrarya
[1]: https://docs.microsoft.com/en-us/windows/win32/dlls/dynamic-link-library-search-order
[2]: https://docs.microsoft.com/en-us/windows/win32/api/libloaderapi/nf-libloaderapi-loadlibraryexa
[3]: https://linux.die.net/man/3/dlopen
@jpobst jpobst deleted the d15-3-branch-patch branch March 8, 2021 17:02
@github-actions github-actions bot locked and limited conversation to collaborators Feb 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants