On Fri, 2019-10-25 at 10:32 -0700, Lakshmi Ramasubramanian wrote:

On 10/25/2019 10:24 AM, Nayna Jain wrote:

quoted

On 10/24/19 10:20 AM, Lakshmi Ramasubramanian wrote:

quoted

On 10/23/19 8:47 PM, Nayna Jain wrote:

Hi Nayna,

quoted

+void process_buffer_measurement(const void *buf, int size,
+                const char *eventname, enum ima_hooks func,
+                int pcr)
  {
      int ret = 0;
      struct ima_template_entry *entry = NULL;

quoted

+    if (func) {

Let's comment this line.  Perhaps something like /*Unnecessary for
auxiliary buffer measurements */

quoted

quoted

quoted

+        security_task_getsecid(current, &secid);
+        action = ima_get_action(NULL, current_cred(), secid, 0, func,
+                    &pcr, &template);
+        if (!(action & IMA_MEASURE))
+            return;
+    }

In your change set process_buffer_measurement is called with NONE for 
the parameter func. So ima_get_action (the above if block) will not be 
executed.

Wouldn't it better to update ima_get_action (and related functions) to 
handle the ima policy (func param)?

The idea is to use ima-buf template for the auxiliary measurement 
record. The auxiliary measurement record is an additional record to the 
one already created based on the existing policy. When func is passed as 
NONE, it represents it is an additional record. I am not sure what you 
mean by updating ima_get_action, it is already handling the ima policy.

I was referring to using "func" in process_buffer_measurement to 
determine ima action. In my opinion, process_buffer_measurement should 
be generic.

ima_get_action() should instead determine the required ima action, 
template, pcr, etc. based on "func" passed to it.

Nayna's original patch moved ima_get_action() into the caller, but
that resulted in code duplication in each of the callers.  This
solution differentiates between the initial, which requires calling
ima_get_action(), and auxiliary buffer measurement records.

Mimi