From: Tyler Hicks <tyhicks@linux.microsoft.com>
To: Tushar Sugandhi <tusharsu@linux.microsoft.com>
Cc: zohar@linux.ibm.com, stephen.smalley.work@gmail.com,
casey@schaufler-ca.com, agk@redhat.com, snitzer@redhat.com,
gmazyland@gmail.com, paul@paul-moore.com, sashal@kernel.org,
jmorris@namei.org, nramas@linux.microsoft.com,
linux-integrity@vger.kernel.org, selinux@vger.kernel.org,
linux-security-module@vger.kernel.org,
linux-kernel@vger.kernel.org, dm-devel@redhat.com
Subject: Re: [PATCH v8 8/8] selinux: include a consumer of the new IMA critical data hook
Date: Fri, 11 Dec 2020 18:32:15 -0600 [thread overview]
Message-ID: <20201212003215.GG4951@sequoia> (raw)
In-Reply-To: <20201211235807.30815-9-tusharsu@linux.microsoft.com>
On 2020-12-11 15:58:07, Tushar Sugandhi wrote:
> From: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
>
> SELinux stores the active policy in memory, so the changes to this data
> at runtime would have an impact on the security guarantees provided
> by SELinux. Measuring in-memory SELinux policy through IMA subsystem
> provides a secure way for the attestation service to remotely validate
> the policy contents at runtime.
>
> Measure the hash of the loaded policy by calling the IMA hook
> ima_measure_critical_data(). Since the size of the loaded policy can
> be large (several MB), measure the hash of the policy instead of
> the entire policy to avoid bloating the IMA log entry.
>
> Add "selinux" to the list of supported data sources maintained by IMA
> to enable measuring SELinux data.
>
> To enable SELinux data measurement, the following steps are required:
>
> 1, Add "ima_policy=critical_data" to the kernel command line arguments
> to enable measuring SELinux data at boot time.
> For example,
> BOOT_IMAGE=/boot/vmlinuz-5.10.0-rc1+ root=UUID=fd643309-a5d2-4ed3-b10d-3c579a5fab2f ro nomodeset security=selinux ima_policy=critical_data
>
> 2, Add the following rule to /etc/ima/ima-policy
> measure func=CRITICAL_DATA data_source=selinux
>
> Sample measurement of the hash of SELinux policy:
>
> To verify the measured data with the current SELinux policy run
> the following commands and verify the output hash values match.
>
> sha256sum /sys/fs/selinux/policy | cut -d' ' -f 1
>
> grep "selinux-policy-hash" /sys/kernel/security/integrity/ima/ascii_runtime_measurements | tail -1 | cut -d' ' -f 6
>
> Note that the actual verification of SELinux policy would require loading
> the expected policy into an identical kernel on a pristine/known-safe
> system and run the sha256sum /sys/kernel/selinux/policy there to get
> the expected hash.
>
> Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
> Suggested-by: Stephen Smalley <stephen.smalley.work@gmail.com>
This looks good but I've got one small suggestion below if you roll a
v9. Feel free to add:
Reviewed-by: Tyler Hicks <tyhicks@linux.microsoft.com>
> diff --git a/security/selinux/measure.c b/security/selinux/measure.c
> new file mode 100644
> index 000000000000..a070d8dae403
> --- /dev/null
> +++ b/security/selinux/measure.c
> @@ -0,0 +1,81 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Measure SELinux state using IMA subsystem.
> + */
> +#include <linux/vmalloc.h>
> +#include <linux/ktime.h>
> +#include <linux/ima.h>
> +#include "security.h"
> +
> +/*
> + * This function creates a unique name by appending the timestamp to
> + * the given string. This string is passed as "event_name" to the IMA
> + * hook to measure the given SELinux data.
> + *
> + * The data provided by SELinux to the IMA subsystem for measuring may have
> + * already been measured (for instance the same state existed earlier).
> + * But for SELinux the current data represents a state change and hence
> + * needs to be measured again. To enable this, pass a unique "event_name"
> + * to the IMA hook so that IMA subsystem will always measure the given data.
> + *
> + * For example,
> + * At time T0 SELinux data to be measured is "foo". IMA measures it.
> + * At time T1 the data is changed to "bar". IMA measures it.
> + * At time T2 the data is changed to "foo" again. IMA will not measure it
> + * (since it was already measured) unless the event_name, for instance,
> + * is different in this call.
> + */
> +static char *selinux_event_name(const char *name_prefix)
> +{
> + char *event_name = NULL;
> + struct timespec64 cur_time;
> +
> + ktime_get_real_ts64(&cur_time);
> + event_name = kasprintf(GFP_KERNEL, "%s-%lld:%09ld", name_prefix,
> + cur_time.tv_sec, cur_time.tv_nsec);
> + return event_name;
There's no longer a need to store the return of kasprintf() in a
variable. Just 'return kasprint(...);' and get rid of the event_name
variable.
Tyler
next prev parent reply other threads:[~2020-12-12 0:48 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-12-11 23:57 [PATCH v8 0/8] IMA: support for measuring kernel integrity critical data Tushar Sugandhi
2020-12-11 23:58 ` [PATCH v8 1/8] IMA: generalize keyring specific measurement constructs Tushar Sugandhi
2020-12-12 0:08 ` Tyler Hicks
2020-12-11 23:58 ` [PATCH v8 2/8] IMA: add support to measure buffer data hash Tushar Sugandhi
2020-12-12 0:09 ` Tyler Hicks
2020-12-11 23:58 ` [PATCH v8 3/8] IMA: define a hook to measure kernel integrity critical data Tushar Sugandhi
2020-12-12 0:13 ` Tyler Hicks
2020-12-11 23:58 ` [PATCH v8 4/8] IMA: add policy rule to measure " Tushar Sugandhi
2020-12-12 0:25 ` Tyler Hicks
2020-12-12 1:17 ` Tushar Sugandhi
2020-12-12 14:47 ` Tyler Hicks
2020-12-12 17:34 ` Tushar Sugandhi
2020-12-11 23:58 ` [PATCH v8 5/8] IMA: limit critical data measurement based on a label Tushar Sugandhi
2020-12-11 23:58 ` [PATCH v8 6/8] IMA: extend critical data hook to limit the " Tushar Sugandhi
2020-12-11 23:58 ` [PATCH v8 7/8] IMA: define a builtin critical data measurement policy Tushar Sugandhi
2020-12-11 23:58 ` [PATCH v8 8/8] selinux: include a consumer of the new IMA critical data hook Tushar Sugandhi
2020-12-12 0:32 ` Tyler Hicks [this message]
2020-12-12 0:33 ` Lakshmi Ramasubramanian
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20201212003215.GG4951@sequoia \
--to=tyhicks@linux.microsoft.com \
--cc=agk@redhat.com \
--cc=casey@schaufler-ca.com \
--cc=dm-devel@redhat.com \
--cc=gmazyland@gmail.com \
--cc=jmorris@namei.org \
--cc=linux-integrity@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-security-module@vger.kernel.org \
--cc=nramas@linux.microsoft.com \
--cc=paul@paul-moore.com \
--cc=sashal@kernel.org \
--cc=selinux@vger.kernel.org \
--cc=snitzer@redhat.com \
--cc=stephen.smalley.work@gmail.com \
--cc=tusharsu@linux.microsoft.com \
--cc=zohar@linux.ibm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).