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

2014-01-17 Thread Lennart Poettering
On Thu, 16.01.14 15:42, Maciej Wereski (m.were...@partner.samsung.com) wrote:

 
 11.12.2013 at 15:15 Lennart Poettering lenn...@poettering.net 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


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

2014-01-16 Thread Maciej Wereski

11.12.2013 at 15:15 Lennart Poettering lenn...@poettering.net 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 RD 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

2013-12-11 Thread Maciej Wereski

10.12.2013 at 20:48 Lennart Poettering lenn...@poettering.net 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 RD 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

2013-12-11 Thread Lennart Poettering
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


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

2013-12-04 Thread 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
---
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/programlisting
 place of 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 d, D, f, F, L, p, c, b, z
+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
 /refsect2
 
@@ -312,7 +327,7 @@ L/tmp/foobar ----   
/dev/null/programlisting
 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./para
+R, L, t lines./para
 /refsect2
 
 refsect2
@@ -324,7 +339,7 @@ L/tmp/foobar ----   
/dev/null/programlisting
 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./para
+These parameters are ignored for x, r, R, L, t 
lines./para
 /refsect2
 
 refsect2
@@ -377,8 +392,10 @@ L/tmp/foobar ----   
/dev/null/programlisting
 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./para
+
 /refsect2
 
 /refsect1
@@ -390,7 +407,8 @@ L/tmp/foobar ----   
/dev/null/programlisting
 paracommandscreen/command needs two directories 
created at boot with specific modes and ownership./para
 
 programlistingd /var/run/screens  1777 root root 10d
-d /var/run/uscreens 0755 root root 10d12h/programlisting
+d /var/run/uscreens 0755 root root 10d12h
+t /var/run/screen - - - - user.name=John Koval 
security.SMACK64=screen/programlisting
 /example
 example
 title/etc/tmpfiles.d/abrt.conf example/title
diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c
index b7f6a2e..ec5efb6 100644
--- a/src/tmpfiles/tmpfiles.c
+++