[RFC][PATCH] fs: set xattrs in initramfs from regular files

Although rootfs (tmpfs) supports xattrs, they are not set due to the
limitation of the cpio format. A new format called 'newcx' was proposed to
overcome this limitation.

However, it looks like that adding a new format is not simple: 15 kernel
patches; user space tools must support the new format; mistakes made in the
past should be avoided; it is unclear whether the kernel should switch from
cpio to tar.

The aim of this patch is to provide the same functionality without
introducing a new format. The value of xattrs is placed in regular files
having the same file name as the files xattrs are added to, plus a
separator and the xattr name (<filename>.xattr-<xattr name>).

Example:

'/bin/cat.xattr-security.ima' is the name of a file containing the value of
the security.ima xattr to be added to /bin/cat.

At kernel initialization time, the kernel iterates over the rootfs
filesystem, and if it encounters files with the '.xattr-' separator, it
reads the content and adds the xattr to the file without the suffix.

This proposal requires that LSMs and IMA allow the read and setxattr
operations. This should not be a concern since: files with xattr values
are not parsed by the kernel; user space processes are not yet executed.

It would be possible to include all xattrs in the same file, but this
increases the risk of the kernel being compromised by parsing the content.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 fs/Makefile        |   2 +-
 fs/readxattr.c     | 171 +++++++++++++++++++++++++++++++++++++++++++++
 include/linux/fs.h |   2 +
 init/main.c        |   1 +
 4 files changed, 175 insertions(+), 1 deletion(-)
 create mode 100644 fs/readxattr.c

diff --git a/fs/Makefile b/fs/Makefile
index 293733f61594..738e1a4e4aff 100644
--- a/fs/Makefile
+++ b/fs/Makefile

@@ -12,7 +12,7 @@ obj-y :=	open.o read_write.o file_table.o super.o \
 		attr.o bad_inode.o file.o filesystems.o namespace.o \
 		seq_file.o xattr.o libfs.o fs-writeback.o \
 		pnode.o splice.o sync.o utimes.o d_path.o \
-		stack.o fs_struct.o statfs.o fs_pin.o nsfs.o
+		stack.o fs_struct.o statfs.o fs_pin.o nsfs.o readxattr.o
 
 ifeq ($(CONFIG_BLOCK),y)
 obj-y +=	buffer.o block_dev.o direct-io.o mpage.o

diff --git a/fs/readxattr.c b/fs/readxattr.c
new file mode 100644
index 000000000000..01838f6f1e92
--- /dev/null
+++ b/fs/readxattr.c

@@ -0,0 +1,171 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2018 Huawei Technologies Duesseldorf GmbH
+ *
+ * Author: Roberto Sassu <roberto.sassu@huawei.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation, version 2 of the
+ * License.
+ *
+ * File: readxattr.c
+ *      Read extended attributes from regular files in the initial ram disk
+ */
+
+#include <linux/kernel.h>
+#include <linux/export.h>
+#include <linux/xattr.h>
+#include <linux/file.h>
+#include <linux/cred.h>
+#include <linux/namei.h>
+#include <linux/fs.h>
+
+#include "internal.h"
+
+#define SETXATTR_FILENAME ".setxattr"
+#define FILENAME_XATTR_SEP ".xattr-"
+
+
+struct readdir_callback {
+	struct dir_context ctx;
+	struct path *path;
+};
+
+LIST_HEAD(dir_list);
+
+struct dir_path {
+	struct list_head next;
+	struct path path;
+};
+
+static int __init read_set_xattr(struct dir_context *__ctx, const char *name,
+				 int namelen, loff_t offset, u64 ino,
+				 unsigned int d_type)
+{
+	struct readdir_callback *ctx = container_of(__ctx, typeof(*ctx), ctx);
+	struct path *dir = ctx->path, source_path, target_path;
+	char filename[NAME_MAX + 1], *xattrname, *separator;
+	struct dir_path *subdir;
+	struct file *file;
+	void *datap;
+	loff_t size;
+	int result;
+
+	if (!strcmp(name, ".") || !strcmp(name, ".."))
+		return 0;
+
+	result = vfs_path_lookup(dir->dentry, dir->mnt, name, 0, &source_path);
+	if (result)
+		return 0;
+
+	size = i_size_read(source_path.dentry->d_inode);
+	if (size > XATTR_SIZE_MAX)
+		goto out;
+
+	if (source_path.dentry->d_inode->i_sb != dir->dentry->d_inode->i_sb)
+		goto out;
+
+	if (!S_ISREG(source_path.dentry->d_inode->i_mode) &&
+	    !S_ISDIR(source_path.dentry->d_inode->i_mode))
+		goto out;
+
+	if (S_ISREG(source_path.dentry->d_inode->i_mode)) {
+		separator = strstr(name, FILENAME_XATTR_SEP);
+		if (!separator)
+			goto out;
+
+		xattrname = separator + sizeof(FILENAME_XATTR_SEP) - 1;
+		if (strlen(xattrname) > XATTR_NAME_MAX)
+			goto out;
+	} else {
+		subdir = kmalloc(sizeof(*subdir), GFP_KERNEL);
+		if (subdir) {
+			subdir->path.dentry = source_path.dentry;
+			subdir->path.mnt = source_path.mnt;
+
+			list_add(&subdir->next, &dir_list);
+		}
+
+		return 0;
+	}
+
+	file = dentry_open(&source_path, O_RDONLY, current_cred());
+	if (IS_ERR(file))
+		goto out;
+
+	result = kernel_read_file(file, &datap, &size, 0, READING_XATTR);
+	if (result)
+		goto out_fput;
+
+	if (separator != name) {
+		snprintf(filename, sizeof(filename), "%.*s",
+			 (int)(namelen - strlen(separator)), name);
+
+		result = vfs_path_lookup(dir->dentry, dir->mnt, filename, 0,
+					&target_path);
+		if (result)
+			goto out_vfree;
+
+		inode_lock(target_path.dentry->d_inode);
+	} else {
+		target_path.dentry = dir->dentry;
+		target_path.mnt = dir->mnt;
+	}
+
+	__vfs_setxattr_noperm(target_path.dentry, xattrname, datap, size, 0);
+
+	if (separator != name) {
+		inode_unlock(target_path.dentry->d_inode);
+		path_put(&target_path);
+	}
+out_vfree:
+	vfree(datap);
+out_fput:
+	fput(file);
+out:
+	path_put(&source_path);
+	return 0;
+}
+
+void __init set_xattrs_initrd(void)
+{
+	struct readdir_callback buf = {
+		.ctx.actor = read_set_xattr,
+	};
+
+	struct dir_path dir, *cur_dir;
+	struct path path;
+	struct file *file;
+	int result;
+
+	result = kern_path(SETXATTR_FILENAME, 0, &path);
+	if (result)
+		return;
+
+	path_put(&path);
+
+	result = kern_path("/", 0, &dir.path);
+	if (result)
+		return;
+
+	list_add(&dir.next, &dir_list);
+
+	while (!list_empty(&dir_list)) {
+		cur_dir = list_first_entry(&dir_list, typeof(*cur_dir), next);
+
+		file = dentry_open(&cur_dir->path, O_RDONLY, current_cred());
+		if (file) {
+			buf.path = &cur_dir->path;
+			iterate_dir(file, &buf.ctx);
+			fput(file);
+		}
+
+		path_put(&cur_dir->path);
+		list_del(&cur_dir->next);
+
+		if (cur_dir != &dir)
+			kfree(cur_dir);
+	}
+}
+EXPORT_SYMBOL_GPL(set_xattrs_initrd);

diff --git a/include/linux/fs.h b/include/linux/fs.h
index c95c0807471f..b04edc1c32e9 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h

@@ -2894,6 +2894,7 @@ extern int do_pipe_flags(int *, int);
 	id(KEXEC_INITRAMFS, kexec-initramfs)	\
 	id(POLICY, security-policy)		\
 	id(X509_CERTIFICATE, x509-certificate)	\
+	id(XATTR, xattr)	\
 	id(MAX_ID, )
 
 #define __fid_enumify(ENUM, dummy) READING_ ## ENUM,

@@ -3156,6 +3157,7 @@ const char *simple_get_link(struct dentry *, struct inode *,
 extern const struct inode_operations simple_symlink_inode_operations;
 
 extern int iterate_dir(struct file *, struct dir_context *);
+extern void set_xattrs_initrd(void);
 
 extern int vfs_statx(int, const char __user *, int, struct kstat *, u32);
 extern int vfs_statx_fd(unsigned int, struct kstat *, u32, unsigned int);

diff --git a/init/main.c b/init/main.c
index ee147103ba1b..a2f63bc8f9d4 100644
--- a/init/main.c
+++ b/init/main.c

@@ -1180,5 +1180,6 @@ static noinline void __init kernel_init_freeable(void)
 	 */
 
 	integrity_load_keys();
+	set_xattrs_initrd();
 	load_default_modules();
 }

-- 
2.17.1

On 11/22/18 9:49 AM, Roberto Sassu wrote:

Although rootfs (tmpfs) supports xattrs, they are not set due to the
limitation of the cpio format. A new format called 'newcx' was proposed to
overcome this limitation.

I got email about that format the day before you posted this, by the way.

However, it looks like that adding a new format is not simple: 15 kernel
patches; user space tools must support the new format; mistakes made in the
past should be avoided; it is unclear whether the kernel should switch from
cpio to tar.

The kernel _can't_ switch from cpio to tar without breaking backwards
compatability, it could only add tar as a second format it supported (remember
cpio images can be sideloaded so a new rootfs can be used with an existing
initramfs, plus existing build systems generate them and would still need to if
they wanted to keep supporting older kernels), and then once you've got two
formats somebody will propose zip support, and let's just not go there please.

The changes to the userspace tools are trivial (I say that as the maintainer of
toybox, which has a cpio). The argument was about things like 64 bit timestamps
(y2038 problem), nanosecond support, sparse files, etc. And I think the argument
had largely died down?

Keep in mind the squashfs guy spent 5 years trying to get his filesystem merged
(https://lwn.net/Articles/563578/), I spent several years trying to get my perl
removal patch merged (and only work up the enthusiasm to resubmit
http://lists.busybox.net/pipermail/buildroot/2015-March/123385.html
https://patchwork.kernel.org/patch/9193529/ https://lkml.org/lkml/2017/9/13/651
about once a year because dealing with linux-kernel is just no fun for hobbyists
anymore).

The aim of this patch is to provide the same functionality without
introducing a new format. The value of xattrs is placed in regular files
having the same file name as the files xattrs are added to, plus a
separator and the xattr name (<filename>.xattr-<xattr name>).

I think you're solving the wrong problem, but that's just my opinion.

Rob

On 11/23/2018 11:30 AM, Mimi Zohar wrote:

On Fri, 2018-11-23 at 11:03 -0800, Casey Schaufler wrote:

quoted

On 11/22/2018 7:49 AM, Roberto Sassu wrote:

quoted

Although rootfs (tmpfs) supports xattrs, they are not set due to the
limitation of the cpio format. A new format called 'newcx' was proposed to
overcome this limitation.

However, it looks like that adding a new format is not simple: 15 kernel
patches; user space tools must support the new format; mistakes made in the
past should be avoided; it is unclear whether the kernel should switch from
cpio to tar.

The aim of this patch is to provide the same functionality without
introducing a new format. The value of xattrs is placed in regular files
having the same file name as the files xattrs are added to, plus a
separator and the xattr name (<filename>.xattr-<xattr name>).

Example:

'/bin/cat.xattr-security.ima' is the name of a file containing the value of
the security.ima xattr to be added to /bin/cat.

At kernel initialization time, the kernel iterates over the rootfs
filesystem, and if it encounters files with the '.xattr-' separator, it
reads the content and adds the xattr to the file without the suffix.

No.

Really, no.

It would be incredibly easy to use this mechanism to break
into systems.
 

quoted

This proposal requires that LSMs and IMA allow the read and setxattr
operations. This should not be a concern since: files with xattr values
are not parsed by the kernel; user space processes are not yet executed.

It would be possible to include all xattrs in the same file, but this
increases the risk of the kernel being compromised by parsing the content.

The kernel mustn't do this.

Mustn't do what?  Store the xattr as separate detached files, 
include all the xattrs in a single or per security/LSM xattr attribute
file(s), or either?

Any and all of the above. The proposed behavior is a kludge
around making the installation tools work correctly. Sure, it
may be easier to change the kernel than to change the utilities.
That's doesn't make it right.

Mimi

On 11/22/2018 7:49 AM, Roberto Sassu wrote:

Although rootfs (tmpfs) supports xattrs, they are not set due to the
limitation of the cpio format. A new format called 'newcx' was proposed to
overcome this limitation.

However, it looks like that adding a new format is not simple: 15 kernel
patches; user space tools must support the new format; mistakes made in the
past should be avoided; it is unclear whether the kernel should switch from
cpio to tar.

The aim of this patch is to provide the same functionality without
introducing a new format. The value of xattrs is placed in regular files
having the same file name as the files xattrs are added to, plus a
separator and the xattr name (<filename>.xattr-<xattr name>).

Example:

'/bin/cat.xattr-security.ima' is the name of a file containing the value of
the security.ima xattr to be added to /bin/cat.

At kernel initialization time, the kernel iterates over the rootfs
filesystem, and if it encounters files with the '.xattr-' separator, it
reads the content and adds the xattr to the file without the suffix.

No.

Really, no.

It would be incredibly easy to use this mechanism to break
into systems.
 

This proposal requires that LSMs and IMA allow the read and setxattr
operations. This should not be a concern since: files with xattr values
are not parsed by the kernel; user space processes are not yet executed.

It would be possible to include all xattrs in the same file, but this
increases the risk of the kernel being compromised by parsing the content.

The kernel mustn't do this.

On Fri, 2018-11-23 at 11:03 -0800, Casey Schaufler wrote:

On 11/22/2018 7:49 AM, Roberto Sassu wrote:

quoted

Although rootfs (tmpfs) supports xattrs, they are not set due to the
limitation of the cpio format. A new format called 'newcx' was proposed to
overcome this limitation.

However, it looks like that adding a new format is not simple: 15 kernel
patches; user space tools must support the new format; mistakes made in the
past should be avoided; it is unclear whether the kernel should switch from
cpio to tar.

The aim of this patch is to provide the same functionality without
introducing a new format. The value of xattrs is placed in regular files
having the same file name as the files xattrs are added to, plus a
separator and the xattr name (<filename>.xattr-<xattr name>).

Example:

'/bin/cat.xattr-security.ima' is the name of a file containing the value of
the security.ima xattr to be added to /bin/cat.

At kernel initialization time, the kernel iterates over the rootfs
filesystem, and if it encounters files with the '.xattr-' separator, it
reads the content and adds the xattr to the file without the suffix.

No.

Really, no.

It would be incredibly easy to use this mechanism to break
into systems.
 

quoted

This proposal requires that LSMs and IMA allow the read and setxattr
operations. This should not be a concern since: files with xattr values
are not parsed by the kernel; user space processes are not yet executed.

It would be possible to include all xattrs in the same file, but this
increases the risk of the kernel being compromised by parsing the content.

The kernel mustn't do this.

Mustn't do what?  Store the xattr as separate detached files, 
include all the xattrs in a single or per security/LSM xattr attribute
file(s), or either?

Mimi

On 11/26/2018 6:42 PM, Rob Landley wrote:

On 11/26/18 6:56 AM, Roberto Sassu wrote:

quoted

On 11/23/2018 9:21 PM, Rob Landley wrote:

quoted

quoted

The aim of this patch is to provide the same functionality without
introducing a new format. The value of xattrs is placed in regular files
having the same file name as the files xattrs are added to, plus a
separator and the xattr name (<filename>.xattr-<xattr name>).

I think you're solving the wrong problem, but that's just my opinion.

Instead of iterating over rootfs, would it be better to detect files
with extended attributes (from the file name) when the cpio image is
parsed by the kernel,

Huh, I thought at first glance that's what the new approach _was_ doing.

In band signaling in the archive is ugly, still requires new tools to create it,

For SElinux, the changes would be minimal. Instead of adding the
xattr, setfiles would create a regular file with the suffix, in the
temporary directory containing the files to be added to the CPIO image.

For IMA, I think there is also a tool to write file signatures. It
shouldn't be a problem to do the same modification I proposed for
SELinux.

doesn't address the y2038 issue... (Although we could cheat, treat the time
signature as unsigned, and buy another few decades.)

But doing that in the filesystem _after_ you extract the archive is beyond ugly.

quoted

and call sys_lsetxattr() in do_copy()? This part
can be turned on by introducing a new type in the existing format (if
possible).

The impact of this alternative is very low, and LSMs/IMA would be able,
with minimum effort, to enforce policies on files in the initial ram
disk.

The cpio extension isn't a big deal, I was pondering doing it myself in toybox
(and submitting a kernel patch to consume the output) before Mimi approached me.
(I did the initmpfs stuff already, I've stomped around in this area before.) I
just didn't because mimi sent their patch first, so I waited for that to work
its way though.

Unfortunately, it's simple enough that there was a bit of bikeshedding. (You can
store time in milliseconds as a 64 bit number without worrying about the range,
but if you do it as nanoseconds you need two fields, but people spoke up and
said "but if you don't store the nanoseconds the rounding causes spurious time
differences when between filesystems and it confuses our build system about what
has and hasn't changed"...)

The new in-band signaling proposal is, at best, a hack. (Filename lengths are
capped at 255 in the VFS, can you strip the xattrs on a long filename by having
the extension fail to create in the filesystem? Or do you have an arbitrary max
length on filenames because you need to reserve room for the extension?)

Yes, that would be a limitation. Alternatively, files with xattrs could
be placed in a subdir. For example:

/bin/cat
/bin/.xattr-<xattr name>/cat

Roberto

Rob

-- 
HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Bo PENG, Jian LI, Yanli SHI

On Fri, 2018-11-23 at 18:07 -0800, Casey Schaufler wrote:

On 11/23/2018 11:30 AM, Mimi Zohar wrote:

quoted

On Fri, 2018-11-23 at 11:03 -0800, Casey Schaufler wrote:

quoted

On 11/22/2018 7:49 AM, Roberto Sassu wrote:

quoted

Although rootfs (tmpfs) supports xattrs, they are not set due to the
limitation of the cpio format. A new format called 'newcx' was proposed to
overcome this limitation.

However, it looks like that adding a new format is not simple: 15 kernel
patches; user space tools must support the new format; mistakes made in the
past should be avoided; it is unclear whether the kernel should switch from
cpio to tar.

The aim of this patch is to provide the same functionality without
introducing a new format. The value of xattrs is placed in regular files
having the same file name as the files xattrs are added to, plus a
separator and the xattr name (<filename>.xattr-<xattr name>).

Example:

'/bin/cat.xattr-security.ima' is the name of a file containing the value of
the security.ima xattr to be added to /bin/cat.

At kernel initialization time, the kernel iterates over the rootfs
filesystem, and if it encounters files with the '.xattr-' separator, it
reads the content and adds the xattr to the file without the suffix.

No.

Really, no.

It would be incredibly easy to use this mechanism to break
into systems.

Assuming that the initramfs itself was signed, how?

quoted

quoted

quoted

This proposal requires that LSMs and IMA allow the read and setxattr
operations. This should not be a concern since: files with xattr values
are not parsed by the kernel; user space processes are not yet executed.

It would be possible to include all xattrs in the same file, but this
increases the risk of the kernel being compromised by parsing the content.

The kernel mustn't do this.

Mustn't do what?  Store the xattr as separate detached files, 
include all the xattrs in a single or per security/LSM xattr attribute
file(s), or either?

Any and all of the above. The proposed behavior is a kludge
around making the installation tools work correctly. Sure, it
may be easier to change the kernel than to change the utilities.
That's doesn't make it right.

Modifying userspace tools, as Rob Landley pointed out in terms of
toybox, isn't difficult.  The difficulty has been in reviewing and
upstreaming the kernel CPIO changes.

This patch was posted in order to address the lack of xattr support in
the initramfs.  Before totally dismissing this or a similar solution,
is there a safe method for including the xattrs?

Would defining an LSM hook here help?  Each LSM would define its own
method for storing and applying, or restoring, xattr labels.

Mimi

On 11/23/2018 9:21 PM, Rob Landley wrote:

On 11/22/18 9:49 AM, Roberto Sassu wrote:

quoted

Although rootfs (tmpfs) supports xattrs, they are not set due to the
limitation of the cpio format. A new format called 'newcx' was proposed to
overcome this limitation.

I got email about that format the day before you posted this, by the way.

quoted

However, it looks like that adding a new format is not simple: 15 kernel
patches; user space tools must support the new format; mistakes made in the
past should be avoided; it is unclear whether the kernel should switch from
cpio to tar.

The kernel _can't_ switch from cpio to tar without breaking backwards
compatability, it could only add tar as a second format it supported (remember
cpio images can be sideloaded so a new rootfs can be used with an existing
initramfs, plus existing build systems generate them and would still need to if
they wanted to keep supporting older kernels), and then once you've got two
formats somebody will propose zip support, and let's just not go there please.

The changes to the userspace tools are trivial (I say that as the maintainer of
toybox, which has a cpio). The argument was about things like 64 bit timestamps
(y2038 problem), nanosecond support, sparse files, etc. And I think the argument
had largely died down?

Keep in mind the squashfs guy spent 5 years trying to get his filesystem merged
(https://lwn.net/Articles/563578/), I spent several years trying to get my perl
removal patch merged (and only work up the enthusiasm to resubmit
http://lists.busybox.net/pipermail/buildroot/2015-March/123385.html
https://patchwork.kernel.org/patch/9193529/ https://lkml.org/lkml/2017/9/13/651
about once a year because dealing with linux-kernel is just no fun for hobbyists
anymore).

quoted

The aim of this patch is to provide the same functionality without
introducing a new format. The value of xattrs is placed in regular files
having the same file name as the files xattrs are added to, plus a
separator and the xattr name (<filename>.xattr-<xattr name>).

I think you're solving the wrong problem, but that's just my opinion.

Instead of iterating over rootfs, would it be better to detect files
with extended attributes (from the file name) when the cpio image is
parsed by the kernel, and call sys_lsetxattr() in do_copy()? This part
can be turned on by introducing a new type in the existing format (if
possible).

The impact of this alternative is very low, and LSMs/IMA would be able,
with minimum effort, to enforce policies on files in the initial ram
disk.

Roberto

Rob

-- 
HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Bo PENG, Jian LI, Yanli SHI

On 11/26/2018 4:51 AM, Mimi Zohar wrote:

On Fri, 2018-11-23 at 18:07 -0800, Casey Schaufler wrote:

quoted

On 11/23/2018 11:30 AM, Mimi Zohar wrote:

quoted

On Fri, 2018-11-23 at 11:03 -0800, Casey Schaufler wrote:

quoted

On 11/22/2018 7:49 AM, Roberto Sassu wrote:

quoted

Although rootfs (tmpfs) supports xattrs, they are not set due to the
limitation of the cpio format. A new format called 'newcx' was proposed to
overcome this limitation.

However, it looks like that adding a new format is not simple: 15 kernel
patches; user space tools must support the new format; mistakes made in the
past should be avoided; it is unclear whether the kernel should switch from
cpio to tar.

The aim of this patch is to provide the same functionality without
introducing a new format. The value of xattrs is placed in regular files
having the same file name as the files xattrs are added to, plus a
separator and the xattr name (<filename>.xattr-<xattr name>).

Example:

'/bin/cat.xattr-security.ima' is the name of a file containing the value of
the security.ima xattr to be added to /bin/cat.

At kernel initialization time, the kernel iterates over the rootfs
filesystem, and if it encounters files with the '.xattr-' separator, it
reads the content and adds the xattr to the file without the suffix.

No.

Really, no.

It would be incredibly easy to use this mechanism to break
into systems.

Assuming that the initramfs itself was signed, how?

I don't share your faith in signatures.

quoted

quoted

quoted

quoted

This proposal requires that LSMs and IMA allow the read and setxattr
operations. This should not be a concern since: files with xattr values
are not parsed by the kernel; user space processes are not yet executed.

It would be possible to include all xattrs in the same file, but this
increases the risk of the kernel being compromised by parsing the content.

The kernel mustn't do this.

Mustn't do what?  Store the xattr as separate detached files, 
include all the xattrs in a single or per security/LSM xattr attribute
file(s), or either?

Any and all of the above. The proposed behavior is a kludge
around making the installation tools work correctly. Sure, it
may be easier to change the kernel than to change the utilities.
That's doesn't make it right.

Modifying userspace tools, as Rob Landley pointed out in terms of
toybox, isn't difficult.  The difficulty has been in reviewing and
upstreaming the kernel CPIO changes.

No sympathy from me there. And why wouldn't this scheme require
just as much review?

This patch was posted in order to address the lack of xattr support in
the initramfs.  Before totally dismissing this or a similar solution,
is there a safe method for including the xattrs?

The extension to CPIO sounds right to me.

Would defining an LSM hook here help?  Each LSM would define its own
method for storing and applying, or restoring, xattr labels.

I'm more concerned about how this could be used with file capabilities
than I am with access control attributes. I don't see how adding a hook
for this special case would help.

Mimi

On 11/26/2018 4:56 AM, Roberto Sassu wrote:

On 11/23/2018 9:21 PM, Rob Landley wrote:

quoted

On 11/22/18 9:49 AM, Roberto Sassu wrote:

quoted

Although rootfs (tmpfs) supports xattrs, they are not set due to the
limitation of the cpio format. A new format called 'newcx' was proposed to
overcome this limitation.

I got email about that format the day before you posted this, by the way.

quoted

However, it looks like that adding a new format is not simple: 15 kernel
patches; user space tools must support the new format; mistakes made in the
past should be avoided; it is unclear whether the kernel should switch from
cpio to tar.

The kernel _can't_ switch from cpio to tar without breaking backwards
compatability, it could only add tar as a second format it supported (remember
cpio images can be sideloaded so a new rootfs can be used with an existing
initramfs, plus existing build systems generate them and would still need to if
they wanted to keep supporting older kernels), and then once you've got two
formats somebody will propose zip support, and let's just not go there please.

The changes to the userspace tools are trivial (I say that as the maintainer of
toybox, which has a cpio). The argument was about things like 64 bit timestamps
(y2038 problem), nanosecond support, sparse files, etc. And I think the argument
had largely died down?

Keep in mind the squashfs guy spent 5 years trying to get his filesystem merged
(https://lwn.net/Articles/563578/), I spent several years trying to get my perl
removal patch merged (and only work up the enthusiasm to resubmit
http://lists.busybox.net/pipermail/buildroot/2015-March/123385.html
https://patchwork.kernel.org/patch/9193529/ https://lkml.org/lkml/2017/9/13/651
about once a year because dealing with linux-kernel is just no fun for hobbyists
anymore).

quoted

The aim of this patch is to provide the same functionality without
introducing a new format. The value of xattrs is placed in regular files
having the same file name as the files xattrs are added to, plus a
separator and the xattr name (<filename>.xattr-<xattr name>).

I think you're solving the wrong problem, but that's just my opinion.

Instead of iterating over rootfs, would it be better to detect files
with extended attributes (from the file name) when the cpio image is
parsed by the kernel, and call sys_lsetxattr() in do_copy()? This part
can be turned on by introducing a new type in the existing format (if
possible).

A very similar approach was used in at least one MLS Unix
system back in the day. It used tar, but would have worked
just as well with CPIO. Any file with a specific name
was assumed to contain the security attributes for the
preceding file, and tar invoked a helper program to set
them. No change to the tar format was required, and if
you read an archive with a generic tar you just got multiple
entries for the special name. No format or special types
required.

The impact of this alternative is very low, and LSMs/IMA would be able,
with minimum effort, to enforce policies on files in the initial ram
disk.

True. And it worked. But it was still a kludge.

On 11/26/18 6:56 AM, Roberto Sassu wrote:

On 11/23/2018 9:21 PM, Rob Landley wrote:

quoted

quoted

The aim of this patch is to provide the same functionality without
introducing a new format. The value of xattrs is placed in regular files
having the same file name as the files xattrs are added to, plus a
separator and the xattr name (<filename>.xattr-<xattr name>).

I think you're solving the wrong problem, but that's just my opinion.

Instead of iterating over rootfs, would it be better to detect files
with extended attributes (from the file name) when the cpio image is
parsed by the kernel,

Huh, I thought at first glance that's what the new approach _was_ doing.

In band signaling in the archive is ugly, still requires new tools to create it,
doesn't address the y2038 issue... (Although we could cheat, treat the time
signature as unsigned, and buy another few decades.)

But doing that in the filesystem _after_ you extract the archive is beyond ugly.

and call sys_lsetxattr() in do_copy()? This part
can be turned on by introducing a new type in the existing format (if
possible).

The impact of this alternative is very low, and LSMs/IMA would be able,
with minimum effort, to enforce policies on files in the initial ram
disk.

The cpio extension isn't a big deal, I was pondering doing it myself in toybox
(and submitting a kernel patch to consume the output) before Mimi approached me.
(I did the initmpfs stuff already, I've stomped around in this area before.) I
just didn't because mimi sent their patch first, so I waited for that to work
its way though.

Unfortunately, it's simple enough that there was a bit of bikeshedding. (You can
store time in milliseconds as a 64 bit number without worrying about the range,
but if you do it as nanoseconds you need two fields, but people spoke up and
said "but if you don't store the nanoseconds the rounding causes spurious time
differences when between filesystems and it confuses our build system about what
has and hasn't changed"...)

The new in-band signaling proposal is, at best, a hack. (Filename lengths are
capped at 255 in the VFS, can you strip the xattrs on a long filename by having
the extension fail to create in the filesystem? Or do you have an arbitrary max
length on filenames because you need to reserve room for the extension?)

Rob