777948 - (ASLR-b2g) Consider implementing address space layout randomization (ASLR) for B2G

Status: RESOLVED FIXED

(Firefox OS Graveyard :: General, enhancement)

Description

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Having address space layout randomization (ASLR) eventually on B2G will be a decent additional security feature. Some BH 2012 talks also mentioned ASLR being one of the technologies to make phone hacking a little more difficult (note: it does not make it impossible.)

ref http://en.wikipedia.org/wiki/ASLR

See also bug 777917 for adding Full Disk Encryption support to B2G.

There might be a speed penalty with ASLR - this may need more investigation. I've also heard somewhere that having gonk update to 4.1 (Android bits?) might help, but others might be able to provide more of an insight.

Comment 1

[Guillaume Destuynder [:kang] [this account is no longer active]]

Android 4.1 implements full ASLR, if gonk is updated to Android 4.1 then we would inherit the work that has already been done (also it would be compatible with a newer/faster toolchain w/ GCC 4.7)

Gonk & Gecko seems to both need their own settings for this (mainly relro, bind now link flags and pie).

Comment 2

[Guillaume Destuynder [:kang] [this account is no longer active]]

Attachment: 5 patches for gonk ASLR support.

Here's a few patches that add ASLR (pie/bind now/relro/fortify support in fact) for Gonk.

Note that this does not build Gecko wil PIE, as it uses a custom linker, so b2g and content processes will not be fully randomized.

Comment 3

[Guillaume Destuynder [:kang] [this account is no longer active]]

Attachment: 4 patches for gecko aslr

while i patched the non-moz-specific parts of the linker, i suppose only patch 4 is necessary

this works fine on my device.

with all the patches listed, everything seems to have full aslr, except some specific programs such as adbd (same issue on android 4.2).

Comment 4

[Guillaume Destuynder [:kang] [this account is no longer active]]

:cjones, do you know how to go forward with this ?

My main concern is that I need proper performance testing of the above, and proper understanding of elfhack / the moz linker. Then I probably put them up for review and see if we can have this implemented in a future version.

Comment 5

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Let's get the patches cleaned up and then get review from glandium.

Comment 6

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Guillaume has some WIP patches, assigning to him (feel free to change if necessary).

Comment 7

[Guillaume Destuynder [:kang] [this account is no longer active]]

https://datazilla.mozilla.org/b2g/ this would give a good estimate of the perf impact if we could run it on a ASLR branch

Comment 8

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

This will pretty soon be a competitive parity tech-p1 issue, but it's not in the current device tier.  Until then, this is a security feature that puts us above competition.

Comment 9

[Guillaume Destuynder [:kang] [this account is no longer active]]

Glandium, how do you want these patches?
The first file is a patch for bionic, backported from Android 4.1. It's a serie of git concatenated patches.

The second file includes bionic patches as we have copied their linker there - but these parts might not be in use.

It also has a patch to have the correct configure flags, in order to have the b2g and container processes compiled w/ PIE so that ASLR works properly, although it could be made differently (like the --hardened flag proposal, or something of that sort)

Comment 10

[Mike Hommey [:glandium]]

I guess a branch on github would do.

Comment 11

[Guillaume Destuynder [:kang] [this account is no longer active]]

This is what I currently have:
https://github.com/gdestuynder/platform_build/commits/master
https://github.com/gdestuynder/mozilla-central/commits/master
https://github.com/gdestuynder/bionic/commits/master

I haven't sent pull requests, because only mozilla-central is on github, and that change is minimal. I can send one tho. I can also change it. All my commits are appended and committed by github user "kangsterizer". The branch I use on my end is "master" as it's my own repo and I don't have other branches. Those can be pulled into a different branch of course.

Here's how to verify ASLR works:

Find b2g//plugin-container/vold PIDs (or any other process. Note that some process don't support full ASLR yet, even in Android, such as adb server).
Check the maps (cat /proc/<PID>/maps) for base addresses.

For example:

Before ASLR patches:

B2G
00008000-0001f000 r-xp 00000000 1f:05 263        /system/b2g/b2g
0001f000-00020000 rw-p 00016000 1f:05 263        /system/b2g/b2g
0016b000-001ad000 rw-p 00000000 00:00 0          [heap]
400ea000-4012c000 r-xp 00000000 1f:05 760        /system/lib/libc.so
4012c000-4012f000 rw-p 00042000 1f:05 760        /system/lib/libc.so
b0001000-b0009000 r-xp 00001000 1f:05 933        /system/bin/linker
b0009000-b000a000 rw-p 00008000 1f:05 933        /system/bin/linker
befde000-befff000 rw-p 00000000 00:00 0          [stack]

VOLD
00008000-00017000 r-xp 00000000 1f:05 844        /system/bin/vold
00017000-00018000 rw-p 0000f000 1f:05 844        /system/bin/vold
018b1000-018b7000 rw-p 00000000 00:00 0          [heap]
400fd000-4013f000 r-xp 00000000 1f:05 760        /system/lib/libc.so
4013f000-40142000 rw-p 00042000 1f:05 760        /system/lib/libc.so
b0001000-b0009000 r-xp 00001000 1f:05 933        /system/bin/linker
b0009000-b000a000 rw-p 00008000 1f:05 933        /system/bin/linker
bec6d000-bec8e000 rw-p 00000000 00:00 0          [stack]

After ASLR patches:

B2G
400ea000-40101000 r-xp 00000000 1f:05 968        /system/b2g/b2g
40101000-40102000 rw-p 00017000 1f:05 968        /system/b2g/b2g
41c41000-41c4b000 rw-p 00000000 00:00 0          [heap]
40102000-40144000 r-xp 00000000 1f:05 413        /system/lib/libc.so
40144000-40147000 rw-p 00042000 1f:05 413        /system/lib/libc.so
40174000-40187000 r-xp 00000000 1f:05 1021       /system/bin/linker
40187000-40189000 rw-p 00012000 1f:05 1021       /system/bin/linker
be881000-be8a2000 rw-p 00000000 00:00 0          [stack]

VOLD
4003d000-4004c000 r-xp 00000000 1f:05 1067       /system/bin/vold
4004c000-4004e000 rw-p 0000e000 1f:05 1067       /system/bin/vold
4187b000-41881000 rw-p 00000000 00:00 0          [heap]
40082000-400c4000 r-xp 00000000 1f:05 413        /system/lib/libc.so
400c4000-400c7000 rw-p 00042000 1f:05 413        /system/lib/libc.so
4004e000-40061000 r-xp 00000000 1f:05 1021       /system/bin/linker
40061000-40063000 rw-p 00012000 1f:05 1021       /system/bin/linker
be7f0000-be811000 rw-p 00000000 00:00 0          [stack]

Other notes:
1) I haven't patched the linker from mozilla-central on github (only bionic's linker). I'm not sure what it's used for. I checked on my Android device, Firefox is also compiled with PIE-aware linker. The above samples are from a device freshly flashed with the github commits.

2) Merging Android 4.1+ will also provide the necessary non-mozilla-central functionality. (my patches are basically a cherry-pick of these)

Let me know if there's anything else I can do

Comment 12

[Mike Hommey [:glandium]]

The linker in mozilla-central isn't used for executables, so it doesn't need support for PIE.

That being said, mozilla-central changes need to go through the "normal" process, not pull requests, which means you need to attach a patch here in bugzilla and request review for it. Then it will land on mercurial. I'll already note here my review comments for that patch: -pie is a LDFLAGS, not a C*FLAGS. Also, you can't use -fPIE on everything, only on the bits that end up being built in a program. Likewise for -pie, you can only add it to LDFLAGS when linking programs, not shared libraries.

Finally, the platform_build change[1] does more than advertized. It adds source fortification and a few other flags that are not related to PIE. They may be desired, but are a different subject (and -D_FORTIFY_SOURCE=1 adds overhead). It also removes "-Wl,-T,$(BUILD_SYSTEM)/armelf.x" and I doubt you really want to go there.

1. https://github.com/gdestuynder/platform_build/commit/7f6ea97a44dd92e7b829e5bab3cc264ffab6af96

Comment 13

[Guillaume Destuynder [:kang] [this account is no longer active]]

with Android 4.3 coming, the only necessary patch will be the gecko patch when it's cleaned up

Comment 14

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

I observed on a 1.0 release build that we have some randomization for libraries — about 8 bits each — so a return-to-libc (or return-to-libxul) would fail with high probability, even though the base executable is at a known address.  This is what comment 11 shows as the "before".

In light of this, note bug 811671 comment #129 — we seem to be planning to release 1.2 with no ASLR in order to use global prelinking, to share what would otherwise be .data.rel.ro between processes; this means that plain return-to-lib{c,xul,...} would work.  In contrast, 1.3 will have Nuwa, which will provide that sharing regardless, so we can reenable it then.  (In that case every b2g process on a given system at a given time will have the same load addresses, but they will be randomized at boot — assuming, as mentioned in the referenced comment, that we have enough entropy at that point.)

Comment 15

[Dave Hylands [:dhylands]]

So are you assuming that the Nuwa template process never dies?

I would expect that there might be circumstances under which the Nuwa template dies or is killed and would get relannched.

Comment 16

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

Condensing some discussion on IRC:

In addition to what comment #15 mentions, the Nuwa process itself is exec'ed, so that's two copies of the relro segment to start with.

But it's 1.5 MB total, and we should be able to make it unlikely to be killed by the LMK, and I conjecture that the Nuwa process's USS should be relatively small (or could be made so).

Comment 17

[Florian Bender]

Is there anything left blocking this work? Can this continue now, please?

Comment 18

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

Now that bug 857628 has landed, it should be simple to get position-independent executables on JB and up.  Also, all of our post-ICS devices are at least 4.2, which already has the relocatable linker patch [dcbc378].  I think relro and bind-at-load should be separate bugs.

Also, comment #16 is no longer true as of bug 977026; child processes can and usually will have the same address space layout as the parent.  In any case, at this point there shouldn't be any need for device manufacturers to use prelinking; if they are, we should try to determine their concerns and address them, but I think that should also be a separate bug.

[dcbc378]: https://android.googlesource.com/platform/bionic/+/dcbc3787bfb9a272a010

Comment 19

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

Comment on attachment 700766 [details] [diff] [review]
5 patches for gonk ASLR support.

Since we don't need to support Android 4.1, and 4.2 already has the ASLR patches, and 4.0 (ICS) seems to have never been in scope for this bug, it looks like we no longer need this attachment.

Comment 20

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

Attachment: Link to Github pull-request: https://github.com/mozilla-b2g/gonk-misc/pull/205

Try run: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=c342714a4fe2 (using `-gt 15` for the version condition; I've repushed with `-ge 17` to be clearer (≥ vs. >) and to be more conservative about the version requirement; note that Try covers only {15, 18, 19}, I'd need a flatfish or vixen to test 17, and we have no device that's 16 as far as I know).

Comment 21

[Michael Wu [:mwu]]

Comment on attachment 8535467 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gonk-misc/pull/205

Good timing - we actually need this for gonk-l porting.

Can you do this in a gecko build file? configure.in is probably fine.

Comment 22

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

Attachment: bug777948-b2g-pie-hg0.diff

Redone as a patch for Gecko's autoconf bits.  (Minimum version changed back to 16, not that it matters, after rereading the first comments on this bug.)

Comment 23

[Michael Wu [:mwu]]

Comment on attachment 8535872 [details] [diff] [review]
bug777948-b2g-pie-hg0.diff

Review of attachment 8535872 [details] [diff] [review]:
-----------------------------------------------------------------

Don't think we're ever going to support 4.1, but this won't hurt.

Comment 24

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

Try run: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=4319ca88b881

Comment 25

[Michael Wu [:mwu]]

Marking checkin-needed since we want this ASAP for gonk-l support.

Comment 26

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/b2g-inbound/rev/770e5a186d6f

Comment 27

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/770e5a186d6f