os-issue-tracker icon indicating copy to clipboard operation
os-issue-tracker copied to clipboard

Native libraries loading protection

Open ao-anssi opened this issue 3 years ago • 17 comments

Hi I've implemented a small patch for android and I wondered if you would be interested in it :

Its motivation is to allow a developer or a packager of an app to forbid the loading by an application of an unexpected native library (downloaded by the app or hidden in the resources, as there is already the android linker namespaces for system libraries). For example if an app includes several third party libraries, the developer may want to prevent them from downloading/using unexpected native code.

So the princip is to allow the loading of a library by a user application only if the hash of the library is present in a list of allowed hashes put in the resources of the application by the developer.

To do so I modified the linker in bionic.

Let me know if you are interested and I will share you the code (I still have to make sure that the code is as clean as possible but at least you will have the principles).

ao-anssi avatar Apr 11 '22 13:04 ao-anssi

Applications should either use the modern library loading directly from the apk or at least properly packaged libraries extracted as apk_data_file by the package manager which can be read/executed by the application but not written. Android already forbid execute_no_trans for app_data_file for modern API levels (i.e. cannot run executables from there, must be properly packaged and run from apk_data_file) and this should be extended to removing execute / execmod from app_data_file too. I think most apps would be compatible with this but a significant number still manually extract their libraries to app data. It's against Play Store policy if they dynamically download them and the violation can be reported.

thestinger avatar Apr 11 '22 13:04 thestinger

We have these 2 issues filed:

https://github.com/GrapheneOS/os-issue-tracker/issues/33 https://github.com/GrapheneOS/os-issue-tracker/issues/21

#21 is about removing execmem, app_data_file execute/execmod and ashmem execute as we've done for base OS apps when the feature is toggled on. Apps could request force enabling this protection with an attribute instead of the user needing to toggle it on. Unfortunately this can't be fully upstreamed due to ART having a JIT which is disabled on GrapheneOS (we use full AOT compilation instead).

thestinger avatar Apr 11 '22 13:04 thestinger

@ao-anssi We would be interested in seeing your patches for this

flawedworld avatar Apr 16 '22 14:04 flawedworld

Hi

Thank you for interest :-) Here are the two patches. I've made them on Android AOSP 12 with the tag android-12.0.0_r26. I have simply put a hook in the bionic do_dlopen function of linker.cpp. It looks in the loading app if it contains a file res/raw/shalist.txt and which is of the form :

/lib/armv8/foo.so d030a4842fa5eb26b9262546bf69da62552e4f04508a03e16d5dd206cc7ba26e /lib/armv8/bar.so 1bc59653b096859a13bd2dd4a9d0cdd536178983556badf8a346573686bedd8e

I also join you a little experimental app to test, be careful it contains two libtestlibloading2.so which are different (one in res/raw and one in lib).

I'm sorry the C/CPP code may not be as clean as required, but you will have all the principles.

Feel free to ask any question 2022-3-30-lib_loading_checking.tar.gz

ao-anssi avatar Apr 19 '22 08:04 ao-anssi

Reading it again, I must say the code is still the one of a poc

ao-anssi avatar Apr 19 '22 08:04 ao-anssi

@ao-anssi What do you think about implementing it by making new SELinux domains based on untrusted_app where app_data_file execute is removed and then coming up with a way to dynamically switch apps to those alternate domains? There was someone who was working on that upstream for AOSP a long time ago.

Google ended up simply outright removing execute_no_trans (running executables) for app_data_file for modern API levels which upset a lot of people since there's no opt-out. They may plan to do the same for app_data_file execute but a lot more apps are using that since they extract their libraries to app data instead of running them from the apk or libraries/executables extracted by the package manager (apk_data_file for either when it's done in the data partition rather than built into the OS).

Few apps need app_data_file execute and if they're going to be changed they might as well be changed to use the proper library packaging system. They can also have their libraries distributed via Play Feature Delivery or other apps tied to theirs.

thestinger avatar Apr 19 '22 11:04 thestinger

@thestinger Thank you, yes why not, sorry I haven't had time to think about it. The only difference is that the do_dlopen hook still allow some specific libraries execution, whereas selinux will block all execution, but that's not necessarily a problem and the selinux prevention will certainly be stronger, I will have look.

ao-anssi avatar Apr 19 '22 13:04 ao-anssi

We remove app_data_file execute for base OS apps already (can look in our sepolicy repository on GitHub) in GrapheneOS along with all other forms of dynamic native code execution but we have no mechanism for optionally doing it for third party apps. I think an opt-in or opt-out feature could be landed in AOSP. It's quite possible it could be landed for AOSP 14 where app_data_file execute is disallowed by default for the new API level. I think it's too late for AOSP 13.

It's possible to remove execmem and ashmem execute too, which fully closes the holes to enforce no dynamic native code generation / execution for storage or memory. The main issue blocking that for AOSP is that they have the ART JIT compiler, which we disable in favor of full AOT compilation. It would be entirely possible to have toggles for those too. Downstream, it would have to be opt-in to enforcement, but upstream it could be opt-out for a new API level and they would likely be willing to do it, although for execmem it's not really possible for a standard AOSP build.

thestinger avatar Apr 19 '22 13:04 thestinger

Just to be sure, is not the code resulting of the jit compilation the one in /data/app/.../oat/arm(64) or /data/dalvik-cache ? If yes isn't its selinux label dalvikcache_data_file ?

ao-anssi avatar Apr 19 '22 13:04 ao-anssi

Those are the AOT compilation outputs. JIT compilation happens in-memory and AOT compilation outputs aren't writable by the app or produced by anything in the app context.

AOSP uses a mix of JIT compilation and an interpreter initially, outputs JIT profiles, and then uses those to do profile guided AOT compilation in the background regularly along with recompiling as part of OS upgrades in the background. AOT compilation outputs can't be written by the app and they can only indirectly influence them via the JIT profiles output by the ART JIT compiler which they could technically mess with, but it would only be able to add or remove code from AOT compilation.

JIT compilation needs execmem and ashmem execute. With the JIT disabled, neither is required for most apps, only a few like Firefox implementing their own JIT compiler. Chromium's JIT compiler is only used in the isolated_app domain rather than untrusted_app since it uses an isolatedProcess for renderers, with site isolation enforcement depending on the available memory (Vanadium always uses strict site isolation).

app_data_file execute_no_trans was already removed upstream in AOSP for modern API levels so apps running native executables had to switch to properly including them with the native library / executable system where the package manager extracts them and they are apk_data_file (not writable by the app). Libraries can also be mapped directly from the apk which is the preferred and non-legacy approach for libraries since it's most efficient in terms of storage space and is more secure since the apk gets verified occasionally, or constantly for all reads from it if v4 app signing is being used (v4 signing is meant for store-based signing and the purpose is a stronger form of attestation used by the Play Integrity API which we aren't using ourselves for a non-Play system yet but will likely use in Auditor when we figure it out).

thestinger avatar Apr 19 '22 14:04 thestinger

dalvikcache_data_file and apk_data_file execute can only be removed for base OS apps that are fully precompiled and not updated out-of-band. There's no way to get rid of those for user installed apps or system apps updated out-of-band without disabling AOT compilation for them. Disabling both AOT and JIT compilation would require only using the interpreter which is very slow.

Apps can already opt-out of AOT compilation while still having JIT compilation via useEmbeddedDex which is used by our Auditor app, so it's fully interpreted on GrapheneOS where AOT compilation is disabled. It's only really useful for system apps updated out-of-band or apps using attestation where it can slightly strengthen the security model by not having persistent native code outside the apk if they also use embedded native libraries or have no native libraries of their own.

thestinger avatar Apr 19 '22 14:04 thestinger

Also, a consequence of removing apk_data_file execute for system apps is that they can't execute the WebView libraries if the WebView is updated out-of-band, which is the main reason why GrapheneOS has to make an OS release every time there's a Chromium release (we'd also have to teach our app repository client to deal with Trichrome updates and we'd want to figure out how to rename the app id from org.chromium.chrome to app.vanadium.browser before adding it to our repository, which should be possible with original-package but it didn't work for Chromium last time we tried due to missing support for features it uses).

thestinger avatar Apr 19 '22 14:04 thestinger

app_data_file -> readable/writable/executable by apps (GrapheneOS removes executable for base OS apps not updated out-of-band, although we could also remove it for base OS apps that are updated out-of-band but our policy isn't that granular, and we only update our standalone projects out-of-band, we distinguish based on signing key for sepolicy) apk_data_file -> readable/executable by apps (GrapheneOS removes it for base OS apps not updated out-of-band)

thestinger avatar Apr 19 '22 14:04 thestinger

To be sure that I understand well, starting with app_data_file : if JIT compilation result is not written on disk then to remove execution of app_data_file in selinux shouldn't break the app execution, is this true ?

I am not sure that dynamically change selinux context of an application is so good, because it may open a door for an attacker to modify the selinux context of an app. Wouldn't be better to set the right context of the application at the installation time ?

ao-anssi avatar Apr 25 '22 10:04 ao-anssi

I am not sure that dynamically change selinux context of an application is so good, because it may open a door for an attacker to modify the selinux context of an app. Wouldn't be better to set the right context of the application at the installation time ?

The Zygote has a defined list of domains that it's allowed to transition to and sets the appropriate domain for the app as part of spawning it. It's just how it works. Apps aren't an executable but rather they're all instances of app_process so there's nowhere to set a label for them statically, and the filesystem labels in data aren't meant to be trusted anyway by being able to reset all of them based on file paths.

thestinger avatar Apr 25 '22 11:04 thestinger

To be sure that I understand well, starting with app_data_file : if JIT compilation result is not written on disk then to remove execution of app_data_file in selinux shouldn't break the app execution, is this true ?

Removing app_data_file execute won't break ART JIT. It will only break mapping and executing data in the app's data directory, i.e. loading native libraries, or otherwise mapping something PROT_EXEC.

Removing execmem would break JIT in general and it's also possible removing ashmem execute would interfere depending on how it currently works. Ideally that would be available as an option too, but it requires not using the ART JIT, which implies using full AOT compilation instead or it would be very slow (interpreter only). AOT compilation requires extra time installing and adds time to OS updates in the background. Neither of those is particularly problematic but it does require more disk space than using profile-guided AOT where it's trying to avoid compiling cold / unused code.

thestinger avatar Apr 25 '22 11:04 thestinger

Thank you very much for your answers, I wrote a little too quickly

ao-anssi avatar Apr 25 '22 11:04 ao-anssi

Going to keep this open via https://github.com/GrapheneOS/os-issue-tracker/issues/33 and https://github.com/GrapheneOS/os-issue-tracker/issues/21 but I'd rather do it with the SELinux approach and focus on apps bundling their libraries and executables in the way that's recommended especially since upstream already banned doing it wrong for executables via SELinux policy a while ago.

thestinger avatar Oct 23 '22 08:10 thestinger