magisk: Better hiddens

[MlgmXyysd]

• magisk: Better hidden SELinux Permissive state
• magisk: Spoof residual items in kernel cmdline to make some apps happy

• More tests may be required

[osm0sis]

About the SELinux thing..
topjohnwu/Magisk#1477
And
topjohnwu/Magisk@bc6a14d

I forget the why of it now, but John intentionally switched from setting the prop to 0 (the correct value apparently, per above) to deleting the prop.

[kdrag0n]

Apps aren't normally supposed to be able to read cmdline, so why not chmod 640 /proc/cmdline for permissive users?

[MlgmXyysd]

Apps aren't normally supposed to be able to read cmdline, so why not chmod 640 /proc/cmdline for permissive users?

Both are OK. Which do you think is better?

[MlgmXyysd]

About the SELinux thing.. topjohnwu/Magisk#1477 And topjohnwu/Magisk@bc6a14d

I forget the why of it now, but John intentionally switched from setting the prop to 0 (the correct value apparently, per above) to deleting the prop.

I saw this, but don't understand why.
I can't find any documents about this prop. But some Q&A told me that when SELinux Enforcing, this prop should be 1.

[kdrag0n]

Apps aren't normally supposed to be able to read cmdline, so why not chmod 640 /proc/cmdline for permissive users?

Both are OK. Which do you think is better?

In that case, I'd prefer chmod as it's much simpler and better matches the expected environment.

[osm0sis]

I saw this, but don't understand why. I can't find any documents about this prop. But some Q&A told me that when SELinux Enforcing, this prop should be 1.

Right, so there's definitely conflicting information about whether it should be 0 or 1 for enforcing. Maybe that's why he deleted it instead? My feel is it should stay the way John figured out.

[osm0sis]

Some of those SELinux bind mounts and chmods might interfere with integral new Magisk stuff: topjohnwu/Magisk@2fb49ad

References:
topjohnwu/Magisk@49f2590
topjohnwu/Magisk@e841aab
topjohnwu/Magisk@753808a

[HuskyDG]

How about this?

magiskpolicy --live 'permissive *'
setenforce 1

or add permissive * into sepolicy.rule

getenforce return Enforcing but all context are in permissive mode

[osm0sis]

Live policy patching causes kernel panic on several vendors. Also, no. We should not nuke SELinux. 🙃

[HuskyDG]

When selinux is enforced, app will not have permission to get selinux status, so i think chmod 660 is enough

[osm0sis]

Also perhaps not advisable anymore: #166 (comment)

But, having read into it more now, perhaps could work since all the Magisk sepolicy hijacking stuff is pre-init.. 🤔

[ipdev99]

What if we move it around a bit. 🤔
Skip the bind option.

#!/system/bin/sh

# Some sensitive [secure] properties are dynamic and need to be adjusted (set) during boot and/or after boot complete.
#
# Universal SafetyNet Fix
# kdrag0n @ xda-developers

# Module directory set by Magisk.
MODDIR=${0%/*}

remove_prop() {
    if [[ $(getprop $1) ]]; then
        resetprop --delete $1
    fi
}

# Remove properties.
remove_prop ro.build.selinux

# If selinux is permissive adjust the file permissions.
if [[ $(cat /sys/fs/selinux/enforce) -eq 0 ]]; then
    chmod 0640 /proc/cmdline
    echo "1" > $MODDIR/enforce
    chmod 0640 $MODDIR/enforce
    su -c mount $MODDIR/enforce /sys/fs/selinux/enforce
    chmod 0440 /sys/fs/selinux/policy
fi

[ipdev99]

@kdrag0n

Apps aren't normally supposed to be able to read cmdline, so why not chmod 640 /proc/cmdline for permissive users?

@MlgmXyysd

Both are OK. Which do you think is better?

Looking a little more into it..

Do we need to set cmdline to 0640?

Both enforcing and permissive [ stock and/custom rom(s) ] are set to 0440.
-r--r----- 1 root radio u:object_r:proc_cmdline:s0 0 2022-07-19 20:50 /proc/cmdline

Setting cmdline to 0640 only gives write permission to root.
Unless there is a reason we (as root) need to write to cmdline, would it not be better to reinforce the 0440 permission?
Reset /proc/cmdline to 0440 incase some custom rom sets it different.

[HuskyDG]

And apps will detect permission flag...

[agnostic-apollo]

Came over this yesterday, so thought I should post.

Firstly, ro.boot.selinux should either be 0, 1, or not set. Using enforcing is not correct. Moreover, it seems to only signify if selinux is supported/enabled in kernel, not whether its in enforcing or permissive mode. Check mock configs, its used along with ro.build.selinux.enforce and both are 1.

https://cs.android.com/search?q=content:%22ro.build.selinux%22&start=1

And it seems to be only used for android tv devices Settings app to change selinux mode now and should normally be unset on handheld devices (I don't see any references anymore for selinux in current Settings app source). Root checking apps shouldn't rely on it since it normally wouldn't be set and even if value was set to 0, it would only mean selinux is not supported on the device, which doesn't mean a rooted device for android < 5 at least or devices that are not following google's CTS, although hopefully all production devices are running with it enabled.

https://source.android.com/docs/security/features/selinux#background

https://source.android.com/docs/security/features/selinux/validate#switching_to_permissive

https://cs.android.com/android/_/android/platform/build/+/9d8a51f537cc1191655e0d8edc7eaffde2503ac7

https://cs.android.com/android/_/android/platform/build/+/92ca0197ed22897633ed9241c9f4ae2128ef5c13

https://android-review.googlesource.com/c/platform/packages/apps/Settings/+/32210

https://cs.android.com/android/platform/superproject/+/android-13.0.0_r41:packages/apps/TvSettings/SettingsAPI/java/com/android/tv/settings/library/about/AboutState.java;l=162

https://cs.android.com/search?q=content:%22selinux%22&ss=android%2Fplatform%2Fsuperproject:packages%2Fapps%2FSettings%2F

Actual values can be checked with syscalls, which non privileged apps shouldn't be able to check due to permission denials.

https://cs.android.com/android/platform/superproject/+/android-13.0.0_r41:frameworks/base/core/java/android/os/SELinux.java;l=51

https://cs.android.com/android/platform/superproject/+/android-13.0.0_r41:frameworks/base/core/jni/android_os_SELinux.cpp;l=451

https://cs.android.com/android/platform/superproject/+/android-13.0.0_r41:external/selinux/libselinux/src/enabled.c;l=11

Also if checking with rootbeer, note that it will check if value != 1 to signify rooted devices, which is obviously not right. It was using = 1 before, but "fix" is not available on playstore builds.

https://github.com/scottyab/rootbeer/blob/091a157959a2de58abc4b51b99fb9189ecd284e2/rootbeerlib/src/main/java/com/scottyab/rootbeer/util/Utils.java#L12

scottyab/rootbeer@091a157

scottyab/rootbeer#183

[malikzype]

Can we check this on priority and roll out a new version if everything looks good?

[rehannali]

Bank app checking SELinux as well on S21 Ultra. If everything looks good, when will a new version roll out with SELinux hidden so I can use the app as expected?

[MlgmXyysd]

Firstly, ro.boot.selinux should either be 0, 1, or not set. Using enforcing is not correct. Moreover, it seems to only signify if selinux is supported/enabled in kernel, not whether its in enforcing or permissive mode. Check mock configs, its used along with ro.build.selinux.enforce and both are 1.

ro.boot.selinux should be enforcing or permissive or not set, not 0 or 1.

Because ro.boot. prefix prop is generated by Kernel cmdline and BootLoader androidboot. prefix automatically. Set SELinux Permissive in Kernel cmdline is androidboot.selinux=permissive, so ro.boot.selinux will be permissive instead of 0.

[agnostic-apollo]

ro.boot.selinux should be enforcing or permissive or not set, not 0 or 1.

Because ro.boot. prefix prop is generated by Kernel cmdline and BootLoader androidboot. prefix automatically. Set SELinux Permissive in Kernel cmdline is androidboot.selinux=permissive, so ro.boot.selinux will be permissive instead of 0.

https://cs.android.com/android/_/android/platform/build/+/9d8a51f537cc1191655e0d8edc7eaffde2503ac7

[t0ma5]

how can I edit android.os.SystemProperties to avoid the false positive? Thanks in advance!

[osm0sis]

Necrobump to clear up the confusion above...

You guys were talking about different props:

ro.build.selinux=1 or 0 (but still debatable if either can be considered universally "correct", since it seems to differ across OEMs/devices, per the old Magisk issue I linked)

ro.boot.selinux=enforcing or permissive

[agnostic-apollo]

Ah, damn, you are right. Not sure why I overlooked that. Apologies.