[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
