[PATCH 1/2] evm: add file system uuid to EVM hmac

[PATCH 1/2] evm: add file system uuid to EVM hmac

[Mimi Zohar]

From: Dmitry Kasatkin <dmitry.kasatkin@intel.com>

EVM uses the same key for all file systems to calculate the HMAC,
making it possible to paste inodes from one file system on to another
one, without EVM being able to detect it.  To prevent such an attack,
it is necessary to make the EVM HMAC file system specific.

This patch uses the file system UUID, a file system unique identifier,
to bind the EVM HMAC to the file system. The value inode->i_sb->s_uuid
is used for the HMAC hash calculation, instead of using it for deriving
the file system specific key.  Initializing the key for every inode HMAC
calculation is a bit more expensive operation than adding the uuid to
the HMAC hash.

Changing the HMAC calculation method or adding additional info to the
calculation, requires existing EVM labeled file systems to be relabeled.
This patch adds a Kconfig HMAC version option for backwards compatability.

Changelog v1:
- squash "hmac version setting"
Changelog v0:
- add missing Kconfig depends (Mimi)

Signed-off-by: Dmitry Kasatkin <dmitry.kasatkin@intel.com>
Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
---
 security/integrity/evm/Kconfig      | 13 +++++++++++++
 security/integrity/evm/evm.h        |  1 +
 security/integrity/evm/evm_crypto.c |  3 +++
 security/integrity/evm/evm_main.c   |  1 +
 4 files changed, 18 insertions(+)

diff --git a/security/integrity/evm/Kconfig b/security/integrity/evm/Kconfig
index afbb59d..fea9749 100644
--- a/security/integrity/evm/Kconfig
+++ b/security/integrity/evm/Kconfig
@@ -11,3 +11,16 @@ config EVM
 	  integrity attacks.
 
 	  If you are unsure how to answer this question, answer N.
+
+config EVM_HMAC_VERSION
+	int "EVM HMAC version"
+	depends on EVM
+	default 2
+	help
+	  This options adds EVM HMAC version support.
+	  1 - original version
+	  2 - add per filesystem unique identifier (UUID) (default)
+
+	  WARNING: changing the HMAC calculation method or adding 
+	  additional info to the calculation, requires existing EVM
+	  labeled file systems to be relabeled.  
diff --git a/security/integrity/evm/evm.h b/security/integrity/evm/evm.h
index 3eb30c6..30bd1ec 100644
--- a/security/integrity/evm/evm.h
+++ b/security/integrity/evm/evm.h
@@ -24,6 +24,7 @@
 extern int evm_initialized;
 extern char *evm_hmac;
 extern char *evm_hash;
+extern int evm_hmac_version;
 
 extern struct crypto_shash *hmac_tfm;
 extern struct crypto_shash *hash_tfm;
diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
index dfb2691..ff8e2ab 100644
--- a/security/integrity/evm/evm_crypto.c
+++ b/security/integrity/evm/evm_crypto.c
@@ -110,6 +110,9 @@ static void hmac_add_misc(struct shash_desc *desc, struct inode *inode,
 	hmac_misc.gid = from_kgid(&init_user_ns, inode->i_gid);
 	hmac_misc.mode = inode->i_mode;
 	crypto_shash_update(desc, (const u8 *)&hmac_misc, sizeof hmac_misc);
+	if (evm_hmac_version > 1)
+		crypto_shash_update(desc, inode->i_sb->s_uuid,
+				    sizeof(inode->i_sb->s_uuid));
 	crypto_shash_final(desc, digest);
 }
 
diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
index a78a5e2..cdbde17 100644
--- a/security/integrity/evm/evm_main.c
+++ b/security/integrity/evm/evm_main.c
@@ -26,6 +26,7 @@ int evm_initialized;
 
 char *evm_hmac = "hmac(sha1)";
 char *evm_hash = "sha1";
+int evm_hmac_version = CONFIG_EVM_HMAC_VERSION;
 
 char *evm_config_xattrnames[] = {
 #ifdef CONFIG_SECURITY_SELINUX
-- 
1.8.1.rc3

[PATCH 2/2] ima: add policy support for file system uuid

[Mimi Zohar]

From: Dmitry Kasatkin <dmitry.kasatkin@intel.com>

The IMA policy permits specifying rules to enable or disable
measurement/appraisal/audit based on the file system magic number.
If, for example, the policy contains an ext4 measurement rule,
the rule is enabled for all ext4 partitions.

Sometimes it might be necessary to enable measurement/appraisal/audit
only for one partition and disable it for another partition of the
same type.  With the existing IMA policy syntax, this can not be done.

This patch provides support for IMA policy rules to specify the file
system by its UUID (eg. fsuuid=397449cd-687d-4145-8698-7fed4a3e0363).

For partitions not being appraised, it might be a good idea to mount
file systems with the 'noexec' option to prevent executing non-verified
binaries.

Signed-off-by: Dmitry Kasatkin <dmitry.kasatkin@intel.com>
Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
---
 Documentation/ABI/testing/ima_policy |  4 +++-
 security/integrity/ima/ima_policy.c  | 22 +++++++++++++++++++++-
 2 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy
index de16de3..f1c5cc9 100644
--- a/Documentation/ABI/testing/ima_policy
+++ b/Documentation/ABI/testing/ima_policy
@@ -19,7 +19,8 @@ Description:
 
 		action: measure | dont_measure | appraise | dont_appraise | audit
 		condition:= base | lsm  [option]
-			base:	[[func=] [mask=] [fsmagic=] [uid=] [fowner]]
+			base:	[[func=] [mask=] [fsmagic=] [fsuuid=] [uid=]
+				 [fowner]]
 			lsm:	[[subj_user=] [subj_role=] [subj_type=]
 				 [obj_user=] [obj_role=] [obj_type=]]
 			option:	[[appraise_type=]]
@@ -27,6 +28,7 @@ Description:
 		base: 	func:= [BPRM_CHECK][MMAP_CHECK][FILE_CHECK][MODULE_CHECK]
 			mask:= [MAY_READ] [MAY_WRITE] [MAY_APPEND] [MAY_EXEC]
 			fsmagic:= hex value
+			fsuuid:= file system UUID (e.g 8bcbe394-4f13-4144-be8e-5aa9ea2ce2f6)
 			uid:= decimal value
 			fowner:=decimal value
 		lsm:  	are LSM specific
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index 4adcd0f..23f49e3 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -16,6 +16,7 @@
 #include <linux/magic.h>
 #include <linux/parser.h>
 #include <linux/slab.h>
+#include <linux/genhd.h>
 
 #include "ima.h"
 
@@ -25,6 +26,7 @@
 #define IMA_FSMAGIC	0x0004
 #define IMA_UID		0x0008
 #define IMA_FOWNER	0x0010
+#define IMA_FSUUID	0x0020
 
 #define UNKNOWN		0
 #define MEASURE		0x0001	/* same as IMA_MEASURE */
@@ -45,6 +47,7 @@ struct ima_rule_entry {
 	enum ima_hooks func;
 	int mask;
 	unsigned long fsmagic;
+	u8 fsuuid[16];
 	kuid_t uid;
 	kuid_t fowner;
 	struct {
@@ -172,6 +175,9 @@ static bool ima_match_rules(struct ima_rule_entry *rule,
 	if ((rule->flags & IMA_FSMAGIC)
 	    && rule->fsmagic != inode->i_sb->s_magic)
 		return false;
+	if ((rule->flags & IMA_FSUUID) &&
+		memcmp(rule->fsuuid, inode->i_sb->s_uuid, sizeof(rule->fsuuid)))
+		return false;
 	if ((rule->flags & IMA_UID) && !uid_eq(rule->uid, cred->uid))
 		return false;
 	if ((rule->flags & IMA_FOWNER) && !uid_eq(rule->fowner, inode->i_uid))
@@ -346,7 +352,7 @@ enum {
 	Opt_obj_user, Opt_obj_role, Opt_obj_type,
 	Opt_subj_user, Opt_subj_role, Opt_subj_type,
 	Opt_func, Opt_mask, Opt_fsmagic, Opt_uid, Opt_fowner,
-	Opt_appraise_type
+	Opt_appraise_type, Opt_fsuuid
 };
 
 static match_table_t policy_tokens = {
@@ -364,6 +370,7 @@ static match_table_t policy_tokens = {
 	{Opt_func, "func=%s"},
 	{Opt_mask, "mask=%s"},
 	{Opt_fsmagic, "fsmagic=%s"},
+	{Opt_fsuuid, "fsuuid=%s"},
 	{Opt_uid, "uid=%s"},
 	{Opt_fowner, "fowner=%s"},
 	{Opt_appraise_type, "appraise_type=%s"},
@@ -519,6 +526,19 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
 			if (!result)
 				entry->flags |= IMA_FSMAGIC;
 			break;
+		case Opt_fsuuid:
+			ima_log_string(ab, "fsuuid", args[0].from);
+
+			if (memchr_inv(entry->fsuuid, 0x00,
+			    sizeof(entry->fsuuid))) {
+				result = -EINVAL;
+				break;
+			}
+
+			part_pack_uuid(args[0].from, entry->fsuuid);
+			entry->flags |= IMA_FSUUID;
+			result = 0;
+			break;
 		case Opt_uid:
 			ima_log_string(ab, "uid", args[0].from);
 
-- 
1.8.1.rc3

Re: [PATCH 2/2] ima: add policy support for file system uuid

[David Rientjes]

On Tue, 5 Feb 2013, Mimi Zohar wrote:

> diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
> index 4adcd0f..23f49e3 100644
> --- a/security/integrity/ima/ima_policy.c
> +++ b/security/integrity/ima/ima_policy.c
> @@ -16,6 +16,7 @@
>  #include <linux/magic.h>
>  #include <linux/parser.h>
>  #include <linux/slab.h>
> +#include <linux/genhd.h>
>  
>  #include "ima.h"
>  
> @@ -25,6 +26,7 @@
>  #define IMA_FSMAGIC	0x0004
>  #define IMA_UID		0x0008
>  #define IMA_FOWNER	0x0010
> +#define IMA_FSUUID	0x0020
>  
>  #define UNKNOWN		0
>  #define MEASURE		0x0001	/* same as IMA_MEASURE */
> @@ -45,6 +47,7 @@ struct ima_rule_entry {
>  	enum ima_hooks func;
>  	int mask;
>  	unsigned long fsmagic;
> +	u8 fsuuid[16];
>  	kuid_t uid;
>  	kuid_t fowner;
>  	struct {
> @@ -172,6 +175,9 @@ static bool ima_match_rules(struct ima_rule_entry *rule,
>  	if ((rule->flags & IMA_FSMAGIC)
>  	    && rule->fsmagic != inode->i_sb->s_magic)
>  		return false;
> +	if ((rule->flags & IMA_FSUUID) &&
> +		memcmp(rule->fsuuid, inode->i_sb->s_uuid, sizeof(rule->fsuuid)))
> +		return false;
>  	if ((rule->flags & IMA_UID) && !uid_eq(rule->uid, cred->uid))
>  		return false;
>  	if ((rule->flags & IMA_FOWNER) && !uid_eq(rule->fowner, inode->i_uid))
> @@ -346,7 +352,7 @@ enum {
>  	Opt_obj_user, Opt_obj_role, Opt_obj_type,
>  	Opt_subj_user, Opt_subj_role, Opt_subj_type,
>  	Opt_func, Opt_mask, Opt_fsmagic, Opt_uid, Opt_fowner,
> -	Opt_appraise_type
> +	Opt_appraise_type, Opt_fsuuid
>  };
>  
>  static match_table_t policy_tokens = {
> @@ -364,6 +370,7 @@ static match_table_t policy_tokens = {
>  	{Opt_func, "func=%s"},
>  	{Opt_mask, "mask=%s"},
>  	{Opt_fsmagic, "fsmagic=%s"},
> +	{Opt_fsuuid, "fsuuid=%s"},
>  	{Opt_uid, "uid=%s"},
>  	{Opt_fowner, "fowner=%s"},
>  	{Opt_appraise_type, "appraise_type=%s"},
> @@ -519,6 +526,19 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
>  			if (!result)
>  				entry->flags |= IMA_FSMAGIC;
>  			break;
> +		case Opt_fsuuid:
> +			ima_log_string(ab, "fsuuid", args[0].from);
> +
> +			if (memchr_inv(entry->fsuuid, 0x00,
> +			    sizeof(entry->fsuuid))) {
> +				result = -EINVAL;
> +				break;
> +			}
> +
> +			part_pack_uuid(args[0].from, entry->fsuuid);
> +			entry->flags |= IMA_FSUUID;
> +			result = 0;
> +			break;
>  		case Opt_uid:
>  			ima_log_string(ab, "uid", args[0].from);
>  

We don't have part_pack_uuid() without CONFIG_BLOCK, so should this return 
-ENOTSUPP if that option is not enabled?

Re: [PATCH 2/2] ima: add policy support for file system uuid

[Mimi Zohar]

On Thu, 2013-02-21 at 13:54 -0800, David Rientjes wrote:
> On Tue, 5 Feb 2013, Mimi Zohar wrote:
> 
> > diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
> > index 4adcd0f..23f49e3 100644
> > --- a/security/integrity/ima/ima_policy.c
> > +++ b/security/integrity/ima/ima_policy.c
> > @@ -16,6 +16,7 @@
> >  #include <linux/magic.h>
> >  #include <linux/parser.h>
> >  #include <linux/slab.h>
> > +#include <linux/genhd.h>
> >  
> >  #include "ima.h"
> >  
> > @@ -25,6 +26,7 @@
> >  #define IMA_FSMAGIC	0x0004
> >  #define IMA_UID		0x0008
> >  #define IMA_FOWNER	0x0010
> > +#define IMA_FSUUID	0x0020
> >  
> >  #define UNKNOWN		0
> >  #define MEASURE		0x0001	/* same as IMA_MEASURE */
> > @@ -45,6 +47,7 @@ struct ima_rule_entry {
> >  	enum ima_hooks func;
> >  	int mask;
> >  	unsigned long fsmagic;
> > +	u8 fsuuid[16];
> >  	kuid_t uid;
> >  	kuid_t fowner;
> >  	struct {
> > @@ -172,6 +175,9 @@ static bool ima_match_rules(struct ima_rule_entry *rule,
> >  	if ((rule->flags & IMA_FSMAGIC)
> >  	    && rule->fsmagic != inode->i_sb->s_magic)
> >  		return false;
> > +	if ((rule->flags & IMA_FSUUID) &&
> > +		memcmp(rule->fsuuid, inode->i_sb->s_uuid, sizeof(rule->fsuuid)))
> > +		return false;
> >  	if ((rule->flags & IMA_UID) && !uid_eq(rule->uid, cred->uid))
> >  		return false;
> >  	if ((rule->flags & IMA_FOWNER) && !uid_eq(rule->fowner, inode->i_uid))
> > @@ -346,7 +352,7 @@ enum {
> >  	Opt_obj_user, Opt_obj_role, Opt_obj_type,
> >  	Opt_subj_user, Opt_subj_role, Opt_subj_type,
> >  	Opt_func, Opt_mask, Opt_fsmagic, Opt_uid, Opt_fowner,
> > -	Opt_appraise_type
> > +	Opt_appraise_type, Opt_fsuuid
> >  };
> >  
> >  static match_table_t policy_tokens = {
> > @@ -364,6 +370,7 @@ static match_table_t policy_tokens = {
> >  	{Opt_func, "func=%s"},
> >  	{Opt_mask, "mask=%s"},
> >  	{Opt_fsmagic, "fsmagic=%s"},
> > +	{Opt_fsuuid, "fsuuid=%s"},
> >  	{Opt_uid, "uid=%s"},
> >  	{Opt_fowner, "fowner=%s"},
> >  	{Opt_appraise_type, "appraise_type=%s"},
> > @@ -519,6 +526,19 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
> >  			if (!result)
> >  				entry->flags |= IMA_FSMAGIC;
> >  			break;
> > +		case Opt_fsuuid:
> > +			ima_log_string(ab, "fsuuid", args[0].from);
> > +
> > +			if (memchr_inv(entry->fsuuid, 0x00,
> > +			    sizeof(entry->fsuuid))) {
> > +				result = -EINVAL;
> > +				break;
> > +			}
> > +
> > +			part_pack_uuid(args[0].from, entry->fsuuid);
> > +			entry->flags |= IMA_FSUUID;
> > +			result = 0;
> > +			break;
> >  		case Opt_uid:
> >  			ima_log_string(ab, "uid", args[0].from);
> >  
> 
> We don't have part_pack_uuid() without CONFIG_BLOCK, so should this return 
> -ENOTSUPP if that option is not enabled?

Yes, this problem showed up in Randy's randconfig.  He suggested moving
part_pack_uuid() outside of the "ifdef CONFIG_BLOCK" to always make it
visible - http://marc.info/?l=linux-next&m=136139276002173&w=2.

thanks,

Mimi

Re: [PATCH 2/2] ima: add policy support for file system uuid

[David Rientjes]

On Thu, 21 Feb 2013, Mimi Zohar wrote:

> > > diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
> > > index 4adcd0f..23f49e3 100644
> > > --- a/security/integrity/ima/ima_policy.c
> > > +++ b/security/integrity/ima/ima_policy.c
> > > @@ -16,6 +16,7 @@
> > >  #include <linux/magic.h>
> > >  #include <linux/parser.h>
> > >  #include <linux/slab.h>
> > > +#include <linux/genhd.h>
> > >  
> > >  #include "ima.h"
> > >  
> > > @@ -25,6 +26,7 @@
> > >  #define IMA_FSMAGIC	0x0004
> > >  #define IMA_UID		0x0008
> > >  #define IMA_FOWNER	0x0010
> > > +#define IMA_FSUUID	0x0020
> > >  
> > >  #define UNKNOWN		0
> > >  #define MEASURE		0x0001	/* same as IMA_MEASURE */
> > > @@ -45,6 +47,7 @@ struct ima_rule_entry {
> > >  	enum ima_hooks func;
> > >  	int mask;
> > >  	unsigned long fsmagic;
> > > +	u8 fsuuid[16];
> > >  	kuid_t uid;
> > >  	kuid_t fowner;
> > >  	struct {
> > > @@ -172,6 +175,9 @@ static bool ima_match_rules(struct ima_rule_entry *rule,
> > >  	if ((rule->flags & IMA_FSMAGIC)
> > >  	    && rule->fsmagic != inode->i_sb->s_magic)
> > >  		return false;
> > > +	if ((rule->flags & IMA_FSUUID) &&
> > > +		memcmp(rule->fsuuid, inode->i_sb->s_uuid, sizeof(rule->fsuuid)))
> > > +		return false;
> > >  	if ((rule->flags & IMA_UID) && !uid_eq(rule->uid, cred->uid))
> > >  		return false;
> > >  	if ((rule->flags & IMA_FOWNER) && !uid_eq(rule->fowner, inode->i_uid))
> > > @@ -346,7 +352,7 @@ enum {
> > >  	Opt_obj_user, Opt_obj_role, Opt_obj_type,
> > >  	Opt_subj_user, Opt_subj_role, Opt_subj_type,
> > >  	Opt_func, Opt_mask, Opt_fsmagic, Opt_uid, Opt_fowner,
> > > -	Opt_appraise_type
> > > +	Opt_appraise_type, Opt_fsuuid
> > >  };
> > >  
> > >  static match_table_t policy_tokens = {
> > > @@ -364,6 +370,7 @@ static match_table_t policy_tokens = {
> > >  	{Opt_func, "func=%s"},
> > >  	{Opt_mask, "mask=%s"},
> > >  	{Opt_fsmagic, "fsmagic=%s"},
> > > +	{Opt_fsuuid, "fsuuid=%s"},
> > >  	{Opt_uid, "uid=%s"},
> > >  	{Opt_fowner, "fowner=%s"},
> > >  	{Opt_appraise_type, "appraise_type=%s"},
> > > @@ -519,6 +526,19 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
> > >  			if (!result)
> > >  				entry->flags |= IMA_FSMAGIC;
> > >  			break;
> > > +		case Opt_fsuuid:
> > > +			ima_log_string(ab, "fsuuid", args[0].from);
> > > +
> > > +			if (memchr_inv(entry->fsuuid, 0x00,
> > > +			    sizeof(entry->fsuuid))) {
> > > +				result = -EINVAL;
> > > +				break;
> > > +			}
> > > +
> > > +			part_pack_uuid(args[0].from, entry->fsuuid);
> > > +			entry->flags |= IMA_FSUUID;
> > > +			result = 0;
> > > +			break;
> > >  		case Opt_uid:
> > >  			ima_log_string(ab, "uid", args[0].from);
> > >  
> > 
> > We don't have part_pack_uuid() without CONFIG_BLOCK, so should this return 
> > -ENOTSUPP if that option is not enabled?
> 
> Yes, this problem showed up in Randy's randconfig.  He suggested moving
> part_pack_uuid() outside of the "ifdef CONFIG_BLOCK" to always make it
> visible - http://marc.info/?l=linux-next&m=136139276002173&w=2.
> 

Who's pushing this to linux-next?