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.. https://github.com/topjohnwu/Magisk/issues/1477 And https://github.com/topjohnwu/Magisk/commit/bc6a14d30f74a217e6c88835e5cbf452a3d7255b

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: https://github.com/topjohnwu/Magisk/commit/2fb49ad7806bbe50ac60ea223cba93e6cb54a226

References: https://github.com/topjohnwu/Magisk/commit/49f259065d5d2a1d748ddb5c1e38afd365eaa1c1 https://github.com/topjohnwu/Magisk/commit/e841aab9e73cb4ec4b9419c13b27d12b1599cbb3 https://github.com/topjohnwu/Magisk/commit/753808a4ce524f6979dc580fa00dc9baafee620c

[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: https://github.com/kdrag0n/safetynet-fix/pull/166#issuecomment-1073115540

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. :thinking: 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...

[HuskyDG]

I think no way to hide permissive completely. App can just test some thing such as switching context to u:r:magisk:s0, test setprop, etc... On Android 10+, app_zygote can call setuid() syscall if permissive+ seccomp disabled. It's better to use enforcing.

[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

https://github.com/scottyab/rootbeer/commit/091a157959a2de58abc4b51b99fb9189ecd284e2

https://github.com/scottyab/rootbeer/issues/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