Hi,
On 23-04-18 23:11, Luis R. Rodriguez wrote:
Hans, please see use of READING_FIRMWARE_PREALLOC_BUFFER, we'll need a new ID
and security for this type of request so IMA can reject it if the policy is
configured for it.
Hmm, interesting, actually it seems like the whole existence
of READING_FIRMWARE_PREALLOC_BUFFER is a mistake, the IMA
framework really does not care if we are loading the firmware
into memory allocated by the firmware-loader code, or into
memory allocated by the device-driver requesting the firmware.
As such the current IMA code (from v4.17-rc2) actually does
not handle READING_FIRMWARE_PREALLOC_BUFFER at all, here
are bits of code from: security/integrity/ima/ima_main.c:
static int read_idmap[READING_MAX_ID] = {
[READING_FIRMWARE] = FIRMWARE_CHECK,
[READING_MODULE] = MODULE_CHECK,
[READING_KEXEC_IMAGE] = KEXEC_KERNEL_CHECK,
[READING_KEXEC_INITRAMFS] = KEXEC_INITRAMFS_CHECK,
[READING_POLICY] = POLICY_CHECK
};
int ima_post_read_file(struct file *file, void *buf, loff_t size,
...
if (!file && read_id == READING_FIRMWARE) {
if ((ima_appraise & IMA_APPRAISE_FIRMWARE) &&
(ima_appraise & IMA_APPRAISE_ENFORCE))
return -EACCES; /* INTEGRITY_UNKNOWN */
return 0;
}
Which show that the IMA code is not handling
READING_FIRMWARE_PREALLOC_BUFFER as it should (I believe it
should handle it the same as READING_FIRMWARE).
Now we could fix that, but the only user of
READING_FIRMWARE_PREALLOC_BUFFER is the code which originally
introduced it:
https://patchwork.kernel.org/patch/9162011/
So I believe it might be better to instead replace it
with just READING_FIRMWARE and find another way to tell
kernel_read_file() that there is a pre-allocated buffer,
perhaps the easiest way there is that *buf must be
NULL when the caller wants kernel_read_file() to
vmalloc the mem. This would of course require auditing
all callers that the buf which the pass in is initialized
to NULL.
Either way adding a third READING_FIRMWARE_FOO to the
kernel_read_file_id enum seems like a bad idea, from
the IMA pov firmware is firmware.
What this whole exercise has shown me though is that
I need to call security_kernel_post_read_file() when
loading EFI embedded firmware. I will add a call to
security_kernel_post_read_file() for v4 of the patch-set.
Please Cc Kees in future patches.
Will do.
Regards,
Hans
