music-app should run under confinement under click

[Jamie Strandboge]

For historical reasons, currently music-app runs unconfined. With media-hub now in place, music-app no longer needs to be unconfined. Because of bug #1303962 and bug #1315381 (lack of apparmor/trust-store integration in mediascanner2) and because music-app still uses grilo, music-app needs special confinement though. Please update click/apparmor.json to be:
{
    "policy_version": 1.1,
    "policy_groups": [
        "audio",
        "networking",
        "usermetrics"
    ],
    "read_path": [
      "/etc/fstab",
      "/proc/[0-9]*/mounts",
      "/usr/share/grilo-plugins/",
      "@{HOME}/.cache/media-art/",
      "@{HOME}/.cache/mediascanner/"
    ],
    "write_path": [
      "@{HOME}/.local/share/grilo-plugins/"
    ]
}

Once music-app moves away from grilo we can remove the write_path and update read_path accordingly. Once it moves to mediascanner2 and bug #1303962 is fixed in mediascanner2, then read_path can be removed entirely.

[Andrew Hayzen (ahayzen)]

I believe we still need to be unconfined until we use background playlists in the media-hub (which is WIP) otherwise next/previous selection will not work when the app is suspended?

[Victor Thompson (vthompson)]

I was under the same impression as Andrew.

[Jamie Strandboge (jdstrand)]

I've been running the app all morning under this confinement and haven't experienced any issues. I used the 'Songs' playlist on random. Should I be doing something different?

[Jamie Strandboge (jdstrand)]

(Note, I am using r8 in devel-proposed)

[Andrew Hayzen (ahayzen)]

The following steps we would *not* expect to work under confinement
1) Select a set of track to play so that there is more than 1 in the queue (eg clicking the top song in the songs tab)
2) Optionally seek near the end of the track
3) Turn the screen off before the track ends so that the app is suspended
4) The player progresses onto the next track

I would expect step 4 not to work until the screen is turned back on.

[Victor Thompson (vthompson)]

Jamie, did you first remove the app's lifecycle exception? I believe it is hardcoded--so Unity-mir would probably need to be recompiled after removing it from the code.

[Victor Thompson (vthompson)]

The code to do this is here:

http://bazaar.launchpad.net/~mir-team/unity-mir/trunk/view/head:/src/modules/Unity/Application/application_manager.cpp#L184

[Jamie Strandboge (jdstrand)]

I didn't remove the lifecycle exception since that is separate from the apps confinement. Running music-app under confinement and using:
$ adb shell tail -f /var/log/syslog | grep DEN

shows no denials, even for powerd (other than one for dbus RequestName which shouldn't be needed on the phone). Are you saying additional apparmor rules are needed to function correctly? It seems to be working fine now.

[Jamie Strandboge (jdstrand)]

"The following steps we would *not* expect to work under confinement
1) Select a set of track to play so that there is more than 1 in the queue (eg clicking the top song in the songs tab)
2) Optionally seek near the end of the track
3) Turn the screen off before the track ends so that the app is suspended
4) The player progresses onto the next track

I would expect step 4 not to work until the screen is turned back on."

I just tested and it works fine.

[Jamie Strandboge (jdstrand)]

As mentioned, apparmor confinement is separate from application lifecycle. I did not touch lifecycle-- I only added confinement.

[Victor Thompson (vthompson)]

Ok, that sounds good to me. This change can proceed. Removing the lifecycle exception is blocked by the move to the updated media-hub with tracklist support.

[Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot)]

Fix committed into lp:music-app at revision None, scheduled for release in music-app, milestone 1.0

[Andrew Hayzen (ahayzen)]

What is preventing us now going back to being confined? mediascanner/thumbnailer?

Also note when this is done we will need write access to ~/Music/ for content-hub imports.

I assume we can't put a translation in the apparmor profile? As for now can only import files into ~/Music/Imported (we may allow the user to decide at a later stage) but "Imported" is translated.

[Jamie Strandboge (jdstrand)]

AppArmor has rudimentary support for translations of xdg user dirs, but note, supporting translated directories on the phone has been ruled out because they are problematic for a whole bunch of reasons, instead the toolkit should handle these for the user rather than handling it in the filesystem.

So, we could've added this to write_path:
owner @{HOME}/Music/Imported/ r,
owner @{HOME}/Music/Imported/** rwk,

but as pointed out, we can't now. We could add the music_files policy group, which uses:
# Usage: reserved
owner @{HOME}/Music/ r,
owner @{HOME}/Music/** rwk,

but then that gets us back to the music-app having full access to all the music files which is one thing we were trying to avoid (we want a system where a 3rd party music app only needs to use the 'common' policy groups). This could be resolved by setting the import directory to ~/.local/share/com.ubuntu.music/imported and then have a symlink from @{HOME}/Music/Imported to ~/.local/share/com.ubuntu.music/imported. Something other than the music-app has to create the symlink.

[Jamie Strandboge (jdstrand)]

I think everything is needed for mediascanner2-- have you tried music-app under confinement (for now, using the music_files) policy group)?

[Andrew Hayzen (ahayzen)]

For those following this bug it is currently blocked by bug 1373086.

[Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot)]

Fix committed into lp:music-app/remix at revision 681, scheduled for release in music-app, milestone music-app-14.10-week-40