search

GrapheneOS / Vanadium

Privacy and security enhanced releases of Chromium for GrapheneOS. Vanadium provides the WebView and standard user-facing browser on GrapheneOS. It depends on hardening in other GrapheneOS repositories and doesn't include patches not relevant to the build targets used on GrapheneOS.
https://vanadium.app/
Other
886 stars 69 forks source link

remove mremap from the syscall whitelist again, again #170

Closed qua3k closed 2 years ago

qua3k commented 2 years ago

There was a patch for this in the previous incarnation of GrapheneOS due to the replacement of the (jemalloc?) allocator with a port of OpenBSD malloc for arm64. Chrome has switched to PartitionAlloc on all platforms by intercepting all malloc requests with PartitionAlloc-Everywhere which means we can bring this back. This has been in Hexavalent for a while without any noted problems but we've recently began testing removing it from the renderer process without any problems as of yet.

csagan5 commented 2 years ago

@qua3k the other 2 patches under linux/ do not have effects on Android, correct?

qua3k commented 2 years ago

They apply to Android but that's not going to be the approach taken by GrapheneOS to prevent dynamic code generation, which will likely involve transitioning to a more restrictive SELinux domain without the exceptions allowing dynamic code generation.

qua3k commented 2 years ago

I've also been working with Matt Denton of Google on getting this upstream; it's being tracked as crbug.com/1288042.

csagan5 commented 2 years ago

That is fantastic news! I gave up upstreaming patches some time ago because the official tools took me quite some time to set up, I would otherwise have contributed a few small ones (and definitely not privacy/security-related, although @uazo might have some of that kind); never thought I could just drop a link like you did.

Sorry to continue the off-topic but: I am planning to include those patches under linux/ in Bromite, I am aware of the possibility of crashes that you are discussing there with Matt Denton. I can try answering your question that you ask there about the switch/default: compiler will throw an error/warning when some case is not covered, perhaps that is the reason?

A more on-topic comment: mremap was introduced here: https://github.com/chromium/chromium/commit/d71e68da4fd871edaad91addf986bb0b40d6fdf8 (I am surprised that a security relaxing patch was greenlit in review with little resistance) but the referred safe browsing zip unpacking functionality is not used in Bromite/Vanadium.

qua3k commented 2 years ago

compiler will throw an error/warning when some case is not covered, perhaps that is the reason?

No. They enumerated all the syscalls needed back when they first started working on the seccomp-bpf sandbox and initially allowed many of those syscalls before they had a sane seccomp policy. It's since been fixed and that code has largely been untouched for many years.

I am surprised that a security relaxing patch was greenlit in review with little resistance

They aren't able to just refuse to relax the sandbox when mremap is used by many allocators as an efficient realloc. There wasn't any way around it and it's likely the same reason that Vanadium doesn't have this patch after GrapheneOS switched to hardened_malloc. It's not limited to the safe browsing zip functionality as realloc is used throughout the codebase.

qua3k commented 2 years ago

In case it wasn't clear before: we should remove __NR_mremap from sandbox/linux/seccomp-bpf-helpers/baseline_policy_android.cc as the fine grained seccomp filters for each process type aren't available on Android.

csagan5 commented 2 years ago

we should remove __NR_mremap from sandbox/linux/seccomp-bpf-helpers/baseline_policy_android.cc as the fine grained seccomp filters for each process type aren't available on Android.

I am going to test this patch on Android. Meanwhile let's see what upstream will say in https://crbug.com/1288042.

thestinger commented 2 years ago

@csagan5 My issue requesting you remove our patches was not based on licensing but rather the morality of taking code from people you're now being hostile towards. You prevented me from responding or editing my post and I expect the issue I filed to be deleted rather than misrepresented with no way to respond.

Banning me for politely requesting that you stop using our code has been reciprocated here and will be applied to all contributors to your project going forward across all GrapheneOS platforms. If you want to work out a resolution we can try to do that but things are not going to be the same going forward.

You're mistaken that you only have minor changes from Vanadium and GrapheneOS. Hexavalent's lead developer is a GrapheneOS core developer and multiple people who have submitted patches to Vanadium, Hexavalent and/or Bromite are GrapheneOS contributors/developers. Perhaps you don't realize people like Zoraver are GrapheneOS community members / contributors.

So far nothing has been announced by us about this but I did strip away the information about Bromite from our site and I'm considering what further steps should be taken.