Skip to content

Commit

Permalink
update sepolicy
Browse files Browse the repository at this point in the history
  • Loading branch information
@kotori2
kotori2 committed Nov 25, 2020
1 parent 4570d8f commit 8482237
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 11 deletions.
17 changes: 6 additions & 11 deletions edxp-core/template_override/post-fs-data.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,17 +41,12 @@ PATH_PREFIX="/data/user_de/0/"
#PATH_PREFIX_LEGACY="/data/user/0/"

sepolicy() {
# necessary for using mmap in system_server process
# read configs set in our app
# for built-in apps // TODO: maybe narrow down the target classes
# read module apk file in zygote
# TODO: remove coredomain sepolicy
supolicy --live "allow system_server system_server process { execmem }"\
"allow system_server system_server memprotect { mmap_zero }"\
"allow coredomain coredomain process { execmem }"\
"allow coredomain app_data_file * *"\
"attradd { system_app platform_app } mlstrustedsubject"\
"allow zygote apk_data_file * *"
# Should be deprecated now. This is for debug only.
supolicy --live "allow system_server system_server process execmem" \
"allow system_server system_server memprotect mmap_zero" \
"allow zygote app_data_file dir { search read open }" \
"allow zygote app_data_file file { getattr read open }" \
"allow zygote app_data_file dir { getattr search read open }"
}

#if [[ ${ANDROID_SDK} -ge 24 ]]; then
Expand Down
1 change: 1 addition & 0 deletions edxp-core/template_override/sepolicy.rule
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,4 @@ allow system_server system_server process execmem
allow system_server system_server memprotect mmap_zero
allow zygote app_data_file dir { search read open }
allow zygote app_data_file file { getattr read open }
allow zygote app_data_file dir { getattr search read open }

41 comments on commit 8482237

@C3C0
Copy link
Contributor

@C3C0 C3C0 commented on 8482237 Dec 5, 2020

Choose a reason for hiding this comment

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

Is there any reason for removing attradd { system_app platform_app } mlstrustedsubject ? This prevents many modules from reading module preferences during zygote init.

@yujincheng08
Copy link
Contributor

@yujincheng08 yujincheng08 commented on 8482237 Dec 5, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

We provide a new way to read module preferences in #703

Reading /data/user_de in system server or normal app processes is not recommended now.

@kotori2
Copy link
Contributor Author

@kotori2 kotori2 commented on 8482237 Dec 5, 2020

Choose a reason for hiding this comment

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

Reading config files directly from apps data folder is a bad behavior. Any app can detect/read your current config.

@C3C0
Copy link
Contributor

@C3C0 C3C0 commented on 8482237 Dec 5, 2020

Choose a reason for hiding this comment

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

That rule allowed reading for system/platform apps such as SystemUI. Not normal apps. Is there a documentation for this new way of reading preferences?

@yujincheng08
Copy link
Contributor

@yujincheng08 yujincheng08 commented on 8482237 Dec 5, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

  1. Change ur module to target xposed api 93.
  2. Then EdXposed will hook thegetSharedPreference of ur module to the new public folder
  3. In other apps, when you use XSharedPreference to access configurations of ur module, it will redirect to the new public folder

In short, as long as you didn't use some magic in writing & reading configurations, the only thing you need to do is to change the target xposed api to 93.

See 7471143, 0721077, and b236247

@C3C0
Copy link
Contributor

@C3C0 C3C0 commented on 8482237 Dec 5, 2020

Choose a reason for hiding this comment

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

But this would force all the users to update exposed to the latest which is still canary. One thing that is not clear. Your sepolicy rules still allow reading app data file for zygote. My module reads the preferences zygote only. Those are the automatically propagated to new processes forked from zygote. Given those rules such reading in zygote no longer works despite sepolicy rules indicate it should be possible. Where's the catch?

@MlgmXyysd
Copy link
Member

Choose a reason for hiding this comment

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

If users do not have updates, they can still use it by simply enabling Skip Check in the manager(or installer) settings

@yujincheng08
Copy link
Contributor

Choose a reason for hiding this comment

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

Zygote will no longer load modules now in regard to the detection and the upcoming built-in Riru in Magisk.

@C3C0
Copy link
Contributor

@C3C0 C3C0 commented on 8482237 Dec 5, 2020

Choose a reason for hiding this comment

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

Where's that "public" folder for preferences? Doesn't it mean that any app can read preferences when they are "public"? Or does "public" have other meaning?

@yujincheng08
Copy link
Contributor

@yujincheng08 yujincheng08 commented on 8482237 Dec 5, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

public means all apps have readding accessibility.

@C3C0
Copy link
Contributor

@C3C0 C3C0 commented on 8482237 Dec 5, 2020

Choose a reason for hiding this comment

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

This contradicts what kotori wrote above
"Reading config files directly from apps data folder is a bad behavior. Any app can detect/read your current config."

@MlgmXyysd
Copy link
Member

@MlgmXyysd MlgmXyysd commented on 8482237 Dec 5, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Some applications can use the public file to detect the framework or module and refuse to work, like bank apps, or SafetyNet

@yujincheng08
Copy link
Contributor

Choose a reason for hiding this comment

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

Its name is randomized. They cannot brute-force the name.

@C3C0
Copy link
Contributor

@C3C0 C3C0 commented on 8482237 Dec 5, 2020

Choose a reason for hiding this comment

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

OK. I'll check. The thing is I have some special handling of preferences in my module especially to ensure correct file permissions. https://github.com/GravityBox/GravityBox/blob/q/GravityBox/src/main/java/com/ceco/q/gravitybox/WorldReadablePrefs.java

@yujincheng08
Copy link
Contributor

@yujincheng08 yujincheng08 commented on 8482237 Dec 5, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

As long as you are using getSharedPrefence to access preference, those are okay because they don't affect the file in the public dir. You are just manipulating some irrelevant files.

@C3C0
Copy link
Contributor

@C3C0 C3C0 commented on 8482237 Dec 5, 2020

Choose a reason for hiding this comment

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

Also just realized my module has functionality to backup/restore the settings and I believe other modules have such functionality as well. Not sure how this is gonna work.

@yujincheng08
Copy link
Contributor

@yujincheng08 yujincheng08 commented on 8482237 Dec 5, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Aha, the configurations won't be deleted even if you remove the modules. (it's still under consideration).

But something user wants to migrate the configurations. In that time, maybe you can grab the mFile by reflection (but not recommended).

I think the best way is that EdXposed manager provides a way to backup/restore...

edit: if your backup/restore is not using manipulating raw files, that should work, btw.

@C3C0
Copy link
Contributor

@C3C0 C3C0 commented on 8482237 Dec 6, 2020

Choose a reason for hiding this comment

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

Well. It basically copies the whole xml file from external storage. Shouldn't be a problem provided module can somehow get the absolute path of its "public" folder.

Btw, not sure if such hiding mechanisms are worth the effort. If I wanted to detect xposed I would simply use package manager to scan manifest metadata of apps on the device. Yes, this does not mean xposed is active but I'm quite sure those who want to detect apply this strict rule that it is enough to find any xposed module installed.

@kotori2
Copy link
Contributor Author

@kotori2 kotori2 commented on 8482237 Dec 6, 2020 via email

Choose a reason for hiding this comment

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

@yujincheng08
Copy link
Contributor

Choose a reason for hiding this comment

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

I can definitely disable that app from reading the app list.

@C3C0
Copy link
Contributor

@C3C0 C3C0 commented on 8482237 Dec 6, 2020

Choose a reason for hiding this comment

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

I can definitely disable that app from reading the app list.

Not sure if this approach is good. App can have legit functionality related to scanning apps on the device except for metadata checking. This would prevent it from working.

@Mikanoshi
Copy link

@Mikanoshi Mikanoshi commented on 8482237 Dec 6, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I'm currently using stock getSharedPreferences to write module's settings and then read them from shared_prefs.xml directly in initZygote.
If targeting api 93 how to know that 93 EdXposed is actually used? or just try to read the old way, if it fails then use XSharedPreferences?
I need backward compatibility, XSharedPreferences were broken in older versions.

I save backup to sdcard as a map object and then restore it using stock SharedPreferences, so if EdXposed 93 saves prefs to some magical public folder that can be accessed even by the least privileged process then I'm all for it :)

@yujincheng08
Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, that's what we want to do to recover XSharedPreferences.

And definitely, a fallback is required when XSharedPreferences is not working. One way is to use XSharedPreferences and use getFile to check if it's readable.

@C3C0
Copy link
Contributor

@C3C0 C3C0 commented on 8482237 Dec 20, 2020

Choose a reason for hiding this comment

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

Another problem I just realized. My module is large and some sub-parts of it have their own preference files for optimization purposes - not to read one huge preference file when I need only sub-part specific settings. Thus module has total of 4 preference files. It looks like this won't work when using new v93 XsharedPreferences?

@Mikanoshi
Copy link

Choose a reason for hiding this comment

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

Are those 4 just different names for SharedPreferences? Then it should work I guess.

It will still be reading and parsing xml every time any process starts. There should be a better replacement for data storage that is available to any mod. Load once and then have fast access to it.

@kotori2
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There should be a better replacement for data storage that is available to any mod.

/data/misc/GB_{random}

@Mikanoshi
Copy link

Choose a reason for hiding this comment

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

I meant memory where you store 1 copy of data, not loading it from file each time. Like ContentProvider that serves SharedPreferences, but that starts earlier. It's about hooks in system_server that are applied before system has finished loading, e.g. if you need to hook a service constructor.

@Mikanoshi
Copy link

Choose a reason for hiding this comment

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

What about adding new service inside system_server that can be accessed from everywhere?
It could use XSharedPreferences to load prefs once and then make them available for other processes.
SELinux allows this only for predefined services in the ROM, so it will require additional supolicy rule.

@yujincheng08
Copy link
Contributor

@yujincheng08 yujincheng08 commented on 8482237 Dec 22, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

There's a riru module named storage redirection. It starts its own app_process for a new service without extra sepolicy.
But it's future work. We will wait until the next release of Shizuku.

@Mikanoshi
Copy link

Choose a reason for hiding this comment

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

What's in a next Shizuku version? Can its service be used as a data storage?

@yujincheng08
Copy link
Contributor

Choose a reason for hiding this comment

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

Yes. And it can provide a root binder for apps. But it might start after system_server process. So I doubt if it works for the modules hooking system server.

@Mikanoshi
Copy link

Choose a reason for hiding this comment

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

Maybe I could find a predefined unused service in MIUI :) or just make my Magisk module with required supolicy.
I really don't like the idea of loading prefs xml on each process start.
BTW as app list mode makes initZygote execute for each process, can I move XSharedPreferences from there to handleLoadPackage to only call it for required packages?
They are executed with the same privileges but on different steps of process creation, right?

@yujincheng08
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe I could find a predefined unused service in MIUI :)

You can use SerialService, which we consider to use before. But take care because someone might also be hooking it.

can I move XSharedPreferences from there to handleLoadPackage

Definitely yes. But sometimes you still want to do something in initzygote when you are hooking system server. And from the wiki, loading prefs lazily is recommended by us. And try to split configuration files into multiple files for better Fine-graineness.

@Mikanoshi
Copy link

Choose a reason for hiding this comment

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

LOL
E/SELinux: avc: denied { find } for service=serial pid=28958 uid=10216 scontext=u:r:untrusted_app_27:s0:c512,c768 tcontext=u:object_r:serial_service:s0 tclass=service_manager permissive=0
Regular apps cannot access that service.

@yujincheng08
Copy link
Contributor

Choose a reason for hiding this comment

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

Try to use reflection. I remembered I successfully got the binder of SerialService by reflection. Maybe I am in permissive context at that time.

@Mikanoshi
Copy link

Choose a reason for hiding this comment

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

Any examples? I do this:

Class<?> smCls = Class.forName("android.os.ServiceManager");
Method getSevice = smCls.getMethod("getService", String.class);
getSevice.setAccessible(true);
IHelperService mService = IHelperService.Stub.asInterface((IBinder)getSevice.invoke(null, "serial"));

@yujincheng08
Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, then I was in permissive context without selinux preventing me.

@Mikanoshi
Copy link

@Mikanoshi Mikanoshi commented on 8482237 Dec 22, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

But sometimes you still want to do something in initzygote when you are hooking system server.

That can be done in handleLoadPackage for (pkg = android, process = android) or there are some downsides?
It's more about hooking Android framework classes for every app.
I can split shared prefs into different categories, but in initZygote there are 60+ mods that hook framework classes and they use prefs from all these categories, so now I have to add another xml that contains initZygote prefs 💢
One mod can contain both SystemUI/system_server and initZygote hooks, so prefs will go into different files... What a fun time sorting that all out.

@Mikanoshi
Copy link

Choose a reason for hiding this comment

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

I just measured the exec times of different parts of my initZygote method and turns out splitting shared prefs into different files is just a waste of time.
My 65kb XML takes 10-20ms to load, splitting it will make 2 smaller XMLs load for hooked processes, that might be even slower.
And the rest of the code in zygote takes 1.5-2 seconds to execute, so prefs loading is nothing compared to applying dozens of hooks anyway.

@C3C0
Copy link
Contributor

@C3C0 C3C0 commented on 8482237 Dec 25, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Hey all. Thanks for the info. My preference handling is now updated and is backwards compatible so works with EdXposed v0.4 as well. In-app backup/restore works as well.

While I'm here maybe somebody has a clue. I'm hooking android.inputmethodservice.InputMethodService to intercept onKeyUp, onKyeDown methods in initZygote. This allows me to mangle keys for IME in any app. It worked fine in EdXposed v0.4 and I can't figure out why it's not working in v0.5 anymore.

@Mikanoshi
Copy link

@Mikanoshi Mikanoshi commented on 8482237 Dec 25, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I have the exact same hook for cursor control by volume buttons. Is it hooking without errors but hook is not being executed? Looking forward to all these problems when I finally decide to install 0.5 :)

Please sign in to comment.