diff mbox series

[v3,1/9] KEYS: Defined an IMA hook to measure keys on key create or update

Message ID 20191031011910.2574-2-nramas@linux.microsoft.com (mailing list archive)
State New, archived
Headers show
Series KEYS: Measure keys when they are created or updated | expand

Commit Message

Lakshmi Ramasubramanian Oct. 31, 2019, 1:19 a.m. UTC
Asymmetric keys used for verifying file signatures or certificates
are currently not included in the IMA measurement list.

This patch defines a new IMA hook namely ima_post_key_create_or_update()
to measure asymmetric keys.

Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
---
 security/integrity/ima/ima.h      |  2 ++
 security/integrity/ima/ima_main.c | 26 ++++++++++++++++++++++++++
 2 files changed, 28 insertions(+)

Comments

Sasha Levin Oct. 31, 2019, 9:10 a.m. UTC | #1
On Wed, Oct 30, 2019 at 06:19:02PM -0700, Lakshmi Ramasubramanian wrote:
>Asymmetric keys used for verifying file signatures or certificates
>are currently not included in the IMA measurement list.
>
>This patch defines a new IMA hook namely ima_post_key_create_or_update()
>to measure asymmetric keys.
>
>Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>

What are the prerequisites for this patch?
Mimi Zohar Oct. 31, 2019, 12:10 p.m. UTC | #2
On Wed, 2019-10-30 at 18:19 -0700, Lakshmi Ramasubramanian wrote:
> Asymmetric keys used for verifying file signatures or certificates
> are currently not included in the IMA measurement list.
> 
> This patch defines a new IMA hook namely ima_post_key_create_or_update()
> to measure asymmetric keys.

It's not enough for the kernel to be able to compile the kernel after
applying all the patches in a patch set.  After applying each patch,
the kernel should build properly, otherwise it is not bi-sect safe.
 Refer to "3) Separate your changes" of
"Documentation/process/submitting-patches.rst.
 
> 
> Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
> ---
>  security/integrity/ima/ima.h      |  2 ++
>  security/integrity/ima/ima_main.c | 26 ++++++++++++++++++++++++++
>  2 files changed, 28 insertions(+)
> 
> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index 997a57137351..22d0628faf56 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h
> @@ -21,6 +21,8 @@
>  #include <linux/tpm.h>
>  #include <linux/audit.h>
>  #include <crypto/hash_info.h>
> +#include <crypto/public_key.h>
> +#include <keys/asymmetric-type.h>
>  
>  #include "../integrity.h"
>  
> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> index 492b8f241d39..18e1bc105be7 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -635,6 +635,9 @@ void process_buffer_measurement(const void *buf, int size,
>  	int action = 0;
>  	u32 secid;
>  
> +	if (!ima_policy_flag)
> +		return;
> +
>  	if (func) {
>  		security_task_getsecid(current, &secid);
>  		action = ima_get_action(NULL, current_cred(), secid, 0, func,
> @@ -695,6 +698,29 @@ void ima_kexec_cmdline(const void *buf, int size)
>  	}
>  }
>  
> +/**
> + * 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;
> +
> +	if (key->type != &key_type_asymmetric)
> +		return;
> +
> +	pk = key->payload.data[asym_crypto];
> +	process_buffer_measurement(pk->key, pk->keylen,
> +				   keyring->description,
> +				   NONE, 0);

This patch should also define the new "func".

Mimi

> +}
> +
>  static int __init init_ima(void)
>  {
>  	int error;
Lakshmi Ramasubramanian Oct. 31, 2019, 3:08 p.m. UTC | #3
On 10/31/19 5:10 AM, Mimi Zohar wrote:

> On Wed, 2019-10-30 at 18:19 -0700, Lakshmi Ramasubramanian wrote:
>> Asymmetric keys used for verifying file signatures or certificates
>> are currently not included in the IMA measurement list.
>>
>> This patch defines a new IMA hook namely ima_post_key_create_or_update()
>> to measure asymmetric keys.
> 
> It's not enough for the kernel to be able to compile the kernel after
> applying all the patches in a patch set.  After applying each patch,
> the kernel should build properly, otherwise it is not bi-sect safe.
>   Refer to "3) Separate your changes" of
> "Documentation/process/submitting-patches.rst.

I started with kernel version 5.3 for this patch set.
I applied Nayna's process_buffer_measurement() patch and then built my 
changes on top of that.
This patch has no other dependency as far as I know.

Are you seeing a build break after applying this patch alone?

(PATCH v3 1/9) KEYS: Defined an IMA hook to measure keys on key create 
or update
> 
> This patch should also define the new "func".
> 

Ok - I'll make that change.

thanks,
  -lakshmi
Sasha Levin Oct. 31, 2019, 3:27 p.m. UTC | #4
On Thu, Oct 31, 2019 at 08:08:48AM -0700, Lakshmi Ramasubramanian wrote:
>On 10/31/19 5:10 AM, Mimi Zohar wrote:
>
>>On Wed, 2019-10-30 at 18:19 -0700, Lakshmi Ramasubramanian wrote:
>>>Asymmetric keys used for verifying file signatures or certificates
>>>are currently not included in the IMA measurement list.
>>>
>>>This patch defines a new IMA hook namely ima_post_key_create_or_update()
>>>to measure asymmetric keys.
>>
>>It's not enough for the kernel to be able to compile the kernel after
>>applying all the patches in a patch set.  After applying each patch,
>>the kernel should build properly, otherwise it is not bi-sect safe.
>>  Refer to "3) Separate your changes" of
>>"Documentation/process/submitting-patches.rst.
>
>I started with kernel version 5.3 for this patch set.
>I applied Nayna's process_buffer_measurement() patch and then built my 
>changes on top of that.
>This patch has no other dependency as far as I know.
>
>Are you seeing a build break after applying this patch alone?
>
>(PATCH v3 1/9) KEYS: Defined an IMA hook to measure keys on key create 
>or update

I couldn't even apply this patch: Nayna's series (v10) doesn't apply on
top of 5.3 to begin with, and while it does apply on mainline, this
first patch wouldn't apply on top.
Lakshmi Ramasubramanian Oct. 31, 2019, 3:27 p.m. UTC | #5
On 10/31/19 2:10 AM, Sasha Levin wrote:

Hi Sasha,

> On Wed, Oct 30, 2019 at 06:19:02PM -0700, Lakshmi Ramasubramanian wrote:
>> Asymmetric keys used for verifying file signatures or certificates
>> are currently not included in the IMA measurement list.
>>
>> This patch defines a new IMA hook namely ima_post_key_create_or_update()
>> to measure asymmetric keys.
>>
>> Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
> 
> What are the prerequisites for this patch?

I built this patch set on kernel v5.3

I applied the following patch provided by Nayna Jain@IBM and then added 
my changes:

	[PATCH v9 5/8] ima: make process_buffer_measurement() generic

thanks,
  -lakshmi
Sasha Levin Oct. 31, 2019, 3:36 p.m. UTC | #6
On Thu, Oct 31, 2019 at 08:27:47AM -0700, Lakshmi Ramasubramanian wrote:
>On 10/31/19 2:10 AM, Sasha Levin wrote:
>
>Hi Sasha,
>
>>On Wed, Oct 30, 2019 at 06:19:02PM -0700, Lakshmi Ramasubramanian wrote:
>>>Asymmetric keys used for verifying file signatures or certificates
>>>are currently not included in the IMA measurement list.
>>>
>>>This patch defines a new IMA hook namely ima_post_key_create_or_update()
>>>to measure asymmetric keys.
>>>
>>>Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
>>
>>What are the prerequisites for this patch?
>
>I built this patch set on kernel v5.3
>
>I applied the following patch provided by Nayna Jain@IBM and then 
>added my changes:
>
>	[PATCH v9 5/8] ima: make process_buffer_measurement() generic

$ git checkout v5.3
HEAD is now at 4d856f72c10ec Linux 5.3
$ git am ~/incoming/_PATCH_v9_5-8_ima_make_process_buffer_measurement__generic.patch
Applying: ima: make process_buffer_measurement() generic
error: patch failed: security/integrity/ima/ima.h:217
error: security/integrity/ima/ima.h: patch does not apply

What am I missing?
Mimi Zohar Oct. 31, 2019, 3:37 p.m. UTC | #7
On Thu, 2019-10-31 at 11:27 -0400, Sasha Levin wrote:
> On Thu, Oct 31, 2019 at 08:08:48AM -0700, Lakshmi Ramasubramanian wrote:
> >On 10/31/19 5:10 AM, Mimi Zohar wrote:
> >
> >>On Wed, 2019-10-30 at 18:19 -0700, Lakshmi Ramasubramanian wrote:
> >>>Asymmetric keys used for verifying file signatures or certificates
> >>>are currently not included in the IMA measurement list.
> >>>
> >>>This patch defines a new IMA hook namely ima_post_key_create_or_update()
> >>>to measure asymmetric keys.
> >>
> >>It's not enough for the kernel to be able to compile the kernel after
> >>applying all the patches in a patch set.  After applying each patch,
> >>the kernel should build properly, otherwise it is not bi-sect safe.
> >>  Refer to "3) Separate your changes" of
> >>"Documentation/process/submitting-patches.rst.
> >
> >I started with kernel version 5.3 for this patch set.
> >I applied Nayna's process_buffer_measurement() patch and then built my 
> >changes on top of that.
> >This patch has no other dependency as far as I know.
> >
> >Are you seeing a build break after applying this patch alone?
> >
> >(PATCH v3 1/9) KEYS: Defined an IMA hook to measure keys on key create 
> >or update
> 
> I couldn't even apply this patch: Nayna's series (v10) doesn't apply on
> top of 5.3 to begin with, and while it does apply on mainline, this
> first patch wouldn't apply on top.

Lakshmi, development is always on top of mainline.  In this case,
 please use 5.4.0-rc3 and apply Nayna's v10 patch set.

Mimi
Lakshmi Ramasubramanian Oct. 31, 2019, 3:42 p.m. UTC | #8
On 10/31/19 8:37 AM, Mimi Zohar wrote:

>> I couldn't even apply this patch: Nayna's series (v10) doesn't apply  >> top of 5.3 to begin with, and while it does apply on mainline, 
this>> first patch wouldn't apply on top.
> Lakshmi, development is always on top of mainline.  In this case,
>   please use 5.4.0-rc3 and apply Nayna's v10 patch set.
> 
> Mimi


Thanks for the info Mimi.

I initially started with v5.4, but the kernel I built wouldn't boot on 
my machine :(

I'll update to the latest v5.4 changes and try again.

thanks,
  -lakshmi
diff mbox series

Patch

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 997a57137351..22d0628faf56 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -21,6 +21,8 @@ 
 #include <linux/tpm.h>
 #include <linux/audit.h>
 #include <crypto/hash_info.h>
+#include <crypto/public_key.h>
+#include <keys/asymmetric-type.h>
 
 #include "../integrity.h"
 
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 492b8f241d39..18e1bc105be7 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -635,6 +635,9 @@  void process_buffer_measurement(const void *buf, int size,
 	int action = 0;
 	u32 secid;
 
+	if (!ima_policy_flag)
+		return;
+
 	if (func) {
 		security_task_getsecid(current, &secid);
 		action = ima_get_action(NULL, current_cred(), secid, 0, func,
@@ -695,6 +698,29 @@  void ima_kexec_cmdline(const void *buf, int size)
 	}
 }
 
+/**
+ * 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;
+
+	if (key->type != &key_type_asymmetric)
+		return;
+
+	pk = key->payload.data[asym_crypto];
+	process_buffer_measurement(pk->key, pk->keylen,
+				   keyring->description,
+				   NONE, 0);
+}
+
 static int __init init_ima(void)
 {
 	int error;