| Message ID | 20191118223818.3353-3-nramas@linux.microsoft.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KEYS: Measure keys when they are created or updated | expand |
> On Nov 18, 2019, at 3:38 PM, Lakshmi Ramasubramanian <nramas@linux.microsoft.com> wrote: > > Measure asymmetric keys used for verifying file signatures, > certificates, etc. > > This patch defines a new IMA hook namely ima_post_key_create_or_update() > to measure asymmetric keys. > > The IMA hook is defined in a new file namely ima_asymmetric_keys.c > which is built only if CONFIG_ASYMMETRIC_PUBLIC_KEY_SUBTYPE is enabled. > > Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com> > Cc: Sasha Levin <sashal@kernel.org> > Cc: James Morris <jamorris@linux.microsoft.com> > --- > security/integrity/ima/Makefile | 1 + > security/integrity/ima/ima_asymmetric_keys.c | 51 ++++++++++++++++++++ > 2 files changed, 52 insertions(+) > create mode 100644 security/integrity/ima/ima_asymmetric_keys.c > > diff --git a/security/integrity/ima/Makefile b/security/integrity/ima/Makefile > index 31d57cdf2421..207a0a9eb72c 100644 > --- a/security/integrity/ima/Makefile > +++ b/security/integrity/ima/Makefile > @@ -12,3 +12,4 @@ ima-$(CONFIG_IMA_APPRAISE) += ima_appraise.o > ima-$(CONFIG_IMA_APPRAISE_MODSIG) += ima_modsig.o > ima-$(CONFIG_HAVE_IMA_KEXEC) += ima_kexec.o > obj-$(CONFIG_IMA_BLACKLIST_KEYRING) += ima_mok.o > +obj-$(CONFIG_ASYMMETRIC_PUBLIC_KEY_SUBTYPE) += ima_asymmetric_keys.o > diff --git a/security/integrity/ima/ima_asymmetric_keys.c b/security/integrity/ima/ima_asymmetric_keys.c > new file mode 100644 > index 000000000000..f6884641a622 > --- /dev/null > +++ b/security/integrity/ima/ima_asymmetric_keys.c > @@ -0,0 +1,51 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * Copyright (C) 2019 Microsoft Corporation > + * > + * Author: Lakshmi Ramasubramanian (nramas@linux.microsoft.com) > + * > + * File: ima_asymmetric_keys.c > + * Defines an IMA hook to measure asymmetric keys on key > + * create or update. > + */ > + > +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt > + > +#include <crypto/public_key.h> > +#include <keys/asymmetric-type.h> > +#include "ima.h" > + > +/** > + * ima_post_key_create_or_update - measure asymmetric keys > + * @keyring: keyring to which the key is linked to > + * @key: created or updated key > + * @flags: key flags > + * @create: flag indicating whether the key was created or updated > + * > + * Keys can only be measured, not appraised. > + */ > +void ima_post_key_create_or_update(struct key *keyring, struct key *key, > + unsigned long flags, bool create) > +{ > + const struct public_key *pk; > + > + /* Only asymmetric keys are handled by this hook. */ > + if (key->type != &key_type_asymmetric) > + return; > + > + /* Get the public_key of the given asymmetric key to measure. */ > + pk = key->payload.data[asym_crypto]; > + > + /* > + * keyring->description points to the name of the keyring > + * (such as ".builtin_trusted_keys", ".ima", etc.) to > + * which the given key is linked to. > + * > + * The name of the keyring is passed in the "eventname" > + * parameter to process_buffer_measurement() and is set > + * in the "eventname" field in ima_event_data for > + * the key measurement IMA event. > + */ > + process_buffer_measurement(pk->key, pk->keylen, > + keyring->description, KEY_CHECK, 0); I’m interested in using this patch series, however I get the following on every boot: [ 1.185105] Loading compiled-in X.509 certificates [ 1.190240] BUG: kernel NULL pointer dereference, address: 0000000000000000 [ 1.191835] #PF: supervisor read access in kernel mode [ 1.193041] #PF: error_code(0x0000) - not-present page [ 1.194224] PGD 0 P4D 0 [ 1.194832] Oops: 0000 [#1] SMP PTI [ 1.195654] CPU: 1 PID: 1 Comm: swapper/0 Not tainted 5.4.0-rc7.imakeys.rc1.x86_64 #1 [ 1.197667] Hardware name: [ 1.198987] RIP: 0010:ima_match_policy+0x69/0x4e0 [ 1.200072] Code: 48 89 45 90 65 48 8b 04 25 28 00 00 00 48 89 45 d0 31 c0 4d 85 ff 74 08 e8 94 14 00 00 49 89 07 48 8b 05 8a 43 7f 01 45 31 e4 <48> 8b 18 48 39 d8 0f 84 25 02 00 00 41 8d 46 f5 45 89 e0 4c 8b 65 [ 1.204401] RSP: 0018:ffffb9f30001bac8 EFLAGS: 00010246 [ 1.205622] RAX: 0000000000000000 RBX: ffff9e659de81800 RCX: 000000000000000c [ 1.207275] RDX: 0000000000000000 RSI: ffffffff9b13cdf8 RDI: ffffffff9b13cdf8 [ 1.208938] RBP: ffffb9f30001bb48 R08: ffffffff9b529200 R09: 0000000000000000 [ 1.210560] R10: ffff9e6447d06c00 R11: 0000000082c49530 R12: 0000000000000000 [ 1.212279] R13: 0000000000000000 R14: 000000000000000c R15: ffffb9f30001bbb0 [ 1.213944] FS: 0000000000000000(0000) GS:ffff9e65b7a80000(0000) knlGS:0000000000000000 [ 1.215768] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 1.217114] CR2: 0000000000000000 CR3: 000000014240a001 CR4: 0000000000760ee0 [ 1.218734] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 [ 1.220481] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 [ 1.222139] PKRU: 55555554 [ 1.222749] Call Trace: [ 1.223344] ? crypto_destroy_tfm+0x5f/0xb0 [ 1.224315] ima_get_action+0x2c/0x30 [ 1.225148] process_buffer_measurement+0x1da/0x230 [ 1.226306] ima_post_key_create_or_update+0x3b/0x40 [ 1.227459] key_create_or_update+0x371/0x5c0 [ 1.228494] load_system_certificate_list+0x99/0xfa [ 1.229588] ? system_trusted_keyring_init+0xfb/0xfb [ 1.230705] ? do_early_param+0x95/0x95 [ 1.231574] do_one_initcall+0x4a/0x1fa [ 1.232444] ? do_early_param+0x95/0x95 [ 1.233313] kernel_init_freeable+0x1c2/0x267 [ 1.234300] ? rest_init+0xb0/0xb0 [ 1.235075] kernel_init+0xe/0x110 [ 1.235842] ret_from_fork+0x24/0x50 [ 1.236659] Modules linked in: [ 1.237358] CR2: 0000000000000000 [ 1.238112] ---[ end trace 163f3f61dfaef23f ]— I believe this is because ima_rules used within ima_match_policy has not been initialized yet, when process_buffer_measurement is called above.
On 11/20/2019 3:28 PM, Eric Snowberg wrote: Hi Eric, > > I’m interested in using this patch series, however I get the following on every boot: > [ 1.222749] Call Trace: > [ 1.223344] ? crypto_destroy_tfm+0x5f/0xb0 > [ 1.224315] ima_get_action+0x2c/0x30 > [ 1.225148] process_buffer_measurement+0x1da/0x230 > [ 1.226306] ima_post_key_create_or_update+0x3b/0x40 This is happening because IMA is not yet initialized when the IMA hook is called. I had the following check in process_buffer_measurement() as part of my patch, but removed it since it is being upstreamed separately (by Mimi) if (!ima_policy_flag) return; Until this change is in, please add the above line locally on entry to process_buffer_measurement() to get around the issue. thanks, -lakshmi
On Wed, 2019-11-20 at 15:40 -0800, Lakshmi Ramasubramanian wrote: > On 11/20/2019 3:28 PM, Eric Snowberg wrote: > Hi Eric, > > > > > I’m interested in using this patch series, however I get the following on every boot: > > > [ 1.222749] Call Trace: > > [ 1.223344] ? crypto_destroy_tfm+0x5f/0xb0 > > [ 1.224315] ima_get_action+0x2c/0x30 > > [ 1.225148] process_buffer_measurement+0x1da/0x230 > > [ 1.226306] ima_post_key_create_or_update+0x3b/0x40 > > This is happening because IMA is not yet initialized when the IMA hook > is called. > > I had the following check in process_buffer_measurement() as part of my > patch, but removed it since it is being upstreamed separately (by Mimi) > > if (!ima_policy_flag) > return; Did you post it as a separate patch? I can't seem to find it. Mimi > > Until this change is in, please add the above line locally on entry to > process_buffer_measurement() to get around the issue. > > thanks, > -lakshmi
On 11/20/19 5:22 PM, Mimi Zohar wrote: >> I had the following check in process_buffer_measurement() as part of my >> patch, but removed it since it is being upstreamed separately (by Mimi) >> >> if (!ima_policy_flag) >> return; > > Did you post it as a separate patch? I can't seem to find it. > > Mimi No - I removed the above change from my patch since you mentioned it's being upstreamed separately. I didn't realize you wanted me to include the above change alone in a separate patch (in my patch set). Sorry - I guess I misunderstood. I can do that when I send an update - I expect to by the end of this week. thanks, -lakshmi
On 11/20/19 5:22 PM, Mimi Zohar wrote: >> I had the following check in process_buffer_measurement() as part of my >> patch, but removed it since it is being upstreamed separately (by Mimi) >> >> if (!ima_policy_flag) >> return; > > Did you post it as a separate patch? I can't seem to find it. > > Mimi > I have sent a separate patch with just this change (to check ima_policy_flag in process_buffer_measurement()). thanks, -lakshmi
diff --git a/security/integrity/ima/Makefile b/security/integrity/ima/Makefile index 31d57cdf2421..207a0a9eb72c 100644 --- a/security/integrity/ima/Makefile +++ b/security/integrity/ima/Makefile @@ -12,3 +12,4 @@ ima-$(CONFIG_IMA_APPRAISE) += ima_appraise.o ima-$(CONFIG_IMA_APPRAISE_MODSIG) += ima_modsig.o ima-$(CONFIG_HAVE_IMA_KEXEC) += ima_kexec.o obj-$(CONFIG_IMA_BLACKLIST_KEYRING) += ima_mok.o +obj-$(CONFIG_ASYMMETRIC_PUBLIC_KEY_SUBTYPE) += ima_asymmetric_keys.o diff --git a/security/integrity/ima/ima_asymmetric_keys.c b/security/integrity/ima/ima_asymmetric_keys.c new file mode 100644 index 000000000000..f6884641a622 --- /dev/null +++ b/security/integrity/ima/ima_asymmetric_keys.c @@ -0,0 +1,51 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright (C) 2019 Microsoft Corporation + * + * Author: Lakshmi Ramasubramanian (nramas@linux.microsoft.com) + * + * File: ima_asymmetric_keys.c + * Defines an IMA hook to measure asymmetric keys on key + * create or update. + */ + +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt + +#include <crypto/public_key.h> +#include <keys/asymmetric-type.h> +#include "ima.h" + +/** + * ima_post_key_create_or_update - measure asymmetric keys + * @keyring: keyring to which the key is linked to + * @key: created or updated key + * @flags: key flags + * @create: flag indicating whether the key was created or updated + * + * Keys can only be measured, not appraised. + */ +void ima_post_key_create_or_update(struct key *keyring, struct key *key, + unsigned long flags, bool create) +{ + const struct public_key *pk; + + /* Only asymmetric keys are handled by this hook. */ + if (key->type != &key_type_asymmetric) + return; + + /* Get the public_key of the given asymmetric key to measure. */ + pk = key->payload.data[asym_crypto]; + + /* + * keyring->description points to the name of the keyring + * (such as ".builtin_trusted_keys", ".ima", etc.) to + * which the given key is linked to. + * + * The name of the keyring is passed in the "eventname" + * parameter to process_buffer_measurement() and is set + * in the "eventname" field in ima_event_data for + * the key measurement IMA event. + */ + process_buffer_measurement(pk->key, pk->keylen, + keyring->description, KEY_CHECK, 0); +}
Measure asymmetric keys used for verifying file signatures, certificates, etc. This patch defines a new IMA hook namely ima_post_key_create_or_update() to measure asymmetric keys. The IMA hook is defined in a new file namely ima_asymmetric_keys.c which is built only if CONFIG_ASYMMETRIC_PUBLIC_KEY_SUBTYPE is enabled. Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com> Cc: Sasha Levin <sashal@kernel.org> Cc: James Morris <jamorris@linux.microsoft.com> --- security/integrity/ima/Makefile | 1 + security/integrity/ima/ima_asymmetric_keys.c | 51 ++++++++++++++++++++ 2 files changed, 52 insertions(+) create mode 100644 security/integrity/ima/ima_asymmetric_keys.c