Merge branch 'noroot' of github.com:SafetyInObscurity/mctp

Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>

Author: jk-ozlabs

conf/mctpd-dbus.conf

@@ -2,9 +2,33 @@
 <!DOCTYPE busconfig PUBLIC "-//freedesktop//DTD D-BUS Bus Configuration 1.0//EN"
         "http://www.freedesktop.org/standards/dbus/1.0/busconfig.dtd">
 <busconfig>
+ <policy context="default">
+  <deny send_destination="au.com.codeconstruct.MCTP1"/>
+
+  <allow receive_sender="au.com.codeconstruct.MCTP1"/>
+
+  <allow send_destination="au.com.codeconstruct.MCTP1"
+         send_interface="org.freedesktop.DBus.Propeties"
+         send_member="Get"/>
+
+  <allow send_destination="au.com.codeconstruct.MCTP1"
+         send_interface="org.freedesktop.DBus.Propeties"
+         send_member="GetAll"/>
+
+  <allow send_destination="au.com.codeconstruct.MCTP1"
+         send_interface="org.freedesktop.DBus.Introspectable"/>
+
+  <allow send_destination="au.com.codeconstruct.MCTP1"
+         send_interface="org.freedesktop.DBus.ObjectManager"/>
+ </policy>
+ <policy user="mctpd">
+  <allow own="au.com.codeconstruct.MCTP1"/>
+ </policy>
+ <policy group="mctp-admin">
+  <allow send_destination="au.com.codeconstruct.MCTP1"/>
+ </policy>
  <policy user="root">
   <allow own="au.com.codeconstruct.MCTP1"/>
   <allow send_destination="au.com.codeconstruct.MCTP1"/>
-  <allow receive_sender="au.com.codeconstruct.MCTP1"/>
  </policy>
 </busconfig>

conf/mctpd.service

@@ -8,6 +8,8 @@ After=mctp-local.target
 Type=dbus
 BusName=au.com.codeconstruct.MCTP1
 ExecStart=/usr/sbin/mctpd
+User=mctpd
+AmbientCapabilities=CAP_NET_BIND_SERVICE CAP_NET_ADMIN
 
 [Install]
 WantedBy=mctp.target

docs/mctpd.md

@@ -500,3 +500,14 @@ match = { path = "/devices/pci0000:00/0000:00:08.3/*" }
 ```
 
 Paths have the `/sys` prefix stripped.
+
+## Capabilities
+
+`mctpd` requires the `CAP_NET_BIND_SERVICE` and `CAP_NET_ADMIN` capabilites to run.
+If the service is not running as root these should be added to the `AmbientCapabilities`
+setting in the service's config.
+If the target platform has the `libcap` library available,
+the `mctpd` process will drop `CAP_NET_BIND_SERVICE` when it is no longer needed.
+
+Because `mctpd` may be exposed to potentially hostile inputs,
+it shouldn't be given more capabilities than necessary.

meson.build

@@ -16,6 +16,8 @@ add_project_arguments('-Wno-unused-parameter', language : 'c')
 
 libsystemd = dependency('libsystemd', version: '>=247', required: false)
 
+libcap = dependency('libcap', required: false)
+
 conf = configuration_data()
 conf.set10('HAVE_LINUX_MCTP_H',
     cc.has_header('linux/mctp.h'),
@@ -37,6 +39,10 @@ conf.set10('MCTPD_WRITABLE_CONNECTIVITY',
     get_option('unsafe-writable-connectivity'),
     description: 'Allow writes to the Connectivity member of the au.com.codeconstruct.MCTP.Endpoint1 interface on endpoint objects')
 
+conf.set10('HAVE_LIBCAP',
+    libcap.found(),
+    description: 'Require mctpd to minimise its capabilities')
+
 conf.set_quoted('MCTPD_CONF_FILE_DEFAULT',
     join_paths(get_option('prefix'), get_option('sysconfdir'), 'mctpd.conf'),
     description: 'Default configuration file path',
@@ -89,7 +95,7 @@ if libsystemd.found()
         sources: [
             'src/mctpd.c',
         ] + netlink_sources + util_sources + ops_sources,
-        dependencies: [libsystemd, toml_dep],
+        dependencies: [libsystemd, toml_dep, libcap],
         install: true,
         install_dir: get_option('sbindir'),
         c_args: ['-DOPS_SD_EVENT=1'],
@@ -100,7 +106,7 @@ if libsystemd.found()
             'src/mctpd.c',
         ] + test_ops_sources + netlink_sources + util_sources,
         include_directories:  include_directories('src'),
-        dependencies: [libsystemd, toml_dep],
+        dependencies: [libsystemd, toml_dep, libcap],
         c_args: ['-DOPS_SD_EVENT=1'],
     )
 endif

src/mctpd.c

@@ -9,6 +9,10 @@
 #define _GNU_SOURCE
 #include "config.h"
 
+#if HAVE_LIBCAP
+#include <sys/capability.h>
+#endif
+
 #include <assert.h>
 #include <systemd/sd-bus-vtable.h>
 #include <time.h>
@@ -6347,6 +6351,50 @@ static int endpoint_allocate_eids(struct peer *peer)
 	return 0;
 }
 
+#if HAVE_LIBCAP
+
+static int drop_bind_cap(void)
+{
+	cap_flag_t flags[] = { CAP_EFFECTIVE, CAP_INHERITABLE, CAP_PERMITTED };
+	cap_value_t bind = CAP_NET_BIND_SERVICE;
+	cap_t cap;
+	size_t i;
+	int rc;
+
+	cap = cap_get_proc();
+	if (!cap)
+		return -errno;
+
+	for (i = 0; i < ARRAY_SIZE(flags); i++) {
+		rc = cap_set_flag(cap, flags[i], 1, &bind, CAP_CLEAR);
+		if (rc < 0) {
+			rc = -errno;
+			cap_free(cap);
+			return rc;
+		}
+	}
+
+	rc = cap_set_proc(cap);
+	if (rc < 0) {
+		rc = -errno;
+		cap_free(cap);
+		return rc;
+	}
+
+	rc = cap_free(cap);
+	if (rc < 0)
+		return -errno;
+
+	return 0;
+}
+#else
+
+static int drop_bind_cap(void)
+{
+	return 0;
+}
+#endif
+
 int main(int argc, char **argv)
 {
 	struct ctx ctxi = { 0 }, *ctx = &ctxi;
@@ -6402,6 +6450,13 @@ int main(int argc, char **argv)
 		return 1;
 	}
 
+	rc = drop_bind_cap();
+	if (rc < 0) {
+		warnx("Error dropping capabilities, returned %s %d",
+		      strerror(-rc), rc);
+		return 1;
+	}
+
 	// All setup must be complete by here, we might immediately
 	// get requests from waiting clients.
 	rc = request_dbus(ctx);