On Thu, 2019-04-18 at 00:51 -0300, Thiago Jung Bauermann wrote:
> Define new "d-modsig" template field which holds the digest that is
> expected to match the one contained in the modsig, and also new "modsig"
> template field which holds the appended file signature.
>
> Add a new "ima-modsig" defined template descriptor with the new fields as
> well as the ones from the "ima-sig" descriptor.
>
> Change ima_store_measurement() to accept a struct modsig * argument so that
> it can be passed along to the templates via struct ima_event_data.
>
> Suggested-by: Mimi Zohar <zohar@xxxxxxxxxxxxx>
> Signed-off-by: Thiago Jung Bauermann <bauerman@xxxxxxxxxxxxx>
Thanks, Roberto. Just some thoughts inline below.
Reviewed-by: Mimi Zohar <zohar@xxxxxxxxxxxxx>
> ---
<snip>
> +/*
> + * Validating the appended signature included in the measurement list requires
> + * the file hash calculated without the appended signature (i.e., the 'd-modsig'
> + * field). Therefore, notify the user if they have the 'modsig' field but not
> + * the 'd-modsig' field in the template.
> + */
> +static void check_current_template_modsig(void)
> +{
> +#define MSG "template with 'modsig' field also needs 'd-modsig' field\n"
> + struct ima_template_desc *template;
> + bool has_modsig, has_dmodsig;
> + static bool checked;
> + int i;
> +
> + /* We only need to notify the user once. */
> + if (checked)
> + return;
> +
> + has_modsig = has_dmodsig = false;
> + template = ima_template_desc_current();
> + for (i = 0; i < template->num_fields; i++) {
> + if (!strcmp(template->fields[i]->field_id, "modsig"))
> + has_modsig = true;
> + else if (!strcmp(template->fields[i]->field_id, "d-modsig"))
> + has_dmodsig = true;
> + }
> +
> + if (has_modsig && !has_dmodsig)
> + pr_notice(MSG);
> +
> + checked = true;
> +#undef MSG
> +}
> +
There was some recent discussion about supporting per IMA policy rule
template formats. This feature will allow just the kexec kernel image
to require ima-modsig. When per policy rule template formats support
is upstreamed, this function will need to be updated.
<snip>
>
> @@ -389,3 +425,25 @@ int ima_eventsig_init(struct ima_event_data *event_data,
> return ima_write_template_field_data(xattr_value, event_data->xattr_len,
> DATA_FMT_HEX, field_data);
> }
> +
> +int ima_eventmodsig_init(struct ima_event_data *event_data,
> + struct ima_field_data *field_data)
> +{
> + const void *data;
> + u32 data_len;
> + int rc;
> +
> + if (!event_data->modsig)
> + return 0;
> +
> + /*
> + * The xattr_value for IMA_MODSIG is a runtime structure containing
> + * pointers. Get its raw data instead.
> + */
"xattr_value"? The comment needs some clarification.
Mimi
> + rc = ima_modsig_serialize(event_data->modsig, &data, &data_len);
> + if (rc)
> + return rc;
> +
> + return ima_write_template_field_data(data, data_len,
> + DATA_FMT_HEX, field_data);
> +}
