diff mbox series

[RFC,1/2] LSM: Allow dynamically appendable LSM modules.

Message ID cc8e16bb-5083-01da-4a77-d251a76dc8ff@I-love.SAKURA.ne.jp (mailing list archive)
State RFC
Headers show
Series [RFC,1/2] LSM: Allow dynamically appendable LSM modules. | expand

Checks

Context Check Description
netdev/tree_selection success Not a local patch

Commit Message

Tetsuo Handa Sept. 27, 2023, 3:08 p.m. UTC
Recently, the LSM community is trying to make drastic changes.

Crispin Cowan has explained

  It is Linus' comments that spurred me to want to start this undertaking.  He
  observes that there are many different security approaches, each with their own
  advocates.  He doesn't want to arbitrate which of them should be "the" Linux
  security approach, and would rather that Linux can support any of them.

  That is the purpose of this project:  to allow Linux to support a variety of
  security models, so that security developers don't have to have the "my dog's
  bigger than your dog" argument, and users can choose the security model that
  suits their needs.

when the LSM project started [1].

However, Casey Schaufler is trying to make users difficult to choose the
security model that suits their needs, by requiring LSM ID value which is
assigned to only LSM modules that succeeded to become in-tree [2].
Therefore, I'm asking Casey and Paul Moore to change their mind to allow
assigning LSM ID value to any LSM modules (so that users can choose the
security model that suits their needs) [3].

I expect that LSM ID value will be assigned to any publicly available LSM
modules. Otherwise, it is mostly pointless to propose this patch; there
will be little LSM modules to built into vmlinux; let alone dynamically
loading as LKM-based LSMs.

Also, KP Singh is trying to replace the linked list with static calls in
order to reduce overhead of indirect calls [4]. However, this change
assumed that any LSM modules are built-in. I don't like such assumption
because I still consider that LSM modules which are not built into vmlinux
will be wanted by users [5].

Then, Casey told me to supply my implementation of loadable security
modules [6]. Therefore, I post this patch as basic changes needed for
allowing dynamically appendable LSM modules (and an example of appendable
LSM modules). This patch was tested on only x86_64.

Question for KP Singh would be how can we allow dynamically appendable
LSM modules if current linked list is replaced with static calls with
minimal-sized array...

Link: https://marc.info/?l=linux-security-module&m=98706471912438&w=2 [1]
Link: https://lkml.kernel.org/r/20230912205658.3432-2-casey@schaufler-ca.com [2]
Link: https://lkml.kernel.org/r/6e1c25f5-b78c-8b4e-ddc3-484129c4c0ec@I-love.SAKURA.ne.jp [3]
Link: https://lkml.kernel.org/r/20230918212459.1937798-1-kpsingh@kernel.org [4]
Link: https://lkml.kernel.org/r/ed785c86-a1d8-caff-c629-f8a50549e05b@I-love.SAKURA.ne.jp [5]
Link: https://lkml.kernel.org/r/36c7cf74-508f-1690-f86a-bb18ec686fcf@schaufler-ca.com [6]
Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
---
 include/linux/lsm_hooks.h |   2 +
 security/security.c       | 107 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 109 insertions(+)

Comments

Greg KH Sept. 27, 2023, 3:33 p.m. UTC | #1
On Thu, Sep 28, 2023 at 12:08:47AM +0900, Tetsuo Handa wrote:
> +extern int register_loadable_lsm(struct security_hook_list *hooks, int count,
> +				 const char *lsm);

naming nit, this should be "noun_verb" where ever possible to make it
easier to handle global symbols.  So "lsm_register()" perhaps?

thanks,

greg k-h
KP Singh Sept. 27, 2023, 4:02 p.m. UTC | #2
On Wed, Sep 27, 2023 at 5:09 PM Tetsuo Handa
<penguin-kernel@i-love.sakura.ne.jp> wrote:
>
> Recently, the LSM community is trying to make drastic changes.
>
> Crispin Cowan has explained
>
>   It is Linus' comments that spurred me to want to start this undertaking.  He
>   observes that there are many different security approaches, each with their own
>   advocates.  He doesn't want to arbitrate which of them should be "the" Linux
>   security approach, and would rather that Linux can support any of them.
>
>   That is the purpose of this project:  to allow Linux to support a variety of
>   security models, so that security developers don't have to have the "my dog's
>   bigger than your dog" argument, and users can choose the security model that
>   suits their needs.
>
> when the LSM project started [1].
>
> However, Casey Schaufler is trying to make users difficult to choose the
> security model that suits their needs, by requiring LSM ID value which is
> assigned to only LSM modules that succeeded to become in-tree [2].
> Therefore, I'm asking Casey and Paul Moore to change their mind to allow
> assigning LSM ID value to any LSM modules (so that users can choose the
> security model that suits their needs) [3].
>
> I expect that LSM ID value will be assigned to any publicly available LSM
> modules. Otherwise, it is mostly pointless to propose this patch; there
> will be little LSM modules to built into vmlinux; let alone dynamically
> loading as LKM-based LSMs.
>
> Also, KP Singh is trying to replace the linked list with static calls in
> order to reduce overhead of indirect calls [4]. However, this change
> assumed that any LSM modules are built-in. I don't like such assumption
> because I still consider that LSM modules which are not built into vmlinux
> will be wanted by users [5].
>
> Then, Casey told me to supply my implementation of loadable security
> modules [6]. Therefore, I post this patch as basic changes needed for
> allowing dynamically appendable LSM modules (and an example of appendable
> LSM modules). This patch was tested on only x86_64.
>
> Question for KP Singh would be how can we allow dynamically appendable
> LSM modules if current linked list is replaced with static calls with
> minimal-sized array...

As I suggested in the other thread:

https://lore.kernel.org/bpf/20230918212459.1937798-1-kpsingh@kernel.org/T/#md21b9d9cc769f39e451d20364857b693d3fcb587

You can add extra static call slots and fallback to a linked list
based implementation if you have more than say N modules [1] and
fallback to a linked list implementation [2].

for [1] you can just do MAX_LSM_COUNT you can just do:

#ifdef CONFIG_MODULAR_LSM
#define MODULAR_LSM_ENABLED "1,1,1,1"
#endif

and use it in the LSM_COUNT.

for [2] you can choose to export a better module API than directly
exposing security_hook_heads.

Now, comes the question of whether we need dynamically loaded LSMs, I
am not in favor of this.Please share your limitations of BPF as you
mentioned and what's missing to implement dynamic LSMs. My question
still remains unanswered.

Until I hear the real limitations of using BPF, it's a NAK from me.

>
> Link: https://marc.info/?l=linux-security-module&m=98706471912438&w=2 [1]
> Link: https://lkml.kernel.org/r/20230912205658.3432-2-casey@schaufler-ca.com [2]
> Link: https://lkml.kernel.org/r/6e1c25f5-b78c-8b4e-ddc3-484129c4c0ec@I-love.SAKURA.ne.jp [3]
> Link: https://lkml.kernel.org/r/20230918212459.1937798-1-kpsingh@kernel.org [4]
> Link: https://lkml.kernel.org/r/ed785c86-a1d8-caff-c629-f8a50549e05b@I-love.SAKURA.ne.jp [5]
> Link: https://lkml.kernel.org/r/36c7cf74-508f-1690-f86a-bb18ec686fcf@schaufler-ca.com [6]
> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> ---
>  include/linux/lsm_hooks.h |   2 +
>  security/security.c       | 107 ++++++++++++++++++++++++++++++++++++++
>  2 files changed, 109 insertions(+)
>
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index dcb5e5b5eb13..73db3c41df26 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -105,6 +105,8 @@ extern char *lsm_names;
>
>  extern void security_add_hooks(struct security_hook_list *hooks, int count,
>                                 const char *lsm);
> +extern int register_loadable_lsm(struct security_hook_list *hooks, int count,
> +                                const char *lsm);
>
>  #define LSM_FLAG_LEGACY_MAJOR  BIT(0)
>  #define LSM_FLAG_EXCLUSIVE     BIT(1)
> diff --git a/security/security.c b/security/security.c
> index 23b129d482a7..6c64b7afb251 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -74,6 +74,7 @@ const char *const lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX + 1] = {
>  };
>
>  struct security_hook_heads security_hook_heads __ro_after_init;
> +EXPORT_SYMBOL_GPL(security_hook_heads);

Rather than exposting security_hook_heads, this should actually export
security_hook_module_register. This should internally handle any data
structures used and also not need the special magic that you did for
__ro_after_init.

- KP

>  static BLOCKING_NOTIFIER_HEAD(blocking_lsm_notifier_chain);
>
>  static struct kmem_cache *lsm_file_cache;
> @@ -537,6 +538,112 @@ void __init security_add_hooks(struct security_hook_list *hooks, int count,
>         }
>  }
>
> +#if defined(CONFIG_STRICT_KERNEL_RWX)
> +#define MAX_RO_PAGES 1024 /* Wild guess. Can be minimized by dynamic allocation. */
> +static struct page *ro_pages[MAX_RO_PAGES]; /* Pages that are marked read-only. */
> +static unsigned int ro_pages_len; /* Number of pages that are marked read-only. */
> +
> +/* Check whether a page containing given address does not have _PAGE_BIT_RW bit. */
> +static bool lsm_test_page_ro(void *addr)
> +{
> +       unsigned int i;
> +       int unused;
> +       struct page *page;
> +
> +       page = (struct page *) lookup_address((unsigned long) addr, &unused);
> +       if (!page)
> +               return false;
> +       if (test_bit(_PAGE_BIT_RW, &(page->flags)))
> +               return true;
> +       for (i = 0; i < ro_pages_len; i++)
> +               if (page == ro_pages[i])
> +                       return true;
> +       if (ro_pages_len == MAX_RO_PAGES)
> +               return false;
> +       ro_pages[ro_pages_len++] = page;
> +       return true;
> +}
> +
> +/* Find pages which do not have _PAGE_BIT_RW bit. */
> +static bool check_ro_pages(struct security_hook_list *hooks, int count)
> +{
> +       int i;
> +       struct hlist_head *list = &security_hook_heads.capable;
> +
> +       if (!copy_to_kernel_nofault(list, list, sizeof(void *)))
> +               return true;
> +       for (i = 0; i < count; i++) {
> +               struct hlist_head *head = hooks[i].head;
> +               struct security_hook_list *shp;
> +
> +               if (!lsm_test_page_ro(&head->first))
> +                       return false;
> +               hlist_for_each_entry(shp, head, list)
> +                       if (!lsm_test_page_ro(&shp->list.next) ||
> +                           !lsm_test_page_ro(&shp->list.pprev))
> +                               return false;
> +       }
> +       return true;
> +}
> +#endif
> +
> +/**
> + * register_loadable_lsm - Add a dynamically appendable module's hooks to the hook lists.
> + * @hooks: the hooks to add
> + * @count: the number of hooks to add
> + * @lsm: the name of the security module
> + *
> + * Each dynamically appendable LSM has to register its hooks with the infrastructure.
> + *
> + * Assumes that this function is called from module_init() function where
> + * call to this function is already serialized by module_mutex lock.
> + */
> +int register_loadable_lsm(struct security_hook_list *hooks, int count,
> +                         const char *lsm)
> +{
> +       int i;
> +       char *cp;
> +
> +       // TODO: Check whether proposed hooks can co-exist with already chained hooks,
> +       //       and bail out here if one of hooks cannot co-exist...
> +
> +#if defined(CONFIG_STRICT_KERNEL_RWX)
> +       // Find pages which needs to make temporarily writable.
> +       ro_pages_len = 0;
> +       if (!check_ro_pages(hooks, count)) {
> +               pr_err("Can't make security_hook_heads again writable. Retry with rodata=off kernel command line option added.\n");
> +               return -EINVAL;
> +       }
> +       pr_info("ro_pages_len=%d\n", ro_pages_len);
> +#endif
> +       // At least "capability" is already included.
> +       cp = kasprintf(GFP_KERNEL, "%s,%s", lsm_names, lsm);
> +       if (!cp) {
> +               pr_err("%s - Cannot get memory.\n", __func__);
> +               return -ENOMEM;
> +       }
> +#if defined(CONFIG_STRICT_KERNEL_RWX)
> +       // Make security_hook_heads (and hooks chained) temporarily writable.
> +       for (i = 0; i < ro_pages_len; i++)
> +               set_bit(_PAGE_BIT_RW, &(ro_pages[i]->flags));
> +#endif
> +       // Register dynamically appendable module's hooks.
> +       for (i = 0; i < count; i++) {
> +               hooks[i].lsm = lsm;
> +               hlist_add_tail_rcu(&hooks[i].list, hooks[i].head);
> +       }
> +#if defined(CONFIG_STRICT_KERNEL_RWX)
> +       // Make security_hook_heads (and hooks chained) again read-only.
> +       for (i = 0; i < ro_pages_len; i++)
> +               clear_bit(_PAGE_BIT_RW, &(ro_pages[i]->flags));
> +#endif
> +       // TODO: Wait for reader side before kfree().
> +       kfree(lsm_names);
> +       lsm_names = cp;
> +       return 0;
> +}
> +EXPORT_SYMBOL_GPL(register_loadable_lsm);
> +
>  int call_blocking_lsm_notifier(enum lsm_event event, void *data)
>  {
>         return blocking_notifier_call_chain(&blocking_lsm_notifier_chain,
> --
> 2.18.4
Casey Schaufler Sept. 27, 2023, 4:37 p.m. UTC | #3
On 9/27/2023 8:08 AM, Tetsuo Handa wrote:
> Recently, the LSM community is trying to make drastic changes.

I'd call them "significant" or "important" rather than "drastic".

> Crispin Cowan has explained
>
>   It is Linus' comments that spurred me to want to start this undertaking.  He
>   observes that there are many different security approaches, each with their own
>   advocates.  He doesn't want to arbitrate which of them should be "the" Linux
>   security approach, and would rather that Linux can support any of them.
>
>   That is the purpose of this project:  to allow Linux to support a variety of
>   security models, so that security developers don't have to have the "my dog's
>   bigger than your dog" argument, and users can choose the security model that
>   suits their needs.
>
> when the LSM project started [1].
>
> However, Casey Schaufler is trying to make users difficult to choose the
> security model that suits their needs, by requiring LSM ID value which is
> assigned to only LSM modules that succeeded to become in-tree [2].

This statement is demonstrably false, and I'm tired of hearing it.

> Therefore, I'm asking Casey and Paul Moore to change their mind to allow
> assigning LSM ID value to any LSM modules (so that users can choose the
> security model that suits their needs) [3].
>
> I expect that LSM ID value will be assigned to any publicly available LSM
> modules. Otherwise, it is mostly pointless to propose this patch; there
> will be little LSM modules to built into vmlinux; let alone dynamically
> loading as LKM-based LSMs.
>
> Also, KP Singh is trying to replace the linked list with static calls in
> order to reduce overhead of indirect calls [4]. However, this change
> assumed that any LSM modules are built-in. I don't like such assumption
> because I still consider that LSM modules which are not built into vmlinux
> will be wanted by users [5].
>
> Then, Casey told me to supply my implementation of loadable security
> modules [6]. Therefore, I post this patch as basic changes needed for
> allowing dynamically appendable LSM modules (and an example of appendable
> LSM modules). This patch was tested on only x86_64.

Thank you for doing so. I will be mostly off line for the next few weeks,
and will review the proposal fully on my return. I will provide some
initial feedback below.

> Question for KP Singh would be how can we allow dynamically appendable
> LSM modules if current linked list is replaced with static calls with
> minimal-sized array...
>
> Link: https://marc.info/?l=linux-security-module&m=98706471912438&w=2 [1]
> Link: https://lkml.kernel.org/r/20230912205658.3432-2-casey@schaufler-ca.com [2]
> Link: https://lkml.kernel.org/r/6e1c25f5-b78c-8b4e-ddc3-484129c4c0ec@I-love.SAKURA.ne.jp [3]
> Link: https://lkml.kernel.org/r/20230918212459.1937798-1-kpsingh@kernel.org [4]
> Link: https://lkml.kernel.org/r/ed785c86-a1d8-caff-c629-f8a50549e05b@I-love.SAKURA.ne.jp [5]
> Link: https://lkml.kernel.org/r/36c7cf74-508f-1690-f86a-bb18ec686fcf@schaufler-ca.com [6]
> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> ---
>  include/linux/lsm_hooks.h |   2 +
>  security/security.c       | 107 ++++++++++++++++++++++++++++++++++++++
>  2 files changed, 109 insertions(+)
>
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index dcb5e5b5eb13..73db3c41df26 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -105,6 +105,8 @@ extern char *lsm_names;
>  
>  extern void security_add_hooks(struct security_hook_list *hooks, int count,
>  				const char *lsm);
> +extern int register_loadable_lsm(struct security_hook_list *hooks, int count,
> +				 const char *lsm);
>  
>  #define LSM_FLAG_LEGACY_MAJOR	BIT(0)
>  #define LSM_FLAG_EXCLUSIVE	BIT(1)
> diff --git a/security/security.c b/security/security.c
> index 23b129d482a7..6c64b7afb251 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -74,6 +74,7 @@ const char *const lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX + 1] = {
>  };
>  
>  struct security_hook_heads security_hook_heads __ro_after_init;
> +EXPORT_SYMBOL_GPL(security_hook_heads);

Why disrupt the protection of security_hook_heads? You could easily add

struct security_hook_heads security_loadable_hook_heads
EXPORT_SYMBOL_GPL(security_loadable_hook_heads);

and add the loaded hooks there. A system that does not use loadable
modules would be unaffected by the ability to load modules.

>  static BLOCKING_NOTIFIER_HEAD(blocking_lsm_notifier_chain);
>  
>  static struct kmem_cache *lsm_file_cache;
> @@ -537,6 +538,112 @@ void __init security_add_hooks(struct security_hook_list *hooks, int count,
>  	}
>  }
>  
> +#if defined(CONFIG_STRICT_KERNEL_RWX)
> +#define MAX_RO_PAGES 1024 /* Wild guess. Can be minimized by dynamic allocation. */
> +static struct page *ro_pages[MAX_RO_PAGES]; /* Pages that are marked read-only. */
> +static unsigned int ro_pages_len; /* Number of pages that are marked read-only. */
> +
> +/* Check whether a page containing given address does not have _PAGE_BIT_RW bit. */
> +static bool lsm_test_page_ro(void *addr)
> +{
> +	unsigned int i;
> +	int unused;
> +	struct page *page;
> +
> +	page = (struct page *) lookup_address((unsigned long) addr, &unused);
> +	if (!page)
> +		return false;
> +	if (test_bit(_PAGE_BIT_RW, &(page->flags)))
> +		return true;
> +	for (i = 0; i < ro_pages_len; i++)
> +		if (page == ro_pages[i])
> +			return true;
> +	if (ro_pages_len == MAX_RO_PAGES)
> +		return false;
> +	ro_pages[ro_pages_len++] = page;
> +	return true;
> +}
> +
> +/* Find pages which do not have _PAGE_BIT_RW bit. */
> +static bool check_ro_pages(struct security_hook_list *hooks, int count)
> +{
> +	int i;
> +	struct hlist_head *list = &security_hook_heads.capable;
> +
> +	if (!copy_to_kernel_nofault(list, list, sizeof(void *)))
> +		return true;
> +	for (i = 0; i < count; i++) {
> +		struct hlist_head *head = hooks[i].head;
> +		struct security_hook_list *shp;
> +
> +		if (!lsm_test_page_ro(&head->first))
> +			return false;
> +		hlist_for_each_entry(shp, head, list)
> +			if (!lsm_test_page_ro(&shp->list.next) ||
> +			    !lsm_test_page_ro(&shp->list.pprev))
> +				return false;
> +	}
> +	return true;
> +}
> +#endif

I'm not an expert on modern memory management, but I think introducing
security_loadable_hook_heads would make these functions unnecessary.
Please educate me if I'm wrong.

> +
> +/**
> + * register_loadable_lsm - Add a dynamically appendable module's hooks to the hook lists.
> + * @hooks: the hooks to add
> + * @count: the number of hooks to add
> + * @lsm: the name of the security module
> + *
> + * Each dynamically appendable LSM has to register its hooks with the infrastructure.
> + *
> + * Assumes that this function is called from module_init() function where
> + * call to this function is already serialized by module_mutex lock.
> + */
> +int register_loadable_lsm(struct security_hook_list *hooks, int count,
> +			  const char *lsm)
> +{
> +	int i;
> +	char *cp;
> +
> +	// TODO: Check whether proposed hooks can co-exist with already chained hooks,
> +	//       and bail out here if one of hooks cannot co-exist...
> +
> +#if defined(CONFIG_STRICT_KERNEL_RWX)
> +	// Find pages which needs to make temporarily writable.
> +	ro_pages_len = 0;
> +	if (!check_ro_pages(hooks, count)) {
> +		pr_err("Can't make security_hook_heads again writable. Retry with rodata=off kernel command line option added.\n");
> +		return -EINVAL;
> +	}
> +	pr_info("ro_pages_len=%d\n", ro_pages_len);
> +#endif
> +	// At least "capability" is already included.
> +	cp = kasprintf(GFP_KERNEL, "%s,%s", lsm_names, lsm);
> +	if (!cp) {
> +		pr_err("%s - Cannot get memory.\n", __func__);
> +		return -ENOMEM;
> +	}
> +#if defined(CONFIG_STRICT_KERNEL_RWX)
> +	// Make security_hook_heads (and hooks chained) temporarily writable.
> +	for (i = 0; i < ro_pages_len; i++)
> +		set_bit(_PAGE_BIT_RW, &(ro_pages[i]->flags));
> +#endif
> +	// Register dynamically appendable module's hooks.
> +	for (i = 0; i < count; i++) {
> +		hooks[i].lsm = lsm;
> +		hlist_add_tail_rcu(&hooks[i].list, hooks[i].head);
> +	}
> +#if defined(CONFIG_STRICT_KERNEL_RWX)
> +	// Make security_hook_heads (and hooks chained) again read-only.
> +	for (i = 0; i < ro_pages_len; i++)
> +		clear_bit(_PAGE_BIT_RW, &(ro_pages[i]->flags));
> +#endif
> +	// TODO: Wait for reader side before kfree().
> +	kfree(lsm_names);
> +	lsm_names = cp;
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(register_loadable_lsm);

Most of this code seems unnecessary if you use security_loadable_hook_heads.

There would need to be additions in security.c to invoke the hooks in the new
list, but that would be straightforward. Locking is another matter. I don't see
that addressed here, and I fear that it might have prohibitive performance
impact. Again, I'm not an expert on locking, so you'll need to seek advise
elsewhere.

On a less happy note, you haven't addressed security blobs in any way. You
need to provide a mechanism to allow an LSM to share security blobs with
builtin LSMs and other loadable LSMs.

> +
>  int call_blocking_lsm_notifier(enum lsm_event event, void *data)
>  {
>  	return blocking_notifier_call_chain(&blocking_lsm_notifier_chain,
Tetsuo Handa Oct. 1, 2023, 11:08 a.m. UTC | #4
On 2023/09/28 1:02, KP Singh wrote:
>> Question for KP Singh would be how can we allow dynamically appendable
>> LSM modules if current linked list is replaced with static calls with
>> minimal-sized array...
> 
> As I suggested in the other thread:
> 
> https://lore.kernel.org/bpf/20230918212459.1937798-1-kpsingh@kernel.org/T/#md21b9d9cc769f39e451d20364857b693d3fcb587
> 
> You can add extra static call slots and fallback to a linked list
> based implementation if you have more than say N modules [1] and
> fallback to a linked list implementation [2].

As I explained in the other thread:

https://lkml.kernel.org/r/c1683052-aa5a-e0d5-25ae-40316273ed1b@I-love.SAKURA.ne.jp

build-time configuration does not help at all.

> 
> for [1] you can just do MAX_LSM_COUNT you can just do:
> 
> #ifdef CONFIG_MODULAR_LSM
> #define MODULAR_LSM_ENABLED "1,1,1,1"
> #endif
> 
> and use it in the LSM_COUNT.
> 
> for [2] you can choose to export a better module API than directly
> exposing security_hook_heads.
> 
> Now, comes the question of whether we need dynamically loaded LSMs, I
> am not in favor of this. Please share your limitations of BPF as you
> mentioned and what's missing to implement dynamic LSMs. My question
> still remains unanswered.
> 
> Until I hear the real limitations of using BPF, it's a NAK from me.

Simple questions that TOMOYO/AKARI/CaitSith LSMs depend:

  Q1: How can the BPF allow allocating permanent memory (e.g. kmalloc()) that remains
      the lifetime of the kernel (e.g. before starting the global init process till
      the content of RAM is lost by stopping electric power supply) ?

  Q2: How can the BPF allow interacting with other process (e.g. inter process communication
      using read()/write()) which involves opening some file on the filesystem and sleeping
      for arbitrary duration?



>>  struct security_hook_heads security_hook_heads __ro_after_init;
>> +EXPORT_SYMBOL_GPL(security_hook_heads);
> 
> Rather than exposting security_hook_heads, this should actually export
> security_hook_module_register. This should internally handle any data
> structures used and also not need the special magic that you did for
> __ro_after_init.

I'm fine if security_hook_module_register() (and related code) cannot be
disabled by the kernel configuration.
Tetsuo Handa Oct. 1, 2023, 11:31 a.m. UTC | #5
On 2023/09/28 1:37, Casey Schaufler wrote:
> On 9/27/2023 8:08 AM, Tetsuo Handa wrote:
>> Recently, the LSM community is trying to make drastic changes.
> 
> I'd call them "significant" or "important" rather than "drastic".
> 
>> Crispin Cowan has explained
>>
>>   It is Linus' comments that spurred me to want to start this undertaking.  He
>>   observes that there are many different security approaches, each with their own
>>   advocates.  He doesn't want to arbitrate which of them should be "the" Linux
>>   security approach, and would rather that Linux can support any of them.
>>
>>   That is the purpose of this project:  to allow Linux to support a variety of
>>   security models, so that security developers don't have to have the "my dog's
>>   bigger than your dog" argument, and users can choose the security model that
>>   suits their needs.
>>
>> when the LSM project started [1].
>>
>> However, Casey Schaufler is trying to make users difficult to choose the
>> security model that suits their needs, by requiring LSM ID value which is
>> assigned to only LSM modules that succeeded to become in-tree [2].
> 
> This statement is demonstrably false, and I'm tired of hearing it.

This statement is absolutely true.

Kees Cook said there is no problem if the policy of assigning LSM ID value were

  1) author: "Hello, here is a new LSM I'd like to upstream, here it is. I assigned
              it the next LSM ID."
     maintainer(s): "Okay, sounds good. *review*"

  2) author: "Hello, here is an LSM that has been in active use at $Place,
              and we have $Xxx many userspace applications that we cannot easily
              rebuild. We used LSM ID $Value that is far away from the sequential
              list of LSM IDs, and we'd really prefer to keep that assignment."
    maintainer(s): "Okay, sounds good. *review*"

and I agreed at https://lkml.kernel.org/r/6e1c25f5-b78c-8b4e-ddc3-484129c4c0ec@I-love.SAKURA.ne.jp .

But Paul Moore's response was

  No LSM ID value is guaranteed until it is present in a tagged release
  from Linus' tree, and once a LSM ID is present in a tagged release
  from Linus' tree it should not change.  That's *the* policy.

which means that the policy is not what Kees Cook has said.


>>  struct security_hook_heads security_hook_heads __ro_after_init;
>> +EXPORT_SYMBOL_GPL(security_hook_heads);
> 
> Why disrupt the protection of security_hook_heads? You could easily add
> 
> struct security_hook_heads security_loadable_hook_heads
> EXPORT_SYMBOL_GPL(security_loadable_hook_heads);
> 
> and add the loaded hooks there. A system that does not use loadable
> modules would be unaffected by the ability to load modules.


I'm fine if security_loadable_hook_heads() (and related code) cannot be
disabled by the kernel configuration.

Pasting https://lkml.org/lkml/2007/10/1/192 here again.

  On Mon, 1 Oct 2007, James Morris wrote:
  > 
  > Merging Smack, however, would lock the kernel into the LSM API.  
  > Presently, as SELinux is the only in-tree user, LSM can still be removed.
  
  Hell f*cking NO!
  
  You security people are insane. I'm tired of this "only my version is 
  correct" crap. The whole and only point of LSM was to get away from that.
  
  And anybody who claims that there is "consensus" on SELinux is just in 
  denial.
  
  People are arguing against other peoples security on totally bogus points. 
  First it was AppArmor, now this.
  
  I guess I have to merge AppArmor and SMACK just to get this *disease* off 
  the table. You're acting like a string theorist, claiming that t here is 
  no other viable theory out there. Stop it. It's been going on for too damn 
  long.
  
  			Linus

The situation with LKM-based LSMs is symmetry of that post.
Those who are suspicious about supporting LKM-based LSMs is nothing but

  "Presently, as all in-tree users are built-in, LSM does not need to support LKM-based LSMs."

. That's "only LSM modules which are built into vmlinux are correct" crap.

> On a less happy note, you haven't addressed security blobs in any way. You
> need to provide a mechanism to allow an LSM to share security blobs with
> builtin LSMs and other loadable LSMs.

Not all LKM-based LSMs need to use security blobs. What the LSM infrastructure
needs to do is manage which callback is called (so that undo operation is possible
when something went wrong while traversing the linked list). Everything else can
be managed by individual LSM implementations.
KP Singh Oct. 1, 2023, 2:43 p.m. UTC | #6
On Sun, Oct 1, 2023 at 1:08 PM Tetsuo Handa
<penguin-kernel@i-love.sakura.ne.jp> wrote:
>
> On 2023/09/28 1:02, KP Singh wrote:
> >> Question for KP Singh would be how can we allow dynamically appendable
> >> LSM modules if current linked list is replaced with static calls with
> >> minimal-sized array...
> >
> > As I suggested in the other thread:
> >
> > https://lore.kernel.org/bpf/20230918212459.1937798-1-kpsingh@kernel.org/T/#md21b9d9cc769f39e451d20364857b693d3fcb587
> >
> > You can add extra static call slots and fallback to a linked list
> > based implementation if you have more than say N modules [1] and
> > fallback to a linked list implementation [2].
>
> As I explained in the other thread:
>
> https://lkml.kernel.org/r/c1683052-aa5a-e0d5-25ae-40316273ed1b@I-love.SAKURA.ne.jp
>
> build-time configuration does not help at all.
>
> >
> > for [1] you can just do MAX_LSM_COUNT you can just do:
> >
> > #ifdef CONFIG_MODULAR_LSM
> > #define MODULAR_LSM_ENABLED "1,1,1,1"
> > #endif
> >
> > and use it in the LSM_COUNT.
> >
> > for [2] you can choose to export a better module API than directly
> > exposing security_hook_heads.
> >
> > Now, comes the question of whether we need dynamically loaded LSMs, I
> > am not in favor of this. Please share your limitations of BPF as you
> > mentioned and what's missing to implement dynamic LSMs. My question
> > still remains unanswered.
> >
> > Until I hear the real limitations of using BPF, it's a NAK from me.
>
> Simple questions that TOMOYO/AKARI/CaitSith LSMs depend:
>
>   Q1: How can the BPF allow allocating permanent memory (e.g. kmalloc()) that remains
>       the lifetime of the kernel (e.g. before starting the global init process till
>       the content of RAM is lost by stopping electric power supply) ?

This is very much possible using global BPF maps. Maps can be "pinned"
so that they remain allocated until explicitly freed [or RAM is lost
by stopping electric power supply"]

Here's an example of BPF program that allocates maps:

    https://elixir.bootlin.com/linux/latest/source/tools/testing/selftests/bpf/progs/test_pinning.c#L26

and the corresponding userspace code that does the pinning:

    https://elixir.bootlin.com/linux/latest/source/tools/testing/selftests/bpf/prog_tests/pinning.c

Specifically for LSMs, we also added support for security blobs which
are tied to a particular object and are free with the object, have a
look at the storage which is allocated in the program:

   https://elixir.bootlin.com/linux/latest/source/tools/testing/selftests/bpf/progs/local_storage.c#L79

Again, code and context on what you want to do will let me help you more here.

>
>   Q2: How can the BPF allow interacting with other process (e.g. inter process communication
>       using read()/write()) which involves opening some file on the filesystem and sleeping
>       for arbitrary duration?

The BPF program runs in the kernel context, so yes all of this is
possible. IPC can be done with the bpf_ring_buffer / maps and BPF also
has the ability to send signals. One can poll on the ring buffer on
events and data from the BPF program and do a lots of things.

* e.g. receive and log command line parameters (e.g. from the security
hook bprm_committed_creds).
* Trigger various actions in user space.

Can you share your module code here, so that one can provide more
concrete suggestions?

- KP

>
>
>
> >>  struct security_hook_heads security_hook_heads __ro_after_init;
> >> +EXPORT_SYMBOL_GPL(security_hook_heads);
> >
> > Rather than exposting security_hook_heads, this should actually export
> > security_hook_module_register. This should internally handle any data
> > structures used and also not need the special magic that you did for
> > __ro_after_init.
>
> I'm fine if security_hook_module_register() (and related code) cannot be
> disabled by the kernel configuration.
>
Casey Schaufler Oct. 1, 2023, 3:19 p.m. UTC | #7
On 10/1/2023 4:31 AM, Tetsuo Handa wrote:
> On 2023/09/28 1:37, Casey Schaufler wrote:
>> On 9/27/2023 8:08 AM, Tetsuo Handa wrote:
>>> Recently, the LSM community is trying to make drastic changes.
>> I'd call them "significant" or "important" rather than "drastic".
>>
>>> Crispin Cowan has explained
>>>
>>>   It is Linus' comments that spurred me to want to start this undertaking.  He
>>>   observes that there are many different security approaches, each with their own
>>>   advocates.  He doesn't want to arbitrate which of them should be "the" Linux
>>>   security approach, and would rather that Linux can support any of them.
>>>
>>>   That is the purpose of this project:  to allow Linux to support a variety of
>>>   security models, so that security developers don't have to have the "my dog's
>>>   bigger than your dog" argument, and users can choose the security model that
>>>   suits their needs.
>>>
>>> when the LSM project started [1].
>>>
>>> However, Casey Schaufler is trying to make users difficult to choose the
>>> security model that suits their needs, by requiring LSM ID value which is
>>> assigned to only LSM modules that succeeded to become in-tree [2].
>> This statement is demonstrably false, and I'm tired of hearing it.
> This statement is absolutely true.
>
> Kees Cook said there is no problem if the policy of assigning LSM ID value were
>
>   1) author: "Hello, here is a new LSM I'd like to upstream, here it is. I assigned
>               it the next LSM ID."
>      maintainer(s): "Okay, sounds good. *review*"
>
>   2) author: "Hello, here is an LSM that has been in active use at $Place,
>               and we have $Xxx many userspace applications that we cannot easily
>               rebuild. We used LSM ID $Value that is far away from the sequential
>               list of LSM IDs, and we'd really prefer to keep that assignment."
>     maintainer(s): "Okay, sounds good. *review*"
>
> and I agreed at https://lkml.kernel.org/r/6e1c25f5-b78c-8b4e-ddc3-484129c4c0ec@I-love.SAKURA.ne.jp .
>
> But Paul Moore's response was
>
>   No LSM ID value is guaranteed until it is present in a tagged release
>   from Linus' tree, and once a LSM ID is present in a tagged release
>   from Linus' tree it should not change.  That's *the* policy.
>
> which means that the policy is not what Kees Cook has said.
>
>
>>>  struct security_hook_heads security_hook_heads __ro_after_init;
>>> +EXPORT_SYMBOL_GPL(security_hook_heads);
>> Why disrupt the protection of security_hook_heads? You could easily add
>>
>> struct security_hook_heads security_loadable_hook_heads
>> EXPORT_SYMBOL_GPL(security_loadable_hook_heads);
>>
>> and add the loaded hooks there. A system that does not use loadable
>> modules would be unaffected by the ability to load modules.
>
> I'm fine if security_loadable_hook_heads() (and related code) cannot be
> disabled by the kernel configuration.

CONFIG_SECURITY ensures that you will be unhappy.
Even setting that aside, it's the developer's job to sell the code to
the communities involved. I could rant at certain distros for not including
Smack, but until such time as I've made doing that attractive it really
doesn't make any sense to do so. You don't think I've spent years on stacking
because I want to run Android containers on Ubuntu, do you?

>
> Pasting https://lkml.org/lkml/2007/10/1/192 here again.
>
>   On Mon, 1 Oct 2007, James Morris wrote:
>   > 
>   > Merging Smack, however, would lock the kernel into the LSM API.  
>   > Presently, as SELinux is the only in-tree user, LSM can still be removed.
>   
>   Hell f*cking NO!
>   
>   You security people are insane. I'm tired of this "only my version is 
>   correct" crap. The whole and only point of LSM was to get away from that.
>   
>   And anybody who claims that there is "consensus" on SELinux is just in 
>   denial.
>   
>   People are arguing against other peoples security on totally bogus points. 
>   First it was AppArmor, now this.
>   
>   I guess I have to merge AppArmor and SMACK just to get this *disease* off 
>   the table. You're acting like a string theorist, claiming that t here is 
>   no other viable theory out there. Stop it. It's been going on for too damn 
>   long.
>   
>   			Linus
>
> The situation with LKM-based LSMs is symmetry of that post.
> Those who are suspicious about supporting LKM-based LSMs is nothing but
>
>   "Presently, as all in-tree users are built-in, LSM does not need to support LKM-based LSMs."
>
> . That's "only LSM modules which are built into vmlinux are correct" crap.
>
>> On a less happy note, you haven't addressed security blobs in any way. You
>> need to provide a mechanism to allow an LSM to share security blobs with
>> builtin LSMs and other loadable LSMs.
> Not all LKM-based LSMs need to use security blobs.

If you only want to support "minor" LSMs, those that don't use shared blobs,
the loadable list implementation will suit you just fine. And because you won't
be using any of the LSM infrastructure that needs the LSM ID, that won't be
an issue.

You can make something that will work. Whether you can sell it upstream will
depend on any number of factors. But working code is always a great start.

>  What the LSM infrastructure
> needs to do is manage which callback is called (so that undo operation is possible
> when something went wrong while traversing the linked list). Everything else can
> be managed by individual LSM implementations.
>
Kees Cook Oct. 1, 2023, 3:44 p.m. UTC | #8
On October 1, 2023 4:31:05 AM PDT, Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> wrote:
>Kees Cook said there is no problem if the policy of assigning LSM ID value were
>
>  1) author: "Hello, here is a new LSM I'd like to upstream, here it is. I assigned
>              it the next LSM ID."
>     maintainer(s): "Okay, sounds good. *review*"
>
>  2) author: "Hello, here is an LSM that has been in active use at $Place,
>              and we have $Xxx many userspace applications that we cannot easily
>              rebuild. We used LSM ID $Value that is far away from the sequential
>              list of LSM IDs, and we'd really prefer to keep that assignment."
>    maintainer(s): "Okay, sounds good. *review*"
>
>and I agreed at https://lkml.kernel.org/r/6e1c25f5-b78c-8b4e-ddc3-484129c4c0ec@I-love.SAKURA.ne.jp .
>
>But Paul Moore's response was
>
>  No LSM ID value is guaranteed until it is present in a tagged release
>  from Linus' tree, and once a LSM ID is present in a tagged release
>  from Linus' tree it should not change.  That's *the* policy.
>
>which means that the policy is not what Kees Cook has said.

These don't conflict at all! Paul is saying an ID isn't guaranteed in upstream until it's in upstream. I'm saying the id space is large enough that you could make a new out of tree LSM every second for the next billion years. The upstream assignment process is likely sequential, but that out of sequence LSMs that show a need to be upstream could make a case for their existing value.

But again, I've already demonstrated how there is nothing technical blocking out of tree LSMs. If you want a declarative statement that some theoretical code will land upstream, you will not get it. And that's just normal FLOSS development: any number of technical, social, or political things may cause code to go unaccepted.

-Kees
Tetsuo Handa Oct. 2, 2023, 10:04 a.m. UTC | #9
On 2023/10/02 0:44, Kees Cook wrote:
> On October 1, 2023 4:31:05 AM PDT, Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> wrote:
>> Kees Cook said there is no problem if the policy of assigning LSM ID value were
>>
>>  1) author: "Hello, here is a new LSM I'd like to upstream, here it is. I assigned
>>              it the next LSM ID."
>>     maintainer(s): "Okay, sounds good. *review*"
>>
>>  2) author: "Hello, here is an LSM that has been in active use at $Place,
>>              and we have $Xxx many userspace applications that we cannot easily
>>              rebuild. We used LSM ID $Value that is far away from the sequential
>>              list of LSM IDs, and we'd really prefer to keep that assignment."
>>    maintainer(s): "Okay, sounds good. *review*"
>>
>> and I agreed at https://lkml.kernel.org/r/6e1c25f5-b78c-8b4e-ddc3-484129c4c0ec@I-love.SAKURA.ne.jp .
>>
>> But Paul Moore's response was
>>
>>  No LSM ID value is guaranteed until it is present in a tagged release
>>  from Linus' tree, and once a LSM ID is present in a tagged release
>>  from Linus' tree it should not change.  That's *the* policy.
>>
>> which means that the policy is not what Kees Cook has said.
> 
> These don't conflict at all! Paul is saying an ID isn't guaranteed in upstream
> until it's in upstream. I'm saying the id space is large enough that you could
> make a new out of tree LSM every second for the next billion years. The upstream
> assignment process is likely sequential, but that out of sequence LSMs that show
> a need to be upstream could make a case for their existing value.

Excuse me? If the LSM community wants the assignment sequential, the LSM community
cannot admit the LSM value assigned to a not-yet-in-tree LSM.

If "Okay, sounds good." does not imply that the LSM community admits the LSM value
assigned to a not-yet-in-tree LSM, what did "Okay, sounds good." mean?
Kees Cook Oct. 2, 2023, 5:05 p.m. UTC | #10
On Mon, Oct 02, 2023 at 07:04:27PM +0900, Tetsuo Handa wrote:
> On 2023/10/02 0:44, Kees Cook wrote:
> > On October 1, 2023 4:31:05 AM PDT, Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> wrote:
> >> Kees Cook said there is no problem if the policy of assigning LSM ID value were
> >>
> >>  1) author: "Hello, here is a new LSM I'd like to upstream, here it is. I assigned
> >>              it the next LSM ID."
> >>     maintainer(s): "Okay, sounds good. *review*"
> >>
> >>  2) author: "Hello, here is an LSM that has been in active use at $Place,
> >>              and we have $Xxx many userspace applications that we cannot easily
> >>              rebuild. We used LSM ID $Value that is far away from the sequential
> >>              list of LSM IDs, and we'd really prefer to keep that assignment."
> >>    maintainer(s): "Okay, sounds good. *review*"
> >>
> >> and I agreed at https://lkml.kernel.org/r/6e1c25f5-b78c-8b4e-ddc3-484129c4c0ec@I-love.SAKURA.ne.jp .
> >>
> >> But Paul Moore's response was
> >>
> >>  No LSM ID value is guaranteed until it is present in a tagged release
> >>  from Linus' tree, and once a LSM ID is present in a tagged release
> >>  from Linus' tree it should not change.  That's *the* policy.
> >>
> >> which means that the policy is not what Kees Cook has said.
> > 
> > These don't conflict at all! Paul is saying an ID isn't guaranteed in upstream
> > until it's in upstream. I'm saying the id space is large enough that you could
> > make a new out of tree LSM every second for the next billion years. The upstream
> > assignment process is likely sequential, but that out of sequence LSMs that show
> > a need to be upstream could make a case for their existing value.
> 
> Excuse me? If the LSM community wants the assignment sequential, the LSM community
> cannot admit the LSM value assigned to a not-yet-in-tree LSM.
> 
> If "Okay, sounds good." does not imply that the LSM community admits the LSM value
> assigned to a not-yet-in-tree LSM, what did "Okay, sounds good." mean?

I'm saying that if someone participates with upstream correctly, they'll
get a sequential ID since that is the expected process.

And if an LSM is out of tree for years and years in some large ecosystem
that has deeply hard-coded the LSM ID but now wants the LSM to land
upstream, then it's likely that an out-of-sequence ID would be accepted.

My point is that there is nothing technical stopping an out-of-tree LSM
from existing, and that the political issues for bringing a large
out-of-tree LSM upstream are going to have plenty of other negotiations
around maintaining operational behavior, of which and LSM ID is unlikely
to be a sticking point. Every release that some code (LSM or not) is out
of tree makes it that much harder to land upstream. (In other words, the
challenges to upstreaming a long-time-out-of-tree codebase are much
larger than dealing with an out-of-sequence LSM ID.)

-Kees
Tetsuo Handa Oct. 3, 2023, 2:27 p.m. UTC | #11
On 2023/10/01 23:43, KP Singh wrote:
>>> Now, comes the question of whether we need dynamically loaded LSMs, I
>>> am not in favor of this. Please share your limitations of BPF as you
>>> mentioned and what's missing to implement dynamic LSMs. My question
>>> still remains unanswered.
>>>
>>> Until I hear the real limitations of using BPF, it's a NAK from me.
>>
>> Simple questions that TOMOYO/AKARI/CaitSith LSMs depend:
>>
>>   Q1: How can the BPF allow allocating permanent memory (e.g. kmalloc()) that remains
>>       the lifetime of the kernel (e.g. before starting the global init process till
>>       the content of RAM is lost by stopping electric power supply) ?
> 
> This is very much possible using global BPF maps. Maps can be "pinned"
> so that they remain allocated until explicitly freed [or RAM is lost
> by stopping electric power supply"]
> 
> Here's an example of BPF program that allocates maps:
> 
>     https://elixir.bootlin.com/linux/latest/source/tools/testing/selftests/bpf/progs/test_pinning.c#L26
> 
> and the corresponding userspace code that does the pinning:
> 
>     https://elixir.bootlin.com/linux/latest/source/tools/testing/selftests/bpf/prog_tests/pinning.c

I know nothing about BPF. But that looks "allocate once" (i.e. almost "static char buf[SIZE]").
What I expected is "allocate memory where amount is determined at runtime" (e.g. alloc(), realloc()).

> 
> Specifically for LSMs, we also added support for security blobs which
> are tied to a particular object and are free with the object, have a
> look at the storage which is allocated in the program:
> 
>    https://elixir.bootlin.com/linux/latest/source/tools/testing/selftests/bpf/progs/local_storage.c#L79
> 
> Again, code and context on what you want to do will let me help you more here.

I don't have any BPF code.
I have several LKM-based LSMs in https://osdn.net/projects/akari/scm/svn/tree/head/branches/ .

> 
>>
>>   Q2: How can the BPF allow interacting with other process (e.g. inter process communication
>>       using read()/write()) which involves opening some file on the filesystem and sleeping
>>       for arbitrary duration?
> 
> The BPF program runs in the kernel context, so yes all of this is
> possible. IPC can be done with the bpf_ring_buffer / maps and BPF also
> has the ability to send signals. One can poll on the ring buffer on
> events and data from the BPF program and do a lots of things.

OK, BPF allows sleeping operations; that's good.

Some of core requirements for implementing TOMOYO/AKARI/CaitSith-like programs
using BPF will be:

  The program registered cannot be stopped/removed by the root user.
  This is made possible by either building the program into vmlinux or loading
  the program as a LKM without module_exit() callback. Is it possible to guaranee
  that a BPF program cannot be stopped/removed by user's operations?

  The program registered cannot be terminated by safety mechanisms (e.g. excessive
  CPU time consumption). Are there mechanisms in BPF that wouldn't have terminated
  a program if the program were implemented as a LKM rather than a BPF program?

  Ideally, the BPF program is built into vmlinux and is started before the global init
  process starts. (But whether building into vmlinux is possible does not matter here
  because I have trouble building into vmlinux. As a fallback, when we can start matters.)
  When is the earliest timing for starting a BPF program that must remain till stopping
  electric power supply? Is that when /init in a initramfs starts? Is that when init=
  kernel command line option is processed? More later than when init= is processed?

  Amount of memory needed for managing data is not known at compile time. Thus, I need
  kmalloc()-like memory allocation mechanism rather than allocating from some pool, and
  manage chunk of memory regions using linked list. Does BPF have kmalloc()-like memory
  allocation mechanism that allows allocating up to 32KB (8 pages if PAGE_SIZE=4096).

And maybe somewhere documented question:

  What kernel functions can a BPF program call / what kernel data can a BPF program access?
  The tools/testing/selftests/bpf/progs/test_d_path.c suggests that a BPF program can call
  d_path() defined in fs/d_path.c . But is that because d_path() is marked as EXPORT_SYMBOL() ?
  Or can a BPF program call almost all functions (like SystemTap script can insert hooks into
  almost all functions)? Even functions / data in LKM can be accessed by a BPF program?



On 2023/10/02 22:04, KP Singh wrote:
>>> There are still a bunch of details (e.g. shared blobs) that it doesn't
>>> address. On the other hand, your memory management magic doesn't
>>> address those issues either.
>>
>> Security is always trial-and-error. Just give all Linux users chances to continue
>> trial-and-error. You don't need to forbid LKM-based LSMs just because blob management
>> is not addressed. Please open the LSM infrastructure to anyone.
> 
> It already is, the community is already using BPF LSM.
> 
> e.g. https://github.com/linux-lock/bpflock
> 

Thank you for an example. But the project says

  bpflock is not a mandatory access control labeling solution, and it does not
  intent to replace AppArmor, SELinux, and other MAC solutions. bpflock uses a
  simple declarative security profile.

which is different from what I want to know (whether it is realistic to
implement TOMOYO/AKARI/CaitSith-like programs using BPF).
KP Singh Oct. 3, 2023, 3:09 p.m. UTC | #12
On Tue, Oct 3, 2023 at 4:28 PM Tetsuo Handa
<penguin-kernel@i-love.sakura.ne.jp> wrote:
>
> On 2023/10/01 23:43, KP Singh wrote:
> >>> Now, comes the question of whether we need dynamically loaded LSMs, I
> >>> am not in favor of this. Please share your limitations of BPF as you
> >>> mentioned and what's missing to implement dynamic LSMs. My question
> >>> still remains unanswered.
> >>>
> >>> Until I hear the real limitations of using BPF, it's a NAK from me.
> >>
> >> Simple questions that TOMOYO/AKARI/CaitSith LSMs depend:
> >>
> >>   Q1: How can the BPF allow allocating permanent memory (e.g. kmalloc()) that remains
> >>       the lifetime of the kernel (e.g. before starting the global init process till
> >>       the content of RAM is lost by stopping electric power supply) ?
> >
> > This is very much possible using global BPF maps. Maps can be "pinned"
> > so that they remain allocated until explicitly freed [or RAM is lost
> > by stopping electric power supply"]
> >
> > Here's an example of BPF program that allocates maps:
> >
> >     https://elixir.bootlin.com/linux/latest/source/tools/testing/selftests/bpf/progs/test_pinning.c#L26
> >
> > and the corresponding userspace code that does the pinning:
> >
> >     https://elixir.bootlin.com/linux/latest/source/tools/testing/selftests/bpf/prog_tests/pinning.c
>
> I know nothing about BPF. But that looks "allocate once" (i.e. almost "static char buf[SIZE]").

Happy to help you here!

> What I expected is "allocate memory where amount is determined at runtime" (e.g. alloc(), realloc()).

One can use dynamically sized allocations on the ring buffer with
dynamic pointers:

http://vger.kernel.org/bpfconf2022_material/lsfmmbpf2022-dynptr.pdf

Furthermore, there are some use cases that seemingly need dynamic
memory allocation but not really. e.g. there was a need to audit
command line arguments and while it seems dynamic and one can chunk
the allocation to finite sizes, put these on a ring buffer and process
the chunks.

It would be nice to see more details of where the dynamic allocation
is needed. Security blobs are allocated dynamically but have a fixed
size.

>
> >
> > Specifically for LSMs, we also added support for security blobs which
> > are tied to a particular object and are free with the object, have a
> > look at the storage which is allocated in the program:
> >
> >    https://elixir.bootlin.com/linux/latest/source/tools/testing/selftests/bpf/progs/local_storage.c#L79
> >
> > Again, code and context on what you want to do will let me help you more here.
>
> I don't have any BPF code.
> I have several LKM-based LSMs in https://osdn.net/projects/akari/scm/svn/tree/head/branches/ .

Thanks for the pointers, I will read through them.

>
> >
> >>
> >>   Q2: How can the BPF allow interacting with other process (e.g. inter process communication
> >>       using read()/write()) which involves opening some file on the filesystem and sleeping
> >>       for arbitrary duration?
> >
> > The BPF program runs in the kernel context, so yes all of this is
> > possible. IPC can be done with the bpf_ring_buffer / maps and BPF also
> > has the ability to send signals. One can poll on the ring buffer on
> > events and data from the BPF program and do a lots of things.
>
> OK, BPF allows sleeping operations; that's good.
>
> Some of core requirements for implementing TOMOYO/AKARI/CaitSith-like programs
> using BPF will be:
>
>   The program registered cannot be stopped/removed by the root user.
>   This is made possible by either building the program into vmlinux or loading
>   the program as a LKM without module_exit() callback. Is it possible to guaranee
>   that a BPF program cannot be stopped/removed by user's operations?

Yes, there is a security_bpf hook where a BPF MAC policy can be
implemented and other LSMs do that already.

>
>   The program registered cannot be terminated by safety mechanisms (e.g. excessive
>   CPU time consumption). Are there mechanisms in BPF that wouldn't have terminated
>   a program if the program were implemented as a LKM rather than a BPF program?
>

The kernel does not terminate BPF LSM programs, once a BPF program is
loaded and attached to the LSM hook, it's JITed into a native code.
From there onwards, as far as the kernel is concerned it's just like
any other kernel function.

>   Ideally, the BPF program is built into vmlinux and is started before the global init
>   process starts. (But whether building into vmlinux is possible does not matter here
>   because I have trouble building into vmlinux. As a fallback, when we can start matters.)
>   When is the earliest timing for starting a BPF program that must remain till stopping

The kernel actually supports preloading certain BPF programs during early init.

https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git/commit/?id=0bc23a1d1c8a1b4a5e4b973a7a80a6d067bd3eef

This allows you to preload before init.


>   electric power supply? Is that when /init in a initramfs starts? Is that when init=
>   kernel command line option is processed? More later than when init= is processed?

Also, It depends on whether you trust init or not (e.g. if the init
blob is somehow appraised and measured, then you can trust it to load
the right BPF LSM programs). and then you can choose to not preload
bpf programs in the kernel, rather load them sometime early in /init.

>
>   Amount of memory needed for managing data is not known at compile time. Thus, I need
>   kmalloc()-like memory allocation mechanism rather than allocating from some pool, and
>   manage chunk of memory regions using linked list. Does BPF have kmalloc()-like memory
>   allocation mechanism that allows allocating up to 32KB (8 pages if PAGE_SIZE=4096).
>

You use the ring buffer as a large pool and use dynamic pointers to
carve chunks out of it, if truly dynamic memory is needed.

> And maybe somewhere documented question:
>
>   What kernel functions can a BPF program call / what kernel data can a BPF program access?

BPF programs can access kernel data dynamically (accesses relocated at
load time without needing a recompile) There are lot of good details
in:

https://nakryiko.com/posts/bpf-core-reference-guide/


>   The tools/testing/selftests/bpf/progs/test_d_path.c suggests that a BPF program can call
>   d_path() defined in fs/d_path.c . But is that because d_path() is marked as EXPORT_SYMBOL() ?
>   Or can a BPF program call almost all functions (like SystemTap script can insert hooks into
>   almost all functions)? Even functions / data in LKM can be accessed by a BPF program?
>

It's not all kernel functions, but there is a wide range of helpers
and kfuncs (examples in tools/testing/selftests/bpf) and if there is
something missing, we will help you.

>
>
> On 2023/10/02 22:04, KP Singh wrote:
> >>> There are still a bunch of details (e.g. shared blobs) that it doesn't
> >>> address. On the other hand, your memory management magic doesn't
> >>> address those issues either.
> >>
> >> Security is always trial-and-error. Just give all Linux users chances to continue
> >> trial-and-error. You don't need to forbid LKM-based LSMs just because blob management
> >> is not addressed. Please open the LSM infrastructure to anyone.
> >
> > It already is, the community is already using BPF LSM.
> >
> > e.g. https://github.com/linux-lock/bpflock
> >
>
> Thank you for an example. But the project says
>
>   bpflock is not a mandatory access control labeling solution, and it does not
>   intent to replace AppArmor, SELinux, and other MAC solutions. bpflock uses a
>   simple declarative security profile.
>
> which is different from what I want to know (whether it is realistic to
> implement TOMOYO/AKARI/CaitSith-like programs using BPF).

Agreed, I was sharing it more as a code sample. There is an
interesting talk by Meta at LPC which I quite excited about in this
space:

https://lpc.events/event/17/contributions/1602/

These are just examples of flexible MAC implementations using BPF.

- KP

- KP
>
Paul Moore Oct. 3, 2023, 11:27 p.m. UTC | #13
On Wed, Sep 27, 2023 at 12:02 PM KP Singh <kpsingh@kernel.org> wrote:
>
> Until I hear the real limitations of using BPF, it's a NAK from me.

There is a lot going on in this thread, and while I'm still playing
catch-up from LSS-EU and some time off (ish) it looks like most of the
most important points have already been made, which is great.
However, I did want to comment quickly on the statement above.

We want to be very careful about using an existing upstream LSM as a
reason for blocking the inclusion of a new LSM upstream.  We obviously
want to reject obvious duplicates and proposals that are sufficiently
"close" (with "close" deliberately left ambiguous here), but we don't
want to stifle new ideas simply because an existing LSM claims to "do
it all".  We've recently been trying to document this, with the latest
draft viewable here:

https://github.com/LinuxSecurityModule/kernel#new-lsm-guidelines
Paul Moore Oct. 3, 2023, 11:39 p.m. UTC | #14
On Mon, Oct 2, 2023 at 1:06 PM Kees Cook <keescook@chromium.org> wrote:
> On Mon, Oct 02, 2023 at 07:04:27PM +0900, Tetsuo Handa wrote:
> > On 2023/10/02 0:44, Kees Cook wrote:
> > > On October 1, 2023 4:31:05 AM PDT, Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> wrote:
> > >> Kees Cook said there is no problem if the policy of assigning LSM ID value were
> > >>
> > >>  1) author: "Hello, here is a new LSM I'd like to upstream, here it is. I assigned
> > >>              it the next LSM ID."
> > >>     maintainer(s): "Okay, sounds good. *review*"
> > >>
> > >>  2) author: "Hello, here is an LSM that has been in active use at $Place,
> > >>              and we have $Xxx many userspace applications that we cannot easily
> > >>              rebuild. We used LSM ID $Value that is far away from the sequential
> > >>              list of LSM IDs, and we'd really prefer to keep that assignment."
> > >>    maintainer(s): "Okay, sounds good. *review*"
> > >>
> > >> and I agreed at https://lkml.kernel.org/r/6e1c25f5-b78c-8b4e-ddc3-484129c4c0ec@I-love.SAKURA.ne.jp .
> > >>
> > >> But Paul Moore's response was
> > >>
> > >>  No LSM ID value is guaranteed until it is present in a tagged release
> > >>  from Linus' tree, and once a LSM ID is present in a tagged release
> > >>  from Linus' tree it should not change.  That's *the* policy.
> > >>
> > >> which means that the policy is not what Kees Cook has said.
> > >
> > > These don't conflict at all! Paul is saying an ID isn't guaranteed in upstream
> > > until it's in upstream. I'm saying the id space is large enough that you could
> > > make a new out of tree LSM every second for the next billion years. The upstream
> > > assignment process is likely sequential, but that out of sequence LSMs that show
> > > a need to be upstream could make a case for their existing value.
> >
> > Excuse me? If the LSM community wants the assignment sequential, the LSM community
> > cannot admit the LSM value assigned to a not-yet-in-tree LSM.
> >
> > If "Okay, sounds good." does not imply that the LSM community admits the LSM value
> > assigned to a not-yet-in-tree LSM, what did "Okay, sounds good." mean?
>
> I'm saying that if someone participates with upstream correctly, they'll
> get a sequential ID since that is the expected process.
>
> And if an LSM is out of tree for years and years in some large ecosystem
> that has deeply hard-coded the LSM ID but now wants the LSM to land
> upstream, then it's likely that an out-of-sequence ID would be accepted.
>
> My point is that there is nothing technical stopping an out-of-tree LSM
> from existing, and that the political issues for bringing a large
> out-of-tree LSM upstream are going to have plenty of other negotiations
> around maintaining operational behavior, of which and LSM ID is unlikely
> to be a sticking point. Every release that some code (LSM or not) is out
> of tree makes it that much harder to land upstream. (In other words, the
> challenges to upstreaming a long-time-out-of-tree codebase are much
> larger than dealing with an out-of-sequence LSM ID.)

Tetsuo, just in case there is any doubt in your mind, I agree with
Kees' comments above and I reaffirm my previous statement about LSM ID
guarantees.

As far as I can tell this RFC isn't really about dynamically loadable
LSMs, it's about blocking the LSM syscall work, specifically the LSM
ID tokens.  As I've said many times before, the LSM ID concept is
moving forward and if you can't respect that decision, at least stop
wasting our time.
KP Singh Oct. 3, 2023, 11:41 p.m. UTC | #15
On Wed, Oct 4, 2023 at 1:27 AM Paul Moore <paul@paul-moore.com> wrote:
>
> On Wed, Sep 27, 2023 at 12:02 PM KP Singh <kpsingh@kernel.org> wrote:
> >
> > Until I hear the real limitations of using BPF, it's a NAK from me.
>
> There is a lot going on in this thread, and while I'm still playing
> catch-up from LSS-EU and some time off (ish) it looks like most of the
> most important points have already been made, which is great.
> However, I did want to comment quickly on the statement above.
>
> We want to be very careful about using an existing upstream LSM as a
> reason for blocking the inclusion of a new LSM upstream.  We obviously
> want to reject obvious duplicates and proposals that are sufficiently
> "close" (with "close" deliberately left ambiguous here), but we don't
> want to stifle new ideas simply because an existing LSM claims to "do
> it all".  We've recently been trying to document this, with the latest
> draft viewable here:
>
> https://github.com/LinuxSecurityModule/kernel#new-lsm-guidelines

Thanks for the context and documenting this Paul.

>
> --
> paul-moore.com
Tetsuo Handa Oct. 4, 2023, 10:40 a.m. UTC | #16
On 2023/10/02 0:19, Casey Schaufler wrote:
>> I'm fine if security_loadable_hook_heads() (and related code) cannot be
>> disabled by the kernel configuration.
> 
> CONFIG_SECURITY ensures that you will be unhappy.

I don't care about Linux distributors who chose CONFIG_SECURITY=n in their
kernel configurations. What I'm saying is that security_loadable_hook_heads
(and related code) do not depend on some build-time configuration. Also, I
don't care about Linux distributors who patch their kernel source code in
order to remove security_loadable_hook_heads (and related code) before
building their kernels.

But if a kernel is targeted for specific environment where out-of-tree LKMs
(e.g. storage driver, filesystems) are not required, the person/organization
who builds that kernel can protect that kernel from out-of-tree LKMs
(including LKM-based LSMs) by enforcing module signing functionality.

Also if a kernel is ultimately targeted for specific environment where LKM
support is not required, the person/organization who builds that kernel can
protect that kernel from out-of-tree LKMs (including LKM-based LSMs) by
disabling loadable module functionality.

Linux distributors that I want to run LSMs are generally trying to support
as much users/environments as possible. The combination of enabling loadable
module functionality and not enforcing module signing functionality is a good
balance for that purpose.

> Even setting that aside, it's the developer's job to sell the code to
> the communities involved. I could rant at certain distros for not including
> Smack, but until such time as I've made doing that attractive it really
> doesn't make any sense to do so. You don't think I've spent years on stacking
> because I want to run Android containers on Ubuntu, do you?

Which one ("the LSM community" or "the Linux distributors") do you mean by
"the communities involved" ?

For out-of-tree LKMs (e.g. storage driver, filesystems) that can be loaded as
a loadable kernel module, the provider/developer can directly sell the code to
end users (i.e. they can sell without being accepted by the upstream Linux
community and being enabled by the Linux distributors' kernel configurations).

But for out-of-tree LSMs that cannot be loaded as a loadable kernel module,
the provider/developer currently cannot directly sell the code to end users.

You said

  This makes it sound like LSMs are always developed for corporate use.
  While that is generally true, we should acknowledge that the "sponsor"
  of an LSM could be a corporation/government, a foundation or a hobbyist.
  A large, comprehensive LSM from a billion dollar corporation in support
  of a specific product should require more commitment than a small, targeted
  LSM of general interest from joe@schlobotnit.org. I trust that we would
  have the wisdom to make such a distinction, but I don't think we want to
  scare off developers by making it sound like an LSM is something that only
  a corporation can provide a support plan for.

at https://lkml.kernel.org/r/847729f6-99a6-168e-92a6-b1cff1e6b97f@schaufler-ca.com .

But "it's the developer's job to sell the code to the communities involved" is
too hard for alone developer who can write a code and provide support for that code
but cannot afford doing activities for selling that code (e.g. limited involvement
with communities).

Your "it's the developer's job" comment sounds like "LSMs are always developed by
those corporation/government who has much involvement with communities" which
scares off developers who can't afford doing activities for selling that code.

>>> On a less happy note, you haven't addressed security blobs in any way. You
>>> need to provide a mechanism to allow an LSM to share security blobs with
>>> builtin LSMs and other loadable LSMs.
>> Not all LKM-based LSMs need to use security blobs.
> 
> If you only want to support "minor" LSMs, those that don't use shared blobs,
> the loadable list implementation will suit you just fine. And because you won't
> be using any of the LSM infrastructure that needs the LSM ID, that won't be
> an issue.

Minor LSMs can work without using shared blobs managed by the LSM infrastructure.
AKARI/CaitSith are LKM-based LSMs that do not need to use shared blobs managed by
the LSM infrastructure. TOMOYO does not need an LSM ID value, but you are trying
to make an LSM ID mandatory for using the LSM infrastructure.

> You can make something that will work. Whether you can sell it upstream will
> depend on any number of factors. But working code is always a great start.

Selling a code to the upstream is not sufficient for allowing end users to use
that code.

For https://bugzilla.redhat.com/show_bug.cgi?id=542986 case, the reason that Red Hat
does not enable Smack/TOMOYO/AppArmor is "Smack/TOMOYO/AppArmor are not attractive".

After all, requiring any LSMs to be built-in is an unreasonable barrier compared to
other LKMs (e.g. storage driver, filesystems).
Tetsuo Handa Oct. 4, 2023, 10:48 a.m. UTC | #17
On 2023/10/04 19:40, Tetsuo Handa wrote:
> does not enable Smack/TOMOYO/AppArmor is "Smack/TOMOYO/AppArmor are not attractive".

does not enable Smack/TOMOYO/AppArmor is NOT "Smack/TOMOYO/AppArmor are not attractive".
José Bollo Oct. 5, 2023, 9:47 a.m. UTC | #18
Le Wed, 27 Sep 2023 18:02:32 +0200,
KP Singh <kpsingh@kernel.org> a écrit :

> On Wed, Sep 27, 2023 at 5:09 PM Tetsuo Handa
> <penguin-kernel@i-love.sakura.ne.jp> wrote:
> >
> > Recently, the LSM community is trying to make drastic changes.
> >
> > Crispin Cowan has explained
> >
> >   It is Linus' comments that spurred me to want to start this
> > undertaking.  He observes that there are many different security
> > approaches, each with their own advocates.  He doesn't want to
> > arbitrate which of them should be "the" Linux security approach,
> > and would rather that Linux can support any of them.
> >
> >   That is the purpose of this project:  to allow Linux to support a
> > variety of security models, so that security developers don't have
> > to have the "my dog's bigger than your dog" argument, and users can
> > choose the security model that suits their needs.
> >
> > when the LSM project started [1].
> >
> > However, Casey Schaufler is trying to make users difficult to
> > choose the security model that suits their needs, by requiring LSM
> > ID value which is assigned to only LSM modules that succeeded to
> > become in-tree [2]. Therefore, I'm asking Casey and Paul Moore to
> > change their mind to allow assigning LSM ID value to any LSM
> > modules (so that users can choose the security model that suits
> > their needs) [3].
> >
> > I expect that LSM ID value will be assigned to any publicly
> > available LSM modules. Otherwise, it is mostly pointless to propose
> > this patch; there will be little LSM modules to built into vmlinux;
> > let alone dynamically loading as LKM-based LSMs.
> >
> > Also, KP Singh is trying to replace the linked list with static
> > calls in order to reduce overhead of indirect calls [4]. However,
> > this change assumed that any LSM modules are built-in. I don't like
> > such assumption because I still consider that LSM modules which are
> > not built into vmlinux will be wanted by users [5].
> >
> > Then, Casey told me to supply my implementation of loadable security
> > modules [6]. Therefore, I post this patch as basic changes needed
> > for allowing dynamically appendable LSM modules (and an example of
> > appendable LSM modules). This patch was tested on only x86_64.
> >
> > Question for KP Singh would be how can we allow dynamically
> > appendable LSM modules if current linked list is replaced with
> > static calls with minimal-sized array...  
> 
> As I suggested in the other thread:
> 
> https://lore.kernel.org/bpf/20230918212459.1937798-1-kpsingh@kernel.org/T/#md21b9d9cc769f39e451d20364857b693d3fcb587
> 
> You can add extra static call slots and fallback to a linked list
> based implementation if you have more than say N modules [1] and
> fallback to a linked list implementation [2].
> 
> for [1] you can just do MAX_LSM_COUNT you can just do:
> 
> #ifdef CONFIG_MODULAR_LSM
> #define MODULAR_LSM_ENABLED "1,1,1,1"
> #endif
> 
> and use it in the LSM_COUNT.
> 
> for [2] you can choose to export a better module API than directly
> exposing security_hook_heads.
> 
> Now, comes the question of whether we need dynamically loaded LSMs, I
> am not in favor of this.Please share your limitations of BPF as you
> mentioned and what's missing to implement dynamic LSMs. My question
> still remains unanswered.
> 
> Until I hear the real limitations of using BPF, it's a NAK from me.

Hi all,

I don't understand the reason why you want to enforce implementers to
use your BPF?

Even if it can do any possible thing that security implementer wants,
why enforcing to use it? For experimenting? But then after successful
experimentation the implementer must translate to real LSM and rewrite
almost every thing.

And also why to use faty BPF for a tricky simple stuff?

Regards
José Bollo


> >
> > Link:
> > https://marc.info/?l=linux-security-module&m=98706471912438&w=2 [1]
> > Link:
> > https://lkml.kernel.org/r/20230912205658.3432-2-casey@schaufler-ca.com
> > [2] Link:
> > https://lkml.kernel.org/r/6e1c25f5-b78c-8b4e-ddc3-484129c4c0ec@I-love.SAKURA.ne.jp
> > [3] Link:
> > https://lkml.kernel.org/r/20230918212459.1937798-1-kpsingh@kernel.org
> > [4] Link:
> > https://lkml.kernel.org/r/ed785c86-a1d8-caff-c629-f8a50549e05b@I-love.SAKURA.ne.jp
> > [5] Link:
> > https://lkml.kernel.org/r/36c7cf74-508f-1690-f86a-bb18ec686fcf@schaufler-ca.com
> > [6] Signed-off-by: Tetsuo Handa
> > <penguin-kernel@I-love.SAKURA.ne.jp> --- include/linux/lsm_hooks.h
> > |   2 + security/security.c       | 107
> > ++++++++++++++++++++++++++++++++++++++ 2 files changed, 109
> > insertions(+)
> >
> > diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> > index dcb5e5b5eb13..73db3c41df26 100644
> > --- a/include/linux/lsm_hooks.h
> > +++ b/include/linux/lsm_hooks.h
> > @@ -105,6 +105,8 @@ extern char *lsm_names;
> >
> >  extern void security_add_hooks(struct security_hook_list *hooks,
> > int count, const char *lsm);
> > +extern int register_loadable_lsm(struct security_hook_list *hooks,
> > int count,
> > +                                const char *lsm);
> >
> >  #define LSM_FLAG_LEGACY_MAJOR  BIT(0)
> >  #define LSM_FLAG_EXCLUSIVE     BIT(1)
> > diff --git a/security/security.c b/security/security.c
> > index 23b129d482a7..6c64b7afb251 100644
> > --- a/security/security.c
> > +++ b/security/security.c
> > @@ -74,6 +74,7 @@ const char *const
> > lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX + 1] = { };
> >
> >  struct security_hook_heads security_hook_heads __ro_after_init;
> > +EXPORT_SYMBOL_GPL(security_hook_heads);  
> 
> Rather than exposting security_hook_heads, this should actually export
> security_hook_module_register. This should internally handle any data
> structures used and also not need the special magic that you did for
> __ro_after_init.
> 
> - KP
> 
> >  static BLOCKING_NOTIFIER_HEAD(blocking_lsm_notifier_chain);
> >
> >  static struct kmem_cache *lsm_file_cache;
> > @@ -537,6 +538,112 @@ void __init security_add_hooks(struct
> > security_hook_list *hooks, int count, }
> >  }
> >
> > +#if defined(CONFIG_STRICT_KERNEL_RWX)
> > +#define MAX_RO_PAGES 1024 /* Wild guess. Can be minimized by
> > dynamic allocation. */ +static struct page *ro_pages[MAX_RO_PAGES];
> > /* Pages that are marked read-only. */ +static unsigned int
> > ro_pages_len; /* Number of pages that are marked read-only. */ +
> > +/* Check whether a page containing given address does not have
> > _PAGE_BIT_RW bit. */ +static bool lsm_test_page_ro(void *addr)
> > +{
> > +       unsigned int i;
> > +       int unused;
> > +       struct page *page;
> > +
> > +       page = (struct page *) lookup_address((unsigned long) addr,
> > &unused);
> > +       if (!page)
> > +               return false;
> > +       if (test_bit(_PAGE_BIT_RW, &(page->flags)))
> > +               return true;
> > +       for (i = 0; i < ro_pages_len; i++)
> > +               if (page == ro_pages[i])
> > +                       return true;
> > +       if (ro_pages_len == MAX_RO_PAGES)
> > +               return false;
> > +       ro_pages[ro_pages_len++] = page;
> > +       return true;
> > +}
> > +
> > +/* Find pages which do not have _PAGE_BIT_RW bit. */
> > +static bool check_ro_pages(struct security_hook_list *hooks, int
> > count) +{
> > +       int i;
> > +       struct hlist_head *list = &security_hook_heads.capable;
> > +
> > +       if (!copy_to_kernel_nofault(list, list, sizeof(void *)))
> > +               return true;
> > +       for (i = 0; i < count; i++) {
> > +               struct hlist_head *head = hooks[i].head;
> > +               struct security_hook_list *shp;
> > +
> > +               if (!lsm_test_page_ro(&head->first))
> > +                       return false;
> > +               hlist_for_each_entry(shp, head, list)
> > +                       if (!lsm_test_page_ro(&shp->list.next) ||
> > +                           !lsm_test_page_ro(&shp->list.pprev))
> > +                               return false;
> > +       }
> > +       return true;
> > +}
> > +#endif
> > +
> > +/**
> > + * register_loadable_lsm - Add a dynamically appendable module's
> > hooks to the hook lists.
> > + * @hooks: the hooks to add
> > + * @count: the number of hooks to add
> > + * @lsm: the name of the security module
> > + *
> > + * Each dynamically appendable LSM has to register its hooks with
> > the infrastructure.
> > + *
> > + * Assumes that this function is called from module_init()
> > function where
> > + * call to this function is already serialized by module_mutex
> > lock.
> > + */
> > +int register_loadable_lsm(struct security_hook_list *hooks, int
> > count,
> > +                         const char *lsm)
> > +{
> > +       int i;
> > +       char *cp;
> > +
> > +       // TODO: Check whether proposed hooks can co-exist with
> > already chained hooks,
> > +       //       and bail out here if one of hooks cannot
> > co-exist... +
> > +#if defined(CONFIG_STRICT_KERNEL_RWX)
> > +       // Find pages which needs to make temporarily writable.
> > +       ro_pages_len = 0;
> > +       if (!check_ro_pages(hooks, count)) {
> > +               pr_err("Can't make security_hook_heads again
> > writable. Retry with rodata=off kernel command line option
> > added.\n");
> > +               return -EINVAL;
> > +       }
> > +       pr_info("ro_pages_len=%d\n", ro_pages_len);
> > +#endif
> > +       // At least "capability" is already included.
> > +       cp = kasprintf(GFP_KERNEL, "%s,%s", lsm_names, lsm);
> > +       if (!cp) {
> > +               pr_err("%s - Cannot get memory.\n", __func__);
> > +               return -ENOMEM;
> > +       }
> > +#if defined(CONFIG_STRICT_KERNEL_RWX)
> > +       // Make security_hook_heads (and hooks chained) temporarily
> > writable.
> > +       for (i = 0; i < ro_pages_len; i++)
> > +               set_bit(_PAGE_BIT_RW, &(ro_pages[i]->flags));
> > +#endif
> > +       // Register dynamically appendable module's hooks.
> > +       for (i = 0; i < count; i++) {
> > +               hooks[i].lsm = lsm;
> > +               hlist_add_tail_rcu(&hooks[i].list, hooks[i].head);
> > +       }
> > +#if defined(CONFIG_STRICT_KERNEL_RWX)
> > +       // Make security_hook_heads (and hooks chained) again
> > read-only.
> > +       for (i = 0; i < ro_pages_len; i++)
> > +               clear_bit(_PAGE_BIT_RW, &(ro_pages[i]->flags));
> > +#endif
> > +       // TODO: Wait for reader side before kfree().
> > +       kfree(lsm_names);
> > +       lsm_names = cp;
> > +       return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(register_loadable_lsm);
> > +
> >  int call_blocking_lsm_notifier(enum lsm_event event, void *data)
> >  {
> >         return
> > blocking_notifier_call_chain(&blocking_lsm_notifier_chain, --
> > 2.18.4
Tetsuo Handa Oct. 5, 2023, 10:48 a.m. UTC | #19
On 2023/10/04 8:39, Paul Moore wrote:
> As far as I can tell this RFC isn't really about dynamically loadable
> LSMs, it's about blocking the LSM syscall work, specifically the LSM
> ID tokens.  As I've said many times before, the LSM ID concept is
> moving forward and if you can't respect that decision, at least stop
> wasting our time.

This RFC is mainly about how do we plan to allow LKM-based LSMs.
Two proposals (LSM ID and elimination of linked list) might damage
LKM-based LSMs.

Regarding LSM ID, I'm asserting that assigning stable LSM ID to every LSM
is the *better* usage, for users can find whatever LSMs like CVE database
and developers can avoid possible collisions in the LSM infrastructure and
developers can avoid writing obvious duplicates (like you want to reject
proposals that are sufficiently "close"). If some ID were assigned to
implementations like https://github.com/linux-lock/bpflock , users can find
implementations that fit their needs more easily...

BTW, is bpflock considered as an LSM module entity that should be recognized
(i.e. assigned a stable LSM ID) so that the LSM syscalls can return "bpflock" ?
If users want to know which hook caused an access request to be rejected,
having the granularity of "bpf" might not be sufficient...
Tetsuo Handa Oct. 5, 2023, 1:59 p.m. UTC | #20
On 2023/10/05 18:47, José Bollo wrote:
>> Now, comes the question of whether we need dynamically loaded LSMs, I
>> am not in favor of this. Please share your limitations of BPF as you
>> mentioned and what's missing to implement dynamic LSMs. My question
>> still remains unanswered.
>>
>> Until I hear the real limitations of using BPF, it's a NAK from me.
> 
> Hi all,
> 
> I don't understand the reason why you want to enforce implementers to
> use your BPF?

Because if whatever LSM modules were implemented using BPF, we won't need
to support LKM-based LSM. Supporting LKM-based LSM is expected because
the LSM community cannot accept whatever LSMs and the Linux distributor
cannot accept whatever LSMs.

> 
> Even if it can do any possible thing that security implementer wants,
> why enforcing to use it? For experimenting? But then after successful
> experimentation the implementer must translate to real LSM and rewrite
> almost every thing.

Not for experimenting. The advantage of implementing an LSM module using
BPF is that we can load that LSM without making that LSM module in-tree (i.e.
accepted by the LSM community) and built-in (i.e. accepted by the Linux
distributor). That is, the implementer will not try to rewrite a BPF-based
LSM to non BPF-based LSM if the implementer succeed to write that LSM using BPF.

But remaining out-of-tree (i.e. not accepted by the LSM community) might have
disadvantage that the BPF-based LSM is not identified as a LSM because the LSM ID
value won't be assigned. (I don't know where BPF-based LSMs are located in the
kernel source tree. All BPF-based LSMs except trivial examples included in the
kernel source tree will remain out-of-tree ?)

> 
> And also why to use faty BPF for a tricky simple stuff?
KP Singh Oct. 5, 2023, 2:12 p.m. UTC | #21
On Thu, Oct 5, 2023 at 11:48 AM José Bollo <jobol@nonadev.net> wrote:
>
> Le Wed, 27 Sep 2023 18:02:32 +0200,
> KP Singh <kpsingh@kernel.org> a écrit :
>
> > On Wed, Sep 27, 2023 at 5:09 PM Tetsuo Handa
> > <penguin-kernel@i-love.sakura.ne.jp> wrote:
> > >
> > > Recently, the LSM community is trying to make drastic changes.
> > >
> > > Crispin Cowan has explained
> > >
> > >   It is Linus' comments that spurred me to want to start this
> > > undertaking.  He observes that there are many different security
> > > approaches, each with their own advocates.  He doesn't want to
> > > arbitrate which of them should be "the" Linux security approach,
> > > and would rather that Linux can support any of them.
> > >
> > >   That is the purpose of this project:  to allow Linux to support a
> > > variety of security models, so that security developers don't have
> > > to have the "my dog's bigger than your dog" argument, and users can
> > > choose the security model that suits their needs.
> > >
> > > when the LSM project started [1].
> > >
> > > However, Casey Schaufler is trying to make users difficult to
> > > choose the security model that suits their needs, by requiring LSM
> > > ID value which is assigned to only LSM modules that succeeded to
> > > become in-tree [2]. Therefore, I'm asking Casey and Paul Moore to
> > > change their mind to allow assigning LSM ID value to any LSM
> > > modules (so that users can choose the security model that suits
> > > their needs) [3].
> > >
> > > I expect that LSM ID value will be assigned to any publicly
> > > available LSM modules. Otherwise, it is mostly pointless to propose
> > > this patch; there will be little LSM modules to built into vmlinux;
> > > let alone dynamically loading as LKM-based LSMs.
> > >
> > > Also, KP Singh is trying to replace the linked list with static
> > > calls in order to reduce overhead of indirect calls [4]. However,
> > > this change assumed that any LSM modules are built-in. I don't like
> > > such assumption because I still consider that LSM modules which are
> > > not built into vmlinux will be wanted by users [5].
> > >
> > > Then, Casey told me to supply my implementation of loadable security
> > > modules [6]. Therefore, I post this patch as basic changes needed
> > > for allowing dynamically appendable LSM modules (and an example of
> > > appendable LSM modules). This patch was tested on only x86_64.
> > >
> > > Question for KP Singh would be how can we allow dynamically
> > > appendable LSM modules if current linked list is replaced with
> > > static calls with minimal-sized array...
> >
> > As I suggested in the other thread:
> >
> > https://lore.kernel.org/bpf/20230918212459.1937798-1-kpsingh@kernel.org/T/#md21b9d9cc769f39e451d20364857b693d3fcb587
> >
> > You can add extra static call slots and fallback to a linked list
> > based implementation if you have more than say N modules [1] and
> > fallback to a linked list implementation [2].
> >
> > for [1] you can just do MAX_LSM_COUNT you can just do:
> >
> > #ifdef CONFIG_MODULAR_LSM
> > #define MODULAR_LSM_ENABLED "1,1,1,1"
> > #endif
> >
> > and use it in the LSM_COUNT.
> >
> > for [2] you can choose to export a better module API than directly
> > exposing security_hook_heads.
> >
> > Now, comes the question of whether we need dynamically loaded LSMs, I
> > am not in favor of this.Please share your limitations of BPF as you
> > mentioned and what's missing to implement dynamic LSMs. My question
> > still remains unanswered.
> >
> > Until I hear the real limitations of using BPF, it's a NAK from me.
>
> Hi all,
>
> I don't understand the reason why you want to enforce implementers to
> use your BPF?
>
> Even if it can do any possible thing that security implementer wants,
> why enforcing to use it? For experimenting? But then after successful
> experimentation the implementer must translate to real LSM and rewrite
> almost every thing.
>
> And also why to use faty BPF for a tricky simple stuff?
>

faty BPF? I am not even sure what that means? BPF is compiled to
native code and is used in production systems and not just
experimental stuff. I think you have some catching up to do here!
Casey Schaufler Oct. 20, 2023, 8:40 p.m. UTC | #22
On 10/4/2023 3:40 AM, Tetsuo Handa wrote:
> On 2023/10/02 0:19, Casey Schaufler wrote:
>>> I'm fine if security_loadable_hook_heads() (and related code) cannot be
>>> disabled by the kernel configuration.
>> CONFIG_SECURITY ensures that you will be unhappy.
> I don't care about Linux distributors who chose CONFIG_SECURITY=n in their
> kernel configurations. What I'm saying is that security_loadable_hook_heads
> (and related code) do not depend on some build-time configuration. Also, I
> don't care about Linux distributors who patch their kernel source code in
> order to remove security_loadable_hook_heads (and related code) before
> building their kernels.
>
> But if a kernel is targeted for specific environment where out-of-tree LKMs
> (e.g. storage driver, filesystems) are not required, the person/organization
> who builds that kernel can protect that kernel from out-of-tree LKMs
> (including LKM-based LSMs) by enforcing module signing functionality.
>
> Also if a kernel is ultimately targeted for specific environment where LKM
> support is not required, the person/organization who builds that kernel can
> protect that kernel from out-of-tree LKMs (including LKM-based LSMs) by
> disabling loadable module functionality.
>
> Linux distributors that I want to run LSMs are generally trying to support
> as much users/environments as possible. The combination of enabling loadable
> module functionality and not enforcing module signing functionality is a good
> balance for that purpose.
>
>> Even setting that aside, it's the developer's job to sell the code to
>> the communities involved. I could rant at certain distros for not including
>> Smack, but until such time as I've made doing that attractive it really
>> doesn't make any sense to do so. You don't think I've spent years on stacking
>> because I want to run Android containers on Ubuntu, do you?
> Which one ("the LSM community" or "the Linux distributors") do you mean by
> "the communities involved" ?

There's a reason I used the plural "communities" instead of the singular
"community". In the case of loadable LSMs we're talking about *at least*
the LSM developers, Linux distributors, networking developers, the performance
crowd, all those people wound up in secure/trusted boot, API and real-time.

> For out-of-tree LKMs (e.g. storage driver, filesystems) that can be loaded as
> a loadable kernel module, the provider/developer can directly sell the code to
> end users (i.e. they can sell without being accepted by the upstream Linux
> community and being enabled by the Linux distributors' kernel configurations).
>
> But for out-of-tree LSMs that cannot be loaded as a loadable kernel module,
> the provider/developer currently cannot directly sell the code to end users.
>
> You said
>
>   This makes it sound like LSMs are always developed for corporate use.
>   While that is generally true, we should acknowledge that the "sponsor"
>   of an LSM could be a corporation/government, a foundation or a hobbyist.
>   A large, comprehensive LSM from a billion dollar corporation in support
>   of a specific product should require more commitment than a small, targeted
>   LSM of general interest from joe@schlobotnit.org. I trust that we would
>   have the wisdom to make such a distinction, but I don't think we want to
>   scare off developers by making it sound like an LSM is something that only
>   a corporation can provide a support plan for.
>
> at https://lkml.kernel.org/r/847729f6-99a6-168e-92a6-b1cff1e6b97f@schaufler-ca.com .
>
> But "it's the developer's job to sell the code to the communities involved" is
> too hard for alone developer who can write a code and provide support for that code
> but cannot afford doing activities for selling that code (e.g. limited involvement
> with communities).
>
> Your "it's the developer's job" comment sounds like "LSMs are always developed by
> those corporation/government who has much involvement with communities" which
> scares off developers who can't afford doing activities for selling that code.

Sorry, but you've chosen the wrong person to present that argument to.
Smack was developed without any corporate, government or foundation support.
I wrote it in a theater green room  during rehearsals for a production of
"Madmoiselle Modiste". It has, from time to time, received corporate support,
but is currently completely self funded. Yes, it's hard. Yes, the commitment
could well scare off many developers. If you want easy, create websites.

>
>>>> On a less happy note, you haven't addressed security blobs in any way. You
>>>> need to provide a mechanism to allow an LSM to share security blobs with
>>>> builtin LSMs and other loadable LSMs.
>>> Not all LKM-based LSMs need to use security blobs.
>> If you only want to support "minor" LSMs, those that don't use shared blobs,
>> the loadable list implementation will suit you just fine. And because you won't
>> be using any of the LSM infrastructure that needs the LSM ID, that won't be
>> an issue.
> Minor LSMs can work without using shared blobs managed by the LSM infrastructure.
> AKARI/CaitSith are LKM-based LSMs that do not need to use shared blobs managed by
> the LSM infrastructure. TOMOYO does not need an LSM ID value, but you are trying
> to make an LSM ID mandatory for using the LSM infrastructure.
>
>> You can make something that will work. Whether you can sell it upstream will
>> depend on any number of factors. But working code is always a great start.
> Selling a code to the upstream is not sufficient for allowing end users to use
> that code.
>
> For https://bugzilla.redhat.com/show_bug.cgi?id=542986 case, the reason that Red Hat
> does not enable Smack/TOMOYO/AppArmor is "Smack/TOMOYO/AppArmor are not attractive".

And YAMA is enabled because it *is* attractive to RedHat's support based business
model. Even if we did have loadable LSM support I doubt RedHat would even consider
enabling it. Their model is based on selling support.

> After all, requiring any LSMs to be built-in is an unreasonable barrier compared to
> other LKMs (e.g. storage driver, filesystems).
>
Tetsuo Handa Oct. 21, 2023, 12:21 p.m. UTC | #23
On 2023/10/21 5:40, Casey Schaufler wrote:
>>> You can make something that will work. Whether you can sell it upstream will
>>> depend on any number of factors. But working code is always a great start.
>> Selling a code to the upstream is not sufficient for allowing end users to use
>> that code.
>>
>> For https://bugzilla.redhat.com/show_bug.cgi?id=542986 case, the reason that Red Hat
>> does not enable Smack/TOMOYO/AppArmor is NOT "Smack/TOMOYO/AppArmor are not attractive".
> 
> And YAMA is enabled because it *is* attractive to RedHat's support based business
> model. Even if we did have loadable LSM support I doubt RedHat would even consider
> enabling it. Their model is based on selling support.

I don't expect that Red Hat will enable other LSMs as soon as we made it possible to use
other LSMs via LKM. But making it possible to use other LSMs at user's own risk via LKM
(like device/filesystem drivers) is the first step towards enabling other LSMs.

Somebody other than Red Hat can establish a business model for supporting other LSMs. But
current situation (i.e. requiring replacement of vmlinux ) can not allow such somebody to
establish a business model for supporting other LSMs. What is important is "don't make
LKM-based LSMs conditional (e.g. don't require kernel config option to enable LKM-based
LSMs, unlike device/filesystem drivers can be loaded as long as CONFIG_MODULES=y ).
Tetsuo Handa Oct. 21, 2023, 2:19 p.m. UTC | #24
On 2023/10/04 0:09, KP Singh wrote:
>> What I expected is "allocate memory where amount is determined at runtime" (e.g. alloc(), realloc()).
> 
> One can use dynamically sized allocations on the ring buffer with
> dynamic pointers:
> 
> http://vger.kernel.org/bpfconf2022_material/lsfmmbpf2022-dynptr.pdf
> 
> Furthermore, there are some use cases that seemingly need dynamic
> memory allocation but not really. e.g. there was a need to audit
> command line arguments and while it seems dynamic and one can chunk
> the allocation to finite sizes, put these on a ring buffer and process
> the chunks.
> 
> It would be nice to see more details of where the dynamic allocation
> is needed. Security blobs are allocated dynamically but have a fixed
> size.

Dynamic allocation is not for security blobs. Dynamic allocation is for
holding requested pathnames (short-lived allocation), holding audit logs
(FIFO allocation), holding/appending access control rules (long-lived
allocation).



>> Some of core requirements for implementing TOMOYO/AKARI/CaitSith-like programs
>> using BPF will be:
>>
>>   The program registered cannot be stopped/removed by the root user.
>>   This is made possible by either building the program into vmlinux or loading
>>   the program as a LKM without module_exit() callback. Is it possible to guaranee
>>   that a BPF program cannot be stopped/removed by user's operations?
> 
> Yes, there is a security_bpf hook where a BPF MAC policy can be
> implemented and other LSMs do that already.
> 
>>
>>   The program registered cannot be terminated by safety mechanisms (e.g. excessive
>>   CPU time consumption). Are there mechanisms in BPF that wouldn't have terminated
>>   a program if the program were implemented as a LKM rather than a BPF program?
>>
> 
> The kernel does not terminate BPF LSM programs, once a BPF program is
> loaded and attached to the LSM hook, it's JITed into a native code.
> From there onwards, as far as the kernel is concerned it's just like
> any other kernel function.

I was finally able to build and load tools/testing/selftests/bpf/progs/lsm.c and
tools/testing/selftests/bpf/prog_tests/test_lsm.c , and I found fatal limitation
that the program registered is terminated when the file descriptor which refers to
tools/testing/selftests/bpf/lsm.bpf.o is closed (due to e.g. process termination).
That is, eBPF programs are not reliable/robust enough to implement TOMOYO/AKARI/
CaitSith-like programs. Re-registering when the file descriptor is closed is racy
because some critical operations might fail to be traced/checked by the LSM hooks.

Also, I think that automatic cleanup upon closing the file descriptor implies that
allocating resources (or getting reference counts) that are not managed by the BPF
(e.g. files under /sys/kernel/securitytomoyo/ directory) is not permitted. That's
very bad.

> 
>>
>>   Amount of memory needed for managing data is not known at compile time. Thus, I need
>>   kmalloc()-like memory allocation mechanism rather than allocating from some pool, and
>>   manage chunk of memory regions using linked list. Does BPF have kmalloc()-like memory
>>   allocation mechanism that allows allocating up to 32KB (8 pages if PAGE_SIZE=4096).
>>
> 
> You use the ring buffer as a large pool and use dynamic pointers to
> carve chunks out of it, if truly dynamic memory is needed.

TOMOYO/AKARI/CaitSith-like programs do need dynamic memory allocation, as max amount of
memory varies from less than 1MB to more than 10MB. Preallocation is too much wasteful.



> 
>> And maybe somewhere documented question:
>>
>>   What kernel functions can a BPF program call / what kernel data can a BPF program access?
> 
> BPF programs can access kernel data dynamically (accesses relocated at
> load time without needing a recompile) There are lot of good details
> in:
> 
> https://nakryiko.com/posts/bpf-core-reference-guide/
> 
> 
>>   The tools/testing/selftests/bpf/progs/test_d_path.c suggests that a BPF program can call
>>   d_path() defined in fs/d_path.c . But is that because d_path() is marked as EXPORT_SYMBOL() ?
>>   Or can a BPF program call almost all functions (like SystemTap script can insert hooks into
>>   almost all functions)? Even functions / data in LKM can be accessed by a BPF program?
>>
> 
> It's not all kernel functions, but there is a wide range of helpers
> and kfuncs (examples in tools/testing/selftests/bpf) and if there is
> something missing, we will help you.

I couldn't build tools/testing/selftests/bpf/progs/lsm.c with printk() added.
Sending to /sys/kernel/debug/tracing/trace_pipe via bpf_printk() is not enough for
reporting critical/urgent problems. Synchronous operation is important.

Since printk() is not callable, most of functions which TOMOYO/AKARI/CaitSith-like
programs use seem to be not callable.
KP Singh Oct. 21, 2023, 3:20 p.m. UTC | #25
On Sat, Oct 21, 2023 at 4:19 PM Tetsuo Handa
<penguin-kernel@i-love.sakura.ne.jp> wrote:
>
> On 2023/10/04 0:09, KP Singh wrote:
> >> What I expected is "allocate memory where amount is determined at runtime" (e.g. alloc(), realloc()).
> >
> > One can use dynamically sized allocations on the ring buffer with
> > dynamic pointers:
> >
> > http://vger.kernel.org/bpfconf2022_material/lsfmmbpf2022-dynptr.pdf
> >
> > Furthermore, there are some use cases that seemingly need dynamic
> > memory allocation but not really. e.g. there was a need to audit
> > command line arguments and while it seems dynamic and one can chunk
> > the allocation to finite sizes, put these on a ring buffer and process
> > the chunks.
> >
> > It would be nice to see more details of where the dynamic allocation
> > is needed. Security blobs are allocated dynamically but have a fixed
> > size.
>
> Dynamic allocation is not for security blobs. Dynamic allocation is for
> holding requested pathnames (short-lived allocation), holding audit logs
> (FIFO allocation), holding/appending access control rules (long-lived

This is a ring buffer, BPF already has one and used for the very use
case you mentioned (audit logs). Please read the original RFC and
patches for BPF LSM. We have deployed this at scale and it's very
efficient (memory and compute wise).

> allocation).

This is a map, not all maps need to be preallocated. An access control
rule can fundamentally be implemented as a map.

I recommend reading most of the BPF selftests to learn what can be
done / accomplished.

>
>
>
> >> Some of core requirements for implementing TOMOYO/AKARI/CaitSith-like programs
> >> using BPF will be:
> >>
> >>   The program registered cannot be stopped/removed by the root user.
> >>   This is made possible by either building the program into vmlinux or loading
> >>   the program as a LKM without module_exit() callback. Is it possible to guaranee
> >>   that a BPF program cannot be stopped/removed by user's operations?
> >
> > Yes, there is a security_bpf hook where a BPF MAC policy can be
> > implemented and other LSMs do that already.
> >
> >>
> >>   The program registered cannot be terminated by safety mechanisms (e.g. excessive
> >>   CPU time consumption). Are there mechanisms in BPF that wouldn't have terminated
> >>   a program if the program were implemented as a LKM rather than a BPF program?
> >>
> >
> > The kernel does not terminate BPF LSM programs, once a BPF program is
> > loaded and attached to the LSM hook, it's JITed into a native code.
> > From there onwards, as far as the kernel is concerned it's just like
> > any other kernel function.
>
> I was finally able to build and load tools/testing/selftests/bpf/progs/lsm.c and
> tools/testing/selftests/bpf/prog_tests/test_lsm.c , and I found fatal limitation

Programs can also be pinned on /sys/bpf similar to maps, this allows
them to persist even after the loading program goes away.

Here's an example of a pinned program:

https://elixir.bootlin.com/linux/latest/source/tools/testing/selftests/bpf/flow_dissector_load.c#L39

> that the program registered is terminated when the file descriptor which refers to
> tools/testing/selftests/bpf/lsm.bpf.o is closed (due to e.g. process termination).
> That is, eBPF programs are not reliable/robust enough to implement TOMOYO/AKARI/
> CaitSith-like programs. Re-registering when the file descriptor is closed is racy

Not needed as programs can be pinned too.

> because some critical operations might fail to be traced/checked by the LSM hooks.
>
> Also, I think that automatic cleanup upon closing the file descriptor implies that
> allocating resources (or getting reference counts) that are not managed by the BPF
> (e.g. files under /sys/kernel/securitytomoyo/ directory) is not permitted. That's
> very bad.
>
> >
> >>
> >>   Amount of memory needed for managing data is not known at compile time. Thus, I need
> >>   kmalloc()-like memory allocation mechanism rather than allocating from some pool, and
> >>   manage chunk of memory regions using linked list. Does BPF have kmalloc()-like memory
> >>   allocation mechanism that allows allocating up to 32KB (8 pages if PAGE_SIZE=4096).
> >>
> >
> > You use the ring buffer as a large pool and use dynamic pointers to
> > carve chunks out of it, if truly dynamic memory is needed.
>
> TOMOYO/AKARI/CaitSith-like programs do need dynamic memory allocation, as max amount of
> memory varies from less than 1MB to more than 10MB. Preallocation is too much wasteful.
>
>
>
> >
> >> And maybe somewhere documented question:
> >>
> >>   What kernel functions can a BPF program call / what kernel data can a BPF program access?
> >
> > BPF programs can access kernel data dynamically (accesses relocated at
> > load time without needing a recompile) There are lot of good details
> > in:
> >
> > https://nakryiko.com/posts/bpf-core-reference-guide/
> >
> >
> >>   The tools/testing/selftests/bpf/progs/test_d_path.c suggests that a BPF program can call
> >>   d_path() defined in fs/d_path.c . But is that because d_path() is marked as EXPORT_SYMBOL() ?
> >>   Or can a BPF program call almost all functions (like SystemTap script can insert hooks into
> >>   almost all functions)? Even functions / data in LKM can be accessed by a BPF program?
> >>
> >
> > It's not all kernel functions, but there is a wide range of helpers
> > and kfuncs (examples in tools/testing/selftests/bpf) and if there is
> > something missing, we will help you.
>
> I couldn't build tools/testing/selftests/bpf/progs/lsm.c with printk() added.
> Sending to /sys/kernel/debug/tracing/trace_pipe via bpf_printk() is not enough for
> reporting critical/urgent problems. Synchronous operation is important.

you cannot call any function from within BPF. If you need to call
something they need to be exported as a kfunc (you need to send
patches on the mailing list for it). This is because we want to ensure
that BPF programs can be verified.

>
> Since printk() is not callable, most of functions which TOMOYO/AKARI/CaitSith-like
> programs use seem to be not callable.

It seems like you are trying to 1:1 re-implement an existing LSM's
code base in BPF, that's surely not going to work. You need to think
about the use-case / policy you are trying to implement and then write
the code in BPF independently. Please share concrete examples of the
policy you want to implement and we try to help you. Asking for
features where you want a 1:1 parity with kernel code without concrete
policy use-cases is not going to enable us to help you.

- KP

>
Tetsuo Handa Oct. 22, 2023, 1:34 p.m. UTC | #26
On 2023/10/22 0:20, KP Singh wrote:
>> Since printk() is not callable, most of functions which TOMOYO/AKARI/CaitSith-like
>> programs use seem to be not callable.
> 
> It seems like you are trying to 1:1 re-implement an existing LSM's
> code base in BPF,

Yes, that is the goal. Since you said

  "Until I hear the real limitations of using BPF, it's a NAK from me."

at https://lkml.kernel.org/r/CACYkzJ5k7oYxFgWp9bz1Wmp3n6LcU39Mh-HXFWTKnZnpY-Ef7w@mail.gmail.com ,
I want to know whether it is possible to re-implement TOMOYO LSM as an eBPF program.

If it is possible to re-implement TOMOYO LSM as an eBPF program, my desire to
allow appending LKM-based LSMs after boot will be significantly reduced, which
in turn will become my ACK to "security: Count the LSMs enabled at compile time"
in your "Reduce overhead of LSMs with static calls" proposal.

>                   that's surely not going to work. You need to think
> about the use-case / policy you are trying to implement and then write
> the code in BPF independently. Please share concrete examples of the
> policy you want to implement and we try to help you. Asking for
> features where you want a 1:1 parity with kernel code without concrete
> policy use-cases is not going to enable us to help you.

The code which I want to re-implement using eBPF is all of security/tomoyo/ directory.



>> I couldn't build tools/testing/selftests/bpf/progs/lsm.c with printk() added.
>> Sending to /sys/kernel/debug/tracing/trace_pipe via bpf_printk() is not enough for
>> reporting critical/urgent problems. Synchronous operation is important.
> 
> you cannot call any function from within BPF. If you need to call
> something they need to be exported as a kfunc (you need to send
> patches on the mailing list for it). This is because we want to ensure
> that BPF programs can be verified.

TOMOYO needs to be able to call d_absolute_path() in order to calculate
requested pathname, call call_usermodehelper(UMH_WAIT_PROC) in order to load
policy upon activation, call get_mm_exe_file() in order to know the pathname
of executable, get_user_pages_remote() in order to examine argv/envp passed to
execve() system call etc. etc. in addition to performing complicated comparison
including loop like https://elixir.bootlin.com/linux/v6.6-rc6/source/security/tomoyo/group.c#L120 .

If any of above requirements cannot be satisfied in eBPF, that will become
the real limitations of using BPF.



>> I was finally able to build and load tools/testing/selftests/bpf/progs/lsm.c and
>> tools/testing/selftests/bpf/prog_tests/test_lsm.c , and I found fatal limitation
> 
> Programs can also be pinned on /sys/bpf similar to maps, this allows
> them to persist even after the loading program goes away.
> 
> Here's an example of a pinned program:
> 
> https://elixir.bootlin.com/linux/latest/source/tools/testing/selftests/bpf/flow_dissector_load.c#L39
> 
>> that the program registered is terminated when the file descriptor which refers to
>> tools/testing/selftests/bpf/lsm.bpf.o is closed (due to e.g. process termination).
>> That is, eBPF programs are not reliable/robust enough to implement TOMOYO/AKARI/
>> CaitSith-like programs. Re-registering when the file descriptor is closed is racy
> 
> Not needed as programs can be pinned too.
> 

That's good but not enough. We will need to forbid unlink/umount because detach_program()
says "/* To unpin, it is necessary and sufficient to just remove this dir */". Hooking
security_inode_unlink()/security_sb_umount() and return an error if the requested file was
the eBPF version of TOMOYO (or maps etc. related to the eBPF version of TOMOYO) or the
requested filesystem was sysfs might be able to forbid "unpin" operation... That would be
the next step to check if re-implementing all of security/tomoyo/ directory using eBPF
is possible...
diff mbox series

Patch

diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index dcb5e5b5eb13..73db3c41df26 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -105,6 +105,8 @@  extern char *lsm_names;
 
 extern void security_add_hooks(struct security_hook_list *hooks, int count,
 				const char *lsm);
+extern int register_loadable_lsm(struct security_hook_list *hooks, int count,
+				 const char *lsm);
 
 #define LSM_FLAG_LEGACY_MAJOR	BIT(0)
 #define LSM_FLAG_EXCLUSIVE	BIT(1)
diff --git a/security/security.c b/security/security.c
index 23b129d482a7..6c64b7afb251 100644
--- a/security/security.c
+++ b/security/security.c
@@ -74,6 +74,7 @@  const char *const lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX + 1] = {
 };
 
 struct security_hook_heads security_hook_heads __ro_after_init;
+EXPORT_SYMBOL_GPL(security_hook_heads);
 static BLOCKING_NOTIFIER_HEAD(blocking_lsm_notifier_chain);
 
 static struct kmem_cache *lsm_file_cache;
@@ -537,6 +538,112 @@  void __init security_add_hooks(struct security_hook_list *hooks, int count,
 	}
 }
 
+#if defined(CONFIG_STRICT_KERNEL_RWX)
+#define MAX_RO_PAGES 1024 /* Wild guess. Can be minimized by dynamic allocation. */
+static struct page *ro_pages[MAX_RO_PAGES]; /* Pages that are marked read-only. */
+static unsigned int ro_pages_len; /* Number of pages that are marked read-only. */
+
+/* Check whether a page containing given address does not have _PAGE_BIT_RW bit. */
+static bool lsm_test_page_ro(void *addr)
+{
+	unsigned int i;
+	int unused;
+	struct page *page;
+
+	page = (struct page *) lookup_address((unsigned long) addr, &unused);
+	if (!page)
+		return false;
+	if (test_bit(_PAGE_BIT_RW, &(page->flags)))
+		return true;
+	for (i = 0; i < ro_pages_len; i++)
+		if (page == ro_pages[i])
+			return true;
+	if (ro_pages_len == MAX_RO_PAGES)
+		return false;
+	ro_pages[ro_pages_len++] = page;
+	return true;
+}
+
+/* Find pages which do not have _PAGE_BIT_RW bit. */
+static bool check_ro_pages(struct security_hook_list *hooks, int count)
+{
+	int i;
+	struct hlist_head *list = &security_hook_heads.capable;
+
+	if (!copy_to_kernel_nofault(list, list, sizeof(void *)))
+		return true;
+	for (i = 0; i < count; i++) {
+		struct hlist_head *head = hooks[i].head;
+		struct security_hook_list *shp;
+
+		if (!lsm_test_page_ro(&head->first))
+			return false;
+		hlist_for_each_entry(shp, head, list)
+			if (!lsm_test_page_ro(&shp->list.next) ||
+			    !lsm_test_page_ro(&shp->list.pprev))
+				return false;
+	}
+	return true;
+}
+#endif
+
+/**
+ * register_loadable_lsm - Add a dynamically appendable module's hooks to the hook lists.
+ * @hooks: the hooks to add
+ * @count: the number of hooks to add
+ * @lsm: the name of the security module
+ *
+ * Each dynamically appendable LSM has to register its hooks with the infrastructure.
+ *
+ * Assumes that this function is called from module_init() function where
+ * call to this function is already serialized by module_mutex lock.
+ */
+int register_loadable_lsm(struct security_hook_list *hooks, int count,
+			  const char *lsm)
+{
+	int i;
+	char *cp;
+
+	// TODO: Check whether proposed hooks can co-exist with already chained hooks,
+	//       and bail out here if one of hooks cannot co-exist...
+
+#if defined(CONFIG_STRICT_KERNEL_RWX)
+	// Find pages which needs to make temporarily writable.
+	ro_pages_len = 0;
+	if (!check_ro_pages(hooks, count)) {
+		pr_err("Can't make security_hook_heads again writable. Retry with rodata=off kernel command line option added.\n");
+		return -EINVAL;
+	}
+	pr_info("ro_pages_len=%d\n", ro_pages_len);
+#endif
+	// At least "capability" is already included.
+	cp = kasprintf(GFP_KERNEL, "%s,%s", lsm_names, lsm);
+	if (!cp) {
+		pr_err("%s - Cannot get memory.\n", __func__);
+		return -ENOMEM;
+	}
+#if defined(CONFIG_STRICT_KERNEL_RWX)
+	// Make security_hook_heads (and hooks chained) temporarily writable.
+	for (i = 0; i < ro_pages_len; i++)
+		set_bit(_PAGE_BIT_RW, &(ro_pages[i]->flags));
+#endif
+	// Register dynamically appendable module's hooks.
+	for (i = 0; i < count; i++) {
+		hooks[i].lsm = lsm;
+		hlist_add_tail_rcu(&hooks[i].list, hooks[i].head);
+	}
+#if defined(CONFIG_STRICT_KERNEL_RWX)
+	// Make security_hook_heads (and hooks chained) again read-only.
+	for (i = 0; i < ro_pages_len; i++)
+		clear_bit(_PAGE_BIT_RW, &(ro_pages[i]->flags));
+#endif
+	// TODO: Wait for reader side before kfree().
+	kfree(lsm_names);
+	lsm_names = cp;
+	return 0;
+}
+EXPORT_SYMBOL_GPL(register_loadable_lsm);
+
 int call_blocking_lsm_notifier(enum lsm_event event, void *data)
 {
 	return blocking_notifier_call_chain(&blocking_lsm_notifier_chain,