Merge pull request #17154 from keszybz/crypttab-commas

Allow escaping commas in crypttab

man/crypttab.xml

@@ -60,10 +60,10 @@
     device or file, or a specification of a block device via
     <literal>UUID=</literal> followed by the UUID.</para>
 
-    <para>The third field specifies an absolute path to a file to read the encryption key from. Optionally,
+    <para>The third field specifies an absolute path to a file with the encryption key. Optionally,
     the path may be followed by <literal>:</literal> and an fstab device specification (e.g. starting with
-    <literal>LABEL=</literal> or similar); in which case, the path is relative to the device file system
+    <literal>LABEL=</literal> or similar); in which case the path is taken relative to the device file system
-    root. If the field is not present or set to <literal>none</literal> or <literal>-</literal>, a key file
+    root. If the field is not present or is <literal>none</literal> or <literal>-</literal>, a key file
     named after the volume to unlock (i.e. the first column of the line), suffixed with
     <filename>.key</filename> is automatically loaded from the <filename>/etc/cryptsetup-keys.d/</filename>
     and <filename>/run/cryptsetup-keys.d/</filename> directories, if present. Otherwise, the password has to
@@ -78,12 +78,12 @@
       <varlistentry>
         <term><option>cipher=</option></term>
 
-        <listitem><para>Specifies the cipher to use. See
+        <listitem><para>Specifies the cipher to use. See <citerefentry
-        <citerefentry project='die-net'><refentrytitle>cryptsetup</refentrytitle><manvolnum>8</manvolnum></citerefentry>
+        project='die-net'><refentrytitle>cryptsetup</refentrytitle><manvolnum>8</manvolnum></citerefentry>
-        for possible values and the default value of this option. A
+        for possible values and the default value of this option. A cipher with unpredictable IV values, such
-        cipher with unpredictable IV values, such as
+        as <literal>aes-cbc-essiv:sha256</literal>, is recommended. Embedded commas in the cipher
-        <literal>aes-cbc-essiv:sha256</literal>, is
+        specification need to be escaped by preceding them with a backslash, see example below.</para>
-        recommended.</para></listitem>
+        </listitem>
       </varlistentry>
 
       <varlistentry>
@@ -498,15 +498,17 @@
     <title>Examples</title>
     <example>
       <title>/etc/crypttab example</title>
-      <para>Set up four encrypted block devices. One using LUKS for
+      <para>Set up four encrypted block devices. One using LUKS for normal storage, another one for usage as
-      normal storage, another one for usage as a swap device and two
+      a swap device and two TrueCrypt volumes. For the fourth device, the option string is interpreted as two
-      TrueCrypt volumes.</para>
+      options <literal>cipher=xchacha12,aes-adiantum-plain64</literal>,
+      <literal>keyfile-timeout=10s</literal>.</para>
 
       <programlisting>luks       UUID=2505567a-9e27-4efe-a4d5-15ad146c258b
 swap       /dev/sda7       /dev/urandom       swap
 truecrypt  /dev/sda2       /etc/container_password  tcrypt
 hidden     /mnt/tc_hidden  /dev/null    tcrypt-hidden,tcrypt-keyfile=/etc/keyfile
-external   /dev/sda3       keyfile:LABEL=keydev keyfile-timeout=10s</programlisting>
+external   /dev/sda3       keyfile:LABEL=keydev keyfile-timeout=10s,cipher=xchacha12\,aes-adiantum-plain64
+</programlisting>
     </example>
 
     <example>

src/basic/strv.c

@@ -367,7 +367,7 @@ int strv_split_colon_pairs(char ***t, const char *s) {
         return (int) n;
 }
 
-char *strv_join_prefix(char * const *l, const char *separator, const char *prefix) {
+char *strv_join_full(char * const *l, const char *separator, const char *prefix, bool unescape_separators) {
         char * const *s;
         char *r, *e;
         size_t n, k, m;
@@ -378,11 +378,17 @@ char *strv_join_prefix(char * const *l, const char *separator, const char *prefi
         k = strlen(separator);
         m = strlen_ptr(prefix);
 
+        if (unescape_separators) /* If there separator is multi-char, we won't know how to escape it. */
+                assert(k == 1);
+
         n = 0;
         STRV_FOREACH(s, l) {
                 if (s != l)
                         n += k;
-                n += m + strlen(*s);
+
+                bool needs_escaping = unescape_separators && strchr(*s, separator[0]);
+
+                n += m + strlen(*s) * (1 + needs_escaping);
         }
 
         r = new(char, n+1);
@@ -397,7 +403,16 @@ char *strv_join_prefix(char * const *l, const char *separator, const char *prefi
                 if (prefix)
                         e = stpcpy(e, prefix);
 
-                e = stpcpy(e, *s);
+                bool needs_escaping = unescape_separators && strchr(*s, separator[0]);
+
+                if (needs_escaping)
+                        for (size_t i = 0; (*s)[i]; i++) {
+                                if ((*s)[i] == separator[0])
+                                        *(e++) = '\\';
+                                *(e++) = (*s)[i];
+                        }
+                else
+                        e = stpcpy(e, *s);
         }
 
         *e = 0;

src/basic/strv.h

@@ -91,9 +91,9 @@ static inline char **strv_split(const char *s, const char *separators) {
  * string in the vector is an empty string. */
 int strv_split_colon_pairs(char ***t, const char *s);
 
-char *strv_join_prefix(char * const *l, const char *separator, const char *prefix);
+char *strv_join_full(char * const *l, const char *separator, const char *prefix, bool escape_separtor);
 static inline char *strv_join(char * const *l, const char *separator) {
-        return strv_join_prefix(l, separator, NULL);
+        return strv_join_full(l, separator, NULL, false);
 }
 
 char **strv_parse_nulstr(const char *s, size_t l);

src/core/execute.c

@@ -1927,7 +1927,7 @@ static int build_environment(
                 if (!pre)
                         return -ENOMEM;
 
-                joined = strv_join_prefix(c->directories[t].paths, ":", pre);
+                joined = strv_join_full(c->directories[t].paths, ":", pre, true);
                 if (!joined)
                         return -ENOMEM;
 

src/core/socket.c

@@ -526,8 +526,7 @@ int socket_acquire_peer(Socket *s, int fd, SocketPeer **p) {
         assert(fd >= 0);
         assert(s);
 
-        r = getpeername(fd, &sa.peer.sa, &salen);
+        if (getpeername(fd, &sa.peer.sa, &salen) < 0)
-        if (r < 0)
                 return log_unit_error_errno(UNIT(s), errno, "getpeername failed: %m");
 
         if (!IN_SET(sa.peer.sa.sa_family, AF_INET, AF_INET6, AF_VSOCK)) {

src/cryptsetup/cryptsetup-generator.c

@@ -622,7 +622,6 @@ static int parse_proc_cmdline_item(const char *key, const char *value, void *dat
 static int add_crypttab_devices(void) {
         _cleanup_fclose_ FILE *f = NULL;
         unsigned crypttab_line = 0;
-        struct stat st;
         int r;
 
         if (!arg_read_crypttab)
@@ -635,11 +634,6 @@ static int add_crypttab_devices(void) {
                 return 0;
         }
 
-        if (fstat(fileno(f), &st) < 0) {
-                log_error_errno(errno, "Failed to stat %s: %m", arg_crypttab);
-                return 0;
-        }
-
         for (;;) {
                 _cleanup_free_ char *line = NULL, *name = NULL, *device = NULL, *keyspec = NULL, *options = NULL, *keyfile = NULL, *keydev = NULL;
                 crypto_device *d = NULL;

src/cryptsetup/cryptsetup.c

@@ -294,9 +294,9 @@ static int parse_options(const char *options) {
                 _cleanup_free_ char *word = NULL;
                 int r;
 
-                r = extract_first_word(&options, &word, ",", EXTRACT_DONT_COALESCE_SEPARATORS);
+                r = extract_first_word(&options, &word, ",", EXTRACT_DONT_COALESCE_SEPARATORS | EXTRACT_UNESCAPE_SEPARATORS);
                 if (r < 0)
-                        return log_debug_errno(r, "Failed to parse options: %m");
+                        return log_error_errno(r, "Failed to parse options: %m");
                 if (r == 0)
                         break;
 
@@ -874,6 +874,9 @@ static int run(int argc, char *argv[]) {
                                 return r;
                 }
 
+                log_debug("%s %s ← %s type=%s cipher=%s", __func__,
+                          argv[2], argv[3], strempty(arg_type), strempty(arg_cipher));
+
                 /* A delicious drop of snake oil */
                 (void) mlockall(MCL_FUTURE);
 

src/nspawn/nspawn.c

@@ -5134,9 +5134,12 @@ static int run(int argc, char *argv[]) {
         if (r <= 0)
                 goto finish;
 
-        r = must_be_root();
+        if (geteuid() != 0) {
-        if (r < 0)
+                r = log_warning_errno(SYNTHETIC_ERRNO(EPERM),
+                                      argc >= 2 ? "Need to be root." :
+                                      "Need to be root (and some arguments are usually required).\nHint: try --help");
                 goto finish;
+        }
 
         r = cant_be_in_netns();
         if (r < 0)

src/shared/fstab-util.c

@@ -95,7 +95,19 @@ int fstab_filter_options(const char *opts, const char *names,
 
         if (!ret_filtered) {
                 for (const char *word = opts;;) {
-                        const char *end = word + strcspn(word, ",");
+                        const char *end = word;
+
+                        /* Look for an *non-escaped* comma separator. Only commas can be escaped, so "\," is
+                         * the only valid escape sequence, so we can do a very simple test here. */
+                        for (;;) {
+                                size_t n = strcspn(end, ",");
+
+                                end += n;
+                                if (n > 0 && end[-1] == '\\')
+                                        end++;
+                                else
+                                        break;
+                        }
 
                         NULSTR_FOREACH(name, names) {
                                 if (end < word + strlen(name))
@@ -128,9 +140,10 @@ int fstab_filter_options(const char *opts, const char *names,
                                 break;
                 }
         } else {
-                stor = strv_split(opts, ",");
+                r = strv_split_full(&stor, opts, ",", EXTRACT_DONT_COALESCE_SEPARATORS | EXTRACT_UNESCAPE_SEPARATORS);
-                if (!stor)
+                if (r < 0)
-                        return -ENOMEM;
+                        return r;
+
                 strv = memdup(stor, sizeof(char*) * (strv_length(stor) + 1));
                 if (!strv)
                         return -ENOMEM;
@@ -165,7 +178,7 @@ answer:
         if (ret_filtered) {
                 char *f;
 
-                f = strv_join(strv, ",");
+                f = strv_join_full(strv, ",", NULL, true);
                 if (!f)
                         return -ENOMEM;
 

src/shared/udev-util.c

@@ -118,12 +118,13 @@ static int device_new_from_dev_path(const char *devlink, sd_device **ret_device)
 
         assert(devlink);
 
-        r = stat(devlink, &st);
+        if (stat(devlink, &st) < 0)
-        if (r < 0)
+                return log_full_errno(errno == ENOENT ? LOG_DEBUG : LOG_ERR, errno,
-                return log_full_errno(errno == ENOENT ? LOG_DEBUG : LOG_ERR, errno, "Failed to stat() %s: %m", devlink);
+                                      "Failed to stat() %s: %m", devlink);
 
         if (!S_ISBLK(st.st_mode))
-                return log_error_errno(SYNTHETIC_ERRNO(ENOTBLK), "%s does not point to a block device: %m", devlink);
+                return log_error_errno(SYNTHETIC_ERRNO(ENOTBLK),
+                                       "%s does not point to a block device: %m", devlink);
 
         r = sd_device_new_from_devnum(ret_device, 'b', st.st_rdev);
         if (r < 0)

src/test/test-fstab-util.c

@@ -13,12 +13,11 @@ int fstab_filter_options(const char *opts, const char *names,
 */
 
 static void do_fstab_filter_options(const char *opts,
-                                      const char *remove,
+                                    const char *remove,
-                                      int r_expected,
+                                    int r_expected,
-                                      const char *name_expected,
+                                    const char *name_expected,
-                                      const char *value_expected,
+                                    const char *value_expected,
-                                      const char *filtered_expected) {
+                                    const char *filtered_expected) {
-
         int r;
         const char *name;
         _cleanup_free_ char *value = NULL, *filtered = NULL;
@@ -34,7 +33,7 @@ static void do_fstab_filter_options(const char *opts,
 
         /* also test the malloc-less mode */
         r = fstab_filter_options(opts, remove, &name, NULL, NULL);
-        log_info("\"%s\" → %d, \"%s\", expected %d, \"%s\"",
+        log_info("\"%s\" → %d, \"%s\", expected %d, \"%s\"\n-",
                  opts, r, name,
                  r_expected, name_expected);
         assert_se(r == r_expected);
@@ -54,6 +53,12 @@ static void test_fstab_filter_options(void) {
         do_fstab_filter_options("opt,other", "x-opt\0opt\0", 1, "opt", NULL, "other");
         do_fstab_filter_options("x-opt,other", "opt\0x-opt\0", 1, "x-opt", NULL, "other");
 
+        do_fstab_filter_options("opt=0\\,1,other", "opt\0x-opt\0", 1, "opt", "0,1", "other");
+        do_fstab_filter_options("opt=0,other,x-opt\\,foobar", "x-opt\0opt\0", 1, "opt", "0", "other,x-opt\\,foobar");
+        do_fstab_filter_options("opt,other,x-opt\\,part", "opt\0x-opt\0", 1, "opt", NULL, "other,x-opt\\,part");
+        do_fstab_filter_options("opt,other,part\\,x-opt", "x-opt\0opt\0", 1, "opt", NULL, "other,part\\,x-opt");
+        do_fstab_filter_options("opt,other\\,\\,\\,opt,x-part", "opt\0x-opt\0", 1, "opt", NULL, "other\\,\\,\\,opt,x-part");
+
         do_fstab_filter_options("opto=0,other", "opt\0x-opt\0", 0, NULL, NULL, NULL);
         do_fstab_filter_options("opto,other", "opt\0x-opt\0", 0, NULL, NULL, NULL);
         do_fstab_filter_options("x-opto,other", "opt\0x-opt\0", 0, NULL, NULL, NULL);

src/test/test-strv.c

@@ -162,71 +162,84 @@ static void test_strv_find_startswith(void) {
 }
 
 static void test_strv_join(void) {
-        _cleanup_free_ char *p = NULL, *q = NULL, *r = NULL, *s = NULL, *t = NULL, *v = NULL, *w = NULL;
-
         log_info("/* %s */", __func__);
 
-        p = strv_join((char **)input_table_multiple, ", ");
+        _cleanup_free_ char *p = strv_join((char **)input_table_multiple, ", ");
         assert_se(p);
         assert_se(streq(p, "one, two, three"));
 
-        q = strv_join((char **)input_table_multiple, ";");
+        _cleanup_free_ char *q = strv_join((char **)input_table_multiple, ";");
         assert_se(q);
         assert_se(streq(q, "one;two;three"));
 
-        r = strv_join((char **)input_table_multiple, NULL);
+        _cleanup_free_ char *r = strv_join((char **)input_table_multiple, NULL);
         assert_se(r);
         assert_se(streq(r, "one two three"));
 
-        s = strv_join((char **)input_table_one, ", ");
+        _cleanup_free_ char *s = strv_join(STRV_MAKE("1", "2", "3,3"), ",");
         assert_se(s);
-        assert_se(streq(s, "one"));
+        assert_se(streq(s, "1,2,3,3"));
 
-        t = strv_join((char **)input_table_none, ", ");
+        _cleanup_free_ char *t = strv_join((char **)input_table_one, ", ");
         assert_se(t);
-        assert_se(streq(t, ""));
+        assert_se(streq(t, "one"));
 
-        v = strv_join((char **)input_table_two_empties, ", ");
+        _cleanup_free_ char *u = strv_join((char **)input_table_none, ", ");
+        assert_se(u);
+        assert_se(streq(u, ""));
+
+        _cleanup_free_ char *v = strv_join((char **)input_table_two_empties, ", ");
         assert_se(v);
         assert_se(streq(v, ", "));
 
-        w = strv_join((char **)input_table_one_empty, ", ");
+        _cleanup_free_ char *w = strv_join((char **)input_table_one_empty, ", ");
         assert_se(w);
         assert_se(streq(w, ""));
 }
 
-static void test_strv_join_prefix(void) {
+static void test_strv_join_full(void) {
-        _cleanup_free_ char *p = NULL, *q = NULL, *r = NULL, *s = NULL, *t = NULL, *v = NULL, *w = NULL;
-
         log_info("/* %s */", __func__);
 
-        p = strv_join_prefix((char **)input_table_multiple, ", ", "foo");
+        _cleanup_free_ char *p = strv_join_full((char **)input_table_multiple, ", ", "foo", false);
         assert_se(p);
         assert_se(streq(p, "fooone, footwo, foothree"));
 
-        q = strv_join_prefix((char **)input_table_multiple, ";", "foo");
+        _cleanup_free_ char *q = strv_join_full((char **)input_table_multiple, ";", "foo", false);
         assert_se(q);
         assert_se(streq(q, "fooone;footwo;foothree"));
 
-        r = strv_join_prefix((char **)input_table_multiple, NULL, "foo");
+        _cleanup_free_ char *r = strv_join_full(STRV_MAKE("a", "a;b", "a:c"), ";", NULL, true);
         assert_se(r);
-        assert_se(streq(r, "fooone footwo foothree"));
+        assert_se(streq(r, "a;a\\;b;a:c"));
 
-        s = strv_join_prefix((char **)input_table_one, ", ", "foo");
+        _cleanup_free_ char *s = strv_join_full(STRV_MAKE("a", "a;b", "a;;c", ";", ";x"), ";", NULL, true);
         assert_se(s);
-        assert_se(streq(s, "fooone"));
+        assert_se(streq(s, "a;a\\;b;a\\;\\;c;\\;;\\;x"));
 
-        t = strv_join_prefix((char **)input_table_none, ", ", "foo");
+        _cleanup_free_ char *t = strv_join_full(STRV_MAKE("a", "a;b", "a:c", ";"), ";", "=", true);
         assert_se(t);
-        assert_se(streq(t, ""));
+        assert_se(streq(t, "=a;=a\\;b;=a:c;=\\;"));
+        t = mfree(t);
 
-        v = strv_join_prefix((char **)input_table_two_empties, ", ", "foo");
+        _cleanup_free_ char *u = strv_join_full((char **)input_table_multiple, NULL, "foo", false);
+        assert_se(u);
+        assert_se(streq(u, "fooone footwo foothree"));
+
+        _cleanup_free_ char *v = strv_join_full((char **)input_table_one, ", ", "foo", false);
         assert_se(v);
-        assert_se(streq(v, "foo, foo"));
+        assert_se(streq(v, "fooone"));
 
-        w = strv_join_prefix((char **)input_table_one_empty, ", ", "foo");
+        _cleanup_free_ char *w = strv_join_full((char **)input_table_none, ", ", "foo", false);
         assert_se(w);
-        assert_se(streq(w, "foo"));
+        assert_se(streq(w, ""));
+
+        _cleanup_free_ char *x = strv_join_full((char **)input_table_two_empties, ", ", "foo", false);
+        assert_se(x);
+        assert_se(streq(x, "foo, foo"));
+
+        _cleanup_free_ char *y = strv_join_full((char **)input_table_one_empty, ", ", "foo", false);
+        assert_se(y);
+        assert_se(streq(y, "foo"));
 }
 
 static void test_strv_unquote(const char *quoted, char **list) {
@@ -995,7 +1008,7 @@ int main(int argc, char *argv[]) {
         test_strv_find_prefix();
         test_strv_find_startswith();
         test_strv_join();
-        test_strv_join_prefix();
+        test_strv_join_full();
 
         test_strv_unquote("    foo=bar     \"waldo\"    zzz    ", STRV_MAKE("foo=bar", "waldo", "zzz"));
         test_strv_unquote("", STRV_MAKE_EMPTY);

src/udev/udev-builtin-keyboard.c

@@ -110,10 +110,8 @@ static const char* parse_token(const char *current, int32_t *val_out) {
 static int override_abs(sd_device *dev, int fd, unsigned evcode, const char *value) {
         struct input_absinfo absinfo;
         const char *next;
-        int r;
 
-        r = ioctl(fd, EVIOCGABS(evcode), &absinfo);
+        if (ioctl(fd, EVIOCGABS(evcode), &absinfo) < 0)
-        if (r < 0)
                 return log_device_error_errno(dev, errno, "Failed to call EVIOCGABS");
 
         next = parse_token(value, &absinfo.minimum);
@@ -122,12 +120,12 @@ static int override_abs(sd_device *dev, int fd, unsigned evcode, const char *val
         next = parse_token(next, &absinfo.fuzz);
         next = parse_token(next, &absinfo.flat);
         if (!next)
-                return log_device_error_errno(dev, SYNTHETIC_ERRNO(EINVAL), "Failed to parse EV_ABS override '%s'", value);
+                return log_device_error_errno(dev, SYNTHETIC_ERRNO(EINVAL),
+                                              "Failed to parse EV_ABS override '%s'", value);
 
         log_device_debug(dev, "keyboard: %x overridden with %"PRIi32"/%"PRIi32"/%"PRIi32"/%"PRIi32"/%"PRIi32,
                          evcode, absinfo.minimum, absinfo.maximum, absinfo.resolution, absinfo.fuzz, absinfo.flat);
-        r = ioctl(fd, EVIOCSABS(evcode), &absinfo);
+        if (ioctl(fd, EVIOCSABS(evcode), &absinfo) < 0)
-        if (r < 0)
                 return log_device_error_errno(dev, errno, "Failed to call EVIOCSABS");
 
         return 0;