diff mbox series

[RFC,v15,12/21] security: add security_bdev_setintegrity() hook

Message ID 1710560151-28904-13-git-send-email-wufan@linux.microsoft.com (mailing list archive)
State Changes Requested
Delegated to: Paul Moore
Headers show
Series Integrity Policy Enforcement LSM (IPE) | expand

Commit Message

Fan Wu March 16, 2024, 3:35 a.m. UTC
This patch introduces a new hook to save block device's integrity
data. For example, for dm-verity, LSMs can use this hook to save
the roothash signature of a dm-verity into the security blob,
and LSMs can make access decisions based on the data inside
the signature, like the signer certificate.

Signed-off-by: Fan Wu <wufan@linux.microsoft.com>

--
v1-v14:
  + Not present

v15:
  + Introduced
---
 include/linux/lsm_hook_defs.h |  2 ++
 include/linux/security.h      | 14 ++++++++++++++
 security/security.c           | 28 ++++++++++++++++++++++++++++
 3 files changed, 44 insertions(+)

Comments

Paul Moore March 19, 2024, 11 p.m. UTC | #1
On Mar 15, 2024 Fan Wu <wufan@linux.microsoft.com> wrote:
> 
> This patch introduces a new hook to save block device's integrity
> data. For example, for dm-verity, LSMs can use this hook to save
> the roothash signature of a dm-verity into the security blob,
> and LSMs can make access decisions based on the data inside
> the signature, like the signer certificate.
> 
> Signed-off-by: Fan Wu <wufan@linux.microsoft.com>
> 
> --
> v1-v14:
>   + Not present
> 
> v15:
>   + Introduced
> 
> ---
>  include/linux/lsm_hook_defs.h |  2 ++
>  include/linux/security.h      | 14 ++++++++++++++
>  security/security.c           | 28 ++++++++++++++++++++++++++++
>  3 files changed, 44 insertions(+)

I'm not sure why you made this a separate patch, help?  If there is
no significant reason why this is separate, please squash it together
with patch 11/21.

> diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
> index c335404470dc..6808ae763913 100644
> --- a/include/linux/lsm_hook_defs.h
> +++ b/include/linux/lsm_hook_defs.h
> @@ -455,4 +455,6 @@ LSM_HOOK(void, LSM_RET_VOID, initramfs_populated, void)
>  
>  LSM_HOOK(int, 0, bdev_alloc_security, struct block_device *bdev)
>  LSM_HOOK(void, LSM_RET_VOID, bdev_free_security, struct block_device *bdev)
> +LSM_HOOK(int, 0, bdev_setintegrity, struct block_device *bdev,
> +	 enum lsm_intgr_type type, const void *value, size_t size)
>  
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 9965b5c50df4..eaff8868766a 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -83,6 +83,10 @@ enum lsm_event {
>  	LSM_POLICY_CHANGE,
>  };
>  
> +enum lsm_intgr_type {
> +	__LSM_INTGR_MAX
> +};
> +
>  /*
>   * These are reasons that can be passed to the security_locked_down()
>   * LSM hook. Lockdown reasons that protect kernel integrity (ie, the
> @@ -511,6 +515,9 @@ int lsm_fill_user_ctx(struct lsm_ctx __user *uctx, u32 *uctx_len,
>  		      void *val, size_t val_len, u64 id, u64 flags);
>  int security_bdev_alloc(struct block_device *bdev);
>  void security_bdev_free(struct block_device *bdev);
> +int security_bdev_setintegrity(struct block_device *bdev,
> +			       enum lsm_intgr_type type, const void *value,
> +			       size_t size);
>  #else /* CONFIG_SECURITY */
>  
>  static inline int call_blocking_lsm_notifier(enum lsm_event event, void *data)
> @@ -1495,6 +1502,13 @@ static inline void security_bdev_free(struct block_device *bdev)
>  {
>  }
>  
> +static inline int security_bdev_setintegrity(struct block_device *bdev,
> +					     enum lsm_intgr_type, type,
> +					     const void *value, size_t size)
> +{
> +	return 0;
> +}
> +
>  #endif	/* CONFIG_SECURITY */
>  
>  #if defined(CONFIG_SECURITY) && defined(CONFIG_WATCH_QUEUE)
> diff --git a/security/security.c b/security/security.c
> index 4274bbee40d0..8d88529ac904 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -5637,6 +5637,34 @@ void security_bdev_free(struct block_device *bdev)
>  }
>  EXPORT_SYMBOL(security_bdev_free);
>  
> +/**
> + * security_bdev_setintegrity() - Set the bdev's integrity data

Let's just say "Set the device's integrity data" and not ask people to
figure out "bdev", although I will admit it should be fairly obvious :)

> + * @bdev: block device
> + * @type: type of integrity, e.g. hash digest, signature, etc
> + * @value: the integrity value
> + * @size: size of the integrity value
> + *
> + * Register a verified integrity measurement of a bdev with the LSM.
> + *
> + * Return: Returns 0 on success, negative values on failure.
> + */
> +int security_bdev_setintegrity(struct block_device *bdev,
> +			       enum lsm_intgr_type type, const void *value,
> +			       size_t size)
> +{
> +	int rc = 0;
> +	struct security_hook_list *p;
> +
> +	hlist_for_each_entry(p, &security_hook_heads.bdev_setintegrity, list) {
> +		rc = p->hook.bdev_setintegrity(bdev, type, value, size);
> +		if (rc)
> +			return rc;
> +	}
> +
> +	return LSM_RET_DEFAULT(bdev_setintegrity);

We can just use the call_int_hook() macro here instead of open coding
everything, right?

> +}
> +EXPORT_SYMBOL(security_bdev_setintegrity);
> +
>  #ifdef CONFIG_PERF_EVENTS
>  /**
>   * security_perf_event_open() - Check if a perf event open is allowed
> -- 
> 2.44.0

--
paul-moore.com
Jarkko Sakkinen March 20, 2024, 8:28 a.m. UTC | #2
On Wed Mar 20, 2024 at 1:00 AM EET, Paul Moore wrote:
> On Mar 15, 2024 Fan Wu <wufan@linux.microsoft.com> wrote:
> > 
> > This patch introduces a new hook to save block device's integrity
> > data. For example, for dm-verity, LSMs can use this hook to save
> > the roothash signature of a dm-verity into the security blob,
> > and LSMs can make access decisions based on the data inside
> > the signature, like the signer certificate.
> > 
> > Signed-off-by: Fan Wu <wufan@linux.microsoft.com>
> > 
> > --
> > v1-v14:
> >   + Not present
> > 
> > v15:
> >   + Introduced
> > 
> > ---
> >  include/linux/lsm_hook_defs.h |  2 ++
> >  include/linux/security.h      | 14 ++++++++++++++
> >  security/security.c           | 28 ++++++++++++++++++++++++++++
> >  3 files changed, 44 insertions(+)
>
> I'm not sure why you made this a separate patch, help?  If there is
> no significant reason why this is separate, please squash it together
> with patch 11/21.

Off-topic: it is weird to have *RFC* patch set at v15.

RFC by de-facto is something that can be safely ignored if you don't
have bandwidth. 15 versions of anything that can be safely ignored
is by definition spamming :-) I mean just conceptually.

So does the RFC still hold or what the heck is going on with this one?

Haven't followed for some time now...

BR, Jarkko
Jarkko Sakkinen March 20, 2024, 8:31 a.m. UTC | #3
On Wed Mar 20, 2024 at 10:28 AM EET, Jarkko Sakkinen wrote:
> On Wed Mar 20, 2024 at 1:00 AM EET, Paul Moore wrote:
> > On Mar 15, 2024 Fan Wu <wufan@linux.microsoft.com> wrote:
> > > 
> > > This patch introduces a new hook to save block device's integrity
> > > data. For example, for dm-verity, LSMs can use this hook to save
> > > the roothash signature of a dm-verity into the security blob,
> > > and LSMs can make access decisions based on the data inside
> > > the signature, like the signer certificate.
> > > 
> > > Signed-off-by: Fan Wu <wufan@linux.microsoft.com>
> > > 
> > > --
> > > v1-v14:
> > >   + Not present
> > > 
> > > v15:
> > >   + Introduced
> > > 
> > > ---
> > >  include/linux/lsm_hook_defs.h |  2 ++
> > >  include/linux/security.h      | 14 ++++++++++++++
> > >  security/security.c           | 28 ++++++++++++++++++++++++++++
> > >  3 files changed, 44 insertions(+)
> >
> > I'm not sure why you made this a separate patch, help?  If there is
> > no significant reason why this is separate, please squash it together
> > with patch 11/21.
>
> Off-topic: it is weird to have *RFC* patch set at v15.
>
> RFC by de-facto is something that can be safely ignored if you don't
> have bandwidth. 15 versions of anything that can be safely ignored
> is by definition spamming :-) I mean just conceptually.
>
> So does the RFC still hold or what the heck is going on with this one?
>
> Haven't followed for some time now...

I mean if this RFC trend continues I'll just put auto-filter for this
thread to put straight to the bin.  There's enough non-RFC patch sets
to review.

BR, Jarkko
Fan Wu March 20, 2024, 8:31 p.m. UTC | #4
On 3/20/2024 1:31 AM, Jarkko Sakkinen wrote:
> On Wed Mar 20, 2024 at 10:28 AM EET, Jarkko Sakkinen wrote:
>> On Wed Mar 20, 2024 at 1:00 AM EET, Paul Moore wrote:
>>> On Mar 15, 2024 Fan Wu <wufan@linux.microsoft.com> wrote:
>>>>
>>>> This patch introduces a new hook to save block device's integrity
>>>> data. For example, for dm-verity, LSMs can use this hook to save
>>>> the roothash signature of a dm-verity into the security blob,
>>>> and LSMs can make access decisions based on the data inside
>>>> the signature, like the signer certificate.
>>>>
>>>> Signed-off-by: Fan Wu <wufan@linux.microsoft.com>
>>>>
>>>> --
>>>> v1-v14:
>>>>    + Not present
>>>>
>>>> v15:
>>>>    + Introduced
>>>>
>>>> ---
>>>>   include/linux/lsm_hook_defs.h |  2 ++
>>>>   include/linux/security.h      | 14 ++++++++++++++
>>>>   security/security.c           | 28 ++++++++++++++++++++++++++++
>>>>   3 files changed, 44 insertions(+)
>>>
>>> I'm not sure why you made this a separate patch, help?  If there is
>>> no significant reason why this is separate, please squash it together
>>> with patch 11/21.
>>
>> Off-topic: it is weird to have *RFC* patch set at v15.
>>
>> RFC by de-facto is something that can be safely ignored if you don't
>> have bandwidth. 15 versions of anything that can be safely ignored
>> is by definition spamming :-) I mean just conceptually.
>>
>> So does the RFC still hold or what the heck is going on with this one?
>>
>> Haven't followed for some time now...
> 
> I mean if this RFC trend continues I'll just put auto-filter for this
> thread to put straight to the bin.  There's enough non-RFC patch sets
> to review.
> 
> BR, Jarkko

Sorry about the confusion with the RFC tag – I wasn't fully aware of its 
conventional meaning and how it's perceived in terms of importance and 
urgency. Point taken, and I'll make sure to remove the RFC tag for 
future submissions. Definitely not my intention to clog up the workflow 
or seem like I'm spamming.

-Fan
Jarkko Sakkinen March 21, 2024, 5:25 p.m. UTC | #5
On Wed Mar 20, 2024 at 10:31 PM EET, Fan Wu wrote:
>
>
> On 3/20/2024 1:31 AM, Jarkko Sakkinen wrote:
> > On Wed Mar 20, 2024 at 10:28 AM EET, Jarkko Sakkinen wrote:
> >> On Wed Mar 20, 2024 at 1:00 AM EET, Paul Moore wrote:
> >>> On Mar 15, 2024 Fan Wu <wufan@linux.microsoft.com> wrote:
> >>>>
> >>>> This patch introduces a new hook to save block device's integrity
> >>>> data. For example, for dm-verity, LSMs can use this hook to save
> >>>> the roothash signature of a dm-verity into the security blob,
> >>>> and LSMs can make access decisions based on the data inside
> >>>> the signature, like the signer certificate.
> >>>>
> >>>> Signed-off-by: Fan Wu <wufan@linux.microsoft.com>
> >>>>
> >>>> --
> >>>> v1-v14:
> >>>>    + Not present
> >>>>
> >>>> v15:
> >>>>    + Introduced
> >>>>
> >>>> ---
> >>>>   include/linux/lsm_hook_defs.h |  2 ++
> >>>>   include/linux/security.h      | 14 ++++++++++++++
> >>>>   security/security.c           | 28 ++++++++++++++++++++++++++++
> >>>>   3 files changed, 44 insertions(+)
> >>>
> >>> I'm not sure why you made this a separate patch, help?  If there is
> >>> no significant reason why this is separate, please squash it together
> >>> with patch 11/21.
> >>
> >> Off-topic: it is weird to have *RFC* patch set at v15.
> >>
> >> RFC by de-facto is something that can be safely ignored if you don't
> >> have bandwidth. 15 versions of anything that can be safely ignored
> >> is by definition spamming :-) I mean just conceptually.
> >>
> >> So does the RFC still hold or what the heck is going on with this one?
> >>
> >> Haven't followed for some time now...
> > 
> > I mean if this RFC trend continues I'll just put auto-filter for this
> > thread to put straight to the bin.  There's enough non-RFC patch sets
> > to review.
> > 
> > BR, Jarkko
>
> Sorry about the confusion with the RFC tag – I wasn't fully aware of its 
> conventional meaning and how it's perceived in terms of importance and 
> urgency. Point taken, and I'll make sure to remove the RFC tag for 
> future submissions. Definitely not my intention to clog up the workflow 
> or seem like I'm spamming.

OK cool! Just wanted to point this out also because it already looks
good enough not to be considered as RFC in my eyes :-) If you keep RFC
it is by definition "look into if you have the bandwidth but please
do not take this to mainline". No means to nitpick here...

BR, Jarkko
diff mbox series

Patch

diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
index c335404470dc..6808ae763913 100644
--- a/include/linux/lsm_hook_defs.h
+++ b/include/linux/lsm_hook_defs.h
@@ -455,4 +455,6 @@  LSM_HOOK(void, LSM_RET_VOID, initramfs_populated, void)
 
 LSM_HOOK(int, 0, bdev_alloc_security, struct block_device *bdev)
 LSM_HOOK(void, LSM_RET_VOID, bdev_free_security, struct block_device *bdev)
+LSM_HOOK(int, 0, bdev_setintegrity, struct block_device *bdev,
+	 enum lsm_intgr_type type, const void *value, size_t size)
 
diff --git a/include/linux/security.h b/include/linux/security.h
index 9965b5c50df4..eaff8868766a 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -83,6 +83,10 @@  enum lsm_event {
 	LSM_POLICY_CHANGE,
 };
 
+enum lsm_intgr_type {
+	__LSM_INTGR_MAX
+};
+
 /*
  * These are reasons that can be passed to the security_locked_down()
  * LSM hook. Lockdown reasons that protect kernel integrity (ie, the
@@ -511,6 +515,9 @@  int lsm_fill_user_ctx(struct lsm_ctx __user *uctx, u32 *uctx_len,
 		      void *val, size_t val_len, u64 id, u64 flags);
 int security_bdev_alloc(struct block_device *bdev);
 void security_bdev_free(struct block_device *bdev);
+int security_bdev_setintegrity(struct block_device *bdev,
+			       enum lsm_intgr_type type, const void *value,
+			       size_t size);
 #else /* CONFIG_SECURITY */
 
 static inline int call_blocking_lsm_notifier(enum lsm_event event, void *data)
@@ -1495,6 +1502,13 @@  static inline void security_bdev_free(struct block_device *bdev)
 {
 }
 
+static inline int security_bdev_setintegrity(struct block_device *bdev,
+					     enum lsm_intgr_type, type,
+					     const void *value, size_t size)
+{
+	return 0;
+}
+
 #endif	/* CONFIG_SECURITY */
 
 #if defined(CONFIG_SECURITY) && defined(CONFIG_WATCH_QUEUE)
diff --git a/security/security.c b/security/security.c
index 4274bbee40d0..8d88529ac904 100644
--- a/security/security.c
+++ b/security/security.c
@@ -5637,6 +5637,34 @@  void security_bdev_free(struct block_device *bdev)
 }
 EXPORT_SYMBOL(security_bdev_free);
 
+/**
+ * security_bdev_setintegrity() - Set the bdev's integrity data
+ * @bdev: block device
+ * @type: type of integrity, e.g. hash digest, signature, etc
+ * @value: the integrity value
+ * @size: size of the integrity value
+ *
+ * Register a verified integrity measurement of a bdev with the LSM.
+ *
+ * Return: Returns 0 on success, negative values on failure.
+ */
+int security_bdev_setintegrity(struct block_device *bdev,
+			       enum lsm_intgr_type type, const void *value,
+			       size_t size)
+{
+	int rc = 0;
+	struct security_hook_list *p;
+
+	hlist_for_each_entry(p, &security_hook_heads.bdev_setintegrity, list) {
+		rc = p->hook.bdev_setintegrity(bdev, type, value, size);
+		if (rc)
+			return rc;
+	}
+
+	return LSM_RET_DEFAULT(bdev_setintegrity);
+}
+EXPORT_SYMBOL(security_bdev_setintegrity);
+
 #ifdef CONFIG_PERF_EVENTS
 /**
  * security_perf_event_open() - Check if a perf event open is allowed