[systemd-devel] [PATCHv5] tmpfiles, man: Add xattr support to tmpfiles
This patch makes it possible to set extended attributes on files created by tmpfiles. This can be especially used to set SMACK security labels on volatile files and directories. It is done by adding new line of type "t". Such line should contain attributes in Argument field, using following format: name=value All other fields are ignored. If value contains spaces, then it must be surrounded by quotation marks. User can also put quotation mark in value by escaping it with backslash. Example: D /var/run/cups - - - - t /var/run/cups - - - - security.SMACK64=printing --- I'm sorry for late reply, but there were some unexpected events which excluded me from life for a while. v5: * fixes for HAVE_XATTR undefined * use cunescape() instead of strreplace() * cache result of strv_length() * fix typo in manpage v4: * grammar fix in man * style fix v3: * "may be used" instead of "should be used" in manpage * use strv_isempty() instead of != NULL * rework item_set_xattrs() with split_pair() * remove copy_item_contents() * use hashmap_replace() instead of removed copy_item_contents() * use strv_extend() instead of strv_append() * cleanup --- man/tmpfiles.d.xml | 26 +- src/tmpfiles/tmpfiles.c | 216 +--- 2 files changed, 225 insertions(+), 17 deletions(-) diff --git a/man/tmpfiles.d.xml b/man/tmpfiles.d.xml index 1c079f6..676eded 100644 --- a/man/tmpfiles.d.xml +++ b/man/tmpfiles.d.xml @@ -248,6 +248,21 @@ L/tmp/foobar ---- /dev/null place of normal path names. + + +t +Set extended +attributes on item. It may be +used in conjunction with other +types (only d, D, f, F, L, p, c, b, z +makes sense). If used as a standalone +line, then systemd-tmpfiles + will try to set extended +attributes on specified path. +This can be especially used to set +SMACK labels. + + @@ -312,7 +327,7 @@ L/tmp/foobar ---- /dev/null objects. For z, Z lines, if omitted or when set to -, the file access mode will not be modified. This parameter is ignored for x, r, -R, L lines. +R, L, t lines. @@ -324,7 +339,7 @@ L/tmp/foobar ---- /dev/null omitted or when set to -, the default 0 (root) is used. For z, Z lines, when omitted or when set to -, the file ownership will not be modified. -These parameters are ignored for x, r, R, L lines. +These parameters are ignored for x, r, R, L, t lines. @@ -377,8 +392,10 @@ L/tmp/foobar ---- /dev/null minor formatted as integers, separated by :, e.g. "1:3". For f, F, w may be used to specify a short string that is written to the file, -suffixed by a newline. Ignored for all other +suffixed by a newline. For t determines extended +attributes to be set. Ignored for all other lines. + @@ -390,7 +407,8 @@ L/tmp/foobar ---- /dev/null screen needs two directories created at boot with specific modes and ownership. d /var/run/screens 1777 root root 10d -d /var/run/uscreens 0755 root root 10d12h +d /var/run/uscreens 0755 root root 10d12h +t /var/run/screen - - - - user.name="John Koval" security.SMACK64=screen /etc/tmpfiles.d/abrt.conf example diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c index b7f6a2e..ec5efb6 100644 --- a/src/tmpfiles/tmpfiles.c +++ b/src/tmpfiles/tmpfiles.c @@ -39,6 +39,9 @@ #include #include #include +#ifdef HAVE_XATTR +#include +#endif #include "log.h" #include "util.h" @@ -78,7 +81,10 @@ typedef enum ItemType { REMOVE_PATH = 'r', RECURSIVE_REMOVE_PATH = 'R', RELABEL_PATH = 'z', -RECURSIVE_RELABEL_PATH = 'Z' +RECURSIVE_RELABEL_PATH = 'Z', + +/* Th
Re: [systemd-devel] [PATCHv5] tmpfiles, man: Add xattr support to tmpfiles
On Wed, 04.12.13 15:27, Maciej Wereski (m.were...@partner.samsung.com) wrote: > > +#ifdef HAVE_XATTR > +static int get_xattrs_from_arg(Item *i){ > +_cleanup_free_ char *xattr = NULL; > +_cleanup_strv_free_ char **tmp = NULL; > +char *p; > +unsigned n, len, strv_len; > + > +assert(i); > +if (i->type != SET_XATTR) > +return 0; > + > +if (!i->argument) { > +log_error("%s: Argument can't be empty!", i->path); > +return -EBADMSG; > +} > +xattr = new0(char, strlen(i->argument)+1); > +if (!xattr) > +return log_oom(); > + > +tmp = strv_split(i->argument, WHITESPACE); > +if (!tmp) > +return log_oom(); > + > +strv_len = strv_length(tmp); > +for (n = 0; n < strv_len; ++n) { Sounds like a job for the STRV_FOREACH() macro. Since you don't actually need the strv as strv here it sounds like you actually really want to use FOREACH_WORD_QUOTED() for this, which will also do the unquoting for you. > +len = strlen(tmp[n]); > +strncpy(xattr, tmp[n], len+1); If you know the length anyway you shiuld probably use memcpy() here. > +p = strchr(xattr, '='); > +if (!p) { > +log_error("%s: Attribute has incorrect format.", > i->path); > +return -EBADMSG; > +} > +if (p[1] == '\"') { > +while (true) { > +if (!p) > +p = tmp[n]; > +else > +p += 2; > +p = strchr(p, '\"'); > +if (p && xattr[p-xattr-1] != '\\') > +break; > +p = NULL; > +++n; > +if (n == strv_len) > +break; > +len += strlen(tmp[n]) + 1; > +strncat(xattr, " ", 1); > +strncat(xattr, tmp[n], len); > +} > +} strncat() is an awful command. There are always better ways. In this case FOREACH_WORD_QUOTED sounds like the better option... > +strstrip(xattr); > +if (strv_extend(&i->xattrs, xattr) < 0) > +return log_oom(); It sounds like a better idea to allocate the new attribute with strndup() right from the beginning in each loop, and then use strv_push() to just make it part of the strv... > +} > + > +free(i->argument); > +i->argument = NULL; Hmm, did I miss something, why do you explicitly free() that here? wouldn't that be cleaned up anyway at the end for you? > +return 0; > +} > +#endif > + > +static int item_set_xattrs(Item *i, const char *path) { > +#ifdef HAVE_XATTR > +char **x; > +int n; > + > +assert(i); > +assert(path); > + > +n = get_xattrs_from_arg(i); > +if (n < 0) > +return n; In most cases we call the return code variable "r", and use "n" for storing the number of items of something. Not that it matters much though, ... Otherwise looks OK. Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCHv5] tmpfiles, man: Add xattr support to tmpfiles
10.12.2013 at 20:48 Lennart Poettering wrote: On Wed, 04.12.13 15:27, Maciej Wereski (m.were...@partner.samsung.com) wrote: +#ifdef HAVE_XATTR +static int get_xattrs_from_arg(Item *i){ +_cleanup_free_ char *xattr = NULL; +_cleanup_strv_free_ char **tmp = NULL; +char *p; +unsigned n, len, strv_len; + +assert(i); +if (i->type != SET_XATTR) +return 0; + +if (!i->argument) { +log_error("%s: Argument can't be empty!", i->path); +return -EBADMSG; +} +xattr = new0(char, strlen(i->argument)+1); +if (!xattr) +return log_oom(); + +tmp = strv_split(i->argument, WHITESPACE); +if (!tmp) +return log_oom(); + +strv_len = strv_length(tmp); +for (n = 0; n < strv_len; ++n) { Sounds like a job for the STRV_FOREACH() macro. Since you don't actually need the strv as strv here it sounds like you actually really want to use FOREACH_WORD_QUOTED() for this, which will also do the unquoting for you. Well, FOREACH_WORD_QUOTED() won't work properly, because quotation marks aren't first chars in strings (e.g. user.name="John Smith"). Maybe better idea would be to introduce mandatory separator (e.g. semicolon) instead of quotation marks. regards, -- Maciej Wereski Samsung R&D Institute Poland Samsung Electronics m.were...@partner.samsung.com ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCHv5] tmpfiles, man: Add xattr support to tmpfiles
On Wed, 11.12.13 14:24, Maciej Wereski (m.were...@partner.samsung.com) wrote: > >>+xattr = new0(char, strlen(i->argument)+1); > >>+if (!xattr) > >>+return log_oom(); > >>+ > >>+tmp = strv_split(i->argument, WHITESPACE); > >>+if (!tmp) > >>+return log_oom(); > >>+ > >>+strv_len = strv_length(tmp); > >>+for (n = 0; n < strv_len; ++n) { > > > >Sounds like a job for the STRV_FOREACH() macro. Since you don't actually > >need the strv as strv here it sounds like you actually really want to > >use FOREACH_WORD_QUOTED() for this, which will also do the unquoting for > >you. > > Well, FOREACH_WORD_QUOTED() won't work properly, because quotation marks > aren't first chars in strings (e.g. user.name="John Smith"). Maybe better > idea would be to introduce mandatory separator (e.g. semicolon) instead of > quotation marks. Yeah, FOREACH_WORD_QUOTED() is quite badly designed. We should fix it to do somewhat sane quoting and escaping. I'll look into it. Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCHv5] tmpfiles, man: Add xattr support to tmpfiles
11.12.2013 at 15:15 Lennart Poettering wrote: On Wed, 11.12.13 14:24, Maciej Wereski (m.were...@partner.samsung.com) wrote: >>+xattr = new0(char, strlen(i->argument)+1); >>+if (!xattr) >>+return log_oom(); >>+ >>+tmp = strv_split(i->argument, WHITESPACE); >>+if (!tmp) >>+return log_oom(); >>+ >>+strv_len = strv_length(tmp); >>+for (n = 0; n < strv_len; ++n) { > >Sounds like a job for the STRV_FOREACH() macro. Since you don't actually >need the strv as strv here it sounds like you actually really want to >use FOREACH_WORD_QUOTED() for this, which will also do the unquoting for >you. Well, FOREACH_WORD_QUOTED() won't work properly, because quotation marks aren't first chars in strings (e.g. user.name="John Smith"). Maybe better idea would be to introduce mandatory separator (e.g. semicolon) instead of quotation marks. Yeah, FOREACH_WORD_QUOTED() is quite badly designed. We should fix it to do somewhat sane quoting and escaping. I'll look into it. There is one problem with using it in this patch. In my case quotation mark isn't first char of the string, so using pointer and length won't get rid of it. String needs to be modified. regards, -- Maciej Wereski Samsung R&D Institute Poland Samsung Electronics m.were...@partner.samsung.com ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCHv5] tmpfiles, man: Add xattr support to tmpfiles
On Thu, 16.01.14 15:42, Maciej Wereski (m.were...@partner.samsung.com) wrote: > > 11.12.2013 at 15:15 Lennart Poettering wrote: > > >On Wed, 11.12.13 14:24, Maciej Wereski > >(m.were...@partner.samsung.com) wrote: > > > +xattr = new0(char, strlen(i->argument)+1); > +if (!xattr) > +return log_oom(); > + > +tmp = strv_split(i->argument, WHITESPACE); > +if (!tmp) > +return log_oom(); > + > +strv_len = strv_length(tmp); > +for (n = 0; n < strv_len; ++n) { > >>> > >>>Sounds like a job for the STRV_FOREACH() macro. Since you don't > >>actually > >>>need the strv as strv here it sounds like you actually really want to > >>>use FOREACH_WORD_QUOTED() for this, which will also do the > >>unquoting for > >>>you. > >> > >>Well, FOREACH_WORD_QUOTED() won't work properly, because quotation marks > >>aren't first chars in strings (e.g. user.name="John Smith"). > >>Maybe better > >>idea would be to introduce mandatory separator (e.g. semicolon) > >>instead of > >>quotation marks. > > > >Yeah, FOREACH_WORD_QUOTED() is quite badly designed. We should fix it to > >do somewhat sane quoting and escaping. I'll look into it. > > There is one problem with using it in this patch. In my case > quotation mark isn't first char of the string, so using pointer and > length won't get rid of it. String needs to be modified. Yeah, this has been a long-standing issue actually, the unquoting logic is pretty broken here, we should probably rework FOREACH_WORD_QUOTED() to allocate a buffer for this and do proper unquoting. I have been planning to do this for a while. In fact, we should probably have a closer look that we implement similar quoting rules for all file formats we define. Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel