cmd/snap-confine: pass sc_invocation instead of numerous args around

cmd/Makefile.am

@@ -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])))
@@ -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 \

cmd/snap-confine/mount-support.c

@@ -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).
@@ -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
@@ -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.

cmd/snap-confine/mount-support.h

@@ -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
@@ -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.

cmd/snap-confine/ns-support.c

@@ -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 };
@@ -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
@@ -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;
 
@@ -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;
 }
 
@@ -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 };
@@ -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.

cmd/snap-confine/ns-support.h

@@ -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.
@@ -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.

cmd/snap-confine/snap-confine-invocation.c

@@ -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);
+    }
+}

cmd/snap-confine/snap-confine-invocation.h

@@ -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