[PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk

[PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk

[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 marshaling pathnames and xattrs in a file called
.xattr-list. They are unmarshaled by the CPIO parser after all files have
been extracted.

The difference from v1 (https://lkml.org/lkml/2018/11/22/1182) is that all
xattrs are stored in a single file and not per file (solves the file name
limitation issue, as it is not necessary to add a suffix to files
containing xattrs).

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, as opposed to
defining a new one. As seen from the discussion, if a new format has to be
defined, it should fix the issues of the existing format, which requires
more time.

To fulfill both requirements, adding support for xattrs in a short time and
defining a new image format properly, this patch set takes an incremental
approach: it introduces a parser of xattrs that can be used either if
xattrs are in a regular file or directly added to the image (this patch set
reuses patch 9/15 of the existing proposal); in addition, it introduces a
wrapper of the xattr parser, to read xattrs from a file.

The changes introduced by this patch set don't cause any compatibility
issue: kernels without the xattr parser simply extracts .xattr-list and
don't unmarshal xattrs; kernels with the xattr parser don't unmarshal
xattrs if .xattr-list is not found in the image.

From the kernel space perspective, backporting this functionality to older
kernels should be very easy. It is sufficient to add a call to the new
function do_readxattrs(). From the user space perspective, no change is
required for the use case. A new dracut module (module-setup.sh) will
execute:

getfattr --absolute-names -d -P -R -e hex -m security.ima \
    <file list> | xattr.awk -b > ${initdir}/.xattr-list

where xattr.awk is the script that marshals xattrs (see patch 3/3). The
same can be done with the initramfs-tools ram disk generator.

Changelog

v1:

- move xattr unmarshaling to CPIO parser


Mimi Zohar (1):
  initramfs: set extended attributes

Roberto Sassu (2):
  fs: add ksys_lsetxattr() wrapper
  initramfs: introduce do_readxattrs()

 fs/xattr.c               |   9 ++-
 include/linux/syscalls.h |   3 +
 init/initramfs.c         | 152 ++++++++++++++++++++++++++++++++++++++-
 3 files changed, 161 insertions(+), 3 deletions(-)

-- 
2.17.1

[PATCH v2 1/3] fs: add ksys_lsetxattr() wrapper

[Roberto Sassu]

Similarly to commit 03450e271a16 ("fs: add ksys_fchmod() and do_fchmodat()
helpers and ksys_chmod() wrapper; remove in-kernel calls to syscall"), this
patch introduces the ksys_lsetxattr() helper to avoid in-kernel calls to
the sys_lsetxattr() syscall.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 fs/xattr.c               | 9 ++++++++-
 include/linux/syscalls.h | 3 +++
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/fs/xattr.c b/fs/xattr.c
index 0d6a6a4af861..422b3d481edb 100644
--- a/fs/xattr.c
+++ b/fs/xattr.c
@@ -484,11 +484,18 @@ SYSCALL_DEFINE5(setxattr, const char __user *, pathname,
 	return path_setxattr(pathname, name, value, size, flags, LOOKUP_FOLLOW);
 }
 
+int ksys_lsetxattr(const char __user *pathname,
+		   const char __user *name, const void __user *value,
+		   size_t size, int flags)
+{
+	return path_setxattr(pathname, name, value, size, flags, 0);
+}
+
 SYSCALL_DEFINE5(lsetxattr, const char __user *, pathname,
 		const char __user *, name, const void __user *, value,
 		size_t, size, int, flags)
 {
-	return path_setxattr(pathname, name, value, size, flags, 0);
+	return ksys_lsetxattr(pathname, name, value, size, flags);
 }
 
 SYSCALL_DEFINE5(fsetxattr, int, fd, const char __user *, name,
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index e446806a561f..b639f13cd1f8 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -1260,6 +1260,9 @@ int ksys_ipc(unsigned int call, int first, unsigned long second,
 	unsigned long third, void __user * ptr, long fifth);
 int compat_ksys_ipc(u32 call, int first, int second,
 	u32 third, u32 ptr, u32 fifth);
+int ksys_lsetxattr(const char __user *pathname,
+		   const char __user *name, const void __user *value,
+		   size_t size, int flags);
 
 /*
  * The following kernel syscall equivalents are just wrappers to fs-internal
-- 
2.17.1

[PATCH v2 2/3] initramfs: set extended attributes

[Roberto Sassu]

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

This patch adds xattrs to a file, with name and value taken from a supplied
buffer. The data format is:

<xattr #N data len (ASCII, 8 chars)><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(), replaced
        sys_lsetxattr() with ksys_lsetxattr(), added check for
        xattr_entry_size, added check for hdr->c_size, replaced strlen()
        with strnlen()]

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>
---
 init/initramfs.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 61 insertions(+), 2 deletions(-)

diff --git a/init/initramfs.c b/init/initramfs.c
index 4749e1115eef..98c2aa4b5ab4 100644
--- a/init/initramfs.c
+++ b/init/initramfs.c
@@ -146,7 +146,8 @@ 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 u32 name_len, xattr_len;
+static __initdata u64 body_len;
 static __initdata uid_t uid;
 static __initdata gid_t gid;
 static __initdata unsigned rdev;
@@ -218,7 +219,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, *xattr_buf;
 
 static int __init do_start(void)
 {
@@ -392,6 +393,64 @@ static int __init do_symlink(void)
 	return 0;
 }
 
+struct xattr_hdr {
+	char c_size[8]; /* total size including c_size field */
+	char c_data[];  /* <name>\0<value> */
+};
+
+static int __init do_setxattrs(void)
+{
+	char *buf = xattr_buf;
+	char *bufend = buf + xattr_len;
+	struct xattr_hdr *hdr;
+	char str[sizeof(hdr->c_size) + 1];
+
+	if (!xattr_len)
+		return 0;
+
+	str[sizeof(hdr->c_size)] = 0;
+
+	while (buf < bufend) {
+		char *xattr_name, *xattr_value;
+		unsigned long xattr_entry_size;
+		unsigned long xattr_name_size, xattr_value_size;
+		int ret;
+
+		if (buf + sizeof(hdr->c_size) > bufend) {
+			error("malformed xattrs");
+			break;
+		}
+
+		hdr = (struct xattr_hdr *)buf;
+		memcpy(str, hdr->c_size, sizeof(hdr->c_size));
+		ret = kstrtoul(str, 16, &xattr_entry_size);
+		buf += xattr_entry_size;
+		if (ret || buf > bufend || !xattr_entry_size) {
+			error("malformed xattrs");
+			break;
+		}
+
+		xattr_name = hdr->c_data;
+		xattr_name_size = strnlen(xattr_name,
+					xattr_entry_size - sizeof(hdr->c_size));
+		if (xattr_name_size == xattr_entry_size - sizeof(hdr->c_size)) {
+			error("malformed xattrs");
+			break;
+		}
+
+		xattr_value = xattr_name + xattr_name_size + 1;
+		xattr_value_size = buf - xattr_value;
+
+		ret = ksys_lsetxattr(name_buf, xattr_name, xattr_value,
+				     xattr_value_size, 0);
+
+		pr_debug("%s: %s size: %lu val: %s (ret: %d)\n", name_buf,
+			 xattr_name, xattr_value_size, xattr_value, ret);
+	}
+
+	return 0;
+}
+
 static __initdata int (*actions[])(void) = {
 	[Start]		= do_start,
 	[Collect]	= do_collect,
-- 
2.17.1

[PATCH v2 3/3] initramfs: introduce do_readxattrs()

[Roberto Sassu]

This patch adds support for an alternative method to add xattrs to files in
the rootfs filesystem. Instead of extracting them directly from the ram
disk image, they are extracted from a regular file called .xattr-list, that
can be added by any ram disk generator available today.

.xattr-list can be generated by executing:

$ getfattr --absolute-names -d -P -R -e hex -m - \
      <file list> | xattr.awk -b > ${initdir}/.xattr-list

where the content of the xattr.awk script is:

#! /usr/bin/awk -f
{
  if (!length($0)) {
    printf("%.10x%s\0", len, file);
    for (x in xattr) {
      printf("%.8x%s\0", xattr_len[x], x);
      for (i = 0; i < length(xattr[x]) / 2; i++) {
        printf("%c", strtonum("0x"substr(xattr[x], i * 2 + 1, 2)));
      }
    }
    i = 0;
    delete xattr;
    delete xattr_len;
    next;
  };
  if (i == 0) {
    file=$3;
    len=length(file) + 8 + 1;
  }
  if (i > 0) {
    split($0, a, "=");
    xattr[a[1]]=substr(a[2], 3);
    xattr_len[a[1]]=length(a[1]) + 1 + 8 + length(xattr[a[1]]) / 2;
    len+=xattr_len[a[1]];
  };
  i++;
}

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 init/initramfs.c | 89 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 89 insertions(+)

diff --git a/init/initramfs.c b/init/initramfs.c
index 98c2aa4b5ab4..91f35a84c592 100644
--- a/init/initramfs.c
+++ b/init/initramfs.c
@@ -11,6 +11,9 @@
 #include <linux/utime.h>
 #include <linux/file.h>
 
+#define XATTR_LIST_FILENAME ".xattr-list"
+
+
 static ssize_t __init xwrite(int fd, const char *p, size_t count)
 {
 	ssize_t out = 0;
@@ -451,6 +454,91 @@ static int __init do_setxattrs(void)
 	return 0;
 }
 
+struct path_hdr {
+	char p_size[10]; /* total size including p_size field */
+	char p_data[];  /* <path>\0<xattrs> */
+};
+
+static int __init do_readxattrs(void)
+{
+	struct path_hdr hdr;
+	char str[sizeof(hdr.p_size) + 1];
+	unsigned long file_entry_size;
+	size_t size, name_buf_size, total_size;
+	struct kstat st;
+	int ret, fd;
+
+	ret = vfs_lstat(XATTR_LIST_FILENAME, &st);
+	if (ret < 0)
+		return ret;
+
+	total_size = st.size;
+
+	fd = ksys_open(XATTR_LIST_FILENAME, O_RDONLY, 0);
+	if (fd < 0)
+		return fd;
+
+	while (total_size) {
+		size = ksys_read(fd, (char *)&hdr, sizeof(hdr));
+		if (size != sizeof(hdr)) {
+			ret = -EIO;
+			goto out;
+		}
+
+		total_size -= size;
+
+		memcpy(str, hdr.p_size, sizeof(hdr.p_size));
+		ret = kstrtoul(str, 16, &file_entry_size);
+		if (ret < 0)
+			goto out;
+
+		file_entry_size -= sizeof(sizeof(hdr.p_size));
+		if (file_entry_size > total_size) {
+			ret = -EINVAL;
+			goto out;
+		}
+
+		name_buf = vmalloc(file_entry_size);
+		if (!name_buf) {
+			ret = -ENOMEM;
+			goto out;
+		}
+
+		size = ksys_read(fd, name_buf, file_entry_size);
+		if (size != file_entry_size) {
+			ret = -EIO;
+			goto out_free;
+		}
+
+		total_size -= size;
+
+		name_buf_size = strnlen(name_buf, file_entry_size);
+		if (name_buf_size == file_entry_size) {
+			ret = -EINVAL;
+			goto out_free;
+		}
+
+		xattr_buf = name_buf + name_buf_size + 1;
+		xattr_len = file_entry_size - name_buf_size - 1;
+
+		ret = do_setxattrs();
+		vfree(name_buf);
+		name_buf = NULL;
+
+		if (ret < 0)
+			break;
+	}
+out_free:
+	vfree(name_buf);
+out:
+	ksys_close(fd);
+
+	if (ret < 0)
+		error("Unable to parse xattrs");
+
+	return ret;
+}
+
 static __initdata int (*actions[])(void) = {
 	[Start]		= do_start,
 	[Collect]	= do_collect,
@@ -554,6 +642,7 @@ static char * __init unpack_to_rootfs(char *buf, unsigned long len)
 		buf += my_inptr;
 		len -= my_inptr;
 	}
+	do_readxattrs();
 	dir_utime();
 	kfree(name_buf);
 	kfree(symlink_buf);
-- 
2.17.1

Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk

[Rob Landley]

On 5/9/19 6:24 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 marshaling pathnames and xattrs in a file called
> .xattr-list. They are unmarshaled by the CPIO parser after all files have
> been extracted.

So it's in-band signalling that has a higher peak memory requirement.

> 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, as opposed to
> defining a new one. As seen from the discussion, if a new format has to be
> defined, it should fix the issues of the existing format, which requires
> more time.

So you've explicitly chosen _not_ to address Y2038 while you're there.

Rob

Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk

[Roberto Sassu]

On 5/9/2019 8:34 PM, Rob Landley wrote:
> On 5/9/19 6:24 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 marshaling pathnames and xattrs in a file called
>> .xattr-list. They are unmarshaled by the CPIO parser after all files have
>> been extracted.
> 
> So it's in-band signalling that has a higher peak memory requirement.

This can be modified. Now I allocate the memory necessary for the path
and all xattrs of a file (max: .xattr-list size - 10 bytes). I could
process each xattr individually (max: 255 + 1 + 65536 bytes).


>> 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, as opposed to
>> defining a new one. As seen from the discussion, if a new format has to be
>> defined, it should fix the issues of the existing format, which requires
>> more time.
> 
> So you've explicitly chosen _not_ to address Y2038 while you're there.

Can you be more specific?

Thanks

Roberto


> Rob
> 

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

Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk

[Mimi Zohar]

On Fri, 2019-05-10 at 08:56 +0200, Roberto Sassu wrote:
> On 5/9/2019 8:34 PM, Rob Landley wrote:
> > On 5/9/19 6:24 AM, Roberto Sassu wrote:

> >> 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, as opposed to
> >> defining a new one. As seen from the discussion, if a new format has to be
> >> defined, it should fix the issues of the existing format, which requires
> >> more time.
> > 
> > So you've explicitly chosen _not_ to address Y2038 while you're there.
> 
> Can you be more specific?

Right, this patch set avoids incrementing the CPIO magic number and
the resulting changes required (eg. increasing the timestamp field
size), by including a file with the security xattrs in the CPIO.  In
either case, including the security xattrs in the initramfs header or
as a separate file, the initramfs, itself, needs to be signed.

Mimi

Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk

[Rob Landley]

On 5/10/19 6:49 AM, Mimi Zohar wrote:
> On Fri, 2019-05-10 at 08:56 +0200, Roberto Sassu wrote:
>> On 5/9/2019 8:34 PM, Rob Landley wrote:
>>> On 5/9/19 6:24 AM, Roberto Sassu wrote:
> 
>>>> 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, as opposed to
>>>> defining a new one. As seen from the discussion, if a new format has to be
>>>> defined, it should fix the issues of the existing format, which requires
>>>> more time.
>>>
>>> So you've explicitly chosen _not_ to address Y2038 while you're there.
>>
>> Can you be more specific?
> 
> Right, this patch set avoids incrementing the CPIO magic number and
> the resulting changes required (eg. increasing the timestamp field
> size), by including a file with the security xattrs in the CPIO.  In
> either case, including the security xattrs in the initramfs header or
> as a separate file, the initramfs, itself, needs to be signed.

The /init binary in the initramfs runs as root and launches all other processes
on the system. Presumably it can write any xattrs it wants to, and doesn't need
any extra permissions granted to it to do so. But as soon as you start putting
xattrs on _other_ files within the initramfs that are _not_ necessarily running
as PID 1, _that's_ when the need to sign the initramfs comes in?

Presumably the signing occurs on the gzipped file. How does that affect the cpio
parsing _after_ it's decompressed? Why would that be part of _this_ patch?

Rob

Re: [PATCH v2 1/3] fs: add ksys_lsetxattr() wrapper

[Jann Horn]

On Thu, May 09, 2019 at 01:24:18PM +0200, Roberto Sassu wrote:
> Similarly to commit 03450e271a16 ("fs: add ksys_fchmod() and do_fchmodat()
> helpers and ksys_chmod() wrapper; remove in-kernel calls to syscall"), this
> patch introduces the ksys_lsetxattr() helper to avoid in-kernel calls to
> the sys_lsetxattr() syscall.
> 
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
[...]
> +int ksys_lsetxattr(const char __user *pathname,
> +		   const char __user *name, const void __user *value,
> +		   size_t size, int flags)
> +{
> +	return path_setxattr(pathname, name, value, size, flags, 0);
> +}

Instead of exposing ksys_lsetxattr(), wouldn't it be cleaner to use
kern_path() and vfs_setxattr(), or something like that? Otherwise you're
adding more code that has to cast between kernel and user pointers.

Re: [PATCH v2 3/3] initramfs: introduce do_readxattrs()

[Jann Horn]

On Thu, May 09, 2019 at 01:24:20PM +0200, Roberto Sassu wrote:
> This patch adds support for an alternative method to add xattrs to files in
> the rootfs filesystem. Instead of extracting them directly from the ram
> disk image, they are extracted from a regular file called .xattr-list, that
> can be added by any ram disk generator available today.
[...]
> +struct path_hdr {
> +	char p_size[10]; /* total size including p_size field */
> +	char p_data[];  /* <path>\0<xattrs> */
> +};
> +
> +static int __init do_readxattrs(void)
> +{
> +	struct path_hdr hdr;
> +	char str[sizeof(hdr.p_size) + 1];
> +	unsigned long file_entry_size;
> +	size_t size, name_buf_size, total_size;
> +	struct kstat st;
> +	int ret, fd;
> +
> +	ret = vfs_lstat(XATTR_LIST_FILENAME, &st);
> +	if (ret < 0)
> +		return ret;
> +
> +	total_size = st.size;
> +
> +	fd = ksys_open(XATTR_LIST_FILENAME, O_RDONLY, 0);
> +	if (fd < 0)
> +		return fd;
> +
> +	while (total_size) {
> +		size = ksys_read(fd, (char *)&hdr, sizeof(hdr));
[...]
> +	ksys_close(fd);
> +
> +	if (ret < 0)
> +		error("Unable to parse xattrs");
> +
> +	return ret;
> +}

Please use something like filp_open()+kernel_read()+fput() instead of
ksys_open()+ksys_read()+ksys_close(). I understand that some of the init
code needs to use the syscall wrappers because no equivalent VFS
functions are available, but please use the VFS functions when that's
easy to do.

Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk

[Mimi Zohar]

On Fri, 2019-05-10 at 15:46 -0500, Rob Landley wrote:
> On 5/10/19 6:49 AM, Mimi Zohar wrote:
> > On Fri, 2019-05-10 at 08:56 +0200, Roberto Sassu wrote:
> >> On 5/9/2019 8:34 PM, Rob Landley wrote:
> >>> On 5/9/19 6:24 AM, Roberto Sassu wrote:
> > 
> >>>> 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, as opposed to
> >>>> defining a new one. As seen from the discussion, if a new format has to be
> >>>> defined, it should fix the issues of the existing format, which requires
> >>>> more time.
> >>>
> >>> So you've explicitly chosen _not_ to address Y2038 while you're there.
> >>
> >> Can you be more specific?
> > 
> > Right, this patch set avoids incrementing the CPIO magic number and
> > the resulting changes required (eg. increasing the timestamp field
> > size), by including a file with the security xattrs in the CPIO.  In
> > either case, including the security xattrs in the initramfs header or
> > as a separate file, the initramfs, itself, needs to be signed.
> 
> The /init binary in the initramfs runs as root and launches all other processes
> on the system. Presumably it can write any xattrs it wants to, and doesn't need
> any extra permissions granted to it to do so. But as soon as you start putting
> xattrs on _other_ files within the initramfs that are _not_ necessarily running
> as PID 1, _that's_ when the need to sign the initramfs comes in?
> 
> Presumably the signing occurs on the gzipped file. How does that affect the cpio
> parsing _after_ it's decompressed? Why would that be part of _this_ patch?

The signing and verification of the initramfs is a separate issue, not
part of this patch set.  The only reason for mentioning it here was to
say that both methods of including the security xattrs require the
initramfs be signed.  Just as the kernel image needs to be signed and
verified, the initramfs should be too.

Mimi

Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk

[Andy Lutomirski]

On Thu, May 9, 2019 at 4:27 AM Roberto Sassu <roberto.sassu@huawei.com> 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 marshaling pathnames and xattrs in a file called
> .xattr-list. They are unmarshaled by the CPIO parser after all files have
> been extracted.
>
> The difference from v1 (https://lkml.org/lkml/2018/11/22/1182) is that all
> xattrs are stored in a single file and not per file (solves the file name
> limitation issue, as it is not necessary to add a suffix to files
> containing xattrs).
>
> 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, as opposed to
> defining a new one. As seen from the discussion, if a new format has to be
> defined, it should fix the issues of the existing format, which requires
> more time.

I read some of those emails.  ISTM that adding TAR support should be
seriously considered.  Sure, it's baroque, but it's very, very well
supported, and it does exactly what we need.

--Andy

Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk

[Rob Landley]

On 5/11/19 5:44 PM, Andy Lutomirski wrote:
> I read some of those emails.  ISTM that adding TAR support should be
> seriously considered.  Sure, it's baroque, but it's very, very well
> supported, and it does exactly what we need.

Which means you now have two parsers supported in parallel forevermore, and are
reversing the design decision initially made when this went in without new info.

Also, I just did a tar implementation for toybox: It took me a month to debug it
(_not_ starting from scratch but from a submission), I only just added sparse
file support (because something in the android build was generating a sparse
file), there are historical tarballs I know it won't extract (I'm just testing
against what the current one produces with the default flags), and I haven't
even started on xattr support yet.

Instead I was experimenting with corner cases like "S records replace the
prefix[] field starting at byte 386 with an offset/length pair array, but
prefix[] starts at 345, do those first 41 bytes still function as a prefix and
is there any circumstance under which existing tar binaries will populate them?
Also, why does every instance of an S record generated by gnu/tar end with a
gratuitous length zero segment?"

"cpio -H newc" is a _much_ simpler format. And posix no longer specifies
_either_ format usefully, hasn't for years. From toybox tar's header comment:

 * For the command, see
 *   http://pubs.opengroup.org/onlinepubs/007908799/xcu/tar.html
 * For the modern file format, see
 *
http://pubs.opengroup.org/onlinepubs/9699919799/utilities/pax.html#tag_20_92_13_06
 *   https://en.wikipedia.org/wiki/Tar_(computing)#File_format
 *   https://www.gnu.org/software/tar/manual/html_node/Tar-Internals.html

And no, that isn't _enough_ information, you still have to "tar | hd" a lot and
squint. (There's no current spec, it's pieced together from multiple sources
because posix abdicated responsibility for this to Jorg Schilling.)

Rob

P.S. Yes that gnu/dammit page starts with a "this will be deleted" comment which
according to archive.org has been there for over a dozen years.

P.P.S. Sadly, if you want an actually standardized standard format where
implementations adhere to the standard: IETF RFC 1991 was published in 1996 and
remains compatible with files an archivers in service. Or we could stick with
cpio and make minor changes to it, since we have to remain backwards compatible
with it _anyway_....

Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk

[Rob Landley]

On 5/11/19 11:04 PM, Rob Landley wrote:
> P.P.S. Sadly, if you want an actually standardized standard format where
> implementations adhere to the standard: IETF RFC 1991 was published in 1996 and

Nope, darn it, checked my notes and that wasn't it. I thought zip had an RFC,
it's just zlib, deflate, and gzip, and that's not the number of any of them.

I still think sticking with a lightly modified cpio makes the most sense,
just... in band signalling that _doesn't_ solve the y2038 problem, the file size
limit, or address sparse files seems kinda silly.

Rob

Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk

[Dominik Brodowski]

On Thu, May 09, 2019 at 01:24:17PM +0200, Roberto Sassu wrote:
> This proposal consists in marshaling pathnames and xattrs in a file called
> .xattr-list. They are unmarshaled by the CPIO parser after all files have
> been extracted.

Couldn't this parsing of the .xattr-list file and the setting of the xattrs
be done equivalently by the initramfs' /init? Why is kernel involvement
actually required here?

Thanks,
	Dominik

Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk

[hpa]

On May 12, 2019 2:17:48 AM PDT, Dominik Brodowski <linux@dominikbrodowski.net> wrote:
>On Thu, May 09, 2019 at 01:24:17PM +0200, Roberto Sassu wrote:
>> This proposal consists in marshaling pathnames and xattrs in a file
>called
>> .xattr-list. They are unmarshaled by the CPIO parser after all files
>have
>> been extracted.
>
>Couldn't this parsing of the .xattr-list file and the setting of the
>xattrs
>be done equivalently by the initramfs' /init? Why is kernel involvement
>actually required here?
>
>Thanks,
>	Dominik

There are a lot of things that could/should be done that way...
-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk

[Mimi Zohar]

On Sun, 2019-05-12 at 11:17 +0200, Dominik Brodowski wrote:
> On Thu, May 09, 2019 at 01:24:17PM +0200, Roberto Sassu wrote:
> > This proposal consists in marshaling pathnames and xattrs in a file called
> > .xattr-list. They are unmarshaled by the CPIO parser after all files have
> > been extracted.
> 
> Couldn't this parsing of the .xattr-list file and the setting of the xattrs
> be done equivalently by the initramfs' /init? Why is kernel involvement
> actually required here?

It's too late.  The /init itself should be signed and verified.

Mimi

Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk

[Dominik Brodowski]

On Sun, May 12, 2019 at 03:18:16AM -0700, hpa@zytor.com wrote:
> > Couldn't this parsing of the .xattr-list file and the setting of the xattrs
> > be done equivalently by the initramfs' /init? Why is kernel involvement
> > actually required here?
> 
> There are a lot of things that could/should be done that way...

Indeed... so why not try to avoid adding more such "things", and keeping
them in userspace (or in a fork_usermode_blob)?


On Sun, May 12, 2019 at 08:52:47AM -0400, Mimi Zohar wrote:
> It's too late.  The /init itself should be signed and verified.

Could you elaborate a bit more about the threat model, and why deferring
this to the initramfs is too late?

Thanks,
	Dominik

Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk

[Rob Landley]

On 5/12/19 7:52 AM, Mimi Zohar wrote:
> On Sun, 2019-05-12 at 11:17 +0200, Dominik Brodowski wrote:
>> On Thu, May 09, 2019 at 01:24:17PM +0200, Roberto Sassu wrote:
>>> This proposal consists in marshaling pathnames and xattrs in a file called
>>> .xattr-list. They are unmarshaled by the CPIO parser after all files have
>>> been extracted.
>>
>> Couldn't this parsing of the .xattr-list file and the setting of the xattrs
>> be done equivalently by the initramfs' /init? Why is kernel involvement
>> actually required here?
> 
> It's too late.  The /init itself should be signed and verified.

If the initramfs cpio.gz image was signed and verified by the extractor, how is
the init in it _not_ verified?

Rob

Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk

[Arvind Sankar]

On Sun, May 12, 2019 at 05:05:48PM +0000, Rob Landley wrote:
> On 5/12/19 7:52 AM, Mimi Zohar wrote:
> > On Sun, 2019-05-12 at 11:17 +0200, Dominik Brodowski wrote:
> >> On Thu, May 09, 2019 at 01:24:17PM +0200, Roberto Sassu wrote:
> >>> This proposal consists in marshaling pathnames and xattrs in a file called
> >>> .xattr-list. They are unmarshaled by the CPIO parser after all files have
> >>> been extracted.
> >>
> >> Couldn't this parsing of the .xattr-list file and the setting of the xattrs
> >> be done equivalently by the initramfs' /init? Why is kernel involvement
> >> actually required here?
> > 
> > It's too late.  The /init itself should be signed and verified.
> 
> If the initramfs cpio.gz image was signed and verified by the extractor, how is
> the init in it _not_ verified?
> 
> Ro

Wouldn't the below work even before enforcing signatures on external
initramfs:
1. Create an embedded initramfs with an /init that does the xattr
parsing/setting. This will be verified as part of the kernel image
signature, so no new code required.
2. Add a config option/boot parameter to panic the kernel if an external
initramfs attempts to overwrite anything in the embedded initramfs. This
prevents overwriting the embedded /init even if the external initramfs
is unverified.

Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk

[Mimi Zohar]

On Sun, 2019-05-12 at 17:31 +0200, Dominik Brodowski wrote:
> On Sun, May 12, 2019 at 08:52:47AM -0400, Mimi Zohar wrote:


> > It's too late.  The /init itself should be signed and verified.
> 
> Could you elaborate a bit more about the threat model, and why deferring
> this to the initramfs is too late?

The IMA policy defines a number of different methods of identifying
which files to measure, appraise, audit.[1]  Without xattrs, the
granularity of the policy rules is severely limited.  Without xattrs,
a filesystem is either in policy, or not.

With an IMA policy rule requiring rootfs (tmpfs) files to be verified,
then /init needs to be properly labeled, otherwise /init will fail to
execute.

Mimi

[1] Documentation/ABI/testing/ima_policy

Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk

[hpa]

On May 12, 2019 5:02:30 PM PDT, Mimi Zohar <zohar@linux.ibm.com> wrote:
>On Sun, 2019-05-12 at 17:31 +0200, Dominik Brodowski wrote:
>> On Sun, May 12, 2019 at 08:52:47AM -0400, Mimi Zohar wrote:
>
>
>> > It's too late.  The /init itself should be signed and verified.
>> 
>> Could you elaborate a bit more about the threat model, and why
>deferring
>> this to the initramfs is too late?
>
>The IMA policy defines a number of different methods of identifying
>which files to measure, appraise, audit.[1]  Without xattrs, the
>granularity of the policy rules is severely limited.  Without xattrs,
>a filesystem is either in policy, or not.
>
>With an IMA policy rule requiring rootfs (tmpfs) files to be verified,
>then /init needs to be properly labeled, otherwise /init will fail to
>execute.
>
>Mimi
>
>[1] Documentation/ABI/testing/ima_policy

And the question is what is the sense in that, especially if /init is provided as play of the kernel itself.
-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk

[hpa]

On May 12, 2019 8:31:05 AM PDT, Dominik Brodowski <linux@dominikbrodowski.net> wrote:
>On Sun, May 12, 2019 at 03:18:16AM -0700, hpa@zytor.com wrote:
>> > Couldn't this parsing of the .xattr-list file and the setting of
>the xattrs
>> > be done equivalently by the initramfs' /init? Why is kernel
>involvement
>> > actually required here?
>> 
>> There are a lot of things that could/should be done that way...
>
>Indeed... so why not try to avoid adding more such "things", and
>keeping
>them in userspace (or in a fork_usermode_blob)?
>
>
>On Sun, May 12, 2019 at 08:52:47AM -0400, Mimi Zohar wrote:
>> It's too late.  The /init itself should be signed and verified.
>
>Could you elaborate a bit more about the threat model, and why
>deferring
>this to the initramfs is too late?
>
>Thanks,
>	Dominik

I tried over 10 years ago to make exactly that happen... it was called the klibc project. Linus turned it down because he felt that it didn't provide enough immediate benefit to justify the complexity, which of course creates the thousand-cuts problem: there will never be *one single* event that *by itself* justifies the transition.
-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk

[Roberto Sassu]

On 5/12/2019 9:43 PM, Arvind Sankar wrote:
> On Sun, May 12, 2019 at 05:05:48PM +0000, Rob Landley wrote:
>> On 5/12/19 7:52 AM, Mimi Zohar wrote:
>>> On Sun, 2019-05-12 at 11:17 +0200, Dominik Brodowski wrote:
>>>> On Thu, May 09, 2019 at 01:24:17PM +0200, Roberto Sassu wrote:
>>>>> This proposal consists in marshaling pathnames and xattrs in a file called
>>>>> .xattr-list. They are unmarshaled by the CPIO parser after all files have
>>>>> been extracted.
>>>>
>>>> Couldn't this parsing of the .xattr-list file and the setting of the xattrs
>>>> be done equivalently by the initramfs' /init? Why is kernel involvement
>>>> actually required here?
>>>
>>> It's too late.  The /init itself should be signed and verified.
>>
>> If the initramfs cpio.gz image was signed and verified by the extractor, how is
>> the init in it _not_ verified?
>>
>> Ro
> 
> Wouldn't the below work even before enforcing signatures on external
> initramfs:
> 1. Create an embedded initramfs with an /init that does the xattr
> parsing/setting. This will be verified as part of the kernel image
> signature, so no new code required.
> 2. Add a config option/boot parameter to panic the kernel if an external
> initramfs attempts to overwrite anything in the embedded initramfs. This
> prevents overwriting the embedded /init even if the external initramfs
> is unverified.

Unfortunately, it wouldn't work. IMA is already initialized and it would
verify /init in the embedded initial ram disk. The only reason why
opening .xattr-list works is that IMA is not yet initialized
(late_initcall vs rootfs_initcall).

Allowing a kernel with integrity enforcement to parse the CPIO image
without verifying it first is the weak point. However, extracted files
are not used, and before they are used they are verified. At the time
they are verified, they (included /init) must already have a signature
or otherwise access would be denied.

This scheme relies on the ability of the kernel to not be corrupted in
the event it parses a malformed CPIO image. Mimi suggested to use
digital signatures to prevent this issue, but it cannot be used in all
scenarios, since conventional systems generate the initial ram disk
locally.

Roberto

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

Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk

[Rob Landley]

On 5/13/19 2:49 AM, Roberto Sassu wrote:
> On 5/12/2019 9:43 PM, Arvind Sankar wrote:
>> On Sun, May 12, 2019 at 05:05:48PM +0000, Rob Landley wrote:
>>> On 5/12/19 7:52 AM, Mimi Zohar wrote:
>>>> On Sun, 2019-05-12 at 11:17 +0200, Dominik Brodowski wrote:
>>>>> On Thu, May 09, 2019 at 01:24:17PM +0200, Roberto Sassu wrote:
>>>>>> This proposal consists in marshaling pathnames and xattrs in a file called
>>>>>> .xattr-list. They are unmarshaled by the CPIO parser after all files have
>>>>>> been extracted.
>>>>>
>>>>> Couldn't this parsing of the .xattr-list file and the setting of the xattrs
>>>>> be done equivalently by the initramfs' /init? Why is kernel involvement
>>>>> actually required here?
>>>>
>>>> It's too late.  The /init itself should be signed and verified.
>>>
>>> If the initramfs cpio.gz image was signed and verified by the extractor, how is
>>> the init in it _not_ verified?
>>>
>>> Ro
>>
>> Wouldn't the below work even before enforcing signatures on external
>> initramfs:
>> 1. Create an embedded initramfs with an /init that does the xattr
>> parsing/setting. This will be verified as part of the kernel image
>> signature, so no new code required.
>> 2. Add a config option/boot parameter to panic the kernel if an external
>> initramfs attempts to overwrite anything in the embedded initramfs. This
>> prevents overwriting the embedded /init even if the external initramfs
>> is unverified.
> 
> Unfortunately, it wouldn't work. IMA is already initialized and it would
> verify /init in the embedded initial ram disk.

So you made broken infrastructure that's causing you problems. Sounds unfortunate.

> The only reason why
> opening .xattr-list works is that IMA is not yet initialized
> (late_initcall vs rootfs_initcall).

Launching init before enabling ima is bad because... you didn't think of it?

> Allowing a kernel with integrity enforcement to parse the CPIO image
> without verifying it first is the weak point.

If you don't verify the CPIO image then in theory it could have anything in it,
yes. You seem to believe that signing individual files is more secure than
signing the archive. This is certainly a point of view.

> However, extracted files
> are not used, and before they are used they are verified. At the time
> they are verified, they (included /init) must already have a signature
> or otherwise access would be denied.

You build infrastructure that works a certain way, the rest of the system
doesn't fit your assumptions, so you need to change the rest of the system to
fit your assumptions.

> This scheme relies on the ability of the kernel to not be corrupted in
> the event it parses a malformed CPIO image.

I'm unaware of any buffer overruns or wild pointer traversals in the cpio
extraction code. You can fill up all physical memory with initramfs and lock the
system hard, though.

It still only parses them at boot time before launching PID 1, right? So you
have a local physical exploit and you're trying to prevent people from working
around your Xbox copy protection without a mod chip?

> Mimi suggested to use
> digital signatures to prevent this issue, but it cannot be used in all
> scenarios, since conventional systems generate the initial ram disk
> locally.

So you use a proprietary init binary you can't rebuild from source, and put it
in a cpio where /dev/urandom is a file with known contents? Clearly, not
exploitable at all. (And we update the initramfs.cpio but not the kernel because
clearly keeping the kernel up to date is less important to security...)

Whatever happened to https://lwn.net/Articles/532778/ ? Modules are signed
in-band in the file, but you need xattrs for some reason?

> Roberto

Rob

Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk

[Mimi Zohar]

On Mon, 2019-05-13 at 04:07 -0500, Rob Landley wrote:

> > Allowing a kernel with integrity enforcement to parse the CPIO image
> > without verifying it first is the weak point.
> 
> If you don't verify the CPIO image then in theory it could have anything in it,
> yes. You seem to believe that signing individual files is more secure than
> signing the archive. This is certainly a point of view.

Nobody is claiming that signing and verifying individual files is more
secure.  We are saying that in some environments BOTH are needed.  In
many environments today the initramfs IS being signed and verified.

Unfortunately not all environments can sign the initramfs today,
because the initramfs is not distributed with the kernel image, but
generated on the target system.

Mimi

Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk

[Roberto Sassu]

On 5/13/2019 11:07 AM, Rob Landley wrote:
> 
> 
> On 5/13/19 2:49 AM, Roberto Sassu wrote:
>> On 5/12/2019 9:43 PM, Arvind Sankar wrote:
>>> On Sun, May 12, 2019 at 05:05:48PM +0000, Rob Landley wrote:
>>>> On 5/12/19 7:52 AM, Mimi Zohar wrote:
>>>>> On Sun, 2019-05-12 at 11:17 +0200, Dominik Brodowski wrote:
>>>>>> On Thu, May 09, 2019 at 01:24:17PM +0200, Roberto Sassu wrote:
>>>>>>> This proposal consists in marshaling pathnames and xattrs in a file called
>>>>>>> .xattr-list. They are unmarshaled by the CPIO parser after all files have
>>>>>>> been extracted.
>>>>>>
>>>>>> Couldn't this parsing of the .xattr-list file and the setting of the xattrs
>>>>>> be done equivalently by the initramfs' /init? Why is kernel involvement
>>>>>> actually required here?
>>>>>
>>>>> It's too late.  The /init itself should be signed and verified.
>>>>
>>>> If the initramfs cpio.gz image was signed and verified by the extractor, how is
>>>> the init in it _not_ verified?
>>>>
>>>> Ro
>>>
>>> Wouldn't the below work even before enforcing signatures on external
>>> initramfs:
>>> 1. Create an embedded initramfs with an /init that does the xattr
>>> parsing/setting. This will be verified as part of the kernel image
>>> signature, so no new code required.
>>> 2. Add a config option/boot parameter to panic the kernel if an external
>>> initramfs attempts to overwrite anything in the embedded initramfs. This
>>> prevents overwriting the embedded /init even if the external initramfs
>>> is unverified.
>>
>> Unfortunately, it wouldn't work. IMA is already initialized and it would
>> verify /init in the embedded initial ram disk.
> 
> So you made broken infrastructure that's causing you problems. Sounds unfortunate.

The idea is to be able to verify anything that is accessed, as soon as
rootfs is available, without distinction between embedded or external
initial ram disk.

Also, requiring an embedded initramfs for xattrs would be an issue for
systems that use it for other purposes.


>> The only reason why
>> opening .xattr-list works is that IMA is not yet initialized
>> (late_initcall vs rootfs_initcall).
> 
> Launching init before enabling ima is bad because... you didn't think of it?

No, because /init can potentially compromise the integrity of the
system.


>> Allowing a kernel with integrity enforcement to parse the CPIO image
>> without verifying it first is the weak point.
> 
> If you don't verify the CPIO image then in theory it could have anything in it,
> yes. You seem to believe that signing individual files is more secure than
> signing the archive. This is certainly a point of view.

As I wrote above, signing the CPIO image would be more secure, if this
option is available. However, a disadvantage would be that you have to
sign the CPIO image every time a file changes.


>> However, extracted files
>> are not used, and before they are used they are verified. At the time
>> they are verified, they (included /init) must already have a signature
>> or otherwise access would be denied.
> 
> You build infrastructure that works a certain way, the rest of the system
> doesn't fit your assumptions, so you need to change the rest of the system to
> fit your assumptions.

Requiring file metadata to make decisions seems reasonable. Also
mandatory access controls do that. The objective of this patch set is to
have uniform behavior regardless of the filesystem used.


>> This scheme relies on the ability of the kernel to not be corrupted in
>> the event it parses a malformed CPIO image.
> 
> I'm unaware of any buffer overruns or wild pointer traversals in the cpio
> extraction code. You can fill up all physical memory with initramfs and lock the
> system hard, though.
> 
> It still only parses them at boot time before launching PID 1, right? So you
> have a local physical exploit and you're trying to prevent people from working
> around your Xbox copy protection without a mod chip?

What do you mean exactly?


>> Mimi suggested to use
>> digital signatures to prevent this issue, but it cannot be used in all
>> scenarios, since conventional systems generate the initial ram disk
>> locally.
> 
> So you use a proprietary init binary you can't rebuild from source, and put it
> in a cpio where /dev/urandom is a file with known contents? Clearly, not
> exploitable at all. (And we update the initramfs.cpio but not the kernel because
> clearly keeping the kernel up to date is less important to security...)

By signing the CPIO image, the kernel wouldn't even attempt to parse it,
as the image would be rejected by the boot loader if the signature is
invalid.


> Whatever happened to https://lwn.net/Articles/532778/ ? Modules are signed
> in-band in the file, but you need xattrs for some reason?

Appending just the signature would be possible. It won't work if you
have multiple metadata for the same file.

Also appending the signature alone won't solve the parsing issue. Still,
the kernel has to parse something that could be malformed.

Roberto


>> Roberto
> 
> Rob
> 

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

Re: [PATCH v2 3/3] initramfs: introduce do_readxattrs()

[Roberto Sassu]

On 5/10/2019 11:33 PM, Jann Horn wrote:
> On Thu, May 09, 2019 at 01:24:20PM +0200, Roberto Sassu wrote:
>> This patch adds support for an alternative method to add xattrs to files in
>> the rootfs filesystem. Instead of extracting them directly from the ram
>> disk image, they are extracted from a regular file called .xattr-list, that
>> can be added by any ram disk generator available today.
> [...]
>> +struct path_hdr {
>> +	char p_size[10]; /* total size including p_size field */
>> +	char p_data[];  /* <path>\0<xattrs> */
>> +};
>> +
>> +static int __init do_readxattrs(void)
>> +{
>> +	struct path_hdr hdr;
>> +	char str[sizeof(hdr.p_size) + 1];
>> +	unsigned long file_entry_size;
>> +	size_t size, name_buf_size, total_size;
>> +	struct kstat st;
>> +	int ret, fd;
>> +
>> +	ret = vfs_lstat(XATTR_LIST_FILENAME, &st);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	total_size = st.size;
>> +
>> +	fd = ksys_open(XATTR_LIST_FILENAME, O_RDONLY, 0);
>> +	if (fd < 0)
>> +		return fd;
>> +
>> +	while (total_size) {
>> +		size = ksys_read(fd, (char *)&hdr, sizeof(hdr));
> [...]
>> +	ksys_close(fd);
>> +
>> +	if (ret < 0)
>> +		error("Unable to parse xattrs");
>> +
>> +	return ret;
>> +}
> 
> Please use something like filp_open()+kernel_read()+fput() instead of
> ksys_open()+ksys_read()+ksys_close(). I understand that some of the init
> code needs to use the syscall wrappers because no equivalent VFS
> functions are available, but please use the VFS functions when that's
> easy to do.

Ok. Thanks for the suggestion.

Roberto

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

Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk

[Arvind Sankar]

On Mon, May 13, 2019 at 02:47:04PM +0200, Roberto Sassu wrote:
> On 5/13/2019 11:07 AM, Rob Landley wrote:
> > 
> > 
> > On 5/13/19 2:49 AM, Roberto Sassu wrote:
> >> On 5/12/2019 9:43 PM, Arvind Sankar wrote:
> >>> On Sun, May 12, 2019 at 05:05:48PM +0000, Rob Landley wrote:
> >>>> On 5/12/19 7:52 AM, Mimi Zohar wrote:
> >>>>> On Sun, 2019-05-12 at 11:17 +0200, Dominik Brodowski wrote:
> >>>>>> On Thu, May 09, 2019 at 01:24:17PM +0200, Roberto Sassu wrote:
> >>>>>>> This proposal consists in marshaling pathnames and xattrs in a file called
> >>>>>>> .xattr-list. They are unmarshaled by the CPIO parser after all files have
> >>>>>>> been extracted.
> >>>>>>
> >>>>>> Couldn't this parsing of the .xattr-list file and the setting of the xattrs
> >>>>>> be done equivalently by the initramfs' /init? Why is kernel involvement
> >>>>>> actually required here?
> >>>>>
> >>>>> It's too late.  The /init itself should be signed and verified.
> >>>>
> >>>> If the initramfs cpio.gz image was signed and verified by the extractor, how is
> >>>> the init in it _not_ verified?
> >>>>
> >>>> Ro
> >>>
> >>> Wouldn't the below work even before enforcing signatures on external
> >>> initramfs:
> >>> 1. Create an embedded initramfs with an /init that does the xattr
> >>> parsing/setting. This will be verified as part of the kernel image
> >>> signature, so no new code required.
> >>> 2. Add a config option/boot parameter to panic the kernel if an external
> >>> initramfs attempts to overwrite anything in the embedded initramfs. This
> >>> prevents overwriting the embedded /init even if the external initramfs
> >>> is unverified.
> >>
> >> Unfortunately, it wouldn't work. IMA is already initialized and it would
> >> verify /init in the embedded initial ram disk.
How does this work today then? Is it actually the case that initramfs
just cannot be used on an IMA-enabled system, or it can but it leaves
the initramfs unverified and we're trying to fix that? I had assumed the
latter.
> > 
> > So you made broken infrastructure that's causing you problems. Sounds unfortunate.
> 
> The idea is to be able to verify anything that is accessed, as soon as
> rootfs is available, without distinction between embedded or external
> initial ram disk.
> 
> Also, requiring an embedded initramfs for xattrs would be an issue for
> systems that use it for other purposes.
> 
The embedded initramfs can do other things, it just has to do
the xattr stuff in addition, no?
> 
> >> The only reason why
> >> opening .xattr-list works is that IMA is not yet initialized
> >> (late_initcall vs rootfs_initcall).
> > 
> > Launching init before enabling ima is bad because... you didn't think of it?
> 
> No, because /init can potentially compromise the integrity of the
> system.
> 
How? The /init in the embedded initramfs is part of a trusted kernel
image that has been verified by the bootloader.
> 
> >> Allowing a kernel with integrity enforcement to parse the CPIO image
> >> without verifying it first is the weak point.
> > 
> > If you don't verify the CPIO image then in theory it could have anything in it,
> > yes. You seem to believe that signing individual files is more secure than
> > signing the archive. This is certainly a point of view.
> 
> As I wrote above, signing the CPIO image would be more secure, if this
> option is available. However, a disadvantage would be that you have to
> sign the CPIO image every time a file changes.
> 
> 
> >> However, extracted files
> >> are not used, and before they are used they are verified. At the time
> >> they are verified, they (included /init) must already have a signature
> >> or otherwise access would be denied.
> > 
> > You build infrastructure that works a certain way, the rest of the system
> > doesn't fit your assumptions, so you need to change the rest of the system to
> > fit your assumptions.
> 
> Requiring file metadata to make decisions seems reasonable. Also
> mandatory access controls do that. The objective of this patch set is to
> have uniform behavior regardless of the filesystem used.
> 
> 
> >> This scheme relies on the ability of the kernel to not be corrupted in
> >> the event it parses a malformed CPIO image.
> > 
> > I'm unaware of any buffer overruns or wild pointer traversals in the cpio
> > extraction code. You can fill up all physical memory with initramfs and lock the
> > system hard, though.
> > 
> > It still only parses them at boot time before launching PID 1, right? So you
> > have a local physical exploit and you're trying to prevent people from working
> > around your Xbox copy protection without a mod chip?
> 
> What do you mean exactly?
> 
> 
> >> Mimi suggested to use
> >> digital signatures to prevent this issue, but it cannot be used in all
> >> scenarios, since conventional systems generate the initial ram disk
> >> locally.
> > 
> > So you use a proprietary init binary you can't rebuild from source, and put it
> > in a cpio where /dev/urandom is a file with known contents? Clearly, not
> > exploitable at all. (And we update the initramfs.cpio but not the kernel because
> > clearly keeping the kernel up to date is less important to security...)
> 
> By signing the CPIO image, the kernel wouldn't even attempt to parse it,
> as the image would be rejected by the boot loader if the signature is
> invalid.
> 
If it were signed yes, but you just said that it isn't possible to sign
it in all cases (if initramfs is generated locally). I actually didn't
follow that bit -- if initramfs is generated locally, and it isn't
possible to sign locally, where would the IMA hashes for the contents of
the initramfs come from? Is the idea that each file within the initramfs
would be an existing, signed, file, but you could locally create an initramfs
with some subset of those unmodified files? Even assuming this is the
case, isn't the eventual intention to also appraise directories, to
prevent holes where files might be moved around/deleted/renamed etc, so
this problem would resurface anyway?
Also eventually we need to check special nodes like device nodes etc to
make sure they haven't been tampered with, as in Rob's urandom
suggestion?
> 
> > Whatever happened to https://lwn.net/Articles/532778/ ? Modules are signed
> > in-band in the file, but you need xattrs for some reason?
> 
> Appending just the signature would be possible. It won't work if you
> have multiple metadata for the same file.
> 
> Also appending the signature alone won't solve the parsing issue. Still,
> the kernel has to parse something that could be malformed.
> 
> Roberto
> 
> 
> >> Roberto
> > 
> > Rob
> > 
> 
> -- 
> HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
> Managing Director: Bo PENG, Jian LI, Yanli SHI

Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk

[Arvind Sankar]

On Mon, May 13, 2019 at 01:20:08PM -0400, Arvind Sankar wrote:
> On Mon, May 13, 2019 at 02:47:04PM +0200, Roberto Sassu wrote:
> > On 5/13/2019 11:07 AM, Rob Landley wrote:
> > > 
> > > 
> > > On 5/13/19 2:49 AM, Roberto Sassu wrote:
> > >> On 5/12/2019 9:43 PM, Arvind Sankar wrote:
> > >>> On Sun, May 12, 2019 at 05:05:48PM +0000, Rob Landley wrote:
> > >>>> On 5/12/19 7:52 AM, Mimi Zohar wrote:
> > >>>>> On Sun, 2019-05-12 at 11:17 +0200, Dominik Brodowski wrote:
> > >>>>>> On Thu, May 09, 2019 at 01:24:17PM +0200, Roberto Sassu wrote:
> > >>>>>>> This proposal consists in marshaling pathnames and xattrs in a file called
> > >>>>>>> .xattr-list. They are unmarshaled by the CPIO parser after all files have
> > >>>>>>> been extracted.
> > >>>>>>
> > >>>>>> Couldn't this parsing of the .xattr-list file and the setting of the xattrs
> > >>>>>> be done equivalently by the initramfs' /init? Why is kernel involvement
> > >>>>>> actually required here?
> > >>>>>
> > >>>>> It's too late.  The /init itself should be signed and verified.
> > >>>>
> > >>>> If the initramfs cpio.gz image was signed and verified by the extractor, how is
> > >>>> the init in it _not_ verified?
> > >>>>
> > >>>> Ro
> > >>>
> > >>> Wouldn't the below work even before enforcing signatures on external
> > >>> initramfs:
> > >>> 1. Create an embedded initramfs with an /init that does the xattr
> > >>> parsing/setting. This will be verified as part of the kernel image
> > >>> signature, so no new code required.
> > >>> 2. Add a config option/boot parameter to panic the kernel if an external
> > >>> initramfs attempts to overwrite anything in the embedded initramfs. This
> > >>> prevents overwriting the embedded /init even if the external initramfs
> > >>> is unverified.
> > >>
> > >> Unfortunately, it wouldn't work. IMA is already initialized and it would
> > >> verify /init in the embedded initial ram disk.
> How does this work today then? Is it actually the case that initramfs
> just cannot be used on an IMA-enabled system, or it can but it leaves
> the initramfs unverified and we're trying to fix that? I had assumed the
> latter.
Oooh, it's done not by starting IMA later, but by loading a default
policy that ignores the initramfs?
> > > 
> > > So you made broken infrastructure that's causing you problems. Sounds unfortunate.
> > 
> > The idea is to be able to verify anything that is accessed, as soon as
> > rootfs is available, without distinction between embedded or external
> > initial ram disk.
> > 
> > Also, requiring an embedded initramfs for xattrs would be an issue for
> > systems that use it for other purposes.
> > 
> The embedded initramfs can do other things, it just has to do
> the xattr stuff in addition, no?
> > 
> > >> The only reason why
> > >> opening .xattr-list works is that IMA is not yet initialized
> > >> (late_initcall vs rootfs_initcall).
> > > 
> > > Launching init before enabling ima is bad because... you didn't think of it?
> > 
> > No, because /init can potentially compromise the integrity of the
> > system.
> > 
> How? The /init in the embedded initramfs is part of a trusted kernel
> image that has been verified by the bootloader.
> > 
> > >> Allowing a kernel with integrity enforcement to parse the CPIO image
> > >> without verifying it first is the weak point.
> > > 
> > > If you don't verify the CPIO image then in theory it could have anything in it,
> > > yes. You seem to believe that signing individual files is more secure than
> > > signing the archive. This is certainly a point of view.
> > 
> > As I wrote above, signing the CPIO image would be more secure, if this
> > option is available. However, a disadvantage would be that you have to
> > sign the CPIO image every time a file changes.
> > 
> > 
> > >> However, extracted files
> > >> are not used, and before they are used they are verified. At the time
> > >> they are verified, they (included /init) must already have a signature
> > >> or otherwise access would be denied.
> > > 
> > > You build infrastructure that works a certain way, the rest of the system
> > > doesn't fit your assumptions, so you need to change the rest of the system to
> > > fit your assumptions.
> > 
> > Requiring file metadata to make decisions seems reasonable. Also
> > mandatory access controls do that. The objective of this patch set is to
> > have uniform behavior regardless of the filesystem used.
> > 
> > 
> > >> This scheme relies on the ability of the kernel to not be corrupted in
> > >> the event it parses a malformed CPIO image.
> > > 
> > > I'm unaware of any buffer overruns or wild pointer traversals in the cpio
> > > extraction code. You can fill up all physical memory with initramfs and lock the
> > > system hard, though.
> > > 
> > > It still only parses them at boot time before launching PID 1, right? So you
> > > have a local physical exploit and you're trying to prevent people from working
> > > around your Xbox copy protection without a mod chip?
> > 
> > What do you mean exactly?
> > 
> > 
> > >> Mimi suggested to use
> > >> digital signatures to prevent this issue, but it cannot be used in all
> > >> scenarios, since conventional systems generate the initial ram disk
> > >> locally.
> > > 
> > > So you use a proprietary init binary you can't rebuild from source, and put it
> > > in a cpio where /dev/urandom is a file with known contents? Clearly, not
> > > exploitable at all. (And we update the initramfs.cpio but not the kernel because
> > > clearly keeping the kernel up to date is less important to security...)
> > 
> > By signing the CPIO image, the kernel wouldn't even attempt to parse it,
> > as the image would be rejected by the boot loader if the signature is
> > invalid.
> > 
> If it were signed yes, but you just said that it isn't possible to sign
> it in all cases (if initramfs is generated locally). I actually didn't
> follow that bit -- if initramfs is generated locally, and it isn't
> possible to sign locally, where would the IMA hashes for the contents of
> the initramfs come from? Is the idea that each file within the initramfs
> would be an existing, signed, file, but you could locally create an initramfs
> with some subset of those unmodified files? Even assuming this is the
> case, isn't the eventual intention to also appraise directories, to
> prevent holes where files might be moved around/deleted/renamed etc, so
> this problem would resurface anyway?
> Also eventually we need to check special nodes like device nodes etc to
> make sure they haven't been tampered with, as in Rob's urandom
> suggestion?
> > 
> > > Whatever happened to https://lwn.net/Articles/532778/ ? Modules are signed
> > > in-band in the file, but you need xattrs for some reason?
> > 
> > Appending just the signature would be possible. It won't work if you
> > have multiple metadata for the same file.
> > 
> > Also appending the signature alone won't solve the parsing issue. Still,
> > the kernel has to parse something that could be malformed.
> > 
> > Roberto
> > 
> > 
> > >> Roberto
> > > 
> > > Rob
> > > 
> > 
> > -- 
> > HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
> > Managing Director: Bo PENG, Jian LI, Yanli SHI

Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk

[Arvind Sankar]

On Mon, May 13, 2019 at 01:20:08PM -0400, Arvind Sankar wrote:
> On Mon, May 13, 2019 at 02:47:04PM +0200, Roberto Sassu wrote:
> > On 5/13/2019 11:07 AM, Rob Landley wrote:
> > > 
> > > 
> > > On 5/13/19 2:49 AM, Roberto Sassu wrote:
> > >> On 5/12/2019 9:43 PM, Arvind Sankar wrote:
> > >>> On Sun, May 12, 2019 at 05:05:48PM +0000, Rob Landley wrote:
> > >>>> On 5/12/19 7:52 AM, Mimi Zohar wrote:
> > >>>>> On Sun, 2019-05-12 at 11:17 +0200, Dominik Brodowski wrote:
> > >>>>>> On Thu, May 09, 2019 at 01:24:17PM +0200, Roberto Sassu wrote:
> > >>>>>>> This proposal consists in marshaling pathnames and xattrs in a file called
> > >>>>>>> .xattr-list. They are unmarshaled by the CPIO parser after all files have
> > >>>>>>> been extracted.
> > >>>>>>
> > >>>>>> Couldn't this parsing of the .xattr-list file and the setting of the xattrs
> > >>>>>> be done equivalently by the initramfs' /init? Why is kernel involvement
> > >>>>>> actually required here?
> > >>>>>
> > >>>>> It's too late.  The /init itself should be signed and verified.
> > >>>>
> > >>>> If the initramfs cpio.gz image was signed and verified by the extractor, how is
> > >>>> the init in it _not_ verified?
> > >>>>
> > >>>> Ro
> > >>>
> > >>> Wouldn't the below work even before enforcing signatures on external
> > >>> initramfs:
> > >>> 1. Create an embedded initramfs with an /init that does the xattr
> > >>> parsing/setting. This will be verified as part of the kernel image
> > >>> signature, so no new code required.
> > >>> 2. Add a config option/boot parameter to panic the kernel if an external
> > >>> initramfs attempts to overwrite anything in the embedded initramfs. This
> > >>> prevents overwriting the embedded /init even if the external initramfs
> > >>> is unverified.
> > >>
> > >> Unfortunately, it wouldn't work. IMA is already initialized and it would
> > >> verify /init in the embedded initial ram disk.
> How does this work today then? Is it actually the case that initramfs
> just cannot be used on an IMA-enabled system, or it can but it leaves
> the initramfs unverified and we're trying to fix that? I had assumed the
> latter.
Oooh, it's done not by starting IMA appraisal later, but by loading a
default policy to ignore initramfs?
> > > 
> > > So you made broken infrastructure that's causing you problems. Sounds unfortunate.
> > 
> > The idea is to be able to verify anything that is accessed, as soon as
> > rootfs is available, without distinction between embedded or external
> > initial ram disk.
> > 
> > Also, requiring an embedded initramfs for xattrs would be an issue for
> > systems that use it for other purposes.
> > 
> The embedded initramfs can do other things, it just has to do
> the xattr stuff in addition, no?
> > 
> > >> The only reason why
> > >> opening .xattr-list works is that IMA is not yet initialized
> > >> (late_initcall vs rootfs_initcall).
> > > 
> > > Launching init before enabling ima is bad because... you didn't think of it?
> > 
> > No, because /init can potentially compromise the integrity of the
> > system.
> > 
> How? The /init in the embedded initramfs is part of a trusted kernel
> image that has been verified by the bootloader.
> > 
> > >> Allowing a kernel with integrity enforcement to parse the CPIO image
> > >> without verifying it first is the weak point.
> > > 
> > > If you don't verify the CPIO image then in theory it could have anything in it,
> > > yes. You seem to believe that signing individual files is more secure than
> > > signing the archive. This is certainly a point of view.
> > 
> > As I wrote above, signing the CPIO image would be more secure, if this
> > option is available. However, a disadvantage would be that you have to
> > sign the CPIO image every time a file changes.
> > 
> > 
> > >> However, extracted files
> > >> are not used, and before they are used they are verified. At the time
> > >> they are verified, they (included /init) must already have a signature
> > >> or otherwise access would be denied.
> > > 
> > > You build infrastructure that works a certain way, the rest of the system
> > > doesn't fit your assumptions, so you need to change the rest of the system to
> > > fit your assumptions.
> > 
> > Requiring file metadata to make decisions seems reasonable. Also
> > mandatory access controls do that. The objective of this patch set is to
> > have uniform behavior regardless of the filesystem used.
> > 
> > 
> > >> This scheme relies on the ability of the kernel to not be corrupted in
> > >> the event it parses a malformed CPIO image.
> > > 
> > > I'm unaware of any buffer overruns or wild pointer traversals in the cpio
> > > extraction code. You can fill up all physical memory with initramfs and lock the
> > > system hard, though.
> > > 
> > > It still only parses them at boot time before launching PID 1, right? So you
> > > have a local physical exploit and you're trying to prevent people from working
> > > around your Xbox copy protection without a mod chip?
> > 
> > What do you mean exactly?
> > 
> > 
> > >> Mimi suggested to use
> > >> digital signatures to prevent this issue, but it cannot be used in all
> > >> scenarios, since conventional systems generate the initial ram disk
> > >> locally.
> > > 
> > > So you use a proprietary init binary you can't rebuild from source, and put it
> > > in a cpio where /dev/urandom is a file with known contents? Clearly, not
> > > exploitable at all. (And we update the initramfs.cpio but not the kernel because
> > > clearly keeping the kernel up to date is less important to security...)
> > 
> > By signing the CPIO image, the kernel wouldn't even attempt to parse it,
> > as the image would be rejected by the boot loader if the signature is
> > invalid.
> > 
> If it were signed yes, but you just said that it isn't possible to sign
> it in all cases (if initramfs is generated locally). I actually didn't
> follow that bit -- if initramfs is generated locally, and it isn't
> possible to sign locally, where would the IMA hashes for the contents of
> the initramfs come from? Is the idea that each file within the initramfs
> would be an existing, signed, file, but you could locally create an initramfs
> with some subset of those unmodified files? Even assuming this is the
> case, isn't the eventual intention to also appraise directories, to
> prevent holes where files might be moved around/deleted/renamed etc, so
> this problem would resurface anyway?
> Also eventually we need to check special nodes like device nodes etc to
> make sure they haven't been tampered with, as in Rob's urandom
> suggestion?
> > 
> > > Whatever happened to https://lwn.net/Articles/532778/ ? Modules are signed
> > > in-band in the file, but you need xattrs for some reason?
> > 
> > Appending just the signature would be possible. It won't work if you
> > have multiple metadata for the same file.
> > 
> > Also appending the signature alone won't solve the parsing issue. Still,
> > the kernel has to parse something that could be malformed.
> > 
> > Roberto
> > 
> > 
> > >> Roberto
> > > 
> > > Rob
> > > 
> > 
> > -- 
> > HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
> > Managing Director: Bo PENG, Jian LI, Yanli SHI

Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk

[Mimi Zohar]

> > How does this work today then? Is it actually the case that initramfs
> > just cannot be used on an IMA-enabled system, or it can but it leaves
> > the initramfs unverified and we're trying to fix that? I had assumed the
> > latter.
> Oooh, it's done not by starting IMA appraisal later, but by loading a
> default policy to ignore initramfs?

Right, when rootfs is a tmpfs filesystem, it supports xattrs, allowing
for finer grained policies to be defined.  This patch set would allow
a builtin IMA appraise policy to be defined which includes tmpfs.

Mimi

Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk

[Arvind Sankar]

On Mon, May 13, 2019 at 02:36:24PM -0400, Mimi Zohar wrote:
> 
> > > How does this work today then? Is it actually the case that initramfs
> > > just cannot be used on an IMA-enabled system, or it can but it leaves
> > > the initramfs unverified and we're trying to fix that? I had assumed the
> > > latter.
> > Oooh, it's done not by starting IMA appraisal later, but by loading a
> > default policy to ignore initramfs?
> 
> Right, when rootfs is a tmpfs filesystem, it supports xattrs, allowing
> for finer grained policies to be defined.  This patch set would allow
> a builtin IMA appraise policy to be defined which includes tmpfs.
> 
> Mimi
> 
Ok, but wouldn't my idea still work? Leave the default compiled-in
policy set to not appraise initramfs. The embedded /init sets all the
xattrs, changes the policy to appraise tmpfs, and then exec's the real
init? Then everything except the embedded /init and the file with the
xattrs will be appraised, and the embedded /init was verified as part of
the kernel image signature. The only additional kernel change needed
then is to add a config option to the kernel to disallow overwriting the
embedded initramfs (or at least the embedded /init).

Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk

[Mimi Zohar]

On Mon, 2019-05-13 at 14:47 -0400, Arvind Sankar wrote:
> On Mon, May 13, 2019 at 02:36:24PM -0400, Mimi Zohar wrote:
> > 
> > > > How does this work today then? Is it actually the case that initramfs
> > > > just cannot be used on an IMA-enabled system, or it can but it leaves
> > > > the initramfs unverified and we're trying to fix that? I had assumed the
> > > > latter.
> > > Oooh, it's done not by starting IMA appraisal later, but by loading a
> > > default policy to ignore initramfs?
> > 
> > Right, when rootfs is a tmpfs filesystem, it supports xattrs, allowing
> > for finer grained policies to be defined.  This patch set would allow
> > a builtin IMA appraise policy to be defined which includes tmpfs.

Clarification: finer grain IMA policy rules are normally defined in
terms of LSM labels.  The LSMs need to enabled, before writing IMA
policy rules in terms of the LSM labels.

> > 
> Ok, but wouldn't my idea still work? Leave the default compiled-in
> policy set to not appraise initramfs. The embedded /init sets all the
> xattrs, changes the policy to appraise tmpfs, and then exec's the real
> init? Then everything except the embedded /init and the file with the
> xattrs will be appraised, and the embedded /init was verified as part of
> the kernel image signature. The only additional kernel change needed
> then is to add a config option to the kernel to disallow overwriting the
> embedded initramfs (or at least the embedded /init).

Yes and no.  The current IMA design allows a builtin policy to be
specified on the boot command line ("ima_policy="), so that it exists
from boot, and allows it to be replaced once with a custom policy.
 After that, assuming that CONFIG_IMA_WRITE_POLICY is configured,
additional rules may be appended.  As your embedded /init solution
already replaces the builtin policy, the IMA policy couldn't currently
be replaced a second time with a custom policy based on LSM labels.

Mimi

Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk

[Rob Landley]

On 5/13/19 7:47 AM, Roberto Sassu wrote:
> On 5/13/2019 11:07 AM, Rob Landley wrote:
>>>> Wouldn't the below work even before enforcing signatures on external
>>>> initramfs:
>>>> 1. Create an embedded initramfs with an /init that does the xattr
>>>> parsing/setting. This will be verified as part of the kernel image
>>>> signature, so no new code required.
>>>> 2. Add a config option/boot parameter to panic the kernel if an external
>>>> initramfs attempts to overwrite anything in the embedded initramfs. This
>>>> prevents overwriting the embedded /init even if the external initramfs
>>>> is unverified.
>>>
>>> Unfortunately, it wouldn't work. IMA is already initialized and it would
>>> verify /init in the embedded initial ram disk.
>>
>> So you made broken infrastructure that's causing you problems. Sounds
>> unfortunate.
> 
> The idea is to be able to verify anything that is accessed, as soon as
> rootfs is available, without distinction between embedded or external
> initial ram disk.

If /init is in the internal one and you can't overwrite files with an external
one, all your init has to be is something that applies the xattrs, enables your
paranoia mode, and then execs something else.

Heck, I do that sort of set up in shell scripts all the time. Running the shell
script as PID 1 and then having it exec the "real init" binary at the end:

https://github.com/landley/mkroot/blob/83def3cbae21/mkroot.sh#L205

If your first init binary is in the initramfs statically linked into the kernel
image, and the cpio code is doing open(O_EXCL), then it's as verified as any
other kernel code and runs "securely" until it decides to run something else.

> Also, requiring an embedded initramfs for xattrs would be an issue for
> systems that use it for other purposes.

I'm the guy who wrote the initmpfs code. (And has pending patches to improve it
that will probably never go upstream because I'm a hobbyist and dealing with the
 linux-kernel clique is the opposite of fun. I'm only in this conversation
because I was cc'd.)

You can totally use initramfs for lots of purposes simultaneously.

>>> The only reason why
>>> opening .xattr-list works is that IMA is not yet initialized
>>> (late_initcall vs rootfs_initcall).
>>
>> Launching init before enabling ima is bad because... you didn't think of it?
> 
> No, because /init can potentially compromise the integrity of the
> system.

Which isn't a problem if it was statically linked in the kernel, or if your
external cpio.gz was signed. You want a signed binary but don't want the
signature _in_ the binary...

>>> Allowing a kernel with integrity enforcement to parse the CPIO image
>>> without verifying it first is the weak point.
>>
>> If you don't verify the CPIO image then in theory it could have anything in it,
>> yes. You seem to believe that signing individual files is more secure than
>> signing the archive. This is certainly a point of view.
> 
> As I wrote above, signing the CPIO image would be more secure, if this
> option is available. However, a disadvantage would be that you have to
> sign the CPIO image every time a file changes.

Which is why there's a cpio in the kernel and an external cpio loaded via the
old initrd mechanism and BOTH files wind up in the cpio and there's a way to
make it O_EXCL so it can't overwrite, and then the /init binary inside the
kernel's cpio can do any other weird verification you need to do before anything
else gets a chance to run so why are you having ring 0 kernel code read a file
out of the filesystem and act upon it?

(Heck, you can mv /newinit /init before the exec /init so the file isn't on the
system anymore by the time the other stuff gets to run...)

>>> However, extracted files
>>> are not used, and before they are used they are verified. At the time
>>> they are verified, they (included /init) must already have a signature
>>> or otherwise access would be denied.
>>
>> You build infrastructure that works a certain way, the rest of the system
>> doesn't fit your assumptions, so you need to change the rest of the system to
>> fit your assumptions.
> 
> Requiring file metadata to make decisions seems reasonable. Also
> mandatory access controls do that. The objective of this patch set is to
> have uniform behavior regardless of the filesystem used.

If it's in the file's contents you get uniform behavior regardless of the
filesystem used. And "mandatory access controls do that" is basically restating
what _I_ said in the paragraph above.

The "infrastructure you have that works a certain way" is called "mandatory
access controls". Good to know. Your patch changes the rest of the system to
match the assumptions of the new code, because changing those assumptions
appears literally unthinkable.

>>> This scheme relies on the ability of the kernel to not be corrupted in
>>> the event it parses a malformed CPIO image.
>>
>> I'm unaware of any buffer overruns or wild pointer traversals in the cpio
>> extraction code. You can fill up all physical memory with initramfs and lock the
>> system hard, though.
>>
>> It still only parses them at boot time before launching PID 1, right? So you
>> have a local physical exploit and you're trying to prevent people from working
>> around your Xbox copy protection without a mod chip?
> 
> What do you mean exactly?

That you're not remotely the first person to do this?

You're attempting to prevent anyone from running third party code on your system
without buying a license from you first. You're creating a system with no user
serviceable parts, that only runs authorized software from the Apple Store or
other walled garden. No sideloading allowed.

Which is your choice, sure. But why do you need new infrastructure to do it?
People have already _done_ this. They're just by nature proprietary and don't
like sharing with the group when not forced by lawyers, so they come up with
ways that don't involve modifying GPLv2 software (or shipping GPLv3 software,
ever, for any reason).

>>> Mimi suggested to use
>>> digital signatures to prevent this issue, but it cannot be used in all
>>> scenarios, since conventional systems generate the initial ram disk
>>> locally.
>>
>> So you use a proprietary init binary you can't rebuild from source, and put it
>> in a cpio where /dev/urandom is a file with known contents? Clearly, not
>> exploitable at all. (And we update the initramfs.cpio but not the kernel because
>> clearly keeping the kernel up to date is less important to security...)
> 
> By signing the CPIO image, the kernel wouldn't even attempt to parse it,
> as the image would be rejected by the boot loader if the signature is
> invalid.

So you have _more_ assumptions tripping you up. Great. So add a signature in a
format your bootloader doesn't recognize, since it's the kernel that should
verify it, not your bootloader?

It sounds like your problem is bureaucratic, not technical.

>> Whatever happened to https://lwn.net/Articles/532778/ ? Modules are signed
>> in-band in the file, but you need xattrs for some reason?
> 
> Appending just the signature would be possible. It won't work if you
> have multiple metadata for the same file.

Call the elf sections SIG1 SIG2 SIG3, or have a section full of keyword=value
strings? How is this a hard problem?

> Also appending the signature alone won't solve the parsing issue. Still,
> the kernel has to parse something that could be malformed.

Your new in-band signaling file you're making xattrs from could be malformed,
one of the xattrs you add could be "banana=aaaaaaaaaaaaaaaaaaaaaaaaaaa..." going
on for 12 megabytes...

Rob

Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk

[Rob Landley]

On 5/13/19 5:09 PM, Mimi Zohar wrote:
>> Ok, but wouldn't my idea still work? Leave the default compiled-in
>> policy set to not appraise initramfs. The embedded /init sets all the
>> xattrs, changes the policy to appraise tmpfs, and then exec's the real
>> init? Then everything except the embedded /init and the file with the
>> xattrs will be appraised, and the embedded /init was verified as part of
>> the kernel image signature. The only additional kernel change needed
>> then is to add a config option to the kernel to disallow overwriting the
>> embedded initramfs (or at least the embedded /init).
> 
> Yes and no.  The current IMA design allows a builtin policy to be
> specified on the boot command line ("ima_policy="), so that it exists
> from boot, and allows it to be replaced once with a custom policy.
>  After that, assuming that CONFIG_IMA_WRITE_POLICY is configured,
> additional rules may be appended.  As your embedded /init solution
> already replaces the builtin policy, the IMA policy couldn't currently
> be replaced a second time with a custom policy based on LSM labels.

So your design assumption you're changing other code to work around in that
instance is the policy can only be replaced once rather than having a "finalize"
option when it's set, making it immutable from then on.

Rob

Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk

[Roberto Sassu]

On 5/14/2019 8:06 AM, Rob Landley wrote:
> On 5/13/19 7:47 AM, Roberto Sassu wrote:
>> On 5/13/2019 11:07 AM, Rob Landley wrote:
>>>>> Wouldn't the below work even before enforcing signatures on external
>>>>> initramfs:
>>>>> 1. Create an embedded initramfs with an /init that does the xattr
>>>>> parsing/setting. This will be verified as part of the kernel image
>>>>> signature, so no new code required.
>>>>> 2. Add a config option/boot parameter to panic the kernel if an external
>>>>> initramfs attempts to overwrite anything in the embedded initramfs. This
>>>>> prevents overwriting the embedded /init even if the external initramfs
>>>>> is unverified.
>>>>
>>>> Unfortunately, it wouldn't work. IMA is already initialized and it would
>>>> verify /init in the embedded initial ram disk.
>>>
>>> So you made broken infrastructure that's causing you problems. Sounds
>>> unfortunate.
>>
>> The idea is to be able to verify anything that is accessed, as soon as
>> rootfs is available, without distinction between embedded or external
>> initial ram disk.
> 
> If /init is in the internal one and you can't overwrite files with an external
> one, all your init has to be is something that applies the xattrs, enables your
> paranoia mode, and then execs something else.

Shouldn't file metadata be handled by the same code that extracts the
content? Instead, file content is extracted by the kernel, and we are
adding another step to the boot process, to execute a new binary with a
link to libc.

 From the perspective of a remote verifier that checks the software
running on the system, would it be easier to check less than 150 lines
of code, or a CPIO image containing a binary + libc?


> Heck, I do that sort of set up in shell scripts all the time. Running the shell
> script as PID 1 and then having it exec the "real init" binary at the end:
> 
> https://github.com/landley/mkroot/blob/83def3cbae21/mkroot.sh#L205
> 
> If your first init binary is in the initramfs statically linked into the kernel
> image, and the cpio code is doing open(O_EXCL), then it's as verified as any
> other kernel code and runs "securely" until it decides to run something else.
> 
>> Also, requiring an embedded initramfs for xattrs would be an issue for
>> systems that use it for other purposes.
> 
> I'm the guy who wrote the initmpfs code. (And has pending patches to improve it
> that will probably never go upstream because I'm a hobbyist and dealing with the
>   linux-kernel clique is the opposite of fun. I'm only in this conversation
> because I was cc'd.)
> 
> You can totally use initramfs for lots of purposes simultaneously.

Yes, I agree. However, adding an initramfs to initialize another
initramfs when you can simply extract file content and metadata with the
same parser, this for me it is difficult to justify.


>>>> The only reason why
>>>> opening .xattr-list works is that IMA is not yet initialized
>>>> (late_initcall vs rootfs_initcall).
>>>
>>> Launching init before enabling ima is bad because... you didn't think of it?
>>
>> No, because /init can potentially compromise the integrity of the
>> system.
> 
> Which isn't a problem if it was statically linked in the kernel, or if your
> external cpio.gz was signed. You want a signed binary but don't want the
> signature _in_ the binary...

It is not just for binaries. How you would deal with arbitrary file
formats?


>>>> Allowing a kernel with integrity enforcement to parse the CPIO image
>>>> without verifying it first is the weak point.
>>>
>>> If you don't verify the CPIO image then in theory it could have anything in it,
>>> yes. You seem to believe that signing individual files is more secure than
>>> signing the archive. This is certainly a point of view.
>>
>> As I wrote above, signing the CPIO image would be more secure, if this
>> option is available. However, a disadvantage would be that you have to
>> sign the CPIO image every time a file changes.
> 
> Which is why there's a cpio in the kernel and an external cpio loaded via the
> old initrd mechanism and BOTH files wind up in the cpio and there's a way to
> make it O_EXCL so it can't overwrite, and then the /init binary inside the
> kernel's cpio can do any other weird verification you need to do before anything
> else gets a chance to run so why are you having ring 0 kernel code read a file
> out of the filesystem and act upon it?

The CPIO parser already invokes many system calls.


> (Heck, you can mv /newinit /init before the exec /init so the file isn't on the
> system anymore by the time the other stuff gets to run...)
> 
>>>> However, extracted files
>>>> are not used, and before they are used they are verified. At the time
>>>> they are verified, they (included /init) must already have a signature
>>>> or otherwise access would be denied.
>>>
>>> You build infrastructure that works a certain way, the rest of the system
>>> doesn't fit your assumptions, so you need to change the rest of the system to
>>> fit your assumptions.
>>
>> Requiring file metadata to make decisions seems reasonable. Also
>> mandatory access controls do that. The objective of this patch set is to
>> have uniform behavior regardless of the filesystem used.
> 
> If it's in the file's contents you get uniform behavior regardless of the
> filesystem used. And "mandatory access controls do that" is basically restating
> what _I_ said in the paragraph above.

As I said, that does not work with arbitrary file formats.


> The "infrastructure you have that works a certain way" is called "mandatory
> access controls". Good to know. Your patch changes the rest of the system to
> match the assumptions of the new code, because changing those assumptions
> appears literally unthinkable.

All I want to do is to have the same behavior as if there is no initial
ram disk. And given that inode-based MACs read the labels from xattrs,
the assumption that the system provides xattrs even in the inital ram
disk seems reasonable.


>>>> This scheme relies on the ability of the kernel to not be corrupted in
>>>> the event it parses a malformed CPIO image.
>>>
>>> I'm unaware of any buffer overruns or wild pointer traversals in the cpio
>>> extraction code. You can fill up all physical memory with initramfs and lock the
>>> system hard, though.
>>>
>>> It still only parses them at boot time before launching PID 1, right? So you
>>> have a local physical exploit and you're trying to prevent people from working
>>> around your Xbox copy protection without a mod chip?
>>
>> What do you mean exactly?
> 
> That you're not remotely the first person to do this?
> 
> You're attempting to prevent anyone from running third party code on your system
> without buying a license from you first. You're creating a system with no user
> serviceable parts, that only runs authorized software from the Apple Store or
> other walled garden. No sideloading allowed.

This is one use case. The main purpose of IMA is to preserve the
integrity of the Trusted Computing Base (TCB, the critical part of the
system), or to detect integrity violations without enforcement. This is
done by ensuring that the software comes from the vendor. Applications
owned by users are allowed to run, as the Discrectionary Access Control
(DAC) prevents attacks to the TCB. I'm working on a more advanced scheme
that relies on MAC.


> Which is your choice, sure. But why do you need new infrastructure to do it?
> People have already _done_ this. They're just by nature proprietary and don't
> like sharing with the group when not forced by lawyers, so they come up with
> ways that don't involve modifying GPLv2 software (or shipping GPLv3 software,
> ever, for any reason).
> 
>>>> Mimi suggested to use
>>>> digital signatures to prevent this issue, but it cannot be used in all
>>>> scenarios, since conventional systems generate the initial ram disk
>>>> locally.
>>>
>>> So you use a proprietary init binary you can't rebuild from source, and put it
>>> in a cpio where /dev/urandom is a file with known contents? Clearly, not
>>> exploitable at all. (And we update the initramfs.cpio but not the kernel because
>>> clearly keeping the kernel up to date is less important to security...)
>>
>> By signing the CPIO image, the kernel wouldn't even attempt to parse it,
>> as the image would be rejected by the boot loader if the signature is
>> invalid.
> 
> So you have _more_ assumptions tripping you up. Great. So add a signature in a
> format your bootloader doesn't recognize, since it's the kernel that should
> verify it, not your bootloader?
> 
> It sounds like your problem is bureaucratic, not technical.

The boot loader verifies the CPIO image, when this is possible. The
kernel verifies individual files when the CPIO image is not signed.

If a remote verifier wants to verify the measurement of the CPIO image,
and he only has reference digests for each file, he has to build the
CPIO image with files reference digests were calculated from, and in the
same way it was done by the system target of the evaluation.


>>> Whatever happened to https://lwn.net/Articles/532778/ ? Modules are signed
>>> in-band in the file, but you need xattrs for some reason?
>>
>> Appending just the signature would be possible. It won't work if you
>> have multiple metadata for the same file.
> 
> Call the elf sections SIG1 SIG2 SIG3, or have a section full of keyword=value
> strings? How is this a hard problem?
> 
>> Also appending the signature alone won't solve the parsing issue. Still,
>> the kernel has to parse something that could be malformed.
> 
> Your new in-band signaling file you're making xattrs from could be malformed,
> one of the xattrs you add could be "banana=aaaaaaaaaaaaaaaaaaaaaaaaaaa..." going
> on for 12 megabytes...

ksys_lsetxattr() checks the limits.

Roberto


> Rob
> 

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

Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk

[Arvind Sankar]

On Tue, May 14, 2019 at 01:06:45AM -0500, Rob Landley wrote:
> On 5/13/19 5:09 PM, Mimi Zohar wrote:
> >> Ok, but wouldn't my idea still work? Leave the default compiled-in
> >> policy set to not appraise initramfs. The embedded /init sets all the
> >> xattrs, changes the policy to appraise tmpfs, and then exec's the real
> >> init? Then everything except the embedded /init and the file with the
> >> xattrs will be appraised, and the embedded /init was verified as part of
> >> the kernel image signature. The only additional kernel change needed
> >> then is to add a config option to the kernel to disallow overwriting the
> >> embedded initramfs (or at least the embedded /init).
> > 
> > Yes and no.  The current IMA design allows a builtin policy to be
> > specified on the boot command line ("ima_policy="), so that it exists
> > from boot, and allows it to be replaced once with a custom policy.
> >  After that, assuming that CONFIG_IMA_WRITE_POLICY is configured,
> > additional rules may be appended.  As your embedded /init solution
> > already replaces the builtin policy, the IMA policy couldn't currently
> > be replaced a second time with a custom policy based on LSM labels.
> 
> So your design assumption you're changing other code to work around in that
> instance is the policy can only be replaced once rather than having a "finalize"
> option when it's set, making it immutable from then on.
> 
> Rob
I agree it would be better to have a finalize option. Outside of my
idea, it seems the current setup would make it so while developing an
IMA policy you need to keep rebooting to test your changes?

I'd suggest having a knob that starts out unrestricted, and can be
one-way changed to append-only or immutable. This seems like a good idea
even if you decide the embedded image is too much trouble or unworkable
for other reasons.

Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk

[Rob Landley]

On 5/14/19 6:52 AM, Roberto Sassu wrote:
> On 5/14/2019 8:06 AM, Rob Landley wrote:
>> On 5/13/19 7:47 AM, Roberto Sassu wrote:
>>> On 5/13/2019 11:07 AM, Rob Landley wrote:
>>>>>> Wouldn't the below work even before enforcing signatures on external
>>>>>> initramfs:
>>>>>> 1. Create an embedded initramfs with an /init that does the xattr
>>>>>> parsing/setting. This will be verified as part of the kernel image
>>>>>> signature, so no new code required.
>>>>>> 2. Add a config option/boot parameter to panic the kernel if an external
>>>>>> initramfs attempts to overwrite anything in the embedded initramfs. This
>>>>>> prevents overwriting the embedded /init even if the external initramfs
>>>>>> is unverified.
>>>>>
>>>>> Unfortunately, it wouldn't work. IMA is already initialized and it would
>>>>> verify /init in the embedded initial ram disk.
>>>>
>>>> So you made broken infrastructure that's causing you problems. Sounds
>>>> unfortunate.
>>>
>>> The idea is to be able to verify anything that is accessed, as soon as
>>> rootfs is available, without distinction between embedded or external
>>> initial ram disk.
>>
>> If /init is in the internal one and you can't overwrite files with an external
>> one, all your init has to be is something that applies the xattrs, enables your
>> paranoia mode, and then execs something else.
> 
> Shouldn't file metadata be handled by the same code that extracts the
> content? Instead, file content is extracted by the kernel, and we are
> adding another step to the boot process, to execute a new binary with a
> link to libc.

I haven't made a dynamically linked initramfs in years (except a couple for
testing purposes). But then I don't deploy glibc, so...

> From the perspective of a remote verifier that checks the software
> running on the system, would it be easier to check less than 150 lines
> of code, or a CPIO image containing a binary + libc?

https://github.com/torvalds/linux/blob/master/tools/include/nolibc/nolibc.h

(I have a todo item to add sh4 and m68k and ppc and such sections to that, but
see "I've needed to resubmit
http://lkml.iu.edu/hypermail/linux/kernel/1709.1/03561.html for a couple years
now but it works for me locally and dealing with linux-kernel is just no fun
anymore"...)

>> You can totally use initramfs for lots of purposes simultaneously.
> 
> Yes, I agree. However, adding an initramfs to initialize another
> initramfs when you can simply extract file content and metadata with the
> same parser, this for me it is difficult to justify.

You just said it's simpler to modify the kernel than do a thing you can already
do in userspace. You realize that, right?

>>>>> The only reason why
>>>>> opening .xattr-list works is that IMA is not yet initialized
>>>>> (late_initcall vs rootfs_initcall).
>>>>
>>>> Launching init before enabling ima is bad because... you didn't think of it?
>>>
>>> No, because /init can potentially compromise the integrity of the
>>> system.
>>
>> Which isn't a problem if it was statically linked in the kernel, or if your
>> external cpio.gz was signed. You want a signed binary but don't want the
>> signature _in_ the binary...
> 
> It is not just for binaries. How you would deal with arbitrary file
> formats?

I'm confused, are you saying that /init can/should be an arbitrary file format,
or that a cpio statically linked into the kernel can't contain files in
arbitrary formats?

>> Which is why there's a cpio in the kernel and an external cpio loaded via the
>> old initrd mechanism and BOTH files wind up in the cpio and there's a way to
>> make it O_EXCL so it can't overwrite, and then the /init binary inside the
>> kernel's cpio can do any other weird verification you need to do before anything
>> else gets a chance to run so why are you having ring 0 kernel code read a file
>> out of the filesystem and act upon it?
> 
> The CPIO parser already invokes many system calls.

The one in the kernel doesn't call system calls, no. Once userspace is running
it can do what it likes. The one statically linked into the kernel was set up by
the same people who built the kernel; if you're letting arbitrary kernels run on
your system it's kinda over already from a security context?

>> If it's in the file's contents you get uniform behavior regardless of the
>> filesystem used. And "mandatory access controls do that" is basically restating
>> what _I_ said in the paragraph above.
> 
> As I said, that does not work with arbitrary file formats.

an /init binary can parse your .inbandsignalling file to apply xattrs to
arbitrary files before handing off to something else, and a cpio.gz statically
linked into the kernel can contain arbitrary files.

>> The "infrastructure you have that works a certain way" is called "mandatory
>> access controls". Good to know. Your patch changes the rest of the system to
>> match the assumptions of the new code, because changing those assumptions
>> appears literally unthinkable.
> 
> All I want to do is to have the same behavior as if there is no initial
> ram disk. And given that inode-based MACs read the labels from xattrs,
> the assumption that the system provides xattrs even in the inital ram
> disk seems reasonable.

There was a previous proposal for a new revision of cpio that I don't remember
particularly objecting to? Which did things that can't trivially be done in
userspace?

>>> What do you mean exactly?
>>
>> That you're not remotely the first person to do this?
>>
>> You're attempting to prevent anyone from running third party code on your system
>> without buying a license from you first. You're creating a system with no user
>> serviceable parts, that only runs authorized software from the Apple Store or
>> other walled garden. No sideloading allowed.
> 
> This is one use case. The main purpose of IMA is to preserve the
> integrity of the Trusted Computing Base (TCB, the critical part of the
> system), or to detect integrity violations without enforcement. This is
> done by ensuring that the software comes from the vendor. Applications
> owned by users are allowed to run, as the Discrectionary Access Control
> (DAC) prevents attacks to the TCB. I'm working on a more advanced scheme
> that relies on MAC.

Sure, same general idea as Apple's lobbying against "right to repair".

https://appleinsider.com/articles/19/03/18/california-reintroduces-right-to-repair-bill-after-previous-effort-failed

The vendor can prevent any unauthorized software from running on the device, or
even retroactively remove older software to force upgrades:

https://www.macrumors.com/2019/05/13/adobe-creative-cloud-legal-action-older-apps/

Or anything else, of course:

https://www.zdnet.com/article/why-amazon-is-within-its-rights-to-remove-access-to-your-kindle-books/

*shrug* Your choice, of course...

>> So you have _more_ assumptions tripping you up. Great. So add a signature in a
>> format your bootloader doesn't recognize, since it's the kernel that should
>> verify it, not your bootloader?
>>
>> It sounds like your problem is bureaucratic, not technical.
> 
> The boot loader verifies the CPIO image, when this is possible. The
> kernel verifies individual files when the CPIO image is not signed.
> 
> If a remote verifier wants to verify the measurement of the CPIO image,
> and he only has reference digests for each file, he has to build the
> CPIO image with files reference digests were calculated from, and in the
> same way it was done by the system target of the evaluation.

And your init program can parse your .inbandsignaling file to put the xattrs on
the files and then poke the "now enforce" button.

>>>> Whatever happened to https://lwn.net/Articles/532778/ ? Modules are signed
>>>> in-band in the file, but you need xattrs for some reason?
>>>
>>> Appending just the signature would be possible. It won't work if you
>>> have multiple metadata for the same file.
>>
>> Call the elf sections SIG1 SIG2 SIG3, or have a section full of keyword=value
>> strings? How is this a hard problem?
>>
>>> Also appending the signature alone won't solve the parsing issue. Still,
>>> the kernel has to parse something that could be malformed.
>>
>> Your new in-band signaling file you're making xattrs from could be malformed,
>> one of the xattrs you add could be "banana=aaaaaaaaaaaaaaaaaaaaaaaaaaa..." going
>> on for 12 megabytes...
> 
> ksys_lsetxattr() checks the limits.

Not if it caused an oom error extracting your .inbandsignaling file before it
got consumed. (The kernel has to parse something that could be malformed and
that's bad reading ELF information like linux has done loading binaries since
1996, but it's ok for xattrs with a system call?)

*shrug* I've made my opinion clear and don't think this thread is useful at this
point, I'm not the maintainer with merge authority, so I'm gonna mute it now.

Rob

Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk

[Andy Lutomirski]

On Mon, May 13, 2019 at 5:47 AM Roberto Sassu <roberto.sassu@huawei.com> wrote:
>
> On 5/13/2019 11:07 AM, Rob Landley wrote:
> >
> >
> > On 5/13/19 2:49 AM, Roberto Sassu wrote:
> >> On 5/12/2019 9:43 PM, Arvind Sankar wrote:
> >>> On Sun, May 12, 2019 at 05:05:48PM +0000, Rob Landley wrote:
> >>>> On 5/12/19 7:52 AM, Mimi Zohar wrote:
> >>>>> On Sun, 2019-05-12 at 11:17 +0200, Dominik Brodowski wrote:
> >>>>>> On Thu, May 09, 2019 at 01:24:17PM +0200, Roberto Sassu wrote:
> >>>>>>> This proposal consists in marshaling pathnames and xattrs in a file called
> >>>>>>> .xattr-list. They are unmarshaled by the CPIO parser after all files have
> >>>>>>> been extracted.
> >>>>>>
> >>>>>> Couldn't this parsing of the .xattr-list file and the setting of the xattrs
> >>>>>> be done equivalently by the initramfs' /init? Why is kernel involvement
> >>>>>> actually required here?
> >>>>>
> >>>>> It's too late.  The /init itself should be signed and verified.
> >>>>
> >>>> If the initramfs cpio.gz image was signed and verified by the extractor, how is
> >>>> the init in it _not_ verified?
> >>>>
> >>>> Ro
> >>>
> >>> Wouldn't the below work even before enforcing signatures on external
> >>> initramfs:
> >>> 1. Create an embedded initramfs with an /init that does the xattr
> >>> parsing/setting. This will be verified as part of the kernel image
> >>> signature, so no new code required.
> >>> 2. Add a config option/boot parameter to panic the kernel if an external
> >>> initramfs attempts to overwrite anything in the embedded initramfs. This
> >>> prevents overwriting the embedded /init even if the external initramfs
> >>> is unverified.
> >>
> >> Unfortunately, it wouldn't work. IMA is already initialized and it would
> >> verify /init in the embedded initial ram disk.
> >
> > So you made broken infrastructure that's causing you problems. Sounds unfortunate.
>
> The idea is to be able to verify anything that is accessed, as soon as
> rootfs is available, without distinction between embedded or external
> initial ram disk.
>
> Also, requiring an embedded initramfs for xattrs would be an issue for
> systems that use it for other purposes.
>
>
> >> The only reason why
> >> opening .xattr-list works is that IMA is not yet initialized
> >> (late_initcall vs rootfs_initcall).
> >
> > Launching init before enabling ima is bad because... you didn't think of it?
>
> No, because /init can potentially compromise the integrity of the
> system.

I think Rob is right here.  If /init was statically built into the
kernel image, it has no more ability to compromise the kernel than
anything else in the kernel.  What's the problem here?

Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk

[Arvind Sankar]

On Tue, May 14, 2019 at 01:52:42PM +0200, Roberto Sassu wrote:
> On 5/14/2019 8:06 AM, Rob Landley wrote:
> > On 5/13/19 7:47 AM, Roberto Sassu wrote:
> >> On 5/13/2019 11:07 AM, Rob Landley wrote:
> >>>>> Wouldn't the below work even before enforcing signatures on external
> >>>>> initramfs:
> >>>>> 1. Create an embedded initramfs with an /init that does the xattr
> >>>>> parsing/setting. This will be verified as part of the kernel image
> >>>>> signature, so no new code required.
> >>>>> 2. Add a config option/boot parameter to panic the kernel if an external
> >>>>> initramfs attempts to overwrite anything in the embedded initramfs. This
> >>>>> prevents overwriting the embedded /init even if the external initramfs
> >>>>> is unverified.
> >>>>
> >>>> Unfortunately, it wouldn't work. IMA is already initialized and it would
> >>>> verify /init in the embedded initial ram disk.
> >>>
> >>> So you made broken infrastructure that's causing you problems. Sounds
> >>> unfortunate.
> >>
> >> The idea is to be able to verify anything that is accessed, as soon as
> >> rootfs is available, without distinction between embedded or external
> >> initial ram disk.
> > 
> > If /init is in the internal one and you can't overwrite files with an external
> > one, all your init has to be is something that applies the xattrs, enables your
> > paranoia mode, and then execs something else.
> 
> Shouldn't file metadata be handled by the same code that extracts the
> content? Instead, file content is extracted by the kernel, and we are
> adding another step to the boot process, to execute a new binary with a
> link to libc.
> 
>  From the perspective of a remote verifier that checks the software
> running on the system, would it be easier to check less than 150 lines
> of code, or a CPIO image containing a binary + libc?
> 
Isn't the remote attestation process just involving the boot measurement
to verify that software has not changed, not actually reviewing the
code? How does it matter what's inside the kernel image as long as we
know it hasn't changed from when it was installed?
You don't need much of a libc in any case -- the userspace code is not
going to be doing anything different from what you're proposing to do
inside the kernel, and I would expect the userland code to be
easier to test and verify for correctness than code embedded in the
kernel's boot process. It shouldn't be any more complex than the kernel
version.
It's also much easier to change/customize it for the end
system's requirements rather than setting the process in stone by
putting it inside the kernel.
> 
> > Heck, I do that sort of set up in shell scripts all the time. Running the shell
> > script as PID 1 and then having it exec the "real init" binary at the end:
> > 
> > https://github.com/landley/mkroot/blob/83def3cbae21/mkroot.sh#L205
> > 
> > If your first init binary is in the initramfs statically linked into the kernel
> > image, and the cpio code is doing open(O_EXCL), then it's as verified as any
> > other kernel code and runs "securely" until it decides to run something else.
> > 
> >> Also, requiring an embedded initramfs for xattrs would be an issue for
> >> systems that use it for other purposes.
> > 
> > I'm the guy who wrote the initmpfs code. (And has pending patches to improve it
> > that will probably never go upstream because I'm a hobbyist and dealing with the
> >   linux-kernel clique is the opposite of fun. I'm only in this conversation
> > because I was cc'd.)
> > 
> > You can totally use initramfs for lots of purposes simultaneously.
> 
> Yes, I agree. However, adding an initramfs to initialize another
> initramfs when you can simply extract file content and metadata with the
> same parser, this for me it is difficult to justify.
> 
It's not really the same parser, right? It's just code that runs after
the parser is done, and the question is whether that code needs to live
inside the kernel or can be done in userspace.
> 
> >>>> The only reason why
> >>>> opening .xattr-list works is that IMA is not yet initialized
> >>>> (late_initcall vs rootfs_initcall).
> >>>
> >>> Launching init before enabling ima is bad because... you didn't think of it?
> >>
> >> No, because /init can potentially compromise the integrity of the
> >> system.
> > 
> > Which isn't a problem if it was statically linked in the kernel, or if your
> > external cpio.gz was signed. You want a signed binary but don't want the
> > signature _in_ the binary...
> 
> It is not just for binaries. How you would deal with arbitrary file
> formats?
> 
> 
> >>>> Allowing a kernel with integrity enforcement to parse the CPIO image
> >>>> without verifying it first is the weak point.
> >>>
> >>> If you don't verify the CPIO image then in theory it could have anything in it,
> >>> yes. You seem to believe that signing individual files is more secure than
> >>> signing the archive. This is certainly a point of view.
> >>
> >> As I wrote above, signing the CPIO image would be more secure, if this
> >> option is available. However, a disadvantage would be that you have to
> >> sign the CPIO image every time a file changes.
> > 
> > Which is why there's a cpio in the kernel and an external cpio loaded via the
> > old initrd mechanism and BOTH files wind up in the cpio and there's a way to
> > make it O_EXCL so it can't overwrite, and then the /init binary inside the
> > kernel's cpio can do any other weird verification you need to do before anything
> > else gets a chance to run so why are you having ring 0 kernel code read a file
> > out of the filesystem and act upon it?
> 
> The CPIO parser already invokes many system calls.
> 
> 
> > (Heck, you can mv /newinit /init before the exec /init so the file isn't on the
> > system anymore by the time the other stuff gets to run...)
> > 
> >>>> However, extracted files
> >>>> are not used, and before they are used they are verified. At the time
> >>>> they are verified, they (included /init) must already have a signature
> >>>> or otherwise access would be denied.
> >>>
> >>> You build infrastructure that works a certain way, the rest of the system
> >>> doesn't fit your assumptions, so you need to change the rest of the system to
> >>> fit your assumptions.
> >>
> >> Requiring file metadata to make decisions seems reasonable. Also
> >> mandatory access controls do that. The objective of this patch set is to
> >> have uniform behavior regardless of the filesystem used.
> > 
> > If it's in the file's contents you get uniform behavior regardless of the
> > filesystem used. And "mandatory access controls do that" is basically restating
> > what _I_ said in the paragraph above.
> 
> As I said, that does not work with arbitrary file formats.
> 
> 
> > The "infrastructure you have that works a certain way" is called "mandatory
> > access controls". Good to know. Your patch changes the rest of the system to
> > match the assumptions of the new code, because changing those assumptions
> > appears literally unthinkable.
> 
> All I want to do is to have the same behavior as if there is no initial
> ram disk. And given that inode-based MACs read the labels from xattrs,
> the assumption that the system provides xattrs even in the inital ram
> disk seems reasonable.
> 
> 
> >>>> This scheme relies on the ability of the kernel to not be corrupted in
> >>>> the event it parses a malformed CPIO image.
> >>>
> >>> I'm unaware of any buffer overruns or wild pointer traversals in the cpio
> >>> extraction code. You can fill up all physical memory with initramfs and lock the
> >>> system hard, though.
> >>>
> >>> It still only parses them at boot time before launching PID 1, right? So you
> >>> have a local physical exploit and you're trying to prevent people from working
> >>> around your Xbox copy protection without a mod chip?
> >>
> >> What do you mean exactly?
> > 
> > That you're not remotely the first person to do this?
> > 
> > You're attempting to prevent anyone from running third party code on your system
> > without buying a license from you first. You're creating a system with no user
> > serviceable parts, that only runs authorized software from the Apple Store or
> > other walled garden. No sideloading allowed.
> 
> This is one use case. The main purpose of IMA is to preserve the
> integrity of the Trusted Computing Base (TCB, the critical part of the
> system), or to detect integrity violations without enforcement. This is
> done by ensuring that the software comes from the vendor. Applications
> owned by users are allowed to run, as the Discrectionary Access Control
> (DAC) prevents attacks to the TCB. I'm working on a more advanced scheme
> that relies on MAC.
> 
> 
> > Which is your choice, sure. But why do you need new infrastructure to do it?
> > People have already _done_ this. They're just by nature proprietary and don't
> > like sharing with the group when not forced by lawyers, so they come up with
> > ways that don't involve modifying GPLv2 software (or shipping GPLv3 software,
> > ever, for any reason).
> > 
> >>>> Mimi suggested to use
> >>>> digital signatures to prevent this issue, but it cannot be used in all
> >>>> scenarios, since conventional systems generate the initial ram disk
> >>>> locally.
> >>>
> >>> So you use a proprietary init binary you can't rebuild from source, and put it
> >>> in a cpio where /dev/urandom is a file with known contents? Clearly, not
> >>> exploitable at all. (And we update the initramfs.cpio but not the kernel because
> >>> clearly keeping the kernel up to date is less important to security...)
> >>
> >> By signing the CPIO image, the kernel wouldn't even attempt to parse it,
> >> as the image would be rejected by the boot loader if the signature is
> >> invalid.
> > 
> > So you have _more_ assumptions tripping you up. Great. So add a signature in a
> > format your bootloader doesn't recognize, since it's the kernel that should
> > verify it, not your bootloader?
> > 
> > It sounds like your problem is bureaucratic, not technical.
> 
> The boot loader verifies the CPIO image, when this is possible. The
> kernel verifies individual files when the CPIO image is not signed.
> 
> If a remote verifier wants to verify the measurement of the CPIO image,
> and he only has reference digests for each file, he has to build the
> CPIO image with files reference digests were calculated from, and in the
> same way it was done by the system target of the evaluation.
> 
> 
> >>> Whatever happened to https://lwn.net/Articles/532778/ ? Modules are signed
> >>> in-band in the file, but you need xattrs for some reason?
> >>
> >> Appending just the signature would be possible. It won't work if you
> >> have multiple metadata for the same file.
> > 
> > Call the elf sections SIG1 SIG2 SIG3, or have a section full of keyword=value
> > strings? How is this a hard problem?
> > 
> >> Also appending the signature alone won't solve the parsing issue. Still,
> >> the kernel has to parse something that could be malformed.
> > 
> > Your new in-band signaling file you're making xattrs from could be malformed,
> > one of the xattrs you add could be "banana=aaaaaaaaaaaaaaaaaaaaaaaaaaa..." going
> > on for 12 megabytes...
> 
> ksys_lsetxattr() checks the limits.
> 
> Roberto
> 
> 
> > Rob
> > 
> 
> -- 
> HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
> Managing Director: Bo PENG, Jian LI, Yanli SHI

Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk

[Arvind Sankar]

On Tue, May 14, 2019 at 11:27:04AM -0400, Arvind Sankar wrote:
> It's also much easier to change/customize it for the end
> system's requirements rather than setting the process in stone by
> putting it inside the kernel.

As an example, if you allow unverified external initramfs, it seems to
me that it can try to play games that wouldn't be prevented by the
in-kernel code: setup /dev in a weird way to try to trick /init, or more
easily, replace /init by /bin/sh so you get a shell prompt while only
the initramfs is loaded. It's easy to imagine that a system would want
to lock itself down to prevent abuses like this.
So you might already want an embedded initramfs that can be trusted and
that can't be overwritten by an external one even outside the
security.ima stuff.

Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk

[Roberto Sassu]

On 5/14/2019 5:19 PM, Andy Lutomirski wrote:
> On Mon, May 13, 2019 at 5:47 AM Roberto Sassu <roberto.sassu@huawei.com> wrote:
>>
>> On 5/13/2019 11:07 AM, Rob Landley wrote:
>>>
>>>
>>> On 5/13/19 2:49 AM, Roberto Sassu wrote:
>>>> On 5/12/2019 9:43 PM, Arvind Sankar wrote:
>>>>> On Sun, May 12, 2019 at 05:05:48PM +0000, Rob Landley wrote:
>>>>>> On 5/12/19 7:52 AM, Mimi Zohar wrote:
>>>>>>> On Sun, 2019-05-12 at 11:17 +0200, Dominik Brodowski wrote:
>>>>>>>> On Thu, May 09, 2019 at 01:24:17PM +0200, Roberto Sassu wrote:
>>>>>>>>> This proposal consists in marshaling pathnames and xattrs in a file called
>>>>>>>>> .xattr-list. They are unmarshaled by the CPIO parser after all files have
>>>>>>>>> been extracted.
>>>>>>>>
>>>>>>>> Couldn't this parsing of the .xattr-list file and the setting of the xattrs
>>>>>>>> be done equivalently by the initramfs' /init? Why is kernel involvement
>>>>>>>> actually required here?
>>>>>>>
>>>>>>> It's too late.  The /init itself should be signed and verified.
>>>>>>
>>>>>> If the initramfs cpio.gz image was signed and verified by the extractor, how is
>>>>>> the init in it _not_ verified?
>>>>>>
>>>>>> Ro
>>>>>
>>>>> Wouldn't the below work even before enforcing signatures on external
>>>>> initramfs:
>>>>> 1. Create an embedded initramfs with an /init that does the xattr
>>>>> parsing/setting. This will be verified as part of the kernel image
>>>>> signature, so no new code required.
>>>>> 2. Add a config option/boot parameter to panic the kernel if an external
>>>>> initramfs attempts to overwrite anything in the embedded initramfs. This
>>>>> prevents overwriting the embedded /init even if the external initramfs
>>>>> is unverified.
>>>>
>>>> Unfortunately, it wouldn't work. IMA is already initialized and it would
>>>> verify /init in the embedded initial ram disk.
>>>
>>> So you made broken infrastructure that's causing you problems. Sounds unfortunate.
>>
>> The idea is to be able to verify anything that is accessed, as soon as
>> rootfs is available, without distinction between embedded or external
>> initial ram disk.
>>
>> Also, requiring an embedded initramfs for xattrs would be an issue for
>> systems that use it for other purposes.
>>
>>
>>>> The only reason why
>>>> opening .xattr-list works is that IMA is not yet initialized
>>>> (late_initcall vs rootfs_initcall).
>>>
>>> Launching init before enabling ima is bad because... you didn't think of it?
>>
>> No, because /init can potentially compromise the integrity of the
>> system.
> 
> I think Rob is right here.  If /init was statically built into the
> kernel image, it has no more ability to compromise the kernel than
> anything else in the kernel.  What's the problem here?

Right, the measurement/signature verification of the kernel image is
sufficient.

Now, assuming that we defer the IMA initialization until /init in the
embedded initramfs has been executed, the problem is how to handle
processes launched with the user mode helper or files directly read by
the kernel (if it can happen before /init is executed). If IMA is not
yet enabled, these operations will be performed without measurement and
signature verification.

Roberto

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

Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk

[Greg KH]

On Tue, May 14, 2019 at 06:33:29PM +0200, Roberto Sassu wrote:
> On 5/14/2019 5:19 PM, Andy Lutomirski wrote:
> > On Mon, May 13, 2019 at 5:47 AM Roberto Sassu <roberto.sassu@huawei.com> wrote:
> > > 
> > > On 5/13/2019 11:07 AM, Rob Landley wrote:
> > > > 
> > > > 
> > > > On 5/13/19 2:49 AM, Roberto Sassu wrote:
> > > > > On 5/12/2019 9:43 PM, Arvind Sankar wrote:
> > > > > > On Sun, May 12, 2019 at 05:05:48PM +0000, Rob Landley wrote:
> > > > > > > On 5/12/19 7:52 AM, Mimi Zohar wrote:
> > > > > > > > On Sun, 2019-05-12 at 11:17 +0200, Dominik Brodowski wrote:
> > > > > > > > > On Thu, May 09, 2019 at 01:24:17PM +0200, Roberto Sassu wrote:
> > > > > > > > > > This proposal consists in marshaling pathnames and xattrs in a file called
> > > > > > > > > > .xattr-list. They are unmarshaled by the CPIO parser after all files have
> > > > > > > > > > been extracted.
> > > > > > > > > 
> > > > > > > > > Couldn't this parsing of the .xattr-list file and the setting of the xattrs
> > > > > > > > > be done equivalently by the initramfs' /init? Why is kernel involvement
> > > > > > > > > actually required here?
> > > > > > > > 
> > > > > > > > It's too late.  The /init itself should be signed and verified.
> > > > > > > 
> > > > > > > If the initramfs cpio.gz image was signed and verified by the extractor, how is
> > > > > > > the init in it _not_ verified?
> > > > > > > 
> > > > > > > Ro
> > > > > > 
> > > > > > Wouldn't the below work even before enforcing signatures on external
> > > > > > initramfs:
> > > > > > 1. Create an embedded initramfs with an /init that does the xattr
> > > > > > parsing/setting. This will be verified as part of the kernel image
> > > > > > signature, so no new code required.
> > > > > > 2. Add a config option/boot parameter to panic the kernel if an external
> > > > > > initramfs attempts to overwrite anything in the embedded initramfs. This
> > > > > > prevents overwriting the embedded /init even if the external initramfs
> > > > > > is unverified.
> > > > > 
> > > > > Unfortunately, it wouldn't work. IMA is already initialized and it would
> > > > > verify /init in the embedded initial ram disk.
> > > > 
> > > > So you made broken infrastructure that's causing you problems. Sounds unfortunate.
> > > 
> > > The idea is to be able to verify anything that is accessed, as soon as
> > > rootfs is available, without distinction between embedded or external
> > > initial ram disk.
> > > 
> > > Also, requiring an embedded initramfs for xattrs would be an issue for
> > > systems that use it for other purposes.
> > > 
> > > 
> > > > > The only reason why
> > > > > opening .xattr-list works is that IMA is not yet initialized
> > > > > (late_initcall vs rootfs_initcall).
> > > > 
> > > > Launching init before enabling ima is bad because... you didn't think of it?
> > > 
> > > No, because /init can potentially compromise the integrity of the
> > > system.
> > 
> > I think Rob is right here.  If /init was statically built into the
> > kernel image, it has no more ability to compromise the kernel than
> > anything else in the kernel.  What's the problem here?
> 
> Right, the measurement/signature verification of the kernel image is
> sufficient.
> 
> Now, assuming that we defer the IMA initialization until /init in the
> embedded initramfs has been executed, the problem is how to handle
> processes launched with the user mode helper or files directly read by
> the kernel (if it can happen before /init is executed). If IMA is not
> yet enabled, these operations will be performed without measurement and
> signature verification.

If you really care about this, don't launch any user mode helper
programs (hint, you have the kernel option to control this and funnel
everything into one, or no, binaries).  And don't allow the kernel to
read any files either, again, you have control over this.

Or start IMA earlier if you need/want/care about this.

thanks,

greg k-h

Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk

[Roberto Sassu]

On 5/14/2019 6:58 PM, Greg KH wrote:
> On Tue, May 14, 2019 at 06:33:29PM +0200, Roberto Sassu wrote:
>> On 5/14/2019 5:19 PM, Andy Lutomirski wrote:
>>> On Mon, May 13, 2019 at 5:47 AM Roberto Sassu <roberto.sassu@huawei.com> wrote:
>>>>
>>>> On 5/13/2019 11:07 AM, Rob Landley wrote:
>>>>>
>>>>>
>>>>> On 5/13/19 2:49 AM, Roberto Sassu wrote:
>>>>>> On 5/12/2019 9:43 PM, Arvind Sankar wrote:
>>>>>>> On Sun, May 12, 2019 at 05:05:48PM +0000, Rob Landley wrote:
>>>>>>>> On 5/12/19 7:52 AM, Mimi Zohar wrote:
>>>>>>>>> On Sun, 2019-05-12 at 11:17 +0200, Dominik Brodowski wrote:
>>>>>>>>>> On Thu, May 09, 2019 at 01:24:17PM +0200, Roberto Sassu wrote:
>>>>>>>>>>> This proposal consists in marshaling pathnames and xattrs in a file called
>>>>>>>>>>> .xattr-list. They are unmarshaled by the CPIO parser after all files have
>>>>>>>>>>> been extracted.
>>>>>>>>>>
>>>>>>>>>> Couldn't this parsing of the .xattr-list file and the setting of the xattrs
>>>>>>>>>> be done equivalently by the initramfs' /init? Why is kernel involvement
>>>>>>>>>> actually required here?
>>>>>>>>>
>>>>>>>>> It's too late.  The /init itself should be signed and verified.
>>>>>>>>
>>>>>>>> If the initramfs cpio.gz image was signed and verified by the extractor, how is
>>>>>>>> the init in it _not_ verified?
>>>>>>>>
>>>>>>>> Ro
>>>>>>>
>>>>>>> Wouldn't the below work even before enforcing signatures on external
>>>>>>> initramfs:
>>>>>>> 1. Create an embedded initramfs with an /init that does the xattr
>>>>>>> parsing/setting. This will be verified as part of the kernel image
>>>>>>> signature, so no new code required.
>>>>>>> 2. Add a config option/boot parameter to panic the kernel if an external
>>>>>>> initramfs attempts to overwrite anything in the embedded initramfs. This
>>>>>>> prevents overwriting the embedded /init even if the external initramfs
>>>>>>> is unverified.
>>>>>>
>>>>>> Unfortunately, it wouldn't work. IMA is already initialized and it would
>>>>>> verify /init in the embedded initial ram disk.
>>>>>
>>>>> So you made broken infrastructure that's causing you problems. Sounds unfortunate.
>>>>
>>>> The idea is to be able to verify anything that is accessed, as soon as
>>>> rootfs is available, without distinction between embedded or external
>>>> initial ram disk.
>>>>
>>>> Also, requiring an embedded initramfs for xattrs would be an issue for
>>>> systems that use it for other purposes.
>>>>
>>>>
>>>>>> The only reason why
>>>>>> opening .xattr-list works is that IMA is not yet initialized
>>>>>> (late_initcall vs rootfs_initcall).
>>>>>
>>>>> Launching init before enabling ima is bad because... you didn't think of it?
>>>>
>>>> No, because /init can potentially compromise the integrity of the
>>>> system.
>>>
>>> I think Rob is right here.  If /init was statically built into the
>>> kernel image, it has no more ability to compromise the kernel than
>>> anything else in the kernel.  What's the problem here?
>>
>> Right, the measurement/signature verification of the kernel image is
>> sufficient.
>>
>> Now, assuming that we defer the IMA initialization until /init in the
>> embedded initramfs has been executed, the problem is how to handle
>> processes launched with the user mode helper or files directly read by
>> the kernel (if it can happen before /init is executed). If IMA is not
>> yet enabled, these operations will be performed without measurement and
>> signature verification.
> 
> If you really care about this, don't launch any user mode helper
> programs (hint, you have the kernel option to control this and funnel
> everything into one, or no, binaries).  And don't allow the kernel to
> read any files either, again, you have control over this.
> 
> Or start IMA earlier if you need/want/care about this.

Yes, this is how it works now. It couldn't start earlier than
late_initcall, as it has to wait until the TPM driver is initialized.

Anyway, it is enabled at the time /init is executed. And this would be
an issue because launching /init and reading xattrs from /.xattr-list
would be denied (the signature is missing).

And /.xattr-list won't have a signature, if initramfs is generated
locally.

Roberto

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

Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk

[Roberto Sassu]

On 5/14/2019 5:57 PM, Arvind Sankar wrote:
> On Tue, May 14, 2019 at 11:27:04AM -0400, Arvind Sankar wrote:
>> It's also much easier to change/customize it for the end
>> system's requirements rather than setting the process in stone by
>> putting it inside the kernel.
> 
> As an example, if you allow unverified external initramfs, it seems to
> me that it can try to play games that wouldn't be prevented by the
> in-kernel code: setup /dev in a weird way to try to trick /init, or more
> easily, replace /init by /bin/sh so you get a shell prompt while only
> the initramfs is loaded. It's easy to imagine that a system would want
> to lock itself down to prevent abuses like this.

Yes, these issues should be addressed. But the purpose of this patch set
is simply to set xattrs. And existing protection mechanisms can be
improved later when the basic functionality is there.


> So you might already want an embedded initramfs that can be trusted and
> that can't be overwritten by an external one even outside the
> security.ima stuff.

The same problems exist also the root filesystem. These should be solved
regardless of the filesystem used, for remote attestation and for local
enforcement.

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

Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk

[James Bottomley]

On Tue, 2019-05-14 at 08:19 -0700, Andy Lutomirski wrote:
> On Mon, May 13, 2019 at 5:47 AM Roberto Sassu <roberto.sassu@huawei.c
> om> wrote:
> > On 5/13/2019 11:07 AM, Rob Landley wrote:
[...]
> > > > The only reason why opening .xattr-list works is that IMA is
> > > > not yet initialized (late_initcall vs rootfs_initcall).
> > > 
> > > Launching init before enabling ima is bad because... you didn't
> > > think of it?
> > 
> > No, because /init can potentially compromise the integrity of the
> > system.
> 
> I think Rob is right here.  If /init was statically built into the
> kernel image, it has no more ability to compromise the kernel than
> anything else in the kernel.  What's the problem here?

The specific problem is that unless you own the kernel signing key,
which is really untrue for most distribution consumers because the
distro owns the key, you cannot build the initrd statically into the
kernel.  You can take the distro signed kernel, link it with the initrd
then resign the combination with your key, provided you insert your key
into the MoK variables as a trusted secure boot key, but the distros
have been unhappy recommending this as standard practice.

If our model for security is going to be to link the kernel and the
initrd statically to give signature protection over the aggregate then
we need to figure out how to execute this via the distros.  If we
accept that the split model, where the distro owns and signs the kernel
but the machine owner builds and is responsible for the initrd, then we
need to explore split security models like this proposal.

James

Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk

[Rob Landley]

On 5/14/19 2:18 PM, James Bottomley wrote:
>> I think Rob is right here.  If /init was statically built into the
>> kernel image, it has no more ability to compromise the kernel than
>> anything else in the kernel.  What's the problem here?
> 
> The specific problem is that unless you own the kernel signing key,
> which is really untrue for most distribution consumers because the
> distro owns the key, you cannot build the initrd statically into the
> kernel.  You can take the distro signed kernel, link it with the initrd
> then resign the combination with your key, provided you insert your key
> into the MoK variables as a trusted secure boot key, but the distros
> have been unhappy recommending this as standard practice.
> 
> If our model for security is going to be to link the kernel and the
> initrd statically to give signature protection over the aggregate then
> we need to figure out how to execute this via the distros.  If we
> accept that the split model, where the distro owns and signs the kernel
> but the machine owner builds and is responsible for the initrd, then we
> need to explore split security models like this proposal.

You can have a built-in and an external initrd? The second extracts over the
first? (I know because once upon a time conflicting files would append. It
sounds like the desired behavior here is O_EXCL fail and move on.)

Rob

Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk

[James Bottomley]

On Tue, 2019-05-14 at 18:39 -0500, Rob Landley wrote:
> On 5/14/19 2:18 PM, James Bottomley wrote:
> > > I think Rob is right here.  If /init was statically built into
> > > the kernel image, it has no more ability to compromise the kernel
> > > than anything else in the kernel.  What's the problem here?
> > 
> > The specific problem is that unless you own the kernel signing key,
> > which is really untrue for most distribution consumers because the
> > distro owns the key, you cannot build the initrd statically into
> > the kernel.  You can take the distro signed kernel, link it with
> > the initrd then resign the combination with your key, provided you
> > insert your key into the MoK variables as a trusted secure boot
> > key, but the distros have been unhappy recommending this as
> > standard practice.
> > 
> > If our model for security is going to be to link the kernel and the
> > initrd statically to give signature protection over the aggregate
> > then we need to figure out how to execute this via the distros.  If
> > we accept that the split model, where the distro owns and signs the
> > kernel but the machine owner builds and is responsible for the
> > initrd, then we need to explore split security models like this
> > proposal.
> 
> You can have a built-in and an external initrd? The second extracts
> over the first? (I know because once upon a time conflicting files
> would append. It sounds like the desired behavior here is O_EXCL fail
> and move on.)

Technically yes, because the first initrd could find the second by some
predefined means, extract it to a temporary directory and do a
pivot_root() and then the second would do some stuff, find the real
root and do a pivot_root() again.  However, while possible, wouldn't it
just add to the rendezvous complexity without adding any benefits? even
if the first initrd is built and signed by the distro and the second is
built by you, the first has to verify the second somehow.  I suppose
the second could be tar extracted, which would add xattrs, if that's
the goal?

James

Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk

[Arvind Sankar]

On Tue, May 14, 2019 at 07:20:15PM +0200, Roberto Sassu wrote:
> On 5/14/2019 6:58 PM, Greg KH wrote:
> > On Tue, May 14, 2019 at 06:33:29PM +0200, Roberto Sassu wrote:
> >> Right, the measurement/signature verification of the kernel image is
> >> sufficient.
> >>
> >> Now, assuming that we defer the IMA initialization until /init in the
> >> embedded initramfs has been executed, the problem is how to handle
> >> processes launched with the user mode helper or files directly read by
> >> the kernel (if it can happen before /init is executed). If IMA is not
> >> yet enabled, these operations will be performed without measurement and
> >> signature verification.
> > 
> > If you really care about this, don't launch any user mode helper
> > programs (hint, you have the kernel option to control this and funnel
> > everything into one, or no, binaries).  And don't allow the kernel to
> > read any files either, again, you have control over this.
> > 
> > Or start IMA earlier if you need/want/care about this.
> 
> Yes, this is how it works now. It couldn't start earlier than
> late_initcall, as it has to wait until the TPM driver is initialized.
> 
> Anyway, it is enabled at the time /init is executed. And this would be
> an issue because launching /init and reading xattrs from /.xattr-list
> would be denied (the signature is missing).
> 
> And /.xattr-list won't have a signature, if initramfs is generated
> locally.
> 
> Roberto
> 
> -- 
> HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
> Managing Director: Bo PENG, Jian LI, Yanli SHI

The uevent and firmware loader user mode helpers are both obsolete I
believe, so those shouldn't be an issue.

There is still the internal firmware loader (CONFIG_FW_LOADER). If this
is built-in, there's probably no way to 100% stop it racing with /init if we
depend on an embedded /init and a malicious external initramfs image
contains /lib/firmware, but it can be built as an external module, in
which case there should be no danger until the boot process actually loads it.

Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk

[Arvind Sankar]

On Tue, May 14, 2019 at 04:54:12PM -0700, James Bottomley wrote:
> On Tue, 2019-05-14 at 18:39 -0500, Rob Landley wrote:
> > On 5/14/19 2:18 PM, James Bottomley wrote:
> > > > I think Rob is right here.  If /init was statically built into
> > > > the kernel image, it has no more ability to compromise the kernel
> > > > than anything else in the kernel.  What's the problem here?
> > > 
> > > The specific problem is that unless you own the kernel signing key,
> > > which is really untrue for most distribution consumers because the
> > > distro owns the key, you cannot build the initrd statically into
> > > the kernel.  You can take the distro signed kernel, link it with
> > > the initrd then resign the combination with your key, provided you
> > > insert your key into the MoK variables as a trusted secure boot
> > > key, but the distros have been unhappy recommending this as
> > > standard practice.
> > > 
> > > If our model for security is going to be to link the kernel and the
> > > initrd statically to give signature protection over the aggregate
> > > then we need to figure out how to execute this via the distros.  If
> > > we accept that the split model, where the distro owns and signs the
> > > kernel but the machine owner builds and is responsible for the
> > > initrd, then we need to explore split security models like this
> > > proposal.
> > 
> > You can have a built-in and an external initrd? The second extracts
> > over the first? (I know because once upon a time conflicting files
> > would append. It sounds like the desired behavior here is O_EXCL fail
> > and move on.)
> 
> Technically yes, because the first initrd could find the second by some
> predefined means, extract it to a temporary directory and do a
> pivot_root() and then the second would do some stuff, find the real
> root and do a pivot_root() again.  However, while possible, wouldn't it
> just add to the rendezvous complexity without adding any benefits? even
> if the first initrd is built and signed by the distro and the second is
> built by you, the first has to verify the second somehow.  I suppose
> the second could be tar extracted, which would add xattrs, if that's
> the goal?
> 
> James
> 
You can specify multiple initrd's to the boot loader, and they get
loaded in sequence into memory and parsed by the kernel before /init is
launched. Currently I believe later ones will overwrite the earlier
ones, which is why we've been talking about adding an option to prevent
that. You don't have to mess with manually finding/parsing initramfs's
which wouldn't even be feasible since you may not have the drivers
loaded yet to access the device/filesystem on which they live.

Once that's done, the embedded /init is just going to do in userspace
wht the current patch does in the kernel. So all the files in the
external initramfs(es) would need to have IMA signatures via the special
xattr file.

Note that if you want the flexibility to be able to load one or both of
two external initramfs's, the current in-kernel proposal wouldn't be
enough -- the xattr specification would have to be more flexible (eg
reading .xattr-list* to allow each initramfs to specifiy its own
xattrs. This sort of enhancement would be much easier to handle with the
userspace variant.

Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk

[Arvind Sankar]

On Tue, May 14, 2019 at 07:44:42PM +0200, Roberto Sassu wrote:
> On 5/14/2019 5:57 PM, Arvind Sankar wrote:
> > On Tue, May 14, 2019 at 11:27:04AM -0400, Arvind Sankar wrote:
> >> It's also much easier to change/customize it for the end
> >> system's requirements rather than setting the process in stone by
> >> putting it inside the kernel.
> > 
> > As an example, if you allow unverified external initramfs, it seems to
> > me that it can try to play games that wouldn't be prevented by the
> > in-kernel code: setup /dev in a weird way to try to trick /init, or more
> > easily, replace /init by /bin/sh so you get a shell prompt while only
> > the initramfs is loaded. It's easy to imagine that a system would want
> > to lock itself down to prevent abuses like this.
> 
> Yes, these issues should be addressed. But the purpose of this patch set
> is simply to set xattrs. And existing protection mechanisms can be
> improved later when the basic functionality is there.
> 
Yeah but it's much easier to enhance it when it lives in userspace and
can be tailored to a particular system's requirements. Eg a lot of the
issues will disappear if you don't have to allow for external initramfs
at all, so those systems can have a very simple embedded /init that
doesn't have to do much.
> 
> > So you might already want an embedded initramfs that can be trusted and
> > that can't be overwritten by an external one even outside the
> > security.ima stuff.
> 
> The same problems exist also the root filesystem. These should be solved
> regardless of the filesystem used, for remote attestation and for local
> enforcement.
> 
> -- 
> HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
> Managing Director: Bo PENG, Jian LI, Yanli SHI

Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk

[Roberto Sassu]

On 5/15/2019 2:52 AM, Arvind Sankar wrote:
> On Tue, May 14, 2019 at 04:54:12PM -0700, James Bottomley wrote:
>> On Tue, 2019-05-14 at 18:39 -0500, Rob Landley wrote:
>>> On 5/14/19 2:18 PM, James Bottomley wrote:
>>>>> I think Rob is right here.  If /init was statically built into
>>>>> the kernel image, it has no more ability to compromise the kernel
>>>>> than anything else in the kernel.  What's the problem here?
>>>>
>>>> The specific problem is that unless you own the kernel signing key,
>>>> which is really untrue for most distribution consumers because the
>>>> distro owns the key, you cannot build the initrd statically into
>>>> the kernel.  You can take the distro signed kernel, link it with
>>>> the initrd then resign the combination with your key, provided you
>>>> insert your key into the MoK variables as a trusted secure boot
>>>> key, but the distros have been unhappy recommending this as
>>>> standard practice.
>>>>
>>>> If our model for security is going to be to link the kernel and the
>>>> initrd statically to give signature protection over the aggregate
>>>> then we need to figure out how to execute this via the distros.  If
>>>> we accept that the split model, where the distro owns and signs the
>>>> kernel but the machine owner builds and is responsible for the
>>>> initrd, then we need to explore split security models like this
>>>> proposal.
>>>
>>> You can have a built-in and an external initrd? The second extracts
>>> over the first? (I know because once upon a time conflicting files
>>> would append. It sounds like the desired behavior here is O_EXCL fail
>>> and move on.)
>>
>> Technically yes, because the first initrd could find the second by some
>> predefined means, extract it to a temporary directory and do a
>> pivot_root() and then the second would do some stuff, find the real
>> root and do a pivot_root() again.  However, while possible, wouldn't it
>> just add to the rendezvous complexity without adding any benefits? even
>> if the first initrd is built and signed by the distro and the second is
>> built by you, the first has to verify the second somehow.  I suppose
>> the second could be tar extracted, which would add xattrs, if that's
>> the goal?
>>
>> James
>>
> You can specify multiple initrd's to the boot loader, and they get
> loaded in sequence into memory and parsed by the kernel before /init is
> launched. Currently I believe later ones will overwrite the earlier
> ones, which is why we've been talking about adding an option to prevent
> that. You don't have to mess with manually finding/parsing initramfs's
> which wouldn't even be feasible since you may not have the drivers
> loaded yet to access the device/filesystem on which they live.
> 
> Once that's done, the embedded /init is just going to do in userspace
> wht the current patch does in the kernel. So all the files in the
> external initramfs(es) would need to have IMA signatures via the special
> xattr file.

So, the scheme you are proposing is not equivalent: using the distro key
to verify signatures, compared to adding a new user key to verify the
initramfs he builds. Why would it be necessary for the user to share
responsibility with the distro, if the only files he uses come from the
distro?


> Note that if you want the flexibility to be able to load one or both of
> two external initramfs's, the current in-kernel proposal wouldn't be
> enough -- the xattr specification would have to be more flexible (eg
> reading .xattr-list* to allow each initramfs to specifiy its own
> xattrs. This sort of enhancement would be much easier to handle with the
> userspace variant.

Yes, the alternative solution is to parse .xattr-list at the time it is
extracted. The .xattr-list of each initramfs will be processed. Also,
the CPIO parser doesn't have to reopen the file after all other files
have been extracted.

Roberto

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

Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk

[Arvind Sankar]

On Wed, May 15, 2019 at 01:19:04PM +0200, Roberto Sassu wrote:
> On 5/15/2019 2:52 AM, Arvind Sankar wrote:
> > You can specify multiple initrd's to the boot loader, and they get
> > loaded in sequence into memory and parsed by the kernel before /init is
> > launched. Currently I believe later ones will overwrite the earlier
> > ones, which is why we've been talking about adding an option to prevent
> > that. You don't have to mess with manually finding/parsing initramfs's
> > which wouldn't even be feasible since you may not have the drivers
> > loaded yet to access the device/filesystem on which they live.
> > 
> > Once that's done, the embedded /init is just going to do in userspace
> > wht the current patch does in the kernel. So all the files in the
> > external initramfs(es) would need to have IMA signatures via the special
> > xattr file.
> 
> So, the scheme you are proposing is not equivalent: using the distro key
> to verify signatures, compared to adding a new user key to verify the
> initramfs he builds. Why would it be necessary for the user to share
> responsibility with the distro, if the only files he uses come from the
> distro?
> 
I don't understand what you mean? The IMA hashes are signed by some key,
but I don't see how what that key is needs to be different between the
two proposals. If the only files used are from the distro, in my scheme
as well you can use the signatures and key provided by the distro. If
they're not, then in your scheme as well you would have to allow for a
local signing key to be used. Both schemes are using the same
.xattr-list file, no?

If the external initramfs is to be signed, and it is built locally, in
both schemes there will have to be a provision for a local signing key,
but this key in any case is verified by the bootloader so there can't
be a difference between the two schemes since they're the same there.

What is the difference you're seeing here?
> 
> > Note that if you want the flexibility to be able to load one or both of
> > two external initramfs's, the current in-kernel proposal wouldn't be
> > enough -- the xattr specification would have to be more flexible (eg
> > reading .xattr-list* to allow each initramfs to specifiy its own
> > xattrs. This sort of enhancement would be much easier to handle with the
> > userspace variant.
> 
> Yes, the alternative solution is to parse .xattr-list at the time it is
> extracted. The .xattr-list of each initramfs will be processed. Also,
> the CPIO parser doesn't have to reopen the file after all other files
> have been extracted.
> 
> Roberto
Right, I guess this would be sort of the minimal "modification" to the
CPIO format to allow it to support xattrs.
> 
> -- 
> HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
> Managing Director: Bo PENG, Jian LI, Yanli SHI

Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk

[Roberto Sassu]

On 5/15/2019 6:08 PM, Arvind Sankar wrote:
> On Wed, May 15, 2019 at 01:19:04PM +0200, Roberto Sassu wrote:
>> On 5/15/2019 2:52 AM, Arvind Sankar wrote:
>>> You can specify multiple initrd's to the boot loader, and they get
>>> loaded in sequence into memory and parsed by the kernel before /init is
>>> launched. Currently I believe later ones will overwrite the earlier
>>> ones, which is why we've been talking about adding an option to prevent
>>> that. You don't have to mess with manually finding/parsing initramfs's
>>> which wouldn't even be feasible since you may not have the drivers
>>> loaded yet to access the device/filesystem on which they live.
>>>
>>> Once that's done, the embedded /init is just going to do in userspace
>>> wht the current patch does in the kernel. So all the files in the
>>> external initramfs(es) would need to have IMA signatures via the special
>>> xattr file.
>>
>> So, the scheme you are proposing is not equivalent: using the distro key
>> to verify signatures, compared to adding a new user key to verify the
>> initramfs he builds. Why would it be necessary for the user to share
>> responsibility with the distro, if the only files he uses come from the
>> distro?
>>
> I don't understand what you mean? The IMA hashes are signed by some key,
> but I don't see how what that key is needs to be different between the
> two proposals. If the only files used are from the distro, in my scheme
> as well you can use the signatures and key provided by the distro. If
> they're not, then in your scheme as well you would have to allow for a
> local signing key to be used. Both schemes are using the same
> .xattr-list file, no?

I was referring to James's proposal to load an external initramfs from
the embedded initramfs. If the embedded initramfs opens the external
initramfs when IMA is enabled, the external initramfs needs to be
signed with a local signing key. But I read your answer that this
wouldn't be feasible. You have to specify all initramfs in the boot
loader configuration.

I think deferring IMA initialization is not the safest approach, as it
cannot be guaranteed for all possible scenarios that there won't be any
file read before /init is executed.

But if IMA is enabled, there is the problem of who signs .xattr-list.
There should be a local signing key that it is not necessary if the user
only accesses distro files.


> If the external initramfs is to be signed, and it is built locally, in
> both schemes there will have to be a provision for a local signing key,
> but this key in any case is verified by the bootloader so there can't
> be a difference between the two schemes since they're the same there.
> 
> What is the difference you're seeing here?
>>
>>> Note that if you want the flexibility to be able to load one or both of
>>> two external initramfs's, the current in-kernel proposal wouldn't be
>>> enough -- the xattr specification would have to be more flexible (eg
>>> reading .xattr-list* to allow each initramfs to specifiy its own
>>> xattrs. This sort of enhancement would be much easier to handle with the
>>> userspace variant.
>>
>> Yes, the alternative solution is to parse .xattr-list at the time it is
>> extracted. The .xattr-list of each initramfs will be processed. Also,
>> the CPIO parser doesn't have to reopen the file after all other files
>> have been extracted.
>>
>> Roberto
> Right, I guess this would be sort of the minimal "modification" to the
> CPIO format to allow it to support xattrs.

I would try to do it without modification of the CPIO format. However,
at the time .xattr-list is parsed (in do_copy() before .xattr-list is
closed), it is not guaranteed that all files are extracted. These must
be created before xattrs are added, but the file type must be correct,
otherwise clean_path() removes the existing file with xattrs.

Roberto

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

Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk

[Arvind Sankar]

On Wed, May 15, 2019 at 07:06:52PM +0200, Roberto Sassu wrote:
> On 5/15/2019 6:08 PM, Arvind Sankar wrote:
> > On Wed, May 15, 2019 at 01:19:04PM +0200, Roberto Sassu wrote:
> >> On 5/15/2019 2:52 AM, Arvind Sankar wrote:
> > I don't understand what you mean? The IMA hashes are signed by some key,
> > but I don't see how what that key is needs to be different between the
> > two proposals. If the only files used are from the distro, in my scheme
> > as well you can use the signatures and key provided by the distro. If
> > they're not, then in your scheme as well you would have to allow for a
> > local signing key to be used. Both schemes are using the same
> > .xattr-list file, no?
> 
> I was referring to James's proposal to load an external initramfs from
> the embedded initramfs. If the embedded initramfs opens the external
> initramfs when IMA is enabled, the external initramfs needs to be
> signed with a local signing key. But I read your answer that this
> wouldn't be feasible. You have to specify all initramfs in the boot
> loader configuration.
> 
> I think deferring IMA initialization is not the safest approach, as it
> cannot be guaranteed for all possible scenarios that there won't be any
> file read before /init is executed.
> 
> But if IMA is enabled, there is the problem of who signs .xattr-list.
> There should be a local signing key that it is not necessary if the user
> only accesses distro files.
> 
I think that's a separate issue. If you want to allow people to be able
to put files onto the system that will be IMA verified, they need to
have some way to locally sign them whether it's inside an initramfs or
on a real root filesystem.
> 
> > Right, I guess this would be sort of the minimal "modification" to the
> > CPIO format to allow it to support xattrs.
> 
> I would try to do it without modification of the CPIO format. However,
> at the time .xattr-list is parsed (in do_copy() before .xattr-list is
> closed), it is not guaranteed that all files are extracted. These must
> be created before xattrs are added, but the file type must be correct,
> otherwise clean_path() removes the existing file with xattrs.
> 
> Roberto
> 
> -- 
> HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
> Managing Director: Bo PENG, Jian LI, Yanli SHI

Right by "modification" in quotes I meant the format is actually the
same, but the kernel now interprets it a bit differently.

Regarding the order you don't have to handle that in the kernel. The
kernel CPIO format is already restricted in that directories have to be
specified before the files that contain them for example. It can very
well be restricted so that an .xattr-list can only specify xattrs for
files that were already extracted, else you bail out with an error. The
archive creation tooling can easily handle that. If someone wants to
shoot themselves in the foot by trying to add more files/replace
existing files after the .xattr-list its ok, the IMA policy will prevent
such files from being accessed and they can fix the archive for the next
boot.

Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk

[Roberto Sassu]

On 5/16/2019 7:29 AM, Arvind Sankar wrote:
> On Wed, May 15, 2019 at 07:06:52PM +0200, Roberto Sassu wrote:
>> On 5/15/2019 6:08 PM, Arvind Sankar wrote:
>>> On Wed, May 15, 2019 at 01:19:04PM +0200, Roberto Sassu wrote:
>>>> On 5/15/2019 2:52 AM, Arvind Sankar wrote:
>>> I don't understand what you mean? The IMA hashes are signed by some key,
>>> but I don't see how what that key is needs to be different between the
>>> two proposals. If the only files used are from the distro, in my scheme
>>> as well you can use the signatures and key provided by the distro. If
>>> they're not, then in your scheme as well you would have to allow for a
>>> local signing key to be used. Both schemes are using the same
>>> .xattr-list file, no?
>>
>> I was referring to James's proposal to load an external initramfs from
>> the embedded initramfs. If the embedded initramfs opens the external
>> initramfs when IMA is enabled, the external initramfs needs to be
>> signed with a local signing key. But I read your answer that this
>> wouldn't be feasible. You have to specify all initramfs in the boot
>> loader configuration.
>>
>> I think deferring IMA initialization is not the safest approach, as it
>> cannot be guaranteed for all possible scenarios that there won't be any
>> file read before /init is executed.
>>
>> But if IMA is enabled, there is the problem of who signs .xattr-list.
>> There should be a local signing key that it is not necessary if the user
>> only accesses distro files.
>>
> I think that's a separate issue. If you want to allow people to be able
> to put files onto the system that will be IMA verified, they need to
> have some way to locally sign them whether it's inside an initramfs or
> on a real root filesystem.

Yes. But this shouldn't be a requirement. If I have only files signed by
the distro, I should be able to do appraisal without a local signing
key.

I made an IMA extension called IMA Digest Lists, that extracts reference
digests from RPM headers and performs appraisal based on the loaded
white lists.  The only keys that must be in the kernel for signature
verification are the PGP keys of the distro (plus the public key for the
RPM parser, which at the moment is different).

.xattr-list is generated by my custom dracut module and contains the
signature of the digest lists and the parser.


>>> Right, I guess this would be sort of the minimal "modification" to the
>>> CPIO format to allow it to support xattrs.
>>
>> I would try to do it without modification of the CPIO format. However,
>> at the time .xattr-list is parsed (in do_copy() before .xattr-list is
>> closed), it is not guaranteed that all files are extracted. These must
>> be created before xattrs are added, but the file type must be correct,
>> otherwise clean_path() removes the existing file with xattrs.
>>
>> Roberto
>>
>> -- 
>> HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
>> Managing Director: Bo PENG, Jian LI, Yanli SHI
> 
> Right by "modification" in quotes I meant the format is actually the
> same, but the kernel now interprets it a bit differently.
> 
> Regarding the order you don't have to handle that in the kernel. The
> kernel CPIO format is already restricted in that directories have to be
> specified before the files that contain them for example. It can very
> well be restricted so that an .xattr-list can only specify xattrs for
> files that were already extracted, else you bail out with an error. The
> archive creation tooling can easily handle that. If someone wants to
> shoot themselves in the foot by trying to add more files/replace
> existing files after the .xattr-list its ok, the IMA policy will prevent
> such files from being accessed and they can fix the archive for the next
> boot.

Unfortunately, dracut sorts the files before adding them to the CPIO
image (.xattr-list is at the beginning). I could move xattrs from the
existing file to the file with different mode, but this makes the code
more complex. I think it is better to call do_readxattrs() after files
are extracted, or when .xattr-list is going to be replaced by another
one in the next initramfs.

Roberto

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

Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk

[Mimi Zohar]

On Thu, 2019-05-16 at 01:29 -0400, Arvind Sankar wrote:

> I think that's a separate issue. If you want to allow people to be able
> to put files onto the system that will be IMA verified, they need to
> have some way to locally sign them whether it's inside an initramfs or
> on a real root filesystem.

Anyone building their own kernel can build their own key into the
kernel image.  Another option is to build the kernel with  
CONFIG_SYSTEM_EXTRA_CERTIFICATE enabled, allowing an additional
certificate to be inserted into the kernel image post build.  The
additional certificate will be loaded onto the builtin kernel keyring.
 Certificates signed with the private key can then be added to the IMA
keyring.  By modifying the kernel image, the kernel image obviously
needs to be resigned.  Additional patches "Certificate insertion
support for x86 bzImages" were posted, but have not been upstreamed.

This patch set adds the security xattrs needed by IMA.

Mimi