Re: [systemd-devel] [PATCH v8] tmpfiles, man: Add xattr support to tmpfiles

[Lennart Poettering]

On Thu, 13.11.14 09:11, Maciej Wereski (m.were...@partner.samsung.com) wrote:

Sorry for the late review.

> +static int get_xattrs_from_arg(Item *i) {
> +char *xattr;
> +const char *p;
> +int n;
> +
> +assert(i);
> +if (i->type != SET_XATTR)
> +return 0;
> +
> +if (!i->argument) {
> +log_error("%s: Argument can't be empty!", i->path);
> +return -EBADMSG;
> +}
> +p = i->argument;
> +
> +while ((n = unquote_first_word(&p, &xattr, false)) > 0) {
> +if (strv_consume(&i->xattrs, xattr) < 0)
> +return log_oom();
> +}
> +
> +return n;

I think we really should parse the xattrs into key and value at the
time of parsing, not at the time of applying things. If something
parses incorrectly this should be noted before we do actually do
anything.

i think you can even stick to an strv for this, and simply store key
and value alternating in the list. We have STRV_FOREACH_PAIR in place
to iterate through such a list of pairs.

> -r = hashmap_put(h, i->path, i);
> -if (r < 0) {
> -log_error("Failed to insert item %s: %s", i->path, 
> strerror(-r));
> -return r;
> +if (i->type == SET_XATTR) {
> +char **tmp;
> +r = get_xattrs_from_arg(i);
> +if (r < 0)
> +return r;
> +tmp = existing->xattrs;
> +r = strv_extend_strv(&existing->xattrs, i->xattrs);
> +if (r < 0) {
> +strv_free(tmp);
> +return log_oom();
> +}

strv_extend_strv() actually realloc()s the existing array, you are
*not* supposed to free the old version! 

Lennart

-- 
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH v8] tmpfiles, man: Add xattr support to tmpfiles

[Maciej Wereski]

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
---
v8:
* update man

v7:
* use strv_consume() instead of strv_extend()
* use only lsetxattr()
* do not label in 'z' option
* style fixes and cleanup

v6:
* rebase
* man fixes
* use glibc xattr
* use unquote_first_word() instead of own parsing logic

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  |  32 +--
 src/tmpfiles/tmpfiles.c | 145 
 2 files changed, 159 insertions(+), 18 deletions(-)

diff --git a/man/tmpfiles.d.xml b/man/tmpfiles.d.xml
index 1b14d69..4f2e640 100644
--- a/man/tmpfiles.d.xml
+++ b/man/tmpfiles.d.xml
@@ -343,6 +343,25 @@ L/tmp/foobar ----   
/dev/null
 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, 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.
+
+
 
 
 If the exclamation mark is used, this
@@ -430,7 +449,7 @@ r! /tmp/.X[0-9]*-lock
 will not be modified. This parameter is
 ignored for x,
 r, R,
-L lines.
+L, t 
lines.
 
 Optionally, if prefixed with
 ~, the access mode is masked
@@ -462,8 +481,8 @@ r! /tmp/.X[0-9]*-lock
 ownership will not be modified. These
 parameters are ignored for
 x, r,
-R, L
-lines.
+R, L,
+t lines.
 
 
 
@@ -527,8 +546,8 @@ r! /tmp/.X[0-9]*-lock
 specify a short string that is written to the
 file, suffixed by a newline. For
 C, specifies the source file
-or directory. Ignored for all other
-lines.
+or directory. For t determines
+extended attributes to be set. Ignored for all other 
lines.
 
 
 
@@ -540,7 +559,8 @@ r! /tmp/.X[0-9]*-lock
 screen needs two directories 
created at boot with specific modes and ownership.
 
 d /run/screens  1777 root root 10d
-d /run/uscreens 0755 root root 10d12h
+d /run/uscreens 0755 root root 10d12h
+t /run/screen - - - - user.name="John Smith" 
security.SMACK64=screen
 
 
 /etc/tmpfiles.d/abrt.conf example
diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c
index 1e4675f..c5bb4e7 100644
--- a/src/tmpfiles/tmpfiles.c
+++ b/src/tmpfiles/tmpfiles.c
@@ -39,6 +39,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "log.h"
 #include "util.h"
@@ -71,6 +72,7 @@ typedef enum ItemType {
 CREATE_CHAR_DEVICE = 'c',
 CREATE_BLOCK_DEVICE = 'b',
 COPY_FILES = 'C',
+SET_XATTR = 't',