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

Re: [Linux-ima-user] [PATCH] evm: add exceptions for inodes without getxattr() support.

[Mikhail K. <vie...@vi...>]

В Wed, 04 Jan 2017 09:15:47 -0500
Mimi Zohar <zo...@li...> пишет:

> On Wed, 2017-01-04 at 05:49 +0300, Mikhail Kurinnoi wrote:
> > В Tue, 03 Jan 2017 18:42:34 -0500
> > Mimi Zohar <zo...@li...> пишет:
> >   
> > > On Tue, 2017-01-03 at 21:03 +0300, Mikhail Kurinnoi wrote:  
> > > > В Tue, 03 Jan 2017 08:13:42 -0500
> > > > Mimi Zohar <zo...@li...> пишет:    
> > >   
> > > > > > Let me fix this misunderstanding by tests.
> > > > > > Test with default code:
> > > > > > 
> > > > > > # touch /var/tmp/test
> > > > > > # getfattr -m . -d -e hex /var/tmp/test
> > > > > > (no output, file don't have any xattrs)
> > > > > > # setfattr -n security.foo -v "123" /var/tmp/test      
> > > > > 
> > > > > Right, the ops exist.
> > > > >     
> > > > > > # getfattr -m . -d -e hex /var/tmp/test
> > > > > >     getfattr: Removing leading '/' from absolute path names
> > > > > >     # file: var/tmp/test
> > > > > >     security.foo=0x313233
> > > > > > # chmod 666 /var/tmp/test
> > > > > >     chmod: changing permissions of '/var/tmp/test':
> > > > > > Operation not permitted      
> > > > > 
> > > > > All files in the IMA appraise policy require an IMA hash or
> > > > > signature. With the "ima_appraise_policy", all files owned by
> > > > > root must be appraised either based on the file hash or file
> > > > > signature.  Other policies might require file signatures.    
> > > > 
> > > > 
> > > > I think the issue also related to evm_calc_hmac_or_hash().  
> 
> > > > Before cycle, we set error to -ENODATA. In the same time, if we
> > > > don't have any xattrs for protection, error will never changed
> > > > from -ENODATA to 0. So, we will return -ENODATA error into
> > > > evm_update_evmxattr(). In this case, evm_update_evmxattr() will
> > > > not protect uid/gid/mode by security.evm, but remove
> > > > security.evm instead, if it's available.    
> > > 
> > > For files not in policy (there aren't any xattrs), I was able to
> > > do the chmod without problems.
> > > 
> > > # touch /var/tmp/test
> > > # getfattr -m ^security -e hex --dump /var/tmp/test
> > > # chmod 666 /var/tmp/test
> > > # getfattr -m ^security -e hex --dump /var/tmp/test
> > > # ls -lat /var/tmp/test
> > > -rw-rw-rw- 1 root root 0 Jan  3 18:16 /var/tmp/test
> > >
> > > We've already shown that you can write security xattrs
> > > (security.foo). So it isn't an issue that the xattr ops don't
> > > exist.  Why isn't it returning INTEGRITY_NOXATTRS?  
> > 
> > I have exactly same question, and I still don't have answer for it.
> > It must returning INTEGRITY_NOXATTRS or don't be able set any
> > "security." xattrs. 
> > Plus, we should have -EOPNOTSUPP error returned by
> > vfs_getxattr_alloc() if we have xattrs support issue, I don't
> > understand why -EOPNOTSUPP error returned by
> > evm_find_protected_xattrs() instead.  
> 
>  Unlike SELinux, it seems that kernel/commoncaps:
> get_vfs_caps_from_disk() treats -ENODATA and -EOPNOTSUPP the same, as
> though there is no data.  Could you see if modifying
> evm_find_protected_xattrs() like below fixes the problem?
> 
>                if (error < 0) {
>                         if (error == -ENODATA || error == -EOPNOTSUPP)
>                                 continue;
>                         return error;
>                 }
>                 count++;
> 

Mimi, I can confirm that this code modification fix the issue.
I can create now files without xattrs and use chmod on both - tmpfs
and ext4 partitions covered by "dont_appraise" policy.



-- 
Best regards,
Mikhail Kurinnoi

Re: [Linux-ima-user] [PATCH] evm: add exceptions for inodes without getxattr() support.

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

On Wed, 2017-01-04 at 05:49 +0300, Mikhail Kurinnoi wrote:
> В Tue, 03 Jan 2017 18:42:34 -0500
> Mimi Zohar <zo...@li...> пишет:
> 
> > On Tue, 2017-01-03 at 21:03 +0300, Mikhail Kurinnoi wrote:
> > > В Tue, 03 Jan 2017 08:13:42 -0500
> > > Mimi Zohar <zo...@li...> пишет:  
> > 
> > > > > Let me fix this misunderstanding by tests.
> > > > > Test with default code:
> > > > > 
> > > > > # touch /var/tmp/test
> > > > > # getfattr -m . -d -e hex /var/tmp/test
> > > > > (no output, file don't have any xattrs)
> > > > > # setfattr -n security.foo -v "123" /var/tmp/test    
> > > > 
> > > > Right, the ops exist.
> > > >   
> > > > > # getfattr -m . -d -e hex /var/tmp/test
> > > > >     getfattr: Removing leading '/' from absolute path names
> > > > >     # file: var/tmp/test
> > > > >     security.foo=0x313233
> > > > > # chmod 666 /var/tmp/test
> > > > >     chmod: changing permissions of '/var/tmp/test': Operation
> > > > > not permitted    
> > > > 
> > > > All files in the IMA appraise policy require an IMA hash or
> > > > signature. With the "ima_appraise_policy", all files owned by
> > > > root must be appraised either based on the file hash or file
> > > > signature.  Other policies might require file signatures.  
> > > 
> > > 
> > > I think the issue also related to evm_calc_hmac_or_hash().

> > > Before cycle, we set error to -ENODATA. In the same time, if we
> > > don't have any xattrs for protection, error will never changed from
> > > -ENODATA to 0. So, we will return -ENODATA error into
> > > evm_update_evmxattr(). In this case, evm_update_evmxattr() will not
> > > protect uid/gid/mode by security.evm, but remove security.evm
> > > instead, if it's available.  
> > 
> > For files not in policy (there aren't any xattrs), I was able to do
> > the chmod without problems.
> > 
> > # touch /var/tmp/test
> > # getfattr -m ^security -e hex --dump /var/tmp/test
> > # chmod 666 /var/tmp/test
> > # getfattr -m ^security -e hex --dump /var/tmp/test
> > # ls -lat /var/tmp/test
> > -rw-rw-rw- 1 root root 0 Jan  3 18:16 /var/tmp/test
> >
> > We've already shown that you can write security xattrs (security.foo).
> > So it isn't an issue that the xattr ops don't exist.  Why isn't it
> > returning INTEGRITY_NOXATTRS?
> 
> I have exactly same question, and I still don't have answer for it. It
> must returning INTEGRITY_NOXATTRS or don't be able set any "security."
> xattrs. 
> Plus, we should have -EOPNOTSUPP error returned by
> vfs_getxattr_alloc() if we have xattrs support issue, I don't
> understand why -EOPNOTSUPP error returned by
> evm_find_protected_xattrs() instead.

 Unlike SELinux, it seems that kernel/commoncaps:
get_vfs_caps_from_disk() treats -ENODATA and -EOPNOTSUPP the same, as
though there is no data.  Could you see if modifying
evm_find_protected_xattrs() like below fixes the problem?

               if (error < 0) {
                        if (error == -ENODATA || error == -EOPNOTSUPP)
                                continue;
                        return error;
                }
                count++;

Thanks!

Mimi


> I look at EVM code carefully and found, that this code was created to
> be used with SELinux/SMACK only, just look at
> security_inode_init_security(), evm_inode_init_security(),
> evm_init_hmac() or evm_calc_hmac_or_hash() - all this code count on
> SELinux/SMAC hooks during inode creation (and prevent EVM inode init
> if hooks is not set) and request at least one protected xattr that
> should be on inode all the time. This could be changed in easy way and
> make code universal:
> 1) We should correct evm_calc_hmac_or_hash() in order to work without
> protected xattrs, don't return -ENODATA if cycle is over successfully.
> 2) EVM should also work with new inode in
> security_inode_init_security() even if we don't have SELinux or any
> other xattr based lsm. Please note, security_inode_init_security()
> don't return -EOPNOTSUPP error, it return 0 instead. So, we safe to
> call evm_inode_init_security() if inode_init_security hook return
> -EOPNOTSUPP.
> 3) evm_inode_init_security() and evm_init_hmac() should count, that we
> may don't provide lsm_xattr in case we don't have xattr based lsm
> enabled.
> 
> After this changes, EVM will work with files covered by
> "dont_appraise" policy or not in policy in the same way as it work
> with SELinux enabled.
> This patch fix issue in the way that EVM work properly now for me too
> even with SELinux disabled. I did all previous tests and don't faced
> with any issues, I also don't see any errors in syslog now.
> 
> 
> Signed-off-by: Mikhail Kurinnoi <vie...@vi...>
> 
> --- a/security/integrity/evm/evm_crypto.c
> +++ b/security/integrity/evm/evm_crypto.c
> @@ -227,6 +227,7 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry,
>  		xattr_size = size;
>  		crypto_shash_update(desc, (const u8 *)xattr_value, xattr_size);
>  	}
> +	error = 0;
>  	if (type == EVM_IMA_XATTR_DIGSIG)
>  		hmac_add_misc_digsig(desc, inode, digest);
>  	else
> @@ -292,7 +293,8 @@ int evm_init_hmac(struct inode *inode
>  		return PTR_ERR(desc);
>  	}
>  
> -	crypto_shash_update(desc, lsm_xattr->value, lsm_xattr->value_len);
> +	if (lsm_xattr)
> +		crypto_shash_update(desc, lsm_xattr->value, lsm_xattr->value_len);
>  	hmac_add_misc(desc, inode, hmac_val);
>  	kfree(desc);
>  	return 0;
> --- a/security/integrity/evm/evm_main.c
> +++ b/security/integrity/evm/evm_main.c
> @@ -470,7 +526,7 @@ int evm_inode_init_security(struct inode *inode,
>  	struct evm_ima_xattr_data *xattr_data;
>  	int rc;
>  
> -	if (!evm_initialized || !evm_protected_xattr(lsm_xattr->name))
> +	if (!evm_initialized || (lsm_xattr && !evm_protected_xattr(lsm_xattr->name)))
>  		return 0;
>  
>  	xattr_data = kzalloc(sizeof(*xattr_data), GFP_NOFS);
> --- a/security/security.c
> +++ b/security/security.c
> @@ -384,10 +384,19 @@ int security_inode_init_security(struct inode *inode
>  						&lsm_xattr->name,
>  						&lsm_xattr->value,
>  						&lsm_xattr->value_len);
> -	if (ret)
> +	if (ret && (ret != -EOPNOTSUPP))
>  		goto out;
> -
> -	evm_xattr = lsm_xattr + 1;
> +	
> +	if (ret == -EOPNOTSUPP) {
> +		if (lsm_xattr->value != NULL)
> +			kfree(lsm_xattr->value);
> +		memset(new_xattrs, 0, sizeof(new_xattrs));
> +		lsm_xattr = NULL;
> +		evm_xattr = new_xattrs;
> +	}
> +	else
> +		evm_xattr = lsm_xattr + 1;
> +	
>  	ret = evm_inode_init_security(inode, lsm_xattr, evm_xattr);
>  	if (ret)
>  		goto out;
> 

Re: [Linux-ima-user] [PATCH] evm: add exceptions for inodes without getxattr() support.

[Mikhail K. <vie...@vi...>]

В Tue, 03 Jan 2017 18:42:34 -0500
Mimi Zohar <zo...@li...> пишет:

> On Tue, 2017-01-03 at 21:03 +0300, Mikhail Kurinnoi wrote:
> > В Tue, 03 Jan 2017 08:13:42 -0500
> > Mimi Zohar <zo...@li...> пишет:  
> 
> > > > Let me fix this misunderstanding by tests.
> > > > Test with default code:
> > > > 
> > > > # touch /var/tmp/test
> > > > # getfattr -m . -d -e hex /var/tmp/test
> > > > (no output, file don't have any xattrs)
> > > > # setfattr -n security.foo -v "123" /var/tmp/test    
> > > 
> > > Right, the ops exist.
> > >   
> > > > # getfattr -m . -d -e hex /var/tmp/test
> > > >     getfattr: Removing leading '/' from absolute path names
> > > >     # file: var/tmp/test
> > > >     security.foo=0x313233
> > > > # chmod 666 /var/tmp/test
> > > >     chmod: changing permissions of '/var/tmp/test': Operation
> > > > not permitted    
> > > 
> > > All files in the IMA appraise policy require an IMA hash or
> > > signature. With the "ima_appraise_policy", all files owned by
> > > root must be appraised either based on the file hash or file
> > > signature.  Other policies might require file signatures.  
> > 
> > 
> > I think the issue also related to evm_calc_hmac_or_hash().
> > Before cycle, we set error to -ENODATA. In the same time, if we
> > don't have any xattrs for protection, error will never changed from
> > -ENODATA to 0. So, we will return -ENODATA error into
> > evm_update_evmxattr(). In this case, evm_update_evmxattr() will not
> > protect uid/gid/mode by security.evm, but remove security.evm
> > instead, if it's available.  
> 
> For files not in policy (there aren't any xattrs), I was able to do
> the chmod without problems.
> 
> # touch /var/tmp/test
> # getfattr -m ^security -e hex --dump /var/tmp/test
> # chmod 666 /var/tmp/test
> # getfattr -m ^security -e hex --dump /var/tmp/test
> # ls -lat /var/tmp/test
> -rw-rw-rw- 1 root root 0 Jan  3 18:16 /var/tmp/test
>
> We've already shown that you can write security xattrs (security.foo).
> So it isn't an issue that the xattr ops don't exist.  Why isn't it
> returning INTEGRITY_NOXATTRS?

I have exactly same question, and I still don't have answer for it. It must returning INTEGRITY_NOXATTRS or don't be able set any "security." xattrs. 
Plus, we should have -EOPNOTSUPP error returned by vfs_getxattr_alloc() if we have xattrs support issue, I don't understand why -EOPNOTSUPP error returned by evm_find_protected_xattrs() instead.


I look at EVM code carefully and found, that this code was created to be used with SELinux/SMACK only, just look at security_inode_init_security(), evm_inode_init_security(), evm_init_hmac() or evm_calc_hmac_or_hash() - all this code count on SELinux/SMAC hooks during inode creation (and prevent EVM inode init if hooks is not set) and request at least one protected xattr that should be on inode all the time. This could be changed in easy way and make code universal:
1) We should correct evm_calc_hmac_or_hash() in order to work without protected xattrs, don't return -ENODATA if cycle is over successfully.
2) EVM should also work with new inode in security_inode_init_security() even if we don't have SELinux or any other xattr based lsm. Please note, security_inode_init_security() don't return -EOPNOTSUPP error, it return 0 instead. So, we safe to call evm_inode_init_security() if inode_init_security hook return -EOPNOTSUPP.
3) evm_inode_init_security() and evm_init_hmac() should count, that we may don't provide lsm_xattr in case we don't have xattr based lsm enabled.

After this changes, EVM will work with files covered by "dont_appraise" policy or not in policy in the same way as it work with SELinux enabled.
This patch fix issue in the way that EVM work properly now for me too even with SELinux disabled. I did all previous tests and don't faced with any issues, I also don't see any errors in syslog now.


Signed-off-by: Mikhail Kurinnoi <vie...@vi...>

--- a/security/integrity/evm/evm_crypto.c
+++ b/security/integrity/evm/evm_crypto.c
@@ -227,6 +227,7 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry,
 		xattr_size = size;
 		crypto_shash_update(desc, (const u8 *)xattr_value, xattr_size);
 	}
+	error = 0;
 	if (type == EVM_IMA_XATTR_DIGSIG)
 		hmac_add_misc_digsig(desc, inode, digest);
 	else
@@ -292,7 +293,8 @@ int evm_init_hmac(struct inode *inode
 		return PTR_ERR(desc);
 	}
 
-	crypto_shash_update(desc, lsm_xattr->value, lsm_xattr->value_len);
+	if (lsm_xattr)
+		crypto_shash_update(desc, lsm_xattr->value, lsm_xattr->value_len);
 	hmac_add_misc(desc, inode, hmac_val);
 	kfree(desc);
 	return 0;
--- a/security/integrity/evm/evm_main.c
+++ b/security/integrity/evm/evm_main.c
@@ -470,7 +526,7 @@ int evm_inode_init_security(struct inode *inode,
 	struct evm_ima_xattr_data *xattr_data;
 	int rc;
 
-	if (!evm_initialized || !evm_protected_xattr(lsm_xattr->name))
+	if (!evm_initialized || (lsm_xattr && !evm_protected_xattr(lsm_xattr->name)))
 		return 0;
 
 	xattr_data = kzalloc(sizeof(*xattr_data), GFP_NOFS);
--- a/security/security.c
+++ b/security/security.c
@@ -384,10 +384,19 @@ int security_inode_init_security(struct inode *inode
 						&lsm_xattr->name,
 						&lsm_xattr->value,
 						&lsm_xattr->value_len);
-	if (ret)
+	if (ret && (ret != -EOPNOTSUPP))
 		goto out;
-
-	evm_xattr = lsm_xattr + 1;
+	
+	if (ret == -EOPNOTSUPP) {
+		if (lsm_xattr->value != NULL)
+			kfree(lsm_xattr->value);
+		memset(new_xattrs, 0, sizeof(new_xattrs));
+		lsm_xattr = NULL;
+		evm_xattr = new_xattrs;
+	}
+	else
+		evm_xattr = lsm_xattr + 1;
+	
 	ret = evm_inode_init_security(inode, lsm_xattr, evm_xattr);
 	if (ret)
 		goto out;

Re: [Linux-ima-user] [PATCH] evm: add exceptions for inodes without getxattr() support.

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

On Tue, 2017-01-03 at 21:03 +0300, Mikhail Kurinnoi wrote:
> В Tue, 03 Jan 2017 08:13:42 -0500
> Mimi Zohar <zo...@li...> пишет:

> > > Let me fix this misunderstanding by tests.
> > > Test with default code:
> > > 
> > > # touch /var/tmp/test
> > > # getfattr -m . -d -e hex /var/tmp/test
> > > (no output, file don't have any xattrs)
> > > # setfattr -n security.foo -v "123" /var/tmp/test  
> > 
> > Right, the ops exist.
> > 
> > > # getfattr -m . -d -e hex /var/tmp/test
> > >     getfattr: Removing leading '/' from absolute path names
> > >     # file: var/tmp/test
> > >     security.foo=0x313233
> > > # chmod 666 /var/tmp/test
> > >     chmod: changing permissions of '/var/tmp/test': Operation not
> > > permitted  
> > 
> > All files in the IMA appraise policy require an IMA hash or signature.
> > With the "ima_appraise_policy", all files owned by root must be
> > appraised either based on the file hash or file signature.  Other
> > policies might require file signatures.
> 
> 
> I think the issue also related to evm_calc_hmac_or_hash().
> Before cycle, we set error to -ENODATA. In the same time, if we don't
> have any xattrs for protection, error will never changed from -ENODATA
> to 0. So, we will return -ENODATA error into evm_update_evmxattr(). In
> this case, evm_update_evmxattr() will not protect uid/gid/mode by
> security.evm, but remove security.evm instead, if it's available.

For files not in policy (there aren't any xattrs), I was able to do the
chmod without problems.

# touch /var/tmp/test
# getfattr -m ^security -e hex --dump /var/tmp/test
# chmod 666 /var/tmp/test
# getfattr -m ^security -e hex --dump /var/tmp/test
# ls -lat /var/tmp/test
-rw-rw-rw- 1 root root 0 Jan  3 18:16 /var/tmp/test

We've already shown that you can write security xattrs (security.foo).
So it isn't an issue that the xattr ops don't exist.  Why isn't it
returning INTEGRITY_NOXATTRS?

Mimi

> Probably, we should protect uid/gid/mode even if we don't have xattrs
> for protection. In this case all files will have security.evm xattr.

Re: [Linux-ima-user] evm: runaway loop in verify_signature() produce deadlock in process_measurement().

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

On Tue, 2017-01-03 at 21:00 +0300, Mikhail Kurinnoi wrote:
> В Tue, 03 Jan 2017 10:55:37 -0500
> Mimi Zohar <zo...@li...> пишет:
> 
> > On Tue, 2017-01-03 at 18:02 +0300, Mikhail Kurinnoi wrote:
> > > В Tue, 03 Jan 2017 08:42:50 -0500
> > > Mimi Zohar <zo...@li...> пишет:
> > >   
> > > > On Mon, 2017-01-02 at 23:34 +0300, Mikhail Kurinnoi wrote:  
> > > > > I switched my tests on another disk and faced with deadlock
> > > > > during boot. The deadlock are reproducible if EVM enabled and
> > > > > EVM x509 cert is loaded in kernel code (CONFIG_EVM_LOAD_X509)
> > > > > with default IMA policy.    
> > > > 
> > > > These build configuration options are designed to be used in an
> > > > environment with a signed init.  If you're not interested in
> > > > appraising the init, then wait until you're ready to load the
> > > > keys.  
> > > 
> > > I have this issue if I load cert and EVM keys from initramfs by
> > > script with CONFIG_EVM_LOAD_X509 disabled in kernel.
> > > 
> > > I have no issue with EVM digital signature or HMAC during boot or
> > > after boot, I faced with deadlock on switch root only because kernel
> > > want to check what crypto-related kernel modules I have installed on
> > > real root with /bin/kmod, but, since /bin/kmod also was signed by
> > > EVM digital signature, and I could have crypto-related kernel
> > > modules installed in real root, kernel call /bin/kmod one more
> > > time, but we already have this inode locked in
> > > process_measurement()...  
> > 
> > I lost you here...  If the EVM and IMA keys used to verify /bin/kmod
> > are loaded onto their respective keyrings, then there shouldn't be a
> > problem.  The verification status result of the first /bin/kmod would
> > be cached.
> 
> Mimi, the issue not connected to EVM and IMA keys and certs. They all
> are loaded onto their respective keyrings for sure. Moreover, this
> issue not connected directly to IMA or EVM at all. This is all about 
> kernel crypto work, that EVM, probably, should take into account.
> 
> What I am trying to explain in another form:
> 0) IMA/EVM fully loaded and fully functional. Switch to real root.
> 1) Kernel(IMA/EVM) trying to verify /sbin/init EVM digital signature, call
> verify_signature().
> 2) Kernel(crypto) see rsa+sha1.
> 3) Kernel(crypto) know, we could have 3 sha1 modules.
> 4) Kernel(crypto) ask "what sha1 kernel modules we have installed into
> system" and call /bin/kmod.
> 5) Kernel(IMA/EVM) trying to verify /bin/kmod EVM digital signature, call
> verify_signature().
> 6) Kernel(crypto) see rsa+sha1.
> 7) Kernel(crypto) know, we could have 3 sha1 modules.
> 8) Kernel(crypto) ask "what sha1 modules we have installed into system"
> and call /bin/kmod.
> 9) Kernel(IMA/EVM) trying to verify /bin/kmod EVM digital signature, wait, 
> we already lock /bin/kmod inode in p5.
> 
> In my case, kernel have all 3 modules built-in:
> CONFIG_CRYPTO_SHA1=y
> CONFIG_CRYPTO_SHA1_SSSE3=y
> CONFIG_CRYPTO_SHA1_MB=y
> but kernel will use /bin/kmod to check installed into system kernel modules 
> any way.

It makes no sense to call kmod to load the crypto kernel modules if
they're builtin.  Out of curiosity, are you also seeing this problem if
only "CONFIG_CRYPTO_SHA1" is enabled?

> This is first verification for /bin/kmod, so, no cache for /bin/kmod
> yet. We just now changed the root, we don't have any cache for inodes
> in this partition, this is the problem. We can't use /bin/kmod before
> EVM verification and we can't verify /bin/kmod EVM signature, since we
> need /bin/kmod already verified for this.

A temporary fix for now would be to access /sysroot/bin/kmod from the
initramfs.

Mimi

Re: [Linux-ima-user] [PATCH] evm: add exceptions for inodes without getxattr() support.

[Mikhail K. <vie...@vi...>]

В Tue, 03 Jan 2017 08:13:42 -0500
Mimi Zohar <zo...@li...> пишет:

> On Fri, 2016-12-30 at 19:01 +0300, Mikhail Kurinnoi wrote:
> > В Fri, 30 Dec 2016 08:15:31 -0500
> > Mimi Zohar <zo...@li...> пишет:
> >   
> > > On Thu, 2016-12-29 at 23:56 +0300, Mikhail Kurinnoi wrote:
> > >   
> > > > > On Fedora, "touch" creates the selinux label, which results
> > > > > in an EVM label as well.  Everything works as expected.    
> > > > 
> > > > 
> > > > I don't use SELinux, with "dont_appraise" policy "touch" don't
> > > > creates any xattrs that should be protected. And, since I see
> > > > -EOPNOTSUPP error from getxattr(), this mean ->list, ->get,
> > > > and ->set xattr handler operations removed in inode and I have
> > > > disabled xattr support in inode. Have no idea why it's so, this
> > > > is probably some kind of xattrs related work issue.  
> 
> From your test below, you are able to write security.foo.  So, the
> setxattr/getxattr ops are valid.  This sounds like commit
> "c68ed80c97d9 ima: limit file hash setting by user to fix and log
> modes" is preventing writing security.ima hashes.  We've just
> reverted this patch during the lastest open window.  Writing a file
> signature as security.ima should work.

I reverted back this patch on my kernel sources. Nothing changed.
If file covered by dont_appraise policy, evmctl still return:
setxattr failed: /var/tmp/test
errno: Operation not permitted (1)



> > > It's kind of irrelevant, but I'd like to know why it isn't
> > > working. Are you running these tests in a VM or container?  (If
> > > in a container, what type of container?)  
> > 
> > I am testing on hardware directly and don't use containers.
> > 
> >   
> > > > I tested "security.foo" xattr, but this is not working. Looks
> > > > like this work only with xattrs that should be protected by
> > > > EVM.  
> > > 
> > > By "not working", what do you mean?  Are you able to write the
> > > security.foo xattr?  
> > 
> > Let me fix this misunderstanding by tests.
> > Test with default code:
> > 
> > # touch /var/tmp/test
> > # getfattr -m . -d -e hex /var/tmp/test
> > (no output, file don't have any xattrs)
> > # setfattr -n security.foo -v "123" /var/tmp/test  
> 
> Right, the ops exist.
> 
> > # getfattr -m . -d -e hex /var/tmp/test
> >     getfattr: Removing leading '/' from absolute path names
> >     # file: var/tmp/test
> >     security.foo=0x313233
> > # chmod 666 /var/tmp/test
> >     chmod: changing permissions of '/var/tmp/test': Operation not
> > permitted  
> 
> All files in the IMA appraise policy require an IMA hash or signature.
> With the "ima_appraise_policy", all files owned by root must be
> appraised either based on the file hash or file signature.  Other
> policies might require file signatures.


I think the issue also related to evm_calc_hmac_or_hash().
Before cycle, we set error to -ENODATA. In the same time, if we don't
have any xattrs for protection, error will never changed from -ENODATA
to 0. So, we will return -ENODATA error into evm_update_evmxattr(). In
this case, evm_update_evmxattr() will not protect uid/gid/mode by
security.evm, but remove security.evm instead, if it's available.

Probably, we should protect uid/gid/mode even if we don't have xattrs
for protection. In this case all files will have security.evm xattr.


-- 
Best regards,
Mikhail Kurinnoi

Re: [Linux-ima-user] evm: runaway loop in verify_signature() produce deadlock in process_measurement().

[Mikhail K. <vie...@vi...>]

В Tue, 03 Jan 2017 10:55:37 -0500
Mimi Zohar <zo...@li...> пишет:

> On Tue, 2017-01-03 at 18:02 +0300, Mikhail Kurinnoi wrote:
> > В Tue, 03 Jan 2017 08:42:50 -0500
> > Mimi Zohar <zo...@li...> пишет:
> >   
> > > On Mon, 2017-01-02 at 23:34 +0300, Mikhail Kurinnoi wrote:  
> > > > I switched my tests on another disk and faced with deadlock
> > > > during boot. The deadlock are reproducible if EVM enabled and
> > > > EVM x509 cert is loaded in kernel code (CONFIG_EVM_LOAD_X509)
> > > > with default IMA policy.    
> > > 
> > > These build configuration options are designed to be used in an
> > > environment with a signed init.  If you're not interested in
> > > appraising the init, then wait until you're ready to load the
> > > keys.  
> > 
> > I have this issue if I load cert and EVM keys from initramfs by
> > script with CONFIG_EVM_LOAD_X509 disabled in kernel.
> > 
> > I have no issue with EVM digital signature or HMAC during boot or
> > after boot, I faced with deadlock on switch root only because kernel
> > want to check what crypto-related kernel modules I have installed on
> > real root with /bin/kmod, but, since /bin/kmod also was signed by
> > EVM digital signature, and I could have crypto-related kernel
> > modules installed in real root, kernel call /bin/kmod one more
> > time, but we already have this inode locked in
> > process_measurement()...  
> 
> I lost you here...  If the EVM and IMA keys used to verify /bin/kmod
> are loaded onto their respective keyrings, then there shouldn't be a
> problem.  The verification status result of the first /bin/kmod would
> be cached.

Mimi, the issue not connected to EVM and IMA keys and certs. They all
are loaded onto their respective keyrings for sure. Moreover, this
issue not connected directly to IMA or EVM at all. This is all about 
kernel crypto work, that EVM, probably, should take into account.

What I am trying to explain in another form:
0) IMA/EVM fully loaded and fully functional. Switch to real root.
1) Kernel(IMA/EVM) trying to verify /sbin/init EVM digital signature, call
verify_signature().
2) Kernel(crypto) see rsa+sha1.
3) Kernel(crypto) know, we could have 3 sha1 modules.
4) Kernel(crypto) ask "what sha1 kernel modules we have installed into
system" and call /bin/kmod.
5) Kernel(IMA/EVM) trying to verify /bin/kmod EVM digital signature, call
verify_signature().
6) Kernel(crypto) see rsa+sha1.
7) Kernel(crypto) know, we could have 3 sha1 modules.
8) Kernel(crypto) ask "what sha1 modules we have installed into system"
and call /bin/kmod.
9) Kernel(IMA/EVM) trying to verify /bin/kmod EVM digital signature, wait, 
we already lock /bin/kmod inode in p5.

In my case, kernel have all 3 modules built-in:
CONFIG_CRYPTO_SHA1=y
CONFIG_CRYPTO_SHA1_SSSE3=y
CONFIG_CRYPTO_SHA1_MB=y
but kernel will use /bin/kmod to check installed into system kernel modules 
any way.

This is first verification for /bin/kmod, so, no cache for /bin/kmod
yet. We just now changed the root, we don't have any cache for inodes
in this partition, this is the problem. We can't use /bin/kmod before
EVM verification and we can't verify /bin/kmod EVM signature, since we
need /bin/kmod already verified for this.


-- 
Best regards,
Mikhail Kurinnoi

Re: [Linux-ima-user] evm: runaway loop in verify_signature() produce deadlock in process_measurement().

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

On Tue, 2017-01-03 at 18:02 +0300, Mikhail Kurinnoi wrote:
> В Tue, 03 Jan 2017 08:42:50 -0500
> Mimi Zohar <zo...@li...> пишет:
> 
> > On Mon, 2017-01-02 at 23:34 +0300, Mikhail Kurinnoi wrote:
> > > I switched my tests on another disk and faced with deadlock during
> > > boot. The deadlock are reproducible if EVM enabled and EVM x509 cert
> > > is loaded in kernel code (CONFIG_EVM_LOAD_X509) with default IMA
> > > policy.  
> > 
> > These build configuration options are designed to be used in an
> > environment with a signed init.  If you're not interested in
> > appraising the init, then wait until you're ready to load the keys.
> 
> I have this issue if I load cert and EVM keys from initramfs by
> script with CONFIG_EVM_LOAD_X509 disabled in kernel.
> 
> I have no issue with EVM digital signature or HMAC during boot or
> after boot, I faced with deadlock on switch root only because kernel
> want to check what crypto-related kernel modules I have installed on
> real root with /bin/kmod, but, since /bin/kmod also was signed by EVM
> digital signature, and I could have crypto-related kernel modules
> installed in real root, kernel call /bin/kmod one more time, but we
> already have this inode locked in process_measurement()...

I lost you here...  If the EVM and IMA keys used to verify /bin/kmod are
loaded onto their respective keyrings, then there shouldn't be a
problem.  The verification status result of the first /bin/kmod would be
cached.

> Is the any way tell crypto modules don't check external (non build-in)
> kernel modules on verify_signature() call in asymmetric_verify()?

The IMA policy is really flexible, allowing you to do whatever you want,
but I'm not convinced this is a good idea.

Kernel modules can be verified using the normal kernel build methods
and/or verified using IMA.  The normal kernel build has a couple of
different options controlling whether kernel module signatures are
required or not.

CONFIG_MODULE_SIG=y
CONFIG_MODULE_SIG_FORCE=y
CONFIG_MODULE_SIG_ALL=y

Assuming that the kernel build method requires all kernel modules to be
signed, the IMA policy "appraise func=module_check" rule can be used in
conjunction with the kernel build method.  These kernel modules would
not require IMA/EVM signatures as well.  All other kernel modules, that
haven't been signed using the builtin kernel method, would require
IMA/EVM signatures.

The builtin "ima_appraise_tcb" policy does not require kernel modules to
be signed.

I hope I answered your question.

Mimi

Re: [Linux-ima-user] evm: runaway loop in verify_signature() produce deadlock in process_measurement().

[Mikhail K. <vie...@vi...>]

В Tue, 03 Jan 2017 08:42:50 -0500
Mimi Zohar <zo...@li...> пишет:

> On Mon, 2017-01-02 at 23:34 +0300, Mikhail Kurinnoi wrote:
> > I switched my tests on another disk and faced with deadlock during
> > boot. The deadlock are reproducible if EVM enabled and EVM x509 cert
> > is loaded in kernel code (CONFIG_EVM_LOAD_X509) with default IMA
> > policy.  
> 
> These build configuration options are designed to be used in an
> environment with a signed init.  If you're not interested in
> appraising the init, then wait until you're ready to load the keys.

I have this issue if I load cert and EVM keys from initramfs by
script with CONFIG_EVM_LOAD_X509 disabled in kernel.

I have no issue with EVM digital signature or HMAC during boot or
after boot, I faced with deadlock on switch root only because kernel
want to check what crypto-related kernel modules I have installed on
real root with /bin/kmod, but, since /bin/kmod also was signed by EVM
digital signature, and I could have crypto-related kernel modules
installed in real root, kernel call /bin/kmod one more time, but we
already have this inode locked in process_measurement()...


Is the any way tell crypto modules don't check external (non build-in)
kernel modules on verify_signature() call in asymmetric_verify()?


-- 
Best regards,
Mikhail Kurinnoi

Re: [Linux-ima-user] evm: runaway loop in verify_signature() produce deadlock in process_measurement().

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

On Mon, 2017-01-02 at 23:34 +0300, Mikhail Kurinnoi wrote:
> I switched my tests on another disk and faced with deadlock during
> boot. The deadlock are reproducible if EVM enabled and EVM x509 cert
> is loaded in kernel code (CONFIG_EVM_LOAD_X509) with default IMA
> policy.

These build configuration options are designed to be used in an
environment with a signed init.  If you're not interested in appraising
the init, then wait until you're ready to load the keys.

Mimi

> Deadlock starts from first file signed by EVM digital signature. In my
> case it was /sbin/init, since I only now move the system on new disk,
> all binary files was signed with:
> evmctl sign -a sha512 --imasig -r -t fm --key /privkey_ima.pem ./
> 
> After some investigation I found:
> 1) As soon, as system switched to real root, EVM have to work with
> file signed by EVM digital signature.
> 2) For EVM digital signature verification, kernel
> (comm="kworker/u8:6") trying to detect/load kernel modules related to
> "sha1" from user space.
> 3) Kernel trying to use /bin/kmod from user space.
> 4) Since we have /bin/kmod (and dependencies) also signed by EVM
> digital signature we have locked both files (/sbin/init and /bin/kmod
> for example) till we don't verify /bin/kmod EVM digital signature.
> 5) Since we need /bin/kmod in order to verify /bin/kmod EVM digital
> signature, we have runaway loop in verify_signature() that produce
> deadlock in process_measurement().
> 
> In my case, I was forced manually update EVM digital signature to HMAC
> for /bin/kmod and all its dependencies, before I was able to boot.
> I think, this is really bad situation, when for EVM work we need
> proper signed files from user space.
> 
> 

Re: [Linux-ima-user] [PATCH] evm: add exceptions for inodes without getxattr() support.

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

On Tue, 2017-01-03 at 08:13 -0500, Mimi Zohar wrote:

> All files in the IMA appraise policy require an IMA hash or signature.
> With the "ima_appraise_policy", all files owned by root must be

That should have been "ima_appraise_tcb" policy.

Mimi

> appraised either based on the file hash or file signature.  Other
> policies might require file signatures.

Re: [Linux-ima-user] [PATCH] evm: add exceptions for inodes without getxattr() support.

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

On Fri, 2016-12-30 at 19:01 +0300, Mikhail Kurinnoi wrote:
> В Fri, 30 Dec 2016 08:15:31 -0500
> Mimi Zohar <zo...@li...> пишет:
> 
> > On Thu, 2016-12-29 at 23:56 +0300, Mikhail Kurinnoi wrote:
> > 
> > > > On Fedora, "touch" creates the selinux label, which results in an
> > > > EVM label as well.  Everything works as expected.  
> > > 
> > > 
> > > I don't use SELinux, with "dont_appraise" policy "touch" don't
> > > creates any xattrs that should be protected. And, since I see
> > > -EOPNOTSUPP error from getxattr(), this mean ->list, ->get, and
> > > ->set xattr handler operations removed in inode and I have disabled
> > > xattr support in inode. Have no idea why it's so, this is probably
> > > some kind of xattrs related work issue.

>From your test below, you are able to write security.foo.  So, the
setxattr/getxattr ops are valid.  This sounds like commit "c68ed80c97d9
ima: limit file hash setting by user to fix and log modes" is preventing
writing security.ima hashes.  We've just reverted this patch during the
lastest open window.  Writing a file signature as security.ima should
work.
 
> > It's kind of irrelevant, but I'd like to know why it isn't working.
> > Are you running these tests in a VM or container?  (If in a
> > container, what type of container?)
> 
> I am testing on hardware directly and don't use containers.
> 
> 
> > > I tested "security.foo" xattr, but this is not working. Looks like
> > > this work only with xattrs that should be protected by EVM.
> > 
> > By "not working", what do you mean?  Are you able to write the
> > security.foo xattr?
> 
> Let me fix this misunderstanding by tests.
> Test with default code:
> 
> # touch /var/tmp/test
> # getfattr -m . -d -e hex /var/tmp/test
> (no output, file don't have any xattrs)
> # setfattr -n security.foo -v "123" /var/tmp/test

Right, the ops exist.

> # getfattr -m . -d -e hex /var/tmp/test
>     getfattr: Removing leading '/' from absolute path names
>     # file: var/tmp/test
>     security.foo=0x313233
> # chmod 666 /var/tmp/test
>     chmod: changing permissions of '/var/tmp/test': Operation not permitted

All files in the IMA appraise policy require an IMA hash or signature.
With the "ima_appraise_policy", all files owned by root must be
appraised either based on the file hash or file signature.  Other
policies might require file signatures.

Mimi 

> # getfattr -m . -d -e hex /var/tmp/test
>     getfattr: Removing leading '/' from absolute path names
>     # file: var/tmp/test
>     security.foo=0x313233
> 
> audit syslog output:
> pid=17259 uid=0 auid=1000 ses=3 op="appraise_metadata" cause="fail" comm="chmod" name="test" fowner=0 dev="dm-2" ino=18 res=0
> 
> 
> Test with "security.foo" added into evm_config_xattrnames[] and evm_protect_xattr() corrected:
> 	if (strcmp(xattr_name, "security.foo") == 0)
> 		return 0;
> (yes, a little bit tricky, but I need xattr protected by EVM that I permitted to add).
> 
> # touch /var/tmp/test
> # getfattr -m . -d -e hex /var/tmp/test
> (no output, file don't have any xattrs)
> # setfattr -n security.foo -v "123" /var/tmp/test
> # getfattr -m . -d -e hex /var/tmp/test
>     getfattr: Removing leading '/' from absolute path names
>     # file: var/tmp/test
>     security.evm=0x023515cb04bd7868d4cc10e04c90f80297acbb33c2
>     security.foo=0x313233
> # chmod 666 /var/tmp/test
> # ls -l /var/tmp/test
>     -rw-rw-rw- 1 root root 0 Dec 30 17:25 /var/tmp/test
> # getfattr -m . -d -e hex /var/tmp/test
>     getfattr: Removing leading '/' from absolute path names
>     # file: var/tmp/test
>     security.evm=0x023522bf071da41b6c7ddf74098f55f1b1df2ae768
>     security.foo=0x313233
> 
> 
> Tests, after my patch applied (default evm_config_xattrnames[]), as some kind of confirmation, that evm_verify_hmac() return INTEGRITY_UNKNOWN error, in case of evm_verify_hmac() this mean we have some xattrs support issue in inode/FS (as you understand, this patch don't fix the initial issue in any way, I add this test in order to provide you as much information as I can).
> 
> # touch /var/tmp/test
> # getfattr -m . -d -e hex /var/tmp/test
> (no output, file don't have any xattrs)
> # setfattr -n security.foo -v "123" /var/tmp/test
> # getfattr -m . -d -e hex /var/tmp/test
>     getfattr: Removing leading '/' from absolute path names
>     # file: var/tmp/test
>     security.foo=0x313233
> # chmod 666 /var/tmp/test
> # ls -l /var/tmp/test
>     -rw-rw-rw- 1 root root 0 Dec 30 17:57 /var/tmp/test
> # getfattr -m . -d -e hex /var/tmp/test
>     getfattr: Removing leading '/' from absolute path names
>     # file: var/tmp/test
>     security.foo=0x313233
> 
> 

[Linux-ima-user] evm: runaway loop in verify_signature() produce deadlock in process_measurement().

[Mikhail K. <vie...@vi...>]

I switched my tests on another disk and faced with deadlock during boot. The deadlock are reproducible if EVM enabled and EVM x509 cert is loaded in kernel code (CONFIG_EVM_LOAD_X509) with default IMA policy.

Deadlock starts from first file signed by EVM digital signature. In my case it was /sbin/init, since I only now move the system on new disk, all binary files was signed with:
evmctl sign -a sha512 --imasig -r -t fm --key /privkey_ima.pem ./

After some investigation I found:
1) As soon, as system switched to real root, EVM have to work with file signed by EVM digital signature.
2) For EVM digital signature verification, kernel (comm="kworker/u8:6") trying to detect/load kernel modules related to "sha1" from user space.
3) Kernel trying to use /bin/kmod from user space.
4) Since we have /bin/kmod (and dependencies) also signed by EVM digital signature we have locked both files (/sbin/init and /bin/kmod for example) till we don't verify /bin/kmod EVM digital signature.
5) Since we need /bin/kmod in order to verify /bin/kmod EVM digital signature, we have runaway loop in verify_signature() that produce deadlock in process_measurement().

In my case, I was forced manually update EVM digital signature to HMAC for /bin/kmod and all its dependencies, before I was able to boot.
I think, this is really bad situation, when for EVM work we need proper signed files from user space.


-- 
Best regards,
Mikhail Kurinnoi

Re: [Linux-ima-user] [PATCH] evm: add exceptions for inodes without getxattr() support.

[Mikhail K. <vie...@vi...>]

В Fri, 30 Dec 2016 08:15:31 -0500
Mimi Zohar <zo...@li...> пишет:

> On Thu, 2016-12-29 at 23:56 +0300, Mikhail Kurinnoi wrote:
> 
> > > On Fedora, "touch" creates the selinux label, which results in an
> > > EVM label as well.  Everything works as expected.  
> > 
> > 
> > I don't use SELinux, with "dont_appraise" policy "touch" don't
> > creates any xattrs that should be protected. And, since I see
> > -EOPNOTSUPP error from getxattr(), this mean ->list, ->get, and
> > ->set xattr handler operations removed in inode and I have disabled
> > xattr support in inode. Have no idea why it's so, this is probably
> > some kind of xattrs related work issue.
> 
> It's kind of irrelevant, but I'd like to know why it isn't working.
> Are you running these tests in a VM or container?  (If in a
> container, what type of container?)

I am testing on hardware directly and don't use containers.


> > I tested "security.foo" xattr, but this is not working. Looks like
> > this work only with xattrs that should be protected by EVM.
> 
> By "not working", what do you mean?  Are you able to write the
> security.foo xattr?

Let me fix this misunderstanding by tests.
Test with default code:

# touch /var/tmp/test
# getfattr -m . -d -e hex /var/tmp/test
(no output, file don't have any xattrs)
# setfattr -n security.foo -v "123" /var/tmp/test
# getfattr -m . -d -e hex /var/tmp/test
    getfattr: Removing leading '/' from absolute path names
    # file: var/tmp/test
    security.foo=0x313233
# chmod 666 /var/tmp/test
    chmod: changing permissions of '/var/tmp/test': Operation not permitted
# getfattr -m . -d -e hex /var/tmp/test
    getfattr: Removing leading '/' from absolute path names
    # file: var/tmp/test
    security.foo=0x313233

audit syslog output:
pid=17259 uid=0 auid=1000 ses=3 op="appraise_metadata" cause="fail" comm="chmod" name="test" fowner=0 dev="dm-2" ino=18 res=0


Test with "security.foo" added into evm_config_xattrnames[] and evm_protect_xattr() corrected:
	if (strcmp(xattr_name, "security.foo") == 0)
		return 0;
(yes, a little bit tricky, but I need xattr protected by EVM that I permitted to add).

# touch /var/tmp/test
# getfattr -m . -d -e hex /var/tmp/test
(no output, file don't have any xattrs)
# setfattr -n security.foo -v "123" /var/tmp/test
# getfattr -m . -d -e hex /var/tmp/test
    getfattr: Removing leading '/' from absolute path names
    # file: var/tmp/test
    security.evm=0x023515cb04bd7868d4cc10e04c90f80297acbb33c2
    security.foo=0x313233
# chmod 666 /var/tmp/test
# ls -l /var/tmp/test
    -rw-rw-rw- 1 root root 0 Dec 30 17:25 /var/tmp/test
# getfattr -m . -d -e hex /var/tmp/test
    getfattr: Removing leading '/' from absolute path names
    # file: var/tmp/test
    security.evm=0x023522bf071da41b6c7ddf74098f55f1b1df2ae768
    security.foo=0x313233


Tests, after my patch applied (default evm_config_xattrnames[]), as some kind of confirmation, that evm_verify_hmac() return INTEGRITY_UNKNOWN error, in case of evm_verify_hmac() this mean we have some xattrs support issue in inode/FS (as you understand, this patch don't fix the initial issue in any way, I add this test in order to provide you as much information as I can).

# touch /var/tmp/test
# getfattr -m . -d -e hex /var/tmp/test
(no output, file don't have any xattrs)
# setfattr -n security.foo -v "123" /var/tmp/test
# getfattr -m . -d -e hex /var/tmp/test
    getfattr: Removing leading '/' from absolute path names
    # file: var/tmp/test
    security.foo=0x313233
# chmod 666 /var/tmp/test
# ls -l /var/tmp/test
    -rw-rw-rw- 1 root root 0 Dec 30 17:57 /var/tmp/test
# getfattr -m . -d -e hex /var/tmp/test
    getfattr: Removing leading '/' from absolute path names
    # file: var/tmp/test
    security.foo=0x313233


-- 
Best regards,
Mikhail Kurinnoi

Re: [Linux-ima-user] [PATCH] evm: add exceptions for inodes without getxattr() support.

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

On Thu, 2016-12-29 at 23:56 +0300, Mikhail Kurinnoi wrote:

> > On Fedora, "touch" creates the selinux label, which results in an EVM
> > label as well.  Everything works as expected.  
> 
> 
> I don't use SELinux, with "dont_appraise" policy "touch" don't creates 
> any xattrs that should be protected. And, since I see -EOPNOTSUPP error
> from getxattr(), this mean ->list, ->get, and ->set xattr handler
> operations removed in inode and I have disabled xattr support in inode.
> Have no idea why it's so, this is probably some kind of xattrs related
> work issue.

It's kind of irrelevant, but I'd like to know why it isn't working.  Are
you running these tests in a VM or container?  (If in a container, what
type of container?)

> I tested "security.foo" xattr, but this is not working. Looks like this
> work only with xattrs that should be protected by EVM.

By "not working", what do you mean?  Are you able to write the
security.foo xattr?

> In case of "appraise" policy, "touch" creates ima xattr, and I can work
> with attr/xattr changes.
> 
> I think, we probably also have some xattrs related issue in EVM
> code, since FS remove xattr support from inode that EVM only work
> with (in case of "dont_appraise", without SELinux and so on). In the
> same time I don't have any issues if EVM disabled.

Only files in policy have a security.ima xattr.  Doing a chmod
potentially changes whether or not the file is in policy.

Mimi

Re: [Linux-ima-user] [PATCH] evm: add exceptions for inodes without getxattr() support.

[Mikhail K. <vie...@vi...>]

В Thu, 29 Dec 2016 10:42:13 -0500
Mimi Zohar <zo...@li...> пишет:

> On Thu, 2016-12-29 at 07:02 +0300, Mikhail Kurinnoi wrote:
> > В Wed, 28 Dec 2016 15:18:20 -0500
> > Mimi Zohar <zo...@li...> пишет:
> >   
> > > On Wed, 2016-12-28 at 21:08 +0300, Mikhail Kurinnoi wrote:  
> > > > В Wed, 28 Dec 2016 09:24:36 -0500
> > > > Mimi Zohar <zo...@li...> пишет:
> > > >     
> > > > > On Wed, 2016-12-28 at 15:51 +0300, Mikhail Kurinnoi wrote:    
> > > > > > Removed from kernel boot command line "ima_appraise=fix
> > > > > > evm=fix", I faced with a little bit strange audit log
> > > > > > output. For example: ...
> > > > > > pid=3237 uid=0 auid=4294967295 ses=4294967295
> > > > > > op="appraise_metadata" cause="fail" comm="cp"
> > > > > > name="depconfig" fowner=0 dev="tmpfs" ino=6839 res=0
> > > > > > pid=4418 uid=0 auid=4294967295 ses=4294967295
> > > > > > op="appraise_metadata" cause="fail" comm="chgrp" name="utmp"
> > > > > > fowner=0 dev="tmpfs" ino=11295 res=0
> > > > > > pid=4419 uid=0 auid=4294967295 ses=4294967295
> > > > > > op="appraise_metadata" cause="fail" comm="chmod" name="utmp"
> > > > > > fowner=0 dev="tmpfs" ino=11295 res=0
> > > > > > pid=5040 uid=0 auid=4294967295 ses=4294967295
> > > > > > op="appraise_metadata" cause="fail" comm="cupsd" name="0"
> > > > > > fowner=0 dev="tmpfs" ino=10778 res=0
> > > > > > pid=5611 uid=1000 auid=1000 ses=3 op="appraise_metadata"
> > > > > > cause="fail" comm="X" name=".tX0-lock" fowner=0 dev="tmpfs"
> > > > > > ino=11650 res=0 ...
> > > > > > 
> > > > > > This output were logged with default policy
> > > > > > (ima_appraise_tcb ima_tcb). As you can see
> > > > > > (op="appraise_metadata"), this issue connected to
> > > > > > evm_inode_setattr(), evm_inode_removexattr() and
> > > > > > evm_inode_setxattr(). After some digging I found, that we
> > > > > > don't count on getxattr() support in inode. I mean, we
> > > > > > don't count on EOPNOTSUPP error code from
> > > > > > evm_find_protected_xattrs(), as result, evm_verify_hmac()
> > > > > > will return only INTEGRITY_FAIL error and legitimate
> > > > > > attr/xattr(acls) changes will be not allowed by EVM.      
> > > > > 
> > > > > Before EVM allows a file to write file metadata it validates
> > > > > the existing security.evm xattr.  Only if it is valid, does
> > > > > EVM permit the file metadata to change.   Otherwise the
> > > > > updated security.evm xattr would be based on bogus file
> > > > > metadata.  Is the issue really that getxattr() isn't
> > > > > supported or rather that these files were created under a
> > > > > different policy, which didn't label the files properly?    
> > > > 
> > > > As I mentioned, I also test with default policy this time from
> > > > the beginning to be sure this is not my policy issue again. For
> > > > tmpfs in default policy we have only 2 lines with
> > > > "dont_appraise" + "dont_measure" connected to magic number, and
> > > > this is upper lines. My custom policy have same upper lines
> > > > with magic numbers.
> > > > 
> > > > Tmpfs support xattrs for sure, I tested this.
> > > > Also, I add test ext4 FS partition with "dont_appraise" +
> > > > "dont_measure" rules as upper lines of policy file. Result was
> > > > the same. evm_find_protected_xattrs() return EOPNOTSUPP if I
> > > > create new file and use chmod on it. If I change test ext4 FS
> > > > partition policy to "appraise" I have no issues. So, this is
> > > > not tmpfs or xattrs support issue.    
> > > 
> > > It sounds like the file does not have the security.evm xattr
> > > before you do the chmod.  Therefore it is failing.  Please show
> > > the test steps with the security xattrs after each step.  
> > 
> > 
> > 1.
> > 
> > /tmp - tmpfs mount point
> > default policy ("dont_appraise" + "dont_measure" for tmpfs).
> > 
> > # touch /tmp/test
> > # getfattr -m . -d -e hex /tmp/test
> > (no output, file don't have any xattrs)
> > # chmod 666 /tmp/test
> >   chmod: changing permissions of '/tmp/test': Operation not
> > permitted  
> 
> Before fixing things, let's make sure we understand the real problem.
> Assuming the following Kconfig options are enabled,
> evm_protect_xattr(), if there weren't any xattrs, would return 0.
> 
> CONFIG_TMPFS=y
> CONFIG_TMPFS_POSIX_ACL=y
> CONFIG_TMPFS_XATTR=y

Yes, this options are enabled.

> If these aren't enabled, then -EOPNOTSUPP would be returned.  

Not exactly, even on FS with xattrs support, xattrs could be disabled
in inode "on the fly" by removing ->list, ->get, and ->set handler
operations. In this case -EOPNOTSUPP also will be returned.

we have 2 situation connected to -EOPNOTSUPP error: 
1) FS don't support xattrs (!inode->i_op->getxattr return true);
2) FS support xattrs, but xattrs disabled in inode
(!inode->i_op->getxattr return false, but, inode->i_op->getxattr()
return -EOPNOTSUPP).

In both cases EVM should care about xattr support in FS/inode and don't
corrupt system work as it doing now.


> IMA is
> dependent on evm_verifyxattr() returning INTEGRITY_UNKNOWN, so we
> can't just return 0 in this case as well.

I think we should keep INTEGRITY_UNKNOWN for -EOPNOTSUPP.
My changes (patch) will switch return for evm_verifyxattr() from
INTEGRITY_FAIL to INTEGRITY_UNKNOWN in case of
evm_find_protected_xattrs() return -EOPNOTSUPP.
This is same situation in evm_verify_hmac() what we have with
vfs_getxattr_alloc() return -EOPNOTSUPP.


> > # getfattr -m . -d -e hex /tmp/test
> > (no output, file don't have any xattrs)
> > # evmctl sign -a sha512 --imasig --key /privkey_ima.pem /tmp/test
> >   setxattr failed: /tmp/test
> >   errno: Operation not permitted (1)  
> 
> If the above Kconfig options weren't enabled, then you wouldn't be
> able to write any security xattrs (eg. security.foo).  This would be
> independent of the EVM/IMA.
> 
> On Fedora, "touch" creates the selinux label, which results in an EVM
> label as well.  Everything works as expected.  


I don't use SELinux, with "dont_appraise" policy "touch" don't creates 
any xattrs that should be protected. And, since I see -EOPNOTSUPP error
from getxattr(), this mean ->list, ->get, and ->set xattr handler
operations removed in inode and I have disabled xattr support in inode.
Have no idea why it's so, this is probably some kind of xattrs related
work issue.

I tested "security.foo" xattr, but this is not working. Looks like this
work only with xattrs that should be protected by EVM.

In case of "appraise" policy, "touch" creates ima xattr, and I can work
with attr/xattr changes.

I think, we probably also have some xattrs related issue in EVM
code, since FS remove xattr support from inode that EVM only work
with (in case of "dont_appraise", without SELinux and so on). In the
same time I don't have any issues if EVM disabled.


> When I get back from
> vacation, I'll set up a test environment.

This would be nice, Mimi.


-- 
Best regards,
Mikhail Kurinnoi

Re: [Linux-ima-user] [PATCH] evm: add exceptions for inodes without getxattr() support.

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

On Thu, 2016-12-29 at 07:02 +0300, Mikhail Kurinnoi wrote:
> В Wed, 28 Dec 2016 15:18:20 -0500
> Mimi Zohar <zo...@li...> пишет:
> 
> > On Wed, 2016-12-28 at 21:08 +0300, Mikhail Kurinnoi wrote:
> > > В Wed, 28 Dec 2016 09:24:36 -0500
> > > Mimi Zohar <zo...@li...> пишет:
> > >   
> > > > On Wed, 2016-12-28 at 15:51 +0300, Mikhail Kurinnoi wrote:  
> > > > > Removed from kernel boot command line "ima_appraise=fix
> > > > > evm=fix", I faced with a little bit strange audit log output.
> > > > > For example: ...
> > > > > pid=3237 uid=0 auid=4294967295 ses=4294967295
> > > > > op="appraise_metadata" cause="fail" comm="cp" name="depconfig"
> > > > > fowner=0 dev="tmpfs" ino=6839 res=0
> > > > > pid=4418 uid=0 auid=4294967295 ses=4294967295
> > > > > op="appraise_metadata" cause="fail" comm="chgrp" name="utmp"
> > > > > fowner=0 dev="tmpfs" ino=11295 res=0
> > > > > pid=4419 uid=0 auid=4294967295 ses=4294967295
> > > > > op="appraise_metadata" cause="fail" comm="chmod" name="utmp"
> > > > > fowner=0 dev="tmpfs" ino=11295 res=0
> > > > > pid=5040 uid=0 auid=4294967295 ses=4294967295
> > > > > op="appraise_metadata" cause="fail" comm="cupsd" name="0"
> > > > > fowner=0 dev="tmpfs" ino=10778 res=0
> > > > > pid=5611 uid=1000 auid=1000 ses=3 op="appraise_metadata"
> > > > > cause="fail" comm="X" name=".tX0-lock" fowner=0 dev="tmpfs"
> > > > > ino=11650 res=0 ...
> > > > > 
> > > > > This output were logged with default policy (ima_appraise_tcb
> > > > > ima_tcb). As you can see (op="appraise_metadata"), this issue
> > > > > connected to evm_inode_setattr(), evm_inode_removexattr() and
> > > > > evm_inode_setxattr(). After some digging I found, that we don't
> > > > > count on getxattr() support in inode. I mean, we don't count on
> > > > > EOPNOTSUPP error code from evm_find_protected_xattrs(), as
> > > > > result, evm_verify_hmac() will return only INTEGRITY_FAIL error
> > > > > and legitimate attr/xattr(acls) changes will be not allowed by
> > > > > EVM.    
> > > > 
> > > > Before EVM allows a file to write file metadata it validates the
> > > > existing security.evm xattr.  Only if it is valid, does EVM
> > > > permit the file metadata to change.   Otherwise the updated
> > > > security.evm xattr would be based on bogus file metadata.  Is the
> > > > issue really that getxattr() isn't supported or rather that these
> > > > files were created under a different policy, which didn't label
> > > > the files properly?  
> > > 
> > > As I mentioned, I also test with default policy this time from the
> > > beginning to be sure this is not my policy issue again. For tmpfs
> > > in default policy we have only 2 lines with "dont_appraise" +
> > > "dont_measure" connected to magic number, and this is upper lines.
> > > My custom policy have same upper lines with magic numbers.
> > > 
> > > Tmpfs support xattrs for sure, I tested this.
> > > Also, I add test ext4 FS partition with "dont_appraise" +
> > > "dont_measure" rules as upper lines of policy file. Result was the
> > > same. evm_find_protected_xattrs() return EOPNOTSUPP if I create 
> > > new file and use chmod on it. If I change test ext4 FS partition
> > > policy to "appraise" I have no issues. So, this is not tmpfs or
> > > xattrs support issue.  
> > 
> > It sounds like the file does not have the security.evm xattr before
> > you do the chmod.  Therefore it is failing.  Please show the test
> > steps with the security xattrs after each step.
> 
> 
> 1.
> 
> /tmp - tmpfs mount point
> default policy ("dont_appraise" + "dont_measure" for tmpfs).
> 
> # touch /tmp/test
> # getfattr -m . -d -e hex /tmp/test
> (no output, file don't have any xattrs)
> # chmod 666 /tmp/test
>   chmod: changing permissions of '/tmp/test': Operation not permitted

Before fixing things, let's make sure we understand the real problem.
Assuming the following Kconfig options are enabled, evm_protect_xattr(),
if there weren't any xattrs, would return 0.

CONFIG_TMPFS=y
CONFIG_TMPFS_POSIX_ACL=y
CONFIG_TMPFS_XATTR=y

If these aren't enabled, then -EOPNOTSUPP would be returned.  IMA is
dependent on evm_verifyxattr() returning INTEGRITY_UNKNOWN, so we can't
just return 0 in this case as well.

> # getfattr -m . -d -e hex /tmp/test
> (no output, file don't have any xattrs)
> # evmctl sign -a sha512 --imasig --key /privkey_ima.pem /tmp/test
>   setxattr failed: /tmp/test
>   errno: Operation not permitted (1)

If the above Kconfig options weren't enabled, then you wouldn't be able
to write any security xattrs (eg. security.foo).  This would be
independent of the EVM/IMA.

On Fedora, "touch" creates the selinux label, which results in an EVM
label as well.  Everything works as expected.  When I get back from
vacation, I'll set up a test environment.

Mimi

> # getfattr -m . -d -e hex /tmp/test
> (no output, file don't have any xattrs)
> 
> audit log:
> pid=5989 uid=0 auid=1000 ses=3 op="appraise_metadata" cause="fail" comm="chmod" name="test" fowner=0 dev="tmpfs" ino=11884 res=0
> pid=6062 uid=0 auid=1000 ses=3 op="appraise_metadata" cause="fail" comm="evmctl" name="test" fowner=0 dev="tmpfs" ino=11884 res=0
> 
> 
> 2. 
> 
> /var/tmp - ext4 FS mount point (my test ext4 FS partition)
> custom policy based on default policy, "dont_appraise" + "dont_measure" for partition as upper policy lines.
> 
> # touch /var/tmp/test
> # getfattr -m . -d -e hex /var/tmp/test
> (no output, file don't have any xattrs)
> # chmod 666 /var/tmp/test
>   chmod: changing permissions of '/var/tmp/test': Operation not permitted
> # getfattr -m . -d -e hex /var/tmp/test
> (no output, file don't have any xattrs)
> # evmctl sign -a sha512 --imasig --key /privkey_ima.pem /var/tmp/test
>   setxattr failed: /var/tmp/test
>   errno: Operation not permitted (1)
> # getfattr -m . -d -e hex /var/tmp/test
> (no output, file don't have any xattrs)
> 
> audit log:
> pid=7762 uid=0 auid=1000 ses=3 op="appraise_metadata" cause="fail" comm="chmod" name="test" fowner=0 dev="dm-2" ino=21 res=0
> pid=7954 uid=0 auid=1000 ses=3 op="appraise_metadata" cause="fail" comm="evmctl" name="test" fowner=0 dev="dm-2" ino=21 res=0
> 
> 
> 3.
> 
> / - ext4 FS mount point
> default policy
> 
> # touch /test
> # getfattr -m . -d -e hex /test
>   getfattr: Removing leading '/' from absolute path names
>   # file: test
> security.evm=0x025aad22f99f6e283e966f827022ba59475442d41e
> security.ima=0x0406cf83e1357eefb8bdf1542850d66d8007d620e4050b5715dc83f4a921d36ce9ce47d0d13c5d85f2b0ff8318d2877eec2f63b931bd47417a81a538327af927da3e
> # chmod 666 /test
> # getfattr -m . -d -e hex /test
>   getfattr: Removing leading '/' from absolute path names
>   # file: test
> security.evm=0x02c920be3c9ff5e1460b5bf1a63658dc74b8669aed
> security.ima=0x0406cf83e1357eefb8bdf1542850d66d8007d620e4050b5715dc83f4a921d36ce9ce47d0d13c5d85f2b0ff8318d2877eec2f63b931bd47417a81a538327af927da3e
> # evmctl sign -a sha512 --imasig --key /privkey_ima.pem /test
> # getfattr -m . -d -e hex /test
>   getfattr: Removing leading '/' from absolute path names
>   # file: test
> security.evm=0x030202bd74b09a02008b04f659b5d3b743c7f36ecd75eaa085fd8184a925a229453b88a0edae0196b717da8de82523d9a1ce2c083877bceb30dcc4b354b5ecce0ce67f789c5d60adbdf286096ecafbc27b3f0fd993a5438f39077fcf74b05881b96679c7e9772872d07ee281acf1c06f76f316d01a203b2ad8eef7c1f7a00301d238ba8b79943eb9065de5f402cb37bc53cbfcdca8f24bd2d7ddb80d681435f67d323f228cbf82619bae909fa31c12c94a8ca974a5b7bd95b7d773c15536c0b2c7d90d1b53d209ca500da0c0cf4cc128fdf740dd4b21dc8ab057aa4c30e6c44e4dde8892c124b8b81f0c5440825d2d31c6f15bd7d9d21fe09a1c2be12237c8cdada99f9d00380e3b3f083072c7ae8abf017c5e654986dd1f892402ae0d874dda59304f2b57b1cbdb6efd844b53309f694bedf37019ebcf682575db151b6d0ade8defde5f54838f178a5537beae3b4fa3f2172e8acb13b97ff955a73eb6d875cb81d28783c3af24b287a5ac563ae72cf3f00168c634d83ecf0ff0ec2dd470f9062ece4517c08533f4f81a6003c8bffad6a9788e62380b70215ead64ea4e67db2356398874ed3b3faf3c88cf3ee6640bdd4c50033df112c8d80a746035ed329588dc622d9bb8d6517e2ac193198489f3d24c04f014cd1d487cf15db9aa24618409b0919b1f684504bddd5a6e3a59231805dcebb2dc234b2c362e693d5bb76aeb8ab79c22be55d7f5099b
> security.ima=0x030206bd74b09a02004f92457d0c9808ec8e033d099aaf79ce80ec01ba8be87e6981174eebd219185f2af0da5800d328659bab984d2c2325b5443602f4703d9f2affda6b89a6cfb2684c260fd8ef38b3c9c42c5670f8efc66e721dcfe1db92b28181603342be0d0e74c30007eb95f0da7e7b8d5f51d70b64d763f12c0aa22ab064f5654fc0323915be3e7552d88aab563430a9d3537ddf4406119cbf87a86787102e765e3378dd5eb10cc36335d2264c6dfee8448047d9366c986b364feaaa9287481a17796d8c6549963cd22fbdac5ed030217e1972b31eb5ee22d5965ad2ecc137aff3e91b0cc6409568052f3b6e56836e6b5585997aac56eed055d62a959bf7dcba1ec1e3998db0b7a25c9ac9b1f6b1e7cf05052c053305ad5618342122705e57e71bd7bf1570718704d143a4663d690265129c6ccd3994e0a38a25f194b29fffe246220f2755c2b494f6f785b2ce56ef20558b0d1c8c8edde0b24614f8392a89959a02b2ee08042350eebfc928d574af21ff94b63a955dce530e4401c27e53d16b7d414c392dcd101d567b4b72976d307bac6848f574c4a1d6085be31c185b69cdd07ea0cfe014548eae829bc0a9a07aa46371dfbd30208a3d20573a2f5f0ca80b776ca1954e63027959c5df1ecbd77a70f42713a450d51a79e0c3f50eb12f0f966db2cf30273fb8bbf1956729b17fd34da59d5e6d51089207d92be635c406f66531589a15f0b5
> # chmod 644 /test
> # getfattr -m . -d -e hex /test
>   getfattr: Removing leading '/' from absolute path names
>   # file: test
> security.evm=0x02499ed05040352f132106ae50d8d30f3d42c52b1b
> security.ima=0x030206bd74b09a02004f92457d0c9808ec8e033d099aaf79ce80ec01ba8be87e6981174eebd219185f2af0da5800d328659bab984d2c2325b5443602f4703d9f2affda6b89a6cfb2684c260fd8ef38b3c9c42c5670f8efc66e721dcfe1db92b28181603342be0d0e74c30007eb95f0da7e7b8d5f51d70b64d763f12c0aa22ab064f5654fc0323915be3e7552d88aab563430a9d3537ddf4406119cbf87a86787102e765e3378dd5eb10cc36335d2264c6dfee8448047d9366c986b364feaaa9287481a17796d8c6549963cd22fbdac5ed030217e1972b31eb5ee22d5965ad2ecc137aff3e91b0cc6409568052f3b6e56836e6b5585997aac56eed055d62a959bf7dcba1ec1e3998db0b7a25c9ac9b1f6b1e7cf05052c053305ad5618342122705e57e71bd7bf1570718704d143a4663d690265129c6ccd3994e0a38a25f194b29fffe246220f2755c2b494f6f785b2ce56ef20558b0d1c8c8edde0b24614f8392a89959a02b2ee08042350eebfc928d574af21ff94b63a955dce530e4401c27e53d16b7d414c392dcd101d567b4b72976d307bac6848f574c4a1d6085be31c185b69cdd07ea0cfe014548eae829bc0a9a07aa46371dfbd30208a3d20573a2f5f0ca80b776ca1954e63027959c5df1ecbd77a70f42713a450d51a79e0c3f50eb12f0f966db2cf30273fb8bbf1956729b17fd34da59d5e6d51089207d92be635c406f66531589a15f0b5
> 
> 
> 
> Tests with kernel boot options "ima_appraise=fix evm=fix".
> 
> 4.
> 
> /tmp - tmpfs mount point
> default policy ("dont_appraise" + "dont_measure" for tmpfs).
> 
> # touch /tmp/test
> # getfattr -m . -d -e hex /tmp/test
> (no output, file don't have any xattrs)
> # chmod 666 /tmp/test
> # getfattr -m . -d -e hex /tmp/test
> (no output, file don't have any xattrs)
> # evmctl ima_hash -a sha512 /tmp/test
> # getfattr -m . -d -e hex /tmp/test
>   getfattr: Removing leading '/' from absolute path names
>   # file: tmp/test
> security.evm=0x025ca7312e6ee4b2c643b8b17714f90afff5ea6216
> security.ima=0x0406cf83e1357eefb8bdf1542850d66d8007d620e4050b5715dc83f4a921d36ce9ce47d0d13c5d85f2b0ff8318d2877eec2f63b931bd47417a81a538327af927da3e
> # chmod 644 /tmp/test
> # getfattr -m . -d -e hex /tmp/test
> (no output, file don't have any xattrs)
> 
> 
> 5.
> 
> /var/tmp - ext4 FS mount point (my test ext4 FS partition)
> custom policy based on default policy, "dont_appraise" + "dont_measure" for partition as upper policy lines.
> 
> # touch /var/tmp/test
> # getfattr -m . -d -e hex /var/tmp/test
> (no output, file don't have any xattrs)
> # chmod 666 /var/tmp/test
> # getfattr -m . -d -e hex /var/tmp/test
> (no output, file don't have any xattrs)
> # evmctl sign -a sha512 --imasig --key /privkey_ima.pem /var/tmp/test
> # getfattr -m . -d -e hex /var/tmp/test
>   getfattr: Removing leading '/' from absolute path names
>   # file: var/tmp/test
> security.evm=0x030202bd74b09a02003024fa292867c225340b3725754d21f5c61e7120a12478900458188dd5628527381ddbdbe931dc654c33fb61b9936216d1390521f9a8f5f868d8b70fdcf74a00dcea74a024292abb0006c102852abd393b569d19601b9dc8af56b73d621f37dd30d901f580f6344dc9bcfebf1059bded66e057f4a7b1f471fa4c3728051b0056b6e79b8ffdf05dd6fd651e4adabc68e4bfe7f0c5d1fc5f43c841d729ee636aaead68e9afbb5ff702e2b66a31004fcf461cad9edf80ca46b6648a463751d0b35045224ca618e3e6cfd0a98b53a6badc44a324f5e32cd98d6334152449f2c33cd91309bb6bfa41e055feb3d886e2f78dc413fc9dc0a56573bb2a6f3ebd05c92776682019ad486756c983846f9df4150665b28ca260527b38278dc41dd2eeda9be010af10f0c8f744b74a58c4b8c644705a180a658509fa29540ed7436ba0d27c2e569374eaec69f43453244aa66c034beeb14d8d6dd22fcd2a39d6e29d63342b5d498a9d347aff874b2f4445a33a95c4e6cde98c8e3bf4ed01fdfde6b100880b02cf4b2c6e43c94241f96569d57fd52d93f48bcfeaf638424ab4da6d03d09960c5f3cbef3f4a29a60bbce7fd756c5370d5035136763f7108200c6e019195bb81c42a16d0f3bb481388c07d78ff6faceafc1af59350b3693edfd6db5b48377632f73a85c760168834cdffecdc06df3a72993309ebbc38f5181a598ea26719c0d44e
> security.ima=0x030206bd74b09a02004f92457d0c9808ec8e033d099aaf79ce80ec01ba8be87e6981174eebd219185f2af0da5800d328659bab984d2c2325b5443602f4703d9f2affda6b89a6cfb2684c260fd8ef38b3c9c42c5670f8efc66e721dcfe1db92b28181603342be0d0e74c30007eb95f0da7e7b8d5f51d70b64d763f12c0aa22ab064f5654fc0323915be3e7552d88aab563430a9d3537ddf4406119cbf87a86787102e765e3378dd5eb10cc36335d2264c6dfee8448047d9366c986b364feaaa9287481a17796d8c6549963cd22fbdac5ed030217e1972b31eb5ee22d5965ad2ecc137aff3e91b0cc6409568052f3b6e56836e6b5585997aac56eed055d62a959bf7dcba1ec1e3998db0b7a25c9ac9b1f6b1e7cf05052c053305ad5618342122705e57e71bd7bf1570718704d143a4663d690265129c6ccd3994e0a38a25f194b29fffe246220f2755c2b494f6f785b2ce56ef20558b0d1c8c8edde0b24614f8392a89959a02b2ee08042350eebfc928d574af21ff94b63a955dce530e4401c27e53d16b7d414c392dcd101d567b4b72976d307bac6848f574c4a1d6085be31c185b69cdd07ea0cfe014548eae829bc0a9a07aa46371dfbd30208a3d20573a2f5f0ca80b776ca1954e63027959c5df1ecbd77a70f42713a450d51a79e0c3f50eb12f0f966db2cf30273fb8bbf1956729b17fd34da59d5e6d51089207d92be635c406f66531589a15f0b5
> # chmod 644 /var/tmp/test
> # getfattr -m . -d -e hex /var/tmp/test
> (no output, file don't have any xattrs)
> 
> 
> 

Re: [Linux-ima-user] [PATCH] evm: add exceptions for inodes without getxattr() support.

[Mikhail K. <vie...@vi...>]

I found this kernel upstream patch:
https://github.com/torvalds/linux/commit/6c6ef9f26e598fb977f60935e109cd5b266c941a
From this patch description:
...
Filesystems that do not support xattrs on some inodes should clear the
IOP_XATTR i_opflags flag in those inodes.  (Right now, some filesystems
have checks to disable xattrs on some inodes in the ->list, ->get, and
->set xattr handler operations instead.)  The IOP_XATTR flag is
automatically cleared in inodes of filesystems that don't have xattr
support.
...


I was digging a little bit more in getxattr() in my kernel v4.7 (that
don't have IOP_XATTR related patches), and looks like this is true.
xattr_resolve_name() could not find ->get handler operation for inode
and return EOPNOTSUPP error.

We have inode->i_op->getxattr test that return true (since FS have
xattrs support), but as soon as inode->i_op->getxattr() called,
generic_getxattr() will be called (for ext4), that will call
xattr_resolve_name(), xattr_resolve_name() don't find ->get handler
operation and return EOPNOTSUPP error.

From one side we have FS will xattrs support, from another side,
we could have inode without ->list, ->get, and ->set xattr handler
operations, that probably should mean "xattrs is disabled for
this inode".


-- 
Best regards,
Mikhail Kurinnoi

Re: [Linux-ima-user] [PATCH] evm: add exceptions for inodes without getxattr() support.

[Mikhail K. <vie...@vi...>]

В Wed, 28 Dec 2016 15:18:20 -0500
Mimi Zohar <zo...@li...> пишет:

> On Wed, 2016-12-28 at 21:08 +0300, Mikhail Kurinnoi wrote:
> > В Wed, 28 Dec 2016 09:24:36 -0500
> > Mimi Zohar <zo...@li...> пишет:
> >   
> > > On Wed, 2016-12-28 at 15:51 +0300, Mikhail Kurinnoi wrote:  
> > > > Removed from kernel boot command line "ima_appraise=fix
> > > > evm=fix", I faced with a little bit strange audit log output.
> > > > For example: ...
> > > > pid=3237 uid=0 auid=4294967295 ses=4294967295
> > > > op="appraise_metadata" cause="fail" comm="cp" name="depconfig"
> > > > fowner=0 dev="tmpfs" ino=6839 res=0
> > > > pid=4418 uid=0 auid=4294967295 ses=4294967295
> > > > op="appraise_metadata" cause="fail" comm="chgrp" name="utmp"
> > > > fowner=0 dev="tmpfs" ino=11295 res=0
> > > > pid=4419 uid=0 auid=4294967295 ses=4294967295
> > > > op="appraise_metadata" cause="fail" comm="chmod" name="utmp"
> > > > fowner=0 dev="tmpfs" ino=11295 res=0
> > > > pid=5040 uid=0 auid=4294967295 ses=4294967295
> > > > op="appraise_metadata" cause="fail" comm="cupsd" name="0"
> > > > fowner=0 dev="tmpfs" ino=10778 res=0
> > > > pid=5611 uid=1000 auid=1000 ses=3 op="appraise_metadata"
> > > > cause="fail" comm="X" name=".tX0-lock" fowner=0 dev="tmpfs"
> > > > ino=11650 res=0 ...
> > > > 
> > > > This output were logged with default policy (ima_appraise_tcb
> > > > ima_tcb). As you can see (op="appraise_metadata"), this issue
> > > > connected to evm_inode_setattr(), evm_inode_removexattr() and
> > > > evm_inode_setxattr(). After some digging I found, that we don't
> > > > count on getxattr() support in inode. I mean, we don't count on
> > > > EOPNOTSUPP error code from evm_find_protected_xattrs(), as
> > > > result, evm_verify_hmac() will return only INTEGRITY_FAIL error
> > > > and legitimate attr/xattr(acls) changes will be not allowed by
> > > > EVM.    
> > > 
> > > Before EVM allows a file to write file metadata it validates the
> > > existing security.evm xattr.  Only if it is valid, does EVM
> > > permit the file metadata to change.   Otherwise the updated
> > > security.evm xattr would be based on bogus file metadata.  Is the
> > > issue really that getxattr() isn't supported or rather that these
> > > files were created under a different policy, which didn't label
> > > the files properly?  
> > 
> > As I mentioned, I also test with default policy this time from the
> > beginning to be sure this is not my policy issue again. For tmpfs
> > in default policy we have only 2 lines with "dont_appraise" +
> > "dont_measure" connected to magic number, and this is upper lines.
> > My custom policy have same upper lines with magic numbers.
> > 
> > Tmpfs support xattrs for sure, I tested this.
> > Also, I add test ext4 FS partition with "dont_appraise" +
> > "dont_measure" rules as upper lines of policy file. Result was the
> > same. evm_find_protected_xattrs() return EOPNOTSUPP if I create 
> > new file and use chmod on it. If I change test ext4 FS partition
> > policy to "appraise" I have no issues. So, this is not tmpfs or
> > xattrs support issue.  
> 
> It sounds like the file does not have the security.evm xattr before
> you do the chmod.  Therefore it is failing.  Please show the test
> steps with the security xattrs after each step.


1.

/tmp - tmpfs mount point
default policy ("dont_appraise" + "dont_measure" for tmpfs).

# touch /tmp/test
# getfattr -m . -d -e hex /tmp/test
(no output, file don't have any xattrs)
# chmod 666 /tmp/test
  chmod: changing permissions of '/tmp/test': Operation not permitted
# getfattr -m . -d -e hex /tmp/test
(no output, file don't have any xattrs)
# evmctl sign -a sha512 --imasig --key /privkey_ima.pem /tmp/test
  setxattr failed: /tmp/test
  errno: Operation not permitted (1)
# getfattr -m . -d -e hex /tmp/test
(no output, file don't have any xattrs)

audit log:
pid=5989 uid=0 auid=1000 ses=3 op="appraise_metadata" cause="fail" comm="chmod" name="test" fowner=0 dev="tmpfs" ino=11884 res=0
pid=6062 uid=0 auid=1000 ses=3 op="appraise_metadata" cause="fail" comm="evmctl" name="test" fowner=0 dev="tmpfs" ino=11884 res=0


2. 

/var/tmp - ext4 FS mount point (my test ext4 FS partition)
custom policy based on default policy, "dont_appraise" + "dont_measure" for partition as upper policy lines.

# touch /var/tmp/test
# getfattr -m . -d -e hex /var/tmp/test
(no output, file don't have any xattrs)
# chmod 666 /var/tmp/test
  chmod: changing permissions of '/var/tmp/test': Operation not permitted
# getfattr -m . -d -e hex /var/tmp/test
(no output, file don't have any xattrs)
# evmctl sign -a sha512 --imasig --key /privkey_ima.pem /var/tmp/test
  setxattr failed: /var/tmp/test
  errno: Operation not permitted (1)
# getfattr -m . -d -e hex /var/tmp/test
(no output, file don't have any xattrs)

audit log:
pid=7762 uid=0 auid=1000 ses=3 op="appraise_metadata" cause="fail" comm="chmod" name="test" fowner=0 dev="dm-2" ino=21 res=0
pid=7954 uid=0 auid=1000 ses=3 op="appraise_metadata" cause="fail" comm="evmctl" name="test" fowner=0 dev="dm-2" ino=21 res=0


3.

/ - ext4 FS mount point
default policy

# touch /test
# getfattr -m . -d -e hex /test
  getfattr: Removing leading '/' from absolute path names
  # file: test
security.evm=0x025aad22f99f6e283e966f827022ba59475442d41e
security.ima=0x0406cf83e1357eefb8bdf1542850d66d8007d620e4050b5715dc83f4a921d36ce9ce47d0d13c5d85f2b0ff8318d2877eec2f63b931bd47417a81a538327af927da3e
# chmod 666 /test
# getfattr -m . -d -e hex /test
  getfattr: Removing leading '/' from absolute path names
  # file: test
security.evm=0x02c920be3c9ff5e1460b5bf1a63658dc74b8669aed
security.ima=0x0406cf83e1357eefb8bdf1542850d66d8007d620e4050b5715dc83f4a921d36ce9ce47d0d13c5d85f2b0ff8318d2877eec2f63b931bd47417a81a538327af927da3e
# evmctl sign -a sha512 --imasig --key /privkey_ima.pem /test
# getfattr -m . -d -e hex /test
  getfattr: Removing leading '/' from absolute path names
  # file: test
security.evm=0x030202bd74b09a02008b04f659b5d3b743c7f36ecd75eaa085fd8184a925a229453b88a0edae0196b717da8de82523d9a1ce2c083877bceb30dcc4b354b5ecce0ce67f789c5d60adbdf286096ecafbc27b3f0fd993a5438f39077fcf74b05881b96679c7e9772872d07ee281acf1c06f76f316d01a203b2ad8eef7c1f7a00301d238ba8b79943eb9065de5f402cb37bc53cbfcdca8f24bd2d7ddb80d681435f67d323f228cbf82619bae909fa31c12c94a8ca974a5b7bd95b7d773c15536c0b2c7d90d1b53d209ca500da0c0cf4cc128fdf740dd4b21dc8ab057aa4c30e6c44e4dde8892c124b8b81f0c5440825d2d31c6f15bd7d9d21fe09a1c2be12237c8cdada99f9d00380e3b3f083072c7ae8abf017c5e654986dd1f892402ae0d874dda59304f2b57b1cbdb6efd844b53309f694bedf37019ebcf682575db151b6d0ade8defde5f54838f178a5537beae3b4fa3f2172e8acb13b97ff955a73eb6d875cb81d28783c3af24b287a5ac563ae72cf3f00168c634d83ecf0ff0ec2dd470f9062ece4517c08533f4f81a6003c8bffad6a9788e62380b70215ead64ea4e67db2356398874ed3b3faf3c88cf3ee6640bdd4c50033df112c8d80a746035ed329588dc622d9bb8d6517e2ac193198489f3d24c04f014cd1d487cf15db9aa24618409b0919b1f684504bddd5a6e3a59231805dcebb2dc234b2c362e693d5bb76aeb8ab79c22be55d7f5099b
security.ima=0x030206bd74b09a02004f92457d0c9808ec8e033d099aaf79ce80ec01ba8be87e6981174eebd219185f2af0da5800d328659bab984d2c2325b5443602f4703d9f2affda6b89a6cfb2684c260fd8ef38b3c9c42c5670f8efc66e721dcfe1db92b28181603342be0d0e74c30007eb95f0da7e7b8d5f51d70b64d763f12c0aa22ab064f5654fc0323915be3e7552d88aab563430a9d3537ddf4406119cbf87a86787102e765e3378dd5eb10cc36335d2264c6dfee8448047d9366c986b364feaaa9287481a17796d8c6549963cd22fbdac5ed030217e1972b31eb5ee22d5965ad2ecc137aff3e91b0cc6409568052f3b6e56836e6b5585997aac56eed055d62a959bf7dcba1ec1e3998db0b7a25c9ac9b1f6b1e7cf05052c053305ad5618342122705e57e71bd7bf1570718704d143a4663d690265129c6ccd3994e0a38a25f194b29fffe246220f2755c2b494f6f785b2ce56ef20558b0d1c8c8edde0b24614f8392a89959a02b2ee08042350eebfc928d574af21ff94b63a955dce530e4401c27e53d16b7d414c392dcd101d567b4b72976d307bac6848f574c4a1d6085be31c185b69cdd07ea0cfe014548eae829bc0a9a07aa46371dfbd30208a3d20573a2f5f0ca80b776ca1954e63027959c5df1ecbd77a70f42713a450d51a79e0c3f50eb12f0f966db2cf30273fb8bbf1956729b17fd34da59d5e6d51089207d92be635c406f66531589a15f0b5
# chmod 644 /test
# getfattr -m . -d -e hex /test
  getfattr: Removing leading '/' from absolute path names
  # file: test
security.evm=0x02499ed05040352f132106ae50d8d30f3d42c52b1b
security.ima=0x030206bd74b09a02004f92457d0c9808ec8e033d099aaf79ce80ec01ba8be87e6981174eebd219185f2af0da5800d328659bab984d2c2325b5443602f4703d9f2affda6b89a6cfb2684c260fd8ef38b3c9c42c5670f8efc66e721dcfe1db92b28181603342be0d0e74c30007eb95f0da7e7b8d5f51d70b64d763f12c0aa22ab064f5654fc0323915be3e7552d88aab563430a9d3537ddf4406119cbf87a86787102e765e3378dd5eb10cc36335d2264c6dfee8448047d9366c986b364feaaa9287481a17796d8c6549963cd22fbdac5ed030217e1972b31eb5ee22d5965ad2ecc137aff3e91b0cc6409568052f3b6e56836e6b5585997aac56eed055d62a959bf7dcba1ec1e3998db0b7a25c9ac9b1f6b1e7cf05052c053305ad5618342122705e57e71bd7bf1570718704d143a4663d690265129c6ccd3994e0a38a25f194b29fffe246220f2755c2b494f6f785b2ce56ef20558b0d1c8c8edde0b24614f8392a89959a02b2ee08042350eebfc928d574af21ff94b63a955dce530e4401c27e53d16b7d414c392dcd101d567b4b72976d307bac6848f574c4a1d6085be31c185b69cdd07ea0cfe014548eae829bc0a9a07aa46371dfbd30208a3d20573a2f5f0ca80b776ca1954e63027959c5df1ecbd77a70f42713a450d51a79e0c3f50eb12f0f966db2cf30273fb8bbf1956729b17fd34da59d5e6d51089207d92be635c406f66531589a15f0b5



Tests with kernel boot options "ima_appraise=fix evm=fix".

4.

/tmp - tmpfs mount point
default policy ("dont_appraise" + "dont_measure" for tmpfs).

# touch /tmp/test
# getfattr -m . -d -e hex /tmp/test
(no output, file don't have any xattrs)
# chmod 666 /tmp/test
# getfattr -m . -d -e hex /tmp/test
(no output, file don't have any xattrs)
# evmctl ima_hash -a sha512 /tmp/test
# getfattr -m . -d -e hex /tmp/test
  getfattr: Removing leading '/' from absolute path names
  # file: tmp/test
security.evm=0x025ca7312e6ee4b2c643b8b17714f90afff5ea6216
security.ima=0x0406cf83e1357eefb8bdf1542850d66d8007d620e4050b5715dc83f4a921d36ce9ce47d0d13c5d85f2b0ff8318d2877eec2f63b931bd47417a81a538327af927da3e
# chmod 644 /tmp/test
# getfattr -m . -d -e hex /tmp/test
(no output, file don't have any xattrs)


5.

/var/tmp - ext4 FS mount point (my test ext4 FS partition)
custom policy based on default policy, "dont_appraise" + "dont_measure" for partition as upper policy lines.

# touch /var/tmp/test
# getfattr -m . -d -e hex /var/tmp/test
(no output, file don't have any xattrs)
# chmod 666 /var/tmp/test
# getfattr -m . -d -e hex /var/tmp/test
(no output, file don't have any xattrs)
# evmctl sign -a sha512 --imasig --key /privkey_ima.pem /var/tmp/test
# getfattr -m . -d -e hex /var/tmp/test
  getfattr: Removing leading '/' from absolute path names
  # file: var/tmp/test
security.evm=0x030202bd74b09a02003024fa292867c225340b3725754d21f5c61e7120a12478900458188dd5628527381ddbdbe931dc654c33fb61b9936216d1390521f9a8f5f868d8b70fdcf74a00dcea74a024292abb0006c102852abd393b569d19601b9dc8af56b73d621f37dd30d901f580f6344dc9bcfebf1059bded66e057f4a7b1f471fa4c3728051b0056b6e79b8ffdf05dd6fd651e4adabc68e4bfe7f0c5d1fc5f43c841d729ee636aaead68e9afbb5ff702e2b66a31004fcf461cad9edf80ca46b6648a463751d0b35045224ca618e3e6cfd0a98b53a6badc44a324f5e32cd98d6334152449f2c33cd91309bb6bfa41e055feb3d886e2f78dc413fc9dc0a56573bb2a6f3ebd05c92776682019ad486756c983846f9df4150665b28ca260527b38278dc41dd2eeda9be010af10f0c8f744b74a58c4b8c644705a180a658509fa29540ed7436ba0d27c2e569374eaec69f43453244aa66c034beeb14d8d6dd22fcd2a39d6e29d63342b5d498a9d347aff874b2f4445a33a95c4e6cde98c8e3bf4ed01fdfde6b100880b02cf4b2c6e43c94241f96569d57fd52d93f48bcfeaf638424ab4da6d03d09960c5f3cbef3f4a29a60bbce7fd756c5370d5035136763f7108200c6e019195bb81c42a16d0f3bb481388c07d78ff6faceafc1af59350b3693edfd6db5b48377632f73a85c760168834cdffecdc06df3a72993309ebbc38f5181a598ea26719c0d44e
security.ima=0x030206bd74b09a02004f92457d0c9808ec8e033d099aaf79ce80ec01ba8be87e6981174eebd219185f2af0da5800d328659bab984d2c2325b5443602f4703d9f2affda6b89a6cfb2684c260fd8ef38b3c9c42c5670f8efc66e721dcfe1db92b28181603342be0d0e74c30007eb95f0da7e7b8d5f51d70b64d763f12c0aa22ab064f5654fc0323915be3e7552d88aab563430a9d3537ddf4406119cbf87a86787102e765e3378dd5eb10cc36335d2264c6dfee8448047d9366c986b364feaaa9287481a17796d8c6549963cd22fbdac5ed030217e1972b31eb5ee22d5965ad2ecc137aff3e91b0cc6409568052f3b6e56836e6b5585997aac56eed055d62a959bf7dcba1ec1e3998db0b7a25c9ac9b1f6b1e7cf05052c053305ad5618342122705e57e71bd7bf1570718704d143a4663d690265129c6ccd3994e0a38a25f194b29fffe246220f2755c2b494f6f785b2ce56ef20558b0d1c8c8edde0b24614f8392a89959a02b2ee08042350eebfc928d574af21ff94b63a955dce530e4401c27e53d16b7d414c392dcd101d567b4b72976d307bac6848f574c4a1d6085be31c185b69cdd07ea0cfe014548eae829bc0a9a07aa46371dfbd30208a3d20573a2f5f0ca80b776ca1954e63027959c5df1ecbd77a70f42713a450d51a79e0c3f50eb12f0f966db2cf30273fb8bbf1956729b17fd34da59d5e6d51089207d92be635c406f66531589a15f0b5
# chmod 644 /var/tmp/test
# getfattr -m . -d -e hex /var/tmp/test
(no output, file don't have any xattrs)



-- 
Best regards,
Mikhail Kurinnoi

Re: [Linux-ima-user] [PATCH] evm: add exceptions for inodes without getxattr() support.

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

On Wed, 2016-12-28 at 21:08 +0300, Mikhail Kurinnoi wrote:
> В Wed, 28 Dec 2016 09:24:36 -0500
> Mimi Zohar <zo...@li...> пишет:
> 
> > On Wed, 2016-12-28 at 15:51 +0300, Mikhail Kurinnoi wrote:
> > > Removed from kernel boot command line "ima_appraise=fix evm=fix", I
> > > faced with a little bit strange audit log output. For example:
> > > ...
> > > pid=3237 uid=0 auid=4294967295 ses=4294967295 op="appraise_metadata"
> > > cause="fail" comm="cp" name="depconfig" fowner=0 dev="tmpfs"
> > > ino=6839 res=0
> > > pid=4418 uid=0 auid=4294967295 ses=4294967295 op="appraise_metadata"
> > > cause="fail" comm="chgrp" name="utmp" fowner=0 dev="tmpfs" ino=11295
> > > res=0
> > > pid=4419 uid=0 auid=4294967295 ses=4294967295 op="appraise_metadata"
> > > cause="fail" comm="chmod" name="utmp" fowner=0 dev="tmpfs" ino=11295
> > > res=0
> > > pid=5040 uid=0 auid=4294967295 ses=4294967295 op="appraise_metadata"
> > > cause="fail" comm="cupsd" name="0" fowner=0 dev="tmpfs" ino=10778
> > > res=0
> > > pid=5611 uid=1000 auid=1000 ses=3 op="appraise_metadata"
> > > cause="fail" comm="X" name=".tX0-lock" fowner=0 dev="tmpfs"
> > > ino=11650 res=0 ...
> > > 
> > > This output were logged with default policy (ima_appraise_tcb
> > > ima_tcb). As you can see (op="appraise_metadata"), this issue
> > > connected to evm_inode_setattr(), evm_inode_removexattr() and
> > > evm_inode_setxattr(). After some digging I found, that we don't
> > > count on getxattr() support in inode. I mean, we don't count on
> > > EOPNOTSUPP error code from evm_find_protected_xattrs(), as result,
> > > evm_verify_hmac() will return only INTEGRITY_FAIL error and
> > > legitimate attr/xattr(acls) changes will be not allowed by EVM.  
> > 
> > Before EVM allows a file to write file metadata it validates the
> > existing security.evm xattr.  Only if it is valid, does EVM permit the
> > file metadata to change.   Otherwise the updated security.evm xattr
> > would be based on bogus file metadata.  Is the issue really that
> > getxattr() isn't supported or rather that these files were created
> > under a different policy, which didn't label the files properly?
> 
> As I mentioned, I also test with default policy this time from the beginning
> to be sure this is not my policy issue again. For tmpfs in default
> policy we have only 2 lines with "dont_appraise" + "dont_measure" connected
> to magic number, and this is upper lines. My custom policy have same upper
> lines with magic numbers.
> 
> Tmpfs support xattrs for sure, I tested this.
> Also, I add test ext4 FS partition with "dont_appraise" + "dont_measure"
> rules as upper lines of policy file. Result was the same.
> evm_find_protected_xattrs() return EOPNOTSUPP if I create 
> new file and use chmod on it. If I change test ext4 FS partition policy 
> to "appraise" I have no issues. So, this is not tmpfs or xattrs
> support issue.

It sounds like the file does not have the security.evm xattr before you
do the chmod.  Therefore it is failing.  Please show the test steps with
the security xattrs after each step.

thanks,

Mimi


> I just can add, that in evm_find_protected_xattrs() error message
> EOPNOTSUPP returned not by inode->i_op->getxattr test, but by
> inode->i_op->getxattr() execution itself.
> 

Re: [Linux-ima-user] [PATCH] evm: add exceptions for inodes without getxattr() support.

[Mikhail K. <vie...@vi...>]

В Wed, 28 Dec 2016 09:24:36 -0500
Mimi Zohar <zo...@li...> пишет:

> On Wed, 2016-12-28 at 15:51 +0300, Mikhail Kurinnoi wrote:
> > Removed from kernel boot command line "ima_appraise=fix evm=fix", I
> > faced with a little bit strange audit log output. For example:
> > ...
> > pid=3237 uid=0 auid=4294967295 ses=4294967295 op="appraise_metadata"
> > cause="fail" comm="cp" name="depconfig" fowner=0 dev="tmpfs"
> > ino=6839 res=0
> > pid=4418 uid=0 auid=4294967295 ses=4294967295 op="appraise_metadata"
> > cause="fail" comm="chgrp" name="utmp" fowner=0 dev="tmpfs" ino=11295
> > res=0
> > pid=4419 uid=0 auid=4294967295 ses=4294967295 op="appraise_metadata"
> > cause="fail" comm="chmod" name="utmp" fowner=0 dev="tmpfs" ino=11295
> > res=0
> > pid=5040 uid=0 auid=4294967295 ses=4294967295 op="appraise_metadata"
> > cause="fail" comm="cupsd" name="0" fowner=0 dev="tmpfs" ino=10778
> > res=0
> > pid=5611 uid=1000 auid=1000 ses=3 op="appraise_metadata"
> > cause="fail" comm="X" name=".tX0-lock" fowner=0 dev="tmpfs"
> > ino=11650 res=0 ...
> > 
> > This output were logged with default policy (ima_appraise_tcb
> > ima_tcb). As you can see (op="appraise_metadata"), this issue
> > connected to evm_inode_setattr(), evm_inode_removexattr() and
> > evm_inode_setxattr(). After some digging I found, that we don't
> > count on getxattr() support in inode. I mean, we don't count on
> > EOPNOTSUPP error code from evm_find_protected_xattrs(), as result,
> > evm_verify_hmac() will return only INTEGRITY_FAIL error and
> > legitimate attr/xattr(acls) changes will be not allowed by EVM.  
> 
> Before EVM allows a file to write file metadata it validates the
> existing security.evm xattr.  Only if it is valid, does EVM permit the
> file metadata to change.   Otherwise the updated security.evm xattr
> would be based on bogus file metadata.  Is the issue really that
> getxattr() isn't supported or rather that these files were created
> under a different policy, which didn't label the files properly?

As I mentioned, I also test with default policy this time from the beginning
to be sure this is not my policy issue again. For tmpfs in default
policy we have only 2 lines with "dont_appraise" + "dont_measure" connected
to magic number, and this is upper lines. My custom policy have same upper
lines with magic numbers.

Tmpfs support xattrs for sure, I tested this.
Also, I add test ext4 FS partition with "dont_appraise" + "dont_measure"
rules as upper lines of policy file. Result was the same.
evm_find_protected_xattrs() return EOPNOTSUPP if I create 
new file and use chmod on it. If I change test ext4 FS partition policy 
to "appraise" I have no issues. So, this is not tmpfs or xattrs
support issue.

I just can add, that in evm_find_protected_xattrs() error message
EOPNOTSUPP returned not by inode->i_op->getxattr test, but by
inode->i_op->getxattr() execution itself.

-- 
Best regards,
Mikhail Kurinnoi

Re: [Linux-ima-user] [PATCH] evm: add exceptions for inodes without getxattr() support.

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

On Wed, 2016-12-28 at 15:51 +0300, Mikhail Kurinnoi wrote:
> Removed from kernel boot command line "ima_appraise=fix evm=fix", I
> faced with a little bit strange audit log output. For example:
> ...
> pid=3237 uid=0 auid=4294967295 ses=4294967295 op="appraise_metadata"
> cause="fail" comm="cp" name="depconfig" fowner=0 dev="tmpfs" ino=6839
> res=0
> pid=4418 uid=0 auid=4294967295 ses=4294967295 op="appraise_metadata"
> cause="fail" comm="chgrp" name="utmp" fowner=0 dev="tmpfs" ino=11295
> res=0
> pid=4419 uid=0 auid=4294967295 ses=4294967295 op="appraise_metadata"
> cause="fail" comm="chmod" name="utmp" fowner=0 dev="tmpfs" ino=11295
> res=0
> pid=5040 uid=0 auid=4294967295 ses=4294967295 op="appraise_metadata"
> cause="fail" comm="cupsd" name="0" fowner=0 dev="tmpfs" ino=10778
> res=0
> pid=5611 uid=1000 auid=1000 ses=3 op="appraise_metadata" cause="fail"
> comm="X" name=".tX0-lock" fowner=0 dev="tmpfs" ino=11650 res=0
> ...
> 
> This output were logged with default policy (ima_appraise_tcb
> ima_tcb). As you can see (op="appraise_metadata"), this issue
> connected to evm_inode_setattr(), evm_inode_removexattr() and
> evm_inode_setxattr(). After some digging I found, that we don't count
> on getxattr() support in inode. I mean, we don't count on EOPNOTSUPP
> error code from evm_find_protected_xattrs(), as result,
> evm_verify_hmac() will return only INTEGRITY_FAIL error and legitimate
> attr/xattr(acls) changes will be not allowed by EVM.

Before EVM allows a file to write file metadata it validates the
existing security.evm xattr.  Only if it is valid, does EVM permit the
file metadata to change.   Otherwise the updated security.evm xattr
would be based on bogus file metadata.  Is the issue really that
getxattr() isn't supported or rather that these files were created under
a different policy, which didn't label the files properly?  (tmpfs has
xattr support.)

Mimi

> Probably, we should not prevent attr/xattr(acls) changes in EVM code,
> if inode don't support getxattr().
> 
> This patch add exceptions for inodes without getxattr() support.
> 
> Signed-off-by: Mikhail Kurinnoi <vie...@vi...>
> 
> --- a/security/integrity/evm/evm_main.c
> +++ b/security/integrity/evm/evm_main.c
> @@ -138,6 +138,8 @@ static enum integrity_status evm_verify_hmac
>  				evm_status = INTEGRITY_NOLABEL;
>  			else if (rc == 0)
>  				evm_status = INTEGRITY_NOXATTRS; /* new file */
> +			else if (rc == -EOPNOTSUPP)
> +				evm_status = INTEGRITY_UNKNOWN;
>  		} else if (rc == -EOPNOTSUPP) {
>  			evm_status = INTEGRITY_UNKNOWN;
>  		}
> @@ -291,12 +293,14 @@ static int evm_protect_xattr(struct dentry *dentry,
>  			return 0;
>  		evm_status = evm_verify_current_integrity(dentry);
>  		if ((evm_status == INTEGRITY_PASS) ||
> -		    (evm_status == INTEGRITY_NOXATTRS))
> +		    (evm_status == INTEGRITY_NOXATTRS) || 
> +		    (evm_status == INTEGRITY_UNKNOWN))
>  			return 0;
>  		goto out;
>  	}
>  	evm_status = evm_verify_current_integrity(dentry);
> -	if (evm_status == INTEGRITY_NOXATTRS) {
> +	if ((evm_status == INTEGRITY_NOXATTRS) || 
> +	    (evm_status == INTEGRITY_UNKNOWN)) {
>  		struct integrity_iint_cache *iint;
> 
>  		iint = integrity_iint_find(d_backing_inode(dentry));
> @@ -432,7 +436,8 @@ int evm_inode_setattr(struct dentry *dentry, struct iattr *attr)
> 
>  	evm_status = evm_verify_current_integrity(dentry);
>  	if ((evm_status == INTEGRITY_PASS) ||
> -	    (evm_status == INTEGRITY_NOXATTRS))
> +	    (evm_status == INTEGRITY_NOXATTRS) ||
> +	    (evm_status == INTEGRITY_UNKNOWN))
>  		return 0;
>  	integrity_audit_msg(AUDIT_INTEGRITY_METADATA, d_backing_inode(dentry),
>  			    dentry->d_name.name, "appraise_metadata",
> 
> ------------------------------------------------------------------------------
> Check out the vibrant tech community on one of the world's most 
> engaging tech sites, SlashDot.org! http://sdm.link/slashdot
> _______________________________________________
> Linux-ima-user mailing list
> Lin...@li...
> https://lists.sourceforge.net/lists/listinfo/linux-ima-user
> 

[Linux-ima-user] [PATCH] evm: add exceptions for inodes without getxattr() support.

[Mikhail K. <vie...@vi...>]

Removed from kernel boot command line "ima_appraise=fix evm=fix", I faced with a little bit strange audit log output. For example:
...
pid=3237 uid=0 auid=4294967295 ses=4294967295 op="appraise_metadata" cause="fail" comm="cp" name="depconfig" fowner=0 dev="tmpfs" ino=6839 res=0
pid=4418 uid=0 auid=4294967295 ses=4294967295 op="appraise_metadata" cause="fail" comm="chgrp" name="utmp" fowner=0 dev="tmpfs" ino=11295 res=0
pid=4419 uid=0 auid=4294967295 ses=4294967295 op="appraise_metadata" cause="fail" comm="chmod" name="utmp" fowner=0 dev="tmpfs" ino=11295 res=0
pid=5040 uid=0 auid=4294967295 ses=4294967295 op="appraise_metadata" cause="fail" comm="cupsd" name="0" fowner=0 dev="tmpfs" ino=10778 res=0
pid=5611 uid=1000 auid=1000 ses=3 op="appraise_metadata" cause="fail" comm="X" name=".tX0-lock" fowner=0 dev="tmpfs" ino=11650 res=0
...

This output were logged with default policy (ima_appraise_tcb ima_tcb). As you can see (op="appraise_metadata"), this issue connected to evm_inode_setattr(), evm_inode_removexattr() and evm_inode_setxattr(). After some digging I found, that we don't count on getxattr() support in inode. I mean, we don't count on EOPNOTSUPP error code from evm_find_protected_xattrs(), as result, evm_verify_hmac() will return only INTEGRITY_FAIL error and legitimate attr/xattr(acls) changes will be not allowed by EVM.
Probably, we should not prevent attr/xattr(acls) changes in EVM code, if inode don't support getxattr().

This patch add exceptions for inodes without getxattr() support.

Signed-off-by: Mikhail Kurinnoi <vie...@vi...>

--- a/security/integrity/evm/evm_main.c
+++ b/security/integrity/evm/evm_main.c
@@ -138,6 +138,8 @@ static enum integrity_status evm_verify_hmac
 				evm_status = INTEGRITY_NOLABEL;
 			else if (rc == 0)
 				evm_status = INTEGRITY_NOXATTRS; /* new file */
+			else if (rc == -EOPNOTSUPP)
+				evm_status = INTEGRITY_UNKNOWN;
 		} else if (rc == -EOPNOTSUPP) {
 			evm_status = INTEGRITY_UNKNOWN;
 		}
@@ -291,12 +293,14 @@ static int evm_protect_xattr(struct dentry *dentry,
 			return 0;
 		evm_status = evm_verify_current_integrity(dentry);
 		if ((evm_status == INTEGRITY_PASS) ||
-		    (evm_status == INTEGRITY_NOXATTRS))
+		    (evm_status == INTEGRITY_NOXATTRS) || 
+		    (evm_status == INTEGRITY_UNKNOWN))
 			return 0;
 		goto out;
 	}
 	evm_status = evm_verify_current_integrity(dentry);
-	if (evm_status == INTEGRITY_NOXATTRS) {
+	if ((evm_status == INTEGRITY_NOXATTRS) || 
+	    (evm_status == INTEGRITY_UNKNOWN)) {
 		struct integrity_iint_cache *iint;
 
 		iint = integrity_iint_find(d_backing_inode(dentry));
@@ -432,7 +436,8 @@ int evm_inode_setattr(struct dentry *dentry, struct iattr *attr)
 
 	evm_status = evm_verify_current_integrity(dentry);
 	if ((evm_status == INTEGRITY_PASS) ||
-	    (evm_status == INTEGRITY_NOXATTRS))
+	    (evm_status == INTEGRITY_NOXATTRS) ||
+	    (evm_status == INTEGRITY_UNKNOWN))
 		return 0;
 	integrity_audit_msg(AUDIT_INTEGRITY_METADATA, d_backing_inode(dentry),
 			    dentry->d_name.name, "appraise_metadata",

Re: [Linux-ima-user] ima apprise and mkstemp()

[Mikhail K. <vie...@vi...>]

> > > > 2) FS mounted with iversion flag.
> > > > 3) kernel 4.7.10, IMA/EVM-related boot options:
> > > > rootflags=i_version ima_appraise=fix evm=fix    
> > > 
> > > The "boot command line options "ima_appraise=fix" and "evm=fix"
> > > are for fixing a file system missing these xattrs.   Try removing
> > > these options and re-testing.   
> > 
> > I remove "ima_appraise=fix" and "evm=fix for testing purposes. Here
> > are the results:
> > 
> > 1) I have same results for regular file:
> > # touch /test
> > # getfattr -m . -d -e hex /test
> >   getfattr: Removing leading '/' from absolute path names
> >   # file: test
> >   security.evm=0x02bf70fed1341366c0d088b1345f2c38c6d2bcae06
> >   security.ima=0x0406cf83e1357eefb8bdf1542850d66d8007d620e4050b5715dc83f4a921d36ce9ce47d0d13c5d85f2b0ff8318d2877eec2f63b931bd47417a81a538327af927da3e
> > # echo "123" >> /test
> > # getfattr -m . -d -e hex /test
> >   getfattr: Removing leading '/' from absolute path names
> >   # file: test
> >   security.evm=0x0295ecb4eee8f5bca2d60c6b1864b68c5c4e4988ea
> >   security.ima=0x0406ea2fe56bb8c1fb5ada84963b42ed71b764a74b092d75755173ade06f2f4aada9c00d6c302e185035cbe85fdff31698bca93e8661f0cbcef52cf2ff65864fd742
> > 
> > No messages in audit syslog.
> > 
> > 
> > 2) For mkstemp() test results are different:
> > # /a.out
> > # getfattr -m . -d -e hex /test-ah2mFC
> > (!!! still no output here, file don't have any xattrs)  
> 
> Try running your test program as root, before you install your custom
> policy.

Mimi, thanks a lot! I did as you advised, and then compared my
custom policy with default carefully. The issue was this policy lines:

appraise func=FILE_CHECK mask=MAY_READ
appraise func=FILE_CHECK mask=MAY_WRITE
appraise func=FILE_CHECK mask=MAY_APPEND

they don't cover mkstemp() work, in the same time:

appraise

will do it (default policy also use appraise without func/mask). I am a
bit confused. I was sure, thus 3 lines above will cover all works with
regular files. Looks like I still have a mess in my head about
appraise/measure policy flags. Should read manuals more carefully next
time.


-- 
Best regards,
Mikhail Kurinnoi

Re: [Linux-ima-user] ima apprise and mkstemp()

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

On Tue, 2016-12-27 at 17:21 +0300, Mikhail Kurinnoi wrote:
> > > I faced with issue, when created by some programs files don't have
> > > IMA/EVM sign (that should be), for example - git, a lot of gtk2/3
> > > programs, etc.  
> > 
> > For a file to be labeled properly, the file must be defined in the
> > policy.   Normally, the builtin policy ima_appraise_tcb is defined on
> > the boot command and then replaced with a custom policy in the
> > initramfs.        
> 
> Yes, I do exactly in this way. Builtin policy replaced on early boot
> with custom policy in the initramfs.
> 
> 
> > > 2) FS mounted with iversion flag.
> > > 3) kernel 4.7.10, IMA/EVM-related boot options: rootflags=i_version
> > > ima_appraise=fix evm=fix  
> > 
> > The "boot command line options "ima_appraise=fix" and "evm=fix" are
> > for fixing a file system missing these xattrs.   Try removing these
> > options and re-testing. 
> 
> I remove "ima_appraise=fix" and "evm=fix for testing purposes. Here are the results:
> 
> 1) I have same results for regular file:
> # touch /test
> # getfattr -m . -d -e hex /test
>   getfattr: Removing leading '/' from absolute path names
>   # file: test
>   security.evm=0x02bf70fed1341366c0d088b1345f2c38c6d2bcae06
>   security.ima=0x0406cf83e1357eefb8bdf1542850d66d8007d620e4050b5715dc83f4a921d36ce9ce47d0d13c5d85f2b0ff8318d2877eec2f63b931bd47417a81a538327af927da3e
> # echo "123" >> /test
> # getfattr -m . -d -e hex /test
>   getfattr: Removing leading '/' from absolute path names
>   # file: test
>   security.evm=0x0295ecb4eee8f5bca2d60c6b1864b68c5c4e4988ea
>   security.ima=0x0406ea2fe56bb8c1fb5ada84963b42ed71b764a74b092d75755173ade06f2f4aada9c00d6c302e185035cbe85fdff31698bca93e8661f0cbcef52cf2ff65864fd742
> 
> No messages in audit syslog.
> 
> 
> 2) For mkstemp() test results are different:
> # /a.out
> # getfattr -m . -d -e hex /test-ah2mFC
> (!!! still no output here, file don't have any xattrs)

Try running your test program as root, before you install your custom
policy.

Mimi

> # cat /test-ah2mFC
>   cat: /test-ah2mFC: Permission denied
> # echo "123" >> /test-ah2mFC
>   bash: /test-ah2mFC: Permission denied
> 
> Audit syslog messages:
> pid=7372 uid=0 auid=1000 ses=3 op="appraise_data" cause="missing-hash" comm="cat" name="/test-ah2mFC" fowner=0 dev="dm-1" ino=18961 res=0
> pid=6460 uid=0 auid=1000 ses=3 op="appraise_data" cause="missing-hash" comm="bash" name="/test-ah2mFC" fowner=0 dev="dm-1" ino=18961 res=0
> 
> 
> 
> > > First test (create and write into regular file):
> > > # touch /test
> > >   or
> > > # echo "123" > /test
> > > # getfattr -m . -d /test  
> > 
> > When displaying the xattrs, it help to display them in hex ("-e hex"
> > option)
> 
> Thanks for advice, Mimi.
> 
>