Skip to content

Commit

Permalink
Only remove gms if Denylist is enforced
Browse files Browse the repository at this point in the history 
Originally Magisk required the Denylist to be enforced to access the Denylist.
When enforced, Magisk is unloaded while the processes on the Denylist are called.

Now you can access the Denylist when it is not enforced.
Since Magisk runs normally when not enforced, the Denylist is just a list.

No need to remove 'gms' from the Denylist when it is not enforced.
  • Loading branch information
@ipdev99 @kdrag0n
ipdev99 authored and kdrag0n committed Mar 11, 2022
1 parent 7238dd7 commit 0f35514
Showing 1 changed file with 4 additions and 2 deletions.
6 changes: 4 additions & 2 deletions magisk/post-fs-data.sh
View file
@@ -1,4 +1,6 @@
#!/system/bin/sh

# Remove Play Services from DenyList, otherwise the Zygisk module won't load
magisk --denylist rm com.google.android.gms
# Remove Play Services from the Magisk Denylist when set to enforcing.
if magisk --denylist status; then
magisk --denylist rm com.google.android.gms
fi

6 comments on commit 0f35514

@pndwal
Copy link

@pndwal pndwal commented on 0f35514 Mar 19, 2022 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand USNF removes these processes at bootup to prevent conflicts:
- magisk: Remove Play Services from DenyList
The Zygisk module will never load if GMS is in the DenyList. Instead, we have the module force-enable DenyList unmounting after forking.
cd84bf3
also
- magisk: Remove Play Services from DenyList earlier
This ensures that GMS will never start before it's removed from the DenyList, even if another module's service.sh is blocking our script. Suggested by @osm0sis
bc27f22

Since USNF is doing targeted hiding of SafetyNet process whether denylist is on or off to pass SafetyNet, I think we need to be aware that this change may potentially cause conflicts...

Currently many users are running Canyie's Shamiko for hiding root from bank and other apps. Shamiko also does its own hiding and doesn't use denylist functionality but hijacks/borrows this for convenience; it acts on the processes listed.

If Shamiko is loaded it requires denylist to be disabled (not enforced) but still uses the list, simply replacing denylist's modification reversal with it's own hiding functions including hiding isolated process leaks, zygisk etc. for listed processes.

So we'd have both USNF and Shamiko acting on GMS if it's processes were selected in denylist, even when disabled. I'm not sure which module would process GMS first, but it seems to me that this change could potentially undo the protection the commits above provided.

Further, if gms is not removed at boot-time because denylist is 'off', it seems to me users only need to switch it on to break things... at least the way things are now, broken USNF will be fixed on next reboot and adding installing Shamiko will work going forward.

With this change it would potentially break USNF anytime a user installed Shamiko and simply forgot they had gms in denylist, even when they understand the issue!

The current setup allows a symbiosis between two essential hiding modules. With more and more using Shamiko as banks get cleverer, not breaking things with Shamiko and USNF loaded seems important...

@osm0sis
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you tested this change to see if it does indeed cause a conflict, or is this all supposition?

@pndwal
Copy link

@pndwal pndwal commented on 0f35514 May 18, 2022 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Supposition; I'm only an amateur sleuth, probably way out of my depths...

But it seems there is reason for concern as the premise "Since Magisk runs normally when not enforced, the Denylist is just a list" does not hold true when Shamiko is enabled (Shamiko will hide 'root' from / deny root to gms / other processes in denylist).

Further, I wanted to do a simple test, but on my device I cannot break SafetyNet even by adding gms in denylist (enforced), deleting post-fs-data.sh file and rebooting; USNF is clearly working (S/N basic attestation, full pass. When not loaded I get both basic integrity and CTS profile match failures)... I'm assuming therefore that the "if" in the statement "The Zygisk module will never load if GMS is in the DenyList" is dependent on timing and may vary per device; ie. if denylist loads first gms will break USNF (?)

I'm not sure how to go about testing w/ Shamiko active since the litmus test, attempting to break USNF w/ denylist enforced, fails...

Really, I was hoping someone with more skill than me would check that this change won't in fact cause issues with Shamiko active and sourcing denylist... And I'd be happy to learn what I'm missing. 😛

@osm0sis
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So someone should test. @ipdev99, any thoughts?

@ipdev99
Copy link
Contributor Author

@ipdev99 ipdev99 commented on 0f35514 May 19, 2022 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you take a look back at the PR, I mentioned I had gms in the denylist when not enforced for a few weeks at that time.
#150 (comment)

Tested this again tonight.
There was a slight difference tonight than when I was originally testing this PR.

I am currently testing other things on my Pixel 5a, so I checked using my Pixel 6.

Stock (12L) May 2022.
Magisk (commit 34dded3)
Busybox, Nano, Shamiko, Systemless Hosts, USNF and Zygisk-LSPosed Modules.

My DenyList is only:
Momo, com.google.android.gms and com.google.android.gms.unstable

Test one.
Cold Boot.
Denylist is not enforced.
com.google.android.gms and com.google.android.gms.unstable in the Denylist.

  • SafetyNet is passing.
  • Change Denylist to enforced.
  • SafetyNet is passing.
  • Reboot.
  • 'gms' is removed from Denylist during early boot.
  • SafetyNet is passing.

Test two.
Cold Boot.
Denylist is enforced.
com.google.android.gms and com.google.android.gms.unstable are not in the Denylist.

  • SafetyNet is passing.
  • Add com.google.android.gms and com.google.android.gms.unstable to the Denylist.
  • SafetyNet Fail CTS.
  • Change Denylist to not enforced.
  • SafetyNet Fail CTS.
  • Reboot.
  • 'gms' is not removed from Denylist during early boot.
  • SafetyNet is passing.

Note:

Cold Boot refers to boot the device from a powered off state.

SafetyNet only failed 'CTS' since the non-zygisk part of the module was still active.
The boot-time changes to set the sensitive/secure properties were still set to the safe value(s).
Only the Zygisk part of the module was disabled when SafetyNet was checked ('gms' was called).
So SafetyNet used Hardware attestation instead of Basic.

The slight difference between when I originally made the PR for this change and now.
Changes in Magisk Zygisk and Denylist have made a bit of a difference.
Before, SafetyNet would fail after enforcing the denylist in test one just like it does in test two after adding 'gms' to the denylist.

Shamiko will still be enabled/disabled immediately by switching DenyList to not enforced/enforced.
No reboot needed.

It still does not make a difference if gms is in the denylist or not for Shamiko.
#150 (comment)

If it becomes an issue, Shamiko could very well just add a post-fs script to do the same when not enforced.

if ! magisk --denylist status; then
	magisk --denylist rm com.google.android.gms
fi

Hope that all makes sense. 🙃

Cheers. 🤠

@pndwal
Copy link

@pndwal pndwal commented on 0f35514 May 19, 2022 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup; thankyou very much for checking this. 🙂

I've just revisited this, and was actually able to do some meaningful tests w/ Shamiko & denylist not enforced after discovering what really breaks USNF on my device. - I can confirm that Shamiko hiding of gms, including SafetyNet process (com.google.android.gms.unstable) does NOT break USNF. It seems clear that denying root / modifications, which can break USNF, affects processes quite differently in comparison with current Shamiko hiding.

Previously (using Redmi Note 8T, stock android 10 MIUI) I had simply enforced denylist, added gms main process only (com.google.android.gms.unstable) since Dev states "The Zygisk module will never load if GMS is in the DenyList", removed post-fs-data.sh file (to prevent removal of gms processes during boot), and rebooted. This did not break USNF however; it remained functional (S/N full pass).

Today I tested with more processes added to denylist, and discovered that only the SafetyNet process itself (com.google.android.gms.unstable) breaks USNF on my device. Adding all processes except com.google.android.gms.unstable doesn't break USNF...

I'm guessing that adding the main process (com.google.android.gms) may also break USNF for devices without Magisk in /sbin (Android 11+ etc)...

With USNF broken (com.google.android.gms.unstable in denylist / no fake keystore registered) 'litmus test' accomplished, I simply deselected Enforce denylist to enable Shamiko hiding on rebooting, and found USNF works again to allow CTS profile match to pass...

Especially since there seems to be no current conflict in fact, I agree that in the (unlikely) event that Shamiko hiding functionality does change and create one, "Shamiko could very well just add a post-fs script to do the same when [denylist is] not enforced."

Very sorry if this was a futile exercise folks... 🙃

As an aside, perhaps these statements could be clearer:

- magisk: Remove Play Services from DenyList

The Zygisk module will never load if G̶M̶S̶ [GMS SafetyNet process] is in the DenyList. Instead, we have the module force-enable DenyList unmounting after forking.
cd84bf3

and

- magisk: Remove Play Services from DenyList earlier

This ensures that GMS [processes] will never start before ̶i̶t̶'̶s̶ ̶ [being] removed from the DenyList, even if another module's service.sh is blocking our script. Suggested by @osm0sis
bc27f22

Please sign in to comment.