linux-ima-user — Q&A for users of IMA

[Linux-ima-user] IMA Configuration

[Marlon C. <mc...@ce...>]

Hi,

I have some questions about how to configure the IMA and I'm wondering if
you could help me understand some things.

1 - Could I configure the IMA to measure/appraise only specific folder
and/or file?
2 - Is there any example or documentation about how to use different rules
like uid, fowner, uuid, LSM labels, etc to limit which files and folders
must be measured?

In a previous contact with Zohar, she said to me that the question number
one is not implemented. Is this a good improvement to be developed?

-- 
*Marlon Chalegre*
*Software Engineer*

Re: [Linux-ima-user] [PATCH 0/1] EVM/IMA xattr hardening and unprivileged user ns mounts

[Seth F. <set...@ca...>]

On Tue, Aug 02, 2016 at 02:26:29PM -0500, Eric W. Biederman wrote:
> Mimi Zohar <zo...@li...> writes:
> 
> > On Di, 2016-08-02 at 09:09 -0500, Eric W. Biederman wrote:
> >> Mimi Zohar <zo...@li...> writes:
> >> 
> >> > On Mo, 2016-08-01 at 15:38 -0500, Eric W. Biederman wrote:
> >> >> Mimi Zohar <zo...@li...> writes:
> >> >> 
> >> >> > On Mo, 2016-08-01 at 08:19 -0500, Seth Forshee wrote:
> >> >> >> I was taking a look at IMA and EVM in the context of allowing mounts in
> >> >> >> user namespaces from users with only capabilities in that namespace
> >> >> >> (i.e. no global capabilities). As a result of that I found just a
> >> >> >> handful of places where the size of an xattr or the values it contains
> >> >> >> lack sufficient validation to protect against malicious xattrs. The
> >> >> >> fixes for these are worthwhile on their own, so I'm sending them in the
> >> >> >> following patch.
> >> >> >> 
> >> >> >> More generally though I'd like to request some guidance on how IMA and
> >> >> >> EVM should handle xattrs in mounts from globally-unprivileged users. For
> >> >> >> context, the intended use case for this is allowing fuse mounts in user
> >> >> >> namespace containers, and possibly mounts with a limited set of other
> >> >> >> filesystems in the future. All that would actually be needed to mount
> >> >> >> though are private user and mount namespaces.
> >> >> >> 
> >> >> >> Based on what I see, I suspect that we can treat the user ns mounts like
> >> >> >> any other mount. I don't see that this would cause harm as long as the
> >> >> >> filesystem can't trigger bugs in the xattr parsing, and if the system is
> >> >> >> configured for IMA enforcment this policy should also be applied to
> >> >> >> these mounts.
> >> >> >
> >> >> > "security.evm" is written as a result of one of the protected security
> >> >> > xattrs being written.   From your question, I assume that in this use
> >> >> > case scenario there are two concerns:
> >> >> > - parsing improper security.evm xattrs
> >> >> > - updating the protected security xattrs, causing the EVM xattr to
> >> >> > "bless" the change.
> >> >> >
> >> >> > The first issue you're addressing with the patch.  As for the second
> >> >> > issue,  as long as root in the namespace can not write security.xattrs,
> >> >> > there shouldn't be a problem.
> >> >> 
> >> >> Assume that it is allowed that root in a namespace will be allowed to
> >> >> write security xattrs for a filesystem it has mounted.  Certainly for
> >> >> the capability xattr this makes sense.
> >> >
> >> > Yeah, I remember.  Having one capability xattr sounded interesting.
> >> > Have the capability patches been upstreamed?   To label files in a
> >> > container, root in the namespace will need to be able to write an IMA
> >> > xattr.  Whether it is possible to use a single xattr, is yet to be
> >> > determined.
> >
> >> For the most part yes.  As part of this merge window Linus
> >> merged the changes to store s_user_ns on a super block and the code to
> >> use that.
> >
> > Great!  I'll take a look.
> >
> >> I have kept back the code to relax the permission checks in the obvious
> >> cases.  There is little immediate gain from that code and I noticed a
> >> few places where s_user_ns is not being handled properly.  Primarily
> >> IMA and EVM (so this disucssion).
> >
> > Ok
> >
> >> >> If understand you correctly is that the key used to sign the HMAC in
> >> >> security.evm is machine local.  As such I expect the proper semantics
> >> >> here will be to not update the security.evm xattrs on write of a
> >> >> filesystem with s_user_ns != &init_user_ns.
> >> >
> >> > The EVM xattr is verified before it can be changed, to prevent making an
> >> > existing invalid hmac, valid.  If the namespace has the ability to
> >> > change any of the metadata included in the hmac, the EVM  hmac will need
> >> > to be updated with the file metadata, otherwise nobody will be able to
> >> > access the file.
> >> 
> >> To be clear with evm enabled and security.evm attributes present if
> >> the hmac is wrong access to the file is denied?
> >
> > Yes, if the file is included in the IMA policy and the EVM
> > hmac/signature verification fails, then access to the file is denied.
> 
> Makes sense.
> 
> >> What happens in the case of new files and in the case of file without a
> >> security.evm xattr?
> >
> > New files are a special case.  The first EVM protected xattr* written
> > will cause the security.evm xattr to be written, without first
> > validating the existing xattr. 
> >
> > *evm_config_xattrnames[] contains the list of protected xattrs.
> 
> Thank you.
> 
> >> >> On read I am not certain what the right thing to do is, ignore the
> >> >> attribute or do what happens on validation failure.
> >> >
> >> >> > Any changes made offline/elsewhere, would
> >> >> > result in the security.evm HMAC not verifying, causing a denial of
> >> >> > service.  Unless of course the EVM HMAC key has somehow been
> >> >> > leaked/exfiltrated from the kernel.
> >> >> 
> >> >> One scenario where this is likely to come up, and the kind of scenario
> >> >> we are planning for is a filesystem on a usb stick is transported from
> >> >> one machine to another.  One the second system it is mounted directly
> >> >> (ideally) or through fuse running a copy of it's filesystem driver in
> >> >> userspace to guard against filesystem implementation bugs.
> >> >> 
> >> >> So if that filesystem has security.evm xattrs if it appears things will
> >> >> work very poorly.
> >> >
> >> > It could work for files with EVM signatures, obviously not with an HMAC.
> >> 
> >> Fair.
> >> 
> >> What policy would you recommend for (evm and ima) dealing with
> >> filesystems that are mounted by a user namespace root and thus have
> >> sb->s_user_ns != &init_user_ns?
> >
> > I think as long as it can not escape the mount namespace, allowing it to
> > write both the EVM and IMA xattrs should be fine.  In terms of reading
> > the xattrs, real root would need to be able to differentiate the xattrs'
> > origin.
> 
> It is a little more subtle than that, but essentially it is true that
> a filesystem will not propogate into a mount namespace owned by root in
> more privileged user namespace.
> 
> The piece of orgin that is retained in the kernel for a filesystem is
> sb->s_user_ns.  So the information is there.  It is not currently
> exported explicitly to userspace, but who owns the mount namespace a
> filesystem is in, is a big clue.
> 
> But yes any reader knows which filesystem an xattr is coming from so
> this should not be a point of confusion.
> 
> >> Like Seth I am not much of an expert in the semantics of IMA and EVM.
> >> I just know that ima calcualtes a hash of a file and does something with
> >> it.  EVM caculcates a hash of other xattrs and the classic file
> >> attributes.
> >
> > EVM protects file metadata.  For immutable files, such as ELF
> > executables, the security.evm should contain a file signature.  For
> > mutable files, security.evm should contain an hmac.
> 
> You know I am not certain what the distinction between a file signature
> and a hmac is in this context.

I've been reading back through all of this, and I'm not sure any
conclusion was reached.

For my part, I'm uneasy about writing out hmacs to mounts from a
non-root user. So I'll make a proposal - let's not read or write EVM or
IMA xattrs for filesystems from non-init user namespace, essentially
behaving as though they don't support xattrs. Something like the
(untested) patch below.

Thanks,
Seth


diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
index 11c1d30bd705..5a1738524fbb 100644
--- a/security/integrity/evm/evm_crypto.c
+++ b/security/integrity/evm/evm_crypto.c
@@ -182,7 +182,7 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry,
 	int error;
 	int size;
 
-	if (!inode->i_op->getxattr)
+	if (inode->i_sb->s_user_ns != &init_user_ns || !inode->i_op->getxattr)
 		return -EOPNOTSUPP;
 	desc = init_desc(type);
 	if (IS_ERR(desc))
diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
index 35ab453ce861..3d16e4c523c4 100644
--- a/security/integrity/evm/evm_main.c
+++ b/security/integrity/evm/evm_main.c
@@ -78,7 +78,7 @@ static int evm_find_protected_xattrs(struct dentry *dentry)
 	int error;
 	int count = 0;
 
-	if (!inode->i_op->getxattr)
+	if (inode->i_sb->s_user_ns != &init_user_ns || !inode->i_op->getxattr)
 		return -EOPNOTSUPP;
 
 	for (xattr = evm_config_xattrnames; *xattr != NULL; xattr++) {
@@ -118,6 +118,9 @@ static enum integrity_status evm_verify_hmac(struct dentry *dentry,
 	enum integrity_status evm_status = INTEGRITY_PASS;
 	int rc, xattr_len;
 
+	if (d_backing_inode(dentry)->i_sb->s_user_ns != &init_user_ns)
+		return INTEGRITY_UNKNOWN;
+
 	if (iint && iint->evm_status == INTEGRITY_PASS)
 		return iint->evm_status;
 
diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index a13fc6809554..7f6915a4f660 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -50,6 +50,9 @@ static int ima_fix_xattr(struct dentry *dentry,
 	int rc, offset;
 	u8 algo = iint->ima_hash->algo;
 
+	if (d_backing_inode(dentry)->i_sb->s_user_ns != &init_user_ns)
+		return -EOPNOTSUPP;
+
 	if (algo <= HASH_ALGO_SHA1) {
 		offset = 1;
 		iint->ima_hash->xattr.sha1.type = IMA_XATTR_DIGEST;
@@ -170,7 +173,7 @@ int ima_read_xattr(struct dentry *dentry,
 {
 	struct inode *inode = d_backing_inode(dentry);
 
-	if (!inode->i_op->getxattr)
+	if (inode->i_sb->s_user_ns != &init_user_ns || !inode->i_op->getxattr)
 		return 0;
 
 	return vfs_getxattr_alloc(dentry, XATTR_NAME_IMA, (char **)xattr_value,
@@ -198,7 +201,7 @@ int ima_appraise_measurement(enum ima_hooks func,
 	enum integrity_status status = INTEGRITY_UNKNOWN;
 	int rc = xattr_len, hash_start = 0;
 
-	if (!inode->i_op->getxattr)
+	if (inode->i_sb->s_user_ns != &init_user_ns || !inode->i_op->getxattr)
 		return INTEGRITY_UNKNOWN;
 
 	if (rc <= 0) {

Re: [Linux-ima-user] [PATCH 1/1] security/integrity: Harden against malformed xattrs

[Mimi Z. <zo...@li...>]

On Mon, 2016-08-29 at 13:31 -0500, Seth Forshee wrote:
> On Mon, Aug 01, 2016 at 08:19:10AM -0500, Seth Forshee wrote:
> > In general the handling of IMA/EVM xattrs is good, but I found
> > a few locations where either the xattr size or the value of the
> > type field in the xattr are not checked. Add a few simple checks
> > to these locations to prevent malformed or malicious xattrs from
> > causing problems.
> > 
> > Signed-off-by: Seth Forshee <set...@ca...>
> 
> Bump. I haven't seen any feedback on this patch, but I also don't think
> it's been applied anywhere, so I suspect it might have been forgotten.

Thanks for the reminder.

Mimi

Re: [Linux-ima-user] [PATCH 1/1] security/integrity: Harden against malformed xattrs

[Seth F. <set...@ca...>]

On Mon, Aug 01, 2016 at 08:19:10AM -0500, Seth Forshee wrote:
> In general the handling of IMA/EVM xattrs is good, but I found
> a few locations where either the xattr size or the value of the
> type field in the xattr are not checked. Add a few simple checks
> to these locations to prevent malformed or malicious xattrs from
> causing problems.
> 
> Signed-off-by: Seth Forshee <set...@ca...>

Bump. I haven't seen any feedback on this patch, but I also don't think
it's been applied anywhere, so I suspect it might have been forgotten.

Thanks,
Seth

Re: [Linux-ima-user] measure the container files

[Mimi Z. <zo...@li...>]

On Fri, 2016-08-12 at 18:50 +0800, Yan Kun wrote:
> Hi,
> 
> When I use docker to create a container,and create a new file in the
> container,the IMA can measure the new file.But when I modify the new
> file,IMA can not remeasure the file.In host system, I have boot with
> "rootflags=i_version".And IMA can remeasure the file of the OS .Why this

Changing the file does not cause the file to be re-measured.  On next
access the new measurement will be added to the measurement list,
assuming it is mounted with "iversion" and is in policy.   Please check
fstab to make sure the file system is mounted with "iversion".

Mimi

[Linux-ima-user] measure the container files

[Yan K. <sam...@gm...>]

Hi,

When I use docker to create a container,and create a new file in the
container,the IMA can measure the new file.But when I modify the new
file,IMA can not remeasure the file.In host system, I have boot with
"rootflags=i_version".And IMA can remeasure the file of the OS .Why this
can  not fit to files in container?

[Linux-ima-user] a simple question

[Yan K. <sam...@gm...>]

How ima work  in kernel.I mean in runtime,when a file changed,how ima find
it.It runs as a kernel thread or by many hooks?

[Linux-ima-user] the file flags

[Yan K. <sam...@gm...>]

Hi zohar:
             I think the same,but in ima_collect_measurement(),it check the
file's flags,O_DIRECT.

Re: [Linux-ima-user] about the file type

[Mimi Z. <zo...@li...>]

On Thu, 2016-08-04 at 11:01 +0800, Yan Kun wrote:
> Hi all:
>           Does IMA can generate a hash for each file that is loaded by any
> process ?

As long as the file is in policy, IMA will calculate the hash for a new
file or one that is updated.

Mimi

[Linux-ima-user] about the file type

[Yan K. <sam...@gm...>]

Hi all:
          Does IMA can generate a hash for each file that is loaded by any
process ?

Re: [Linux-ima-user] [PATCH 0/1] EVM/IMA xattr hardening and unprivileged user ns mounts

[<ebi...@xm...>]

Mimi Zohar <zo...@li...> writes:

> On Di, 2016-08-02 at 09:09 -0500, Eric W. Biederman wrote:
>> Mimi Zohar <zo...@li...> writes:
>> 
>> > On Mo, 2016-08-01 at 15:38 -0500, Eric W. Biederman wrote:
>> >> Mimi Zohar <zo...@li...> writes:
>> >> 
>> >> > On Mo, 2016-08-01 at 08:19 -0500, Seth Forshee wrote:
>> >> >> I was taking a look at IMA and EVM in the context of allowing mounts in
>> >> >> user namespaces from users with only capabilities in that namespace
>> >> >> (i.e. no global capabilities). As a result of that I found just a
>> >> >> handful of places where the size of an xattr or the values it contains
>> >> >> lack sufficient validation to protect against malicious xattrs. The
>> >> >> fixes for these are worthwhile on their own, so I'm sending them in the
>> >> >> following patch.
>> >> >> 
>> >> >> More generally though I'd like to request some guidance on how IMA and
>> >> >> EVM should handle xattrs in mounts from globally-unprivileged users. For
>> >> >> context, the intended use case for this is allowing fuse mounts in user
>> >> >> namespace containers, and possibly mounts with a limited set of other
>> >> >> filesystems in the future. All that would actually be needed to mount
>> >> >> though are private user and mount namespaces.
>> >> >> 
>> >> >> Based on what I see, I suspect that we can treat the user ns mounts like
>> >> >> any other mount. I don't see that this would cause harm as long as the
>> >> >> filesystem can't trigger bugs in the xattr parsing, and if the system is
>> >> >> configured for IMA enforcment this policy should also be applied to
>> >> >> these mounts.
>> >> >
>> >> > "security.evm" is written as a result of one of the protected security
>> >> > xattrs being written.   From your question, I assume that in this use
>> >> > case scenario there are two concerns:
>> >> > - parsing improper security.evm xattrs
>> >> > - updating the protected security xattrs, causing the EVM xattr to
>> >> > "bless" the change.
>> >> >
>> >> > The first issue you're addressing with the patch.  As for the second
>> >> > issue,  as long as root in the namespace can not write security.xattrs,
>> >> > there shouldn't be a problem.
>> >> 
>> >> Assume that it is allowed that root in a namespace will be allowed to
>> >> write security xattrs for a filesystem it has mounted.  Certainly for
>> >> the capability xattr this makes sense.
>> >
>> > Yeah, I remember.  Having one capability xattr sounded interesting.
>> > Have the capability patches been upstreamed?   To label files in a
>> > container, root in the namespace will need to be able to write an IMA
>> > xattr.  Whether it is possible to use a single xattr, is yet to be
>> > determined.
>
>> For the most part yes.  As part of this merge window Linus
>> merged the changes to store s_user_ns on a super block and the code to
>> use that.
>
> Great!  I'll take a look.
>
>> I have kept back the code to relax the permission checks in the obvious
>> cases.  There is little immediate gain from that code and I noticed a
>> few places where s_user_ns is not being handled properly.  Primarily
>> IMA and EVM (so this disucssion).
>
> Ok
>
>> >> If understand you correctly is that the key used to sign the HMAC in
>> >> security.evm is machine local.  As such I expect the proper semantics
>> >> here will be to not update the security.evm xattrs on write of a
>> >> filesystem with s_user_ns != &init_user_ns.
>> >
>> > The EVM xattr is verified before it can be changed, to prevent making an
>> > existing invalid hmac, valid.  If the namespace has the ability to
>> > change any of the metadata included in the hmac, the EVM  hmac will need
>> > to be updated with the file metadata, otherwise nobody will be able to
>> > access the file.
>> 
>> To be clear with evm enabled and security.evm attributes present if
>> the hmac is wrong access to the file is denied?
>
> Yes, if the file is included in the IMA policy and the EVM
> hmac/signature verification fails, then access to the file is denied.

Makes sense.

>> What happens in the case of new files and in the case of file without a
>> security.evm xattr?
>
> New files are a special case.  The first EVM protected xattr* written
> will cause the security.evm xattr to be written, without first
> validating the existing xattr. 
>
> *evm_config_xattrnames[] contains the list of protected xattrs.

Thank you.

>> >> On read I am not certain what the right thing to do is, ignore the
>> >> attribute or do what happens on validation failure.
>> >
>> >> > Any changes made offline/elsewhere, would
>> >> > result in the security.evm HMAC not verifying, causing a denial of
>> >> > service.  Unless of course the EVM HMAC key has somehow been
>> >> > leaked/exfiltrated from the kernel.
>> >> 
>> >> One scenario where this is likely to come up, and the kind of scenario
>> >> we are planning for is a filesystem on a usb stick is transported from
>> >> one machine to another.  One the second system it is mounted directly
>> >> (ideally) or through fuse running a copy of it's filesystem driver in
>> >> userspace to guard against filesystem implementation bugs.
>> >> 
>> >> So if that filesystem has security.evm xattrs if it appears things will
>> >> work very poorly.
>> >
>> > It could work for files with EVM signatures, obviously not with an HMAC.
>> 
>> Fair.
>> 
>> What policy would you recommend for (evm and ima) dealing with
>> filesystems that are mounted by a user namespace root and thus have
>> sb->s_user_ns != &init_user_ns?
>
> I think as long as it can not escape the mount namespace, allowing it to
> write both the EVM and IMA xattrs should be fine.  In terms of reading
> the xattrs, real root would need to be able to differentiate the xattrs'
> origin.

It is a little more subtle than that, but essentially it is true that
a filesystem will not propogate into a mount namespace owned by root in
more privileged user namespace.

The piece of orgin that is retained in the kernel for a filesystem is
sb->s_user_ns.  So the information is there.  It is not currently
exported explicitly to userspace, but who owns the mount namespace a
filesystem is in, is a big clue.

But yes any reader knows which filesystem an xattr is coming from so
this should not be a point of confusion.

>> Like Seth I am not much of an expert in the semantics of IMA and EVM.
>> I just know that ima calcualtes a hash of a file and does something with
>> it.  EVM caculcates a hash of other xattrs and the classic file
>> attributes.
>
> EVM protects file metadata.  For immutable files, such as ELF
> executables, the security.evm should contain a file signature.  For
> mutable files, security.evm should contain an hmac.

You know I am not certain what the distinction between a file signature
and a hmac is in this context.

>> That last hash as part of the work just merged I have updated to use
>> the uid and gid in s_user_ns instead of &init_user_ns, as on disk that
>> is the namespace the uid and gid are expected to be encoded in.
>
> Ok.  I need to take a look.

It is just the very obvious patch below.

commit 0b3c9761d1e405514a551ed24d3ea89aea26ce14
Author: Seth Forshee <set...@ca...>
Date:   Thu Feb 5 10:44:50 2015 -0600

    evm: Translate user/group ids relative to s_user_ns when computing HMAC
    
    The EVM HMAC should be calculated using the on disk user and
    group ids, so the k[ug]ids in the inode must be translated
    relative to the s_user_ns of the inode's super block.
    
    Signed-off-by: Seth Forshee <set...@ca...>
    Signed-off-by: Eric W. Biederman <ebi...@xm...>

diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
index 30b6b7d0429f..11c1d30bd705 100644
--- a/security/integrity/evm/evm_crypto.c
+++ b/security/integrity/evm/evm_crypto.c
@@ -151,8 +151,8 @@ static void hmac_add_misc(struct shash_desc *desc, struct inode *inode,
        memset(&hmac_misc, 0, sizeof(hmac_misc));
        hmac_misc.ino = inode->i_ino;
        hmac_misc.generation = inode->i_generation;
-       hmac_misc.uid = from_kuid(&init_user_ns, inode->i_uid);
-       hmac_misc.gid = from_kgid(&init_user_ns, inode->i_gid);
+       hmac_misc.uid = from_kuid(inode->i_sb->s_user_ns, inode->i_uid);
+       hmac_misc.gid = from_kgid(inode->i_sb->s_user_ns, inode->i_gid);
        hmac_misc.mode = inode->i_mode;
        crypto_shash_update(desc, (const u8 *)&hmac_misc, sizeof(hmac_misc));
        if (evm_hmac_attrs & EVM_ATTR_FSUUID)

Eric

Re: [Linux-ima-user] [PATCH 0/1] EVM/IMA xattr hardening and unprivileged user ns mounts

[Mimi Z. <zo...@li...>]

On Di, 2016-08-02 at 09:09 -0500, Eric W. Biederman wrote:
> Mimi Zohar <zo...@li...> writes:
> 
> > On Mo, 2016-08-01 at 15:38 -0500, Eric W. Biederman wrote:
> >> Mimi Zohar <zo...@li...> writes:
> >> 
> >> > On Mo, 2016-08-01 at 08:19 -0500, Seth Forshee wrote:
> >> >> I was taking a look at IMA and EVM in the context of allowing mounts in
> >> >> user namespaces from users with only capabilities in that namespace
> >> >> (i.e. no global capabilities). As a result of that I found just a
> >> >> handful of places where the size of an xattr or the values it contains
> >> >> lack sufficient validation to protect against malicious xattrs. The
> >> >> fixes for these are worthwhile on their own, so I'm sending them in the
> >> >> following patch.
> >> >> 
> >> >> More generally though I'd like to request some guidance on how IMA and
> >> >> EVM should handle xattrs in mounts from globally-unprivileged users. For
> >> >> context, the intended use case for this is allowing fuse mounts in user
> >> >> namespace containers, and possibly mounts with a limited set of other
> >> >> filesystems in the future. All that would actually be needed to mount
> >> >> though are private user and mount namespaces.
> >> >> 
> >> >> Based on what I see, I suspect that we can treat the user ns mounts like
> >> >> any other mount. I don't see that this would cause harm as long as the
> >> >> filesystem can't trigger bugs in the xattr parsing, and if the system is
> >> >> configured for IMA enforcment this policy should also be applied to
> >> >> these mounts.
> >> >
> >> > "security.evm" is written as a result of one of the protected security
> >> > xattrs being written.   From your question, I assume that in this use
> >> > case scenario there are two concerns:
> >> > - parsing improper security.evm xattrs
> >> > - updating the protected security xattrs, causing the EVM xattr to
> >> > "bless" the change.
> >> >
> >> > The first issue you're addressing with the patch.  As for the second
> >> > issue,  as long as root in the namespace can not write security.xattrs,
> >> > there shouldn't be a problem.
> >> 
> >> Assume that it is allowed that root in a namespace will be allowed to
> >> write security xattrs for a filesystem it has mounted.  Certainly for
> >> the capability xattr this makes sense.
> >
> > Yeah, I remember.  Having one capability xattr sounded interesting.
> > Have the capability patches been upstreamed?   To label files in a
> > container, root in the namespace will need to be able to write an IMA
> > xattr.  Whether it is possible to use a single xattr, is yet to be
> > determined.

> For the most part yes.  As part of this merge window Linus
> merged the changes to store s_user_ns on a super block and the code to
> use that.

Great!  I'll take a look.

> I have kept back the code to relax the permission checks in the obvious
> cases.  There is little immediate gain from that code and I noticed a
> few places where s_user_ns is not being handled properly.  Primarily
> IMA and EVM (so this disucssion).

Ok

> >> If understand you correctly is that the key used to sign the HMAC in
> >> security.evm is machine local.  As such I expect the proper semantics
> >> here will be to not update the security.evm xattrs on write of a
> >> filesystem with s_user_ns != &init_user_ns.
> >
> > The EVM xattr is verified before it can be changed, to prevent making an
> > existing invalid hmac, valid.  If the namespace has the ability to
> > change any of the metadata included in the hmac, the EVM  hmac will need
> > to be updated with the file metadata, otherwise nobody will be able to
> > access the file.
> 
> To be clear with evm enabled and security.evm attributes present if
> the hmac is wrong access to the file is denied?

Yes, if the file is included in the IMA policy and the EVM
hmac/signature verification fails, then access to the file is denied.

> What happens in the case of new files and in the case of file without a
> security.evm xattr?

New files are a special case.  The first EVM protected xattr* written
will cause the security.evm xattr to be written, without first
validating the existing xattr. 

*evm_config_xattrnames[] contains the list of protected xattrs.

> >> On read I am not certain what the right thing to do is, ignore the
> >> attribute or do what happens on validation failure.
> >
> >> > Any changes made offline/elsewhere, would
> >> > result in the security.evm HMAC not verifying, causing a denial of
> >> > service.  Unless of course the EVM HMAC key has somehow been
> >> > leaked/exfiltrated from the kernel.
> >> 
> >> One scenario where this is likely to come up, and the kind of scenario
> >> we are planning for is a filesystem on a usb stick is transported from
> >> one machine to another.  One the second system it is mounted directly
> >> (ideally) or through fuse running a copy of it's filesystem driver in
> >> userspace to guard against filesystem implementation bugs.
> >> 
> >> So if that filesystem has security.evm xattrs if it appears things will
> >> work very poorly.
> >
> > It could work for files with EVM signatures, obviously not with an HMAC.
> 
> Fair.
> 
> What policy would you recommend for (evm and ima) dealing with
> filesystems that are mounted by a user namespace root and thus have
> sb->s_user_ns != &init_user_ns?

I think as long as it can not escape the mount namespace, allowing it to
write both the EVM and IMA xattrs should be fine.  In terms of reading
the xattrs, real root would need to be able to differentiate the xattrs'
origin.

> Like Seth I am not much of an expert in the semantics of IMA and EVM.
> I just know that ima calcualtes a hash of a file and does something with
> it.  EVM caculcates a hash of other xattrs and the classic file
> attributes.

EVM protects file metadata.  For immutable files, such as ELF
executables, the security.evm should contain a file signature.  For
mutable files, security.evm should contain an hmac.

> That last hash as part of the work just merged I have updated to use
> the uid and gid in s_user_ns instead of &init_user_ns, as on disk that
> is the namespace the uid and gid are expected to be encoded in.

Ok.  I need to take a look.

Thanks!

Mimi

Re: [Linux-ima-user] [PATCH 0/1] EVM/IMA xattr hardening and unprivileged user ns mounts

[<ebi...@xm...>]

Mimi Zohar <zo...@li...> writes:

> On Mo, 2016-08-01 at 15:38 -0500, Eric W. Biederman wrote:
>> Mimi Zohar <zo...@li...> writes:
>> 
>> > On Mo, 2016-08-01 at 08:19 -0500, Seth Forshee wrote:
>> >> I was taking a look at IMA and EVM in the context of allowing mounts in
>> >> user namespaces from users with only capabilities in that namespace
>> >> (i.e. no global capabilities). As a result of that I found just a
>> >> handful of places where the size of an xattr or the values it contains
>> >> lack sufficient validation to protect against malicious xattrs. The
>> >> fixes for these are worthwhile on their own, so I'm sending them in the
>> >> following patch.
>> >> 
>> >> More generally though I'd like to request some guidance on how IMA and
>> >> EVM should handle xattrs in mounts from globally-unprivileged users. For
>> >> context, the intended use case for this is allowing fuse mounts in user
>> >> namespace containers, and possibly mounts with a limited set of other
>> >> filesystems in the future. All that would actually be needed to mount
>> >> though are private user and mount namespaces.
>> >> 
>> >> Based on what I see, I suspect that we can treat the user ns mounts like
>> >> any other mount. I don't see that this would cause harm as long as the
>> >> filesystem can't trigger bugs in the xattr parsing, and if the system is
>> >> configured for IMA enforcment this policy should also be applied to
>> >> these mounts.
>> >
>> > "security.evm" is written as a result of one of the protected security
>> > xattrs being written.   From your question, I assume that in this use
>> > case scenario there are two concerns:
>> > - parsing improper security.evm xattrs
>> > - updating the protected security xattrs, causing the EVM xattr to
>> > "bless" the change.
>> >
>> > The first issue you're addressing with the patch.  As for the second
>> > issue,  as long as root in the namespace can not write security.xattrs,
>> > there shouldn't be a problem.
>> 
>> Assume that it is allowed that root in a namespace will be allowed to
>> write security xattrs for a filesystem it has mounted.  Certainly for
>> the capability xattr this makes sense.
>
> Yeah, I remember.  Having one capability xattr sounded interesting.
> Have the capability patches been upstreamed?   To label files in a
> container, root in the namespace will need to be able to write an IMA
> xattr.  Whether it is possible to use a single xattr, is yet to be
> determined.

For the most part yes.  As part of this merge window Linus
merged the changes to store s_user_ns on a super block and the code to
use that.

I have kept back the code to relax the permission checks in the obvious
cases.  There is little immediate gain from that code and I noticed a
few places where s_user_ns is not being handled properly.  Primarily
IMA and EVM (so this disucssion).

>> If understand you correctly is that the key used to sign the HMAC in
>> security.evm is machine local.  As such I expect the proper semantics
>> here will be to not update the security.evm xattrs on write of a
>> filesystem with s_user_ns != &init_user_ns.
>
> The EVM xattr is verified before it can be changed, to prevent making an
> existing invalid hmac, valid.  If the namespace has the ability to
> change any of the metadata included in the hmac, the EVM  hmac will need
> to be updated with the file metadata, otherwise nobody will be able to
> access the file.

To be clear with evm enabled and security.evm attributes present if
the hmac is wrong access to the file is denied?

What happens in the case of new files and in the case of file without a
security.evm xattr?

>> On read I am not certain what the right thing to do is, ignore the
>> attribute or do what happens on validation failure.
>
>> > Any changes made offline/elsewhere, would
>> > result in the security.evm HMAC not verifying, causing a denial of
>> > service.  Unless of course the EVM HMAC key has somehow been
>> > leaked/exfiltrated from the kernel.
>> 
>> One scenario where this is likely to come up, and the kind of scenario
>> we are planning for is a filesystem on a usb stick is transported from
>> one machine to another.  One the second system it is mounted directly
>> (ideally) or through fuse running a copy of it's filesystem driver in
>> userspace to guard against filesystem implementation bugs.
>> 
>> So if that filesystem has security.evm xattrs if it appears things will
>> work very poorly.
>
> It could work for files with EVM signatures, obviously not with an HMAC.

Fair.

What policy would you recommend for (evm and ima) dealing with
filesystems that are mounted by a user namespace root and thus have
sb->s_user_ns != &init_user_ns?

Like Seth I am not much of an expert in the semantics of IMA and EVM.
I just know that ima calcualtes a hash of a file and does something with
it.  EVM caculcates a hash of other xattrs and the classic file
attributes.

That last hash as part of the work just merged I have updated to use
the uid and gid in s_user_ns instead of &init_user_ns, as on disk that
is the namespace the uid and gid are expected to be encoded in.

Eric

[Linux-ima-user] [PATCH 1144/1285] Replace numeric parameter like 0444 with macro

[Baole Ni <bao...@in...>]

I find that the developers often just specified the numeric value
when calling a macro which is defined with a parameter for access permission.
As we know, these numeric value for access permission have had the corresponding macro,
and that using macro can improve the robustness and readability of the code,
thus, I suggest replacing the numeric parameter with the macro.

Signed-off-by: Chuansheng Liu <chu...@in...>
Signed-off-by: Baole Ni <bao...@in...>
---
 security/integrity/ima/ima_crypto.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
index 38f2ed8..5858470 100644
--- a/security/integrity/ima/ima_crypto.c
+++ b/security/integrity/ima/ima_crypto.c
@@ -34,7 +34,7 @@ struct ahash_completion {
 
 /* minimum file size for ahash use */
 static unsigned long ima_ahash_minsize;
-module_param_named(ahash_minsize, ima_ahash_minsize, ulong, 0644);
+module_param_named(ahash_minsize, ima_ahash_minsize, ulong, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
 MODULE_PARM_DESC(ahash_minsize, "Minimum file size for ahash use");
 
 /* default is 0 - 1 page. */
@@ -61,7 +61,7 @@ static const struct kernel_param_ops param_ops_bufsize = {
 };
 #define param_check_bufsize(name, p) __param_check(name, p, unsigned int)
 
-module_param_named(ahash_bufsize, ima_bufsize, bufsize, 0644);
+module_param_named(ahash_bufsize, ima_bufsize, bufsize, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
 MODULE_PARM_DESC(ahash_bufsize, "Maximum ahash buffer size");
 
 static struct crypto_shash *ima_shash_tfm;
-- 
2.9.2

Re: [Linux-ima-user] [PATCH 0/1] EVM/IMA xattr hardening and unprivileged user ns mounts

[Mimi Z. <zo...@li...>]

On Mo, 2016-08-01 at 15:38 -0500, Eric W. Biederman wrote:
> Mimi Zohar <zo...@li...> writes:
> 
> > On Mo, 2016-08-01 at 08:19 -0500, Seth Forshee wrote:
> >> I was taking a look at IMA and EVM in the context of allowing mounts in
> >> user namespaces from users with only capabilities in that namespace
> >> (i.e. no global capabilities). As a result of that I found just a
> >> handful of places where the size of an xattr or the values it contains
> >> lack sufficient validation to protect against malicious xattrs. The
> >> fixes for these are worthwhile on their own, so I'm sending them in the
> >> following patch.
> >> 
> >> More generally though I'd like to request some guidance on how IMA and
> >> EVM should handle xattrs in mounts from globally-unprivileged users. For
> >> context, the intended use case for this is allowing fuse mounts in user
> >> namespace containers, and possibly mounts with a limited set of other
> >> filesystems in the future. All that would actually be needed to mount
> >> though are private user and mount namespaces.
> >> 
> >> Based on what I see, I suspect that we can treat the user ns mounts like
> >> any other mount. I don't see that this would cause harm as long as the
> >> filesystem can't trigger bugs in the xattr parsing, and if the system is
> >> configured for IMA enforcment this policy should also be applied to
> >> these mounts.
> >
> > "security.evm" is written as a result of one of the protected security
> > xattrs being written.   From your question, I assume that in this use
> > case scenario there are two concerns:
> > - parsing improper security.evm xattrs
> > - updating the protected security xattrs, causing the EVM xattr to
> > "bless" the change.
> >
> > The first issue you're addressing with the patch.  As for the second
> > issue,  as long as root in the namespace can not write security.xattrs,
> > there shouldn't be a problem.
> 
> Assume that it is allowed that root in a namespace will be allowed to
> write security xattrs for a filesystem it has mounted.  Certainly for
> the capability xattr this makes sense.

Yeah, I remember.  Having one capability xattr sounded interesting.
Have the capability patches been upstreamed?   To label files in a
container, root in the namespace will need to be able to write an IMA
xattr.  Whether it is possible to use a single xattr, is yet to be
determined.

> If understand you correctly is that the key used to sign the HMAC in
> security.evm is machine local.  As such I expect the proper semantics
> here will be to not update the security.evm xattrs on write of a
> filesystem with s_user_ns != &init_user_ns.

The EVM xattr is verified before it can be changed, to prevent making an
existing invalid hmac, valid.  If the namespace has the ability to
change any of the metadata included in the hmac, the EVM  hmac will need
to be updated with the file metadata, otherwise nobody will be able to
access the file.

> On read I am not certain what the right thing to do is, ignore the
> attribute or do what happens on validation failure.

> > Any changes made offline/elsewhere, would
> > result in the security.evm HMAC not verifying, causing a denial of
> > service.  Unless of course the EVM HMAC key has somehow been
> > leaked/exfiltrated from the kernel.
> 
> One scenario where this is likely to come up, and the kind of scenario
> we are planning for is a filesystem on a usb stick is transported from
> one machine to another.  One the second system it is mounted directly
> (ideally) or through fuse running a copy of it's filesystem driver in
> userspace to guard against filesystem implementation bugs.
> 
> So if that filesystem has security.evm xattrs if it appears things will
> work very poorly.

It could work for files with EVM signatures, obviously not with an HMAC.

Mimi 

Re: [Linux-ima-user] [PATCH 0/1] EVM/IMA xattr hardening and unprivileged user ns mounts

[Seth F. <set...@ca...>]

On Mon, Aug 01, 2016 at 03:38:02PM -0500, Eric W. Biederman wrote:
> Mimi Zohar <zo...@li...> writes:
> 
> > On Mo, 2016-08-01 at 08:19 -0500, Seth Forshee wrote:
> >> I was taking a look at IMA and EVM in the context of allowing mounts in
> >> user namespaces from users with only capabilities in that namespace
> >> (i.e. no global capabilities). As a result of that I found just a
> >> handful of places where the size of an xattr or the values it contains
> >> lack sufficient validation to protect against malicious xattrs. The
> >> fixes for these are worthwhile on their own, so I'm sending them in the
> >> following patch.
> >> 
> >> More generally though I'd like to request some guidance on how IMA and
> >> EVM should handle xattrs in mounts from globally-unprivileged users. For
> >> context, the intended use case for this is allowing fuse mounts in user
> >> namespace containers, and possibly mounts with a limited set of other
> >> filesystems in the future. All that would actually be needed to mount
> >> though are private user and mount namespaces.
> >> 
> >> Based on what I see, I suspect that we can treat the user ns mounts like
> >> any other mount. I don't see that this would cause harm as long as the
> >> filesystem can't trigger bugs in the xattr parsing, and if the system is
> >> configured for IMA enforcment this policy should also be applied to
> >> these mounts.
> >
> > "security.evm" is written as a result of one of the protected security
> > xattrs being written.   From your question, I assume that in this use
> > case scenario there are two concerns:
> > - parsing improper security.evm xattrs
> > - updating the protected security xattrs, causing the EVM xattr to
> > "bless" the change.
> >
> > The first issue you're addressing with the patch.  As for the second
> > issue,  as long as root in the namespace can not write security.xattrs,
> > there shouldn't be a problem.
> 
> Assume that it is allowed that root in a namespace will be allowed to
> write security xattrs for a filesystem it has mounted.  Certainly for
> the capability xattr this makes sense.
> 
> If understand you correctly is that the key used to sign the HMAC in
> security.evm is machine local.  As such I expect the proper semantics
> here will be to not update the security.evm xattrs on write of a
> filesystem with s_user_ns != &init_user_ns.

I'm in agreement on this.

On the one hand, as long as the kernel is treating the xattrs from
s_user_ns != &init_user_ns with the proper caution (as it must) this
doesn't seem like a major problem.

The other side though is that if the user can turn around and trick the
system into mounting the filesystem in init_user_ns those xattrs are now
trusted. And that's a serious enough concern that we shouldn't write out
the security.evm xattrs in the first place.

Seth

Re: [Linux-ima-user] [PATCH 0/1] EVM/IMA xattr hardening and unprivileged user ns mounts

[<ebi...@xm...>]

Mimi Zohar <zo...@li...> writes:

> On Mo, 2016-08-01 at 08:19 -0500, Seth Forshee wrote:
>> I was taking a look at IMA and EVM in the context of allowing mounts in
>> user namespaces from users with only capabilities in that namespace
>> (i.e. no global capabilities). As a result of that I found just a
>> handful of places where the size of an xattr or the values it contains
>> lack sufficient validation to protect against malicious xattrs. The
>> fixes for these are worthwhile on their own, so I'm sending them in the
>> following patch.
>> 
>> More generally though I'd like to request some guidance on how IMA and
>> EVM should handle xattrs in mounts from globally-unprivileged users. For
>> context, the intended use case for this is allowing fuse mounts in user
>> namespace containers, and possibly mounts with a limited set of other
>> filesystems in the future. All that would actually be needed to mount
>> though are private user and mount namespaces.
>> 
>> Based on what I see, I suspect that we can treat the user ns mounts like
>> any other mount. I don't see that this would cause harm as long as the
>> filesystem can't trigger bugs in the xattr parsing, and if the system is
>> configured for IMA enforcment this policy should also be applied to
>> these mounts.
>
> "security.evm" is written as a result of one of the protected security
> xattrs being written.   From your question, I assume that in this use
> case scenario there are two concerns:
> - parsing improper security.evm xattrs
> - updating the protected security xattrs, causing the EVM xattr to
> "bless" the change.
>
> The first issue you're addressing with the patch.  As for the second
> issue,  as long as root in the namespace can not write security.xattrs,
> there shouldn't be a problem.

Assume that it is allowed that root in a namespace will be allowed to
write security xattrs for a filesystem it has mounted.  Certainly for
the capability xattr this makes sense.

If understand you correctly is that the key used to sign the HMAC in
security.evm is machine local.  As such I expect the proper semantics
here will be to not update the security.evm xattrs on write of a
filesystem with s_user_ns != &init_user_ns.

On read I am not certain what the right thing to do is, ignore the
attribute or do what happens on validation failure.

> Any changes made offline/elsewhere, would
> result in the security.evm HMAC not verifying, causing a denial of
> service.  Unless of course the EVM HMAC key has somehow been
> leaked/exfiltrated from the kernel.

One scenario where this is likely to come up, and the kind of scenario
we are planning for is a filesystem on a usb stick is transported from
one machine to another.  One the second system it is mounted directly
(ideally) or through fuse running a copy of it's filesystem driver in
userspace to guard against filesystem implementation bugs.

So if that filesystem has security.evm xattrs if it appears things will
work very poorly.

Eric

Re: [Linux-ima-user] [PATCH 0/1] EVM/IMA xattr hardening and unprivileged user ns mounts

[Seth F. <set...@ca...>]

On Mon, Aug 01, 2016 at 03:36:26PM -0400, Mimi Zohar wrote:
> On Mo, 2016-08-01 at 08:19 -0500, Seth Forshee wrote:
> > I was taking a look at IMA and EVM in the context of allowing mounts in
> > user namespaces from users with only capabilities in that namespace
> > (i.e. no global capabilities). As a result of that I found just a
> > handful of places where the size of an xattr or the values it contains
> > lack sufficient validation to protect against malicious xattrs. The
> > fixes for these are worthwhile on their own, so I'm sending them in the
> > following patch.
> > 
> > More generally though I'd like to request some guidance on how IMA and
> > EVM should handle xattrs in mounts from globally-unprivileged users. For
> > context, the intended use case for this is allowing fuse mounts in user
> > namespace containers, and possibly mounts with a limited set of other
> > filesystems in the future. All that would actually be needed to mount
> > though are private user and mount namespaces.
> > 
> > Based on what I see, I suspect that we can treat the user ns mounts like
> > any other mount. I don't see that this would cause harm as long as the
> > filesystem can't trigger bugs in the xattr parsing, and if the system is
> > configured for IMA enforcment this policy should also be applied to
> > these mounts.
> 
> "security.evm" is written as a result of one of the protected security
> xattrs being written.   From your question, I assume that in this use
> case scenario there are two concerns:
> - parsing improper security.evm xattrs
> - updating the protected security xattrs, causing the EVM xattr to
> "bless" the change.

Also wheter there are any problems with mounting a filesystem which
contains valid security.evm xattrs which fail verification with the key
in the kernel, or any other scenarios I might not have thought of.

> The first issue you're addressing with the patch.  As for the second
> issue,  as long as root in the namespace can not write security.xattrs,
> there shouldn't be a problem.

We already have patches for the LSMs to make sure that they won't trust
security.* xattrs from user ns mounts, which have been merged into 4.8.
With that in place, would there still be a risk from letting namespace
root write those xattrs?

I'm not necessarily saying that we will let namespace root write those
xattrs, I'm just trying to understand if there's any risk beyond that of
letting the filesystem inject security policy via those xattrs.

> Any changes made offline/elsewhere, would
> result in the security.evm HMAC not verifying, causing a denial of
> service.  Unless of course the EVM HMAC key has somehow been
> leaked/exfiltrated from the kernel.

And by "denial of service" I take it you mean only with respect to that
filesystem? Or something else? When I read through the code I didn't get
the impression that HMAC verification failures would result in broader
problems.

Sorry to be asking fairly basic questions, but I'm not currently a user
of EVM so I want to confirm that my understanding of it is correct so
that we don't end up introducing any vulnerabilities.

Thanks,
Seth

Re: [Linux-ima-user] [PATCH 0/1] EVM/IMA xattr hardening and unprivileged user ns mounts

[Mimi Z. <zo...@li...>]

On Mo, 2016-08-01 at 08:19 -0500, Seth Forshee wrote:
> I was taking a look at IMA and EVM in the context of allowing mounts in
> user namespaces from users with only capabilities in that namespace
> (i.e. no global capabilities). As a result of that I found just a
> handful of places where the size of an xattr or the values it contains
> lack sufficient validation to protect against malicious xattrs. The
> fixes for these are worthwhile on their own, so I'm sending them in the
> following patch.
> 
> More generally though I'd like to request some guidance on how IMA and
> EVM should handle xattrs in mounts from globally-unprivileged users. For
> context, the intended use case for this is allowing fuse mounts in user
> namespace containers, and possibly mounts with a limited set of other
> filesystems in the future. All that would actually be needed to mount
> though are private user and mount namespaces.
> 
> Based on what I see, I suspect that we can treat the user ns mounts like
> any other mount. I don't see that this would cause harm as long as the
> filesystem can't trigger bugs in the xattr parsing, and if the system is
> configured for IMA enforcment this policy should also be applied to
> these mounts.

"security.evm" is written as a result of one of the protected security
xattrs being written.   From your question, I assume that in this use
case scenario there are two concerns:
- parsing improper security.evm xattrs
- updating the protected security xattrs, causing the EVM xattr to
"bless" the change.

The first issue you're addressing with the patch.  As for the second
issue,  as long as root in the namespace can not write security.xattrs,
there shouldn't be a problem.  Any changes made offline/elsewhere, would
result in the security.evm HMAC not verifying, causing a denial of
service.  Unless of course the EVM HMAC key has somehow been
leaked/exfiltrated from the kernel.

Mimi

> The only reasonable alternative I can come up with is to treat these
> mounts as though they don't support xattrs at all. In this case file
> appraisal would evaluate to INTEGRITY_UNKNOWN, and if enforcement were
> enabled executable and libraries would not be allowed on user ns mounts.
> 
> Thoughts?
> 
> Thanks,
> Seth

[Linux-ima-user] [PATCH 1/1] security/integrity: Harden against malformed xattrs

[Seth F. <set...@ca...>]

In general the handling of IMA/EVM xattrs is good, but I found
a few locations where either the xattr size or the value of the
type field in the xattr are not checked. Add a few simple checks
to these locations to prevent malformed or malicious xattrs from
causing problems.

Signed-off-by: Seth Forshee <set...@ca...>
---
 security/integrity/digsig.c           | 2 +-
 security/integrity/evm/evm_main.c     | 4 ++++
 security/integrity/ima/ima_appraise.c | 5 ++++-
 3 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c
index 4304372b323f..106e855e2d9d 100644
--- a/security/integrity/digsig.c
+++ b/security/integrity/digsig.c
@@ -51,7 +51,7 @@ static bool init_keyring __initdata;
 int integrity_digsig_verify(const unsigned int id, const char *sig, int siglen,
 			    const char *digest, int digestlen)
 {
-	if (id >= INTEGRITY_KEYRING_MAX)
+	if (id >= INTEGRITY_KEYRING_MAX || siglen < 2)
 		return -EINVAL;
 
 	if (!keyring[id]) {
diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
index b9e26288d30c..35ab453ce861 100644
--- a/security/integrity/evm/evm_main.c
+++ b/security/integrity/evm/evm_main.c
@@ -145,6 +145,10 @@ static enum integrity_status evm_verify_hmac(struct dentry *dentry,
 	/* check value type */
 	switch (xattr_data->type) {
 	case EVM_XATTR_HMAC:
+		if (xattr_len != sizeof(struct evm_ima_xattr_data)) {
+			evm_status = INTEGRITY_FAIL;
+			goto out;
+		}
 		rc = evm_calc_hmac(dentry, xattr_name, xattr_value,
 				   xattr_value_len, calc.digest);
 		if (rc)
diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index 1bcbc12e03d9..11a17073e8a2 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -130,6 +130,7 @@ enum hash_algo ima_get_hash_algo(struct evm_ima_xattr_data *xattr_value,
 				 int xattr_len)
 {
 	struct signature_v2_hdr *sig;
+	enum hash_algo ret;
 
 	if (!xattr_value || xattr_len < 2)
 		/* return default hash algo */
@@ -143,7 +144,9 @@ enum hash_algo ima_get_hash_algo(struct evm_ima_xattr_data *xattr_value,
 		return sig->hash_algo;
 		break;
 	case IMA_XATTR_DIGEST_NG:
-		return xattr_value->digest[0];
+		ret = xattr_value->digest[0];
+		if (ret < HASH_ALGO__LAST)
+			return ret;
 		break;
 	case IMA_XATTR_DIGEST:
 		/* this is for backward compatibility */
-- 
2.7.4

[Linux-ima-user] [PATCH 0/1] EVM/IMA xattr hardening and unprivileged user ns mounts

[Seth F. <set...@ca...>]

I was taking a look at IMA and EVM in the context of allowing mounts in
user namespaces from users with only capabilities in that namespace
(i.e. no global capabilities). As a result of that I found just a
handful of places where the size of an xattr or the values it contains
lack sufficient validation to protect against malicious xattrs. The
fixes for these are worthwhile on their own, so I'm sending them in the
following patch.

More generally though I'd like to request some guidance on how IMA and
EVM should handle xattrs in mounts from globally-unprivileged users. For
context, the intended use case for this is allowing fuse mounts in user
namespace containers, and possibly mounts with a limited set of other
filesystems in the future. All that would actually be needed to mount
though are private user and mount namespaces.

Based on what I see, I suspect that we can treat the user ns mounts like
any other mount. I don't see that this would cause harm as long as the
filesystem can't trigger bugs in the xattr parsing, and if the system is
configured for IMA enforcment this policy should also be applied to
these mounts.

The only reasonable alternative I can come up with is to treat these
mounts as though they don't support xattrs at all. In this case file
appraisal would evaluate to INTEGRITY_UNKNOWN, and if enforcement were
enabled executable and libraries would not be allowed on user ns mounts.

Thoughts?

Thanks,
Seth

Re: [Linux-ima-user] copying files with security.ima xattr

[Patrick O. <pat...@in...>]

On Mon, 2016-07-18 at 08:08 -0400, Mimi Zohar wrote:
> Commit c68ed80 "ima: limit file hash setting by user to fix and log
> modes" is a form of system hardening.  Before reverting it, let's see if
> there is another option.
> 
> Could we summarize the problem as:
> - the kernel prevents writing security.ima hashes.
> - the kernel only writes security.ima hashes for files that are in
> policy.
> - the userspace tool doesn't know what is in/out of policy.
> - the userspace tool doesn't differentiate between hashes and
> signatures.
> - the boot process doesn't permit changing the boot command line options
> (eg. fix mode).
> - the update tool compares file data and metadata with those on the
> server

Correct.

> Currently, you said there is no IMA policy.  Would it make sense to
> invert that to all files are in policy?  That way the kernel would write
> the file hash as security.ima on all files (that are not signed).
>
> The file metadata would match the update server.  Only the userspace
> tool (bsdtar) would need to be modified to selectively write xattrs  -
> either no security.ima xattrs or only security.ima file signatures.

This would work, but I don't like the prospect of having to patch bsdtar
like that. Including or excluding xattrs by name is conceptually okay
(GNU tar has that), but filtering by value of an xattr adds
domain-specific knowledge to bsdtar which just doesn't belong there.

Perhaps some custom tool build on top of libarchive would be okay -
basically a custom "bsdtar -xf -".

-- 
Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.

Re: [Linux-ima-user] copying files with security.ima xattr

[Mimi Z. <zo...@li...>]

On Mo, 2016-07-18 at 09:22 +0200, Patrick Ohly wrote:
> On Tue, 2016-07-12 at 09:00 -0400, Mimi Zohar wrote:
> > On Thu, 2016-07-07 at 15:02 +0200, Patrick Ohly wrote:
> > > Hello!
> > > 
> > > I just noticed some unexpected behavior. Kernel is 4.4.11, with IMA
> > > enabled in the build configuration.
> > > 
> > > After booting, copying files which have a hash in security.ima (like
> > > 0x0196d920f04ec85c33438117740d5dcae7bbdbfe3a) with rsync or bsdtar fails
> > > in fsetxattr with permission denied. From bsdtar:
> > > 
> > > open("build", O_WRONLY|O_CREAT|O_EXCL|O_LARGEFILE|O_CLOEXEC, 0644) = 3
> > > fcntl64(3, F_GETFD)                     = 0x1 (flags FD_CLOEXEC)
> > > write(3, "-----------------------\nBuild Co"..., 2022) = 2022
> > > ...
> > > fchown32(3, 0, 0)                       = 0
> > > utimensat(3, NULL, [{1467894919, 0}, {1467894885, 0}], 0) = 0
> > > fsetxattr(3, "security.SMACK64", "System::Shared", 14, 0) = 0
> > > fsetxattr(3, "security.ima", "\1\226\331 \360N\310\\3C\201\27t\r]\312\347\273\333\376:", 21, 0) = -1 EPERM (Operation not permitted)
> > > close(3)                                = 0
> > > 
> > > It does not matter whether an IMA policy has been loaded or not.
> > > 
> > > fsetxattr() succeeds if the security.ima value is a signed hash.
> > 
> > Right, there's no good reason for explicitly writing the hash.
> 
> Yes, if the file is under the current policy, then it is redundant. But
> that implies that the user space tool knows exactly whether that
> operation is redundant. That's far from trivial, isn't it?
> 
> And that assumes that the tool in question knows about IMA. bsdtar
> doesn't support IMA, only xattrs. Therefore it will always write
> security.ima, regardless what the value is in the archive that it
> extracts and what the current policy on the target filesystem might be.
> 
> One could argue that the input archive shouldn't have the security.ima
> set when it is not needed, but that (in our case) breaks another use
> case, which is a feature of our update tool which compares files on disk
> (including their meta data, which includes xattrs) against files stored
> on the server. So the server side also needs to have the security.ima.
> 
> >   If the
> > file is in policy, on __fput() the file will automatically be hashed.
> > Only writing a file signature as security.ima xattr is permitted.
> > Refer to commit c68ed80 "ima: limit file hash setting by user to fix and
> > log modes".
> 
> That explains the mechanism, but the commit comes with no further
> explanations. Why the "should not be manually set", i.e. what's the
> downside of allowing it?
> 
> There's also the problem that the patch prevents writing the hash in
> situations where there is not active IMA policy. Your argument was that
> letting the user write the hash is redundant because the kernel will do
> it. But that's not true when the file isn't under the current policy (in
> our case, because no policy has been loaded at all), and still the
> kernel prevents writing the hash.
> 
> > > We ran into this while working on an installer which runs from the
> > > initramfs and copies the content of the rootfs from an USB stick to some
> > > other media, like internal flash.
> > > 
> > > Ideally, that installer should get started before loading an IMA policy.
> > > That way, writing files will be faster (no hashing). But that doesn't
> > > work because the security.ima of hashed files cannot be set.
> > 
> > (I assume you meant signatures can not be written on unhashed files.)   
> 
> No, I meant writing a hash, like 0x0196..e3a in my initial example.
> 
> > > It works after loading an IMA policy which hashes the files on the
> > > target partition: rsync or bsdtar try write security.ima, print an
> > > error, continue, and the kernel sets the same hash that the tools
> > > weren't allowed to set earlier.
> > 
> > Exactly, no need for the userspace application to write the hash.
> 
> There is a need: OS gets installed, all hashes are set as required, then
> OS boots immediately, without going into "fix" mode. Without writing the
> hashes, files would be unusable.
> 
> We could boot into fix mode before installing, but that implies booting
> with different command line parameters, which isn't possible in our
> current setup: we have no boot loader, the firmware directly loads an
> EFI blob which contains kernel, command line parameters and initramfs.
> 
> Our current image then either installs to internal storage or boots from
> USB, depending on some files in the VFAT partition or user interaction.
> But that check happens after the kernel started and thus switching modes
> between fix and enforcement isn't possible anymore.

> > > Besides the slower copy operation, we also get errors that we need to
> > > ignore because (most likely) they are harmless. Not nice, because at
> > > some point there might be genuine errors which also get ignored.
> > 
> > Other than writing file hashes or copying security.evm warnings are
> > there others?
> 
> We are not using security.evm yet (yes, I know, it's a gap that we need
> to close). So right now, all errors are about writing security.ima.

Commit c68ed80 "ima: limit file hash setting by user to fix and log
modes" is a form of system hardening.  Before reverting it, let's see if
there is another option.

Could we summarize the problem as:
- the kernel prevents writing security.ima hashes.
- the kernel only writes security.ima hashes for files that are in
policy.
- the userspace tool doesn't know what is in/out of policy.
- the userspace tool doesn't differentiate between hashes and
signatures.
- the boot process doesn't permit changing the boot command line options
(eg. fix mode).
- the update tool compares file data and metadata with those on the
server

Currently, you said there is no IMA policy.  Would it make sense to
invert that to all files are in policy?  That way the kernel would write
the file hash as security.ima on all files (that are not signed).

The file metadata would match the update server.  Only the userspace
tool (bsdtar) would need to be modified to selectively write xattrs  -
either no security.ima xattrs or only security.ima file signatures.

Mimi

Re: [Linux-ima-user] copying files with security.ima xattr

[Patrick O. <pat...@in...>]

On Tue, 2016-07-12 at 09:00 -0400, Mimi Zohar wrote:
> On Thu, 2016-07-07 at 15:02 +0200, Patrick Ohly wrote:
> > Hello!
> > 
> > I just noticed some unexpected behavior. Kernel is 4.4.11, with IMA
> > enabled in the build configuration.
> > 
> > After booting, copying files which have a hash in security.ima (like
> > 0x0196d920f04ec85c33438117740d5dcae7bbdbfe3a) with rsync or bsdtar fails
> > in fsetxattr with permission denied. From bsdtar:
> > 
> > open("build", O_WRONLY|O_CREAT|O_EXCL|O_LARGEFILE|O_CLOEXEC, 0644) = 3
> > fcntl64(3, F_GETFD)                     = 0x1 (flags FD_CLOEXEC)
> > write(3, "-----------------------\nBuild Co"..., 2022) = 2022
> > ...
> > fchown32(3, 0, 0)                       = 0
> > utimensat(3, NULL, [{1467894919, 0}, {1467894885, 0}], 0) = 0
> > fsetxattr(3, "security.SMACK64", "System::Shared", 14, 0) = 0
> > fsetxattr(3, "security.ima", "\1\226\331 \360N\310\\3C\201\27t\r]\312\347\273\333\376:", 21, 0) = -1 EPERM (Operation not permitted)
> > close(3)                                = 0
> > 
> > It does not matter whether an IMA policy has been loaded or not.
> > 
> > fsetxattr() succeeds if the security.ima value is a signed hash.
> 
> Right, there's no good reason for explicitly writing the hash.

Yes, if the file is under the current policy, then it is redundant. But
that implies that the user space tool knows exactly whether that
operation is redundant. That's far from trivial, isn't it?

And that assumes that the tool in question knows about IMA. bsdtar
doesn't support IMA, only xattrs. Therefore it will always write
security.ima, regardless what the value is in the archive that it
extracts and what the current policy on the target filesystem might be.

One could argue that the input archive shouldn't have the security.ima
set when it is not needed, but that (in our case) breaks another use
case, which is a feature of our update tool which compares files on disk
(including their meta data, which includes xattrs) against files stored
on the server. So the server side also needs to have the security.ima.

>   If the
> file is in policy, on __fput() the file will automatically be hashed.
> Only writing a file signature as security.ima xattr is permitted.
> Refer to commit c68ed80 "ima: limit file hash setting by user to fix and
> log modes".

That explains the mechanism, but the commit comes with no further
explanations. Why the "should not be manually set", i.e. what's the
downside of allowing it?

There's also the problem that the patch prevents writing the hash in
situations where there is not active IMA policy. Your argument was that
letting the user write the hash is redundant because the kernel will do
it. But that's not true when the file isn't under the current policy (in
our case, because no policy has been loaded at all), and still the
kernel prevents writing the hash.

> > We ran into this while working on an installer which runs from the
> > initramfs and copies the content of the rootfs from an USB stick to some
> > other media, like internal flash.
> > 
> > Ideally, that installer should get started before loading an IMA policy.
> > That way, writing files will be faster (no hashing). But that doesn't
> > work because the security.ima of hashed files cannot be set.
> 
> (I assume you meant signatures can not be written on unhashed files.)   

No, I meant writing a hash, like 0x0196..e3a in my initial example.

> > It works after loading an IMA policy which hashes the files on the
> > target partition: rsync or bsdtar try write security.ima, print an
> > error, continue, and the kernel sets the same hash that the tools
> > weren't allowed to set earlier.
> 
> Exactly, no need for the userspace application to write the hash.

There is a need: OS gets installed, all hashes are set as required, then
OS boots immediately, without going into "fix" mode. Without writing the
hashes, files would be unusable.

We could boot into fix mode before installing, but that implies booting
with different command line parameters, which isn't possible in our
current setup: we have no boot loader, the firmware directly loads an
EFI blob which contains kernel, command line parameters and initramfs.

Our current image then either installs to internal storage or boots from
USB, depending on some files in the VFAT partition or user interaction.
But that check happens after the kernel started and thus switching modes
between fix and enforcement isn't possible anymore.

> > Besides the slower copy operation, we also get errors that we need to
> > ignore because (most likely) they are harmless. Not nice, because at
> > some point there might be genuine errors which also get ignored.
> 
> Other than writing file hashes or copying security.evm warnings are
> there others?

We are not using security.evm yet (yes, I know, it's a gap that we need
to close). So right now, all errors are about writing security.ima.

-- 
Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.

Re: [Linux-ima-user] copying files with security.ima xattr

[Mimi Z. <zo...@li...>]

On Thu, 2016-07-07 at 15:02 +0200, Patrick Ohly wrote:
> Hello!
> 
> I just noticed some unexpected behavior. Kernel is 4.4.11, with IMA
> enabled in the build configuration.
> 
> After booting, copying files which have a hash in security.ima (like
> 0x0196d920f04ec85c33438117740d5dcae7bbdbfe3a) with rsync or bsdtar fails
> in fsetxattr with permission denied. From bsdtar:
> 
> open("build", O_WRONLY|O_CREAT|O_EXCL|O_LARGEFILE|O_CLOEXEC, 0644) = 3
> fcntl64(3, F_GETFD)                     = 0x1 (flags FD_CLOEXEC)
> write(3, "-----------------------\nBuild Co"..., 2022) = 2022
> ...
> fchown32(3, 0, 0)                       = 0
> utimensat(3, NULL, [{1467894919, 0}, {1467894885, 0}], 0) = 0
> fsetxattr(3, "security.SMACK64", "System::Shared", 14, 0) = 0
> fsetxattr(3, "security.ima", "\1\226\331 \360N\310\\3C\201\27t\r]\312\347\273\333\376:", 21, 0) = -1 EPERM (Operation not permitted)
> close(3)                                = 0
> 
> It does not matter whether an IMA policy has been loaded or not.
> 
> fsetxattr() succeeds if the security.ima value is a signed hash.

Right, there's no good reason for explicitly writing the hash.  If the
file is in policy, on __fput() the file will automatically be hashed.
Only writing a file signature as security.ima xattr is permitted.
Refer to commit c68ed80 "ima: limit file hash setting by user to fix and
log modes".

> We ran into this while working on an installer which runs from the
> initramfs and copies the content of the rootfs from an USB stick to some
> other media, like internal flash.
> 
> Ideally, that installer should get started before loading an IMA policy.
> That way, writing files will be faster (no hashing). But that doesn't
> work because the security.ima of hashed files cannot be set.

(I assume you meant signatures can not be written on unhashed files.)   

Writing the file signature on new files, before __fput() is called,
would prevent the file hash from being calculated.  For example, GNU tar
writes the file signature before the file data.   (Refer to commit
05d1a71 "ima: add support for creating files using the mknodat syscall")

> It works after loading an IMA policy which hashes the files on the
> target partition: rsync or bsdtar try write security.ima, print an
> error, continue, and the kernel sets the same hash that the tools
> weren't allowed to set earlier.

Exactly, no need for the userspace application to write the hash.

> Besides the slower copy operation, we also get errors that we need to
> ignore because (most likely) they are harmless. Not nice, because at
> some point there might be genuine errors which also get ignored.

Other than writing file hashes or copying security.evm warnings are
there others?

> Would it make sense to always allow setting security.ima when the caller
> has the right priviliges? If someone sets a broken value, that's their
> problem.

The signature itself is currently not verified, before writing it as an
xattr, but the existing security.evm is checked, before allowing any
protected xattrs to be written.  Otherwise any change would  incorporate
any other intended/unintended changes in the security.evm xattr.

For those files included in the IMA policy, either a file hash or
signature needs to be written before the "new" file status is removed
(eg. __fput).   The only exceptions are for "fix" or "log" modes. 

Mimi