core: drop Capabilities= setting
authorLennart Poettering <lennart@poettering.net>
Fri, 12 Feb 2016 22:29:57 +0000 (23:29 +0100)
committerLennart Poettering <lennart@poettering.net>
Sat, 13 Feb 2016 10:59:34 +0000 (11:59 +0100)
The setting is hardly useful (since its effect is generally reduced to zero due
to file system caps), and with the advent of ambient caps an actually useful
replacement exists, hence let's get rid of this.

I am pretty sure this was unused and our man page already recommended against
its use, hence this should be a safe thing to remove.

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 7437938..837b825 100644 (file)
--- a/TODO
+++ b/TODO
@@ -38,8 +38,6 @@ Features:
 
 * cache sd_event_now() result from before the first iteration...
 
-* remove Capabilities=, after all AmbientCapabilities= and CapabilityBoundingSet= should be enough.
-
 * support for the new copy_file_range() syscall
 
 * add systemctl stop --job-mode=triggering that follows TRIGGERED_BY deps and adds them to the same transaction
index 3bcda46..4c05835 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 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
+    <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
     <citerefentry><refentrytitle>systemd.exec</refentrytitle><manvolnum>5</manvolnum></citerefentry>.
     </para>
 
index f0f77c5..008565c 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, 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>
+        <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.</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>ReadWriteDirectories=</varname></term>
         <term><varname>ReadOnlyDirectories=</varname></term>
         <term><varname>InaccessibleDirectories=</varname></term>
index f2fc301..973a601 100644 (file)
@@ -312,7 +312,7 @@ static int property_get_ambient_capabilities(
         return sd_bus_message_append(reply, "t", c->capability_ambient_set);
 }
 
-static int property_get_capabilities(
+static int property_get_empty_string(
                 sd_bus *bus,
                 const char *path,
                 const char *interface,
@@ -321,23 +321,10 @@ static int property_get_capabilities(
                 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);
+        return sd_bus_message_append(reply, "s", "");
 }
 
 static int property_get_syscall_filter(
@@ -700,7 +687,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_capabilities, 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("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 30f7e05..184c72d 100644 (file)
@@ -746,10 +746,10 @@ static int enforce_groups(const ExecContext *context, const char *username, gid_
 static int enforce_user(const ExecContext *context, uid_t uid) {
         assert(context);
 
-        /* Sets (but doesn't lookup) the uid and make sure we keep the
+        /* Sets (but doesn't look up) the uid and make sure we keep the
          * capabilities while doing so. */
 
-        if (context->capabilities || context->capability_ambient_set != 0) {
+        if (context->capability_ambient_set != 0) {
 
                 /* First step: If we need to keep capabilities but
                  * drop privileges we need to make sure we keep our
@@ -761,31 +761,9 @@ 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;
-                }
         }
 
-        /* Third step: actually set the uids */
+        /* Second step: actually set the uids */
         if (setresuid(uid, uid, uid) < 0)
                 return -errno;
 
@@ -1874,21 +1852,6 @@ 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) {
@@ -1927,12 +1890,6 @@ 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)
                         if (prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0) < 0) {
                                 *exit_status = EXIT_NO_NEW_PRIVILEGES;
@@ -2175,11 +2132,6 @@ 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_dirs = strv_free(c->read_only_dirs);
         c->read_write_dirs = strv_free(c->read_write_dirs);
         c->inaccessible_dirs = strv_free(c->inaccessible_dirs);
@@ -2538,14 +2490,6 @@ 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 f720570..41148bc 100644 (file)
@@ -155,10 +155,7 @@ struct ExecContext {
         unsigned long mount_flags;
 
         uint64_t capability_bounding_set;
-
         uint64_t capability_ambient_set;
-
-        cap_t capabilities;
         int secure_bits;
 
         int syslog_priority;
index fde64c9..5568b46 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_exec_capabilities,     0,                             offsetof($1, exec_context)
+$1.Capabilities,                 config_parse_warn_compat,           DISABLED_LEGACY,               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 ba551fb..b31bf83 100644 (file)
@@ -951,38 +951,6 @@ 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,
@@ -3797,7 +3765,6 @@ 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 372d05a..34f15af 100644 (file)
@@ -52,7 +52,6 @@ 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);