Skip to content

Commit

Permalink
Merge pull request #6575 from zyga/tweak/refactor-sc-main-4
Browse files Browse the repository at this point in the history
cmd/snap-confine: pass sc_invocation instead of numerous args around
  • Loading branch information
zyga authored Mar 21, 2019
2 parents 75e8873 + 8e19c0f commit 911ba5e
Show file tree
Hide file tree
Showing 8 changed files with 199 additions and 84 deletions.
4 changes: 4 additions & 0 deletions cmd/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,8 @@ new_format = \
libsnap-confine-private/cgroup-support.h \
snap-confine/seccomp-support-ext.c \
snap-confine/seccomp-support-ext.h \
snap-confine/snap-confine-invocation.c \
snap-confine/snap-confine-invocation.h \
snap-discard-ns/snap-discard-ns.c
.PHONY: fmt
fmt:: $(filter $(addprefix %,$(new_format)),$(foreach dir,$(subdirs),$(wildcard $(srcdir)/$(dir)/*.[ch])))
Expand Down Expand Up @@ -230,6 +232,8 @@ snap_confine_snap_confine_SOURCES = \
snap-confine/seccomp-support.h \
snap-confine/snap-confine-args.c \
snap-confine/snap-confine-args.h \
snap-confine/snap-confine-invocation.c \
snap-confine/snap-confine-invocation.h \
snap-confine/snap-confine.c \
snap-confine/udev-support.c \
snap-confine/udev-support.h \
Expand Down
15 changes: 10 additions & 5 deletions cmd/snap-confine/mount-support.c
Original file line number Diff line number Diff line change
Expand Up @@ -516,9 +516,14 @@ static bool __attribute__ ((used))
}

void sc_populate_mount_ns(struct sc_apparmor *apparmor, int snap_update_ns_fd,
const char *base_snap_name, const char *snap_name,
bool is_normal_mode)
sc_invocation * inv)
{
/* NOTE: this function makes a local modification to base snap name.
* This should not be done like that but for the purpose of refactoring
* being purely a refactoring, this property is preserved by copying
* base_snap_name out of the invocation argument and making
* modifications local. */
const char *base_snap_name = inv->base_snap_name;
// 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).
Expand All @@ -530,7 +535,7 @@ void sc_populate_mount_ns(struct sc_apparmor *apparmor, int snap_update_ns_fd,
// Classify the current distribution, as claimed by /etc/os-release.
sc_distro distro = sc_classify_distro();
// Check which mode we should run in, normal or legacy.
if (is_normal_mode) {
if (inv->is_normal_mode) {
// In normal mode we use the base snap as / and set up several bind mounts.
const struct sc_mount mounts[] = {
{"/dev"}, // because it contains devices on host OS
Expand Down Expand Up @@ -612,14 +617,14 @@ void sc_populate_mount_ns(struct sc_apparmor *apparmor, int snap_update_ns_fd,

// set up private mounts
// TODO: rename this and fold it into bootstrap
setup_private_mount(snap_name);
setup_private_mount(inv->snap_instance);

// set up private /dev/pts
// TODO: fold this into bootstrap
setup_private_pts();

// setup the security backend bind mounts
sc_call_snap_update_ns(snap_update_ns_fd, snap_name, apparmor);
sc_call_snap_update_ns(snap_update_ns_fd, inv->snap_instance, apparmor);

// Try to re-locate back to vanilla working directory. This can fail
// because that directory is no longer present.
Expand Down
4 changes: 2 additions & 2 deletions cmd/snap-confine/mount-support.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#define SNAP_MOUNT_SUPPORT_H

#include "../libsnap-confine-private/apparmor-support.h"
#include "snap-confine-invocation.h"

/**
* Return a file descriptor referencing the snap-update-ns utility
Expand Down Expand Up @@ -51,8 +52,7 @@ int sc_open_snap_discard_ns(void);
* this is impossible it will chdir to SC_VOID_DIR.
**/
void sc_populate_mount_ns(struct sc_apparmor *apparmor, int snap_update_ns_fd,
const char *base_snap_name, const char *snap_name,
bool is_normal_mode);
sc_invocation * inv);

/**
* Ensure that / or /snap is mounted with the SHARED option.
Expand Down
29 changes: 14 additions & 15 deletions cmd/snap-confine/ns-support.c
Original file line number Diff line number Diff line change
Expand Up @@ -306,10 +306,8 @@ enum sc_discard_vote {
// inspect the namespace and send information back via eventfd and then exit
// unconditionally.
static int sc_inspect_and_maybe_discard_stale_ns(int mnt_fd,
const char *snap_name,
const char *base_snap_name,
int snap_discard_ns_fd,
bool is_normal_mode)
const sc_invocation * inv,
int snap_discard_ns_fd)
{
char base_snap_rev[PATH_MAX] = { 0 };
char fname[PATH_MAX] = { 0 };
Expand All @@ -318,16 +316,18 @@ static int sc_inspect_and_maybe_discard_stale_ns(int mnt_fd,

// Read the revision of the base snap by looking at the current symlink.
sc_must_snprintf(fname, sizeof fname, "%s/%s/current",
SNAP_MOUNT_DIR, base_snap_name);
SNAP_MOUNT_DIR, inv->base_snap_name);
if (readlink(fname, base_snap_rev, sizeof base_snap_rev) < 0) {
die("cannot read current revision of snap %s", snap_name);
die("cannot read current revision of snap %s",
inv->snap_instance);
}
if (base_snap_rev[sizeof base_snap_rev - 1] != '\0') {
die("cannot read current revision of snap %s: value too long",
snap_name);
inv->snap_instance);
}
// Find the device that is backing the current revision of the base snap.
base_snap_dev = find_base_snap_device(base_snap_name, base_snap_rev);
base_snap_dev =
find_base_snap_device(inv->base_snap_name, base_snap_rev);

// Store the PID of this process. This is done instead of calls to
// getppid() below because then we can reliably track the PID of the
Expand Down Expand Up @@ -383,6 +383,7 @@ static int sc_inspect_and_maybe_discard_stale_ns(int mnt_fd,
// systemd. This makes us end up in a situation where the outer base
// snap will never match the rootfs inside the mount namespace.
bool should_discard =
inv->
is_normal_mode ? should_discard_current_ns(base_snap_dev) :
false;

Expand Down Expand Up @@ -422,15 +423,15 @@ static int sc_inspect_and_maybe_discard_stale_ns(int mnt_fd,
return 0;
}
// The namespace is stale, let's check if we can discard it.
if (sc_cgroup_freezer_occupied(snap_name)) {
if (sc_cgroup_freezer_occupied(inv->snap_instance)) {
// Some processes are still using the namespace so we cannot discard it
// as that would fracture the view that the set of processes inside
// have on what is mounted.
debug("preserved mount namespace is stale but occupied");
return 0;
}
// The namespace is both stale and empty. We can discard it now.
sc_call_snap_discard_ns(snap_discard_ns_fd, snap_name);
sc_call_snap_discard_ns(snap_discard_ns_fd, inv->snap_instance);
return EAGAIN;
}

Expand All @@ -442,9 +443,8 @@ static void helper_capture_ns(struct sc_mount_ns *group, pid_t parent);
static void helper_capture_per_user_ns(struct sc_mount_ns *group, pid_t parent);

int sc_join_preserved_ns(struct sc_mount_ns *group, struct sc_apparmor
*apparmor, const char *base_snap_name,
const char *snap_name, int snap_discard_ns_fd,
bool is_normal_mode)
*apparmor, const sc_invocation * inv,
int snap_discard_ns_fd)
{
// Open the mount namespace file.
char mnt_fname[PATH_MAX] = { 0 };
Expand Down Expand Up @@ -485,8 +485,7 @@ int sc_join_preserved_ns(struct sc_mount_ns *group, struct sc_apparmor

// Inspect and perhaps discard the preserved mount namespace.
if (sc_inspect_and_maybe_discard_stale_ns
(mnt_fd, snap_name, base_snap_name,
snap_discard_ns_fd, is_normal_mode) == EAGAIN) {
(mnt_fd, inv, snap_discard_ns_fd) == EAGAIN) {
return ESRCH;
}
// Remember the vanilla working directory so that we may attempt to restore it later.
Expand Down
6 changes: 3 additions & 3 deletions cmd/snap-confine/ns-support.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include <stdbool.h>

#include "../libsnap-confine-private/apparmor-support.h"
#include "snap-confine-invocation.h"

/**
* Re-associate the current process with the mount namespace of pid 1.
Expand Down Expand Up @@ -91,9 +92,8 @@ void sc_close_mount_ns(struct sc_mount_ns *group);
* function returns zero.
**/
int sc_join_preserved_ns(struct sc_mount_ns *group, struct sc_apparmor
*apparmor, const char *base_snap_name,
const char *snap_name, int snap_discard_ns_fd,
bool is_normal_mode);
*apparmor, const sc_invocation * inv,
int snap_discard_ns_fd);

/**
* Join a preserved, per-user, mount namespace if one exists.
Expand Down
84 changes: 84 additions & 0 deletions cmd/snap-confine/snap-confine-invocation.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
/*
* Copyright (C) 2019 Canonical Ltd
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License version 3 as
* published by the Free Software Foundation.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*
*/
#include "snap-confine-invocation.h"

#include <stdlib.h>
#include <string.h>

#include "../libsnap-confine-private/cleanup-funcs.h"
#include "../libsnap-confine-private/snap.h"
#include "../libsnap-confine-private/string-utils.h"
#include "../libsnap-confine-private/utils.h"

void sc_init_invocation(sc_invocation *inv, const struct sc_args *args, const char *snap_instance) {
/* Snap instance name is conveyed via untrusted environment. It may be
* unset (typically when experimenting with snap-confine by hand). It
* must also be a valid snap instance name. */
if (snap_instance == NULL) {
die("cannot use NULL snap instance name");
}
sc_instance_name_validate(snap_instance, NULL);

/* The security tag is conveyed via untrusted command line. It must be
* in agreement with snap instance name and must be a valid security
* tag. */
const char *security_tag = sc_args_security_tag(args);
if (!verify_security_tag(security_tag, snap_instance)) {
die("security tag %s not allowed", security_tag);
}

/* The base snap name is conveyed via untrusted, optional, command line
* argument. It may be omitted where it implies the "core" snap is the
* base. */
const char *base_snap_name = sc_args_base_snap(args);
if (base_snap_name == NULL) {
base_snap_name = "core";
}
sc_snap_name_validate(base_snap_name, NULL);

/* The executable is conveyed via untrusted command line. It must be set
* but cannot be validated further than that at this time. It might be
* arguable to validate it to be snap-exec in one of the well-known
* locations or one of the special-cases like strace / gdb but this is
* not done at this time. */
const char *executable = sc_args_executable(args);
if (executable == NULL) {
die("cannot run with NULL executable");
}

/* Invocation helps to pass relevant data to various parts of snap-confine. */
memset(inv, 0, sizeof *inv);
inv->base_snap_name = sc_strdup(base_snap_name);
inv->executable = sc_strdup(executable);
inv->security_tag = sc_strdup(security_tag);
inv->snap_instance = sc_strdup(snap_instance);
inv->classic_confinement = sc_args_is_classic_confinement(args);

debug("security tag: %s", inv->security_tag);
debug("executable: %s", inv->executable);
debug("confinement: %s", inv->classic_confinement ? "classic" : "non-classic");
debug("base snap: %s", inv->base_snap_name);
}

void sc_cleanup_invocation(sc_invocation *inv) {
if (inv != NULL) {
sc_cleanup_string(&inv->snap_instance);
sc_cleanup_string(&inv->base_snap_name);
sc_cleanup_string(&inv->security_tag);
sc_cleanup_string(&inv->executable);
}
}
60 changes: 60 additions & 0 deletions cmd/snap-confine/snap-confine-invocation.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
/*
* Copyright (C) 2019 Canonical Ltd
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License version 3 as
* published by the Free Software Foundation.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*
*/

#ifndef SC_SNAP_CONFINE_INVOCATION_H
#define SC_SNAP_CONFINE_INVOCATION_H

#include <stdbool.h>

#include "snap-confine-args.h"

/**
* sc_invocation contains information about how snap-confine was invoked.
*
* All of the pointer fields have the life-cycle bound to the main process.
**/
typedef struct sc_invocation {
/* Things declared by the system. */
char *snap_instance;
char *base_snap_name;
char *security_tag;
char *executable;
bool classic_confinement;
/* Things derived at runtime. */
bool is_normal_mode;
} sc_invocation;

/**
* sc_init_invocation initializes the invocation object.
*
* Invocation is constructed based on command line arguments as well as
* environment value (SNAP_INSTANCE_NAME). All input is untrusted and is
* validated internally.
**/
void sc_init_invocation(sc_invocation *inv, const struct sc_args *args, const char *snap_instance);

/**
* sc_cleanup_invocation is a cleanup function for sc_invocation.
*
* Cleanup functions are automatically called by the compiler whenever a
* variable gets out of scope, like C++ destructors would.
*
* This function is designed to be used with SC_CLEANUP(sc_cleanup_invocation).
**/
void sc_cleanup_invocation(sc_invocation *inv);

#endif
Loading

0 comments on commit 911ba5e

Please sign in to comment.