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

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

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

Hello!

I just noticed some unexpected behavior. Kernel is 4.4.11, with IMA
enabled in the build configuration.

After booting, copying files which have a hash in security.ima (like
0x0196d920f04ec85c33438117740d5dcae7bbdbfe3a) with rsync or bsdtar fails
in fsetxattr with permission denied. From bsdtar:

open("build", O_WRONLY|O_CREAT|O_EXCL|O_LARGEFILE|O_CLOEXEC, 0644) = 3
fcntl64(3, F_GETFD)                     = 0x1 (flags FD_CLOEXEC)
write(3, "-----------------------\nBuild Co"..., 2022) = 2022
...
fchown32(3, 0, 0)                       = 0
utimensat(3, NULL, [{1467894919, 0}, {1467894885, 0}], 0) = 0
fsetxattr(3, "security.SMACK64", "System::Shared", 14, 0) = 0
fsetxattr(3, "security.ima", "\1\226\331 \360N\310\\3C\201\27t\r]\312\347\273\333\376:", 21, 0) = -1 EPERM (Operation not permitted)
close(3)                                = 0

It does not matter whether an IMA policy has been loaded or not.

fsetxattr() succeeds if the security.ima value is a signed hash.

We ran into this while working on an installer which runs from the
initramfs and copies the content of the rootfs from an USB stick to some
other media, like internal flash.

Ideally, that installer should get started before loading an IMA policy.
That way, writing files will be faster (no hashing). But that doesn't
work because the security.ima of hashed files cannot be set.

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

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

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

-- 
Best Regards, Patrick Ohly

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

Re: [Linux-ima-user] Calculate Template Hash using ruby

[Patrick C. <pat...@us...>]

Looking at some test code to verify the measurement list, I modified the
ruby sample code for "ima-ng" template type entries to work with "ima-sig"
template type entries given there is no signature in the measurement list
entry. The code is here:

# For the entry:
# 10 0093baefab9ca1b8f16e5195e201b9aee9a5b73d ima-sig
# sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855
# /sys/fs/cgroup/systemd/release_agent

#!/usr/bin/env ruby
require 'digest/sha1'
fhash="e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"
fhashhex = [fhash].pack('H*')
fname0="/sys/fs/cgroup/systemd/release_agent\00"
thash="0093baefab9ca1b8f16e5195e201b9aee9a5b73d"
algname="sha256:\00"
signature_length = 0
hash = Digest::SHA1.hexdigest([fhashhex.length+algname.length].pack('i') +
algname  + fhashhex +  [fname0.length].pack('i')  + fname0 +
[signature_length].pack('i'))
print "\nTest: #{thash==hash} ima #{thash} hash: #{hash} \n"


Patrick Callaghan

Re: [Linux-ima-user] Calculate Template Hash using ruby

[Patrick C. <pat...@us...>]

In the thread at
https://sourceforge.net/p/linux-ima/mailman/message/33192047/,
Jens, with help from Andreas, documents a ruby script that verifies the
target hash with a calculated hash for a "ima-ng" entry in the
ascii_measurement_list. Does anyone have a similar ruby or python script
for a "ima-sig" entry?

Note, the ruby script for ima-ng verification is as follows (and parsing
the entry of
"10 79fa39c792abfd03ba7569e1900d155f993b01e8 ima-ng
sha1:037d38f6148770b9565ba3f6b532c7794e37e025 /sbin/init"):

#!/usr/bin/env ruby
require 'digest/sha1'
fhash="037d38f6148770b9565ba3f6b532c7794e37e025"
fhashhex = [fhash].pack('H*')
fname0="/sbin/init\x00"
thash="79fa39c792abfd03ba7569e1900d155f993b01e8"
algname="sha1:\00"
hash = Digest::SHA1.hexdigest([fhashhex.length+algname.length].pack('i')
+ algname  + fhashhex +  [fname0.length].pack('i')  + fname0)
print "\nTest: #{thash==hash} ima #{thash} hash: #{hash} \n"

Thank you.

Patrick Callaghan

[Linux-ima-user] [PATCH v2] ima: fix the string representation of the LSM/IMA hook enumeration ordering

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

This patch fixes the string representation of the LSM/IMA hook enumeration
ordering used for displaying the IMA policy.

Fixes: d9ddf077bb85 ("ima: support for kexec image and initramfs")
Signed-off-by: Mimi Zohar <zo...@li...>
---
 security/integrity/ima/ima_policy.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index be09e2c..3cd0a58 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -884,10 +884,10 @@ static char *func_tokens[] = {
 	"BPRM_CHECK",
 	"MODULE_CHECK",
 	"FIRMWARE_CHECK",
+	"POST_SETATTR",
 	"KEXEC_KERNEL_CHECK",
 	"KEXEC_INITRAMFS_CHECK",
-	"POLICY_CHECK",
-	"POST_SETATTR"
+	"POLICY_CHECK"
 };
 
 void *ima_policy_start(struct seq_file *m, loff_t *pos)
-- 
2.1.0

[Linux-ima-user] [PATCH] ima: fix func_ enumeration ordering

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

This patch fixes the string enumeration ordering used for displaying the
IMA policy.  A better method for keeping the enumeration and string in
sync would be to use preprocessing macros.

Fixes: d9ddf077bb85 ("ima: support for kexec image and initramfs")
Signed-off-by: Mimi Zohar <zo...@li...>
---
 security/integrity/ima/ima_policy.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index be09e2c..2cc5c07 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -873,9 +873,9 @@ static char *mask_tokens[] = {
 
 enum {
 	func_file = 0, func_mmap, func_bprm,
-	func_module, func_firmware, func_post,
+	func_module, func_firmware,
 	func_kexec_kernel, func_kexec_initramfs,
-	func_policy
+	func_policy, func_post
 };
 
 static char *func_tokens[] = {
-- 
2.1.0

Re: [Linux-ima-user] [PATCH] IMA: add INTEGRITY_ASYMMETRIC_KEYS dependency

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

Hi Arnd,

On Sat, 2016-04-16 at 22:33 +0200, Arnd Bergmann wrote:
> The newly added CONFIG_IMA_KEYRINGS_PERMIT_SIGNED_BY_BUILTIN_OR_SECONDARY
> option only makes sense in combination with INTEGRITY_ASYMMETRIC_KEYS,
> otherwise we get a build error:
> 
> warning: (IMA_KEYRINGS_PERMIT_SIGNED_BY_BUILTIN_OR_SECONDARY) selects INTEGRITY_TRUSTED_KEYRING which has unmet direct dependencies (INTEGRITY && SYSTEM_TRUSTED_KEYRING && INTEGRITY_ASYMMETRIC_KEYS)
> security/integrity/evm/evm_main.c: In function 'evm_load_x509':
> security/integrity/evm/evm_main.c:494:7: error: implicit declaration of function 'integrity_load_x509'
>   rc = integrity_load_x509(INTEGRITY_KEYRING_EVM, CONFIG_EVM_X509_PATH);
> 
> This adds a Kconfig dependency.
> 
> Signed-off-by: Arnd Bergmann <ar...@ar...>
> Fixes: 9e1bbe8b8992 ("IMA: Use the the system trusted keyrings instead of .ima_mok")
> ---
>  security/integrity/ima/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
> index aab9b0a53edf..5487827fa86c 100644
> --- a/security/integrity/ima/Kconfig
> +++ b/security/integrity/ima/Kconfig
> @@ -159,6 +159,7 @@ config IMA_KEYRINGS_PERMIT_SIGNED_BY_BUILTIN_OR_SECONDARY
>  	bool "Permit keys validly signed by a built-in or secondary CA cert (EXPERIMENTAL)"
>  	depends on SYSTEM_TRUSTED_KEYRING
>  	depends on SECONDARY_TRUSTED_KEYRING
> +	depends on INTEGRITY_ASYMMETRIC_KEYS
>  	select INTEGRITY_TRUSTED_KEYRING
>  	default n
>  	help

Good catch!  Thank you for reporting the problem.

Enabling this Kconfig option only makes sense if IMA_TRUSTED_KEYRING is
enabled.  I think adding that dependency will resolve the build issues.

Mimi

[Linux-ima-user] [PATCH] IMA: add INTEGRITY_ASYMMETRIC_KEYS dependency

[Arnd B. <ar...@ar...>]

The newly added CONFIG_IMA_KEYRINGS_PERMIT_SIGNED_BY_BUILTIN_OR_SECONDARY
option only makes sense in combination with INTEGRITY_ASYMMETRIC_KEYS,
otherwise we get a build error:

warning: (IMA_KEYRINGS_PERMIT_SIGNED_BY_BUILTIN_OR_SECONDARY) selects INTEGRITY_TRUSTED_KEYRING which has unmet direct dependencies (INTEGRITY && SYSTEM_TRUSTED_KEYRING && INTEGRITY_ASYMMETRIC_KEYS)
security/integrity/evm/evm_main.c: In function 'evm_load_x509':
security/integrity/evm/evm_main.c:494:7: error: implicit declaration of function 'integrity_load_x509'
  rc = integrity_load_x509(INTEGRITY_KEYRING_EVM, CONFIG_EVM_X509_PATH);

This adds a Kconfig dependency.

Signed-off-by: Arnd Bergmann <ar...@ar...>
Fixes: 9e1bbe8b8992 ("IMA: Use the the system trusted keyrings instead of .ima_mok")
---
 security/integrity/ima/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
index aab9b0a53edf..5487827fa86c 100644
--- a/security/integrity/ima/Kconfig
+++ b/security/integrity/ima/Kconfig
@@ -159,6 +159,7 @@ config IMA_KEYRINGS_PERMIT_SIGNED_BY_BUILTIN_OR_SECONDARY
 	bool "Permit keys validly signed by a built-in or secondary CA cert (EXPERIMENTAL)"
 	depends on SYSTEM_TRUSTED_KEYRING
 	depends on SECONDARY_TRUSTED_KEYRING
+	depends on INTEGRITY_ASYMMETRIC_KEYS
 	select INTEGRITY_TRUSTED_KEYRING
 	default n
 	help
-- 
2.7.0

[Linux-ima-user] [PATCH] ima: Use sizeof struct rather than pointer

[Vaishali T. <vai...@or...>]

When sizeof the pointer is applied to a pointer typed expression it
gives the size of the pointer. Here, for the cleanness sake, let's
not use sizeof the pointer. This does not actually change the behavior
of the code as both being pointers, *fields and **fields have the same
size and fields is a ***.

Problem found using Coccinelle.

Signed-off-by: Vaishali Thakkar <vai...@or...>
---
 security/integrity/ima/ima_template.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c
index febd12e..fdba86f 100644
--- a/security/integrity/ima/ima_template.c
+++ b/security/integrity/ima/ima_template.c
@@ -171,11 +171,11 @@ static int template_desc_init_fields(const char *template_fmt,
 	}
 
 	if (fields && num_fields) {
-		*fields = kmalloc_array(i, sizeof(*fields), GFP_KERNEL);
+		*fields = kmalloc_array(i, sizeof(**fields), GFP_KERNEL);
 		if (*fields == NULL)
 			return -ENOMEM;
 
-		memcpy(*fields, found_fields, i * sizeof(*fields));
+		memcpy(*fields, found_fields, i * sizeof(**fields));
 		*num_fields = i;
 	}
 
-- 
2.1.4

Re: [Linux-ima-user] Does ima work on a armv8 platform?

[Dmitry K. <dmi...@gm...>]

Hi,

It does not depend on HW itself.
You can enable it on any HW..
The difference is that ARM HW does not have TPMs, so it is not
possible to extent PCRs...

Dmitry



On Wed, Mar 30, 2016 at 6:15 PM, 杨子涵 <yzh...@12...> wrote:
> Hi, Dmitry,
> Thanks for your reply.
> Could you tell me how do I configure it on Hikey? Can I directly use the
> similar configuration for Huawei P8 or I need additional configs depending
> on my board?
> I have found similar defconfig file but I don't know wether I can use it
> directly.
>
> Thanks.
>
>
>
> At 2016-03-30 23:02:20, "Dmitry Kasatkin" <dmi...@gm...> wrote:
>>Hi,
>>
>>Yes, it works on ARMv8 or v7
>>
>>Dmitry
>>
>>On Wed, Mar 30, 2016 at 2:52 PM, 杨子涵 <yzh...@12...> wrote:
>>> Hi, all,
>>> I'm a new user of IMA. I want to use it on on Hikey board(with Kirin 620
>>> 64
>>> bit ARM CPU) and I have two questions about it. I would appreicate it
>>> very
>>> much if any of you could give some clues.
>>> Here are my questions:
>>>
>>> 1. I find in your wiki page that you have tested it on Huawei P8 phone
>>> which
>>> is equipped with the HiSilicon Kirin 930/935 64 bit ARM CPU, but it
>>> doesn't
>>> tell me wether it supports other armv8 platforms. I'm using a Hikey board
>>> whose CPU is Kirin 620 64 bit. The AOSP has already supported Hikey (see
>>> reference
>>> https://source.android.com/source/devices.html#running-android-hikey  ),
>>> but
>>> IMA has not been tested on it, so I wonder if it will work on Hikey. Do I
>>> need special configurations other than the ones listed in the wiki page?
>>>
>>> 2. Your wiki page tells me that IMA/EVM support was implemented using
>>> Android 5.0.2 source tree. What I would like to know is that wether I can
>>> use it on other arm linux systems, for example, on a 4.4.0 aarch64 linux
>>> instead of Android? Because I don't have a display screen available, so
>>> running Android on it may not be feasible for me at present.
>>>
>>> Thanks a lot.
>>> Tgn
>>>
>>>
>>>
>>>
>>>
>>>
>>> ------------------------------------------------------------------------------
>>> Transform Data into Opportunity.
>>> Accelerate data analysis in your applications with
>>> Intel Data Analytics Acceleration Library.
>>> Click to learn more.
>>> http://pubads.g.doubleclick.net/gampad/clk?id=278785471&iu=/4140
>>> _______________________________________________
>>> Linux-ima-user mailing list
>>> Lin...@li...
>>> https://lists.sourceforge.net/lists/listinfo/linux-ima-user
>>>
>>
>>
>>
>>--
>>Thanks,
>>Dmitry
>
>
>
>



-- 
Thanks,
Dmitry

Re: [Linux-ima-user] Does ima work on a armv8 platform?

[Dmitry K. <dmi...@gm...>]

Hi,

Yes, it works on ARMv8 or v7

Dmitry

On Wed, Mar 30, 2016 at 2:52 PM, 杨子涵 <yzh...@12...> wrote:
> Hi, all,
> I'm a new user of IMA. I want to use it on on Hikey board(with Kirin 620 64
> bit ARM CPU) and I have two questions about it. I would appreicate it very
> much if any of you could give some clues.
> Here are my questions:
>
> 1. I find in your wiki page that you have tested it on Huawei P8 phone which
> is equipped with the HiSilicon Kirin 930/935 64 bit ARM CPU, but it doesn't
> tell me wether it supports other armv8 platforms. I'm using a Hikey board
> whose CPU is Kirin 620 64 bit. The AOSP has already supported Hikey (see
> reference
> https://source.android.com/source/devices.html#running-android-hikey  ), but
> IMA has not been tested on it, so I wonder if it will work on Hikey. Do I
> need special configurations other than the ones listed in the wiki page?
>
> 2. Your wiki page tells me that IMA/EVM support was implemented using
> Android 5.0.2 source tree. What I would like to know is that wether I can
> use it on other arm linux systems, for example, on a 4.4.0 aarch64 linux
> instead of Android? Because I don't have a display screen available, so
> running Android on it may not be feasible for me at present.
>
> Thanks a lot.
> Tgn
>
>
>
>
>
> ------------------------------------------------------------------------------
> Transform Data into Opportunity.
> Accelerate data analysis in your applications with
> Intel Data Analytics Acceleration Library.
> Click to learn more.
> http://pubads.g.doubleclick.net/gampad/clk?id=278785471&iu=/4140
> _______________________________________________
> Linux-ima-user mailing list
> Lin...@li...
> https://lists.sourceforge.net/lists/listinfo/linux-ima-user
>



-- 
Thanks,
Dmitry

[Linux-ima-user] Does ima work on a armv8 platform?

[杨子涵 <yzh...@12...>]

Hi, all,
I'm a new user of IMA. I want to use it on on Hikey board(with Kirin 620 64 bit ARM CPU) and I have two questions about it. I would appreicate it very much if any of you could give some clues.
Here are my questions:


1. I find in your wiki page that you have tested it on Huawei P8 phone which is equipped with the HiSilicon Kirin 930/935 64 bit ARM CPU, but it doesn't tell me wether it supports other armv8 platforms. I'm using a Hikey board whose CPU is Kirin 620 64 bit. The AOSP has already supported Hikey (see reference https://source.android.com/source/devices.html#running-android-hikey  ), but IMA has not been tested on it, so I wonder if it will work on Hikey. Do I need special configurations other than the ones listed in the wiki page?


2. Your wiki page tells me that IMA/EVM support was implemented using Android 5.0.2 source tree. What I would like to know is that wether I can use it on other arm linux systems, for example, on a 4.4.0 aarch64 linux instead of Android? Because I don't have a display screen available, so running Android on it may not be feasible for me at present.


Thanks a lot.
Tgn

[Linux-ima-user] [PATCH 2/2] ima: add support for creating files using the mknodat syscall

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

Commit 3034a14 "ima: pass 'opened' flag to identify newly created files"
stopped identifying empty files as new files.  However new empty files
can be created using the mknodat syscall.  On systems with IMA-appraisal
enabled, these empty files are not labeled with security.ima extended
attributes properly, preventing them from subsequently being opened in
order to write the file data contents.  This patch marks these empty
files, created using mknodat, as new in order to allow the file data
contents to be written.

Files with security.ima xattrs containing a file signature are considered
"immutable" and can not be modified.  The file contents need to be
written, before signing the file.  This patch relaxes this requirement
for new files, allowing the file signature to be written before the file
contents.

Signed-off-by: Mimi Zohar <zo...@li...>
---
 fs/namei.c                            |  2 ++
 include/linux/ima.h                   |  7 ++++++-
 security/integrity/ima/ima_appraise.c |  3 +++
 security/integrity/ima/ima_main.c     | 32 +++++++++++++++++++++++++++++++-
 4 files changed, 42 insertions(+), 2 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index f624d13..30b5254 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -3620,6 +3620,8 @@ retry:
 	switch (mode & S_IFMT) {
 		case 0: case S_IFREG:
 			error = vfs_create(path.dentry->d_inode,dentry,mode,true);
+			if (!error)
+				ima_post_path_mknod(dentry);
 			break;
 		case S_IFCHR: case S_IFBLK:
 			error = vfs_mknod(path.dentry->d_inode,dentry,mode,
diff --git a/include/linux/ima.h b/include/linux/ima.h
index e6516cb..164c002 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -21,7 +21,7 @@ extern int ima_file_mmap(struct file *file, unsigned long prot);
 extern int ima_read_file(struct file *file, enum kernel_read_file_id id);
 extern int ima_post_read_file(struct file *file, void *buf, loff_t size,
 			      enum kernel_read_file_id id);
-
+extern void ima_post_path_mknod(struct dentry *dentry);
 #else
 static inline int ima_bprm_check(struct linux_binprm *bprm)
 {
@@ -54,6 +54,11 @@ static inline int ima_post_read_file(struct file *file, void *buf, loff_t size,
 	return 0;
 }
 
+static inline void ima_post_path_mknod(struct dentry *dentry)
+{
+	return;
+}
+
 #endif /* CONFIG_IMA */
 
 #ifdef CONFIG_IMA_APPRAISE
diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index d2f28a0..69a4362 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -207,6 +207,9 @@ int ima_appraise_measurement(enum ima_hooks func,
 		if (opened & FILE_CREATED) {
 			iint->flags |= IMA_NEW_FILE;
 			status = INTEGRITY_PASS;
+		} else if ((inode->i_size == 0) &&
+			    (iint->flags & IMA_NEW_FILE)) {
+			status = INTEGRITY_PASS;
 		}
 		goto out;
 	}
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 391f417..3297270 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -246,7 +246,8 @@ static int process_measurement(struct file *file, char *buf, loff_t size,
 		ima_audit_measurement(iint, pathname);
 
 out_digsig:
-	if ((mask & MAY_WRITE) && (iint->flags & IMA_DIGSIG))
+	if ((mask & MAY_WRITE) && (iint->flags & IMA_DIGSIG) &&
+	     !(iint->flags & IMA_NEW_FILE))
 		rc = -EACCES;
 	kfree(xattr_value);
 out_free:
@@ -316,6 +317,35 @@ int ima_file_check(struct file *file, int mask, int opened)
 EXPORT_SYMBOL_GPL(ima_file_check);
 
 /**
+ * ima_post_path_mknod - mark as a new inode
+ * @dentry: newly created dentry
+ *
+ * Mark files created via the mknodat syscall as new, so that the
+ * file data can be written later.
+ */
+void ima_post_path_mknod(struct dentry *dentry)
+{
+	struct integrity_iint_cache *iint;
+	struct inode *inode;
+	int must_appraise;
+
+	if (!dentry || !dentry->d_inode)
+		return;
+
+	inode = dentry->d_inode;
+	if (inode->i_size != 0)
+		return;
+
+	must_appraise = ima_must_appraise(inode, MAY_ACCESS, FILE_CHECK);
+	if (!must_appraise)
+		return;
+
+	iint = integrity_inode_get(inode);
+	if (iint)
+		iint->flags |= IMA_NEW_FILE;
+}
+
+/**
  * ima_read_file - pre-measure/appraise hook decision based on policy
  * @file: pointer to the file to be measured/appraised/audit
  * @read_id: caller identifier
-- 
2.1.0

[Linux-ima-user] [PATCH 1/2] ima: fix ima_inode_post_setattr

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

Changing file metadata (eg. uid, guid) could result in having to
re-appraise a file's integrity, but does not change the "new file"
status nor the security.ima xattr.  The IMA_PERMIT_DIRECTIO and
IMA_DIGSIG_REQUIRED flags are policy rule specific.  This patch
only resets these flags, not the IMA_NEW_FILE or IMA_DIGSIG flags.

With this patch, changing the file timestamp will not remove the
file signature on new files.

Reported-by: Dmitry Rozhkov <dmi...@li...>
Signed-off-by: Mimi Zohar <zo...@li...>
---
 security/integrity/ima/ima_appraise.c | 2 +-
 security/integrity/integrity.h        | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index 6b4694a..d2f28a0 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -328,7 +328,7 @@ void ima_inode_post_setattr(struct dentry *dentry)
 	if (iint) {
 		iint->flags &= ~(IMA_APPRAISE | IMA_APPRAISED |
 				 IMA_APPRAISE_SUBMASK | IMA_APPRAISED_SUBMASK |
-				 IMA_ACTION_FLAGS);
+				 IMA_ACTION_RULE_FLAGS);
 		if (must_appraise)
 			iint->flags |= IMA_APPRAISE;
 	}
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index e08935c..90bc57d 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -28,6 +28,7 @@
 
 /* iint cache flags */
 #define IMA_ACTION_FLAGS	0xff000000
+#define IMA_ACTION_RULE_FLAGS	0x06000000
 #define IMA_DIGSIG		0x01000000
 #define IMA_DIGSIG_REQUIRED	0x02000000
 #define IMA_PERMIT_DIRECTIO	0x04000000
-- 
2.1.0

[Linux-ima-user] [PATCH 0/2] ima: tar issues

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

This patch set addresses a couple of problems with writing security.ima
xattrs from tar.  The first patch prevents file signatures stored in the
security.ima xattr from being replaced when the timestamp is updated.
The second patch identifies empty files created using mknodat as new, so
that the file can subsequently be opened in order to write the file
contents.

Mimi 


Mimi Zohar (2):
  ima: fix ima_inode_post_setattr
  ima: add support for creating files using the mknodat syscall

 fs/namei.c                            |  2 ++
 include/linux/ima.h                   |  7 ++++++-
 security/integrity/ima/ima_appraise.c |  5 ++++-
 security/integrity/ima/ima_main.c     | 32 +++++++++++++++++++++++++++++++-
 security/integrity/integrity.h        |  1 +
 5 files changed, 44 insertions(+), 3 deletions(-)

-- 
2.1.0

Re: [Linux-ima-user] [PATCH] ima: fix ima_inode_post_setattr

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

On Mon, 2016-03-07 at 09:40 +0100, Patrick Ohly wrote:
> On Mon, 2016-02-29 at 22:00 -0500, Mimi Zohar wrote:
> > Changing file metadata (eg. uid, guid) could result in having to
> > re-appraise a file's integrity, but does not change the "new file"
> > status nor the security.ima xattr.  The IMA_PERMIT_DIRECTIO and
> > IMA_DIGSIG_REQUIRED flags are policy rule specific.  This patch
> > only resets these flags, not the IMA_NEW_FILE or IMA_DIGSIG flags.
> > 
> > With this patch, changing the file timestamp will not remove the
> > file signature on new files.
> 
> As Dmitry said, this indeed works. Thanks a lot!
> 
> Will you queue it for inclusion in upstream Linux? In which version do
> you expect it to land?

I've queued a couple of patches in next-4.7,  including this one.
Although this patch works, it could probably be cleaned up a bit.

Mimi

Re: [Linux-ima-user] [PATCH] ima: fix ima_inode_post_setattr

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

On Mon, 2016-02-29 at 22:00 -0500, Mimi Zohar wrote:
> Changing file metadata (eg. uid, guid) could result in having to
> re-appraise a file's integrity, but does not change the "new file"
> status nor the security.ima xattr.  The IMA_PERMIT_DIRECTIO and
> IMA_DIGSIG_REQUIRED flags are policy rule specific.  This patch
> only resets these flags, not the IMA_NEW_FILE or IMA_DIGSIG flags.
> 
> With this patch, changing the file timestamp will not remove the
> file signature on new files.

As Dmitry said, this indeed works. Thanks a lot!

Will you queue it for inclusion in upstream Linux? In which version do
you expect it to land?

-- 
Best Regards, Patrick Ohly

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

Re: [Linux-ima-user] [PATCH] ima: fix ima_inode_post_setattr

[Dmitry R. <dmi...@li...>]

Hi!

I can confirm the original bsdtar unpacks archives correctly with this
patch applied to the kernel, the security.ima attribute doesn't get
overwritten by IMA. Thanks!

BR,
Dmitry

On Mon, 2016-02-29 at 22:00 -0500, Mimi Zohar wrote:
> Changing file metadata (eg. uid, guid) could result in having to
> re-appraise a file's integrity, but does not change the "new file"
> status nor the security.ima xattr.  The IMA_PERMIT_DIRECTIO and
> IMA_DIGSIG_REQUIRED flags are policy rule specific.  This patch
> only resets these flags, not the IMA_NEW_FILE or IMA_DIGSIG flags.
> 
> With this patch, changing the file timestamp will not remove the
> file signature on new files.
> 
> Reported-by: Dmitry Rozhkov <dmi...@li...>
> Signed-off-by: Mimi Zohar <zo...@li...>
> ---
>  security/integrity/ima/ima_appraise.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/security/integrity/ima/ima_appraise.c
> b/security/integrity/ima/ima_appraise.c
> index 6b4694a..e304e3a 100644
> --- a/security/integrity/ima/ima_appraise.c
> +++ b/security/integrity/ima/ima_appraise.c
> @@ -328,7 +328,7 @@ void ima_inode_post_setattr(struct dentry
> *dentry)
>  	if (iint) {
>  		iint->flags &= ~(IMA_APPRAISE | IMA_APPRAISED |
>  				 IMA_APPRAISE_SUBMASK |
> IMA_APPRAISED_SUBMASK |
> -				 IMA_ACTION_FLAGS);
> +				 IMA_PERMIT_DIRECTIO |
> IMA_DIGSIG_REQUIRED);
>  		if (must_appraise)
>  			iint->flags |= IMA_APPRAISE;
>  	}

[Linux-ima-user] [PATCH] ima: fix ima_inode_post_setattr

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

Changing file metadata (eg. uid, guid) could result in having to
re-appraise a file's integrity, but does not change the "new file"
status nor the security.ima xattr.  The IMA_PERMIT_DIRECTIO and
IMA_DIGSIG_REQUIRED flags are policy rule specific.  This patch
only resets these flags, not the IMA_NEW_FILE or IMA_DIGSIG flags.

With this patch, changing the file timestamp will not remove the
file signature on new files.

Reported-by: Dmitry Rozhkov <dmi...@li...>
Signed-off-by: Mimi Zohar <zo...@li...>
---
 security/integrity/ima/ima_appraise.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index 6b4694a..e304e3a 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -328,7 +328,7 @@ void ima_inode_post_setattr(struct dentry *dentry)
 	if (iint) {
 		iint->flags &= ~(IMA_APPRAISE | IMA_APPRAISED |
 				 IMA_APPRAISE_SUBMASK | IMA_APPRAISED_SUBMASK |
-				 IMA_ACTION_FLAGS);
+				 IMA_PERMIT_DIRECTIO | IMA_DIGSIG_REQUIRED);
 		if (must_appraise)
 			iint->flags |= IMA_APPRAISE;
 	}
-- 
2.1.0

Re: [Linux-ima-user] IMA overwrites xattr even if it has got correct signature

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

On Mon, 2016-02-29 at 14:11 +0200, Dmitry Rozhkov wrote:
> On Fri, 2016-02-26 at 17:53 +0100, Patrick Ohly wrote:
> > On Fri, 2016-02-26 at 18:26 +0200, Dmitry Rozhkov wrote:
> > > Hm. I've just tried to reproduce my use case with the following
> > > simple
> > > test and got perfectly correct results. Looks like there's
> > > something
> > > fishy with bsdtar still.
> > 
> > Can you do strace dumps to compare the actual syscalls?
> > 
> 
> Yep, strace shows that bsdtar do utimensat() after fsetxattr() and
> before close().
> 
> The test program below reproduces the problem.
> 
> I think if timestamps are not considered to be a part of the hash sums
> (and it seems to be the case since the IMA hash is the same for the
> same file but with different timestamps) then it should be better fixed
> in IMA. But patching bsdtar to have setting xattrs the last operation
> before close() would not harm too.

Thank you for trouble shooting this.  The new file and digsig flags are
being reset in ima_post_setattr().  I'll post a patch shortly.

Mimi

Re: [Linux-ima-user] IMA overwrites xattr even if it has got correct signature

[Dmitry R. <dmi...@li...>]

On Fri, 2016-02-26 at 17:53 +0100, Patrick Ohly wrote:
> On Fri, 2016-02-26 at 18:26 +0200, Dmitry Rozhkov wrote:
> > Hm. I've just tried to reproduce my use case with the following
> > simple
> > test and got perfectly correct results. Looks like there's
> > something
> > fishy with bsdtar still.
> 
> Can you do strace dumps to compare the actual syscalls?
> 

Yep, strace shows that bsdtar do utimensat() after fsetxattr() and
before close().

The test program below reproduces the problem.

I think if timestamps are not considered to be a part of the hash sums
(and it seems to be the case since the IMA hash is the same for the
same file but with different timestamps) then it should be better fixed
in IMA. But patching bsdtar to have setting xattrs the last operation
before close() would not harm too.

#include <sys/types.h>
#include <fcntl.h>
#include <sys/stat.h>
#include <unistd.h>
#include <sys/xattr.h>
#include <string.h>
#include <errno.h>
#include <stdio.h>

enum evm_ima_xattr_type {
        IMA_XATTR_DIGEST = 0x01,
        EVM_XATTR_HMAC,
        EVM_IMA_XATTR_DIGSIG,
        IMA_XATTR_DIGEST_NG,
        IMA_XATTR_LAST
};

int main()
{
        char fakesig[256] =
"abracadabraabracadabraabracadabraabracadabraabracadabraabracadabraabra
cadabraabracadabra";
        fakesig[0] = EVM_IMA_XATTR_DIGSIG;
        int fd = open("test", O_WRONLY | O_CREAT);
        write(fd, "hello", 5);
        fchown(fd, 0, 0);
        int ret = fsetxattr(fd, "security.ima", fakesig,
sizeof(fakesig), 0);

        struct timespec ts[2];
        ts[0].tv_sec = 1456840352;
        ts[0].tv_nsec = 842331383;
        ts[1].tv_sec = 1456692085;
        ts[1].tv_nsec = 1;
        futimens(fd, ts);

        close(fd);
        if (ret) {
            printf("Oops: %s\n", strerror(errno));
        }
        return ret;
}

BR,
Dima

[Linux-ima-user] Signature scheme

[Benjamin H. <he...@dc...>]

Hi,

could someone please briefly explain to the non-experts regarding 
openssl api what kind of signature is used by IMA/EVM in the two 
sign_hash functions as it can be found in 
https://sourceforge.net/p/linux-ima/ima-evm-utils/ci/master/tree/src/libimaevm.c. 
What scheme and maybe what kind of encoding is used? I am looking for 
doing the signature with external tooling and thus have to find out if 
this could work. Or the other way around, what kind of signature can be 
used with IMA/EVM from kernel point of view?

Best regards
Benjamin

Re: [Linux-ima-user] IMA overwrites xattr even if it has got correct signature

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

On Fri, 2016-02-26 at 18:26 +0200, Dmitry Rozhkov wrote:
> Hm. I've just tried to reproduce my use case with the following simple
> test and got perfectly correct results. Looks like there's something
> fishy with bsdtar still.

Can you do strace dumps to compare the actual syscalls?

-- 
Best Regards, Patrick Ohly

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

Re: [Linux-ima-user] IMA overwrites xattr even if it has got correct signature

[Dmitry R. <dmi...@li...>]

On Fri, 2016-02-26 at 08:24 -0500, Mimi Zohar wrote:
> On Fri, 2016-02-26 at 15:00 +0200, Dmitry Rozhkov wrote:
> > On Fri, 2016-02-26 at 06:48 -0500, Mimi Zohar wrote:
> > > On Fri, 2016-02-26 at 08:52 +0100, Patrick Ohly wrote:
> > > > On Thu, 2016-02-25 at 11:09 -0500, Mimi Zohar wrote:
> > > > > On Thu, 2016-02-25 at 15:29 +0200, Dmitry Rozhkov wrote:
> > > > > > On Thu, 2016-02-25 at 07:43 -0500, Mimi Zohar wrote:
> > > > > 
> > > > > > > 
> > > > > > > If security.ima isn't being written because the file is
> > > > > > > not
> > > > > > > in policy,
> > > > > > > that is a different problem.   Perhaps try writing a file
> > > > > > > that is in the
> > > > > > > IMA-appraisal policy.
> > > > > > 
> > > > > > Didn't get it. The file is in the IMA-appraisal policy.
> > > > > > 
> > > > > > AFAIU upon file closing ima_inode_setxattr()
> > > > > > calls ima_reset_appraise_flags() where IMA_DIGSIG is
> > > > > > supposed
> > > > > > to be
> > > > > > set. But right before setting the flag there's the
> > > > > > condition
> > > > > > 
> > > > > > 	iint = integrity_iint_find(inode);
> > > > > > 	if (!iint)
> > > > > > 		return;
> > > > > > 
> > > > > > which is true for newly created (and never closed yet)
> > > > > > files.
> > > > > 
> > > > > There needs to be a FILE_CHECK rule for the iint to be
> > > > > created
> > > > > for a new
> > > > > file.
> > > > 
> > > > The policy is essentially just:
> > > > 
> > > > appraise fowner=0
> > > > measure fowner=0
> > > > 
> > > > with some dont_appraise/measure entries earlier for some
> > > > special
> > > > filesystems.
> > > 
> > > This policy measures and appraises everything owned by root.  So
> > > as
> > > long
> > > as the file is owned by root, the file will be in the file open
> > > (FILE_CHECK) policy.
> > > 
> > 
> > In other words with the policy like
> > 
> > appraise fowner=0
> > measure fowner=0 
> > 
> > the FILE_CHECK rule is implied for all files and iint is guarantied
> > to
> > be created. Then something else is missing for IMA_DIGSIG to be
> > set. I
> > run out of ideas on what it could be.
> 
> No, not all files are included in the policy.  Only files owned by
> root
> on filesystems not explicitly excluded.  Try opening a file for write
> and from another process write the signature as an xattr, before
> closing
> the file.  security.ima should contain the signature after close.

Hm. I've just tried to reproduce my use case with the following simple
test and got perfectly correct results. Looks like there's something
fishy with bsdtar still. Back to drawing board...

#include <sys/types.h>
#include <fcntl.h>
#include <sys/stat.h>
#include <unistd.h>
#include <sys/xattr.h>
#include <string.h>
#include <errno.h>
#include <stdio.h>

enum evm_ima_xattr_type {
        IMA_XATTR_DIGEST = 0x01,
        EVM_XATTR_HMAC,
        EVM_IMA_XATTR_DIGSIG,
        IMA_XATTR_DIGEST_NG,
        IMA_XATTR_LAST
};

int main()
{
        char fakesig[256] =
"abracadabraabracadabraabracadabraabracadabraabracadabraabracadabraabra
cadabraabracadabra";
        fakesig[0] = EVM_IMA_XATTR_DIGSIG;
        int fd = open("test", O_WRONLY | O_CREAT);
        write(fd, "hello", 5);
        int ret = fsetxattr(fd, "security.ima", fakesig,
sizeof(fakesig), 0);
        close(fd);
        if (ret) {
            printf("Oops: %s\n", strerror(errno));
        }
        return ret;
}

BR,
Dima

Re: [Linux-ima-user] IMA overwrites xattr even if it has got correct signature

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

On Fri, 2016-02-26 at 15:00 +0200, Dmitry Rozhkov wrote:
> On Fri, 2016-02-26 at 06:48 -0500, Mimi Zohar wrote:
> > On Fri, 2016-02-26 at 08:52 +0100, Patrick Ohly wrote:
> > > On Thu, 2016-02-25 at 11:09 -0500, Mimi Zohar wrote:
> > > > On Thu, 2016-02-25 at 15:29 +0200, Dmitry Rozhkov wrote:
> > > > > On Thu, 2016-02-25 at 07:43 -0500, Mimi Zohar wrote:
> > > > 
> > > > > > 
> > > > > > If security.ima isn't being written because the file is not
> > > > > > in policy,
> > > > > > that is a different problem.   Perhaps try writing a file
> > > > > > that is in the
> > > > > > IMA-appraisal policy.
> > > > > 
> > > > > Didn't get it. The file is in the IMA-appraisal policy.
> > > > > 
> > > > > AFAIU upon file closing ima_inode_setxattr()
> > > > > calls ima_reset_appraise_flags() where IMA_DIGSIG is supposed
> > > > > to be
> > > > > set. But right before setting the flag there's the condition
> > > > > 
> > > > > 	iint = integrity_iint_find(inode);
> > > > > 	if (!iint)
> > > > > 		return;
> > > > > 
> > > > > which is true for newly created (and never closed yet) files.
> > > > 
> > > > There needs to be a FILE_CHECK rule for the iint to be created
> > > > for a new
> > > > file.
> > > 
> > > The policy is essentially just:
> > > 
> > > appraise fowner=0
> > > measure fowner=0
> > > 
> > > with some dont_appraise/measure entries earlier for some special
> > > filesystems.
> > 
> > This policy measures and appraises everything owned by root.  So as
> > long
> > as the file is owned by root, the file will be in the file open
> > (FILE_CHECK) policy.
> > 
> 
> In other words with the policy like
> 
> appraise fowner=0
> measure fowner=0 
> 
> the FILE_CHECK rule is implied for all files and iint is guarantied to
> be created. Then something else is missing for IMA_DIGSIG to be set. I
> run out of ideas on what it could be.

No, not all files are included in the policy.  Only files owned by root
on filesystems not explicitly excluded.  Try opening a file for write
and from another process write the signature as an xattr, before closing
the file.  security.ima should contain the signature after close.

Mimi

Re: [Linux-ima-user] IMA overwrites xattr even if it has got correct signature

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

On Fri, 2016-02-26 at 06:48 -0500, Mimi Zohar wrote:
> On Fri, 2016-02-26 at 08:52 +0100, Patrick Ohly wrote:
> > On Thu, 2016-02-25 at 11:09 -0500, Mimi Zohar wrote:
> > > On Thu, 2016-02-25 at 15:29 +0200, Dmitry Rozhkov wrote:
> > > > On Thu, 2016-02-25 at 07:43 -0500, Mimi Zohar wrote:
> > > 
> > > > > 
> > > > > If security.ima isn't being written because the file is not in policy,
> > > > > that is a different problem.   Perhaps try writing a file that is in the
> > > > > IMA-appraisal policy.
> > > > 
> > > > Didn't get it. The file is in the IMA-appraisal policy.
> > > > 
> > > > AFAIU upon file closing ima_inode_setxattr()
> > > > calls ima_reset_appraise_flags() where IMA_DIGSIG is supposed to be
> > > > set. But right before setting the flag there's the condition
> > > > 
> > > > 	iint = integrity_iint_find(inode);
> > > > 	if (!iint)
> > > > 		return;
> > > > 
> > > > which is true for newly created (and never closed yet) files.
> > > 
> > > There needs to be a FILE_CHECK rule for the iint to be created for a new
> > > file.
> > 
> > The policy is essentially just:
> > 
> > appraise fowner=0
> > measure fowner=0
> > 
> > with some dont_appraise/measure entries earlier for some special
> > filesystems.
> 
> This policy measures and appraises everything owned by root.  So as long
> as the file is owned by root, the file will be in the file open
> (FILE_CHECK) policy.

Then it remains unclear why the kernel behaves the way it does. The file
Dmitry mentioned must have been owned by root (Dmitry, can you
double-check?), because otherwise it would not have been hashed.

However, which attributes are relevant here? When a new is file created
by root and then potentially has fchown() called on it before close(),
will it still get hashed upon close() even when it is no longer owned by
root at that time?

> > My understanding was that adding a "func" attribute just further limits
> > the scope of a rule. But to be honest, the exact meaning of the various
> > keywords remain a black art to me. I'm aware of the syntax definition in
> > the kernel doc, but that document does not explain the semantic.
> 
> When we first started working on IMA, there was the classic debate of
> safety over performance.  In this case, the debate was over how much or
> how little to measure and appraise.   The thinking was the more you
> measured and appraised the safer the system would be, but performance
> would be impacted.  So, the ima_tcb/ima_appraise_tcb, that we have
> today, attempts to include all files in the tcb.  It is a compromise
> between measuring and appraising too much or too little.
> 
> Work still needs to be done on defining measurement and appraisal
> policies.  Last year Greg Wettstein identified some files that should
> have been measured, but were not included in the ima_tcb.  The newer
> "ima_policy=tcb" includes those files.  Going forward, we should start
> defining and sharing policies. 

What I meant was that the policy language and its meaning is properly
defined.

-- 
Best Regards, Patrick Ohly

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