The kexec boot command line arguments are not currently being
measured.
Currently during soft reboot(kexec)
- the PCRS are not reset
- the command line arguments used for the next kernel are not measured.
This gives the impression to the secure boot attestation that a cold boot took
place.
For secure boot attestation, it is necessary to measure the kernel
command line. For cold boot, the boot loader can be enhanced to measure
these parameters.
(https://mjg59.dreamwidth.org/48897.html)
This patch set aims to address measuring the boot command line during
soft reboot(kexec_file_load).
To achive the above the patch series does the following
-Add a new ima hook: ima_kexec_cmdline which measures the cmdline args
into the ima log, behind a new ima policy entry KEXEC_CMDLINE.
The kexec cmdline hash is stored in the "d-ng" field of the template data.
-Since the cmldine args cannot be appraised, a new template field(buf) is
added. The template field contains the buffer passed(cmldine args), which
can be used to appraise/attest at a later stage.
The kexec cmdline buffer is stored as HEX in the buf field of the event_data.
-Call the ima_kexec_cmdline(...) hook from kexec_file_load call.
The ima logs need to be carried over to the next kernel, which will be followed
up by other patchsets for x86_64 and arm64.
The kexec cmdline hash is stored in the "d-ng" field of the template data.
and can be verified using
sudo cat /sys/kernel/security/integrity/ima/ascii_runtime_measurements |
grep kexec-cmdline | cut -d' ' -f 6 | xxd -r -p | sha256sum
Changelog:
V10(since V9):
-rebased over next-queued-integrity
-code cleanup
V9(since V8):
- code cleanup
V8(since V7):
- added a new ima template name "ima-buf"
- code cleanup
V7:
- rebased to next-queued-testing
https://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity.git/log/?h=next-queued-testing
V6:
-add a new ima hook and policy to measure the cmdline
args(ima_kexec_cmdline)
-add a new template field buf to contain the buffer measured.
[suggested by Mimi Zohar]
add new fields to ima_event_data to store/read buffer data.
[suggested by Roberto]
-call ima_kexec_cmdline from kexec_file_load path
v5:
-add a new ima hook and policy to measure the cmdline
args(ima_kexec_cmdline)
-add a new template field buf to contain the buffer measured.
[suggested by Mimi Zohar]
-call ima_kexec_cmdline from kexec_file_load path
v4:
- per feedback from LSM community, removed the LSM hook and renamed the
IMA policy to KEXEC_CMDLINE
v3: (rebase changes to next-general)
- Add policy checks for buffer[suggested by Mimi Zohar]
- use the IMA_XATTR to add buffer
- Add kexec_cmdline used for kexec file load
- Add an LSM hook to allow usage by other LSM.[suggestd by Mimi Zohar]
v2:
- Add policy checks for buffer[suggested by Mimi Zohar]
- Add an LSM hook to allow usage by other LSM.[suggestd by Mimi Zohar]
- use the IMA_XATTR to add buffer instead of sig template
v1:
-Add kconfigs to control the ima_buffer_check
-measure the cmdline args suffixed with the kernel file name
-add the buffer to the template sig field.
Prakhar Srivastava (3):
Add a new ima hook ima_kexec_cmdline to measure cmdline args
add a new ima template field buf
call ima_kexec_cmdline to measure the cmdline args
Documentation/ABI/testing/ima_policy | 1 +
Documentation/security/IMA-templates.rst | 2 +-
include/linux/ima.h | 2 +
kernel/kexec_file.c | 8 ++-
security/integrity/ima/ima.h | 3 +
security/integrity/ima/ima_api.c | 5 +-
security/integrity/ima/ima_init.c | 2 +-
security/integrity/ima/ima_main.c | 80 +++++++++++++++++++++++
security/integrity/ima/ima_policy.c | 9 +++
security/integrity/ima/ima_template.c | 2 +
security/integrity/ima/ima_template_lib.c | 20 ++++++
security/integrity/ima/ima_template_lib.h | 4 ++
12 files changed, 131 insertions(+), 7 deletions(-)
--
2.17.1
A buffer(kexec boot command line arguments) measured into IMA
measuremnt list cannot be appraised, without already being
aware of the buffer contents. Since hashes are non-reversible,
raw buffer is needed for validation or regenerating hash for
appraisal/attestation.
Add support to store/read the buffer contents in HEX.
The kexec cmdline hash is stored in the "d-ng" field of the
template data,it can be verified using
sudo cat /sys/kernel/security/integrity/ima/ascii_runtime_measurements |
grep kexec-cmdline | cut -d' ' -f 6 | xxd -r -p | sha256sum
- Add two new fields to ima_event_data to hold the buf and
buf_len [Suggested by Roberto]
- Add a new temaplte field 'buf' to be used to store/read
the buffer data.[Suggested by Mimi]
- Updated process_buffer_meaurement to add the buffer to
ima_event_data. process_buffer_measurement added in
"Define a new IMA hook to measure the boot command line
arguments"
- Add a new template policy name ima-buf to represent
'd-ng|n-ng|buf'
Signed-off-by: Prakhar Srivastava <[email protected]>
Reviewed-by: Roberto Sassu <[email protected]>
Reviewed-by: James Morris <[email protected]>
---
Documentation/security/IMA-templates.rst | 7 ++++---
security/integrity/ima/ima.h | 2 ++
security/integrity/ima/ima_main.c | 4 +++-
security/integrity/ima/ima_template.c | 3 +++
security/integrity/ima/ima_template_lib.c | 21 +++++++++++++++++++++
security/integrity/ima/ima_template_lib.h | 4 ++++
6 files changed, 37 insertions(+), 4 deletions(-)
diff --git a/Documentation/security/IMA-templates.rst b/Documentation/security/IMA-templates.rst
index 2cd0e273cc9a..3d1cca287aa4 100644
--- a/Documentation/security/IMA-templates.rst
+++ b/Documentation/security/IMA-templates.rst
@@ -69,15 +69,16 @@ descriptors by adding their identifier to the format string
algorithm (field format: [<hash algo>:]digest, where the digest
prefix is shown only if the hash algorithm is not SHA1 or MD5);
- 'n-ng': the name of the event, without size limitations;
- - 'sig': the file signature.
+ - 'sig': the file signature;
+ - 'buf': the buffer data that was used to generate the hash without size limitations;
Below, there is the list of defined template descriptors:
- "ima": its format is ``d|n``;
- "ima-ng" (default): its format is ``d-ng|n-ng``;
- - "ima-sig": its format is ``d-ng|n-ng|sig``.
-
+ - "ima-sig": its format is ``d-ng|n-ng|sig``;
+ - "ima-buf": its format is ``d-ng|n-ng|buf``;
Use
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index bdca641f9e51..6aa28ab53d27 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -65,6 +65,8 @@ struct ima_event_data {
struct evm_ima_xattr_data *xattr_value;
int xattr_len;
const char *violation;
+ const void *buf;
+ int buf_len;
};
/* IMA template field data definition */
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 2507bee1b762..317c4b6f2c18 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -627,7 +627,9 @@ static void process_buffer_measurement(const void *buf, int size,
struct ima_template_entry *entry = NULL;
struct integrity_iint_cache iint = {};
struct ima_event_data event_data = {.iint = &iint,
- .filename = eventname};
+ .filename = eventname,
+ .buf = buf,
+ .buf_len = size};
struct ima_template_desc *template_desc = NULL;
struct {
struct ima_digest_data hdr;
diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c
index 00dd5a434689..a01a17e5c581 100644
--- a/security/integrity/ima/ima_template.c
+++ b/security/integrity/ima/ima_template.c
@@ -26,6 +26,7 @@ static struct ima_template_desc builtin_templates[] = {
{.name = IMA_TEMPLATE_IMA_NAME, .fmt = IMA_TEMPLATE_IMA_FMT},
{.name = "ima-ng", .fmt = "d-ng|n-ng"},
{.name = "ima-sig", .fmt = "d-ng|n-ng|sig"},
+ {.name = "ima-buf", .fmt = "d-ng|n-ng|buf"},
{.name = "", .fmt = ""}, /* placeholder for a custom format */
};
@@ -43,6 +44,8 @@ static const struct ima_template_field supported_fields[] = {
.field_show = ima_show_template_string},
{.field_id = "sig", .field_init = ima_eventsig_init,
.field_show = ima_show_template_sig},
+ {.field_id = "buf", .field_init = ima_eventbuf_init,
+ .field_show = ima_show_template_buf},
};
#define MAX_TEMPLATE_NAME_LEN 15
diff --git a/security/integrity/ima/ima_template_lib.c b/security/integrity/ima/ima_template_lib.c
index 513b457ae900..baf4de45c5aa 100644
--- a/security/integrity/ima/ima_template_lib.c
+++ b/security/integrity/ima/ima_template_lib.c
@@ -162,6 +162,12 @@ void ima_show_template_sig(struct seq_file *m, enum ima_show_type show,
ima_show_template_field_data(m, show, DATA_FMT_HEX, field_data);
}
+void ima_show_template_buf(struct seq_file *m, enum ima_show_type show,
+ struct ima_field_data *field_data)
+{
+ ima_show_template_field_data(m, show, DATA_FMT_HEX, field_data);
+}
+
/**
* ima_parse_buf() - Parses lengths and data from an input buffer
* @bufstartp: Buffer start address.
@@ -389,3 +395,18 @@ 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);
}
+
+/*
+ * ima_eventbuf_init - include the buffer(kexec-cmldine) as part of the
+ * template data.
+ */
+int ima_eventbuf_init(struct ima_event_data *event_data,
+ struct ima_field_data *field_data)
+{
+ if ((!event_data->buf) || (event_data->buf_len == 0))
+ return 0;
+
+ return ima_write_template_field_data(event_data->buf,
+ event_data->buf_len, DATA_FMT_HEX,
+ field_data);
+}
diff --git a/security/integrity/ima/ima_template_lib.h b/security/integrity/ima/ima_template_lib.h
index 6a3d8b831deb..12f1a8578b31 100644
--- a/security/integrity/ima/ima_template_lib.h
+++ b/security/integrity/ima/ima_template_lib.h
@@ -29,6 +29,8 @@ void ima_show_template_string(struct seq_file *m, enum ima_show_type show,
struct ima_field_data *field_data);
void ima_show_template_sig(struct seq_file *m, enum ima_show_type show,
struct ima_field_data *field_data);
+void ima_show_template_buf(struct seq_file *m, enum ima_show_type show,
+ struct ima_field_data *field_data);
int ima_parse_buf(void *bufstartp, void *bufendp, void **bufcurp,
int maxfields, struct ima_field_data *fields, int *curfields,
unsigned long *len_mask, int enforce_mask, char *bufname);
@@ -42,4 +44,6 @@ int ima_eventname_ng_init(struct ima_event_data *event_data,
struct ima_field_data *field_data);
int ima_eventsig_init(struct ima_event_data *event_data,
struct ima_field_data *field_data);
+int ima_eventbuf_init(struct ima_event_data *event_data,
+ struct ima_field_data *field_data);
#endif /* __LINUX_IMA_TEMPLATE_LIB_H */
--
2.19.1
During soft reboot(kexec_file_load) boot command line
arguments are not measured.
Call ima hook ima_kexec_cmdline to measure the boot command line
arguments into IMA measurement list.
- call ima_kexec_cmdline from kexec_file_load.
- move the call ima_add_kexec_buffer after the cmdline
args have been measured.
Signed-off-by: Prakhar Srivastava <[email protected]>
Reviewed-by: James Morris <[email protected]>
Acked-by: Dave Young <[email protected]>
---
kernel/kexec_file.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index 072b6ee55e3f..b0c724e5d86c 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -198,9 +198,6 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd,
return ret;
image->kernel_buf_len = size;
- /* IMA needs to pass the measurement list to the next kernel. */
- ima_add_kexec_buffer(image);
-
/* Call arch image probe handlers */
ret = arch_kexec_kernel_image_probe(image, image->kernel_buf,
image->kernel_buf_len);
@@ -241,8 +238,14 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd,
ret = -EINVAL;
goto out;
}
+
+ ima_kexec_cmdline(image->cmdline_buf,
+ image->cmdline_buf_len - 1);
}
+ /* IMA needs to pass the measurement list to the next kernel. */
+ ima_add_kexec_buffer(image);
+
/* Call arch image load handlers */
ldata = arch_kexec_kernel_image_load(image);
--
2.19.1
Hello Prakhar,
Prakhar Srivastava <[email protected]> writes:
> diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c
> index 00dd5a434689..a01a17e5c581 100644
> --- a/security/integrity/ima/ima_template.c
> +++ b/security/integrity/ima/ima_template.c
> @@ -26,6 +26,7 @@ static struct ima_template_desc builtin_templates[] = {
> {.name = IMA_TEMPLATE_IMA_NAME, .fmt = IMA_TEMPLATE_IMA_FMT},
> {.name = "ima-ng", .fmt = "d-ng|n-ng"},
> {.name = "ima-sig", .fmt = "d-ng|n-ng|sig"},
> + {.name = "ima-buf", .fmt = "d-ng|n-ng|buf"},
> {.name = "", .fmt = ""}, /* placeholder for a custom format */
> };
>
> @@ -43,6 +44,8 @@ static const struct ima_template_field supported_fields[] = {
> .field_show = ima_show_template_string},
> {.field_id = "sig", .field_init = ima_eventsig_init,
> .field_show = ima_show_template_sig},
> + {.field_id = "buf", .field_init = ima_eventbuf_init,
> + .field_show = ima_show_template_buf},
> };
> #define MAX_TEMPLATE_NAME_LEN 15
Currently, MAX_TEMPLATE_NAME_LEN is the length of a template that
contains all valid fields. It may make sense to increase it since
there's a new field being added.
I suggest using a sizeof() to show where the number comes from (and
which can be visually shown to be correct):
#define MAX_TEMPLATE_NAME_LEN sizeof("d|n|d-ng|n-ng|sig|buf")
The sizeof() is calculated at compile time.
--
Thiago Jung Bauermann
IBM Linux Technology Center
On Mon, 2019-06-24 at 19:03 -0300, Thiago Jung Bauermann wrote:
> Hello Prakhar,
>
> Prakhar Srivastava <[email protected]> writes:
>
> > diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c
> > index 00dd5a434689..a01a17e5c581 100644
> > --- a/security/integrity/ima/ima_template.c
> > +++ b/security/integrity/ima/ima_template.c
> > @@ -26,6 +26,7 @@ static struct ima_template_desc builtin_templates[] = {
> > {.name = IMA_TEMPLATE_IMA_NAME, .fmt = IMA_TEMPLATE_IMA_FMT},
> > {.name = "ima-ng", .fmt = "d-ng|n-ng"},
> > {.name = "ima-sig", .fmt = "d-ng|n-ng|sig"},
> > + {.name = "ima-buf", .fmt = "d-ng|n-ng|buf"},
> > {.name = "", .fmt = ""}, /* placeholder for a custom format */
> > };
> >
> > @@ -43,6 +44,8 @@ static const struct ima_template_field supported_fields[] = {
> > .field_show = ima_show_template_string},
> > {.field_id = "sig", .field_init = ima_eventsig_init,
> > .field_show = ima_show_template_sig},
> > + {.field_id = "buf", .field_init = ima_eventbuf_init,
> > + .field_show = ima_show_template_buf},
> > };
> > #define MAX_TEMPLATE_NAME_LEN 15
>
> Currently, MAX_TEMPLATE_NAME_LEN is the length of a template that
> contains all valid fields. It may make sense to increase it since
> there's a new field being added.
>
> I suggest using a sizeof() to show where the number comes from (and
> which can be visually shown to be correct):
>
> #define MAX_TEMPLATE_NAME_LEN sizeof("d|n|d-ng|n-ng|sig|buf")
>
> The sizeof() is calculated at compile time.
MAX_TEMPLATE_NAME_LEN is used when restoring measurements carried over
from a kexec. 'd' and 'd-ng' should not both be defined in the
template description, nor should 'n' and 'n-ng'. Even without the
duplication, the MAX_TEPLATE_NAME_LEN is greater than the current 15.
Thiago, could you address this as a separate patch?
thanks!
Mimi
Mimi Zohar <[email protected]> writes:
> On Mon, 2019-06-24 at 19:03 -0300, Thiago Jung Bauermann wrote:
>> Hello Prakhar,
>>
>> Prakhar Srivastava <[email protected]> writes:
>>
>> > diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c
>> > index 00dd5a434689..a01a17e5c581 100644
>> > --- a/security/integrity/ima/ima_template.c
>> > +++ b/security/integrity/ima/ima_template.c
>> > @@ -26,6 +26,7 @@ static struct ima_template_desc builtin_templates[] = {
>> > {.name = IMA_TEMPLATE_IMA_NAME, .fmt = IMA_TEMPLATE_IMA_FMT},
>> > {.name = "ima-ng", .fmt = "d-ng|n-ng"},
>> > {.name = "ima-sig", .fmt = "d-ng|n-ng|sig"},
>> > + {.name = "ima-buf", .fmt = "d-ng|n-ng|buf"},
>> > {.name = "", .fmt = ""}, /* placeholder for a custom format */
>> > };
>> >
>> > @@ -43,6 +44,8 @@ static const struct ima_template_field supported_fields[] = {
>> > .field_show = ima_show_template_string},
>> > {.field_id = "sig", .field_init = ima_eventsig_init,
>> > .field_show = ima_show_template_sig},
>> > + {.field_id = "buf", .field_init = ima_eventbuf_init,
>> > + .field_show = ima_show_template_buf},
>> > };
>> > #define MAX_TEMPLATE_NAME_LEN 15
>>
>> Currently, MAX_TEMPLATE_NAME_LEN is the length of a template that
>> contains all valid fields. It may make sense to increase it since
>> there's a new field being added.
>>
>> I suggest using a sizeof() to show where the number comes from (and
>> which can be visually shown to be correct):
>>
>> #define MAX_TEMPLATE_NAME_LEN sizeof("d|n|d-ng|n-ng|sig|buf")
>>
>> The sizeof() is calculated at compile time.
>
> MAX_TEMPLATE_NAME_LEN is used when restoring measurements carried over
> from a kexec. 'd' and 'd-ng' should not both be defined in the
> template description, nor should 'n' and 'n-ng'.
Ah, makes sense. Thanks for that information.
> Even without the
> duplication, the MAX_TEPLATE_NAME_LEN is greater than the current 15.
>
> Thiago, could you address this as a separate patch?
Yes, I just sent a patch.
--
Thiago Jung Bauermann
IBM Linux Technology Center