Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Multi-profiles / Work profile support #795

Open
didierm opened this issue Sep 27, 2023 · 43 comments
Open

Multi-profiles / Work profile support #795

didierm opened this issue Sep 27, 2023 · 43 comments
Labels
enhancement New feature or request

Comments

@didierm
Copy link

didierm commented Sep 27, 2023

Description

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

  • the crash only happens in the Work profile
  • the crash occurs with all apps with external data
  • the crash does not occur when restoring the APK, Data, and/or Device Protected Data

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):

  • Device: Xiaomi Mi 9T
  • Android Version: 13
  • ROM: LineageOS 20 (20-20230920)
  • App's Version: 8.3.3

Additional Notes

  • Logcat of an example application restore (be.irm.kmi.meteo), with only "External Data" selected :
09-27 02:37:01.194 17773 17885 I NeoBackup>-Bitmaps:119::restore: Restoring: be.irm.kmi.meteo (KMI Weather)
09-27 02:37:01.194 17773 17885 D NeoBackup>-Bitmaps:146::restore: pre-process package
09-27 02:37:01.197 17773 17885 D NeoBackup>ShellHandler$Companion:16::runAsRoot: Running Command: sh /data/user/10/com.machiav3lli.backup/files/assets/package.sh pre-restore "toybox"  be.irm.kmi.meteo 1010264
09-27 02:37:01.216  1881  3328 D CoreBackPreview: Window{ffc8310 u10 com.machiav3lli.backup/com.machiav3lli.backup.activities.MainActivityX}: Setting back callback null
09-27 02:37:01.222  1881  3328 W InputManager-JNI: Input channel object 'ffc8310 com.machiav3lli.backup/com.machiav3lli.backup.activities.MainActivityX (client)' was disposed without first being removed with the input manager!
09-27 02:37:01.230  1881 12940 I ActivityManager: Stopping app for user: be.irm.kmi.meteo/0
09-27 02:37:01.256 17773 17885 D NeoBackup>ShellHandler$Companion:85::runAsRoot: Command(s) sh /data/user/10/com.machiav3lli.backup/files/assets/package.sh pre-restore "toybox"  be.irm.kmi.meteo 1010264 ended with 0
09-27 02:37:01.256 17773 17885 W NeoBackup>BaseAppAction:284::preprocessPackage: restore be.irm.kmi.meteo: pre-results: 
09-27 02:37:01.257 17773 17885 I NeoBackup>RestoreAppAction:84::restoreAllData: <be.irm.kmi.meteo> Skip restoring app's data; not part of the backup or restore mode
09-27 02:37:01.257 17773 17885 I NeoBackup>RestoreAppAction:130::restoreAllData: <be.irm.kmi.meteo> Skip restoring app's device protected data; not part of the backup or restore mode
09-27 02:37:01.258 17773 17885 I NeoBackup>RestoreAppAction:152::restoreAllData: <be.irm.kmi.meteo> Restoring app's external data
09-27 02:37:01.258 17773 17885 D NeoBackup>RestoreAppAction:34::restoreExternalData: [be.irm.kmi.meteo] Extracting external_files.tar.gz
09-27 02:37:01.274 17773 17885 D NeoBackup>ShellHandler$Companion:16::runAsRoot: Running Command: "toybox" ls -bdAlZ "/storage/emulated/10/Android/data/be.irm.kmi.meteo"
09-27 02:37:01.296 17773 17885 D NeoBackup>ShellHandler$Companion:85::runAsRoot: Command(s) "toybox" ls -bdAlZ "/storage/emulated/10/Android/data/be.irm.kmi.meteo" ended with 1
09-27 02:37:01.296 17773 17885 D NeoBackup>ShellHandler$Companion:16::runAsRoot: Running Command: "toybox" ls -bdAlZ "/storage/emulated/10/Android/data"
09-27 02:37:01.314 17773 17885 D NeoBackup>ShellHandler$Companion:85::runAsRoot: Command(s) "toybox" ls -bdAlZ "/storage/emulated/10/Android/data" ended with 1
09-27 02:37:01.314 17773 17885 D NeoBackup>-Bitmaps:931::restore: post-process package (to set it back to normal operation)
09-27 02:37:01.315 17773 17885 W NeoBackup>BaseAppAction:172::postprocessPackage: restore be.irm.kmi.meteo: postprocess pre-results: 
09-27 02:37:01.315 17773 17885 D NeoBackup>ShellHandler$Companion:16::runAsRoot: Running Command: sh /data/user/10/com.machiav3lli.backup/files/assets/package.sh post-restore "toybox"  be.irm.kmi.meteo 1010264 
09-27 02:37:01.333 17773 17885 D NeoBackup>ShellHandler$Companion:85::runAsRoot: Command(s) sh /data/user/10/com.machiav3lli.backup/files/assets/package.sh post-restore "toybox"  be.irm.kmi.meteo 1010264  ended with 0
09-27 02:37:01.333 17773 17885 I NeoBackup>-Bitmaps:970::restore: Package{packageName=be.irm.kmi.meteo, appInfo=com.machiav3lli.backup.dbs.entity.AppInfo@414c208f, storageStats=android.app.usage.StorageStats@9b68c48, backupList=[Backup{backupDate=2023-07-03T00:21:17.911, hasApk=true, hasAppData=true, hasDevicesProtectedData=true, hasExternalData=false, hasObbData=false, hasMediaData=false, compressionType='gz', cipherType='null', iv='[B@2d33b3f', cpuArch='arm64-v8a', backupVersionCode='8003', size=24306719, permissions='[android.permission.ACCESS_COARSE_LOCATION, android.permission.ACCESS_FINE_LOCATION, android.permission.VIBRATE, com.google.android.c2dm.permission.RECEIVE]', persistent='false'}, Backup{backupDate=2023-09-24T12:46:38.412, hasApk=true, hasAppData=true, hasDevicesProtectedData=true, hasExternalData=true, hasObbData=false, hasMediaData=false, compressionType='gz', cipherType='null', iv='[B@1b79c0c', cpuArch='arm64-v8a', backupVersionCode='8003', size=24332175, permissions='[android.permission.ACCESS_COARSE_LOCATION, android.permission.ACCESS_FINE_LOCATION, android.permission.VIBRATE, com.google.android.c2dm.permission.RECEIVE]', persistent='false'}]}: Restore done: Backup{backupDate=2023-09-24T12:46:38.412, hasApk=true, hasAppData=true, hasDevicesProtectedData=true, hasExternalData=true, hasObbData=false, hasMediaData=false, compressionType='gz', cipherType='null', iv='[B@1b79c0c', cpuArch='arm64-v8a', backupVersionCode='8003', size=24332175, permissions='[android.permission.ACCESS_COARSE_LOCATION, android.permission.ACCESS_FINE_LOCATION, android.permission.VIBRATE, com.google.android.c2dm.permission.RECEIVE]', persistent='false'}
09-27 02:37:01.335 17773 17885 I NeoBackup>MainPageKt$$ExternalSyntheticLambda0:50::uncaughtException: ============================================================
09-27 02:37:01.335 17773 17885 E NeoBackup>Coil:77::logException: unexpected: 
09-27 02:37:01.335 17773 17885 E NeoBackup>Coil:77::logException: ShellHandler$UnexpectedCommandResult
09-27 02:37:01.335 17773 17885 E NeoBackup>Coil:77::logException: '"toybox" ls -bdAlZ "/storage/emulated/10/Android/data"' failed
09-27 02:37:01.335 17773 17885 E NeoBackup>Coil:77::logException: at com.machiav3lli.backup.handler.ShellHandler.suGetOwnerGroupContext(Unknown Source:81)
09-27 02:37:01.335 17773 17885 E NeoBackup>Coil:77::logException: at com.machiav3lli.backup.actions.RestoreAppAction.getOwnerGroupContextWithWorkaround(Unknown Source:30)
09-27 02:37:01.335 17773 17885 E NeoBackup>Coil:77::logException: at com.machiav3lli.backup.actions.RestoreAppAction.restoreExternalData(Unknown Source:58)
09-27 02:37:01.335 17773 17885 E NeoBackup>Coil:77::logException: at com.machiav3lli.backup.actions.RestoreAppAction.restoreAllData(Unknown Source:162)
09-27 02:37:01.335 17773 17885 E NeoBackup>Coil:77::logException: at coil.util.-Bitmaps.restore(Unknown Source:264)
09-27 02:37:01.335 17773 17885 E NeoBackup>Coil:77::logException: at com.machiav3lli.backup.tasks.RestoreActionTask.doInBackground(Unknown Source:60)
09-27 02:37:01.335 17773 17885 E NeoBackup>Coil:77::logException: at com.machiav3lli.backup.tasks.CoroutinesAsyncTask$execute$2.invokeSuspend(Unknown Source:33)
09-27 02:37:01.335 17773 17885 E NeoBackup>Coil:77::logException: at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(Unknown Source:8)
09-27 02:37:01.335 17773 17885 E NeoBackup>Coil:77::logException: at kotlinx.coroutines.DispatchedTask.run(Unknown Source:109)
09-27 02:37:01.335 17773 17885 E NeoBackup>Coil:77::logException: at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.run(Unknown Source:93)
09-27 02:37:01.336 17773 17885 D NeoBackup>ShellHandler$Companion:16::runAsRoot: Running Command: logcat -d -t 100000 --pid=17773 | grep -v SHELLOUT:
09-27 02:37:01.385 17773 17885 D NeoBackup>ShellHandler$Companion:85::runAsRoot: Command(s) logcat -d -t 100000 --pid=17773 | grep -v SHELLOUT: ended with 0
09-27 02:37:01.429 17773 17885 D NeoBackup>ShellHandler$Companion:16::runAsRoot: Running Command: set
09-27 02:37:01.435 17773 17885 D NeoBackup>ShellHandler$Companion:85::runAsRoot: Command(s) set ended with 0
09-27 02:37:01.574  1881  3328 I ActivityManager: Process com.machiav3lli.backup (pid 17773) has died: fg  TOP 
  • toybox :
# toybox --version
toybox 0.8.6-android
  • folder access :
# toybox ls -bdAlZ /storage/emulated/10/
ls: /storage/emulated/10/: Permission denied

 # toybox ls -bdAlZ /data/media/10/
drwxrws--- 19 media_rw media_rw u:object_r:media_rw_data_file:s0  3452 2023-09-27 01:04 /data/media/10/
  • backup folder contents :
# ls -l /data/media/10/backups/NeoBackup.work/be.irm.kmi.meteo/2023-09-24-12-46-38-412-user_10/
total 23796
-rwxrwx--- 1 u10_a166 media_rw 24292915 2023-09-24 12:46 base.apk
-rwxrwx--- 1 u10_a166 media_rw    38436 2023-09-24 12:46 data.tar.gz
-rwxrwx--- 1 u10_a166 media_rw      685 2023-09-24 12:46 device_protected_files.tar.gz
-rwxrwx--- 1 u10_a166 media_rw      139 2023-09-24 12:46 external_files.tar.gz
@didierm
Copy link
Author

didierm commented Sep 27, 2023

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

@hg42
Copy link
Collaborator

hg42 commented Sep 27, 2023

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 hg42 closed this as completed Sep 27, 2023
@hg42 hg42 reopened this Sep 27, 2023
@hg42
Copy link
Collaborator

hg42 commented Sep 27, 2023

sorry, it seems like I accidentally closed the issue

@didierm
Copy link
Author

didierm commented Sep 27, 2023

@hg42
Thanks for the quick follow-up !

Some feedback :

  1. Concerning your reply, "currently, work profile isn't supported (yet)." :
  1. 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." )

  2. 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 ?

  3. Looking at

    // NOTE: lockups occur in emulator (or A12?) for certain paths
    , 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
Copy link
Collaborator

hg42 commented Sep 27, 2023

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
Copy link
Collaborator

hg42 commented Sep 27, 2023

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
Copy link
Collaborator

hg42 commented Sep 27, 2023

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
Copy link
Author

didierm commented Sep 27, 2023

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
Copy link
Author

didierm commented Sep 27, 2023

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
Copy link
Collaborator

hg42 commented Sep 27, 2023

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
Copy link
Collaborator

hg42 commented Sep 27, 2023

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
Copy link
Author

didierm commented Sep 27, 2023

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
Copy link
Author

didierm commented Sep 27, 2023

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
Copy link
Collaborator

hg42 commented Sep 27, 2023

(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
Copy link
Collaborator

hg42 commented Sep 27, 2023

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
Copy link
Collaborator

hg42 commented Sep 27, 2023

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
Copy link
Author

didierm commented Sep 27, 2023

  • Tested in NB's Terminal ( NB > Settings > Tools > Terminal ) : same results (cfr. screenshot)
  • Magisk Mount Namespace Mode : "Root sessions will inherit their requesterś namespace"
  • SELinux : Enforcing

signal-2023-09-27-162947_mod

@hg42
Copy link
Collaborator

hg42 commented Sep 27, 2023

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
Copy link
Author

didierm commented Sep 27, 2023

  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.
  1. 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
    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
Copy link
Collaborator

hg42 commented Sep 27, 2023

nice workaround :-)

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

@hg42
Copy link
Collaborator

hg42 commented Sep 27, 2023

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
Copy link
Collaborator

hg42 commented Sep 27, 2023

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
Copy link
Collaborator

hg42 commented Sep 27, 2023

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
Copy link
Author

didierm commented Sep 27, 2023

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
Copy link
Collaborator

hg42 commented Sep 27, 2023

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
Copy link
Collaborator

hg42 commented Sep 27, 2023

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
Copy link
Author

didierm commented Sep 27, 2023

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
Copy link
Collaborator

hg42 commented Sep 27, 2023

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
Copy link
Collaborator

hg42 commented Sep 27, 2023

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
Copy link
Collaborator

hg42 commented Sep 27, 2023

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
Copy link
Collaborator

hg42 commented Sep 27, 2023

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
Copy link
Collaborator

hg42 commented Sep 27, 2023

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

@hg42
Copy link
Collaborator

hg42 commented Nov 1, 2023

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:

  • such a solution should be limited to the non-standard case, meaning the standard case (user 0) should not be affected by this change. So if this fails it only affects the other user profiles
  • the assumption should be as limited as possible. E.g. we could eventually derive the mount point relation from standard information like files in /proc or /sys (maybe also from the mount command, but that is again more likely to change in the future [depending on toybox source] and not designed to be machine-readable)
  • the properties of the relation should be checked, e.g. if it contains expected files and if it is writable

@hg42
Copy link
Collaborator

hg42 commented Nov 1, 2023

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 machiav3lli changed the title [Bug] Crash when restoring external data in Work profile Multi-profiles / Work profile support Sep 28, 2024
@machiav3lli machiav3lli added the enhancement New feature or request label Sep 28, 2024
@machiav3lli
Copy link
Member

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

Relevant issues/reports: #772 #583 #845

@MobCode100
Copy link

Just to add, my use case is to store backup from work profile to personal folder. Selecting a folder in work profile as the backup folder, the command ls -al /storage/emulated/ shows the correct folder

--- # ls -al /storage/emulated/  => ok
total 6
drwxrws--- 16 media_rw media_rw 3452 2024-10-09 21:22 10
drwxrwx---  2 media_rw media_rw 3452 1970-01-07 08:24 obb

Whereas, selecting a folder from main profile as the backup folder, the command shows

--- # ls -al /storage/emulated  => ok
total 10
drwxrws--- 17 media_rw media_rw 3452 2024-10-10 23:59 0
drwxrwx---  2 media_rw media_rw 3452 1970-01-07 08:24 obb

The app can't access the user 10 folder anymore, so backups will fail.

@hg42
Copy link
Collaborator

hg42 commented Oct 13, 2024

not sure what you want to say...

  • on what apk are you testing,
  • which shell (e.g. in NB terminal?)
  • which root solution?

The app can't access the user 10 folder anymore

what is meant with anymore? compared to what? (e.g. the NB version where it worked),
if it's the NB version, did you change anything else? e.g. the ROM

@hg42
Copy link
Collaborator

hg42 commented Oct 13, 2024

about #772 "Restoration of work profile backups fail silently, if the profile ids differ"
@ashjas

e.g. backup user 11, but restore on 10

function:

Below 8.3.7, the profile stored in the backup was used (from the properties file, the file/dir name should be irrelevant).
Since 8.3.7 the current profile is used instead, so restoring of a backup from another profile to the current profile should work.

visibility:

the backup items now have a user attached (head icon + number), if it's not the current one

So, #772 should indeed be completed.

@hg42
Copy link
Collaborator

hg42 commented Oct 13, 2024

according to backups cross profile:

  • forget the old mantra "root can do anything everywhere", today it's more complicated (container technics, namespaces, selinux, etc.), it is possible, that uid 0 isn't the most powerful user, or some users can do this, other users can do that

  • some things still work, because root can get access, e.g. root can enter a namespace of another user, but it has to do so (nsenter)

  • profiles are generally isolated, this isolation will probably increase in the future, so it's questionable to invest time in something that works now

  • a multiuser profile is the hardest, because it is encrypted differently

  • using the same password might work with the encryption, but this failed for me lately, maybe each user has a different second key that is combined with the password

  • work profile is a bit different, at least it isn't protected by a different encryption (not sure how it will be in future)

  • the backup directory is accessed via SAF, which could have its own additional access mechanisms

  • SAF is not root access at all, it works by the current user granting the right manually

  • that's why shadowRoot must be used to get into another profile, however the file picker must at least show the other profile (it does not), so the only way to get out of the current user is external storage. Surprisingly, you can select an external folder, but it is not writable (despite open as writable, theoretically the file picker should not show it)

  • at some point I plan to add a rudimentary file picker working with root commands

  • @MichaelZ4714 suggested a simple text entry box to edit the url, this would be a simpler solution to allow root file system access to the backup directory (it would detect file://... and use RootFile in this case, this would need a few changes but it is doable)

@MobCode100
Copy link

MobCode100 commented Oct 15, 2024

not sure what you want to say...

  • on what apk are you testing,
  • which shell (e.g. in NB terminal?)
  • which root solution?

The app can't access the user 10 folder anymore

what is meant with anymore? compared to what? (e.g. the NB version where it worked), if it's the NB version, did you change anything else? e.g. the ROM

I'm sorry for the lack of details.

NB Version: 8.3.8
OS: YAAP 14 (Android 14)
Root: Magisk Alpha (280001)
Use case: Backing up work profile apps (user 10) to a folder inside user 0
Steps (NB installed in work profile):

  • Install Insular and setup work profile
  • Install NB in work profile
  • Select any folder in work profile as the Backup folder
  • The command in NB terminal ls -al /storage/emulated/ shows the correct user ID folder (which is 10)
  • NB can backup successfully
  • Select a folder in main profile (user 0) through SAF
  • ls -al /storage/emulated in NB terminal shows incorrect user ID (which 0)
  • Backing up to that user folder shows error BackupAppAction$BackupFailedException: nullnull

It could be because NB can't see the folder 10 right after selecting a main profile folder. What I meant anymore, is the backup process went smoothly if a folder inside the work profile is selected as the backup folder, compared to when selecting a folder inside main profile.

I hope I made that clear, just to report an additional problem to this github issue. I found a workaround for this using magisk.

@MobCode100
Copy link

  • @MichaelZ4714 suggested a simple text entry box to edit the url, this would be a simpler solution to allow root file system access to the backup directory (it would detect file://... and use RootFile in this case, this would need a few changes but it is doable)

This would be great too, but maybe make it advanced/experimental settings?

@hg42
Copy link
Collaborator

hg42 commented Nov 1, 2024

just a quick note...

the ls command does not matter for the normal backup directory (using SAF).

SAF provides the folder contents and file contents via it's own API.

However, with shadowRoot NB tries to match the SAF folders to real files using root access (for this the ls command would matter).

What you observe (user number seen being dependent on SAF folder selection) might be the system making the granted directory available.

I actually do not understand why the system offers directories it cannot write to.
As far as I know, NB uses the official methods to access SAF, it's not based on root methods or hacks or similar.

I only had a work profile for a short time. I have nearly no experience how it should work.

I remember, that the directory picker allowed to select directories that were not writable later.
But I did not find out what might go wrong. Write access is simply failing.

@hg42
Copy link
Collaborator

hg42 commented Nov 1, 2024

This would be great too, but maybe make it advanced/experimental settings?

in my current implementation there is a pen symbol that allows editing the backup folder preference as text.
If the entry is an absolut path /... or a file://... uri, shadowRoot is enabled automatically and access works via root file access.

The remaining problem is the handling on (first) startup.
Currently, you need to enter DevTools on the permissions page, enter the path and then kill NB to let it pick up the path.
That's not ideal...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants