Change wrapper to use /root instead of $HOME if run as root.

[kyrofa]

Currently Snappy services (which are run as root) use /root/apps/ for $SNAP_APP_USER_DATA_PATH. However, Snappy binaries always use $HOME/apps/, which resolves to the real user's home even if run with sudo.

This PR fixes this inconsistency by having Snappy binaries behave more like Snappy services, and making sure that /root/apps/ is handled the same way as /home/*/apps/ when it comes to upgrades
etc.

Fixes LP: #1466234

[snappy-m-o]

Can one of the admins verify this patch?

[kyrofa]

Note that this PR implements portions of Jamie's Proposal _#_1 here:

Proposal _#_1:

1. continue to have /apps/bin/... wrappers correctly set SNAP_APP_USER_DATA_PATH to /home/$user/apps/... and mkdir this dir when run as non-root
2. adjust /apps/bin/... wrappers to set SNAP_APP_USER_DATA_PATH to /root/apps/... and mkdir this dir when run as root (eg, under sudo)
3. adjust the apparmor policy accordingly
4. adjust snappy to handle /root in the same manner as /home/$user wrt rollbacks and upgrades.

This PR assumes that (1) and (3) are already done, and completes (2) and (4).

[jdstrand]

FYI, '3' is done. '1' will be correctly done as a result of '2' being implemented.

[kyrofa]

@jdstrand excellent!

[jdstrand]

I agree with the approach taken of adjusting the wrapper to check if root using 'id -u' and if it is '0', using /root/apps instead. I have a nitpick: I prefer to use an equal on '0' rather than a not equals for clarity such that the resulting wrapper becomes:

if [ "$(id -u)" = "0" ]; then
export SNAP_APP_USER_DATA_PATH="/root/apps/...
else
export SNAP_APP_USER_DATA_PATH="$HOME/apps/...
fi

I'll let others comment on the go code. Thanks for the pull request! :)

[jdstrand]

Actually, the fix for apparmor policy is in xenial but not in edge:
$ system-image-cli -i
current build number: 287
device name: generic_amd64
channel: ubuntu-core/rolling/edge
last update: 2015-12-17 08:09:44
version version: 287
version ubuntu: 20151217.2
version raw-device: 20151217.2

$ cat /usr/share/snappy/security-policy-version
Listing...
apparmor/now 2.10-0ubuntu6 amd64 [installed,local]
ubuntu-core-security-apparmor/now 16.04.2 all [installed,local]
ubuntu-core-security-seccomp/now 16.04.2 all [installed,local]
ubuntu-core-security-utils/now 16.04.2 amd64 [installed,local]

[kyrofa]

@jdstrand agreed on the clarity nitpick, I went back and forth on that-- fixed. Also, any chance we could get that apparmor fix (and this patch, if possible) back into trusty through wily (really wherever Snappy is released)?

[jdstrand]

Ok, it was a local problem. 16.04 does have the apparmor fix. ubuntu-core-security 15.04.12~ppa14 also has the fix for 15.04 and it is confirmed in the latest 15.04 stable.

[chipaca]

This will leave data behind on purge; look for SnapDataHomeGlob and see where that is used. AFAIR it's mostly encapsulated in snappy/datadir.go and should be easy to fix.

[kyrofa]

@chipaca good catch, thank you. Will indeed be an easy fix once #283 is merged.

[niemeyer]

Is there a good reason for us to duplicate every occurrence of $HOME because it's the root user? Why aren't we building that logic based on the fact $HOME can point to /root, as commonly done elsewhere?

[chipaca]

When running under sudo, $HOME will be e.g. /home/ubuntu; additionally
it'll be set to e.g. /apps/hello/1.0/ before launching the app
On 5 Jan 2016 5:25 pm, "Gustavo Niemeyer" notifications@github.com wrote:

Is there a good reason for us to duplicate every occurrence of $HOME
because it's the root user? Why aren't we building that logic based on the
fact $HOME can point to /root, as commonly done elsewhere?

—
Reply to this email directly or view it on GitHub
ubuntu-core#264 (comment).

[niemeyer]

When running under sudo, $HOME will be e.g. /home/ubuntu; additionally it'll be set to e.g. /apps/hello/1.0/ before launching the app

Right, I understand that this is exactly the issue being addressed. The question is why aren't we fixing the fact HOME is pointing to something else than what we want instead? If the goal was to preserve the behavior of sudo, we would probably not pick proposal 1 from James proposal as I understand it. So, why are we not defining the user's home concept to be more consistent and easily understandable, instead of spreading conditionals through special cases? Reviewing this code and reading the above conversation, I feel like we'll see bugs being reported about more special cases we forgot about soon.

[chipaca]

ah. Yes, good point. Need to step back a little and see if it's doable; it
should be. But it might require wrapping other parts of the libc.

On 5 January 2016 at 18:14, Gustavo Niemeyer notifications@github.com
wrote:

When running under sudo, $HOME will be e.g. /home/ubuntu; additionally
it'll be set to e.g. /apps/hello/1.0/ before launching the app

Right, I understand that this is exactly the issue being addressed. The
question is why aren't we fixing the fact HOME is pointing to something
else than what we want instead? If the goal was to preserve the behavior of
sudo, we would probably not pick proposal 1 from James proposal as I
understand it. So, why are we not defining the user's home concept to be
more consistent and easily understandable, instead of spreading
conditionals through special cases? Reviewing this code and reading the
above conversation, I feel like we'll see bugs being reported about more
special cases we forgot about soon.

—
Reply to this email directly or view it on GitHub
ubuntu-core#264 (comment).

[niemeyer]

I'm closing this so we can more easily wait for it (or another PR) to reflect the agreement in the mailing list.