Re: [PATCH v7 1/8] IMA: generalize keyring specific measurement constructs

2020-12-10 Thread Tushar Sugandhi




On 2020-12-10 2:14 p.m., Tyler Hicks wrote:

On 2020-12-09 11:42:05, Tushar Sugandhi wrote:

IMA functions such as ima_match_keyring(), process_buffer_measurement(),
ima_match_policy() etc. handle data specific to keyrings. Currently,
these constructs are not generic to handle any func specific data.
This makes it harder to extend them without code duplication.

Refactor the keyring specific measurement constructs to be generic and
reusable in other measurement scenarios.

Signed-off-by: Tushar Sugandhi 


I've got a few code cleanup suggestions to ima_match_rule_data() below
but the current patch is fine:

Reviewed-by: Tyler Hicks 


---
  security/integrity/ima/ima.h|  6 ++--
  security/integrity/ima/ima_api.c|  6 ++--
  security/integrity/ima/ima_main.c   |  6 ++--
  security/integrity/ima/ima_policy.c | 49 ++---
  4 files changed, 40 insertions(+), 27 deletions(-)

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 8e8b1e3cb847..e5622ce8cbb1 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -256,7 +256,7 @@ static inline void ima_process_queued_keys(void) {}
  int ima_get_action(struct inode *inode, const struct cred *cred, u32 secid,
   int mask, enum ima_hooks func, int *pcr,
   struct ima_template_desc **template_desc,
-  const char *keyring);
+  const char *func_data);
  int ima_must_measure(struct inode *inode, int mask, enum ima_hooks func);
  int ima_collect_measurement(struct integrity_iint_cache *iint,
struct file *file, void *buf, loff_t size,
@@ -268,7 +268,7 @@ void ima_store_measurement(struct integrity_iint_cache 
*iint, struct file *file,
   struct ima_template_desc *template_desc);
  void process_buffer_measurement(struct inode *inode, const void *buf, int 
size,
const char *eventname, enum ima_hooks func,
-   int pcr, const char *keyring);
+   int pcr, const char *func_data);
  void ima_audit_measurement(struct integrity_iint_cache *iint,
   const unsigned char *filename);
  int ima_alloc_init_template(struct ima_event_data *event_data,
@@ -284,7 +284,7 @@ const char *ima_d_path(const struct path *path, char 
**pathbuf, char *filename);
  int ima_match_policy(struct inode *inode, const struct cred *cred, u32 secid,
 enum ima_hooks func, int mask, int flags, int *pcr,
 struct ima_template_desc **template_desc,
-const char *keyring);
+const char *func_data);
  void ima_init_policy(void);
  void ima_update_policy(void);
  void ima_update_policy_flag(void);
diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index 4f39fb93f278..af218babd198 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c
@@ -170,7 +170,7 @@ void ima_add_violation(struct file *file, const unsigned 
char *filename,
   * @func: caller identifier
   * @pcr: pointer filled in if matched measure policy sets pcr=
   * @template_desc: pointer filled in if matched measure policy sets template=
- * @keyring: keyring name used to determine the action
+ * @func_data: private data specific to @func, can be NULL.
   *
   * The policy is defined in terms of keypairs:
   *subj=, obj=, type=, func=, mask=, fsmagic=
@@ -186,14 +186,14 @@ void ima_add_violation(struct file *file, const unsigned 
char *filename,
  int ima_get_action(struct inode *inode, const struct cred *cred, u32 secid,
   int mask, enum ima_hooks func, int *pcr,
   struct ima_template_desc **template_desc,
-  const char *keyring)
+  const char *func_data)
  {
int flags = IMA_MEASURE | IMA_AUDIT | IMA_APPRAISE | IMA_HASH;
  
  	flags &= ima_policy_flag;
  
  	return ima_match_policy(inode, cred, secid, func, mask, flags, pcr,

-   template_desc, keyring);
+   template_desc, func_data);
  }
  
  /*

diff --git a/security/integrity/ima/ima_main.c 
b/security/integrity/ima/ima_main.c
index 68956e884403..e76ef4bfd0f4 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -786,13 +786,13 @@ int ima_post_load_data(char *buf, loff_t size,
   * @eventname: event name to be used for the buffer entry.
   * @func: IMA hook
   * @pcr: pcr to extend the measurement
- * @keyring: keyring name to determine the action to be performed
+ * @func_data: private data specific to @func, can be NULL.
   *
   * Based on policy, the buffer is measured into the ima log.
   */
  void process_buffer_measurement(struct inode *inode, const void *buf, int 
size,
const char *eventname, enum ima_hooks func,
-   

Re: [PATCH v7 1/8] IMA: generalize keyring specific measurement constructs

2020-12-10 Thread Tyler Hicks
On 2020-12-09 11:42:05, Tushar Sugandhi wrote:
> IMA functions such as ima_match_keyring(), process_buffer_measurement(),
> ima_match_policy() etc. handle data specific to keyrings. Currently,
> these constructs are not generic to handle any func specific data.
> This makes it harder to extend them without code duplication.
> 
> Refactor the keyring specific measurement constructs to be generic and
> reusable in other measurement scenarios.
> 
> Signed-off-by: Tushar Sugandhi 

I've got a few code cleanup suggestions to ima_match_rule_data() below
but the current patch is fine:

Reviewed-by: Tyler Hicks 

> ---
>  security/integrity/ima/ima.h|  6 ++--
>  security/integrity/ima/ima_api.c|  6 ++--
>  security/integrity/ima/ima_main.c   |  6 ++--
>  security/integrity/ima/ima_policy.c | 49 ++---
>  4 files changed, 40 insertions(+), 27 deletions(-)
> 
> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index 8e8b1e3cb847..e5622ce8cbb1 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h
> @@ -256,7 +256,7 @@ static inline void ima_process_queued_keys(void) {}
>  int ima_get_action(struct inode *inode, const struct cred *cred, u32 secid,
>  int mask, enum ima_hooks func, int *pcr,
>  struct ima_template_desc **template_desc,
> -const char *keyring);
> +const char *func_data);
>  int ima_must_measure(struct inode *inode, int mask, enum ima_hooks func);
>  int ima_collect_measurement(struct integrity_iint_cache *iint,
>   struct file *file, void *buf, loff_t size,
> @@ -268,7 +268,7 @@ void ima_store_measurement(struct integrity_iint_cache 
> *iint, struct file *file,
>  struct ima_template_desc *template_desc);
>  void process_buffer_measurement(struct inode *inode, const void *buf, int 
> size,
>   const char *eventname, enum ima_hooks func,
> - int pcr, const char *keyring);
> + int pcr, const char *func_data);
>  void ima_audit_measurement(struct integrity_iint_cache *iint,
>  const unsigned char *filename);
>  int ima_alloc_init_template(struct ima_event_data *event_data,
> @@ -284,7 +284,7 @@ const char *ima_d_path(const struct path *path, char 
> **pathbuf, char *filename);
>  int ima_match_policy(struct inode *inode, const struct cred *cred, u32 secid,
>enum ima_hooks func, int mask, int flags, int *pcr,
>struct ima_template_desc **template_desc,
> -  const char *keyring);
> +  const char *func_data);
>  void ima_init_policy(void);
>  void ima_update_policy(void);
>  void ima_update_policy_flag(void);
> diff --git a/security/integrity/ima/ima_api.c 
> b/security/integrity/ima/ima_api.c
> index 4f39fb93f278..af218babd198 100644
> --- a/security/integrity/ima/ima_api.c
> +++ b/security/integrity/ima/ima_api.c
> @@ -170,7 +170,7 @@ void ima_add_violation(struct file *file, const unsigned 
> char *filename,
>   * @func: caller identifier
>   * @pcr: pointer filled in if matched measure policy sets pcr=
>   * @template_desc: pointer filled in if matched measure policy sets template=
> - * @keyring: keyring name used to determine the action
> + * @func_data: private data specific to @func, can be NULL.
>   *
>   * The policy is defined in terms of keypairs:
>   *   subj=, obj=, type=, func=, mask=, fsmagic=
> @@ -186,14 +186,14 @@ void ima_add_violation(struct file *file, const 
> unsigned char *filename,
>  int ima_get_action(struct inode *inode, const struct cred *cred, u32 secid,
>  int mask, enum ima_hooks func, int *pcr,
>  struct ima_template_desc **template_desc,
> -const char *keyring)
> +const char *func_data)
>  {
>   int flags = IMA_MEASURE | IMA_AUDIT | IMA_APPRAISE | IMA_HASH;
>  
>   flags &= ima_policy_flag;
>  
>   return ima_match_policy(inode, cred, secid, func, mask, flags, pcr,
> - template_desc, keyring);
> + template_desc, func_data);
>  }
>  
>  /*
> diff --git a/security/integrity/ima/ima_main.c 
> b/security/integrity/ima/ima_main.c
> index 68956e884403..e76ef4bfd0f4 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -786,13 +786,13 @@ int ima_post_load_data(char *buf, loff_t size,
>   * @eventname: event name to be used for the buffer entry.
>   * @func: IMA hook
>   * @pcr: pcr to extend the measurement
> - * @keyring: keyring name to determine the action to be performed
> + * @func_data: private data specific to @func, can be NULL.
>   *
>   * Based on policy, the buffer is measured into the ima log.
>   */
>  void process_buffer_measurement(struct inode *inode, const void *buf, int 
> size,
>   

[PATCH v7 1/8] IMA: generalize keyring specific measurement constructs

2020-12-09 Thread Tushar Sugandhi
IMA functions such as ima_match_keyring(), process_buffer_measurement(),
ima_match_policy() etc. handle data specific to keyrings. Currently,
these constructs are not generic to handle any func specific data.
This makes it harder to extend them without code duplication.

Refactor the keyring specific measurement constructs to be generic and
reusable in other measurement scenarios.

Signed-off-by: Tushar Sugandhi 
---
 security/integrity/ima/ima.h|  6 ++--
 security/integrity/ima/ima_api.c|  6 ++--
 security/integrity/ima/ima_main.c   |  6 ++--
 security/integrity/ima/ima_policy.c | 49 ++---
 4 files changed, 40 insertions(+), 27 deletions(-)

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 8e8b1e3cb847..e5622ce8cbb1 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -256,7 +256,7 @@ static inline void ima_process_queued_keys(void) {}
 int ima_get_action(struct inode *inode, const struct cred *cred, u32 secid,
   int mask, enum ima_hooks func, int *pcr,
   struct ima_template_desc **template_desc,
-  const char *keyring);
+  const char *func_data);
 int ima_must_measure(struct inode *inode, int mask, enum ima_hooks func);
 int ima_collect_measurement(struct integrity_iint_cache *iint,
struct file *file, void *buf, loff_t size,
@@ -268,7 +268,7 @@ void ima_store_measurement(struct integrity_iint_cache 
*iint, struct file *file,
   struct ima_template_desc *template_desc);
 void process_buffer_measurement(struct inode *inode, const void *buf, int size,
const char *eventname, enum ima_hooks func,
-   int pcr, const char *keyring);
+   int pcr, const char *func_data);
 void ima_audit_measurement(struct integrity_iint_cache *iint,
   const unsigned char *filename);
 int ima_alloc_init_template(struct ima_event_data *event_data,
@@ -284,7 +284,7 @@ const char *ima_d_path(const struct path *path, char 
**pathbuf, char *filename);
 int ima_match_policy(struct inode *inode, const struct cred *cred, u32 secid,
 enum ima_hooks func, int mask, int flags, int *pcr,
 struct ima_template_desc **template_desc,
-const char *keyring);
+const char *func_data);
 void ima_init_policy(void);
 void ima_update_policy(void);
 void ima_update_policy_flag(void);
diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index 4f39fb93f278..af218babd198 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c
@@ -170,7 +170,7 @@ void ima_add_violation(struct file *file, const unsigned 
char *filename,
  * @func: caller identifier
  * @pcr: pointer filled in if matched measure policy sets pcr=
  * @template_desc: pointer filled in if matched measure policy sets template=
- * @keyring: keyring name used to determine the action
+ * @func_data: private data specific to @func, can be NULL.
  *
  * The policy is defined in terms of keypairs:
  * subj=, obj=, type=, func=, mask=, fsmagic=
@@ -186,14 +186,14 @@ void ima_add_violation(struct file *file, const unsigned 
char *filename,
 int ima_get_action(struct inode *inode, const struct cred *cred, u32 secid,
   int mask, enum ima_hooks func, int *pcr,
   struct ima_template_desc **template_desc,
-  const char *keyring)
+  const char *func_data)
 {
int flags = IMA_MEASURE | IMA_AUDIT | IMA_APPRAISE | IMA_HASH;
 
flags &= ima_policy_flag;
 
return ima_match_policy(inode, cred, secid, func, mask, flags, pcr,
-   template_desc, keyring);
+   template_desc, func_data);
 }
 
 /*
diff --git a/security/integrity/ima/ima_main.c 
b/security/integrity/ima/ima_main.c
index 68956e884403..e76ef4bfd0f4 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -786,13 +786,13 @@ int ima_post_load_data(char *buf, loff_t size,
  * @eventname: event name to be used for the buffer entry.
  * @func: IMA hook
  * @pcr: pcr to extend the measurement
- * @keyring: keyring name to determine the action to be performed
+ * @func_data: private data specific to @func, can be NULL.
  *
  * Based on policy, the buffer is measured into the ima log.
  */
 void process_buffer_measurement(struct inode *inode, const void *buf, int size,
const char *eventname, enum ima_hooks func,
-   int pcr, const char *keyring)
+   int pcr, const char *func_data)
 {
int ret = 0;
const char *audit_cause = "ENOMEM";
@@ -831,7 +831,7 @@ void process_buffer_measurement(struct inode *inode, const 
void *buf, int