fix: allow su operation for SELinux Enforcing systems

[mohankr]

This patch uses virtual resource names for IPC, which is currently not under the purview of
SELinux, for now at least :)

[jcmajor]

Would it be appropriate for su in daemon mode to make an selinux call to set permissions on the socket it created? Would that get it across the line?

[mohankr]

If you could, the constraint I am trying to work against is where the selinux policy cannot be changed (that is, it is embedded in the boot image, like the samsung kernels). The other problem may/may not is the selinux is distributed as a shared lib I believe, we would need to check for whether the system is selinux enabled if you do want to go that route. The abstract resource I think is much cleaner without messing around with the permissions. (currently for locked down boot images we have to "shim" the startup of su in daemon mode anyways, not sure how long even this hack would work, if we the OEM locks that down)..

[koush]

Are abstract resources protected by filesystem permissions? I guess that's a nonissue since the socket peer is identified now. It's been a while since I've digged into this code. Would be good to have other eyes look at this for security problems.

[mohankr]

I don't believe so (it is more of linux thing, BSD for example completely ignores the file system permission for unix domain sockets), hence the patch removes it. Also the current implementation opens up the permissions anyway (0777).
For this particular use case (su daemon), you are probably much better of securing at the communication protocol level (which I believe is being done), then rely on it at the infrastructure level. For a SElinux hardened system, you can apply security at a much for fine grained level for a socket anyways.

[koush]

I think this is a good change. I recall initially wanting to use abstract sockets, but did not since they were not protected by filesystem permissions, which was the original implementation. Since then, also added checking the socket peer credentials as well to allow non-setuid invocation support.

I'll see if I can get a few more eyes on this, as it pretty dramatically changes the internals of how the su daemon communicates with the world :)

[jcmajor]

I think, given the general direction of this app, that, despite the hinderance, SE enforcing represents a significant opportunity. My (limited) reading of the se doco suggests that you can get very fine grained control of access to sockets.

I started a big para about Superuser managing contexts for access to the su binary and then realised all the apps appear as zygote. There might still be something in that.

I looked a bit in the daemon code and can see you have defaulted to denied in places where SE might be configured to deny you access to critical info (like the sockets remote credentials). I haven't looked hard enough yet to say confidently "its good", but it looks to be in reasonable shape. I'll try to review it better this week.

On a slightly different track:

I am currently playing with my 4.3 GS4. I have compiled this patch and put it manually into a system partition, from the providers image, by loop mounting it on linux, then using img2simg and heimdall to get it onto the phone. ** This has not bumped the knox flag. ** I needed to use setfilecon while the image was mounted on linux to give execute rights on the su binary.

I currently get a blank Superuser dialog that flashes almost too fast to read when anything asks for su. The calling app waits until superuser is manually started, and then claims it was denied. I have tried auto allow, no pin, no tick for declared. "There are currently no Superuser app policies". I have one entry in the log for Superuser allow, and three for Titanium backup denied, despite several attempts. I seem to be getting one entry in the log per reboot. Setting auto accept, rebooting and hitting titanium still failed.

For a connectbot local su invoke, logcat shows
su ... su invoked.
su ... starting daemon client 10284 10284
connectbot ... onPause called
connectbot ... onResume called
su ... client exited

Update:
I have root via adb shell. Still looking. I could be blind but it looks like the socket for the interactive response does not get created and does not log an error.

Update2:
Smacking self in head. Forgot to bring patched apk file! Know a whole lot more about workings now though.

[jcmajor]

This patch does not appear to reduce security. I do think I might have found a hole in the original (and this version) though. Not sure how to PM koush. If I'm right, I don't want to post it here till theres a fix.

[koush]

koush @ koushikdutta.com

[Djamal57baraja]

Thank you very much to let me use your Great application.

[koush]

So, Android L dropped, which is in enforcing mode. Connecting to abstract sockets does not work:

I/ShellProcess(14661): Using su...
I/am_on_resume_called(14661): [0,com.koushikdutta.backup.MainActivity]
D/su      (15110): su invoked. 
I/HotwordRecognitionRnr(14582): Hotword detection finished
I/HotwordRecognitionRnr(14582): Stopping hotword detection.
D/su      (15112): starting daemon client 10104 0 
I/auditd  (15109): type=1400 audit(0.0:51): avc:  denied  { connectto } for  comm="su" path=005355504552555345520000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 scontext=u:r:untrusted_app:s0 tcontext=u:r:init:s0 tclass=unix_stream_socket
E/su      (15114): connect failed with 13: Permission denied 

[koush]

root from shell works, but root from other apps results in this connect to abstract socket error. i had a similar issue using abstract sockets for IPC inside a zygote based process in another app.

[koush]

I may need to go back to a FIFO based method.

[koush]

Ugh, apparently fifo doesnt work anymore either. Creating a file as root, and changing ownership to another uid does not work. The new uid still can't access it.

[mohankr]

Yup that is correct..There is way around it by shiming in the
daemon...That's what I did..or obviously create new policy
On Jul 9, 2014 5:48 PM, "Koushik Dutta" [email protected] wrote:

Ugh, apparently fifo doesnt work anymore either. Creating a file as root,
and changing ownership to another uid does not work. The new uid still
can't access it.

—
Reply to this email directly or view it on GitHub
#234 (comment).

[koush]

shimming in the daemon? What do you mean?

[koush]

Well, creating a new policy is not ideal. Requiring a custom boot image is rather annoying.

[mohankr]

Yes, new policy is not ideal as noted before. By "shiming" I meant find a process (there will always be one :) ) that has all the SE Policies that the SU daemon needs for working in a enforcing mode, replace that with a script, that starts both of them. It is of course a hack, it will preserve the custom boot image. The downside is for proprietary roms, it is not future proof. I believe the other SU program/app uses the same approach.
The only realistic option is to use the SElinux API and open up all the policies, I went through the exercise, don't have it handy, with 4.4.2 drop at least for Sammy ROMS, there were way too many policies that needed to be changed. And then of course unless statically compiled, the shared lib must exist on the target ROM.
After shimming I had to do another thing, since samsung's 4.4.2 derivatives the damn /system/bin/sh (ash shell) that is spawned by the daemon, ran into the SELinux enforcing wall, vaguely remember it has permission denied to open a pty (will need to dig it up), of course there is a work around for that too :), might not be an issue for CM or other derivatives.

[eltirol]

Ok

[jockeryeens]

Dd

[toha-toha]

☺️☺️😙😙😙😙😙

[toha-toha]

☺️☺️😙😙😙😙😙

[toha-toha]

☺️☺️😙😙😙😙😙