Revert: "core: drop Capabilities= setting"
authorwchang kim <wchang.kim@samsung.com>
Wed, 9 Nov 2016 08:15:56 +0000 (17:15 +0900)
committerŁukasz Stelmach <l.stelmach@samsung.com>
Fri, 26 Jan 2024 16:49:49 +0000 (17:49 +0100)
This reverts commit 479050b36302a360048c2af5e79683d14ad56fb3

Change-Id: I24367aea159b1decc732b3fbaf448a40e59f2634
Signed-off-by: Woochang Kim <wchang.kim@samsung.com>
TODO
man/sd_bus_creds_get_pid.xml
man/systemd.exec.xml
src/core/dbus-execute.c
src/core/execute.c
src/core/execute.h
src/core/load-fragment-gperf.gperf.m4
src/core/load-fragment.c
src/core/load-fragment.h

diff --git a/TODO b/TODO
index ef25ef5..3d1239f 100644 (file)
--- a/TODO
+++ b/TODO
@@ -95,6 +95,8 @@ Features:
 
 * cache sd_event_now() result from before the first iteration...
 
+* remove Capabilities=, after all AmbientCapabilities= and CapabilityBoundingSet= should be enough.
+
 * add systemctl stop --job-mode=triggering that follows TRIGGERED_BY deps and adds them to the same transaction
 
 * Maybe add a way how users can "pin" units into memory, so that they are not subject to automatic GC?
index 4c05835..3bcda46 100644 (file)
     For processes that are not part of a session, returns -ENXIO.
     </para>
 
-    <para><function>sd_bus_creds_has_effective_cap()</function> will check whether the capability specified by
-    <parameter>capability</parameter> was set in the effective capabilities mask. A positive return value means that it
-    was set, zero means that it was not set, and a negative return value indicates an error. See <citerefentry
-    project='man-pages'><refentrytitle>capabilities</refentrytitle><manvolnum>7</manvolnum></citerefentry> and the
-    <varname>AmbientCapabilities=</varname> and <varname>CapabilityBoundingSet=</varname> settings in
+    <para><function>sd_bus_creds_has_effective_cap()</function> will
+    check whether the capability specified by
+    <parameter>capability</parameter> was set in the effective
+    capabilities mask. A positive return value means that is was
+    set, zero means that it was not set, and a negative return
+    value indicates an error. See
+    <citerefentry project='man-pages'><refentrytitle>capabilities</refentrytitle><manvolnum>7</manvolnum></citerefentry>
+    and <varname>Capabilities=</varname> and
+    <varname>CapabilityBoundingSet=</varname> settings in
     <citerefentry><refentrytitle>systemd.exec</refentrytitle><manvolnum>5</manvolnum></citerefentry>.
     </para>
 
index 41ae6e7..8d6c927 100644 (file)
       <varlistentry>
         <term><varname>CapabilityBoundingSet=</varname></term>
 
-        <listitem><para>Controls which capabilities to include in the capability bounding set for the executed
-        process. See <citerefentry
-        project='man-pages'><refentrytitle>capabilities</refentrytitle><manvolnum>7</manvolnum></citerefentry> for
-        details. Takes a whitespace-separated list of capability names as read by <citerefentry
-        project='mankier'><refentrytitle>cap_from_name</refentrytitle><manvolnum>3</manvolnum></citerefentry>,
-        e.g. <constant>CAP_SYS_ADMIN</constant>, <constant>CAP_DAC_OVERRIDE</constant>,
-        <constant>CAP_SYS_PTRACE</constant>. Capabilities listed will be included in the bounding set, all others are
-        removed. If the list of capabilities is prefixed with <literal>~</literal>, all but the listed capabilities
-        will be included, the effect of the assignment inverted. Note that this option also affects the respective
-        capabilities in the effective, permitted and inheritable capability sets. If this option is not used, the
-        capability bounding set is not modified on process execution, hence no limits on the capabilities of the
-        process are enforced. This option may appear more than once, in which case the bounding sets are merged. If the
-        empty string is assigned to this option, the bounding set is reset to the empty capability set, and all prior
-        settings have no effect.  If set to <literal>~</literal> (without any further argument), the bounding set is
-        reset to the full set of available capabilities, also undoing any previous settings. This does not affect
-        commands prefixed with <literal>+</literal>.</para></listitem>
+        <listitem><para>Controls which capabilities to include in the
+        capability bounding set for the executed process. See
+        <citerefentry project='man-pages'><refentrytitle>capabilities</refentrytitle><manvolnum>7</manvolnum></citerefentry>
+        for details. Takes a whitespace-separated list of capability
+        names as read by
+        <citerefentry project='mankier'><refentrytitle>cap_from_name</refentrytitle><manvolnum>3</manvolnum></citerefentry>,
+        e.g. <constant>CAP_SYS_ADMIN</constant>,
+        <constant>CAP_DAC_OVERRIDE</constant>,
+        <constant>CAP_SYS_PTRACE</constant>. Capabilities listed will
+        be included in the bounding set, all others are removed. If
+        the list of capabilities is prefixed with
+        <literal>~</literal>, all but the listed capabilities will be
+        included, the effect of the assignment inverted. Note that
+        this option also affects the respective capabilities in the
+        effective, permitted and inheritable capability sets, on top
+        of what <varname>Capabilities=</varname> does. If this option
+        is not used, the capability bounding set is not modified on
+        process execution, hence no limits on the capabilities of the
+        process are enforced. This option may appear more than once, in
+        which case the bounding sets are merged. If the empty string
+        is assigned to this option, the bounding set is reset to the
+        empty capability set, and all prior settings have no effect.
+        If set to <literal>~</literal> (without any further argument),
+        the bounding set is reset to the full set of available
+        capabilities, also undoing any previous
+        settings.</para></listitem>
       </varlistentry>
 
       <varlistentry>
       </varlistentry>
 
       <varlistentry>
+        <term><varname>Capabilities=</varname></term>
+        <listitem><para>Controls the
+        <citerefentry project='man-pages'><refentrytitle>capabilities</refentrytitle><manvolnum>7</manvolnum></citerefentry>
+        set for the executed process. Take a capability string
+        describing the effective, permitted and inherited capability
+        sets as documented in
+        <citerefentry project='mankier'><refentrytitle>cap_from_text</refentrytitle><manvolnum>3</manvolnum></citerefentry>.
+        Note that these capability sets are usually influenced (and
+        filtered) by the capabilities attached to the executed file.
+        Due to that <varname>CapabilityBoundingSet=</varname> is
+        probably a much more useful setting.</para></listitem>
+      </varlistentry>
+
+      <varlistentry>
         <term><varname>ReadWritePaths=</varname></term>
         <term><varname>ReadOnlyPaths=</varname></term>
         <term><varname>InaccessiblePaths=</varname></term>
index 307c3d8..3030cbc 100644 (file)
@@ -312,6 +312,7 @@ static int property_get_ambient_capabilities(
         return sd_bus_message_append(reply, "t", c->capability_ambient_set);
 }
 
+#if 0
 static int property_get_empty_string(
                 sd_bus *bus,
                 const char *path,
@@ -326,6 +327,35 @@ static int property_get_empty_string(
 
         return sd_bus_message_append(reply, "s", "");
 }
+#else
+static int property_get_capabilities(
+                sd_bus *bus,
+                const char *path,
+                const char *interface,
+                const char *property,
+                sd_bus_message *reply,
+                void *userdata,
+                sd_bus_error *error) {
+
+        ExecContext *c = userdata;
+        _cleanup_cap_free_charp_ char *t = NULL;
+        const char *s;
+
+        assert(bus);
+        assert(reply);
+        assert(c);
+
+        if (c->capabilities)
+                s = t = cap_to_text(c->capabilities, NULL);
+        else
+                s = "";
+
+        if (!s)
+                return -ENOMEM;
+
+        return sd_bus_message_append(reply, "s", s);
+}
+#endif
 
 static int property_get_syscall_filter(
                 sd_bus *bus,
@@ -687,7 +717,7 @@ const sd_bus_vtable bus_exec_vtable[] = {
         SD_BUS_PROPERTY("SyslogLevelPrefix", "b", bus_property_get_bool, offsetof(ExecContext, syslog_level_prefix), SD_BUS_VTABLE_PROPERTY_CONST),
         SD_BUS_PROPERTY("SyslogLevel", "i", property_get_syslog_level, 0, SD_BUS_VTABLE_PROPERTY_CONST),
         SD_BUS_PROPERTY("SyslogFacility", "i", property_get_syslog_facility, 0, SD_BUS_VTABLE_PROPERTY_CONST),
-        SD_BUS_PROPERTY("Capabilities", "s", property_get_empty_string, 0, SD_BUS_VTABLE_PROPERTY_CONST|SD_BUS_VTABLE_HIDDEN),
+        SD_BUS_PROPERTY("Capabilities", "s", property_get_capabilities, 0, SD_BUS_VTABLE_PROPERTY_CONST),
         SD_BUS_PROPERTY("SecureBits", "i", bus_property_get_int, offsetof(ExecContext, secure_bits), SD_BUS_VTABLE_PROPERTY_CONST),
         SD_BUS_PROPERTY("CapabilityBoundingSet", "t", property_get_capability_bounding_set, 0, SD_BUS_VTABLE_PROPERTY_CONST),
         SD_BUS_PROPERTY("AmbientCapabilities", "t", property_get_ambient_capabilities, 0, SD_BUS_VTABLE_PROPERTY_CONST),
index 807914e..e005986 100644 (file)
@@ -779,7 +779,7 @@ static int enforce_user(const ExecContext *context, uid_t uid) {
         /* Sets (but doesn't look up) the uid and make sure we keep the
          * capabilities while doing so. */
 
-        if (context->capability_ambient_set != 0) {
+        if (context->capabilities || context->capability_ambient_set != 0) {
 
                 /* First step: If we need to keep capabilities but
                  * drop privileges we need to make sure we keep our
@@ -791,9 +791,30 @@ static int enforce_user(const ExecContext *context, uid_t uid) {
                                 if (prctl(PR_SET_SECUREBITS, sb) < 0)
                                         return -errno;
                 }
+                /* Second step: set the capabilities. This will reduce
+                 * the capabilities to the minimum we need. */
+
+                if (context->capabilities) {
+                        _cleanup_cap_free_ cap_t d = NULL;
+                        static const cap_value_t bits[] = {
+                                CAP_SETUID,   /* Necessary so that we can run setresuid() below */
+                                CAP_SETPCAP   /* Necessary so that we can set PR_SET_SECUREBITS later on */
+                        };
+
+                        d = cap_dup(context->capabilities);
+                        if (!d)
+                                return -errno;
+
+                        if (cap_set_flag(d, CAP_EFFECTIVE, ELEMENTSOF(bits), bits, CAP_SET) < 0 ||
+                            cap_set_flag(d, CAP_PERMITTED, ELEMENTSOF(bits), bits, CAP_SET) < 0)
+                                return -errno;
+
+                        if (cap_set_proc(d) < 0)
+                                return -errno;
+                }
         }
 
-        /* Second step: actually set the uids */
+        /* Third step: actually set the uids */
         if (setresuid(uid, uid, uid) < 0)
                 return -errno;
 
@@ -2059,6 +2080,22 @@ static int exec_child(
                                 *exit_status = EXIT_CAPABILITIES;
                                 return r;
                         }
+
+                        if (context->capabilities) {
+
+                                /* The capabilities in ambient set need to be also in the inherited
+                                 * set. If they aren't, trying to get them will fail. Add the ambient
+                                 * set inherited capabilities to the capability set in the context.
+                                 * This is needed because if capabilities are set (using "Capabilities="
+                                 * keyword), they will override whatever we set now. */
+
+                                r = capability_update_inherited_set(context->capabilities, context->capability_ambient_set);
+                                if (r < 0) {
+                                        *exit_status = EXIT_CAPABILITIES;
+                                        return r;
+                                }
+                        }
+
                 }
 
                 if (context->user) {
@@ -2097,6 +2134,12 @@ static int exec_child(
                                 return -errno;
                         }
 
+                if (context->capabilities)
+                        if (cap_set_proc(context->capabilities) < 0) {
+                                *exit_status = EXIT_CAPABILITIES;
+                                return -errno;
+                        }
+
                 if (context->no_new_privileges ||
                     (!have_effective_cap(CAP_SYS_ADMIN) && (use_address_families || context->memory_deny_write_execute || context->restrict_realtime || use_syscall_filter)))
                         if (prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0) < 0) {
@@ -2326,6 +2369,11 @@ void exec_context_done(ExecContext *c) {
 
         c->pam_name = mfree(c->pam_name);
 
+        if (c->capabilities) {
+                cap_free(c->capabilities);
+                c->capabilities = NULL;
+        }
+
         c->read_only_paths = strv_free(c->read_only_paths);
         c->read_write_paths = strv_free(c->read_write_paths);
         c->inaccessible_paths = strv_free(c->inaccessible_paths);
@@ -2688,6 +2736,14 @@ void exec_context_dump(ExecContext *c, FILE* f, const char *prefix) {
                         prefix, strna(lvl_str));
         }
 
+        if (c->capabilities) {
+                _cleanup_cap_free_charp_ char *t;
+
+                t = cap_to_text(c->capabilities, NULL);
+                if (t)
+                        fprintf(f, "%sCapabilities: %s\n", prefix, t);
+        }
+
         if (c->secure_bits)
                 fprintf(f, "%sSecure Bits:%s%s%s%s%s%s\n",
                         prefix,
index 189c4d0..04cc4d6 100644 (file)
@@ -158,6 +158,8 @@ struct ExecContext {
 
         uint64_t capability_bounding_set;
         uint64_t capability_ambient_set;
+
+        cap_t capabilities;
         int secure_bits;
 
         int syslog_priority;
index 6a5c16a..85c5bf7 100644 (file)
@@ -45,7 +45,7 @@ $1.SyslogIdentifier,             config_parse_unit_string_printf,    0,
 $1.SyslogFacility,               config_parse_log_facility,          0,                             offsetof($1, exec_context.syslog_priority)
 $1.SyslogLevel,                  config_parse_log_level,             0,                             offsetof($1, exec_context.syslog_priority)
 $1.SyslogLevelPrefix,            config_parse_bool,                  0,                             offsetof($1, exec_context.syslog_level_prefix)
-$1.Capabilities,                 config_parse_warn_compat,           DISABLED_LEGACY,               offsetof($1, exec_context)
+$1.Capabilities,                 config_parse_exec_capabilities,     0,                             offsetof($1, exec_context)
 $1.SecureBits,                   config_parse_exec_secure_bits,      0,                             offsetof($1, exec_context)
 $1.CapabilityBoundingSet,        config_parse_capability_set,        0,                             offsetof($1, exec_context.capability_bounding_set)
 $1.AmbientCapabilities,          config_parse_capability_set,        0,                             offsetof($1, exec_context.capability_ambient_set)
index a36953f..17b2f5b 100644 (file)
@@ -953,6 +953,38 @@ int config_parse_exec_cpu_affinity(const char *unit,
         return 0;
 }
 
+int config_parse_exec_capabilities(const char *unit,
+                                   const char *filename,
+                                   unsigned line,
+                                   const char *section,
+                                   unsigned section_line,
+                                   const char *lvalue,
+                                   int ltype,
+                                   const char *rvalue,
+                                   void *data,
+                                   void *userdata) {
+
+        ExecContext *c = data;
+        cap_t cap;
+
+        assert(filename);
+        assert(lvalue);
+        assert(rvalue);
+        assert(data);
+
+        cap = cap_from_text(rvalue);
+        if (!cap) {
+                log_syntax(unit, LOG_ERR, filename, line, errno, "Failed to parse capabilities, ignoring: %s", rvalue);
+                return 0;
+        }
+
+        if (c->capabilities)
+                cap_free(c->capabilities);
+        c->capabilities = cap;
+
+        return 0;
+}
+
 int config_parse_exec_secure_bits(const char *unit,
                                   const char *filename,
                                   unsigned line,
@@ -4030,6 +4062,7 @@ void unit_dump_config_items(FILE *f) {
                 { config_parse_input,                 "INPUT" },
                 { config_parse_log_facility,          "FACILITY" },
                 { config_parse_log_level,             "LEVEL" },
+                { config_parse_exec_capabilities,     "CAPABILITIES" },
                 { config_parse_exec_secure_bits,      "SECUREBITS" },
                 { config_parse_capability_set,        "BOUNDINGSET" },
                 { config_parse_limit,                 "LIMIT" },
index b36a2e3..e3ba05b 100644 (file)
@@ -52,6 +52,7 @@ int config_parse_exec_io_priority(const char *unit, const char *filename, unsign
 int config_parse_exec_cpu_sched_policy(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata);
 int config_parse_exec_cpu_sched_prio(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata);
 int config_parse_exec_cpu_affinity(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata);
+int config_parse_exec_capabilities(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata);
 int config_parse_exec_secure_bits(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata);
 int config_parse_capability_set(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata);
 int config_parse_limit(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata);