On 2020-12-09 11:42:08, Tushar Sugandhi wrote:
> A new IMA policy rule is needed for the IMA hook
> ima_measure_critical_data() and the corresponding func CRITICAL_DATA for
> measuring the input buffer. The policy rule should ensure the buffer
> would get measured only when the policy rule allows the action. The
> policy rule should also support the necessary constraints (flags etc.)
> for integrity critical buffer data measurements.
>
> Add a policy rule to define the constraints for restricting integrity
> critical data measurements.
>
> Signed-off-by: Tushar Sugandhi <tusha...@linux.microsoft.com>
> ---
> security/integrity/ima/ima_policy.c | 35 +++++++++++++++++++++++++----
> 1 file changed, 31 insertions(+), 4 deletions(-)
>
> diff --git a/security/integrity/ima/ima_policy.c
> b/security/integrity/ima/ima_policy.c
> index 2a0c0603626e..9a8ee80a3128 100644
> --- a/security/integrity/ima/ima_policy.c
> +++ b/security/integrity/ima/ima_policy.c
> @@ -34,6 +34,7 @@
> #define IMA_PCR 0x0100
> #define IMA_FSNAME 0x0200
> #define IMA_KEYRINGS 0x0400
> +#define IMA_DATA_SOURCE 0x0800
You introduce data_source= in the next patch. This macro shouldn't be
added until the next patch.
>
> #define UNKNOWN 0
> #define MEASURE 0x0001 /* same as IMA_MEASURE */
> @@ -85,6 +86,7 @@ struct ima_rule_entry {
> } lsm[MAX_LSM_RULES];
> char *fsname;
> struct ima_rule_opt_list *keyrings; /* Measure keys added to these
> keyrings */
> + struct ima_rule_opt_list *data_source; /* Measure data from this source
> */
> struct ima_template_desc *template;
> };
>
> @@ -479,6 +481,12 @@ static bool ima_match_rule_data(struct ima_rule_entry
> *rule,
> else
> opt_list = rule->keyrings;
> break;
> + case CRITICAL_DATA:
> + if (!rule->data_source)
> + return true;
> + else
> + opt_list = rule->data_source;
If you take my suggestions on patch #1, remove the else and simply
assign opt_list here, too.
> + break;
> default:
> break;
> }
> @@ -518,13 +526,19 @@ static bool ima_match_rules(struct ima_rule_entry
> *rule, struct inode *inode,
> {
> int i;
>
> - if (func == KEY_CHECK) {
> - return (rule->flags & IMA_FUNC) && (rule->func == func) &&
> - ima_match_rule_data(rule, func_data, cred);
> - }
> if ((rule->flags & IMA_FUNC) &&
> (rule->func != func && func != POST_SETATTR))
> return false;
> +
> + switch (func) {
> + case KEY_CHECK:
> + case CRITICAL_DATA:
> + return ((rule->func == func) &&
> + ima_match_rule_data(rule, func_data, cred));
> + default:
> + break;
> + }
> +
> if ((rule->flags & IMA_MASK) &&
> (rule->mask != mask && func != POST_SETATTR))
> return false;
> @@ -1119,6 +1133,19 @@ static bool ima_validate_rule(struct ima_rule_entry
> *entry)
> if (ima_rule_contains_lsm_cond(entry))
> return false;
>
> + break;
> + case CRITICAL_DATA:
> + if (entry->action & ~(MEASURE | DONT_MEASURE))
> + return false;
> +
> + if (!(entry->flags & IMA_DATA_SOURCE) ||
> + (entry->flags & ~(IMA_FUNC | IMA_UID | IMA_PCR |
> + IMA_DATA_SOURCE)))
IMA_DATA_SOURCE shouldn't exist in this patch. This isn't the right
indentation, either. See how IMA_KEYRINGS is indented in the KEY_CHECK
case above.
Tyler
> + return false;
> +
> + if (ima_rule_contains_lsm_cond(entry))
> + return false;
> +
> break;
> default:
> return false;
> --
> 2.17.1
>
