[systemd-devel] [PATCH v9] 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
---
v9:
* fully parse xattrs in one place
* remove potential double free()

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 | 150 
 2 files changed, 166 insertions(+), 16 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/programlisting
 normal path
 names./para/listitem
 /varlistentry
+
+varlistentry
+termvarnamet/varname/term
+listitemparaSet extended
+attributes on item. It may be
+used in conjunction with other
+types (only varnamed/varname,
+varnameD/varname, 
varnamef/varname,
+varnameF/varname, 
varnameL/varname,
+varnamep/varname, 
varnamec/varname,
+varnameb/varname, makes sense).
+If used as a standalone line, then
+commandsystemd-tmpfiles/command
+will try to set extended
+attributes on specified path.
+This can be especially used to set
+SMACK labels.
+/para/listitem
+/varlistentry
 /variablelist
 
 paraIf the exclamation mark is used, this
@@ -430,7 +449,7 @@ r! /tmp/.X[0-9]*-lock/programlisting
 will not be modified. This parameter is
 ignored for varnamex/varname,
 varnamer/varname, varnameR/varname,
-varnameL/varname lines./para
+varnameL/varname, varnamet/varname 
lines./para
 
 paraOptionally, if prefixed with
 literal~/literal, the access mode is masked
@@ -462,8 +481,8 @@ r! /tmp/.X[0-9]*-lock/programlisting
 ownership will not be modified. These
 parameters are ignored for
 varnamex/varname, varnamer/varname,
-varnameR/varname, varnameL/varname
-lines./para
+varnameR/varname, varnameL/varname,
+varnamet/varname lines./para
 /refsect2
 
 refsect2
@@ -527,8 +546,8 @@ r! /tmp/.X[0-9]*-lock/programlisting
 specify a short string that is written to the
 file, suffixed by a newline. For
 varnameC/varname, specifies the source file
-or directory. Ignored for all other
-lines./para
+or directory. For varnamet/varname determines
+extended attributes to be set. Ignored for all other 
lines./para
 /refsect2
 
 /refsect1
@@ -540,7 +559,8 @@ r! /tmp/.X[0-9]*-lock/programlisting
 paracommandscreen/command needs two directories 
created at boot with specific modes and ownership./para
 
   

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

[Lennart Poettering]

On Wed, 03.12.14 15:33, Maciej Wereski (m.were...@partner.samsung.com) wrote:

  
 +static int get_xattrs_from_arg(Item *i) {
 +char *xattr, *name, *value;
 +const char *p;
 +int r;

Please declare name and value in the inner scope, no need to define
them broader than necessary.

 +
 +assert(i);
 +
 +if (!i-argument) {
 +log_error(%s: Argument can't be empty!, i-path);
 +return -EBADMSG;
 +}
 +p = i-argument;
 +
 +while ((r = unquote_first_word(p, xattr, false))  0) {
 +_cleanup_free_ char *tmp = NULL;
 +r = split_pair(xattr, =, name, value);
 +if (r  0) {
 +log_warning(Illegal xattr found: \%s\ - 
 ignoring., xattr);
 +free(xattr);
 +continue;
 +}
 +free(xattr);
 +if (streq(name, ) || streq(value, )) {
 +log_warning(Malformed xattr found: \%s=%s\ - 
 ignoring., name, value);
 +free(name);
 +free(value);
 +continue;
 +}
 +tmp = unquote(value, \);
 +if (!tmp)
 +return log_oom();

If this OOM path is hit, you don't free name nor value. I'd probably
declare them in the inner scope with _cleanup_free_ so that they are
automatically freed (which of course means you need to override the
vars with NULL explicitly if you want to disable the freeing.

 +free(value);
 +value = cunescape(tmp);
 +if (!value) {
 +free(name);
 +return log_oom();
 +}
 +if (strv_consume(i-xattrs, name)  0) {
 +free(value);
 +return log_oom();
 +}
 +if (strv_consume(i-xattrs, value)  0){
 +free(value);
 +return log_oom();
 +}

strv_consume() actually frees the passed string on failure anyway,
that's why it is call consume... The code above would hence free
value twice on failure. Use strv_push() if you don't want the call
to free the string on failure...

Hmm, I think it would be a good idea to introduce a new
strv_push_pair() and strv_consume_pair() for cases like this, which
add two elements at once. I added this now for you, this should make
the code a bit simple for you.

 +}
 +
 +r = strv_length(i-xattrs) % 2 != 0 ? -EBADMSG : 0;

This check appears unnecessary if the inner error checks work
correctly. (Also it should really print a message on failure here...)

Otherwise looks good!

Lennart

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