Skip to content
This repository has been archived by the owner on Oct 4, 2023. It is now read-only.

Try to preserve current working directory #48

Merged
merged 6 commits into from
Jun 24, 2016
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
21 changes: 21 additions & 0 deletions spread-tests/regression/lp-1595444/task.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
summary: Regression check for https://bugs.launchpad.net/snap-confine/+bug/1595444
# This is blacklisted on debian because we first have to get the dpkg-vendor patches
systems: [-debian-8]
execute: |
echo "Having installed the snapd-hacker-toolbelt snap"
snap list | grep -q snapd-hacker-toolbelt || snap install snapd-hacker-toolbelt

echo "We can go to a location that is available in all snaps (/tmp)"
echo "We can run the 'cwd' tool from busybox and it reports /tmp"
[ "$(cd /tmp && /snap/bin/snapd-hacker-toolbelt.busybox pwd)" = "/tmp" ]

echo "But if we go to a location that is not available to snaps (e.g. $HOME/.cache)"
echo "Then the same 'cwd' tool refuses to run the snap"
mkdir -p "$HOME/.cache/"
cd "$HOME/.cache" || exit 1
# pwd doesn't run
[ "$(/snap/bin/snapd-hacker-toolbelt.busybox pwd)" = "" ]
# there's an accurate error message
[ "$(/snap/bin/snapd-hacker-toolbelt.busybox pwd 2>&1)" = "cannot remain in $HOME/.cache, please run this snap from another location" ]
# snap-confine returns an error code on exit
! /snap/bin/snapd-hacker-toolbelt.busybox pwd
11 changes: 9 additions & 2 deletions spread.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -19,30 +19,37 @@ prepare: |
echo "Spread is running as $(id)"
[ "$REUSE_PROJECT" != 1 ] || exit 0
release_ID="$( . /etc/os-release && echo "${ID:-linux}" )"

case $release_ID in
ubuntu)
# apt update is hanging on security.ubuntu.com with IPv6.
sysctl -w net.ipv6.conf.all.disable_ipv6=1
trap "sysctl -w net.ipv6.conf.all.disable_ipv6=0" EXIT
;;
debian)
echo "deb http://ftp.de.debian.org/debian sid main" > /etc/apt/sources.list.d/snappy.list
;;
esac
case $release_ID in
ubuntu|debian)
apt-get purge -y snap-confine || true
apt-get update
apt-get install --quiet -y fakeroot
# Build a local copy of snap-confine
apt-get install --quiet -y autoconf automake autotools-dev debhelper dh-apparmor dh-autoreconf indent libapparmor-dev libseccomp-dev libudev-dev pkg-config shellcheck udev
test -d /home/test || adduser --quiet --disabled-password --gecos '' test
chown test.test -R ..
sudo -i -u test /bin/sh -c "cd $PWD && DEB_BUILD_OPTIONS=nocheck dpkg-buildpackage -tc -b -Zgzip"
dpkg -i ../snap-confine_*.deb || apt-get -f install -y
dpkg -i ../ubuntu-core-launcher_*.deb || apt-get -f install -y
rm -f ../snap-confine_*.deb ../ubuntu-core-launcher_*.deb
# Install snapd (testes require it)
apt-get install -y snapd
;;
esac
# Install the core snap
snap list | grep -q ubuntu-core || snap install ubuntu-core

suites:
spread-tests/:
summary: Full-system tests for snap-confine
spread-tests/regression/:
summary: Regression tests for past bug-fixes
16 changes: 16 additions & 0 deletions src/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,13 @@ int main(int argc, char **argv)
// this launcher
setup_slave_mount_namespace();

// Get the current working directory before we start fiddling with
// mounts and possibly pivot_root. At the end of the whole process, we
// will try to re-locate to the same directory (if possible).
char vanilla_cwd[512];
if (getcwd(vanilla_cwd, sizeof vanilla_cwd) == NULL) {
die("cannot get the current working directory");
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I suggest using get_current_dir_name() instead of getcwd(), like we do in mount-support.c. This has the advantage of not having to pick an arbitrary size for vanilla_cwd but does mean you need to free() the result.

// do the mounting if run on a non-native snappy system
if (is_running_on_classic_distribution()) {
setup_snappy_os_mounts();
Expand All @@ -104,6 +111,15 @@ int main(int argc, char **argv)
snappy_udev_cleanup(&udev_s);
#endif // ifdef STRICT_CONFINEMENT

// Try to re-locate back to vanilla working directory. This can fail
// because that directory is no longer present.
if (chdir(vanilla_cwd) != 0) {
// NOTE: Use exit rather than die as this produces a more refined error message
Copy link
Contributor

Choose a reason for hiding this comment

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

Hm, I wonder if the perror() from die() might be interessting here for the user. E.g. to see that its a permsision denied error vs a not-exit error and the like.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, I can switch back to die, given that we run as root the only cause for this to fail might be apparmor. In any case, I'll switch to die and we'll see.

fprintf(stderr,
"cannot remain in %s, please run this snap from another location\n",
vanilla_cwd);
exit(1);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since we are die()ing, LGTM with the get_current_dir_name() change.

// the rest does not so temporarily drop privs back to calling
// user (we'll permanently drop after loading seccomp)
if (setegid(real_gid) != 0)
Expand Down