Use patches of the AndroidHardening project #226
Comments
|
I have checked those patches again, most of them are present in other patchsets in use by Bromite or covered with different patches; the 64bit patches (1st and 2nd) are not a good idea, and it is explained on the issue tracker why. Generally what these patches lack is some rationale for the changes, as there is just a subject line. I have selected 5 for possible inclusion in next release although the impact is minimal. |
|
Fixed in |
|
The bulk of the changes are not currently included there, since they aren't updated for the latest releases. It doesn't currently include the past hardening, and a lot of that is also done outside of the repository via the hardened toolchain and base OS. It's only a placeholder with a minimum set of changes for the time being. Disabling field trials substantially reduces the invasiveness of the changes required for the basics, since setting default options actually works properly and a bunch of questionable things don't ever get enabled. The reason for preferring 64-bit processes is providing substantially better exploit mitigations at the expense of slightly more memory usage. In addition to the standard mitigations, it also enables usage of https://github.com/AndroidHardening/hardened_malloc which is provided by the base OS. It's obviously not the same thing without the hardened allocator, libc changes and toolchain changes. |
Thanks for the explanation @thestinger; my only suggestion for improvement would be to expand the git commit message with rationale for the patch changes. I do not currently have plans to switch to a hardened toolchain because of the extra work that would be involved; additionally, the default toolchain is currently tested (also for security issues, I hope) on the Android versions used by Chrome/Chromium and I deem that an advantage. Is there a different location for latest Chromium patches one should look at? (Given that some/most of them might make sense only with a hardened kernel/toolchain).
Do you think this patch could be of any benefit on non-hardened Android? e.g. if compiled with the default Chromium SDK and toolchain, would a 64-bit process benefit from better exploit mitigations on let's say ARM64? |
|
For the records, the patches included when this issue was closed are here: d5dede3 (Other patches were already present, others not applicable)
|
I use the officially tested toolchains with hardening, so there's one for Android and one for Chromium. Some of that involves custom patches, while other parts can now be backported or simply enabled with custom compilation switches. They're close to enabling a subset of CFI on Android by default as they do on x86_64 Linux, but other than that it has to be enabled downstream.
It will provide high entropy ASLR (24-bit to 32-bit depending on whether the kernel uses 3 or 4 level page tables rather than 16-bit for 32-bit processes), high entropy stack canaries (56/64-bit instead of 24/32-bit depending on whether a zero byte is used) and also features like pointer authentication and memory tagging when those are made available in the future. The reason they started preferring 32-bit processes is to save memory, particularly since saving memory makes it feasible to use finer-grained sandboxing. Since I know the devices that I'm targeting have plenty of memory, it's not an issue to have 10% or so higher memory consumption for these processes. Similarly, it's possible to enabling the 'Strict site isolation' feature for substantially better sandboxing isolating each site from each other, rather than just isolating content as a whole from the system. The cost is increased memory usage, and it can be substantial for some websites making very heavy usage of iframes. I'll probably be enabling that by default in the near future, just as I used to enable the WebView sandbox a couple years before they shipped it as standard.
They use -fstack-protector-strong on ChromeOS, but not yet on other platforms, because there's push back against enabling an option resulting in ~1-2% lower performance and ~2-3% larger binaries. The size increase on Android is something they care a lot about since some devices have very little storage space. That also means ever so slightly higher memory / cache usage but not by the full 2-3%. Using -fwrapv (notably only when not using signed integer overflow checking - since it will override it and result in not performing checks) is just common sense since it eliminates the chance of security vulnerabilities being introduced by optimizations based on signed overflow being undefined. That has happened before, and those optimizations don't even add up to a 0.1% performance increase for this kind of software. It's not worth having. The Linux kernel passes -fwrapv and also -fno-strict-aliasing to disable those dangerous optimizations (since there's so much incorrect code they can break). In fact, it's easy to point to dozens of known examples of invalid code that could potentially be broken by those optimizations. I don't feel it's acceptable for projects to be using optimizations that are known to be broken with a bunch of code in their tree. They put barely any effort into even fixing the known cases. Chromium has blacklists for UBSan for 'false positives' (none of which are actually false positives, but rather "undefined, but not a bug beyond potentially being broken by optimizations or even code generation without them") and also for components too full of these bugs for them to currently want to bother with it. That includes a bunch of signed overflow issues (there's sadly no detection for aliasing violations, which are fairly common, but not that common). |
|
Also, using 64-bit means that a hardened allocator like https://github.com/AndroidHardening/hardened_malloc taking advantage of the 64-bit address space will be transparently used if it's present in the OS. So, right now, when running on the AndroidHardening releases. There are other assorted hardening features that will be exclusive to 64-bit too, including the upcoming hardware-based pointer authentication (ARMv8.3), memory tagging (ARMv8.5) and branch target indicator features (ARMv8.5) features. https://www.qualcomm.com/media/documents/files/whitepaper-pointer-authentication-on-armv8-3.pdf Memory tagging primarily has to be implemented by memory allocators, including the system one in libc and the various ones in Chromium. There's a section on a great approach to using it in my allocator README: https://github.com/AndroidHardening/hardened_malloc#memory-tagging It adds tags to the upper bits of pointers and a pointer can only be used to access memory with the appropriate tag assigned, so it can guarantee that linear overflows on the heap are caught along with guaranteeing that use-after-free is caught within a certain number of allocation slot reuses (after 15-16 uses for different allocations the tag will wrap back to the same value). It also provides a decent chance of catching other arbitrary forms of heap corruption. Usage within a memory allocator will be nearly free in terms of overall performance. Maybe 1-2% performance cost for a major improvement. Compilers can also use it for protection on the stack, but that's substantially more expensive. Branch target indicators will be nearly free (probably below 0.1% overall cost) and provide coarse-grained forward-edge (indirect calls) CFI as a baseline. Clang type-based CFI is a lot better, but it's nice to have something that can be applied universally without compatibility or performance issues. Enabling Clang CFI is a slow and painful process since all the bugs involving undefined code need to be fixed in the software. Pointer authentication is a lot less compelling, since the primary use case is protecting return addresses and there are better approaches. It's a form of weak probabilistic backward-edge CFI. It would be nicer to have deterministic protections, and ideally fully precise ones, such as the protection provided by a hardware-based shadow stack. |
@thestinger I could not find the Chromium toolchain in use for the AndroidHardening project; I am currently using Chromium's own suggested toolchain (https://chromium.googlesource.com/chromium/src/build/+/refs/heads/master/install-build-deps-android.sh), if you could share more details/patches about the hardened toolchain for Chromium I could consider integration.
https://bugs.chromium.org/p/chromium/issues/detail?id=469376 It seems to be blocked on a non-public bug.
It is now called
As I understand your patch enables By the way, thanks for the full explanation; I am going to add it to the fwrapv patch used in Bromite.
Thanks for these links, very interesting; I was kinda expecting this technology to come along as I have seen similar solutions pop up in the Intel world over the years. In terms of changes to Bromite, the scope of switching from 32bit to 64bit is simply limited to the WebView since I have not been building a Monochrome release for some time now. I am aware of the popularity of the Bromite SystemWebview for embedding in other ROMs so I wonder whether this change would negatively impact the current user base (and I do not have figures aside from your input on this); since most users do not use a hardened ROM (as far as I know), they would benefit only from higher entropy in ASLR and stack canaries; even for this small improvement, I would be inclined to enable it. Also, there is now a dedicated target for 64-bit Monochrome: chromium/chromium@9cd1720 But no such target for the webview alone?
@thestinger have you considered upstreaming some of this work? I can imagine you are or have been in contact with the guys at Project Zero? |
The bulk of my work hasn't yet been restored for the current releases of Chromium and Android. I'm testing some of my past toolchain hardening locally but it's not yet used for my public releases. Most of my changes are still in old tags on https://github.com/AndroidHardeningArchive (only a subset was in the latest Oreo release of the old incarnation of my work) or in local branches / tags. I'm not sure how aware you are about what happened with my business partner slowly corrupting the previous incarnation of the project and making it stray away from the open source roots before eventually deciding to completely screw me over. I'm now doing it on my own with the explicit decision to never have a business involved to the same extent. They'll be my projects without much trust or responsibility placed in other people. I'm working on getting funding for the projects and expanding the development team beyond myself, since it's a very broad project with a lot of areas requiring deep, complex work. It's going to be a lot different than before though, with everything permissively licensed (MIT) other than the kernel which has to be GPL2, along with keeping companies that are involved at a distance from the project and unable to significantly harm it in any way beyond no longer contributing resources or development time. However, this all means that the work has been set back many years, and it's taking me a lot of time to slowly piece together the same changes that I had from before. The main features are a better implementation of a stack canaries and a feature for zero initializing all uninitialized local variables. The improved stack canaries work by doing an XOR of the random value for the canary with the return address before writing out the canary value. Before checking the canary value and returning, it performs the XOR again, so the canary won't match if the return value was overwritten during the function call. On arm / arm64, return pointer protection also works better than x86 because they use a link register so the return address cannot be overwritten by another thread after this verification but before returning (a tiny window of opportunity, but it does exist on x86 and can be exploited via a race from other threads with a decent enough chance of success to make it work reliably as long as the attack can be repeatedly at a decent rate). I don't plan to upstream this because it would be difficult and there are better approaches like shadow stacks, ideally with hardware support to avoid bypasses. An early version of this was published at https://gist.github.com/thestinger/b8502a881d871fbc75d91bc00576157b but it was substantially improved since then, especially for arm64. I haven't yet ported it to the more recent LLVM releases used by the current versions of LLVM and Android though. I helped push for the zero initialization feature being implemented upstream in LLVM and that was successful. In the future, it will be available in the standard Chromium toolchain and will just need to be enabled in their build configuration.
The blockers are generally bugs it finds, so sometimes they're security issues which are always non-public until they get around to opening them up which they sometimes forget to do. They do sometimes have issues non-public for other reasons, especially those tied to new Android releases since in the past they've tried to keep them secret for a long time, but I don't think that's the case here.
Yeah, the process model is called site-per-process, but the feature providing the security is still referred to as site isolation and Android still has 'Strict site isolation' as a chrome://flags entry.
Ideally, -fwrapv could be always passed, but unfortunately the way it's implemented has silly interactions with other switches. The reason it would still make sense to pass it is because due to their UBSan blacklists, they get far from full coverage with it, so -fwrapv would still be better than nothing where it's not being used. Here's an example of a program where the compiler will actively change the behavior due to optimizing based on the assumption that signed integer overflow is guaranteed to never occur: #include <stdio.h>
#include <limits.h>
int x1(int x) { return (x + 1) > x; }
int x2(int x) { return (x - 1) > x; }
int x3(int x) { return (x + 1) < x; }
int x4(int x) { return (x - 1) < x; }
int main(void) {
printf("%d %d %d %d\n", x1(INT_MAX), x2(INT_MAX), x3(INT_MAX), x4(INT_MAX));
printf("%d %d %d %d\n", x1(INT_MIN), x2(INT_MIN), x3(INT_MIN), x4(INT_MIN));
return 0;
}
This can introduce a vulnerability by removing incorrect but still important security checks such as checking for signed integer overflow after it has already occured which is very common. Adding So, now, getting into why I don't pass Compile and run the same program like this: Production usage of the UBSan sanitizers is done like this:
I've upstreamed a significant amount in the past, but I last the time to keep doing it. My contact with various upstream projects, security researchers at Project Zero, etc. was drastically set back by what happened with my former business partner. He took over my Twitter account which was the primary way I interacted with the security community and I lost access to my work email. He even intentionally broke all of the redirects from the old repository locations to the new ones at https://github.com/AndroidHardening and https://github.com/AndroidHardeningArchive as soon as he had the opportunity. There was also an attack on my Reddit account making me lose access permanently. They spammed false reports from sockpuppet accounts and successfully got it banned for posting a public corporate email address, which they falsely portrayed as posting personal information. Reddit has yet to respond to any of my requests to review this, so I lost the ability to moderate the community there and properly migrate it to a new one with a sticky thread and locking the creation of new threads. He also spent a lot of time spreading misinformation about what has happened and stopping people from easily contacting me, knowing what really occurred, or even realizing that anything had changed at all. He made it look as if the project stalled and died, rather than it continuing without the interference and involvement of the company, which had never owned or controlled it, due to it being set up as an independent open source project that the company was supposed to be supporting (when really it was the other way around). Anyway, everything has been set back years, especially in terms of involvement with other projects and upstreaming the changes. Many of the changes need to be ported from Android 6 and Android 7 to Android 9 and soon 10, along with a similar situation for Chromium. I'm struggling to find enough time to keep everything going without any help and only a small amount of financial support. I'm making progress at restoring it though, as you can see. The only parts of my work that survived the disaster and continued on immediately were the Auditor app for hardware-based attestation (https://github.com/AndroidHardening/Auditor) and the optional remote attestation service it can be used with (https://github.com/AndroidHardening/AttestationServer) which is now hosted at https://attestation.app/. There's an explanation of this at https://attestation.app/about and https://attestation.app/tutorial. Development of it was definitely slowed down and disrupted, but thanks to these being independent apps, I was able to continue development. The hardened Chromium work is closely tied to the hardened toolchain and OS work and I was unable to keep it going. It has also never had releases outside of the OS, especially since part of the hardening involved has to do with restricting system apps (including Chromium) from executing any code outside verified partitions via ART and SELinux changes. |
|
The toolchain hardening consists of those custom features (XOR canaries, zero initialization) along with enabling additional switches (zero initialization should soon be available like this) such as -fwrapv, -fstack-overflow-strong and also using some of the production-oriented sanitizers in the trapping mode. The baseline CFI feature should be finished up by Google soon, and their performance-based blacklists can be disabled to sacrifice some performance for better security. Ideally, the integer overflow checking (at least signed-integer-overflow), bounds checking (object-size, bounds) and other CFI sanitizers (including cast sanitizers) could be enabled too. It's difficult since it uncovers so many bugs, most of which are not actual security issues in practice, so there's a lot of busywork involved to fix these countless bugs or to blacklist them from checks (ideally at a fine-grained level, not the extremely coarse blacklisting they use in many cases right now). I was using the sanitizers in limited scopes before, but it's difficult to maintain. Google does test with UBSan, and in theory, it can be used, but there are a lot of subtle latent bugs and regular regressions. |
|
Also, |
|
@csagan5 By the way, you may need https://github.com/AndroidHardening/chromium_patches/blob/pie/0005-disable-seed-based-field-trials.patch to disable the local field trials, which override some of the internal settings. Passing |
|
I've also confirmed that |
Yes, I am aware - although I have not read too deep into it. I remember having a bitter feeling about the situation and I am first and foremost sorry for the human aspect of the situation.
You might already have done your homework on this, and aside from big public funding partners you could also consider Kickstarter/BountySource if you have not before. I do not use them myself but they seem viable.
You might need some qualified open source project/community management help there, if the option of becoming one yourself is not viable (even if you were qualified, it would drain energies from hands-on development) perhaps you could check if any of such qualified help is available out there? I would expect professionals working with other medium/big open source projects to be the ideal candidates. If you would like to talk about a possible collaboration, feel free to contact me via reddit or email; I might have the correct integrity requirements but might fall short on others, including competence and time.
I am aware of your recent screw-over experience and I wish it to be an isolated special case; going forward, it is up to you to define collaboration methods which prevent harm. Personally for this project I have only received requests about rebranding/sponsorship/cloud integrations which I have politely declined.
This looks like really cool stuff, but a notch or two above my level; I can understand what it does and how it is implemented but I would not be able to review it nor contribute to it.
Yes, the text says "Strict site isolation" and maps to I confirm that it's the same thing.
Was this due to a public image problem or to a mere confusion about the authenticity of your online identity and contributions? I would hope that security researchers and other projects would not judge the source of the work but the work itself, as much as possible.
The incentives at play (from Reddit perspective) will make it difficult for you to solve that in any way that resembles justice, so I would give up on that aspect, at least for now.
I am still very sorry for the human aspect of the whole situation, and secondarily for the technical aspect; I would really hope you can cope and reduce the damage that this situation did to you and your trust, eventually. I am very grateful that you decided to continue this work - also on behalf of other Android users; let me know if I can be of help somehow.
This is all very interesting and beautiful, but I can see how it goes beyond the browser to tie together security in depth.
I expect the media libraries to be full of these.
Thanks for mentioning the difference between those two; I think all field trials are already blocked by this patch? https://github.com/bromite/bromite/blob/73.0.3683.61/build/patches/Disable-fetching-of-all-field-trials.patch
No variations displayed. Yes, I am familiar with the hard-wiring, I have done some myself before discovering how field trials work (but in my case I was simply missing
I see; I will include this patch for future WebView builds. My only remaining concern would be about installation instructions, as there are some community-contributed instructions here on the wiki page (which I have not verified myself) and I wonder if the involved paths would need to be changed. It seems like not since Thanks for your extensive description for the patches and the AndroidHardening project; I was not aware you were continuing your work there (I assume most people are passively in the same shadow cone after the recent issues you mentioned). I will mention the project and the URL on the official website and README of Bromite. |
The paths will be the same, since with or without the patch, it still provides complete 32-bit and 64-bit WebView implementations. It was only in Android Oreo that they enabled the WebView sandbox by default, so before then the entire thing always ran with the architecture of the app using it and that still has to be fully supported for compatibility at least outside builds for only modern releases. The patch only changes the preferred renderer architecture from 32-bit for 64-bit when the renderer is running as an isolated process (Oreo or later by default, but supported on Nougat or later via developer settings, and I used to enable it with some downstream fixes long before it was standard). Their |
Ah, yeah, that seems to disable the effects fully at a different layer. |
|
@thestinger I have added your rationale to the patches: e7e7afd |
|
By the way, a minimal patch is required for preferring 64-bit as of Chromium v74 since |
|
@thestinger I am still using this patch to enable 64-bit for the WebView, I cannot easily see how to enable preference of 64-bit for the main Chromium APK. Do you have a reference patch or source line to look at? |
|
This is the patch for Monochrome: https://github.com/GrapheneOS/chromium_patches/blob/pie/0002-use-64-bit-Monochrome-processes.patch. I haven't tried doing it for the standalone Chrome apks, but I'd expect it just needs: i.e. the same as Monochrome but without needing to deal with choosing how the WebView is included. |
|
https://github.com/GrapheneOS/chromium_patches/blob/pie/0019-enable-strict-site-isolation-by-default-on-Android.patch is also important and site isolation is enabled everywhere other than Android. The reason it's not enabled by default on Android is the additional memory usage from guaranteeing that each site is isolated in dedicated processes rather than running iframes for other sites in-process. It doesn't impact memory usage for sites without things like iframes. |
|
Site isolation is the only real mitigation for Spectre v1. Nothing else fully works, so without it there's a known way to steal data from the browser (sessions and data for other sites). |
|
@thestinger For site isolation I use this compromise: https://github.com/bromite/bromite/blob/74.0.3729.122/build/patches/Enable-site-per-process-isolation-for-devices-with-enough-memory.patch It would disable it with devices with enough memory; see https://bugs.chromium.org/p/chromium/issues/detail?id=844118 The comment in the code where the 1077MB limit is set: Do we have POCs for older Android versions (which presumably run on devices with <1077 MB RAM) for Spectre v1? I would generally just enable the more secure approach, but I am afraid that upstream has verified that browser would simply not be usable on those low-end devices. |
I don't think so, unless I'm missing something.
In general, it will be usable, but some sites with a huge number of iframes may not work well. For example, using GitHub is probably not going to use any extra memory, since I don't see anywhere that it uses iframes. |
|
@thestinger you're right, I was confused by reading through the branches in |
|
I am closing this for now; @thestinger please let me know if you come up with new patches, I'll be watching the repo meanwhile. |
|
Hello @Eddi2015! I am a bot 🤖 Thanks for submitting this issue! The issue will be automatically re-opened after that. Tip: if you use GitHub in Desktop mode the template will be proposed to you when submitting issues. |
kokukyocho
added
the
missing-issue-template
ghost
changed the title
Using -fwrapv (notably only when not using signed integer overflow checking - since it will override it and result in not performing checks) is just common sense since it eliminates the chance of security vulnerabilities being introduced by optimizations based on signed overflow being undefined. That has happened before, and those optimizations don't even add up to a 0.1% performance increase for this kind of software. It's not worth having. The Linux kernel passes -fwrapv and also -fno-strict-aliasing to disable those dangerous optimizations (since there is so much incorrect code they can break). In fact, it is easy to point to dozens of known examples of invalid code that could potentially be broken by those optimizations. It is not acceptable for projects to be using optimizations that are known to be broken with a bunch of code in their tree. They put barely any effort into even fixing the known cases. Chromium has blacklists for UBSan for 'false positives' (none of which are actually false positives, but rather "undefined, but not a bug beyond potentially being broken by optimizations or even code generation without them") and also for components too full of these bugs for them to currently want to bother with it. That includes a bunch of signed overflow issues (there is sadly no detection for aliasing violations, which are fairly common, but not that common). Ideally, -fwrapv could be always passed, but unfortunately the way it is implemented has silly interactions with other switches. The reason it would still make sense to pass it is because due to their UBSan blacklists, they get far from full coverage with it, so -fwrapv would still be better than nothing where it's not being used. Since -fwrapv makes signed integer overflow well-defined, Clang will disable the UBSan checks for signed integer overflow, including in the production-oriented trapping mode used for hardening. Excerpt from bromite/bromite#226 Original License: MIT - https://spdx.org/licenses/MIT.html License: GPL-3.0-only - https://spdx.org/licenses/GPL-3.0-only.html
Is your feature request related to privacy?
It might be related to privacy, but I think it's more about security, which is also important
Is there a patch available for this feature somewhere?
https://github.com/AndroidHardening/chromium_patches
Describe the solution you'd like
Build and update Bromite with those patches that are not yet integrated.
Describe alternatives you've considered
Not building Bromite with those patches, what'd be a pity.
The text was updated successfully, but these errors were encountered: