Merge pull request #4332 from keszybz/nspawn-arguments-3

nspawn --private-users parsing, v2

2 files changed, 55 insertions, 40 deletions

diff --git a/man/systemd-nspawn.xml b/man/systemd-nspawn.xml
index 4439d554a7..5ac54df81a 100644
--- a/man/systemd-nspawn.xml
+++ b/man/systemd-nspawn.xml
@@ -405,35 +405,36 @@
         purposes (usually in the range beyond the host's UID/GID 65536). The parameter may be specified as follows:</para>
 
         <orderedlist>
-          <listitem><para>The value <literal>no</literal> turns off user namespacing. This is the default.</para></listitem>
-
-          <listitem><para>The value <literal>yes</literal> (or the omission of a parameter) turns on user
-          namespacing. The UID/GID range to use is determined automatically from the file ownership of the root
-          directory of the container's directory tree. To use this option, make sure to prepare the directory tree in
-          advance, and ensure that all files and directories in it are owned by UIDs/GIDs in the range you'd like to
-          use. Also, make sure that used file ACLs exclusively reference UIDs/GIDs in the appropriate range. If this
-          mode is used the number of UIDs/GIDs assigned to the container for use is 65536, and the UID/GID of the
-          root directory must be a multiple of 65536.</para></listitem>
-
-          <listitem><para>The value "pick" turns on user namespacing. In this case the UID/GID range is automatically
-          chosen. As first step, the file owner of the root directory of the container's directory tree is read, and it
-          is checked that it is currently not used by the system otherwise (in particular, that no other container is
-          using it). If this check is successful, the UID/GID range determined this way is used, similar to the
-          behaviour if "yes" is specified. If the check is not successful (and thus the UID/GID range indicated in the
-          root directory's file owner is already used elsewhere) a new – currently unused – UID/GID range of 65536
-          UIDs/GIDs is randomly chosen between the host UID/GIDs of 524288 and 1878982656, always starting at a
-          multiple of 65536. This setting implies <option>--private-users-chown</option> (see below), which has the
-          effect that the files and directories in the container's directory tree will be owned by the appropriate
-          users of the range picked. Using this option makes user namespace behaviour fully automatic. Note that the
-          first invocation of a previously unused container image might result in picking a new UID/GID range for it,
-          and thus in the (possibly expensive) file ownership adjustment operation. However, subsequent invocations of
-          the container will be cheap (unless of course the picked UID/GID range is assigned to a different use by
-          then).</para></listitem>
-
-          <listitem><para>Finally if one or two colon-separated numeric parameters are specified, user namespacing is
-          turned on, too. The first parameter specifies the first host UID/GID to assign to the container, the second
-          parameter specifies the number of host UIDs/GIDs to assign to the container. If the second parameter is
-          omitted, 65536 UIDs/GIDs are assigned.</para></listitem>
+          <listitem><para>If one or two colon-separated numers are specified, user namespacing is turned on. The first
+          parameter specifies the first host UID/GID to assign to the container, the second parameter specifies the
+          number of host UIDs/GIDs to assign to the container. If the second parameter is omitted, 65536 UIDs/GIDs are
+          assigned.</para></listitem>
+
+          <listitem><para>If the parameter is omitted, or true, user namespacing is turned on. The UID/GID range to
+          use is determined automatically from the file ownership of the root directory of the container's directory
+          tree. To use this option, make sure to prepare the directory tree in advance, and ensure that all files and
+          directories in it are owned by UIDs/GIDs in the range you'd like to use. Also, make sure that used file ACLs
+          exclusively reference UIDs/GIDs in the appropriate range. If this mode is used the number of UIDs/GIDs
+          assigned to the container for use is 65536, and the UID/GID of the root directory must be a multiple of
+          65536.</para></listitem>
+
+          <listitem><para>If the parameter is false, user namespacing is turned off. This is the default.</para>
+          </listitem>
+
+          <listitem><para>The special value <literal>pick</literal> turns on user namespacing. In this case the UID/GID
+          range is automatically chosen. As first step, the file owner of the root directory of the container's
+          directory tree is read, and it is checked that it is currently not used by the system otherwise (in
+          particular, that no other container is using it). If this check is successful, the UID/GID range determined
+          this way is used, similar to the behaviour if "yes" is specified. If the check is not successful (and thus
+          the UID/GID range indicated in the root directory's file owner is already used elsewhere) a new – currently
+          unused – UID/GID range of 65536 UIDs/GIDs is randomly chosen between the host UID/GIDs of 524288 and
+          1878982656, always starting at a multiple of 65536. This setting implies
+          <option>--private-users-chown</option> (see below), which has the effect that the files and directories in
+          the container's directory tree will be owned by the appropriate users of the range picked. Using this option
+          makes user namespace behaviour fully automatic. Note that the first invocation of a previously unused
+          container image might result in picking a new UID/GID range for it, and thus in the (possibly expensive) file
+          ownership adjustment operation. However, subsequent invocations of the container will be cheap (unless of
+          course the picked UID/GID range is assigned to a different use by then).</para></listitem>
         </orderedlist>
 
         <para>It is recommended to assign at least 65536 UIDs/GIDs to each container, so that the usable UID/GID range in the
diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c
index e066c55dae..a173d171e1 100644
--- a/src/nspawn/nspawn.c
+++ b/src/nspawn/nspawn.c
@@ -913,14 +913,21 @@ static int parse_argv(int argc, char *argv[]) {
 
                         break;
 
-                case ARG_PRIVATE_USERS:
+                case ARG_PRIVATE_USERS: {
+                        int boolean = -1;
 
-                        if (streq_ptr(optarg, "no")) {
+                        if (!optarg)
+                                boolean = true;
+                        else if (!in_charset(optarg, DIGITS))
+                                /* do *not* parse numbers as booleans */
+                                boolean = parse_boolean(optarg);
+
+                        if (boolean == false) {
                                 /* no: User namespacing off */
                                 arg_userns_mode = USER_NAMESPACE_NO;
                                 arg_uid_shift = UID_INVALID;
                                 arg_uid_range = UINT32_C(0x10000);
-                        } else if (!optarg || streq(optarg, "yes")) {
+                        } else if (boolean == true) {
                                 /* yes: User namespacing on, UID range is read from root dir */
                                 arg_userns_mode = USER_NAMESPACE_FIXED;
                                 arg_uid_shift = UID_INVALID;
@@ -931,33 +938,40 @@ static int parse_argv(int argc, char *argv[]) {
                                 arg_uid_shift = UID_INVALID;
                                 arg_uid_range = UINT32_C(0x10000);
                         } else {
+                                _cleanup_free_ char *buffer = NULL;
                                 const char *range, *shift;
 
                                 /* anything else: User namespacing on, UID range is explicitly configured */
 
                                 range = strchr(optarg, ':');
                                 if (range) {
-                                        shift = strndupa(optarg, range - optarg);
+                                        buffer = strndup(optarg, range - optarg);
+                                        if (!buffer)
+                                                return log_oom();
+                                        shift = buffer;
 
                                         range++;
                                         r = safe_atou32(range, &arg_uid_range);
                                         if (r < 0)
-                                                return log_error_errno(r, "Failed to parse UID range '%s': %m", range);
-                                        if (arg_uid_range == 0)
-                                                return log_error_errno(EINVAL, "UID range cannot be 0.");
+                                                return log_error_errno(r, "Failed to parse UID range \"%s\": %m", range);
                                 } else
                                         shift = optarg;
 
-                                if (parse_uid(shift, &arg_uid_shift) < 0) {
-                                        log_error("Failed to parse UID: %s", optarg);
-                                        return -EINVAL;
-                                }
+                                r = parse_uid(shift, &arg_uid_shift);
+                                if (r < 0)
+                                        return log_error_errno(r, "Failed to parse UID \"%s\": %m", optarg);
 
                                 arg_userns_mode = USER_NAMESPACE_FIXED;
                         }
 
+                        if (arg_uid_range <= 0) {
+                                log_error("UID range cannot be 0.");
+                                return -EINVAL;
+                        }
+
                         arg_settings_mask |= SETTING_USERNS;
                         break;
+                }
 
                 case 'U':
                         if (userns_supported()) {