[RFC][PATCH 0/9] extend initramfs archive format to support xattrs

[Mimi Zohar]

Many of the Linux security/integrity features are dependent on file
metadata, stored as extended attributes (xattrs), for making decisions.
These features need to be initialized during initcall and enabled as
early as possible for complete security coverage.

The linux kernel creates the rootfs file system and extracts the contents
of the initramfs, a compressed CPIO archive, onto it. If CONFIG_TMPFS is
enabled (and "root=" is not specified on the boot command line), rootfs
will use tmpfs instead of ramfs by default. Although the tmpfs filesystem
supports xattrs, the CPIO archive specification does not define a method
for including them in the archive.  Other archive formats have added xattr
support (eg. tar).

There are a couple of ways to include and label the rootfs filesystem:
- include a file manifest containing the xattrs in the initramfs
- extend CPIO to support xattrs
- add tar support

This patch set extends the existing newc CPIO archive format to include
xattrs in the initramfs. The changes are limited to gen_init_cpio, the
kernel build tree tool used to generate an initramfs, and init/initramfs.c,
the parser, with minor changes to IMA and EVM.

Mimi


Mimi Zohar (9):
initramfs: separate reading cpio method from header
initramfs: add extended attribute support
gen_init_cpio: replace inline format string with common variable
gen_init_cpio: define new CPIO format to support xattrs
gen_init_cpio: include the file extended attributes
gen_initramfs_list.sh: include xattrs
evm: make rootfs a special case
ima: include tmpfs in ima_appraise_tcb policy
init: remove "root=" command line option test for tmpfs decision

init/do_mounts.c | 2 +-
init/initramfs.c | 95 +++++++++++++++++++++---
scripts/gen_initramfs_list.sh | 2 +-
security/integrity/evm/evm_main.c | 12 ++-
security/integrity/ima/ima_policy.c | 2 +
usr/gen_init_cpio.c | 142 ++++++++++++++++++++++++++++++------
6 files changed, 218 insertions(+), 37 deletions(-)

--
1.8.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

[Mimi Zohar]

[Mimi Zohar]

Both the EVM HMAC and signature xattr formats are file system
specific and can not be copied from one filesystem to another.

EVM differentiates files without any xattrs (INTEGRITY_UNKNOWN)
from those having protected xattrs (INTEGRITY_NOLABEL). This
patch treats the rootfs filesystem as a special case, returning
INTEGRITY_UNKNOWN.

Signed-off-by: Mimi Zohar <zo...@linux.vnet.ibm.com>
---
security/integrity/evm/evm_main.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
index b392fe6..9c71af7 100644
--- a/security/integrity/evm/evm_main.c
+++ b/security/integrity/evm/evm_main.c
@@ -128,11 +128,16 @@ static enum integrity_status evm_verify_hmac(struct dentry *dentry,
if (rc <= 0) {
evm_status = INTEGRITY_FAIL;
if (rc == -ENODATA) {
+ struct super_block *sb = dentry->d_inode->i_sb;
+
rc = evm_find_protected_xattrs(dentry);
- if (rc > 0)
- evm_status = INTEGRITY_NOLABEL;
- else if (rc == 0)
+ if (rc == 0)
evm_status = INTEGRITY_NOXATTRS; /* new file */
+ else if (rc > 0 && sb->s_magic == TMPFS_MAGIC
+ && strcmp(sb->s_id, "rootfs") == 0)
+ evm_status = INTEGRITY_UNKNOWN;
+ else if (rc > 0)
+ evm_status = INTEGRITY_NOLABEL;
} else if (rc == -EOPNOTSUPP) {
evm_status = INTEGRITY_UNKNOWN;

[Mimi Zohar]

This patch writes out the extended attributes included in the cpio file.
As the "security.ima" xattr needs to be written after the file data,
this patch separates extracting and setting the xattrs by defining two
new states "GotXattrs" and "SetXattrs".

Signed-off-by: Mimi Zohar <zo...@linux.vnet.ibm.com>
---

init/initramfs.c | 85 +++++++++++++++++++++++++++++++++++++++++++++++++-------
1 file changed, 75 insertions(+), 10 deletions(-)

diff --git a/init/initramfs.c b/init/initramfs.c
index 527703e..11ffd3e 100644
--- a/init/initramfs.c
+++ b/init/initramfs.c
@@ -52,6 +52,7 @@ static void __init error(char *x)
/* link hash */

#define N_ALIGN(len) ((((len) + 1) & ~3) + 2)
+#define X_ALIGN(len) ((len + 3) & ~3)

static __initdata struct hash {
int ino, minor, major;
@@ -154,19 +155,20 @@ static __initdata time_t mtime;

static __initdata unsigned long ino, major, minor, nlink;
static __initdata umode_t mode;
-static __initdata unsigned long body_len, name_len;
+static __initdata unsigned long body_len, name_len, xattr_buflen;
static __initdata uid_t uid;
static __initdata gid_t gid;
static __initdata unsigned rdev;
+static __initdata int newcx;

static void __init parse_header(char *s)
{
- unsigned long parsed[12];
+ unsigned long parsed[13];
char buf[9];
int i;

buf[8] = '\0';
- for (i = 0, s += 6; i < 12; i++, s += 8) {
+ for (i = 0; i < (!newcx ? 12 : 13); i++, s += 8) {
memcpy(buf, s, 8);
parsed[i] = simple_strtoul(buf, NULL, 16);
}
@@ -181,6 +183,7 @@ static void __init parse_header(char *s)
minor = parsed[8];
rdev = new_encode_dev(MKDEV(parsed[9], parsed[10]));
name_len = parsed[11];
+ xattr_buflen = newcx ? parsed[12] : 0;
}

/* FSM */
@@ -192,7 +195,9 @@ static __initdata enum state {
GotHeader,
SkipIt,
GotName,
+ GotXattrs,
CopyFile,
+ SetXattrs,
GotSymlink,
Reset
} state, next_state;
@@ -209,6 +214,8 @@ static inline void __init eat(unsigned n)
}

static __initdata char *vcollected;
+static __initdata char *ncollected;
+static __initdata char *xcollected;
static __initdata char *collected;
static long remains __initdata;
static __initdata char *collect;
@@ -227,7 +234,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)
{
@@ -251,22 +258,26 @@ static int __init do_collect(void)

static int __init do_format(void)
{
+ newcx = 0;
if (memcmp(collected, "070707", 6)==0) {
error("incorrect cpio method used: use -H newc option");
return 1;
}
- if (memcmp(collected, "070701", 6)) {
+ if (memcmp(collected, "070703", 6) == 0)
+ newcx = 1;
+ else if (memcmp(collected, "070701", 6)) {
error("no cpio magic");
return 1;
}
- read_into(header_buf, 104, GotHeader);
+ read_into(header_buf, !newcx ? 104: 112, GotHeader);
return 0;
}

static int __init do_header(void)
{
parse_header(collected);
- next_header = this_header + N_ALIGN(name_len) + body_len;
+ next_header = this_header + N_ALIGN(name_len) + X_ALIGN(xattr_buflen)
+ + body_len;
next_header = (next_header + 3) & ~3;
state = SkipIt;
if (name_len <= 0 || name_len > PATH_MAX)
@@ -328,8 +339,52 @@ static void __init clean_path(char *path, umode_t mode)
}
}

-static __initdata int wfd;
+static int __init do_xattrs(void)
+{
+ state = next_state;
+ xcollected = kmalloc(xattr_buflen, GFP_KERNEL);
+ if (!xcollected)
+ panic("can't allocate xattr buffer");
+ memcpy(xcollected, collected, xattr_buflen);
+ return 0;
+}
+
+static int __init do_setxattrs(void)
+{
+ char *xattr_name = NULL;
+ int i, offset = 8, num_xattrs = 0;
+ unsigned xattr_value_size;
+ char *buf = xcollected;
+
+ state = SkipIt;
+ next_state = Reset;

+ if (!newcx || xattr_buflen == 0 || !buf)
+ return 0;
+
+ sscanf(buf, "%08X", &num_xattrs);
+
+ /* xattr format: name value-len value */
+ for (i = 0; i < num_xattrs; i++) {
+ void *xattr_buf;
+ int rc;
+
+ xattr_name = buf + offset;
+ offset += (strlen(xattr_name) + 1);
+ sscanf(buf + offset, "%08X", &xattr_value_size);
+ xattr_buf = buf + offset + 8;
+ rc = sys_setxattr(ncollected, xattr_name, xattr_buf,
+ xattr_value_size, 0);
+ pr_debug("%s: %s size: %u (rc: %d)\n", ncollected, xattr_name,
+ xattr_value_size, rc);
+ offset += (8 + xattr_value_size);
+ }
+ kfree(ncollected);
+ kfree(xcollected);
+ return 0;
+}
+
+static __initdata int wfd;
static int __init do_name(void)
{
state = SkipIt;
@@ -370,6 +425,12 @@ static int __init do_name(void)
do_utime(collected, mtime);
}
}
+
+ if (xattr_buflen > 0) {
+ ncollected = kstrdup(collected, GFP_KERNEL);
+ next_state = (state == SkipIt) ? SetXattrs : state;
+ read_into(xattr_buf, X_ALIGN(xattr_buflen), GotXattrs);
+ }
return 0;
}

@@ -382,7 +443,7 @@ static int __init do_copy(void)
do_utime(vcollected, mtime);
kfree(vcollected);
eat(body_len);
- state = SkipIt;
+ state = (newcx && xattr_buflen > 0)? SetXattrs : SkipIt;
return 0;
} else {
if (xwrite(wfd, victim, count) != count)
@@ -412,7 +473,9 @@ static __initdata int (*actions[])(void) = {
[GotHeader] = do_header,
[SkipIt] = do_skip,
[GotName] = do_name,
+ [GotXattrs] = do_xattrs,
[CopyFile] = do_copy,
+ [SetXattrs] = do_setxattrs,
[GotSymlink] = do_symlink,
[Reset] = do_reset,
};
@@ -461,9 +524,10 @@ static char * __init unpack_to_rootfs(char *buf, unsigned long len)
const char *compress_name;
static __initdata char msg_buf[64];

- header_buf = kmalloc(110, GFP_KERNEL);
+ header_buf = kmalloc(118, GFP_KERNEL);
symlink_buf = kmalloc(PATH_MAX + N_ALIGN(PATH_MAX) + 1, GFP_KERNEL);
name_buf = kmalloc(N_ALIGN(PATH_MAX), GFP_KERNEL);
+ xattr_buf = kmalloc(PAGE_SIZE, GFP_KERNEL);

if (!header_buf || !symlink_buf || !name_buf)
panic("can't allocate buffers");
@@ -510,6 +574,7 @@ static char * __init unpack_to_rootfs(char *buf, unsigned long len)
len -= my_inptr;
}
dir_utime();
+ kfree(xattr_buf);
kfree(name_buf);
kfree(symlink_buf);
kfree(header_buf);

[Mimi Zohar]

This patch modifies the gen_initramfs_list.sh script to include xattrs
in the initramfs.

Dracut creates the initramfs using the cpio tool on the system, not
the kernel's gen_init_cpio script. The following commands, for example,
would create an initramfs containing xattrs.

dracut -H -f /boot/initramfs-3.XX.0+.img 3.XX.0+ -M --keep \
--noprelink --nostrip
gen_initramfs_list.sh /var/tmp/initramfs.XXXXXX/ > \
/var/tmp/initramfs_list.XXXXXX

[Sign files here, if not already signed, using evmctl.]

gen_init_cpio -x /var/tmp/initramfs_list.XXXXXX > \
/boot/initramfs-3.XX.0+test.img

Signed-off-by: Mimi Zohar <zo...@linux.vnet.ibm.com>
---

scripts/gen_initramfs_list.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/gen_initramfs_list.sh b/scripts/gen_initramfs_list.sh
index 17fa901..20c3a80 100644
--- a/scripts/gen_initramfs_list.sh
+++ b/scripts/gen_initramfs_list.sh
@@ -307,7 +307,7 @@ if [ ! -z ${output_file} ]; then
fi
fi
cpio_tfile="$(mktemp ${TMPDIR:-/tmp}/cpiofile.XXXXXX)"
- usr/gen_init_cpio $timestamp ${cpio_list} > ${cpio_tfile}
+ usr/gen_init_cpio $timestamp -x ${cpio_list} > ${cpio_tfile}
else
cpio_tfile=${cpio_file}
fi

[Mimi Zohar]

Now that the rootfs includes extended attributes, don't
automatically exclude tmpfs file systems from being appraised.

Signed-off-by: Mimi Zohar <zo...@linux.vnet.ibm.com>
---

security/integrity/evm/evm_main.c | 1 +
security/integrity/ima/ima_policy.c | 2 ++
2 files changed, 3 insertions(+)

diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
index 9c71af7..e942e63 100644
--- a/security/integrity/evm/evm_main.c
+++ b/security/integrity/evm/evm_main.c
@@ -19,6 +19,7 @@
#include <linux/module.h>
#include <linux/crypto.h>
#include <linux/audit.h>
+#include <linux/magic.h>
#include <linux/xattr.h>
#include <linux/integrity.h>
#include <linux/evm.h>
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index d1eefb9..7267eac 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -93,7 +93,9 @@ static struct ima_rule_entry default_appraise_rules[] = {
{.action = DONT_APPRAISE, .fsmagic = PROC_SUPER_MAGIC, .flags = IMA_FSMAGIC},
{.action = DONT_APPRAISE, .fsmagic = SYSFS_MAGIC, .flags = IMA_FSMAGIC},
{.action = DONT_APPRAISE, .fsmagic = DEBUGFS_MAGIC, .flags = IMA_FSMAGIC},
+#ifndef CONFIG_IMA_LOAD_X509
{.action = DONT_APPRAISE, .fsmagic = TMPFS_MAGIC, .flags = IMA_FSMAGIC},
+#endif
{.action = DONT_APPRAISE, .fsmagic = RAMFS_MAGIC, .flags = IMA_FSMAGIC},
{.action = DONT_APPRAISE, .fsmagic = DEVPTS_SUPER_MAGIC, .flags = IMA_FSMAGIC},
{.action = DONT_APPRAISE, .fsmagic = BINFMTFS_MAGIC, .flags = IMA_FSMAGIC},

[Josh Boyer]

The commit log makes it sound like tmpfs should be appraised
unconditionally, but you only have it being appraised if IMA_LOAD_X509
is set. Which is correct (and why isn't it based on whether
CONFIG_IMA_APPRAISE is set)?

Also, what happens if someone creates an initramfs that doesn't
include xattrs and has this option set?

Slightly confusing.

josh

[Josh Boyer]

On Wed, Jan 7, 2015 at 3:52 PM, Mimi Zohar <zo...@linux.vnet.ibm.com> wrote:

> This patch modifies the gen_initramfs_list.sh script to include xattrs
> in the initramfs.
>
> Dracut creates the initramfs using the cpio tool on the system, not
> the kernel's gen_init_cpio script. The following commands, for example,
> would create an initramfs containing xattrs.
>
> dracut -H -f /boot/initramfs-3.XX.0+.img 3.XX.0+ -M --keep \
> --noprelink --nostrip
> gen_initramfs_list.sh /var/tmp/initramfs.XXXXXX/ > \
> /var/tmp/initramfs_list.XXXXXX
>
> [Sign files here, if not already signed, using evmctl.]
>
> gen_init_cpio -x /var/tmp/initramfs_list.XXXXXX > \
> /boot/initramfs-3.XX.0+test.img

That's pretty awkward. I think it highlights the major downside of
this approach in that from a standard distro point of view this
functionality isn't likely to be used. Do you foresee this feature as
something that should be widely used, or something that would be used
more in custom, locked-down machines?

I can understand not wanting to redefine the newc format in userspace
cpio, but if you want this to be easier to use then perhaps working
with dracut upstream to make it support this out of the box would be a
good idea.

josh

[Mimi Zohar]

Right, with commit fd5f4e90 "ima: load x509 certificate from the
kernel", the IMA appraisal key can be loaded before executing "init",
wherever it might be.

Only if IMA-appraisal is enabled and the ima_appraise_tcb is specified
on the kernel boot command line, are the builtin ima_appraise_tcb policy
rules used.

Mimi

[Mimi Zohar]

On Thu, 2015-01-08 at 09:01 -0500, Josh Boyer wrote:
> On Wed, Jan 7, 2015 at 3:52 PM, Mimi Zohar <zo...@linux.vnet.ibm.com> wrote:
> > This patch modifies the gen_initramfs_list.sh script to include xattrs
> > in the initramfs.
> >
> > Dracut creates the initramfs using the cpio tool on the system, not
> > the kernel's gen_init_cpio script. The following commands, for example,
> > would create an initramfs containing xattrs.
> >
> > dracut -H -f /boot/initramfs-3.XX.0+.img 3.XX.0+ -M --keep \
> > --noprelink --nostrip
> > gen_initramfs_list.sh /var/tmp/initramfs.XXXXXX/ > \
> > /var/tmp/initramfs_list.XXXXXX
> >
> > [Sign files here, if not already signed, using evmctl.]
> >
> > gen_init_cpio -x /var/tmp/initramfs_list.XXXXXX > \
> > /boot/initramfs-3.XX.0+test.img
>
> That's pretty awkward. I think it highlights the major downside of
> this approach in that from a standard distro point of view this
> functionality isn't likely to be used. Do you foresee this feature as
> something that should be widely used, or something that would be used
> more in custom, locked-down machines?

Before distros can start enabling these features, software packages need
to come with file signatures. Fin Gunter posted (and shortly will
re-post) patches to include file signatures in RPM patches.

Including file signatures in RPM packages (and similarly in other
software package formats) is the direction we, the linux community, IMHO
should be moving. How long this will take is entirely up to the
distros.

> I can understand not wanting to redefine the newc format in userspace
> cpio, but if you want this to be easier to use then perhaps working
> with dracut upstream to make it support this out of the box would be a
> good idea.

Anyone using dracut/systemd is currently not using tmpfs, as specifying
"root=" on the boot command line reverts to using ramfs. Rob Landley
suggested userspace apps use "ROOT=" instead.
(http://sourceforge.net/p/linux-ima/mailman/message/33189705/)

This patch set was posted as an RFC. Assuming this solution for
including xattrs in the rootfs is acceptable, I'll post the
dracut/systemd changes.

Mimi

[Rob Landley]

On 01/08/2015 09:13 AM, Mimi Zohar wrote:
> On Thu, 2015-01-08 at 09:01 -0500, Josh Boyer wrote:
>> On Wed, Jan 7, 2015 at 3:52 PM, Mimi Zohar <zo...@linux.vnet.ibm.com> wrote:
>> That's pretty awkward. I think it highlights the major downside of
>> this approach in that from a standard distro point of view this
>> functionality isn't likely to be used. Do you foresee this feature as
>> something that should be widely used, or something that would be used
>> more in custom, locked-down machines?
>
> Before distros can start enabling these features, software packages need
> to come with file signatures. Fin Gunter posted (and shortly will
> re-post) patches to include file signatures in RPM patches.

My personal lack of caring about Red Hat's bureaucratic "signing
binaries in triplicate" is probably large enough to be seen from space
(obviously no vendor code has ever contained an exploit that could be
used to run arbitrary code in ring 0, and this totally won't be used for
vendor lock-in, but I remain unconvinced because I'm funny that way)...

But I am curious about how you propose to encode xattrs into the cpio
format. (Which Al Viro chose because it's _simple_. There isn't really a
controlling spec since Posix decided to deprecated it in 2001 and yank
it from SUSv3 onwards. LSB extended several header fields to 8 hex
digits instead of 6, but they still have 32 bit timestamps which seems a
bit short-sighted. If you're going to define a new rev with a new magic
number, there are a couple other things you might wanna fix...)

I ask because I maintain a new from-scratch cpio implementation
(http://landley.net/hg/toybox/file/1571/toys/posix/cpio.c), so I'd
presumably have to add your format extensions to this. Is there any sort
of documentation on them?

The toybox config Android is using has this cpio implementation enabled
(see
https://android.googlesource.com/platform/external/toybox/+/9250c95a8c47/Android.mk)
so I'd rather like to get this sort of detail right...

> Including file signatures in RPM packages (and similarly in other
> software package formats) is the direction we, the linux community, IMHO
> should be moving. How long this will take is entirely up to the
> distros.

Glued down to a trusted platform module such that obviously nobody can
possibly exploit such a system, from
https://www.youtube.com/watch?v=4loZGYqaZ7I to
https://trmm.net/Thunderstrike_31c3

I see this as way, way more about vendor lock-in than security.

>> I can understand not wanting to redefine the newc format in userspace
>> cpio, but if you want this to be easier to use then perhaps working
>> with dracut upstream to make it support this out of the box would be a
>> good idea.
>
> Anyone using dracut/systemd is currently not using tmpfs, as specifying
> "root=" on the boot command line reverts to using ramfs. Rob Landley
> suggested userspace apps use "ROOT=" instead.
> (http://sourceforge.net/p/linux-ima/mailman/message/33189705/)

I'm working on a documentation update, but the old docs I wrote have
gone a bit stale in a number of places so I'm not done yet...

> This patch set was posted as an RFC. Assuming this solution for
> including xattrs in the rootfs is acceptable, I'll post the
> dracut/systemd changes.

(I'm not particularly interested in systemd either, but good luck with
that...)

> Mimi

Rob

[Mimi Zohar]

On Thu, 2015-01-08 at 12:19 -0600, Rob Landley wrote:
>
> But I am curious about how you propose to encode xattrs into the cpio
> format. (Which Al Viro chose because it's _simple_. There isn't really
> a
> controlling spec since Posix decided to deprecated it in 2001 and
> yank
> it from SUSv3 onwards. LSB extended several header fields to 8 hex
> digits instead of 6, but they still have 32 bit timestamps which seems
> a
> bit short-sighted. If you're going to define a new rev with a new
> magic
> number, there are a couple other things you might wanna fix...)

Sounds like a good opportunity to make the other changes as well. We
can include the other changes in this patch set. Is this (initramfs)
the right mailing list for this discussion? Do other people need to be
included?

> I ask because I maintain a new from-scratch cpio implementation
> (http://landley.net/hg/toybox/file/1571/toys/posix/cpio.c), so I'd
> presumably have to add your format extensions to this. Is there any
> sort
> of documentation on them?
>
> The toybox config Android is using has this cpio implementation
> enabled
> (see
> https://android.googlesource.com/platform/external/toybox/+/9250c95a8c47/Android.mk)
> so I'd rather like to get this sort of detail right...

The xattr section, which follows the file name, is of the format:
<number of xattrs> { <xattr name> <xattr data size> <xattr data> } for
each xattr, terminated with a NULL byte and padded to a 4 byte boundary.

The header contains an additional field, before the checksum, containing
the xattr section length, including the NULL byte, but without the
padding.

Note that gen_init_cpio does not include "security.evm" as it is file
system dependent.

Mimi

[Rob Landley]

On 01/08/2015 04:08 PM, Mimi Zohar wrote:
> On Thu, 2015-01-08 at 12:19 -0600, Rob Landley wrote:
>>
>> But I am curious about how you propose to encode xattrs into the cpio
>> format. (Which Al Viro chose because it's _simple_. There isn't really
>> a
>> controlling spec since Posix decided to deprecated it in 2001 and
>> yank
>> it from SUSv3 onwards. LSB extended several header fields to 8 hex
>> digits instead of 6, but they still have 32 bit timestamps which seems
>> a
>> bit short-sighted. If you're going to define a new rev with a new
>> magic
>> number, there are a couple other things you might wanna fix...)
>
> Sounds like a good opportunity to make the other changes as well. We
> can include the other changes in this patch set. Is this (initramfs)
> the right mailing list for this discussion?

I'd forgotten there was such a list until the email came in. :)

> Do other people need to be included?

In theory including the Austin Group (the posix committee mailing list)
might be useful, but in practice they hear about stuff well after the
fact, and they washed their hands of cpio over a decade ago (shortly
after Linux started heavily using it in rpm, and a bit before initramfs).

I note that there are two data formats of interest here:

1) the cpio file layout.

2) the list of files generated by gen_initramfs_list.sh and consumed by
gen_init_cpio.

The fact you're modifying gen_initramfs_list.sh seems to imply that
you're changing the _second_ format as well as the first...? The second
was never actually specified, but it does get used a lot by various
build systems and breaking it would inconvenience people. (Plus I'd need
to update the documentation, but I need to do that anyway.)

Ss long as you're extending the format could you add a second 32 bits of
time data that gets glued to the top half of a uint64_t, and then store
the actual time value in microseconds (so time*1000000)? (I'd say
"nanoseconds" but 63 bits of nanoseconds is 292 years, which is just
short enough I'm uncomfortable with it. I'm just optimistic enough to
think that might inconvenience somebody.)

The other "huh" is the filesize, but 4 gigs per file seems unlikely to
be more than initramfs needs any time soon? (It's possible that RPM
might care in 15 years or so...)

>> I ask because I maintain a new from-scratch cpio implementation
>> (http://landley.net/hg/toybox/file/1571/toys/posix/cpio.c), so I'd
>> presumably have to add your format extensions to this. Is there any
>> sort
>> of documentation on them?
>>
>> The toybox config Android is using has this cpio implementation
>> enabled
>> (see
>> https://android.googlesource.com/platform/external/toybox/+/9250c95a8c47/Android.mk)
>> so I'd rather like to get this sort of detail right...
>
> The xattr section, which follows the file name, is of the format:
> <number of xattrs> { <xattr name> <xattr data size> <xattr data> } for
> each xattr, terminated with a NULL byte and padded to a 4 byte boundary.

where <value> is... 8 bytes of ascii hex digits like the header values?
(Every cpio string is padded to a boundary. Sigh, lemme go read your
patch...)

Ok, 2/9, actual file format parsing. New magic string at the start
"070703" for the new version. (Good, that was my first question: easy
way to distinguish this from the previous format).

- for (i = 0, s += 6; i < 12; i++, s += 8) {
+ for (i = 0; i < (!newcx ? 12 : 13); i++, s += 8) {

You've tested this and the missing s+= 6 didn't cause problems? (Or did
it move somewhere else...? Is that what the 1/9 did grabbing just the
magic instead of the rest of the header data...?)

> The header contains an additional field, before the checksum, containing
> the xattr section length, including the NULL byte, but without the
> padding.

Ah, the old "4 bytes of padding to align to 4 bytes" silliness. (Even
though you can't trust the null to _be_ there and have to set it
yourself after the read.) I'm starting to remember this...

Ok, different header magic, one new 8 hex digit field at the end of the
header (before crc) containing "xattr_bufflen". The start of this buffer
is an 8 digit hex "num_xattrs", which you iterate through and call
strlen() on despite never having assured that the data you read in
actually _does_ contain a null at the end (of the entire buffer). Then
past that supposed null is another 8 digit hex "xattr_value_size", and
that many bytes following you then send to sys_setxattr().

Except for the part about you trusting your input data waaaaay too much,
seems reasonable? I have no idea what sys_setxattr() accepts, but
presumably there's a man page for the system call...

http://man7.org/linux/man-pages/man2/setxattr.2.html

Ok, that's probably enough data to implement it. (Not sure why that man
page isn't in my ubuntu 14.04 install which has manpages-dev installed?

$ man setxattr
No manual entry for setxattr

> Note that gen_init_cpio does not include "security.evm" as it is file
> system dependent.

I have no idea what that is. Should I not include it in the command line
cpio? (Or have a flag?)

The last time I used extended attributes was on OS/2; my only
non-academic interaction with selinux has been looking up how to switch
it off.

I still boggle that Fortune 500 bureaucracies include "must have a
security system designed by the NSA based on the idea of complicating
the system until there are no obvious holes, because after the Snowden
leaks that's clearly a good idea" as part of their certification
processes for reducing the risk of being unable to delegate blame.

I'm also kind of impressed by the longevity of a hack the original Apple
Lisa developers invented in 1981 because their OS didn't have an
equivalent of the unix "file" command, and undoes the central unix
"everything's a flat file" idea to bring us back to the structure
records with magic meanings of the mainframe days.

http://www.folklore.org/StoryView.py?project=Macintosh&story=The_Grand_Unified_Model_The_Finder.txt&sortOrder=Sort+by+Title

Still, dictating what users can and can't do is policy. It exists,
people use it... sigh. I'll try to take a stab at it some evening this week.

> Mimi

Rob

[Mimi Zohar]

Patch "[PATCH 6/9] gen_initramfs_list.sh: include xattrs" is a one line
change that adds the "-x" option to include xattrs in the initramfs, if
they exist. This patch makes the new method (070703) the default format.

> Ss long as you're extending the format could you add a second 32 bits of
> time data that gets glued to the top half of a uint64_t, and then store
> the actual time value in microseconds (so time*1000000)? (I'd say
> "nanoseconds" but 63 bits of nanoseconds is 292 years, which is just
> short enough I'm uncomfortable with it. I'm just optimistic enough to
> think that might inconvenience somebody.)
>
> The other "huh" is the filesize, but 4 gigs per file seems unlikely to
> be more than initramfs needs any time soon? (It's possible that RPM
> might care in 15 years or so...)

4 bytes enough?

> >> I ask because I maintain a new from-scratch cpio implementation
> >> (http://landley.net/hg/toybox/file/1571/toys/posix/cpio.c), so I'd
> >> presumably have to add your format extensions to this. Is there any
> >> sort
> >> of documentation on them?
> >>
> >> The toybox config Android is using has this cpio implementation
> >> enabled
> >> (see
> >> https://android.googlesource.com/platform/external/toybox/+/9250c95a8c47/Android.mk)
> >> so I'd rather like to get this sort of detail right...
> >
> > The xattr section, which follows the file name, is of the format:
> > <number of xattrs> { <xattr name> <xattr data size> <xattr data> } for
> > each xattr, terminated with a NULL byte and padded to a 4 byte boundary.
>
> where <value> is... 8 bytes of ascii hex digits like the header values?
> (Every cpio string is padded to a boundary. Sigh, lemme go read your
> patch...)
>
> Ok, 2/9, actual file format parsing. New magic string at the start
> "070703" for the new version. (Good, that was my first question: easy
> way to distinguish this from the previous format).
>
> - for (i = 0, s += 6; i < 12; i++, s += 8) {
> + for (i = 0; i < (!newcx ? 12 : 13); i++, s += 8) {
>
> You've tested this and the missing s+= 6 didn't cause problems? (Or did
> it move somewhere else...? Is that what the 1/9 did grabbing just the
> magic instead of the rest of the header data...?)

Right, the first patch separates reading the magic string from reading
the rest of the header.

> > The header contains an additional field, before the checksum, containing
> > the xattr section length, including the NULL byte, but without the
> > padding.
>
> Ah, the old "4 bytes of padding to align to 4 bytes" silliness. (Even
> though you can't trust the null to _be_ there and have to set it
> yourself after the read.) I'm starting to remember this...
>
> Ok, different header magic, one new 8 hex digit field at the end of the
> header (before crc) containing "xattr_bufflen". The start of this buffer
> is an 8 digit hex "num_xattrs", which you iterate through and call
> strlen() on despite never having assured that the data you read in
> actually _does_ contain a null at the end (of the entire buffer). Then
> past that supposed null is another 8 digit hex "xattr_value_size", and
> that many bytes following you then send to sys_setxattr().
>
> Except for the part about you trusting your input data waaaaay too much,
> seems reasonable?

Ok

> I have no idea what sys_setxattr() accepts, but
> presumably there's a man page for the system call...
>
> http://man7.org/linux/man-pages/man2/setxattr.2.html
>
> Ok, that's probably enough data to implement it. (Not sure why that man
> page isn't in my ubuntu 14.04 install which has manpages-dev installed?
>
> $ man setxattr
> No manual entry for setxattr
>
> > Note that gen_init_cpio does not include "security.evm" as it is file
> > system dependent.
>
> I have no idea what that is. Should I not include it in the command line
> cpio? (Or have a flag?)

Right, don't include "security.evm". Both the HMAC and signature format
include the inode number, which is filesystem specific.

> The last time I used extended attributes was on OS/2; my only
> non-academic interaction with selinux has been looking up how to switch
> it off.
>
> I still boggle that Fortune 500 bureaucracies include "must have a
> security system designed by the NSA based on the idea of complicating
> the system until there are no obvious holes, because after the Snowden
> leaks that's clearly a good idea" as part of their certification
> processes for reducing the risk of being unable to delegate blame.

I'm the linux-integrity subsystem maintainer, not an LSM maintainer (not
that there is anything wrong in being an LSM maintainer). So, I'm not
quite sure why you keep saying things like this to me. BTW, there are a
number of LSMs these days, not only SELinux.

> I'm also kind of impressed by the longevity of a hack the original Apple
> Lisa developers invented in 1981 because their OS didn't have an
> equivalent of the unix "file" command, and undoes the central unix
> "everything's a flat file" idea to bring us back to the structure
> records with magic meanings of the mainframe days.
>
> http://www.folklore.org/StoryView.py?project=Macintosh&story=The_Grand_Unified_Model_The_Finder.txt&sortOrder=Sort+by+Title
>
> Still, dictating what users can and can't do is policy. It exists,
> people use it...

> sigh. I'll try to take a stab at it some evening this week.
>
> > Mimi
>
> Rob

Thanks!

Mimi

[Rob Landley]

On 01/13/2015 02:20 PM, Mimi Zohar wrote:
> On Tue, 2015-01-13 at 12:48 -0600, Rob Landley wrote:
>> I note that there are two data formats of interest here:
>>
>> 1) the cpio file layout.
>>
>> 2) the list of files generated by gen_initramfs_list.sh and consumed by
>> gen_init_cpio.
>>
>> The fact you're modifying gen_initramfs_list.sh seems to imply that
>> you're changing the _second_ format as well as the first...? The second
>> was never actually specified, but it does get used a lot by various
>> build systems and breaking it would inconvenience people. (Plus I'd need
>> to update the documentation, but I need to do that anyway.)
>
> Patch "[PATCH 6/9] gen_initramfs_list.sh: include xattrs" is a one line
> change that adds the "-x" option to include xattrs in the initramfs, if
> they exist.

Unconditionally. Even if we've configured xattr support out of our
kernel or tmpfs?

> This patch makes the new method (070703) the default format.

So nobody should ever try to build an embedded system (without xattrs)
from something like Red Hat Enterprise (where they just magically show up)?

>> Ss long as you're extending the format could you add a second 32 bits of
>> time data that gets glued to the top half of a uint64_t, and then store
>> the actual time value in microseconds (so time*1000000)? (I'd say
>> "nanoseconds" but 63 bits of nanoseconds is 292 years, which is just
>> short enough I'm uncomfortable with it. I'm just optimistic enough to
>> think that might inconvenience somebody.)
>>
>> The other "huh" is the filesize, but 4 gigs per file seems unlikely to
>> be more than initramfs needs any time soon? (It's possible that RPM
>> might care in 15 years or so...)
>
> 4 bytes enough?

Eh, as long as we're breaking compatibility anyway, we might as well
extend the file size. It's gzipped so the extra run of consecutive
zeroes isn't really an issue, and if tmpfs is going to support 64 bit
file sizes the thing that's populating them should to just to match.
(You can already have memory bigger than 4g. Some crazy person is going
to put a squashfs in tmpfs and loopback mount it, or have a giant video
there, or... Bootloaders being able to cope with this is not my problem. :)

Probably having the new fields at the end (and gluing them to the
earlier ones) makes more sense than having variable sized fields? I
don't have a strong opinion either way.

>> I have no idea what sys_setxattr() accepts, but
>> presumably there's a man page for the system call...
>>
>> http://man7.org/linux/man-pages/man2/setxattr.2.html
>>
>> Ok, that's probably enough data to implement it. (Not sure why that man
>> page isn't in my ubuntu 14.04 install which has manpages-dev installed?
>>
>> $ man setxattr
>> No manual entry for setxattr
>>
>>> Note that gen_init_cpio does not include "security.evm" as it is file
>>> system dependent.
>>
>> I have no idea what that is. Should I not include it in the command line
>> cpio? (Or have a flag?)
>
> Right, don't include "security.evm". Both the HMAC and signature format
> include the inode number, which is filesystem specific.

So save extended attributes but filter out this one magic extended
attribute that we _shouldn't_ save because we know, a priori, that this
data is not portable.

I'm remembering why I didn't want to get this on me.

>> The last time I used extended attributes was on OS/2; my only
>> non-academic interaction with selinux has been looking up how to switch
>> it off.
>>
>> I still boggle that Fortune 500 bureaucracies include "must have a
>> security system designed by the NSA based on the idea of complicating
>> the system until there are no obvious holes, because after the Snowden
>> leaks that's clearly a good idea" as part of their certification
>> processes for reducing the risk of being unable to delegate blame.
>
> I'm the linux-integrity subsystem maintainer, not an LSM maintainer (not
> that there is anything wrong in being an LSM maintainer). So, I'm not
> quite sure why you keep saying things like this to me. BTW, there are
> a number of LSMs these days, not only SELinux.

Yes, and I can't keep 'em straight. The android guys are adding SELinux:

https://android.googlesource.com/platform/external/toybox/+/d5c66a9fd36777f80ba05301dcfa6789b103e486

The Tizen guys are adding something called "smack":
https://git.tizen.org/cgit/platform/upstream/toybox.git

Up until about 3 months ago I had successfully avoided all of this. Oh
well, always something new to learn...

Do each of them have their own rules about what extended attribute data
is not portable and should be filtered out? Or is this one magic entry
it? (I'd RTFM but http://man7.org/linux/man-pages/man5/attr.5.html does
not contain the string "evm".)

Rob

[Mimi Zohar]

On Tue, 2015-01-13 at 15:42 -0600, Rob Landley wrote:
>
> On 01/13/2015 02:20 PM, Mimi Zohar wrote:
> > On Tue, 2015-01-13 at 12:48 -0600, Rob Landley wrote:
> >> I note that there are two data formats of interest here:
> >>
> >> 1) the cpio file layout.
> >>
> >> 2) the list of files generated by gen_initramfs_list.sh and consumed by
> >> gen_init_cpio.
> >>
> >> The fact you're modifying gen_initramfs_list.sh seems to imply that
> >> you're changing the _second_ format as well as the first...? The second
> >> was never actually specified, but it does get used a lot by various
> >> build systems and breaking it would inconvenience people. (Plus I'd need
> >> to update the documentation, but I need to do that anyway.)
> >
> > Patch "[PATCH 6/9] gen_initramfs_list.sh: include xattrs" is a one line
> > change that adds the "-x" option to include xattrs in the initramfs, if
> > they exist.
>
> Unconditionally. Even if we've configured xattr support out of our
> kernel or tmpfs?
>
> > This patch makes the new method (070703) the default format.
>
> So nobody should ever try to build an embedded system (without xattrs)
> from something like Red Hat Enterprise (where they just magically show up)?

Good point. I'll address this in the next post.

> >> Ss long as you're extending the format could you add a second 32 bits of
> >> time data that gets glued to the top half of a uint64_t, and then store
> >> the actual time value in microseconds (so time*1000000)? (I'd say
> >> "nanoseconds" but 63 bits of nanoseconds is 292 years, which is just
> >> short enough I'm uncomfortable with it. I'm just optimistic enough to
> >> think that might inconvenience somebody.)
> >>
> >> The other "huh" is the filesize, but 4 gigs per file seems unlikely to
> >> be more than initramfs needs any time soon? (It's possible that RPM
> >> might care in 15 years or so...)
> >
> > 4 bytes enough?

> Eh, as long as we're breaking compatibility anyway, we might as well
> extend the file size. It's gzipped so the extra run of consecutive
> zeroes isn't really an issue, and if tmpfs is going to support 64 bit
> file sizes the thing that's populating them should to just to match.
> (You can already have memory bigger than 4g. Some crazy person is going
> to put a squashfs in tmpfs and loopback mount it, or have a giant video
> there, or... Bootloaders being able to cope with this is not my problem. :)

> Probably having the new fields at the end (and gluing them to the
> earlier ones) makes more sense than having variable sized fields? I
> don't have a strong opinion either way.

The current file data size header field is a 8 character hexidecimal
string, which is long enough to store 4g (0xFFFFFFFF).

I would assume only 'security.evm' is not portable as it attempts to
tightly bind the file metadata to the file data. Casey? Paul?

Mimi

[Rob Landley]

On 01/13/2015 09:23 PM, Mimi Zohar wrote:
> On Tue, 2015-01-13 at 15:42 -0600, Rob Landley wrote:
>>> 4 bytes enough?
>
>> Eh, as long as we're breaking compatibility anyway, we might as well
>> extend the file size. It's gzipped so the extra run of consecutive
>> zeroes isn't really an issue, and if tmpfs is going to support 64 bit
>> file sizes the thing that's populating them should to just to match.
>> (You can already have memory bigger than 4g. Some crazy person is going
>> to put a squashfs in tmpfs and loopback mount it, or have a giant video
>> there, or... Bootloaders being able to cope with this is not my problem. :)
>
>> Probably having the new fields at the end (and gluing them to the
>> earlier ones) makes more sense than having variable sized fields? I
>> don't have a strong opinion either way.
>
> The current file data size header field is a 8 character hexidecimal
> string, which is long enough to store 4g (0xFFFFFFFF).

The current header fields are all 32 bits, yes. To get a 64 bit field
we'd have to add a second 32 bit field and add it <<32 to the original
one, or else have the header fields be of varying sizes. That was the
"adding a new one to the end" thing mentioned above.

Then again if we add a new field right before the previous size then the
"treat it as 64 bits vs 2 32 bit ones" is an implementation detail
anyway. And for the moment we can just have it be padding that
compresses away and wait for an actual >4g file.

Unless you think nobody will ever need an archive member >4g in
initramfs, even though servers with ~256g are reasonably common today
already?

Rob

[Mimi Zohar]

On Tue, 2015-01-13 at 22:34 -0600, Rob Landley wrote:
>
> On 01/13/2015 09:23 PM, Mimi Zohar wrote:
> > On Tue, 2015-01-13 at 15:42 -0600, Rob Landley wrote:

> Then again if we add a new field right before the previous size then the
> "treat it as 64 bits vs 2 32 bit ones" is an implementation detail
> anyway. And for the moment we can just have it be padding that
> compresses away and wait for an actual >4g file.

Sounds good.

Mimi

[Paul Moore]

On Tuesday, January 13, 2015 10:23:23 PM Mimi Zohar wrote:
> I would assume only 'security.evm' is not portable as it attempts to
> tightly bind the file metadata to the file data. Casey? Paul?

[NOTE: Added the SELinux mailing list to the CC line.]

The SELinux xattr should be portable assuming the security label's semantics
remain constant across the different security policies. If the label is
completely unknown SELinux should handle it correctly, it will be treated as
unlabeled until a module is loaded which defines the label.

Although, this is just for initramfs, yes? If so, I'm not sure this matters
that much from a practical point of view; Stephen or someone else from the
SELinux list may have some thoughts on this.

--
paul moore
security @ redhat