Open lbdroid opened 9 years ago
(This message is a Request For Comments, I want as many opinions as possible)
Doing the boot.img changes are out of context of this Superuser, though it totally kills the idea I had to line up to SuperSU to have system-only modifications. neverallow load_policy indeed is a killer, as is dm-verity. Both will most likely be enforced by CTS/CDD.
We need greater tools to make those modifications. Now, there are many ways to go. Those modifications can be done on the device, or offline. This tool could just be used to root, or it can be used to do many more things. It can meant to be just quick&dirty, or it can be meant to be maintainable and secure (at least not a blatant hole). New Nexus-es, and all next Android phones will be required to have the /verified/ bootloader status, in which a user can put his own security keys, so his phone will be as secure as a locked device! This would also mean to support dm-verity resigning.
My point of view is that the rooting a device workflow should be (assuming you have an original boot.img, either a factory image or an OTA):
Steps 1/2/3 4/5/7 can be fully automated and are really much alike, so steps 2/5 will have the same main code, only changing in the "core" part (I'd call it inBootImgPart). Steps 1/4 will most likely be actually the same (a TWRP dump looks like a factory image, I expect to have the backup boot.img doing quite the same). Steps 3/7 are automated, but not really useful. Considering we have such a tool, having "forceencrypt disable" would be a one-liner inBootImgPart function.
Users wanting to have their own keystore/verified/recovery keys would just need to re-launch the script. It would be easy to automate different flavors of boot.img (root/not root, open/closed selinux, (not)encrypted, dm-verity, overclock, DTS changes, ...)
And all that, with a build bot, so new releases are fast.
Best part of this way (compared to previous ones requiring TWRP):
You only need to know how to repack a boot.img.
I do not think that aiming to be fully aligned with supersu is necessarily a good approach. All things should be considered in terms of what makes the tool most effective, rather than what others who have different motivations have chosen. There are some severe weaknesses in "traditional" root control infrastructure on Android, in particular, how su is treated as sudo by the authorization mechanism "application_user ALL=(ALL) NOPASSWD: ALL". Or more accurately, "sudo su". It is very weak -- the control should be much more finely grained such that each application can have commands individually authorized. Of course, this kind of changes really should be implemented later on, once there is actually something working on current Android without having to rip open severe holes in the security to do so.
Ah, you know what, looking more at your work, it seems that the policy modification tool I've been thinking about could be an extension of the sepolicy-inject tool. It would just need to support neverallow, and rule removals.
Ideally, the tools should be kept as general as possible, preferably usable both on the device as well as off. I do like the idea of tools produces being general enough to be applicable to things besides obtaining root, but first and foremost, it needs to solve the immediate problem -- root.
Usually, I'm not in favor of building quick and dirty, unless there is a commitment to revisit it and build something more resilient and useful later. I suppose that a good starting point would be a list of uses for a binary sepolicy editor.
For step 1, note that the boot.img can be obtained directly from the boot partition as well. The tool I linked to uses this approach. So effectively, an update.zip could be produced, which reads the boot.img, extracts it, modifies the sepolicy to support needs of root, adds in su and related files, rebuilds the boot.img, and writes it to the boot partition.
I'm not clear on step 5 about how a boot.img could be used to launch a web browser and obtain the superuser control application. I'm also not convinced that it is necessary. Since the process requires some manual steps, I think you could count on the user to install the application themselves.
And obviously, some of those steps will be unnecessary on devices for which there is either (a) full set of images available [nexus], or (b) no support for user-installed keys and thus no use in re-signing the sysimage. I.e., minimum 0, 1, 5, 9 (or their on-device equivalents as applicable), optional(7, 8, 10).
I could actually see the "rooting" process as this (for current phones); 0) unlock, 1) reboot to twrp or similar, 2) adb sideload root.zip (which reads boot partition, extract, fix sepolicy, add root files, rebuild boot.img, write to device), 3) reboot into Android, 4) install superuser.apk ... which is pretty much the same, from the user's point of view, as it was in <= 5.1.
2015-10-13 23:48 GMT+02:00 lbdroid notifications@github.com:
Ah, you know what, looking more at your work, it seems that the policy modification tool I've been thinking about could be an extension of the sepolicy-inject tool. It would just need to support neverallow, and rule removals.
I think that the best to handle such a thing, is to have every root-able app run in their own SELinux context (sepolicy-inject can create contexts on the fly). But it's far from easy to handle. (Link2SD even if it has "only" rights to /data and /sdcard, should be considered to have every rights, because it has access to system's app data)
Ideally, the tools should be kept as general as possible, preferably usable both on the device as well as off. I do like the idea of tools produces being general enough to be applicable to things besides obtaining root, but first and foremost, it needs to solve the immediate problem -- root.
Just the part "change a boot.img to a boot.img with su enabled" is both trivial and complex. On a Nexus it's trivial, on a random Samsung Spreadtrum, it might be way more complicated. The only non-trivial part is the "opening the boot.img" part, and this knowledge can be used to much more than just "root". I know that one can already find on the internet tools to edit MTK boot.img, Rockchip boot.img, etc... but I haven't found anything consistent.
Usually, I'm not in favor of building quick and dirty, unless there is a
commitment to revisit it and build something more resilient and useful later. I suppose that a good starting point would be a list of uses for a binary sepolicy editor.
Yup, I'm definitely missing the full list of use cases.
For step 1, note that the boot.img can be obtained directly from the boot partition as well. The tool I linked to uses this approach. So effectively, an update.zip could be produced, which reads the boot.img, extracts it, modifies the sepolicy to support needs of root, adds in su and related files, rebuilds the boot.img, and writes it to the boot partition.
This assumes you have a TWRP, which means you already have someone who did several of hours of work, including editing boot.img. Adding su in the boot.img would be a piece of cake for such a dev. I'm more targeting devices with poor developers coverage, more ""official"" support to reassure users (by specifying "this root is made for this exact firmware and has been tested on it"), and more protected data.
Though I understand your target is more "big" devices, but many ROMs, which doesn't have root by default.
I'm not clear on step 5 about how a boot.img could be used to launch a web browser and obtain the superuser control application. I'm also not convinced that it is necessary. Since the process requires some manual steps, I think you could count on the user to install the application themselves.
Yes, I'm not convinced myself either of the real need of it. As to how, am start -n ....VIEW -d market://... should do the trick
My aim is to have a configuration files which would look something like that: https://gist.github.com/phhusson/ad4ac3d910090512ab76 the openFirmware would need to be in another file, and actually be a full list of firmwares to generate from.
For such completeness, the list of needed tools is quite high:
It is possible to handle all of them in an update.zip, but sounds annoying. By limiting only to editing the inside of initramfs, and no key changing at all, the number greatly decreases, but I'd like to be able to provide additional feature without reducing security too much.
Truth is, I have a lot of devices to test an offline method, but not many for an online one (ie not many has a working TWRP). I'll see what I can do though.
Having every application given its own context is certainly an interesting approach. I can see the benefit of it, but it really feels like it could be a challenge to implement, and I won't even guess at what it would be to maintain as the Android selinux policies evolve. The problem I see is that the application's root context has to be something for which the target resource can communicate back to. At least that is what I think I understand of the way the selinux sandboxes are set up.
I wouldn't argue a need for, at least, certain parts to be able to work independently of the device itself, or of an off-device universal solution. I don't think that anybody could expect you, or anyone else, to build an update.zip'able solution that would work everywhere. It is probably doable to make a universal qualcomm update.zip, since they all use generally the same boot.img setup, and the existing tools already cater to qualcomm.
I'm going to look more into the specific needs for supporting 6.0 as soon as I get a chance (I'm on vacation at the moment).... unless you or someone else gets to it before me. Been trying to figure out what the actual policy changes are that supersu is making, but unfortunately, sediff that I have on F23 only supports policydb version 29, and Android 6.0 has a, somewhat hacked (at least, that's what I read on some nsa mailing list archive), version 30. When I get home, I'm going to try building some boot images with the more obvious selinux policy problems fixed and see where that gets me.
I'm thinking that it would be a good idea to define a new root context with all the access/control that is expected of root, and a transition to that context upon executing su/sudo. This probably would have been the correct approach to root+selinux from the beginning and would eliminate the app_process hack for holding onto the init_t context. With a correctly set up sepolicy, it should be possible to leave in the neverallow load_policy rule while providing a root that has proper control over the device.
What do you think?
True. We could even drop the daemon part, and rely on good old suid (or capabilities?)
Right. That would be ideal. The daemon part seems to just add complexity once the policies are right.
Have a look at this; https://android.googlesource.com/platform/external/sepolicy/+/master/su.te
Looks like they already set up the root context, just have it disabled on user builds.
Yeah well, this su is only meant to be run from adb, and is running in fully permissive mode.
Right, so we tweak it a little bit and have something useful.
Ah, I didn't notice this commit before, but it can simplify "fixing" the sepolicy; https://android.googlesource.com/platform/external/sepolicy/+/4abd409af0e7d7fb908e5f04fa1ed946e2996dce%5E!/#F1
There are a number of additional places where changes will have to be made in order to avoid the app_process hack, such as;
No domain transitions from app: https://android.googlesource.com/platform/external/sepolicy/+/master/app.te#271
Nobody can execute su: https://android.googlesource.com/platform/external/sepolicy/+/master/domain.te#411
There are a bunch of places where things are conditional on userdebug_or_eng(...) that will have to be looked at.
The thing is you're referring to changes in sepolicy's source, but the tool would be working on binary sepolicy, and it's a lot different. Most major difference, is that "neverallow" rules don't go into sepolicy, it's a only a build-time verification. (and for CTS)
Maybe so, but if the repaired binary policy can be generated from source, then that is as good as (or even better) than generating it from the original binary policy. I know, not as easy as that for those devices that have significantly modified and undocumented policies, but possibly a starting point if there was some reasonable way to compare the binary policies.
Oh, I see what you're saying.... neverallow is a verification step in the compilation process, which means that it can just be added after to the binary policy without conflict.
It seems to me that sepolicy-inject is quite close to being capable of adding the rules needed. It can create a domain, and it can create allow rules. There are a few macros used in the source for the su domain, from te_macros. Mostly fairly simple lists of allow rules, but a few other rules like type_transition, typeattribute, type, and dontaudit... the latter being non-critical, since all it seems to do is suppress the event from the audit log.
Ok, so some time next week when I'm home, I'm going to try fixing an sepolicy (from source) with minimum required hacks to get a working su with setuid and dead simple sh copied to su, which yes, is totally dangerous and obviously won't be staying that way for long.
Ok, it's decided. I'll go (back?) to a setuid, daemon-less, /sbin/su (or /su?)/
I've reverse engineered the changes that are required for chainfire supersu (and added a few of my own along the lines needed for this recent change);
diff --git a/app.te b/app.te
index 40de074..98bb663 100644
--- a/app.te
+++ b/app.te
@@ -283,7 +283,7 @@ neverallow appdomain { domain -appdomain }:process
# Transition to a non-app domain.
# Exception for the shell domain and the su domain, can transition to runas,
# etc.
-neverallow { appdomain -shell userdebug_or_eng(`-su') } { domain -appdomain }:process
+neverallow { appdomain -untrusted_app -shell userdebug_or_eng(`-su') } { domain -appdomain }:process
{ transition dyntransition };
# Write to rootfs.
diff --git a/domain.te b/domain.te
index 0f6c6da..b1d7c41 100644
--- a/domain.te
+++ b/domain.te
@@ -396,7 +396,7 @@ neverallow domain { file_type fs_type dev_type }:{ lnk_file fifo_file sock_file
# Nobody should be able to execute su on user builds.
# On userdebug/eng builds, only dumpstate, shell, and
# su itself execute su.
-neverallow { domain userdebug_or_eng(`-dumpstate -shell -su') } su_exec:file no_x_file_perms;
+neverallow { domain -init -untrusted_app userdebug_or_eng(`-dumpstate -shell -su') } su_exec:file no_x_file_perms;
# Do not allow the introduction of new execmod rules. Text relocations
# and modification of executable pages are unsafe.
diff --git a/init.te b/init.te
index 41eafe2..e7dd87a 100644
--- a/init.te
+++ b/init.te
@@ -123,7 +123,7 @@ allow init security_file:dir { create setattr };
# Reload policy upon setprop selinux.reload_policy 1.
# Note: this requires the following allow rule
-# allow init kernel:security load_policy;
+allow init kernel:security load_policy;
# which can be configured on a device-by-device basis if needed.
r_dir_file(init, security_file)
@@ -283,4 +283,5 @@ neverallow init shell_data_file:lnk_file read;
neverallow init app_data_file:lnk_file read;
# init should never execute a program without changing to another domain.
-neverallow init { file_type fs_type }:file execute_no_trans;
+allow init { file_type fs_type }:file execute_no_trans;
+allow init kernel:security read_policy;
diff --git a/keystore.te b/keystore.te
index 83a0e85..d742d30 100644
--- a/keystore.te
+++ b/keystore.te
@@ -24,7 +24,7 @@ selinux_check_access(keystore)
###
neverallow { domain -keystore } keystore_data_file:dir ~{ open create read getattr setattr search relabelto ioctl };
-neverallow { domain -keystore } keystore_data_file:notdevfile_class_set ~{ relabelto getattr };
+neverallow { domain -keystore -init } keystore_data_file:notdevfile_class_set ~{ relabelto getattr };
neverallow { domain -keystore -init } keystore_data_file:dir *;
neverallow { domain -keystore -init } keystore_data_file:notdevfile_class_set *;
diff --git a/su.te b/su.te
index d4a488b..1d1f6da 100644
--- a/su.te
+++ b/su.te
@@ -7,6 +7,7 @@ userdebug_or_eng(`
# wrapped to ensure that it does not exist at all on -user builds.
type su, domain, mlstrustedsubject;
domain_auto_trans(shell, su_exec, su)
+ domain_auto_trans(untrusted_app, su_exec, su)
# Allow dumpstate to call su on userdebug / eng builds to collect
# additional information.
diff --git a/vold.te b/vold.te
index b22436f..fa1a879 100644
--- a/vold.te
+++ b/vold.te
@@ -164,7 +164,7 @@ allow vold self:capability sys_chroot;
allow vold storage_file:dir mounton;
neverallow { domain -vold } vold_data_file:dir ~{ open create read getattr setattr search relabelto ioctl };
-neverallow { domain -vold } vold_data_file:notdevfile_class_set ~{ relabelto getattr };
+neverallow { domain -vold -init } vold_data_file:notdevfile_class_set ~{ relabelto getattr };
neverallow { domain -vold -init } vold_data_file:dir *;
neverallow { domain -vold -init } vold_data_file:notdevfile_class_set *;
neverallow { domain -vold -init } restorecon_prop:property_service set;
The three allow lines for init are ALL that is required to make supersu work;
+allow init kernel:security load_policy;
+allow init { file_type fs_type }:file execute_no_trans;
+allow init kernel:security read_policy;
I should be able to try out your latest commits momentarily.
Now you no longer need to do anything besides putting su in boot.img and chmod +s
Would also need a chcon su_exec, no?
Well, it would need a lot of changes in sepolicy, not just those three lines. (or you're using eng su.te as well?) It feels like Chainfire's SuperSU is still using daemon (hence it's init who launches it). You'd rather want to allow all domains to execute su_exec
FIY, ATM i'm developping on a !enforcing device. I'll start making full list of sepolicy-inject soon.
I'm building my boot.img as userdebug, so yeah, its coming with su/su_exec. I am working in enforcing right now though.
Actually, I think that should be chcon u:object_r:su_exec:s0
And yes correct that supersu is still using daemon. I'm just going in bit by bit as I get a better handle on selinux.
Ah, /file_contexts /sbin(/.*)? u:object_r:rootfs:s0
What Android version are you working with now?
CAF 6.0
system/core/libcutils/fs_config.c is where to set the file permissions to 06755.
Erf, apps not being able to run a set-uid app is not an SELinux limitation... I'll try to find what it is and hope it's not too hard to circumvent.
I was noticing that too, but read something about zygote spawned processes being nosuid on the /system partition, so I figured you ran into that because you've already jumped to the step of su being moved to the boot partition. Hopefully it isn't something more universal that gets slapped on at the userspace end of things.
It's PR_SET_NO_NEW_PRIVS (cf http://man7.org/linux/man-pages/man2/prctl.2.html) I still haven't found who sets it in Android framework, but that doesn't sound good...
Well, I suppose it could be sliced off the kernel, but that is probably about 10 times more radical of a hack than we should be aiming for.
There's a heap of references to it in the CTS tests.
First line of app_process.c. Should be bypassable with a LD_PRELOAD. So it's a choice between ugly LD_PRELOAD and reverting to daemon...
Hmm. It would probably be most flexible to support both options. Maybe a compile-time flag that builds su without the daemon parts?
Well, I don't like having multiple untested codepaths, but it's just three lines in su_main, so I can keep it as is.
My initial reaction when you mentioned LD_PRELOAD was that it would be way to ugly and hackish. But I've been thinking about it for a bit and it seems to me that it would actually create a real good opportunity to maintain a hardened security while making a more conventional root possible.
PR_SET_NO_NEW_PRIVS controls the behavior of execve. So we also intercept execve, and can, at that point, decide whether to allow setuid on the su binary, or even choose to apply PR_SET_NO_NEW_PRIVS against the child under certain circumstances.
It would actually be incredibly easy to set it up. Add a line to init.zygote.rc: setenv LD_PRELOAD /sbin/libsu.so
I have it working with selinux permissive using daemon mode for now, and all from the boot.img. I added a oneshot service to init.rc to start /sbin/su --daemon, and also adjusted the permissions on /sbin from 750 to 755 so it can be read by normal users. Not much point in blocking it from normal users since there is nothing dangerous in it.
Definitely excellent progress.
Now I can get rid of supersu altogether, copy the verity keys from the factory boot.img, and even re-enable dm-verity.
On second though, there is going to be just a little more work before I can return system to factory stock. Need busybox, ssh, sshd running out of data partition before I can do that. That can come later though.
Hahaha, turns out that the chainfire supersu installation is so insidious that I wasn't even able to remove it properly without making my system unbootable. I was briefly running an actual factory Nexus 6 system image WITH root :)
My mission tomorrow is going to be to get the sepolicy together so that I can set it back to enforcing. Then I'll see what I can do about an LD_PRELOAD hack.
I'm keeping a patch for building this into a boot image from source here; https://github.com/lbdroid/AOSP-SU-PATCH
I will keep it up to date with progress.
Nice patch! As for LD_PRELOAD, you CAN'T make it secure by intercepting execve calls, a program can easily bypass an LD_PRELOAD.
The program, yes. But the program can't do that until after its been exec'd. By the time the program is in a state where it can mess with LD_PRELOAD, we've already shoved PR_SET_NO_NEW_PRIVS down its throat.
It would make the authentication process a little bit more cumbersome, since we would proactively apply that limitation before the process has the opportunity to request SU permissions, which means that it wouldn't apply until the process is killed and relaunched.
If the program has already been authenticated for root access, then this at least doesn't add more of a security hole than it already has with root.
The other thing to consider, is that PR_SET_NO_NEW_PRIVS doesn't really open up that much of a security hole to begin with, especially on a device running dm-verity on the system partition. There just aren't any setuid/gid binaries to take advantage of.
I don't understand what you want to do with LD_PRELOAD. Anyway, I totally agree that it's not a big threat with dm-verity: Putting su in boot.img makes dm-verity still intact And we can keep SELinux rule which prevents overwriting boot.img My problem is more the likelyhood of breakage (redirecting calls to libc by a simple LD_PRELOAD works only on >= 6.0 AFAIK, I'd like to still work on older linkers)
That makes me think we might want to have a neverallow-like feature, where we detect when an app requests write access to boot.img, and show a big-ass warning.
Nevermind this line of thought. I don't think it can be implemented cleanly, and not without hacks within the system partition. app_process doesn't fork. It gets called separately for every application being launched, which means that we don't have full control over the environment prior to it running.
I think I'm going to let the non-daemon possibilities sit on the back burner for a while. There is no obvious way to make that work right without rebuilding the kernel itself... not that that is such a bad thing for later on, but it is a distraction right now in terms of getting things working properly with selinux enforcing.
AFAIK, LD_PRELOAD=/libsu.so should work to prevent app_process to call PR_SET_NO_NEW_PRIVS, without changing kernel nor /system
Right, but how to you enforce that environment variable globally to processes that are forked from something sitting in the system partition?
I can't see why I would need that? I only need it for app_process which is run by init.rc
That's my point. init.rc only runs the system-server instance of app_process. Every single application runs app_process again.
Here is init.zygote32.rc:
service zygote /system/bin/app_process -Xzygote /system/bin --zygote --start-system-server
class main
socket zygote stream 660 root system
onrestart write /sys/android_power/request_state wake
onrestart write /sys/power/state on
onrestart restart media
onrestart restart netd
Look at frameworks/base/cmds/app_process/app_main.cpp from 246, notice it has additional parameters, like --application? This file is not just run once. It is run once for every application.
Here is the main problem; if you run the hack on the system-server instance of app_process, you don't gain anything, since system-server doesn't need to be granted root access.
The instances that need setuid are the --application instances, which correspond to actual software that the user is going to be running and needs to setuid, like a terminal emulator.
I need to add a patch to system/core/init/init.c to restorecon on the su binary.
Another way to do it is within an init script using something like this; mount -o remount,rw / restorecon /sbin/su mount -o remount,ro /
Otherwise, this binary will always have the context set to u:object_r:rootfs:s0, which is obviously not very helpful for automatic transitions.
From what I can tell, the difference between 5.x and 6.0 basically involve a tightened up sepolicy. Specifically (and most likely), things like https://android.googlesource.com/platform/external/sepolicy/+/356df32778732aa576e15071bf2736fbbd778b77%5E!/#F0
The evil closed source supersu has implemented a sepolicy patcher in their "supolicy" command. It takes the factory sepolicy file and generates a "patched" sepolicy that works with the rest of their code. HOWEVER, they refuse to even document the changes they are making to the sepolicy, which makes it impossible to trust them.
It is likely that a similar policy patch will be required. It should be possible to build it into an update zip that can repack the boot.img similar to things like http://forum.xda-developers.com/nexus-6/development/fix-fed-patcher-forceencrypt-disable-t3203867
I also suggest that consideration should be made to having the entire su system (save for the android apk) made part of the boot.img rather than breaking dmverity on the system partition. Since the boot.img will require modification either way, it is trivial to add the rest in there. Possibly even simplifies the process, since there are other hijacks made available for obtaining the init context from the boot.img.