Re: [systemd-devel] [PATCH] tmpfiles, man: Add xattr support to tmpfiles
On Wed, 19.06.13 17:02, Maciej Wereski (m.were...@partner.samsung.com) wrote: Heya, I think adding this certainly makes sense, but I am not sure I like the syntax. Maybe it would be simpler to add an extra char for this (a or so?). That way creating a dir and applying an xattr would require two lines instead of one, but the stuff isn't atomic anyway. Admittedly adding a new a isn't particularly nice either, but I have no better idea than that... I've looked into your way and found some problems. In parse_line(), after creating, item is added to hashmap. Key is path, which already exists in map. So adding a would require changing key (path + type?). Hmm, no actually, the best approach is to just update the existing entry I think. So, currently we go through the files line by line and then check if there's already a line for this specific path, and if so, if it is conflicting. Now, what I'd like to see is a slight extension to this for the xattr case: if you have an a line, and there's already some other line, and they match in everything, then simply add the xattr to the existing line. We will apply all attributes only after parsing all files, hence everything should be pretty clean. Of course, you need to be careful when merging the two lines. i.e. you cannot merge all kinds of lines, only some combinations make sense (i.e. merging r and a certainly doesn't make sense. f and a certainly does). Also, be careful that listing first the xattr line and then the other one works equally well as doing it the other way round. I guess a lines should dissolve into other lines, and only survive on their own if there are no other lines matching the same file name. I figure in the long run we might want to come up with an algebra for merging more kinds of line type combinations, but for now I'd keep it simple, and allow merging only these a lines with the very specific other line types you need or where things are obvious. If figure merging a into f, F, w, d, D, p, c, b, z probably makes sense, and that's it. And then later on we could do a similar logic for applying acls, and maybe z should be one of the merging types that is dissolved by others too... But yeah, that shouldn't be of your concern (unless you want to hack this up, too, would be much appreciated ;-)). I've also found something which looks like a typo in lines 782 - 787: case RELABEL_PATH: r = glob_item(i, item_set_perms); if (r 0) return 0; break; Shouldn't it be return r? If it's not, then should I add comment, that it's on purpose? That looks like an error. Fixed locally now, will push soon! Thanks! Lennart -- Lennart Poettering - Red Hat, Inc. ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] tmpfiles, man: Add xattr support to tmpfiles
Hello, On 17.06.2013 at 18:18 Lennart Poettering lenn...@poettering.net wrote: I think adding this certainly makes sense, but I am not sure I like the syntax. Maybe it would be simpler to add an extra char for this (a or so?). That way creating a dir and applying an xattr would require two lines instead of one, but the stuff isn't atomic anyway. Admittedly adding a new a isn't particularly nice either, but I have no better idea than that... I've looked into your way and found some problems. In parse_line(), after creating, item is added to hashmap. Key is path, which already exists in map. So adding a would require changing key (path + type?). Problem on user side is that order matters - if user would add a entry first, than setting attribute would fail, because file wouldn't exist yet. Should I continue adding a, look into Karols proposition or my original patch is acceptable? Anybody having other ideas? I've also found something which looks like a typo in lines 782 - 787: case RELABEL_PATH: r = glob_item(i, item_set_perms); if (r 0) return 0; break; Shouldn't it be return r? If it's not, then should I add comment, that it's on purpose? regards, Maciej -- 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] [PATCH] tmpfiles, man: Add xattr support to tmpfiles
2013/6/17 Lennart Poettering lenn...@poettering.net: On Mon, 17.06.13 16:27, Maciej Wereski (m.were...@partner.samsung.com) wrote: 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. To keep backwards compatibility Argument field is used. If word starts with xattr=, then it is cut out from Argument and parsed. There may be many xattrs. Full format is: xattr=name=value 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. I think adding this certainly makes sense, but I am not sure I like the syntax. Maybe it would be simpler to add an extra char for this (a or so?). That way creating a dir and applying an xattr would require two lines instead of one, but the stuff isn't atomic anyway. Admittedly adding a new a isn't particularly nice either, but I have no better idea than that... FWIW, I would like note that we might see similar problem if... when somebody will try to extend tmpfiles with ACLs. That would result in another type and another line. Maybe Type argument could be extended to contain one primary type and 0 to n optional subtypes where n-th subtype would parse n-th semicolon-terminated argument, i.e. fa /tmp/foobar ---- /dev/null ; security.SMACK=foo fxa /tmp/foobar ---- /dev/null ; security.SMACK=foo ; lmctl=rwx x - xattrs, a - acls I do agree this is a bit abusive... Cheers [ Lennart, sorry for two copies - first one didn't contain ML. ] ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] 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.
To keep backwards compatibility Argument field is used. If word starts
with xattr=, then it is cut out from Argument and parsed. There may be
many xattrs. Full format is:
xattr=name=value
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 - - - - xattr=security.SMACK64=printing
---
man/tmpfiles.d.xml | 22 ++-
src/tmpfiles/tmpfiles.c | 158
2 files changed, 179 insertions(+), 1 deletion(-)
diff --git a/man/tmpfiles.d.xml b/man/tmpfiles.d.xml
index 519f9bc..36021b1 100644
--- a/man/tmpfiles.d.xml
+++ b/man/tmpfiles.d.xml
@@ -309,6 +309,26 @@ L/tmp/foobar ----
/dev/null/programlisting
a short string that is written to the file,
suffixed by a newline. Ignored for all other
lines./para
+
+paraIf commandsystemd-tmpfiles/command was
+compiled with extended attributes support, then
+argument field can be used to set extended attributes.
+Such argument should follow format:/para
+
+
programlistingxattr=replaceablename/replaceable=replaceablevalue/replaceable/programlisting
+
+paraWhere replaceablename/replaceable is
extended
+attribute name and replaceablevalue/replaceable is
+value to be set. If value contains spaces, it must be
+between quotation marks. If value contains quotation
+mark, which should be set, then it must be escaped
with
+backslash./para
+
+paraSuch special arguments are cut out and from
argument
+field, so there's no influence on other meanigs of
argument
+field./para
+
+paraxattr= can be especially used to set SMACK
security labels./para
/refsect2
/refsect1
@@ -320,7 +340,7 @@ 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 xattr=user.owner=John Koval
xattr=security.SMACK64=screen/programlisting
/example
/refsect1
diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c
index f4885ec..9869e58 100644
--- a/src/tmpfiles/tmpfiles.c
+++ b/src/tmpfiles/tmpfiles.c
@@ -39,6 +39,9 @@
#include glob.h
#include fnmatch.h
#include sys/capability.h
+#ifdef HAVE_XATTR
+#include attr/xattr.h
+#endif
#include log.h
#include util.h
@@ -83,6 +86,7 @@ typedef struct Item {
char *path;
char *argument;
+char **xattrs;
uid_t uid;
gid_t gid;
mode_t mode;
@@ -447,6 +451,52 @@ static int item_set_perms(Item *i, const char *path) {
return label_fix(path, false, false);
}
+static int item_set_xattrs(Item *i, const char *path) {
+#ifdef HAVE_XATTR
+char *name, *value;
+char **x;
+int n;
+if (!i-xattrs)
+return 0;
+STRV_FOREACH(x, i-xattrs) {
+value = *x;
+name = strsep(value, =);
+if (name == NULL || value == NULL) {
+log_warning(%s: %s is not valid xattr, ignoring.,
path, *x);
+continue;
+}
+n = strlen(value);
+if (value[n-1] == '\')
+value[n-1] = '\0';
+if (value[0] == '\')
+memmove(value, value+1, n);
+value = strreplace(value, \\\, \);
+if (!value)
+return log_oom();
+n = strlen(value);
+if (i-type == CREATE_SYMLINK) {
+if (lsetxattr(path, name, value, n+1, 0) 0) {
+log_error(lsetxattr(%s) failed: %m, path);
+free(value);
+return -errno;
+}
+}
+else if (setxattr(path, name, value, n+1, 0) 0) {
+log_error(setxattr(%s) failed: %m, path);
+free(value);
+return -errno;
+}
+free(value);
+}
+
Re: [systemd-devel] [PATCH] tmpfiles, man: Add xattr support to tmpfiles
On Mon, 17.06.13 16:27, Maciej Wereski (m.were...@partner.samsung.com) wrote: 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. To keep backwards compatibility Argument field is used. If word starts with xattr=, then it is cut out from Argument and parsed. There may be many xattrs. Full format is: xattr=name=value 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. I think adding this certainly makes sense, but I am not sure I like the syntax. Maybe it would be simpler to add an extra char for this (a or so?). That way creating a dir and applying an xattr would require two lines instead of one, but the stuff isn't atomic anyway. Admittedly adding a new a isn't particularly nice either, but I have no better idea than that... Lennart -- Lennart Poettering - Red Hat, Inc. ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
