[v3,2/8] ima: add policy support for extending different pcrs
diff mbox

Message ID 201606011814.u51IELEH006945@mx0a-001b2d01.pphosted.com
State New
Headers show

Commit Message

Eric Richter June 1, 2016, 6:14 p.m. UTC
This patch defines a new IMA measurement policy rule option
"pcr=", which allows extending different PCRs on a per rule
basis. For example, the system independent files could
extend the default IMA Kconfig specified PCR, while the
system dependent files could extend a different PCR.

The following is an example of this usage with an SELinux
policy; the rule would extend PCR 11 with system
configuration files:

  measure func=FILE_CHECK mask=MAY_READ obj_type=system_conf_t pcr=11

Signed-off-by: Eric Richter <erichte@linux.vnet.ibm.com>
---
 security/integrity/ima/ima_policy.c | 29 ++++++++++++++++++++++++++++-
 1 file changed, 28 insertions(+), 1 deletion(-)

Comments

Mimi Zohar June 8, 2016, 5:08 p.m. UTC | #1
On Wed, 2016-06-01 at 13:14 -0500, Eric Richter wrote:

> diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
> index be09e2c..c20c869 100644
> --- a/security/integrity/ima/ima_policy.c
> +++ b/security/integrity/ima/ima_policy.c
> @@ -32,6 +32,7 @@
>  #define IMA_FSUUID	0x0020
>  #define IMA_INMASK	0x0040
>  #define IMA_EUID	0x0080
> +#define IMA_PCR		0x0100
> 
>  #define UNKNOWN		0
>  #define MEASURE		0x0001	/* same as IMA_MEASURE */
> @@ -40,6 +41,9 @@
>  #define DONT_APPRAISE	0x0008
>  #define AUDIT		0x0040
> 
> +#define INVALID_PCR(a) (((a) < 0) || \
> +	(a) >= (FIELD_SIZEOF(struct integrity_iint_cache,measured_pcrs)))

FIELD_SIZEOF returns the number of bytes, not bits.  I've fixed this and
a couple of checkpatch warnings.  Please take a look at the changes in
the next-4.8-pcrs branch on
git://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity.git

Other than this, the patches look good.

Thanks!

Mimi

> +
>  int ima_policy_flag;
>  static int temp_ima_appraise;


--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Eric Richter June 8, 2016, 8:19 p.m. UTC | #2
On 06/08/2016 12:08 PM, Mimi Zohar wrote:
> On Wed, 2016-06-01 at 13:14 -0500, Eric Richter wrote:
>
>> diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
>> index be09e2c..c20c869 100644
>> --- a/security/integrity/ima/ima_policy.c
>> +++ b/security/integrity/ima/ima_policy.c
>> @@ -32,6 +32,7 @@
>>   #define IMA_FSUUID	0x0020
>>   #define IMA_INMASK	0x0040
>>   #define IMA_EUID	0x0080
>> +#define IMA_PCR		0x0100
>>
>>   #define UNKNOWN		0
>>   #define MEASURE		0x0001	/* same as IMA_MEASURE */
>> @@ -40,6 +41,9 @@
>>   #define DONT_APPRAISE	0x0008
>>   #define AUDIT		0x0040
>>
>> +#define INVALID_PCR(a) (((a) < 0) || \
>> +	(a) >= (FIELD_SIZEOF(struct integrity_iint_cache,measured_pcrs)))
>
> FIELD_SIZEOF returns the number of bytes, not bits.

Good catch, thanks!

> I've fixed this and a couple of checkpatch warnings.  Please
 > take a look at the changes in the next-4.8-pcrs branch on
> git://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity.git

Everything looks good, thanks for fixing the warnings.

> Other than this, the patches look good.
>
> Thanks!
>
> Mimi
>
>> +
>>   int ima_policy_flag;
>>   static int temp_ima_appraise;
>
>

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch
diff mbox

diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index be09e2c..c20c869 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -32,6 +32,7 @@ 
 #define IMA_FSUUID	0x0020
 #define IMA_INMASK	0x0040
 #define IMA_EUID	0x0080
+#define IMA_PCR		0x0100
 
 #define UNKNOWN		0
 #define MEASURE		0x0001	/* same as IMA_MEASURE */
@@ -40,6 +41,9 @@ 
 #define DONT_APPRAISE	0x0008
 #define AUDIT		0x0040
 
+#define INVALID_PCR(a) (((a) < 0) || \
+	(a) >= (FIELD_SIZEOF(struct integrity_iint_cache,measured_pcrs)))
+
 int ima_policy_flag;
 static int temp_ima_appraise;
 
@@ -60,6 +64,7 @@  struct ima_rule_entry {
 	u8 fsuuid[16];
 	kuid_t uid;
 	kuid_t fowner;
+	int pcr;
 	struct {
 		void *rule;	/* LSM file metadata specific */
 		void *args_p;	/* audit value */
@@ -478,7 +483,8 @@  enum {
 	Opt_subj_user, Opt_subj_role, Opt_subj_type,
 	Opt_func, Opt_mask, Opt_fsmagic,
 	Opt_fsuuid, Opt_uid, Opt_euid, Opt_fowner,
-	Opt_appraise_type, Opt_permit_directio
+	Opt_appraise_type, Opt_permit_directio,
+	Opt_pcr
 };
 
 static match_table_t policy_tokens = {
@@ -502,6 +508,7 @@  static match_table_t policy_tokens = {
 	{Opt_fowner, "fowner=%s"},
 	{Opt_appraise_type, "appraise_type=%s"},
 	{Opt_permit_directio, "permit_directio"},
+	{Opt_pcr, "pcr=%s"},
 	{Opt_err, NULL}
 };
 
@@ -774,6 +781,20 @@  static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
 		case Opt_permit_directio:
 			entry->flags |= IMA_PERMIT_DIRECTIO;
 			break;
+		case Opt_pcr:
+			if (entry->action != MEASURE) {
+				result = -EINVAL;
+				break;
+			}
+			ima_log_string(ab, "pcr", args[0].from);
+
+			result = kstrtoint(args[0].from, 10, &entry->pcr);
+			if (result || INVALID_PCR(entry->pcr))
+				result = -EINVAL;
+			else
+				entry->flags |= IMA_PCR;
+
+			break;
 		case Opt_err:
 			ima_log_string(ab, "UNKNOWN", p);
 			result = -EINVAL;
@@ -1011,6 +1032,12 @@  int ima_policy_show(struct seq_file *m, void *v)
 		seq_puts(m, " ");
 	}
 
+	if (entry->flags & IMA_PCR) {
+		snprintf(tbuf, sizeof(tbuf), "%d", entry->pcr);
+		seq_printf(m, pt(Opt_pcr), tbuf);
+		seq_puts(m, " ");
+	}
+
 	if (entry->flags & IMA_FSUUID) {
 		seq_puts(m, "fsuuid=");
 		for (i = 0; i < ARRAY_SIZE(entry->fsuuid); ++i) {