Re: [PATCH 1/3] IMA: add policy condition to measure duplicate critical data

2021-02-09 Thread Tushar Sugandhi




On 2021-02-08 12:45 p.m., Mimi Zohar wrote:

Hi Tushar,

On Fri, 2021-01-29 at 16:45 -0800, Tushar Sugandhi wrote:

IMA needs to support duplicate measurements of integrity
critical data to accurately determine the current state of that data
on the system.  Further, since measurement of duplicate data is not
required for all the use cases, it needs to be policy driven.

Define "allow_dup", a new IMA policy condition, for the IMA func
CRITICAL_DATA to allow duplicate buffer measurement of integrity
critical data.

Limit the ability to measure duplicate buffer data when action is
"measure" and func is CRITICAL_DATA.


Why?!

I wasn't sure if it would break any use-case by supporting this for all 
the files / buffers.  That's why I only wanted to address the scenario 
that we discussed in the last series (critical data measurement).
But as you suggested in this series' cover letter response, I am happy 
to extend it to other scenarios (by disabling "htable" using new Kconfig 
(e.g. CONFIG_IMA_DISABLE_HTABLE)


Signed-off-by: Tushar Sugandhi 
---

diff --git a/security/integrity/ima/ima_policy.c 
b/security/integrity/ima/ima_policy.c
index 9b45d064a87d..b89eb768dd05 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -35,6 +35,7 @@
  #define IMA_FSNAME0x0200
  #define IMA_KEYRINGS  0x0400
  #define IMA_LABEL 0x0800
+#define IMA_ALLOW_DUP  0x1000
  
  #define UNKNOWN		0

  #define MEASURE   0x0001  /* same as IMA_MEASURE */
@@ -87,6 +88,7 @@ struct ima_rule_entry {
char *fsname;
struct ima_rule_opt_list *keyrings; /* Measure keys added to these 
keyrings */
struct ima_rule_opt_list *label; /* Measure data grouped under this 
label */


Defining a new boolean entry shouldn't be necessary.The other
boolean values are just stored in "flags".


Thanks.  Will do the same here.
Thanks,
Tushar

struct ima_template_desc *template;
  };


thanks,

Mimi



Re: [PATCH 1/3] IMA: add policy condition to measure duplicate critical data

2021-02-08 Thread Mimi Zohar
Hi Tushar,

On Fri, 2021-01-29 at 16:45 -0800, Tushar Sugandhi wrote:
> IMA needs to support duplicate measurements of integrity
> critical data to accurately determine the current state of that data
> on the system.  Further, since measurement of duplicate data is not
> required for all the use cases, it needs to be policy driven.
> 
> Define "allow_dup", a new IMA policy condition, for the IMA func
> CRITICAL_DATA to allow duplicate buffer measurement of integrity
> critical data.
> 
> Limit the ability to measure duplicate buffer data when action is
> "measure" and func is CRITICAL_DATA.

Why?!

> 
> Signed-off-by: Tushar Sugandhi 
> ---
> 
> diff --git a/security/integrity/ima/ima_policy.c 
> b/security/integrity/ima/ima_policy.c
> index 9b45d064a87d..b89eb768dd05 100644
> --- a/security/integrity/ima/ima_policy.c
> +++ b/security/integrity/ima/ima_policy.c
> @@ -35,6 +35,7 @@
>  #define IMA_FSNAME   0x0200
>  #define IMA_KEYRINGS 0x0400
>  #define IMA_LABEL0x0800
> +#define IMA_ALLOW_DUP0x1000
>  
>  #define UNKNOWN  0
>  #define MEASURE  0x0001  /* same as IMA_MEASURE */
> @@ -87,6 +88,7 @@ struct ima_rule_entry {
>   char *fsname;
>   struct ima_rule_opt_list *keyrings; /* Measure keys added to these 
> keyrings */
>   struct ima_rule_opt_list *label; /* Measure data grouped under this 
> label */

Defining a new boolean entry shouldn't be necessary.The other
boolean values are just stored in "flags".

>   struct ima_template_desc *template;
>  };

thanks,

Mimi



[PATCH 1/3] IMA: add policy condition to measure duplicate critical data

2021-01-30 Thread Tushar Sugandhi
IMA needs to support duplicate measurements of integrity
critical data to accurately determine the current state of that data
on the system.  Further, since measurement of duplicate data is not
required for all the use cases, it needs to be policy driven.

Define "allow_dup", a new IMA policy condition, for the IMA func
CRITICAL_DATA to allow duplicate buffer measurement of integrity
critical data.

Limit the ability to measure duplicate buffer data when action is
"measure" and func is CRITICAL_DATA.

Signed-off-by: Tushar Sugandhi 
---
 Documentation/ABI/testing/ima_policy |  4 +++-
 security/integrity/ima/ima_policy.c  | 24 ++--
 2 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/Documentation/ABI/testing/ima_policy 
b/Documentation/ABI/testing/ima_policy
index bc8e1cbe5e61..9598287e3bbf 100644
--- a/Documentation/ABI/testing/ima_policy
+++ b/Documentation/ABI/testing/ima_policy
@@ -27,7 +27,7 @@ Description:
lsm:[[subj_user=] [subj_role=] [subj_type=]
 [obj_user=] [obj_role=] [obj_type=]]
option: [[appraise_type=]] [template=] [permit_directio]
-   [appraise_flag=] [keyrings=]
+   [appraise_flag=] [keyrings=] [allow_dup]
  base:
func:= 
[BPRM_CHECK][MMAP_CHECK][CREDS_CHECK][FILE_CHECK]MODULE_CHECK]
[FIRMWARE_CHECK]
@@ -55,6 +55,8 @@ Description:
label:= [selinux]|[kernel_info]|[data_label]
data_label:= a unique string used for grouping and 
limiting critical data.
For example, "selinux" to measure critical data for 
SELinux.
+   allow_dup allows measurement of duplicate data.  Only 
valid
+   when action is "measure" and func is CRITICAL_DATA.
 
  default policy:
# PROC_SUPER_MAGIC
diff --git a/security/integrity/ima/ima_policy.c 
b/security/integrity/ima/ima_policy.c
index 9b45d064a87d..b89eb768dd05 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -35,6 +35,7 @@
 #define IMA_FSNAME 0x0200
 #define IMA_KEYRINGS   0x0400
 #define IMA_LABEL  0x0800
+#define IMA_ALLOW_DUP  0x1000
 
 #define UNKNOWN0
 #define MEASURE0x0001  /* same as IMA_MEASURE */
@@ -87,6 +88,7 @@ struct ima_rule_entry {
char *fsname;
struct ima_rule_opt_list *keyrings; /* Measure keys added to these 
keyrings */
struct ima_rule_opt_list *label; /* Measure data grouped under this 
label */
+   bool allow_dup; /* Allow duplicate buffer entry measurement */
struct ima_template_desc *template;
 };
 
@@ -942,7 +944,7 @@ enum {
Opt_uid_lt, Opt_euid_lt, Opt_fowner_lt,
Opt_appraise_type, Opt_appraise_flag,
Opt_permit_directio, Opt_pcr, Opt_template, Opt_keyrings,
-   Opt_label, Opt_err
+   Opt_label, Opt_allow_dup, Opt_err
 };
 
 static const match_table_t policy_tokens = {
@@ -980,6 +982,7 @@ static const match_table_t policy_tokens = {
{Opt_template, "template=%s"},
{Opt_keyrings, "keyrings=%s"},
{Opt_label, "label=%s"},
+   {Opt_allow_dup, "allow_dup"},
{Opt_err, NULL}
 };
 
@@ -1148,7 +1151,7 @@ static bool ima_validate_rule(struct ima_rule_entry 
*entry)
return false;
 
if (entry->flags & ~(IMA_FUNC | IMA_UID | IMA_PCR |
-IMA_LABEL))
+IMA_LABEL | IMA_ALLOW_DUP))
return false;
 
if (ima_rule_contains_lsm_cond(entry))
@@ -1184,6 +1187,7 @@ static int ima_parse_rule(char *rule, struct 
ima_rule_entry *entry)
entry->uid_op = _eq;
entry->fowner_op = _eq;
entry->action = UNKNOWN;
+   entry->allow_dup = false;
while ((p = strsep(, " \t")) != NULL) {
substring_t args[MAX_OPT_ARGS];
int token;
@@ -1375,6 +1379,19 @@ static int ima_parse_rule(char *rule, struct 
ima_rule_entry *entry)
 
entry->flags |= IMA_LABEL;
break;
+   case Opt_allow_dup:
+   ima_log_string(ab, "allow_dup", NULL);
+
+   if ((entry->func != CRITICAL_DATA) ||
+   (entry->action != MEASURE)) {
+   result = -EINVAL;
+   break;
+   }
+
+   entry->allow_dup = true;
+
+   entry->flags |= IMA_ALLOW_DUP;
+   break;
case Opt_fsuuid:
ima_log_string(ab, "fsuuid", args[0].from);
 
@@ -1761,6 +1778,9 @@ int ima_policy_show(struct seq_file *m, void *v)
seq_puts(m, " ");
}
 
+