> On Feb 6, 2020, at 11:05 AM, Mimi Zohar <zohar@xxxxxxxxxxxxx> wrote:
>
> On Thu, 2020-02-06 at 11:42 -0500, Eric Snowberg wrote:
>> Currently IMA can validate compressed modules containing appended
>> signatures. This adds the ability to also validate uncompressed
>> modules when appraise_type=imasig|modsig.
>>
>> Signed-off-by: Eric Snowberg <eric.snowberg@xxxxxxxxxx>
>
> Your patch description in no way matches the code.
>
How about if I changed the description to the following:
Currently IMA can only validate compressed modules containing appended
signatures when appraise_type=imasig|modsig. An uncompressed module that
is internally signed must still be ima signed.
Add the ability to validate the uncompress module by validating it against
keys contained within the .builtin_trusted_keys keyring. Now when using a
policy such as:
appraise func=MODULE_CHECK appraise_type=imasig|modsig
It will load modules containing an appended signature when either compressed
or uncompressed.
>> ---
>> security/integrity/digsig.c | 9 +++++++--
>> security/integrity/ima/ima_appraise.c | 3 +++
>> security/integrity/integrity.h | 3 ++-
>> 3 files changed, 12 insertions(+), 3 deletions(-)
>>
>> diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c
>> index ea1aae3d07b3..5e0c4d04ab9d 100644
>> --- a/security/integrity/digsig.c
>> +++ b/security/integrity/digsig.c
>> @@ -15,6 +15,7 @@
>> #include <linux/key-type.h>
>> #include <linux/digsig.h>
>> #include <linux/vmalloc.h>
>> +#include <linux/verification.h>
>> #include <crypto/public_key.h>
>> #include <keys/system_keyring.h>
>>
>> @@ -31,6 +32,7 @@ static const char * const keyring_name[INTEGRITY_KEYRING_MAX] = {
>> ".ima",
>> #endif
>> ".platform",
>> + ".builtin_trusted_keys",
>> };
>>
>> #ifdef CONFIG_IMA_KEYRINGS_PERMIT_SIGNED_BY_BUILTIN_OR_SECONDARY
>> @@ -45,8 +47,11 @@ static struct key *integrity_keyring_from_id(const unsigned int id)
>> return ERR_PTR(-EINVAL);
>>
>> if (!keyring[id]) {
>> - keyring[id] =
>> - request_key(&key_type_keyring, keyring_name[id], NULL);
>> + if (id == INTEGRITY_KEYRING_KERNEL)
>> + keyring[id] = VERIFY_USE_SECONDARY_KEYRING;
>> + else
>> + keyring[id] = request_key(&key_type_keyring,
>> + keyring_name[id], NULL);
>> if (IS_ERR(keyring[id])) {
>> int err = PTR_ERR(keyring[id]);
>> pr_err("no %s keyring: %d\n", keyring_name[id], err);
>> diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
>> index 300c8d2943c5..4c009c55d620 100644
>> --- a/security/integrity/ima/ima_appraise.c
>> +++ b/security/integrity/ima/ima_appraise.c
>> @@ -294,6 +294,9 @@ static int modsig_verify(enum ima_hooks func, const struct modsig *modsig,
>> func == KEXEC_KERNEL_CHECK)
>> rc = integrity_modsig_verify(INTEGRITY_KEYRING_PLATFORM,
>> modsig);
>> + if (rc && func == MODULE_CHECK)
>> + rc = integrity_modsig_verify(INTEGRITY_KEYRING_KERNEL, modsig);
>> +
>> if (rc) {
>> *cause = "invalid-signature";
>> *status = INTEGRITY_FAIL;
>> diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
>> index 73fc286834d7..63f0e6bff0e0 100644
>> --- a/security/integrity/integrity.h
>> +++ b/security/integrity/integrity.h
>> @@ -145,7 +145,8 @@ int integrity_kernel_read(struct file *file, loff_t offset,
>> #define INTEGRITY_KEYRING_EVM 0
>> #define INTEGRITY_KEYRING_IMA 1
>> #define INTEGRITY_KEYRING_PLATFORM 2
>> -#define INTEGRITY_KEYRING_MAX 3
>> +#define INTEGRITY_KEYRING_KERNEL 3
>> +#define INTEGRITY_KEYRING_MAX 4
>>
>> extern struct dentry *integrity_dir;
>>
>
