LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH v4 0/3] initramfs: add support for xattrs in the initial ram disk
@ 2019-05-23 12:18 Roberto Sassu
  2019-05-23 12:18 ` [PATCH v4 1/3] initramfs: add file metadata Roberto Sassu
                   ` (4 more replies)
  0 siblings, 5 replies; 15+ messages in thread
From: Roberto Sassu @ 2019-05-23 12:18 UTC (permalink / raw)
  To: viro
  Cc: linux-security-module, linux-integrity, initramfs, linux-api,
	linux-fsdevel, linux-kernel, bug-cpio, zohar, silviu.vlasceanu,
	dmitry.kasatkin, takondra, kamensky, hpa, arnd, rob,
	james.w.mcmechan, niveditas98, Roberto Sassu

This patch set aims at solving the following use case: appraise files from
the initial ram disk. To do that, IMA checks the signature/hash from the
security.ima xattr. Unfortunately, this use case cannot be implemented
currently, as the CPIO format does not support xattrs.

This proposal consists in including file metadata as additional files named
METADATA!!!, for each file added to the ram disk. The CPIO parser in the
kernel recognizes these special files from the file name, and calls the
appropriate parser to add metadata to the previously extracted file. It has
been proposed to use bit 17:16 of the file mode as a way to recognize files
with metadata, but both the kernel and the cpio tool declare the file mode
as unsigned short.

The difference from v2, v3 (https://lkml.org/lkml/2019/5/9/230,
https://lkml.org/lkml/2019/5/17/466) is that file metadata are stored in
separate files instead of a single file. Given that files with metadata
must immediately follow the files metadata will be added to, image
generators have to be modified in this version.

The difference from v1 (https://lkml.org/lkml/2018/11/22/1182) is that
all files have the same name. The file metadata are added to is always the
previous one, and the image generator in user space will make sure that
files are in the correct sequence.

The difference with another proposal
(https://lore.kernel.org/patchwork/cover/888071/) is that xattrs can be
included in an image without changing the image format. Files with metadata
will appear as regular files. It will be task of the parser in the kernel
to process them.

This patch set extends the format of data defined in patch 9/15 of the last
proposal. It adds header version and type, so that new formats can be
defined and arbitrary metadata types can be processed.

The changes introduced by this patch set don't cause any compatibility
issue: kernels without the metadata parser simply extract the special files
and don't process metadata; kernels with the metadata parser don't process
metadata if the special files are not included in the image.

From the kernel space perspective, backporting this functionality to older
kernels should be very easy. It is sufficient to add two calls to the new
function do_process_metadata() in do_copy(), and to check the file name in
do_name(). From the user space perspective, unlike the previous version of
the patch set, it is required to modify the image generators in order to
include metadata as separate files.

Changelog

v3:
- include file metadata as separate files named METADATA!!!
- add the possibility to include in the ram disk arbitrary metadata types

v2:
- replace ksys_lsetxattr() with kern_path() and vfs_setxattr()
  (suggested by Jann Horn)
- replace ksys_open()/ksys_read()/ksys_close() with
  filp_open()/kernel_read()/fput()
  (suggested by Jann Horn)
- use path variable instead of name_buf in do_readxattrs()
- set last byte of str to 0 in do_readxattrs()
- call do_readxattrs() in do_name() before replacing an existing
  .xattr-list
- pass pathname to do_setxattrs()

v1:
- move xattr unmarshaling to CPIO parser


Mimi Zohar (1):
  initramfs: add file metadata

Roberto Sassu (2):
  initramfs: read metadata from special file METADATA!!!
  gen_init_cpio: add support for file metadata

 include/linux/initramfs.h |  21 ++++++
 init/initramfs.c          | 137 +++++++++++++++++++++++++++++++++++++-
 usr/Kconfig               |   8 +++
 usr/Makefile              |   4 +-
 usr/gen_init_cpio.c       | 137 ++++++++++++++++++++++++++++++++++++--
 usr/gen_initramfs_list.sh |  10 ++-
 6 files changed, 305 insertions(+), 12 deletions(-)
 create mode 100644 include/linux/initramfs.h

-- 
2.17.1


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v4 1/3] initramfs: add file metadata
  2019-05-23 12:18 [PATCH v4 0/3] initramfs: add support for xattrs in the initial ram disk Roberto Sassu
@ 2019-05-23 12:18 ` Roberto Sassu
  2019-05-23 12:18 ` [PATCH v4 2/3] initramfs: read metadata from special file METADATA!!! Roberto Sassu
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 15+ messages in thread
From: Roberto Sassu @ 2019-05-23 12:18 UTC (permalink / raw)
  To: viro
  Cc: linux-security-module, linux-integrity, initramfs, linux-api,
	linux-fsdevel, linux-kernel, bug-cpio, zohar, silviu.vlasceanu,
	dmitry.kasatkin, takondra, kamensky, hpa, arnd, rob,
	james.w.mcmechan, niveditas98, Roberto Sassu

From: Mimi Zohar <zohar@linux.vnet.ibm.com>

This patch adds metadata to a file from a supplied buffer. The buffer might
contains multiple metadata records. The format of each record is:

<metadata len (ASCII, 8 chars)><version><type><metadata>

For now, only the TYPE_XATTR metadata type is supported. The specific
format of this metadata type is:

<xattr #N name>\0<xattr #N value>

[kamensky: fixed restoring of xattrs for symbolic links by using
           sys_lsetxattr() instead of sys_setxattr()]

[sassu: removed state management, kept only do_setxattrs(), added support
        for generic file metadata, replaced sys_lsetxattr() with
        vfs_setxattr(), added check for entry_size, added check for
        hdr->c_size, replaced strlen() with strnlen(); moved do_setxattrs()
        before do_name()]

Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
Signed-off-by: Victor Kamensky <kamensky@cisco.com>
Signed-off-by: Taras Kondratiuk <takondra@cisco.com>
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 include/linux/initramfs.h | 21 ++++++++++
 init/initramfs.c          | 88 ++++++++++++++++++++++++++++++++++++++-
 2 files changed, 107 insertions(+), 2 deletions(-)
 create mode 100644 include/linux/initramfs.h

diff --git a/include/linux/initramfs.h b/include/linux/initramfs.h
new file mode 100644
index 000000000000..2f8cee441236
--- /dev/null
+++ b/include/linux/initramfs.h
@@ -0,0 +1,21 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * include/linux/initramfs.h
+ *
+ * Include file for file metadata in the initial ram disk.
+ */
+#ifndef _LINUX_INITRAMFS_H
+#define _LINUX_INITRAMFS_H
+
+#define METADATA_FILENAME "METADATA!!!"
+
+enum metadata_types { TYPE_NONE, TYPE_XATTR, TYPE__LAST };
+
+struct metadata_hdr {
+	char c_size[8];     /* total size including c_size field */
+	char c_version;     /* header version */
+	char c_type;        /* metadata type */
+	char c_metadata[];  /* metadata */
+} __packed;
+
+#endif /*LINUX_INITRAMFS_H*/
diff --git a/init/initramfs.c b/init/initramfs.c
index 178130fd61c2..5de396a6aac0 100644
--- a/init/initramfs.c
+++ b/init/initramfs.c
@@ -10,6 +10,9 @@
 #include <linux/syscalls.h>
 #include <linux/utime.h>
 #include <linux/file.h>
+#include <linux/namei.h>
+#include <linux/xattr.h>
+#include <linux/initramfs.h>
 
 static ssize_t __init xwrite(int fd, const char *p, size_t count)
 {
@@ -146,7 +149,7 @@ static __initdata time64_t mtime;
 
 static __initdata unsigned long ino, major, minor, nlink;
 static __initdata umode_t mode;
-static __initdata unsigned long body_len, name_len;
+static __initdata unsigned long body_len, name_len, metadata_len;
 static __initdata uid_t uid;
 static __initdata gid_t gid;
 static __initdata unsigned rdev;
@@ -218,7 +221,7 @@ static void __init read_into(char *buf, unsigned size, enum state next)
 	}
 }
 
-static __initdata char *header_buf, *symlink_buf, *name_buf;
+static __initdata char *header_buf, *symlink_buf, *name_buf, *metadata_buf;
 
 static int __init do_start(void)
 {
@@ -315,6 +318,87 @@ static int __init maybe_link(void)
 	return 0;
 }
 
+static int __init do_setxattrs(char *pathname, char *buf, size_t size)
+{
+	struct path path;
+	char *xattr_name, *xattr_value;
+	size_t xattr_name_size, xattr_value_size;
+	int ret;
+
+	xattr_name = buf;
+	xattr_name_size = strnlen(xattr_name, size);
+	if (xattr_name_size == size) {
+		error("malformed xattrs");
+		return -EINVAL;
+	}
+
+	xattr_value = xattr_name + xattr_name_size + 1;
+	xattr_value_size = buf + size - xattr_value;
+
+	ret = kern_path(pathname, 0, &path);
+	if (!ret) {
+		ret = vfs_setxattr(path.dentry, xattr_name, xattr_value,
+				   xattr_value_size, 0);
+
+		path_put(&path);
+	}
+
+	pr_debug("%s: %s size: %lu val: %s (ret: %d)\n", pathname,
+		 xattr_name, xattr_value_size, xattr_value, ret);
+
+	return ret;
+}
+
+static int __init __maybe_unused do_parse_metadata(char *pathname)
+{
+	char *buf = metadata_buf;
+	char *bufend = metadata_buf + metadata_len;
+	struct metadata_hdr *hdr;
+	char str[sizeof(hdr->c_size) + 1];
+	size_t entry_size;
+
+	if (!metadata_len)
+		return 0;
+
+	str[sizeof(hdr->c_size)] = 0;
+
+	while (buf < bufend) {
+		int ret;
+
+		if (buf + sizeof(*hdr) > bufend) {
+			error("malformed metadata");
+			break;
+		}
+
+		hdr = (struct metadata_hdr *)buf;
+		if (hdr->c_version != 1) {
+			pr_debug("Unsupported header version\n");
+			break;
+		}
+
+		memcpy(str, hdr->c_size, sizeof(hdr->c_size));
+		ret = kstrtoul(str, 16, &entry_size);
+		if (ret || buf + entry_size > bufend || !entry_size) {
+			error("malformed xattrs");
+			break;
+		}
+
+		switch (hdr->c_type) {
+		case TYPE_XATTR:
+			do_setxattrs(pathname, buf + sizeof(*hdr),
+				     entry_size - sizeof(*hdr));
+			break;
+		default:
+			pr_debug("Unsupported metadata type\n");
+			break;
+		}
+
+		buf += entry_size;
+	}
+
+	return 0;
+}
+
 static __initdata int wfd;
 
 static int __init do_name(void)
-- 
2.17.1


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v4 2/3] initramfs: read metadata from special file METADATA!!!
  2019-05-23 12:18 [PATCH v4 0/3] initramfs: add support for xattrs in the initial ram disk Roberto Sassu
  2019-05-23 12:18 ` [PATCH v4 1/3] initramfs: add file metadata Roberto Sassu
@ 2019-05-23 12:18 ` Roberto Sassu
  2019-07-01 12:54   ` Mimi Zohar
  2019-05-23 12:18 ` [PATCH v4 3/3] gen_init_cpio: add support for file metadata Roberto Sassu
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 15+ messages in thread
From: Roberto Sassu @ 2019-05-23 12:18 UTC (permalink / raw)
  To: viro
  Cc: linux-security-module, linux-integrity, initramfs, linux-api,
	linux-fsdevel, linux-kernel, bug-cpio, zohar, silviu.vlasceanu,
	dmitry.kasatkin, takondra, kamensky, hpa, arnd, rob,
	james.w.mcmechan, niveditas98, Roberto Sassu

Instead of changing the CPIO format, metadata are parsed from regular files
with special name 'METADATA!!!'. This file immediately follows the file
metadata are added to.

This patch checks if the file being extracted has the special name and, if
yes, creates a buffer with the content of that file and calls
do_parse_metadata() to parse metadata from the buffer.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
Reported-by: kbuild test robot <lkp@intel.com>
---
 init/initramfs.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 48 insertions(+), 1 deletion(-)

diff --git a/init/initramfs.c b/init/initramfs.c
index 5de396a6aac0..862c03123de8 100644
--- a/init/initramfs.c
+++ b/init/initramfs.c
@@ -222,6 +222,7 @@ static void __init read_into(char *buf, unsigned size, enum state next)
 }
 
 static __initdata char *header_buf, *symlink_buf, *name_buf, *metadata_buf;
+static __initdata char *metadata_buf_ptr, *previous_name_buf;
 
 static int __init do_start(void)
 {
@@ -400,6 +401,7 @@ static int __init __maybe_unused do_parse_metadata(char *pathname)
 }
 
 static __initdata int wfd;
+static __initdata int metadata;
 
 static int __init do_name(void)
 {
@@ -408,6 +410,10 @@ static int __init do_name(void)
 	if (strcmp(collected, "TRAILER!!!") == 0) {
 		free_hash();
 		return 0;
+	} else if (strcmp(collected, METADATA_FILENAME) == 0) {
+		metadata = 1;
+	} else {
+		memcpy(previous_name_buf, collected, strlen(collected) + 1);
 	}
 	clean_path(collected, mode);
 	if (S_ISREG(mode)) {
@@ -444,11 +450,47 @@ static int __init do_name(void)
 	return 0;
 }
 
+static int __init do_process_metadata(char *buf, int len, bool last)
+{
+	int ret = 0;
+
+	if (!metadata_buf) {
+		metadata_buf_ptr = metadata_buf = kmalloc(body_len, GFP_KERNEL);
+		if (!metadata_buf_ptr) {
+			ret = -ENOMEM;
+			goto out;
+		}
+
+		metadata_len = body_len;
+	}
+
+	if (metadata_buf_ptr + len > metadata_buf + metadata_len) {
+		ret = -EINVAL;
+		goto out;
+	}
+
+	memcpy(metadata_buf_ptr, buf, len);
+	metadata_buf_ptr += len;
+
+	if (last)
+		do_parse_metadata(previous_name_buf);
+out:
+	if (ret < 0 || last) {
+		kfree(metadata_buf);
+		metadata_buf = NULL;
+		metadata = 0;
+	}
+
+	return ret;
+}
+
 static int __init do_copy(void)
 {
 	if (byte_count >= body_len) {
 		if (xwrite(wfd, victim, body_len) != body_len)
 			error("write error");
+		if (metadata)
+			do_process_metadata(victim, body_len, true);
 		ksys_close(wfd);
 		do_utime(vcollected, mtime);
 		kfree(vcollected);
@@ -458,6 +500,8 @@ static int __init do_copy(void)
 	} else {
 		if (xwrite(wfd, victim, byte_count) != byte_count)
 			error("write error");
+		if (metadata)
+			do_process_metadata(victim, byte_count, false);
 		body_len -= byte_count;
 		eat(byte_count);
 		return 1;
@@ -467,6 +511,7 @@ static int __init do_copy(void)
 static int __init do_symlink(void)
 {
 	collected[N_ALIGN(name_len) + body_len] = '\0';
+	memcpy(previous_name_buf, collected, strlen(collected) + 1);
 	clean_path(collected, 0);
 	ksys_symlink(collected + N_ALIGN(name_len), collected);
 	ksys_lchown(collected, uid, gid);
@@ -534,8 +579,9 @@ static char * __init unpack_to_rootfs(char *buf, unsigned long len)
 	header_buf = kmalloc(110, GFP_KERNEL);
 	symlink_buf = kmalloc(PATH_MAX + N_ALIGN(PATH_MAX) + 1, GFP_KERNEL);
 	name_buf = kmalloc(N_ALIGN(PATH_MAX), GFP_KERNEL);
+	previous_name_buf = kmalloc(N_ALIGN(PATH_MAX), GFP_KERNEL);
 
-	if (!header_buf || !symlink_buf || !name_buf)
+	if (!header_buf || !symlink_buf || !name_buf || !previous_name_buf)
 		panic("can't allocate buffers");
 
 	state = Start;
@@ -580,6 +626,7 @@ static char * __init unpack_to_rootfs(char *buf, unsigned long len)
 		len -= my_inptr;
 	}
 	dir_utime();
+	kfree(previous_name_buf);
 	kfree(name_buf);
 	kfree(symlink_buf);
 	kfree(header_buf);
-- 
2.17.1


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v4 3/3] gen_init_cpio: add support for file metadata
  2019-05-23 12:18 [PATCH v4 0/3] initramfs: add support for xattrs in the initial ram disk Roberto Sassu
  2019-05-23 12:18 ` [PATCH v4 1/3] initramfs: add file metadata Roberto Sassu
  2019-05-23 12:18 ` [PATCH v4 2/3] initramfs: read metadata from special file METADATA!!! Roberto Sassu
@ 2019-05-23 12:18 ` Roberto Sassu
  2019-06-30 15:27   ` Mimi Zohar
  2019-06-03  9:31 ` [PATCH v4 0/3] initramfs: add support for xattrs in the initial ram disk Roberto Sassu
  2019-07-01 13:22 ` Mimi Zohar
  4 siblings, 1 reply; 15+ messages in thread
From: Roberto Sassu @ 2019-05-23 12:18 UTC (permalink / raw)
  To: viro
  Cc: linux-security-module, linux-integrity, initramfs, linux-api,
	linux-fsdevel, linux-kernel, bug-cpio, zohar, silviu.vlasceanu,
	dmitry.kasatkin, takondra, kamensky, hpa, arnd, rob,
	james.w.mcmechan, niveditas98, Roberto Sassu

This patch adds support for file metadata (only TYPE_XATTR metadata type).
gen_init_cpio has been modified to read xattrs from files that will be
added to the image and to include file metadata as separate files with the
special name 'METADATA!!!'.

This behavior can be selected by setting the desired file metadata type as
value for CONFIG_INITRAMFS_FILE_METADATA.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 usr/Kconfig               |   8 +++
 usr/Makefile              |   4 +-
 usr/gen_init_cpio.c       | 137 ++++++++++++++++++++++++++++++++++++--
 usr/gen_initramfs_list.sh |  10 ++-
 4 files changed, 150 insertions(+), 9 deletions(-)

diff --git a/usr/Kconfig b/usr/Kconfig
index 43658b8a975e..8d9f54a16440 100644
--- a/usr/Kconfig
+++ b/usr/Kconfig
@@ -233,3 +233,11 @@ config INITRAMFS_COMPRESSION
 	default ".lzma" if RD_LZMA
 	default ".bz2"  if RD_BZIP2
 	default ""
+
+config INITRAMFS_FILE_METADATA
+	string "File metadata type"
+	default ""
+	help
+	  Specify xattr to include xattrs in the image.
+
+	  If you are not sure, leave it blank.
diff --git a/usr/Makefile b/usr/Makefile
index 4a70ae43c9cb..7d5eb3c7b713 100644
--- a/usr/Makefile
+++ b/usr/Makefile
@@ -29,7 +29,9 @@ ramfs-input := $(if $(filter-out "",$(CONFIG_INITRAMFS_SOURCE)), \
 			$(shell echo $(CONFIG_INITRAMFS_SOURCE)),-d)
 ramfs-args  := \
         $(if $(CONFIG_INITRAMFS_ROOT_UID), -u $(CONFIG_INITRAMFS_ROOT_UID)) \
-        $(if $(CONFIG_INITRAMFS_ROOT_GID), -g $(CONFIG_INITRAMFS_ROOT_GID))
+        $(if $(CONFIG_INITRAMFS_ROOT_GID), -g $(CONFIG_INITRAMFS_ROOT_GID)) \
+        $(if $(filter-out "",$(CONFIG_INITRAMFS_FILE_METADATA)), \
+         -e $(CONFIG_INITRAMFS_FILE_METADATA))
 
 # $(datafile_d_y) is used to identify all files included
 # in initramfs and to detect if any files are added/removed.
diff --git a/usr/gen_init_cpio.c b/usr/gen_init_cpio.c
index 03b21189d58b..e93cb1093e77 100644
--- a/usr/gen_init_cpio.c
+++ b/usr/gen_init_cpio.c
@@ -3,6 +3,7 @@
 #include <stdlib.h>
 #include <sys/types.h>
 #include <sys/stat.h>
+#include <sys/xattr.h>
 #include <string.h>
 #include <unistd.h>
 #include <time.h>
@@ -10,6 +11,7 @@
 #include <errno.h>
 #include <ctype.h>
 #include <limits.h>
+#include "../include/linux/initramfs.h"
 
 /*
  * Original work by Jeff Garzik
@@ -24,6 +26,115 @@
 static unsigned int offset;
 static unsigned int ino = 721;
 static time_t default_mtime;
+static char metadata_path[] = "/tmp/cpio-metadata-XXXXXX";
+static int metadata_fd = -1;
+
+static enum metadata_types parse_metadata_type(char *arg)
+{
+	static char *metadata_type_str[TYPE__LAST] = {
+		[TYPE_NONE] = "none",
+		[TYPE_XATTR] = "xattr",
+	};
+	int i;
+
+	for (i = 0; i < TYPE__LAST; i++)
+		if (!strcmp(metadata_type_str[i], arg))
+			return i;
+
+	return TYPE_NONE;
+}
+
+static int cpio_mkfile(const char *name, const char *location,
+		       unsigned int mode, uid_t uid, gid_t gid,
+		       unsigned int nlinks);
+
+static int write_xattrs(const char *path)
+{
+	struct metadata_hdr hdr = { .c_version = 1, .c_type = TYPE_XATTR };
+	char str[sizeof(hdr.c_size) + 1];
+	char *xattr_list, *list_ptr, *xattr_value;
+	ssize_t list_len, name_len, value_len, len;
+	int ret = -EINVAL;
+
+	if (metadata_fd < 0)
+		return 0;
+
+	if (path == metadata_path)
+		return 0;
+
+	list_len = listxattr(path, NULL, 0);
+	if (list_len <= 0)
+		return 0;
+
+	list_ptr = xattr_list = malloc(list_len);
+	if (!list_ptr) {
+		fprintf(stderr, "out of memory\n");
+		return ret;
+	}
+
+	len = listxattr(path, xattr_list, list_len);
+	if (len != list_len)
+		goto out;
+
+	if (ftruncate(metadata_fd, 0))
+		goto out;
+
+	lseek(metadata_fd, 0, SEEK_SET);
+
+	while (list_ptr < xattr_list + list_len) {
+		name_len = strlen(list_ptr);
+
+		value_len = getxattr(path, list_ptr, NULL, 0);
+		if (value_len < 0) {
+			fprintf(stderr, "cannot get xattrs\n");
+			break;
+		}
+
+		if (value_len) {
+			xattr_value = malloc(value_len);
+			if (!xattr_value) {
+				fprintf(stderr, "out of memory\n");
+				break;
+			}
+		} else {
+			xattr_value = NULL;
+		}
+
+		len = getxattr(path, list_ptr, xattr_value, value_len);
+		if (len != value_len)
+			break;
+
+		snprintf(str, sizeof(str), "%.8lx",
+			 sizeof(hdr) + name_len + 1 + value_len);
+
+		memcpy(hdr.c_size, str, sizeof(hdr.c_size));
+
+		if (write(metadata_fd, &hdr, sizeof(hdr)) != sizeof(hdr))
+			break;
+
+		if (write(metadata_fd, list_ptr, name_len + 1) != name_len + 1)
+			break;
+
+		if (write(metadata_fd, xattr_value, value_len) != value_len)
+			break;
+
+		if (fsync(metadata_fd))
+			break;
+
+		list_ptr += name_len + 1;
+		free(xattr_value);
+		xattr_value = NULL;
+	}
+
+	free(xattr_value);
+out:
+	free(xattr_list);
+
+	if (list_ptr != xattr_list + list_len)
+		return ret;
+
+	return cpio_mkfile(METADATA_FILENAME, metadata_path, S_IFREG, 0, 0, 1);
+}
 
 struct file_handler {
 	const char *type;
@@ -128,7 +239,7 @@ static int cpio_mkslink(const char *name, const char *target,
 	push_pad();
 	push_string(target);
 	push_pad();
-	return 0;
+	return write_xattrs(name);
 }
 
 static int cpio_mkslink_line(const char *line)
@@ -174,7 +285,7 @@ static int cpio_mkgeneric(const char *name, unsigned int mode,
 		0);			/* chksum */
 	push_hdr(s);
 	push_rest(name);
-	return 0;
+	return write_xattrs(name);
 }
 
 enum generic_types {
@@ -268,7 +379,7 @@ static int cpio_mknod(const char *name, unsigned int mode,
 		0);			/* chksum */
 	push_hdr(s);
 	push_rest(name);
-	return 0;
+	return write_xattrs(name);
 }
 
 static int cpio_mknod_line(const char *line)
@@ -372,8 +483,7 @@ static int cpio_mkfile(const char *name, const char *location,
 		name += namesize;
 	}
 	ino++;
-	rc = 0;
-	
+	rc = write_xattrs(location);
 error:
 	if (filebuf) free(filebuf);
 	if (file >= 0) close(file);
@@ -526,10 +636,11 @@ int main (int argc, char *argv[])
 	int ec = 0;
 	int line_nr = 0;
 	const char *filename;
+	enum metadata_types metadata_type = TYPE_NONE;
 
 	default_mtime = time(NULL);
 	while (1) {
-		int opt = getopt(argc, argv, "t:h");
+		int opt = getopt(argc, argv, "t:e:h");
 		char *invalid;
 
 		if (opt == -1)
@@ -544,6 +655,9 @@ int main (int argc, char *argv[])
 				exit(1);
 			}
 			break;
+		case 'e':
+			metadata_type = parse_metadata_type(optarg);
+			break;
 		case 'h':
 		case '?':
 			usage(argv[0]);
@@ -565,6 +679,14 @@ int main (int argc, char *argv[])
 		exit(1);
 	}
 
+	if (metadata_type != TYPE_NONE) {
+		metadata_fd = mkstemp(metadata_path);
+		if (metadata_fd < 0) {
+			fprintf(stderr, "cannot create temporary file\n");
+			exit(1);
+		}
+	}
+
 	while (fgets(line, LINE_SIZE, cpio_list)) {
 		int type_idx;
 		size_t slen = strlen(line);
@@ -620,5 +742,8 @@ int main (int argc, char *argv[])
 	if (ec == 0)
 		cpio_trailer();
 
+	if (metadata_type != TYPE_NONE)
+		close(metadata_fd);
+
 	exit(ec);
 }
diff --git a/usr/gen_initramfs_list.sh b/usr/gen_initramfs_list.sh
index 0aad760fcd8c..0907a4043da9 100755
--- a/usr/gen_initramfs_list.sh
+++ b/usr/gen_initramfs_list.sh
@@ -15,7 +15,7 @@ set -e
 usage() {
 cat << EOF
 Usage:
-$0 [-o <file>] [-u <uid>] [-g <gid>] {-d | <cpio_source>} ...
+$0 [-o <file>] [-u <uid>] [-g <gid>] {-d | <cpio_source>} [-e <type>] ...
 	-o <file>      Create compressed initramfs file named <file> using
 		       gen_init_cpio and compressor depending on the extension
 	-u <uid>       User ID to map to user ID 0 (root).
@@ -28,6 +28,7 @@ $0 [-o <file>] [-u <uid>] [-g <gid>] {-d | <cpio_source>} ...
 		       If <cpio_source> is a .cpio file it will be used
 		       as direct input to initramfs.
 	-d             Output the default cpio list.
+	-e <type>      File metadata type to include in the cpio archive.
 
 All options except -o and -l may be repeated and are interpreted
 sequentially and immediately.  -u and -g states are preserved across
@@ -283,6 +284,10 @@ while [ $# -gt 0 ]; do
 			default_list="$arg"
 			${dep_list}default_initramfs
 			;;
+		"-e")   # file metadata type
+			metadata_arg="-e $1"
+			shift
+			;;
 		"-h")
 			usage
 			exit 0
@@ -312,7 +317,8 @@ if [ ! -z ${output_file} ]; then
 			fi
 		fi
 		cpio_tfile="$(mktemp ${TMPDIR:-/tmp}/cpiofile.XXXXXX)"
-		usr/gen_init_cpio $timestamp ${cpio_list} > ${cpio_tfile}
+		usr/gen_init_cpio $metadata_arg $timestamp \
+			${cpio_list} > ${cpio_tfile}
 	else
 		cpio_tfile=${cpio_file}
 	fi
-- 
2.17.1


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v4 0/3] initramfs: add support for xattrs in the initial ram disk
  2019-05-23 12:18 [PATCH v4 0/3] initramfs: add support for xattrs in the initial ram disk Roberto Sassu
                   ` (2 preceding siblings ...)
  2019-05-23 12:18 ` [PATCH v4 3/3] gen_init_cpio: add support for file metadata Roberto Sassu
@ 2019-06-03  9:31 ` Roberto Sassu
  2019-06-03 18:32   ` Rob Landley
  2019-07-01 13:22 ` Mimi Zohar
  4 siblings, 1 reply; 15+ messages in thread
From: Roberto Sassu @ 2019-06-03  9:31 UTC (permalink / raw)
  To: viro
  Cc: linux-security-module, linux-integrity, initramfs, linux-api,
	linux-fsdevel, linux-kernel, bug-cpio, zohar, silviu.vlasceanu,
	dmitry.kasatkin, takondra, kamensky, hpa, arnd, rob,
	james.w.mcmechan, niveditas98

Any opinion on this patch set?

Thanks

Roberto


On 5/23/2019 2:18 PM, Roberto Sassu wrote:
> This patch set aims at solving the following use case: appraise files from
> the initial ram disk. To do that, IMA checks the signature/hash from the
> security.ima xattr. Unfortunately, this use case cannot be implemented
> currently, as the CPIO format does not support xattrs.
> 
> This proposal consists in including file metadata as additional files named
> METADATA!!!, for each file added to the ram disk. The CPIO parser in the
> kernel recognizes these special files from the file name, and calls the
> appropriate parser to add metadata to the previously extracted file. It has
> been proposed to use bit 17:16 of the file mode as a way to recognize files
> with metadata, but both the kernel and the cpio tool declare the file mode
> as unsigned short.
> 
> The difference from v2, v3 (https://lkml.org/lkml/2019/5/9/230,
> https://lkml.org/lkml/2019/5/17/466) is that file metadata are stored in
> separate files instead of a single file. Given that files with metadata
> must immediately follow the files metadata will be added to, image
> generators have to be modified in this version.
> 
> The difference from v1 (https://lkml.org/lkml/2018/11/22/1182) is that
> all files have the same name. The file metadata are added to is always the
> previous one, and the image generator in user space will make sure that
> files are in the correct sequence.
> 
> The difference with another proposal
> (https://lore.kernel.org/patchwork/cover/888071/) is that xattrs can be
> included in an image without changing the image format. Files with metadata
> will appear as regular files. It will be task of the parser in the kernel
> to process them.
> 
> This patch set extends the format of data defined in patch 9/15 of the last
> proposal. It adds header version and type, so that new formats can be
> defined and arbitrary metadata types can be processed.
> 
> The changes introduced by this patch set don't cause any compatibility
> issue: kernels without the metadata parser simply extract the special files
> and don't process metadata; kernels with the metadata parser don't process
> metadata if the special files are not included in the image.
> 
>  From the kernel space perspective, backporting this functionality to older
> kernels should be very easy. It is sufficient to add two calls to the new
> function do_process_metadata() in do_copy(), and to check the file name in
> do_name(). From the user space perspective, unlike the previous version of
> the patch set, it is required to modify the image generators in order to
> include metadata as separate files.
> 
> Changelog
> 
> v3:
> - include file metadata as separate files named METADATA!!!
> - add the possibility to include in the ram disk arbitrary metadata types
> 
> v2:
> - replace ksys_lsetxattr() with kern_path() and vfs_setxattr()
>    (suggested by Jann Horn)
> - replace ksys_open()/ksys_read()/ksys_close() with
>    filp_open()/kernel_read()/fput()
>    (suggested by Jann Horn)
> - use path variable instead of name_buf in do_readxattrs()
> - set last byte of str to 0 in do_readxattrs()
> - call do_readxattrs() in do_name() before replacing an existing
>    .xattr-list
> - pass pathname to do_setxattrs()
> 
> v1:
> - move xattr unmarshaling to CPIO parser
> 
> 
> Mimi Zohar (1):
>    initramfs: add file metadata
> 
> Roberto Sassu (2):
>    initramfs: read metadata from special file METADATA!!!
>    gen_init_cpio: add support for file metadata
> 
>   include/linux/initramfs.h |  21 ++++++
>   init/initramfs.c          | 137 +++++++++++++++++++++++++++++++++++++-
>   usr/Kconfig               |   8 +++
>   usr/Makefile              |   4 +-
>   usr/gen_init_cpio.c       | 137 ++++++++++++++++++++++++++++++++++++--
>   usr/gen_initramfs_list.sh |  10 ++-
>   6 files changed, 305 insertions(+), 12 deletions(-)
>   create mode 100644 include/linux/initramfs.h
> 

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v4 0/3] initramfs: add support for xattrs in the initial ram disk
  2019-06-03  9:31 ` [PATCH v4 0/3] initramfs: add support for xattrs in the initial ram disk Roberto Sassu
@ 2019-06-03 18:32   ` Rob Landley
  2019-06-26  8:15     ` Roberto Sassu
  0 siblings, 1 reply; 15+ messages in thread
From: Rob Landley @ 2019-06-03 18:32 UTC (permalink / raw)
  To: Roberto Sassu, viro
  Cc: linux-security-module, linux-integrity, initramfs, linux-api,
	linux-fsdevel, linux-kernel, bug-cpio, zohar, silviu.vlasceanu,
	dmitry.kasatkin, takondra, kamensky, hpa, arnd, james.w.mcmechan,
	niveditas98

On 6/3/19 4:31 AM, Roberto Sassu wrote:
>> This patch set aims at solving the following use case: appraise files from
>> the initial ram disk. To do that, IMA checks the signature/hash from the
>> security.ima xattr. Unfortunately, this use case cannot be implemented
>> currently, as the CPIO format does not support xattrs.
>>
>> This proposal consists in including file metadata as additional files named
>> METADATA!!!, for each file added to the ram disk. The CPIO parser in the
>> kernel recognizes these special files from the file name, and calls the
>> appropriate parser to add metadata to the previously extracted file. It has
>> been proposed to use bit 17:16 of the file mode as a way to recognize files
>> with metadata, but both the kernel and the cpio tool declare the file mode
>> as unsigned short.
>
> Any opinion on this patch set?
> 
> Thanks
> 
> Roberto

Sorry, I've had the window open since you posted it but haven't gotten around to
it. I'll try to build it later today.

It does look interesting, and I have no objections to the basic approach. I
should be able to add support to toybox cpio over a weekend once I've got the
kernel doing it to test against.

Rob

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v4 0/3] initramfs: add support for xattrs in the initial ram disk
  2019-06-03 18:32   ` Rob Landley
@ 2019-06-26  8:15     ` Roberto Sassu
  2019-06-30 15:39       ` Mimi Zohar
  0 siblings, 1 reply; 15+ messages in thread
From: Roberto Sassu @ 2019-06-26  8:15 UTC (permalink / raw)
  To: Rob Landley, viro
  Cc: linux-security-module, linux-integrity, initramfs, linux-api,
	linux-fsdevel, linux-kernel, bug-cpio, zohar, silviu.vlasceanu,
	dmitry.kasatkin, takondra, kamensky, hpa, arnd, james.w.mcmechan,
	niveditas98

On 6/3/2019 8:32 PM, Rob Landley wrote:
> On 6/3/19 4:31 AM, Roberto Sassu wrote:
>>> This patch set aims at solving the following use case: appraise files from
>>> the initial ram disk. To do that, IMA checks the signature/hash from the
>>> security.ima xattr. Unfortunately, this use case cannot be implemented
>>> currently, as the CPIO format does not support xattrs.
>>>
>>> This proposal consists in including file metadata as additional files named
>>> METADATA!!!, for each file added to the ram disk. The CPIO parser in the
>>> kernel recognizes these special files from the file name, and calls the
>>> appropriate parser to add metadata to the previously extracted file. It has
>>> been proposed to use bit 17:16 of the file mode as a way to recognize files
>>> with metadata, but both the kernel and the cpio tool declare the file mode
>>> as unsigned short.
>>
>> Any opinion on this patch set?
>>
>> Thanks
>>
>> Roberto
> 
> Sorry, I've had the window open since you posted it but haven't gotten around to
> it. I'll try to build it later today.
> 
> It does look interesting, and I have no objections to the basic approach. I
> should be able to add support to toybox cpio over a weekend once I've got the
> kernel doing it to test against.

Ok.

Let me give some instructions so that people can test this patch set.

To add xattrs to the ram disk embedded in the kernel it is sufficient
to set CONFIG_INITRAMFS_FILE_METADATA="xattr" and
CONFIG_INITRAMFS_SOURCE="<file with xattr>" in the kernel configuration.

To add xattrs to the external ram disk, it is necessary to patch cpio:

https://github.com/euleros/cpio/commit/531cabc88e9ecdc3231fad6e4856869baa9a91ef 
(xattr-v1 branch)

and dracut:

https://github.com/euleros/dracut/commit/a2dee56ea80495c2c1871bc73186f7b00dc8bf3b 
(digest-lists branch)

The same modification can be done for mkinitramfs (add '-e xattr' to the
cpio command line).

To simplify the test, it would be sufficient to replace only the cpio
binary and the dracut script with the modified versions. For dracut, the
patch should be applied to the local dracut (after it has been renamed
to dracut.sh).

Then, run:

dracut -e xattr -I <file with xattr> (add -f to overwrite the ram disk)

Xattrs can be seen by stopping the boot process for example by adding
rd.break to the kernel command line.

Roberto

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v4 3/3] gen_init_cpio: add support for file metadata
  2019-05-23 12:18 ` [PATCH v4 3/3] gen_init_cpio: add support for file metadata Roberto Sassu
@ 2019-06-30 15:27   ` Mimi Zohar
  0 siblings, 0 replies; 15+ messages in thread
From: Mimi Zohar @ 2019-06-30 15:27 UTC (permalink / raw)
  To: Roberto Sassu, viro
  Cc: linux-security-module, linux-integrity, initramfs, linux-api,
	linux-fsdevel, linux-kernel, bug-cpio, zohar, silviu.vlasceanu,
	dmitry.kasatkin, takondra, kamensky, hpa, arnd, rob,
	james.w.mcmechan, niveditas98

On Thu, 2019-05-23 at 14:18 +0200, Roberto Sassu wrote:

> diff --git a/usr/Kconfig b/usr/Kconfig
> index 43658b8a975e..8d9f54a16440 100644
> --- a/usr/Kconfig
> +++ b/usr/Kconfig
> @@ -233,3 +233,11 @@ config INITRAMFS_COMPRESSION
>  	default ".lzma" if RD_LZMA
>  	default ".bz2"  if RD_BZIP2
>  	default ""
> +
> +config INITRAMFS_FILE_METADATA
> +	string "File metadata type"
> +	default ""
> +	help
> +	  Specify xattr to include xattrs in the image.
> +
> +	  If you are not sure, leave it blank.
> 	fi

Instead of having to specify the metdata type, let's make this a
choice.

Mimi


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v4 0/3] initramfs: add support for xattrs in the initial ram disk
  2019-06-26  8:15     ` Roberto Sassu
@ 2019-06-30 15:39       ` Mimi Zohar
  2019-07-01 13:42         ` Roberto Sassu
  0 siblings, 1 reply; 15+ messages in thread
From: Mimi Zohar @ 2019-06-30 15:39 UTC (permalink / raw)
  To: Roberto Sassu, Rob Landley, viro
  Cc: linux-security-module, linux-integrity, initramfs, linux-api,
	linux-fsdevel, linux-kernel, bug-cpio, zohar, silviu.vlasceanu,
	dmitry.kasatkin, takondra, kamensky, hpa, arnd, james.w.mcmechan,
	niveditas98

On Wed, 2019-06-26 at 10:15 +0200, Roberto Sassu wrote:
> On 6/3/2019 8:32 PM, Rob Landley wrote:
> > On 6/3/19 4:31 AM, Roberto Sassu wrote:
> >>> This patch set aims at solving the following use case: appraise files from
> >>> the initial ram disk. To do that, IMA checks the signature/hash from the
> >>> security.ima xattr. Unfortunately, this use case cannot be implemented
> >>> currently, as the CPIO format does not support xattrs.
> >>>
> >>> This proposal consists in including file metadata as additional files named
> >>> METADATA!!!, for each file added to the ram disk. The CPIO parser in the
> >>> kernel recognizes these special files from the file name, and calls the
> >>> appropriate parser to add metadata to the previously extracted file. It has
> >>> been proposed to use bit 17:16 of the file mode as a way to recognize files
> >>> with metadata, but both the kernel and the cpio tool declare the file mode
> >>> as unsigned short.
> >>
> >> Any opinion on this patch set?
> >>
> >> Thanks
> >>
> >> Roberto
> > 
> > Sorry, I've had the window open since you posted it but haven't gotten around to
> > it. I'll try to build it later today.
> > 
> > It does look interesting, and I have no objections to the basic approach. I
> > should be able to add support to toybox cpio over a weekend once I've got the
> > kernel doing it to test against.
> 
> Ok.
> 
> Let me give some instructions so that people can test this patch set.
> 
> To add xattrs to the ram disk embedded in the kernel it is sufficient
> to set CONFIG_INITRAMFS_FILE_METADATA="xattr" and
> CONFIG_INITRAMFS_SOURCE="<file with xattr>" in the kernel configuration.
> 
> To add xattrs to the external ram disk, it is necessary to patch cpio:
> 
> https://github.com/euleros/cpio/commit/531cabc88e9ecdc3231fad6e4856869baa9a91ef 
> (xattr-v1 branch)
> 
> and dracut:
> 
> https://github.com/euleros/dracut/commit/a2dee56ea80495c2c1871bc73186f7b00dc8bf3b 
> (digest-lists branch)
> 
> The same modification can be done for mkinitramfs (add '-e xattr' to the
> cpio command line).
> 
> To simplify the test, it would be sufficient to replace only the cpio
> binary and the dracut script with the modified versions. For dracut, the
> patch should be applied to the local dracut (after it has been renamed
> to dracut.sh).
> 
> Then, run:
> 
> dracut -e xattr -I <file with xattr> (add -f to overwrite the ram disk)
> 
> Xattrs can be seen by stopping the boot process for example by adding
> rd.break to the kernel command line.

A simple way of testing, without needing any changes other than the
kernel patches, is to save the dracut temporary directory by supplying
"--keep" on the dracut command line, calling
usr/gen_initramfs_list.sh, followed by usr/gen_init_cpio with the "-e
xattr" option.

If your filesystem already has and copied the security xattrs to the
dracut temporary directory, the script, below, will include them in
the initramfs file.  Otherwise, you'll need to write the desired
security xattrs on the files, using setfattr, in the temporary dracut
directory, before creating the initramfs.

Remember to make sure that the initramfs_list includes "getfattr",
otherwise you'll need to wait until real root is mounted as /sysroot
to see the security xattrs for the rootfs files.

The following script has not been tested on a recent version of
dracut.  Some changes might be needed, as well as some code cleanup.

#!/bin/bash

initramfs_name=/boot/initramfs-`uname -r`.img
initramfs_output_name=${initramfs_name/.img/.test.img}

if [ $# -eq 1 ]; then
        initramfs_name=$1
fi

if [ ! -f "$initramfs_name" ]; then
        echo "Usage; $0 <initramfs pathanem>"
        exit 1
fi

tmp=$(dracut -H -f "$initramfs_name" --keep --noprelink --nostrip 2>&1)
suffix=$(echo $tmp | cut -d ' ' -f 3 | cut -d '.' -f 2)

tmpdir="/var/tmp/dracut.$suffix/initramfs"

if [ ! -d "$tmpdir" ]; then
        echo "$tmpdir does not exist"
        exit 1
fi

usr/gen_initramfs_list.sh ${tmpdir} > usr/initramfs_list
usr/gen_init_cpio -e xattr usr/initramfs_list > usr/initramfs_data.cpio
gzip usr/initramfs_data.cpio

echo "Copying usr/initramfs_data.cpio to $initramfs_output_name"
cp usr/initramfs_data.cpio.gz "$initramfs_output_name"

Mimi


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v4 2/3] initramfs: read metadata from special file METADATA!!!
  2019-05-23 12:18 ` [PATCH v4 2/3] initramfs: read metadata from special file METADATA!!! Roberto Sassu
@ 2019-07-01 12:54   ` Mimi Zohar
  0 siblings, 0 replies; 15+ messages in thread
From: Mimi Zohar @ 2019-07-01 12:54 UTC (permalink / raw)
  To: Roberto Sassu, viro
  Cc: linux-security-module, linux-integrity, initramfs, linux-api,
	linux-fsdevel, linux-kernel, bug-cpio, zohar, silviu.vlasceanu,
	dmitry.kasatkin, takondra, kamensky, hpa, arnd, rob,
	james.w.mcmechan, niveditas98

Hi Roberto,

> diff --git a/init/initramfs.c b/init/initramfs.c
> index 5de396a6aac0..862c03123de8 100644
> --- a/init/initramfs.c
> +++ b/init/initramfs.c

> +static int __init do_process_metadata(char *buf, int len, bool last)
> +{

Part of the problem in upstreaming CPIO xattr support has been the
difficulty in reading and understanding the initramfs code due to a
lack of comments.  At least for any new code, let's add some comments
to simplify the review.  In this case, understanding "last", before
reading the code, would help.

Mimi

> +	int ret = 0;
> +
> +	if (!metadata_buf) {
> +		metadata_buf_ptr = metadata_buf = kmalloc(body_len, GFP_KERNEL);
> +		if (!metadata_buf_ptr) {
> +			ret = -ENOMEM;
> +			goto out;
> +		}
> +
> +		metadata_len = body_len;
> +	}
> +
> +	if (metadata_buf_ptr + len > metadata_buf + metadata_len) {
> +		ret = -EINVAL;
> +		goto out;
> +	}
> +
> +	memcpy(metadata_buf_ptr, buf, len);
> +	metadata_buf_ptr += len;
> +
> +	if (last)
> +		do_parse_metadata(previous_name_buf);
> +out:
> +	if (ret < 0 || last) {
> +		kfree(metadata_buf);
> +		metadata_buf = NULL;
> +		metadata = 0;
> +	}
> +
> +	return ret;
> +}
> +
>  static int __init do_copy(void)
>  {
>  	if (byte_count >= body_len) {
>  		if (xwrite(wfd, victim, body_len) != body_len)
>  			error("write error");
> +		if (metadata)
> +			do_process_metadata(victim, body_len, true);
>  		ksys_close(wfd);
>  		do_utime(vcollected, mtime);
>  		kfree(vcollected);
> @@ -458,6 +500,8 @@ static int __init do_copy(void)
>  	} else {
>  		if (xwrite(wfd, victim, byte_count) != byte_count)
>  			error("write error");
> +		if (metadata)
> +			do_process_metadata(victim, byte_count, false);
>  		body_len -= byte_count;
>  		eat(byte_count);
>  		return 1;
> 


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v4 0/3] initramfs: add support for xattrs in the initial ram disk
  2019-05-23 12:18 [PATCH v4 0/3] initramfs: add support for xattrs in the initial ram disk Roberto Sassu
                   ` (3 preceding siblings ...)
  2019-06-03  9:31 ` [PATCH v4 0/3] initramfs: add support for xattrs in the initial ram disk Roberto Sassu
@ 2019-07-01 13:22 ` Mimi Zohar
  4 siblings, 0 replies; 15+ messages in thread
From: Mimi Zohar @ 2019-07-01 13:22 UTC (permalink / raw)
  To: Roberto Sassu, viro
  Cc: linux-security-module, linux-integrity, initramfs, linux-api,
	linux-fsdevel, linux-kernel, bug-cpio, zohar, silviu.vlasceanu,
	dmitry.kasatkin, takondra, kamensky, hpa, arnd, rob,
	james.w.mcmechan, niveditas98

On Thu, 2019-05-23 at 14:18 +0200, Roberto Sassu wrote:
> This patch set aims at solving the following use case: appraise files from
> the initial ram disk. To do that, IMA checks the signature/hash from the
> security.ima xattr. Unfortunately, this use case cannot be implemented
> currently, as the CPIO format does not support xattrs.
> 
> This proposal consists in including file metadata as additional files named
> METADATA!!!, for each file added to the ram disk. The CPIO parser in the
> kernel recognizes these special files from the file name, and calls the
> appropriate parser to add metadata to the previously extracted file. It has
> been proposed to use bit 17:16 of the file mode as a way to recognize files
> with metadata, but both the kernel and the cpio tool declare the file mode
> as unsigned short.

Thanks, Roberto!

Victor, Taras, Rob, Arvind, Peter, if you're good with this latest
design, could we get some Reviewed-by, Acked-by, or Tested-by?

thanks!

Mimi


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v4 0/3] initramfs: add support for xattrs in the initial ram disk
  2019-06-30 15:39       ` Mimi Zohar
@ 2019-07-01 13:42         ` Roberto Sassu
  2019-07-01 14:31           ` Mimi Zohar
  0 siblings, 1 reply; 15+ messages in thread
From: Roberto Sassu @ 2019-07-01 13:42 UTC (permalink / raw)
  To: Mimi Zohar, Rob Landley, viro
  Cc: linux-security-module, linux-integrity, initramfs, linux-api,
	linux-fsdevel, linux-kernel, bug-cpio, zohar, silviu.vlasceanu,
	dmitry.kasatkin, takondra, kamensky, hpa, arnd, james.w.mcmechan,
	niveditas98

On 6/30/2019 6:39 PM, Mimi Zohar wrote:
> On Wed, 2019-06-26 at 10:15 +0200, Roberto Sassu wrote:
>> On 6/3/2019 8:32 PM, Rob Landley wrote:
>>> On 6/3/19 4:31 AM, Roberto Sassu wrote:
>>>>> This patch set aims at solving the following use case: appraise files from
>>>>> the initial ram disk. To do that, IMA checks the signature/hash from the
>>>>> security.ima xattr. Unfortunately, this use case cannot be implemented
>>>>> currently, as the CPIO format does not support xattrs.
>>>>>
>>>>> This proposal consists in including file metadata as additional files named
>>>>> METADATA!!!, for each file added to the ram disk. The CPIO parser in the
>>>>> kernel recognizes these special files from the file name, and calls the
>>>>> appropriate parser to add metadata to the previously extracted file. It has
>>>>> been proposed to use bit 17:16 of the file mode as a way to recognize files
>>>>> with metadata, but both the kernel and the cpio tool declare the file mode
>>>>> as unsigned short.
>>>>
>>>> Any opinion on this patch set?
>>>>
>>>> Thanks
>>>>
>>>> Roberto
>>>
>>> Sorry, I've had the window open since you posted it but haven't gotten around to
>>> it. I'll try to build it later today.
>>>
>>> It does look interesting, and I have no objections to the basic approach. I
>>> should be able to add support to toybox cpio over a weekend once I've got the
>>> kernel doing it to test against.
>>
>> Ok.
>>
>> Let me give some instructions so that people can test this patch set.
>>
>> To add xattrs to the ram disk embedded in the kernel it is sufficient
>> to set CONFIG_INITRAMFS_FILE_METADATA="xattr" and
>> CONFIG_INITRAMFS_SOURCE="<file with xattr>" in the kernel configuration.
>>
>> To add xattrs to the external ram disk, it is necessary to patch cpio:
>>
>> https://github.com/euleros/cpio/commit/531cabc88e9ecdc3231fad6e4856869baa9a91ef
>> (xattr-v1 branch)
>>
>> and dracut:
>>
>> https://github.com/euleros/dracut/commit/a2dee56ea80495c2c1871bc73186f7b00dc8bf3b
>> (digest-lists branch)
>>
>> The same modification can be done for mkinitramfs (add '-e xattr' to the
>> cpio command line).
>>
>> To simplify the test, it would be sufficient to replace only the cpio
>> binary and the dracut script with the modified versions. For dracut, the
>> patch should be applied to the local dracut (after it has been renamed
>> to dracut.sh).
>>
>> Then, run:
>>
>> dracut -e xattr -I <file with xattr> (add -f to overwrite the ram disk)
>>
>> Xattrs can be seen by stopping the boot process for example by adding
>> rd.break to the kernel command line.
> 
> A simple way of testing, without needing any changes other than the
> kernel patches, is to save the dracut temporary directory by supplying
> "--keep" on the dracut command line, calling
> usr/gen_initramfs_list.sh, followed by usr/gen_init_cpio with the "-e
> xattr" option.

Alternatively, follow the instructions to create the embedded ram disk
with xattrs, and use the existing external ram disk created with dracut
to check if xattrs are created.

Roberto

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v4 0/3] initramfs: add support for xattrs in the initial ram disk
  2019-07-01 13:42         ` Roberto Sassu
@ 2019-07-01 14:31           ` Mimi Zohar
  2019-07-15 16:54             ` Roberto Sassu
  0 siblings, 1 reply; 15+ messages in thread
From: Mimi Zohar @ 2019-07-01 14:31 UTC (permalink / raw)
  To: Roberto Sassu, Rob Landley, viro
  Cc: linux-security-module, linux-integrity, initramfs, linux-api,
	linux-fsdevel, linux-kernel, bug-cpio, zohar, silviu.vlasceanu,
	dmitry.kasatkin, takondra, kamensky, hpa, arnd, james.w.mcmechan,
	niveditas98

On Mon, 2019-07-01 at 16:42 +0300, Roberto Sassu wrote:
> On 6/30/2019 6:39 PM, Mimi Zohar wrote:
> > On Wed, 2019-06-26 at 10:15 +0200, Roberto Sassu wrote:
> >> On 6/3/2019 8:32 PM, Rob Landley wrote:
> >>> On 6/3/19 4:31 AM, Roberto Sassu wrote:
> >>>>> This patch set aims at solving the following use case: appraise files from
> >>>>> the initial ram disk. To do that, IMA checks the signature/hash from the
> >>>>> security.ima xattr. Unfortunately, this use case cannot be implemented
> >>>>> currently, as the CPIO format does not support xattrs.
> >>>>>
> >>>>> This proposal consists in including file metadata as additional files named
> >>>>> METADATA!!!, for each file added to the ram disk. The CPIO parser in the
> >>>>> kernel recognizes these special files from the file name, and calls the
> >>>>> appropriate parser to add metadata to the previously extracted file. It has
> >>>>> been proposed to use bit 17:16 of the file mode as a way to recognize files
> >>>>> with metadata, but both the kernel and the cpio tool declare the file mode
> >>>>> as unsigned short.
> >>>>
> >>>> Any opinion on this patch set?
> >>>>
> >>>> Thanks
> >>>>
> >>>> Roberto
> >>>
> >>> Sorry, I've had the window open since you posted it but haven't gotten around to
> >>> it. I'll try to build it later today.
> >>>
> >>> It does look interesting, and I have no objections to the basic approach. I
> >>> should be able to add support to toybox cpio over a weekend once I've got the
> >>> kernel doing it to test against.
> >>
> >> Ok.
> >>
> >> Let me give some instructions so that people can test this patch set.
> >>
> >> To add xattrs to the ram disk embedded in the kernel it is sufficient
> >> to set CONFIG_INITRAMFS_FILE_METADATA="xattr" and
> >> CONFIG_INITRAMFS_SOURCE="<file with xattr>" in the kernel configuration.
> >>
> >> To add xattrs to the external ram disk, it is necessary to patch cpio:
> >>
> >> https://github.com/euleros/cpio/commit/531cabc88e9ecdc3231fad6e4856869baa9a91ef
> >> (xattr-v1 branch)
> >>
> >> and dracut:
> >>
> >> https://github.com/euleros/dracut/commit/a2dee56ea80495c2c1871bc73186f7b00dc8bf3b
> >> (digest-lists branch)
> >>
> >> The same modification can be done for mkinitramfs (add '-e xattr' to the
> >> cpio command line).
> >>
> >> To simplify the test, it would be sufficient to replace only the cpio
> >> binary and the dracut script with the modified versions. For dracut, the
> >> patch should be applied to the local dracut (after it has been renamed
> >> to dracut.sh).
> >>
> >> Then, run:
> >>
> >> dracut -e xattr -I <file with xattr> (add -f to overwrite the ram disk)
> >>
> >> Xattrs can be seen by stopping the boot process for example by adding
> >> rd.break to the kernel command line.
> > 
> > A simple way of testing, without needing any changes other than the
> > kernel patches, is to save the dracut temporary directory by supplying
> > "--keep" on the dracut command line, calling
> > usr/gen_initramfs_list.sh, followed by usr/gen_init_cpio with the "-e
> > xattr" option.
> 
> Alternatively, follow the instructions to create the embedded ram disk
> with xattrs, and use the existing external ram disk created with dracut
> to check if xattrs are created.

True, but this alternative is for those who normally use dracut to
create an initramfs, but don't want to update cpio or dracut.

Mimi


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v4 0/3] initramfs: add support for xattrs in the initial ram disk
  2019-07-01 14:31           ` Mimi Zohar
@ 2019-07-15 16:54             ` Roberto Sassu
  2019-07-24 15:34               ` Roberto Sassu
  0 siblings, 1 reply; 15+ messages in thread
From: Roberto Sassu @ 2019-07-15 16:54 UTC (permalink / raw)
  To: Rob Landley, hpa, Arvind Sankar
  Cc: Mimi Zohar, viro, linux-security-module, linux-integrity,
	initramfs, linux-api, linux-fsdevel, linux-kernel, bug-cpio,
	zohar, silviu.vlasceanu, dmitry.kasatkin, takondra, kamensky,
	arnd, james.w.mcmechan

Rob, Peter, Arvind, did you have the chance to have a look at this
version of the patch set?

Thanks

Roberto


On 7/1/2019 4:31 PM, Mimi Zohar wrote:
> On Mon, 2019-07-01 at 16:42 +0300, Roberto Sassu wrote:
>> On 6/30/2019 6:39 PM, Mimi Zohar wrote:
>>> On Wed, 2019-06-26 at 10:15 +0200, Roberto Sassu wrote:
>>>> On 6/3/2019 8:32 PM, Rob Landley wrote:
>>>>> On 6/3/19 4:31 AM, Roberto Sassu wrote:
>>>>>>> This patch set aims at solving the following use case: appraise files from
>>>>>>> the initial ram disk. To do that, IMA checks the signature/hash from the
>>>>>>> security.ima xattr. Unfortunately, this use case cannot be implemented
>>>>>>> currently, as the CPIO format does not support xattrs.
>>>>>>>
>>>>>>> This proposal consists in including file metadata as additional files named
>>>>>>> METADATA!!!, for each file added to the ram disk. The CPIO parser in the
>>>>>>> kernel recognizes these special files from the file name, and calls the
>>>>>>> appropriate parser to add metadata to the previously extracted file. It has
>>>>>>> been proposed to use bit 17:16 of the file mode as a way to recognize files
>>>>>>> with metadata, but both the kernel and the cpio tool declare the file mode
>>>>>>> as unsigned short.
>>>>>>
>>>>>> Any opinion on this patch set?
>>>>>>
>>>>>> Thanks
>>>>>>
>>>>>> Roberto
>>>>>
>>>>> Sorry, I've had the window open since you posted it but haven't gotten around to
>>>>> it. I'll try to build it later today.
>>>>>
>>>>> It does look interesting, and I have no objections to the basic approach. I
>>>>> should be able to add support to toybox cpio over a weekend once I've got the
>>>>> kernel doing it to test against.
>>>>
>>>> Ok.
>>>>
>>>> Let me give some instructions so that people can test this patch set.
>>>>
>>>> To add xattrs to the ram disk embedded in the kernel it is sufficient
>>>> to set CONFIG_INITRAMFS_FILE_METADATA="xattr" and
>>>> CONFIG_INITRAMFS_SOURCE="<file with xattr>" in the kernel configuration.
>>>>
>>>> To add xattrs to the external ram disk, it is necessary to patch cpio:
>>>>
>>>> https://github.com/euleros/cpio/commit/531cabc88e9ecdc3231fad6e4856869baa9a91ef
>>>> (xattr-v1 branch)
>>>>
>>>> and dracut:
>>>>
>>>> https://github.com/euleros/dracut/commit/a2dee56ea80495c2c1871bc73186f7b00dc8bf3b
>>>> (digest-lists branch)
>>>>
>>>> The same modification can be done for mkinitramfs (add '-e xattr' to the
>>>> cpio command line).
>>>>
>>>> To simplify the test, it would be sufficient to replace only the cpio
>>>> binary and the dracut script with the modified versions. For dracut, the
>>>> patch should be applied to the local dracut (after it has been renamed
>>>> to dracut.sh).
>>>>
>>>> Then, run:
>>>>
>>>> dracut -e xattr -I <file with xattr> (add -f to overwrite the ram disk)
>>>>
>>>> Xattrs can be seen by stopping the boot process for example by adding
>>>> rd.break to the kernel command line.
>>>
>>> A simple way of testing, without needing any changes other than the
>>> kernel patches, is to save the dracut temporary directory by supplying
>>> "--keep" on the dracut command line, calling
>>> usr/gen_initramfs_list.sh, followed by usr/gen_init_cpio with the "-e
>>> xattr" option.
>>
>> Alternatively, follow the instructions to create the embedded ram disk
>> with xattrs, and use the existing external ram disk created with dracut
>> to check if xattrs are created.
> 
> True, but this alternative is for those who normally use dracut to
> create an initramfs, but don't want to update cpio or dracut.
> 
> Mimi
> 

-- 
HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Li Peng, Li Jian, Shi Yanli

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v4 0/3] initramfs: add support for xattrs in the initial ram disk
  2019-07-15 16:54             ` Roberto Sassu
@ 2019-07-24 15:34               ` Roberto Sassu
  0 siblings, 0 replies; 15+ messages in thread
From: Roberto Sassu @ 2019-07-24 15:34 UTC (permalink / raw)
  To: Rob Landley, hpa, Arvind Sankar
  Cc: Mimi Zohar, viro, linux-security-module, linux-integrity,
	initramfs, linux-api, linux-fsdevel, linux-kernel, bug-cpio,
	zohar, silviu.vlasceanu, dmitry.kasatkin, takondra, kamensky,
	arnd, james.w.mcmechan

Is there anything I didn't address in this patch set, that is delaying
the review? I would appreciate if you can give me a feedback, positive
or negative.

Thanks a lot!

Roberto


On 7/15/2019 6:54 PM, Roberto Sassu wrote:
> Rob, Peter, Arvind, did you have the chance to have a look at this
> version of the patch set?
> 
> Thanks
> 
> Roberto
> 
> 
> On 7/1/2019 4:31 PM, Mimi Zohar wrote:
>> On Mon, 2019-07-01 at 16:42 +0300, Roberto Sassu wrote:
>>> On 6/30/2019 6:39 PM, Mimi Zohar wrote:
>>>> On Wed, 2019-06-26 at 10:15 +0200, Roberto Sassu wrote:
>>>>> On 6/3/2019 8:32 PM, Rob Landley wrote:
>>>>>> On 6/3/19 4:31 AM, Roberto Sassu wrote:
>>>>>>>> This patch set aims at solving the following use case: appraise 
>>>>>>>> files from
>>>>>>>> the initial ram disk. To do that, IMA checks the signature/hash 
>>>>>>>> from the
>>>>>>>> security.ima xattr. Unfortunately, this use case cannot be 
>>>>>>>> implemented
>>>>>>>> currently, as the CPIO format does not support xattrs.
>>>>>>>>
>>>>>>>> This proposal consists in including file metadata as additional 
>>>>>>>> files named
>>>>>>>> METADATA!!!, for each file added to the ram disk. The CPIO 
>>>>>>>> parser in the
>>>>>>>> kernel recognizes these special files from the file name, and 
>>>>>>>> calls the
>>>>>>>> appropriate parser to add metadata to the previously extracted 
>>>>>>>> file. It has
>>>>>>>> been proposed to use bit 17:16 of the file mode as a way to 
>>>>>>>> recognize files
>>>>>>>> with metadata, but both the kernel and the cpio tool declare the 
>>>>>>>> file mode
>>>>>>>> as unsigned short.
>>>>>>>
>>>>>>> Any opinion on this patch set?
>>>>>>>
>>>>>>> Thanks
>>>>>>>
>>>>>>> Roberto
>>>>>>
>>>>>> Sorry, I've had the window open since you posted it but haven't 
>>>>>> gotten around to
>>>>>> it. I'll try to build it later today.
>>>>>>
>>>>>> It does look interesting, and I have no objections to the basic 
>>>>>> approach. I
>>>>>> should be able to add support to toybox cpio over a weekend once 
>>>>>> I've got the
>>>>>> kernel doing it to test against.
>>>>>
>>>>> Ok.
>>>>>
>>>>> Let me give some instructions so that people can test this patch set.
>>>>>
>>>>> To add xattrs to the ram disk embedded in the kernel it is sufficient
>>>>> to set CONFIG_INITRAMFS_FILE_METADATA="xattr" and
>>>>> CONFIG_INITRAMFS_SOURCE="<file with xattr>" in the kernel 
>>>>> configuration.
>>>>>
>>>>> To add xattrs to the external ram disk, it is necessary to patch cpio:
>>>>>
>>>>> https://github.com/euleros/cpio/commit/531cabc88e9ecdc3231fad6e4856869baa9a91ef 
>>>>>
>>>>> (xattr-v1 branch)
>>>>>
>>>>> and dracut:
>>>>>
>>>>> https://github.com/euleros/dracut/commit/a2dee56ea80495c2c1871bc73186f7b00dc8bf3b 
>>>>>
>>>>> (digest-lists branch)
>>>>>
>>>>> The same modification can be done for mkinitramfs (add '-e xattr' 
>>>>> to the
>>>>> cpio command line).
>>>>>
>>>>> To simplify the test, it would be sufficient to replace only the cpio
>>>>> binary and the dracut script with the modified versions. For 
>>>>> dracut, the
>>>>> patch should be applied to the local dracut (after it has been renamed
>>>>> to dracut.sh).
>>>>>
>>>>> Then, run:
>>>>>
>>>>> dracut -e xattr -I <file with xattr> (add -f to overwrite the ram 
>>>>> disk)
>>>>>
>>>>> Xattrs can be seen by stopping the boot process for example by adding
>>>>> rd.break to the kernel command line.
>>>>
>>>> A simple way of testing, without needing any changes other than the
>>>> kernel patches, is to save the dracut temporary directory by supplying
>>>> "--keep" on the dracut command line, calling
>>>> usr/gen_initramfs_list.sh, followed by usr/gen_init_cpio with the "-e
>>>> xattr" option.
>>>
>>> Alternatively, follow the instructions to create the embedded ram disk
>>> with xattrs, and use the existing external ram disk created with dracut
>>> to check if xattrs are created.
>>
>> True, but this alternative is for those who normally use dracut to
>> create an initramfs, but don't want to update cpio or dracut.
>>
>> Mimi
>>
> 

-- 
HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Li Peng, Li Jian, Shi Yanli

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2019-07-24 15:35 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-23 12:18 [PATCH v4 0/3] initramfs: add support for xattrs in the initial ram disk Roberto Sassu
2019-05-23 12:18 ` [PATCH v4 1/3] initramfs: add file metadata Roberto Sassu
2019-05-23 12:18 ` [PATCH v4 2/3] initramfs: read metadata from special file METADATA!!! Roberto Sassu
2019-07-01 12:54   ` Mimi Zohar
2019-05-23 12:18 ` [PATCH v4 3/3] gen_init_cpio: add support for file metadata Roberto Sassu
2019-06-30 15:27   ` Mimi Zohar
2019-06-03  9:31 ` [PATCH v4 0/3] initramfs: add support for xattrs in the initial ram disk Roberto Sassu
2019-06-03 18:32   ` Rob Landley
2019-06-26  8:15     ` Roberto Sassu
2019-06-30 15:39       ` Mimi Zohar
2019-07-01 13:42         ` Roberto Sassu
2019-07-01 14:31           ` Mimi Zohar
2019-07-15 16:54             ` Roberto Sassu
2019-07-24 15:34               ` Roberto Sassu
2019-07-01 13:22 ` Mimi Zohar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).