[OpenWrt-Devel,procd] instance: Add 'mustjail' attribute
diff mbox series

Message ID 20200201162500.35499-1-ldir@darbyshire-bryant.me.uk
State Superseded
Headers show
Series
  • [OpenWrt-Devel,procd] instance: Add 'mustjail' attribute
Related show

Commit Message

Kevin 'ldir' Darbyshire-Bryant Feb. 1, 2020, 4:25 p.m. UTC
Since commit b44417c instance: provide error feedback if ujail binary is
missing, worrying log spam of the form "unable to find /sbin/jail ..."
may be encountered.

On systems not configured with jail capabilities the lack of jail binary
is not an error, whilst on systems with jail capabilities the warning
will be issued and the process is started outside of a jail.

This commit adds a new procd jail parameter 'mustjail' which if set
issues an error and does NOT start the process outside of a jailed
environment.

The original 'unable to find jail binary' warning is output in DEBUG
mode, thus processes started in a 'may jail' but non-jail capable
environment do not spam the log.

Signed-off-by: Kevin Darbyshire-Bryant <ldir@darbyshire-bryant.me.uk>
---
 service/instance.c | 33 +++++++++++++++++++++++----------
 service/instance.h |  1 +
 2 files changed, 24 insertions(+), 10 deletions(-)

Comments

John Crispin Feb. 1, 2020, 9:03 p.m. UTC | #1
On 01/02/2020 17:25, Kevin Darbyshire-Bryant wrote:
> +	bool must_jail;

can we name this requires_jail ?
	John

Patch
diff mbox series

diff --git a/service/instance.c b/service/instance.c
index e872ba0..5dc23e2 100644
--- a/service/instance.c
+++ b/service/instance.c
@@ -101,6 +101,7 @@  enum {
 	JAIL_ATTR_RONLY,
 	JAIL_ATTR_MOUNT,
 	JAIL_ATTR_NETNS,
+	JAIL_ATTR_MUSTJAIL,
 	__JAIL_ATTR_MAX,
 };
 
@@ -114,6 +115,7 @@  static const struct blobmsg_policy jail_attr[__JAIL_ATTR_MAX] = {
 	[JAIL_ATTR_RONLY] = { "ronly", BLOBMSG_TYPE_BOOL },
 	[JAIL_ATTR_MOUNT] = { "mount", BLOBMSG_TYPE_TABLE },
 	[JAIL_ATTR_NETNS] = { "netns", BLOBMSG_TYPE_BOOL },
+	[JAIL_ATTR_MUSTJAIL] = { "mustjail", BLOBMSG_TYPE_BOOL },
 };
 
 struct instance_netdev {
@@ -819,20 +821,16 @@  instance_jail_parse(struct service_instance *in, struct blob_attr *attr)
 {
 	struct blob_attr *tb[__JAIL_ATTR_MAX];
 	struct jail *jail = &in->jail;
-	struct stat s;
-	int r;
-
-	r = stat(UJAIL_BIN_PATH, &s);
-	if (r < 0) {
-		ERROR("unable to find %s: %m (%d)\n", UJAIL_BIN_PATH, r);
-		return 0;
-	}
 
 	blobmsg_parse(jail_attr, __JAIL_ATTR_MAX, tb,
 		blobmsg_data(attr), blobmsg_data_len(attr));
 
 	jail->argc = 2;
 
+	if (tb[JAIL_ATTR_MUSTJAIL]) {
+		in->must_jail = true;
+		jail->argc++;
+	}
 	if (tb[JAIL_ATTR_NAME]) {
 		jail->name = strdup(blobmsg_get_string(tb[JAIL_ATTR_NAME]));
 		jail->argc += 2;
@@ -885,7 +883,7 @@  instance_jail_parse(struct service_instance *in, struct blob_attr *attr)
 	if (in->no_new_privs)
 		jail->argc++;
 
-	return 1;
+	return true;
 }
 
 static bool
@@ -918,7 +916,8 @@  instance_config_parse(struct service_instance *in)
 {
 	struct blob_attr *tb[__INSTANCE_ATTR_MAX];
 	struct blob_attr *cur, *cur2;
-	int rem;
+	struct stat s;
+	int rem, r;
 
 	blobmsg_parse(instance_attr, __INSTANCE_ATTR_MAX, tb,
 		blobmsg_data(in->config), blobmsg_data_len(in->config));
@@ -1004,6 +1003,19 @@  instance_config_parse(struct service_instance *in)
 	if (!in->trace && tb[INSTANCE_ATTR_JAIL])
 		in->has_jail = instance_jail_parse(in, tb[INSTANCE_ATTR_JAIL]);
 
+	if (in->has_jail) {
+		r = stat(UJAIL_BIN_PATH, &s);
+		if (r < 0) {
+			if (in->must_jail) {
+				ERROR("Cannot jail service %s::%s. %s: %m (%d)\n",
+						in->srv->name, in->name, UJAIL_BIN_PATH, r);
+				return false;
+			}
+			DEBUG(2, "unable to find %s: %m (%d)\n", UJAIL_BIN_PATH, r);
+			in->has_jail = false;
+		}
+	}
+
 	if (tb[INSTANCE_ATTR_STDOUT] && blobmsg_get_bool(tb[INSTANCE_ATTR_STDOUT]))
 		in->_stdout.fd.fd = -1;
 
@@ -1146,6 +1158,7 @@  instance_init(struct service_instance *in, struct service *s, struct blob_attr *
 	in->term_timeout = 5;
 	in->syslog_facility = LOG_DAEMON;
 	in->exit_code = 0;
+	in->must_jail = false;
 
 	in->_stdout.fd.fd = -2;
 	in->_stdout.stream.string_data = true;
diff --git a/service/instance.h b/service/instance.h
index 7d91b51..abd91ad 100644
--- a/service/instance.h
+++ b/service/instance.h
@@ -59,6 +59,7 @@  struct service_instance {
 
 	bool trace;
 	bool has_jail;
+	bool must_jail;
 	bool no_new_privs;
 	struct jail jail;
 	char *seccomp;