search

NeoApplications / Neo-Backup

backup manager for android
GNU Affero General Public License v3.0
2.52k stars 124 forks source link

Multi-profiles / Work profile support #795

Open didierm opened 1 year ago

didierm commented 1 year ago

Description

NB crashes when attempting to restore External Data from a backed-up application.

Steps To Reproduce

  1. switch to Work profile
  2. start NB
  3. backup application
  4. (remove application)
  5. restore application (select "External Data")
  6. Crash

Expected behavior

No crash

System Information(please complete the following information):

Additional Notes

didierm commented 1 year ago

(note : I tried using Scoop for extra crash data, but Scoop does not seem to catch crashes on A13/LOS20).

hg42 commented 1 year ago

thanks for a complete report...

currently, work profile isn't supported (yet).

Maybe I contributed parts of that code. Unfortunately, I never used a work profile, so while I understand parts of the concept, I don't have any experience with it's specialties.

hg42 commented 1 year ago

sorry, it seems like I accidentally closed the issue

didierm commented 1 year ago

@hg42 Thanks for the quick follow-up !

Some feedback :

  1. Concerning your reply, "currently, work profile isn't supported (yet)." :

  2. Further testing reveals that restoration failure is not limited to External Data, but fails with Media Data too (and supposedly, also with OBB Data). It stands to reason that the denied access to /storage/emulated/xx (with xx=user id) from the su account is the culprit here. (note : maybe this might shine some light too ? "/data is mount point of underlaying block partition with (kinda) raw access to ext4/f2fs file system, while /storage/emulated is sdcardfs file system emulated on top of /data/media for MTP compatibility. yes, additionally isolated mount namespaces, SELinux or disk quota may disturb disk access as the permissions differ." )

  3. The funny thing is, NB perfectly backups External Data etc. in the Work profile. Apparently, different paths to access the Android/ folder are used for backup and restore ?

  4. Looking at https://github.com/NeoApplications/Neo-Backup/blob/75d354fc9ffc2be648b07825b0add9610fe138b7/app/src/main/java/com/machiav3lli/backup/items/StorageFile.kt#L304 , it seems different approaches are possible. While ls -l /storage/emulated/10/ as su yields a "Permission denied" (and probably causes the NB crash), ls -l /mnt/user/10/emulated/10/ is R/W accessible.

So, I guess it all boils down to this : if NB (in the Work profile) is able to access "Android/" during backup , can the same access method not be applied to the restore process ?

hg42 commented 1 year ago

well, I guess Antonios has some expertise with work profiles. The linked comment (the link did not work but I simply opened issue #505) does not really say that it works completely, "you can..." and "one of the problems is solved", not sure if it was intended to say it would work.

Anyways, I never used work profiles and I don't know much about it, mostly that it's another user number. So I just don't think of work profiles.

When I fix bugs, I may not always take work profiles into account (though not much of the code is influenced by it).

I remember, I worked on the restore part of external data (also media and obb). The function getOwnerGroupContext does not work for the external data. I implemented getOwnerGroupContextWithWorkaround instead.

As far as I remember, NB used a way to determine the permissions etc. from it's own external data.

I changed that to use info from the app itself, I think from the apps data directory. So I used some way to derive the permissions etc. by other means.

hg42 commented 1 year ago

that line in StorageFile is only used, when you have shadowRoot enabled (and the corresponding allow...). Only in this case, NB tries to find a root accessible directory for a SAF path and use that instead of SAF.

hg42 commented 1 year ago

Access to the external data directory is another topic.

I think, NB uses the path it gets from the PackageManager. Have to check the code...

didierm commented 1 year ago

that line in StorageFile is only used, when you have shadowRoot enabled (and the corresponding allow...). Only in this case, NB tries to find a root accessible directory for a SAF path and use that instead of SAF.

Interesting : out of curiosity, I tested with both allowShadowingDefault and shadowRootFile enabled before filing the bug, but it made no apparent difference ... (forgot to mention this, sorry)

didierm commented 1 year ago

I am not an Android programmer, but the fact that NB correctly backups the data of a Work profile app (and hence has access to the data location(s)), suggests that restoration should be possible too ...

hg42 commented 1 year ago

this is the code where the crash happens:

    fun getOwnerGroupContextWithWorkaround(
        // TODO hg42 this is the best I could come up with for now
        app: Package,
        extractTo: String,
    ): Array<String> {
        val uidgidcon = try {
            shell.suGetOwnerGroupContext(extractTo)
        } catch (e: Throwable) {
            val fromParent = shell.suGetOwnerGroupContext(File(extractTo).parent!!)
            val fromData = shell.suGetOwnerGroupContext(app.dataPath)
            arrayOf(
                fromData[0],    // user from app data
                fromParent[1],  // group is independent of app
                fromParent[2]   // context is independent of app //TODO hg42 really? some seem to be restricted to app? or may be they should...
                // note: restorecon does not work, because it sets storage_file instead of media_rw_data_file
                // (returning "?" here would choose restorecon)
            )
        }
        return uidgidcon
    }

it first tries to get the permissions via the normal method. An exception is catched and then it uses a workaround.

In this case info is gathered from the parent of the target directory (that is of the external data directory) and from the apps's data directory.

NB does not decide which path to use, instead it uses the path it gets from the PackageManager.

The method assumes, that if you have access to xxx/Android/data/the.package.name, you also have access to xxx/Android/data. I guess, the backup does not access xxx/Android/data because it does not need owner, group an selinux context. The other assumption is that if an NB running in user 0 can access the Android/data of user 0 storage, an NB running as user 10 can also access storage of user 10.

Can you try to access xxx/Android/data/the.package.name? If that directory is not accessible, the backup should not work either...

hg42 commented 1 year ago

more exactly "toybox" ls -bdAlZ "/storage/emulated/10/Android/data/the.package.name" obviously with the.package.name replaced by the real one (I'm sure I don't have to mention that for you, but may be for other readers)

didierm commented 1 year ago

If I understand your request correctly, I think I already mentioned this in my original comment : (as root/su)

# "toybox" ls -bdAlZ /storage/emulated/10/Android/data/be.irm.kmi.meteo/
ls: /storage/emulated/10/Android/data/be.irm.kmi.meteo/: Permission denied
# "toybox" ls -bdAlZ /data/media/10/Android/data/be.irm.kmi.meteo/
drwxrws--- 3 u10_a264 ext_data_rw u:object_r:media_rw_data_file:s0  3452 2023-09-27 02:50 /data/media/10/Android/data/be.irm.kmi.meteo/
didierm commented 1 year ago

Addendum (sorry for the spam) : the above "Permission denied" error may be limited to an adb session, and not necessarily applicable to the NB crash : https://android.stackexchange.com/questions/221122/how-to-access-storage-emulated-10-multi-users-env-in-adb-shell-on-android-9/221534

hg42 commented 1 year ago

(not with the package name...)

however, if you do this in a terminal or adb it's not the same like in NB (it's user 10). The terminal was originally added by me, to run root commands exactly like NB.

hg42 commented 1 year ago

the user number (0 or 10) is used to isolate those (phone) users (a different concept than linux users). Android probably plays games to isolate them. E.g. selinux contexts, mount name spaces, it might also use other container technics. I think this is one reason, why it only "works", if NB is running in the work profile.

hg42 commented 1 year ago

two things might also play a role:

which mount namespace setting do you have in Magisk? what about selinux, is it permissive?

if changing one and/or the other makes it work, it might give a hint, wehere to search for a reason

didierm commented 1 year ago

signal-2023-09-27-162947_mod

hg42 commented 1 year ago

and you are really running NB from the work profile ?

I would expect, that NB running in work profile only sees 10 in emulated and 0 is hidden instead.

Could you test if that is the case with a terminal installed and running in the work profile?

Maybe the root library (libsu and it's shell) does not run as user 10.

didierm commented 1 year ago

  1. Yes, I am running NB from the Work profile.

  2. I have given misleading information.

    • I stated "NB works perfectly backing up external data in a Work profile" : this is incorrect. As my (successful) NB Work profile backup was run in the context of a LineageOS 17 (A10) to 20 (A13) upgrade, I just re-ran a backup in LOS20/A13.
    • This backup failed with an error message (no crash !) with "Access denied" for the /storage/emulated/10/ path.
    • So it appears access methods have obviously changed after A10.

  3. As /data/media/XX/ is always (up to A13, at least) accessible for root/su (contrary to /storage/emulated/XX/), I wonder whether this can be implemented in NB. For testing purposes, I modified https://github.com/NeoApplications/Neo-Backup/blob/75d354fc9ffc2be648b07825b0add9610fe138b7/app/src/main/assets/files/tar.sh#L35 to

    dir=$(echo $1 | sed 's|/storage/emulated/|/data/media/|')

    ; idem for the "extract" command.

NB now backups and restores External Data without issue.

hg42 commented 1 year ago

nice workaround :-)

I always thought, one das the script will allow some tricks think Kostas also did something with it at some point...)

hg42 commented 1 year ago

however, I don't see substituting the path as a final solution.

It's not a good thing to not use the data that Android gives us and instead assume something that is not guarantied anywhere. It creates a maintaining night mare in the long run.

We already had such experiences, e.g. with shadowRoot or the external data.

So, it's still a question why root access is not possible, or more why root commands seem to run in the normal profile.

hg42 commented 1 year ago

this is interesting...

https://www.reddit.com/r/Magisk/comments/z0zp3q/help_how_to_get_root_access_in_work_profile/

I already thought that Magisk is the problem.

Especially, because people want to hide root in another profile (e.g. for banking apps, or if it is managed by their employer).

hg42 commented 1 year ago

it basically points to this thread:

https://forum.xda-developers.com/t/how-to-enable-magisk-root-on-other-user-profiles-multi-user.3833046/

stating that Magisk must be installed to the secondary user and Multiuser Mode must be User-independent

didierm commented 1 year ago

Not sure about Magisk being the problem. My Multiuser Mode is set to "Device Owner Managed" (= 'Only owner can manage root access and receive request prompts'), and as you can view from the NB Terminal screenshot above (whomai), NB runs as root in the Work profile (with Magisk Superuser rights granted to NB in the Personal profile).

In any case, as far as I understand /storage/emulated/ is an additional sdcardfs layer on top of the raw /data/media/ ...

hg42 commented 1 year ago

please change it to "user independent" and try the ls commands again, especially if /storage/emulated then includes the 10.

This might have more effects than getting the prompt or running root commands.

Though, I wonder why it's not isolated. Is Magisk also installed in the work profile?

Perhaps the nsenter we use to get the mount namespace working changes something.

hg42 commented 1 year ago

maybe we have to add some nsenter arguments

currently it's nsenter --mount=/proc/1/ns/mnt sh

lots of possibilities...

# nsenter --help
usage: nsenter [-t pid] [-F] [-i] [-m] [-n] [-p] [-u] [-U] COMMAND...

Run COMMAND in an existing (set of) namespace(s).

-t  PID to take namespaces from    (--target)
-F  don't fork, even if -p is used (--no-fork)

The namespaces to switch are:

-i  SysV IPC: message queues, semaphores, shared memory (--ipc)
-m  Mount/unmount tree (--mount)
-n  Network address, sockets, routing, iptables (--net)
-p  Process IDs and init, will fork unless -F is used (--pid)
-u  Host and domain names (--uts)
-U  UIDs, GIDs, capabilities (--user)

If -t isn't specified, each namespace argument must provide a path
to a namespace file, ala "-i=/proc/$PID/ns/ipc"

that's available:

# ls -l /proc/1/ns/                                                                                                                                
total 0
lrwxrwxrwx 1 root root 0 2023-09-27 18:32 cgroup -> cgroup:[4026531835]
lrwxrwxrwx 1 root root 0 2023-09-27 18:32 mnt -> mnt:[4026533805]
lrwxrwxrwx 1 root root 0 2023-09-27 18:32 net -> net:[4026531931]

I guess cgroup is for --user [nope, obviously cgroup is cgroup and user is user, wishful thinking, some toybox source also shows an option -C for cgroup]

didierm commented 1 year ago

With Magisk Multiuser Mode set to "User Independent", and Magisk being installed in the Work profile too (required), starting NB in WP requests Superuser access, as expected. After granting SU to NB, starting the NB Terminal app yields the same results as before : /storage/emulated/10/ is not visible, nor accessible, while /0/ is.

As this proves notto be the solution, I will revert my Magisk installation back to my preferred settings. (please note that the XDA advioce you referrer to dates from 2018-2020).

hg42 commented 1 year ago

I would think this kind of installation of Magisk is the correct one from a logical POV.

Or in other words:

Either Magisk and su is global (enabled somehow, but "only device owner" doesn't look like this),

or Magisk and su is only active for the profile where it lives (but how? none of the Multiuser modes doesn't look like this, I would expect "user dependent").

hg42 commented 1 year ago

at this moment I think the problem results from the nsenter command.

Thinking backwards, we used it to make the Magisk setting for mount namespace irrelevant.

we started with nsenter -t 1 -m sh

and I now wonder, why this should be different from the current

nsenter --mount=/proc/1/ns/mnt sh

as far as I understand the help, this is only another method to say "use pid 1" in our case. It has more possibilities, e.g. taking network from another pid than mount. Or doesn't -t 1 use /proc/1/ns/* ?

hg42 commented 1 year ago

in toxbox source for nsenter we see:

if (!filename || !*filename) {
  if (!FLAG(t)) error_exit("need -t or =filename");
    sprintf(filename = toybuf, "/proc/%ld/ns/%s", TT.t, nsnames);
}

so either the filename is given or it is constructed from the -t argument. Also the action is only done for those namespace flags given, so -m limits it to mount namespace in both cases.

hg42 commented 1 year ago

ok, the reasoning in #740 was:

In some roms(tested on Ricedroid 10.2) nsenter -t 1 -m sh gives the following error

ScriptException occurred on external_files backup: com.machiav3lli.backup.actions.BaseAppAction$ScriptException: nsenter: /proc/0/ns/mnt: No such file or directory - nsenter: /proc/0/ns/mnt: No such file or directory

Using nsenter --mount=/proc/1/ns/mnt sh should work in every cases.

it seems the -t 1 used 0 instead of 1, but why? maybe it is a bug in some Android toybox versions

hg42 commented 1 year ago

maybe adding --cgroup=/proc/1/ns/cgroup would be necessary, but the Android toybox doesn't have that --cgroup option

hg42 commented 11 months ago

dir=$(echo $1 | sed 's|/storage/emulated/|/data/media/|')

A workaround like this could be implemented, but it's not a future-proof solution.

Assumptions about such mount point relations are likely to fail in the long run.

Some thoughts:

hg42 commented 11 months ago

I still don't get, why backup seems to work and restore does not. If the directory is not visible, the backup should also fail or at least it should be empty.

-rwxrwx--- 1 u10_a166 media_rw 139 2023-09-24 12:46 external_files.tar.gz

that small size looks like an empty archive?

Can you check if the backup contains real data? You should test with an app, that has some amount of real data in external storage.

machiav3lli commented 1 day ago

We'll centre discussion on work profile support (aka multi-profile) here.

Relevant issues/reports: #772 #583 #845