All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] SELinux: allow other LSMs to use custom mount args
@ 2018-08-28 21:32 Micah Morton
  2018-08-29  4:58   ` Paul Moore
  2018-08-29 16:14   ` Casey Schaufler
  0 siblings, 2 replies; 11+ messages in thread
From: Micah Morton @ 2018-08-28 21:32 UTC (permalink / raw)
  To: linux-security-module

The security_sb_copy_data LSM hook allows LSMs to copy custom string
name/value args passed to mount_fs() into a temporary buffer (called
"secdata") that will be accessible to LSM code during the
security_sb_kern_mount hook further down in mount_fs(). Currently,
SELinux effectively prevents any other LSMs from copying custom mount
args into the temporary buffer (and being able to access them during
security_sb_kern_mount), as it will fail with -EINVAL and print
"SELinux:  unknown mount option" to the kernel message buffer if args it
doesn't recognize are present in the temporary buffer when
selinux_sb_kern_mount is called. This change adds an arg to the list of
those accepted by SELinux during security_sb_kern_mount. SELinux won't
do anything with this arg besides allow the name/value pair to be passed
along to any other LSM that is stacked after SELinux.

Developed on v4.18.

Signed-off-by: Micah Morton <mortonm@chromium.org>
---
 security/selinux/hooks.c            |  7 ++++++-
 security/selinux/include/security.h | 11 ++++++-----
 2 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 2b5ee5fbd652..e70ccc701eb8 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -445,6 +445,7 @@ enum {
 	Opt_rootcontext = 4,
 	Opt_labelsupport = 5,
 	Opt_nextmntopt = 6,
+	Opt_lsm_custom_arg = 7,
 };
 
 #define NUM_SEL_MNT_OPTS	(Opt_nextmntopt - 1)
@@ -455,6 +456,7 @@ static const match_table_t tokens = {
 	{Opt_defcontext, DEFCONTEXT_STR "%s"},
 	{Opt_rootcontext, ROOTCONTEXT_STR "%s"},
 	{Opt_labelsupport, LABELSUPP_STR},
+	{Opt_lsm_custom_arg, LSM_CUSTOM_ARG_STR "%s"},
 	{Opt_error, NULL},
 };
 
@@ -1156,6 +1158,8 @@ static int selinux_parse_opts_str(char *options,
 			break;
 		case Opt_labelsupport:
 			break;
+		case Opt_lsm_custom_arg:
+			break;
 		default:
 			rc = -EINVAL;
 			printk(KERN_WARNING "SELinux:  unknown mount option\n");
@@ -2758,7 +2762,8 @@ static inline int selinux_option(char *option, int len)
 		match_prefix(FSCONTEXT_STR, sizeof(FSCONTEXT_STR)-1, option, len) ||
 		match_prefix(DEFCONTEXT_STR, sizeof(DEFCONTEXT_STR)-1, option, len) ||
 		match_prefix(ROOTCONTEXT_STR, sizeof(ROOTCONTEXT_STR)-1, option, len) ||
-		match_prefix(LABELSUPP_STR, sizeof(LABELSUPP_STR)-1, option, len));
+		match_prefix(LABELSUPP_STR, sizeof(LABELSUPP_STR)-1, option, len) ||
+		match_prefix(LSM_CUSTOM_ARG_STR, sizeof(LSM_CUSTOM_ARG_STR)-1, option, len));
 }
 
 static inline void take_option(char **to, char *from, int *first, int len)
diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h
index 23e762d529fa..0ead836a0625 100644
--- a/security/selinux/include/security.h
+++ b/security/selinux/include/security.h
@@ -59,11 +59,12 @@
 #define SE_SBPROC		0x0200
 #define SE_SBGENFS		0x0400
 
-#define CONTEXT_STR	"context="
-#define FSCONTEXT_STR	"fscontext="
-#define ROOTCONTEXT_STR	"rootcontext="
-#define DEFCONTEXT_STR	"defcontext="
-#define LABELSUPP_STR "seclabel"
+#define CONTEXT_STR         "context="
+#define FSCONTEXT_STR       "fscontext="
+#define ROOTCONTEXT_STR     "rootcontext="
+#define DEFCONTEXT_STR      "defcontext="
+#define LABELSUPP_STR       "seclabel"
+#define LSM_CUSTOM_ARG_STR  "lsm_custom_arg="
 
 struct netlbl_lsm_secattr;
 
-- 
2.19.0.rc0.228.g281dcd1b4d0-goog

^ permalink raw reply related	[flat|nested] 11+ messages in thread
* Re: [PATCH] SELinux: allow other LSMs to use custom mount args
  2018-08-28 21:32 [PATCH] SELinux: allow other LSMs to use custom mount args Micah Morton
@ 2018-08-29  4:58   ` Paul Moore
  2018-08-29 16:14   ` Casey Schaufler
  1 sibling, 0 replies; 11+ messages in thread
From: Paul Moore @ 2018-08-29  4:58 UTC (permalink / raw)
  To: mortonm; +Cc: linux-security-module, selinux

On Tue, Aug 28, 2018 at 5:32 PM Micah Morton <mortonm@chromium.org> wrote:
> The security_sb_copy_data LSM hook allows LSMs to copy custom string
> name/value args passed to mount_fs() into a temporary buffer (called
> "secdata") that will be accessible to LSM code during the
> security_sb_kern_mount hook further down in mount_fs(). Currently,
> SELinux effectively prevents any other LSMs from copying custom mount
> args into the temporary buffer (and being able to access them during
> security_sb_kern_mount), as it will fail with -EINVAL and print
> "SELinux:  unknown mount option" to the kernel message buffer if args it
> doesn't recognize are present in the temporary buffer when
> selinux_sb_kern_mount is called. This change adds an arg to the list of
> those accepted by SELinux during security_sb_kern_mount. SELinux won't
> do anything with this arg besides allow the name/value pair to be passed
> along to any other LSM that is stacked after SELinux.
>
> Developed on v4.18.
>
> Signed-off-by: Micah Morton <mortonm@chromium.org>
> ---
>  security/selinux/hooks.c            |  7 ++++++-
>  security/selinux/include/security.h | 11 ++++++-----
>  2 files changed, 12 insertions(+), 6 deletions(-)

SELinux patches need to be sent to the SELinux mailing list (CC'd) for
proper review.

> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 2b5ee5fbd652..e70ccc701eb8 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -445,6 +445,7 @@ enum {
>         Opt_rootcontext = 4,
>         Opt_labelsupport = 5,
>         Opt_nextmntopt = 6,
> +       Opt_lsm_custom_arg = 7,
>  };
>
>  #define NUM_SEL_MNT_OPTS       (Opt_nextmntopt - 1)
> @@ -455,6 +456,7 @@ static const match_table_t tokens = {
>         {Opt_defcontext, DEFCONTEXT_STR "%s"},
>         {Opt_rootcontext, ROOTCONTEXT_STR "%s"},
>         {Opt_labelsupport, LABELSUPP_STR},
> +       {Opt_lsm_custom_arg, LSM_CUSTOM_ARG_STR "%s"},
>         {Opt_error, NULL},
>  };
>
> @@ -1156,6 +1158,8 @@ static int selinux_parse_opts_str(char *options,
>                         break;
>                 case Opt_labelsupport:
>                         break;
> +               case Opt_lsm_custom_arg:
> +                       break;
>                 default:
>                         rc = -EINVAL;
>                         printk(KERN_WARNING "SELinux:  unknown mount option\n");
> @@ -2758,7 +2762,8 @@ static inline int selinux_option(char *option, int len)
>                 match_prefix(FSCONTEXT_STR, sizeof(FSCONTEXT_STR)-1, option, len) ||
>                 match_prefix(DEFCONTEXT_STR, sizeof(DEFCONTEXT_STR)-1, option, len) ||
>                 match_prefix(ROOTCONTEXT_STR, sizeof(ROOTCONTEXT_STR)-1, option, len) ||
> -               match_prefix(LABELSUPP_STR, sizeof(LABELSUPP_STR)-1, option, len));
> +               match_prefix(LABELSUPP_STR, sizeof(LABELSUPP_STR)-1, option, len) ||
> +               match_prefix(LSM_CUSTOM_ARG_STR, sizeof(LSM_CUSTOM_ARG_STR)-1, option, len));
>  }
>
>  static inline void take_option(char **to, char *from, int *first, int len)
> diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h
> index 23e762d529fa..0ead836a0625 100644
> --- a/security/selinux/include/security.h
> +++ b/security/selinux/include/security.h
> @@ -59,11 +59,12 @@
>  #define SE_SBPROC              0x0200
>  #define SE_SBGENFS             0x0400
>
> -#define CONTEXT_STR    "context="
> -#define FSCONTEXT_STR  "fscontext="
> -#define ROOTCONTEXT_STR        "rootcontext="
> -#define DEFCONTEXT_STR "defcontext="
> -#define LABELSUPP_STR "seclabel"
> +#define CONTEXT_STR         "context="
> +#define FSCONTEXT_STR       "fscontext="
> +#define ROOTCONTEXT_STR     "rootcontext="
> +#define DEFCONTEXT_STR      "defcontext="
> +#define LABELSUPP_STR       "seclabel"
> +#define LSM_CUSTOM_ARG_STR  "lsm_custom_arg="
>
>  struct netlbl_lsm_secattr;
>
> --
> 2.19.0.rc0.228.g281dcd1b4d0-goog
>


-- 
paul moore
www.paul-moore.com

^ permalink raw reply	[flat|nested] 11+ messages in thread
* [PATCH] SELinux: allow other LSMs to use custom mount args
@ 2018-08-29  4:58   ` Paul Moore
  0 siblings, 0 replies; 11+ messages in thread
From: Paul Moore @ 2018-08-29  4:58 UTC (permalink / raw)
  To: linux-security-module

On Tue, Aug 28, 2018 at 5:32 PM Micah Morton <mortonm@chromium.org> wrote:
> The security_sb_copy_data LSM hook allows LSMs to copy custom string
> name/value args passed to mount_fs() into a temporary buffer (called
> "secdata") that will be accessible to LSM code during the
> security_sb_kern_mount hook further down in mount_fs(). Currently,
> SELinux effectively prevents any other LSMs from copying custom mount
> args into the temporary buffer (and being able to access them during
> security_sb_kern_mount), as it will fail with -EINVAL and print
> "SELinux:  unknown mount option" to the kernel message buffer if args it
> doesn't recognize are present in the temporary buffer when
> selinux_sb_kern_mount is called. This change adds an arg to the list of
> those accepted by SELinux during security_sb_kern_mount. SELinux won't
> do anything with this arg besides allow the name/value pair to be passed
> along to any other LSM that is stacked after SELinux.
>
> Developed on v4.18.
>
> Signed-off-by: Micah Morton <mortonm@chromium.org>
> ---
>  security/selinux/hooks.c            |  7 ++++++-
>  security/selinux/include/security.h | 11 ++++++-----
>  2 files changed, 12 insertions(+), 6 deletions(-)

SELinux patches need to be sent to the SELinux mailing list (CC'd) for
proper review.

> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 2b5ee5fbd652..e70ccc701eb8 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -445,6 +445,7 @@ enum {
>         Opt_rootcontext = 4,
>         Opt_labelsupport = 5,
>         Opt_nextmntopt = 6,
> +       Opt_lsm_custom_arg = 7,
>  };
>
>  #define NUM_SEL_MNT_OPTS       (Opt_nextmntopt - 1)
> @@ -455,6 +456,7 @@ static const match_table_t tokens = {
>         {Opt_defcontext, DEFCONTEXT_STR "%s"},
>         {Opt_rootcontext, ROOTCONTEXT_STR "%s"},
>         {Opt_labelsupport, LABELSUPP_STR},
> +       {Opt_lsm_custom_arg, LSM_CUSTOM_ARG_STR "%s"},
>         {Opt_error, NULL},
>  };
>
> @@ -1156,6 +1158,8 @@ static int selinux_parse_opts_str(char *options,
>                         break;
>                 case Opt_labelsupport:
>                         break;
> +               case Opt_lsm_custom_arg:
> +                       break;
>                 default:
>                         rc = -EINVAL;
>                         printk(KERN_WARNING "SELinux:  unknown mount option\n");
> @@ -2758,7 +2762,8 @@ static inline int selinux_option(char *option, int len)
>                 match_prefix(FSCONTEXT_STR, sizeof(FSCONTEXT_STR)-1, option, len) ||
>                 match_prefix(DEFCONTEXT_STR, sizeof(DEFCONTEXT_STR)-1, option, len) ||
>                 match_prefix(ROOTCONTEXT_STR, sizeof(ROOTCONTEXT_STR)-1, option, len) ||
> -               match_prefix(LABELSUPP_STR, sizeof(LABELSUPP_STR)-1, option, len));
> +               match_prefix(LABELSUPP_STR, sizeof(LABELSUPP_STR)-1, option, len) ||
> +               match_prefix(LSM_CUSTOM_ARG_STR, sizeof(LSM_CUSTOM_ARG_STR)-1, option, len));
>  }
>
>  static inline void take_option(char **to, char *from, int *first, int len)
> diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h
> index 23e762d529fa..0ead836a0625 100644
> --- a/security/selinux/include/security.h
> +++ b/security/selinux/include/security.h
> @@ -59,11 +59,12 @@
>  #define SE_SBPROC              0x0200
>  #define SE_SBGENFS             0x0400
>
> -#define CONTEXT_STR    "context="
> -#define FSCONTEXT_STR  "fscontext="
> -#define ROOTCONTEXT_STR        "rootcontext="
> -#define DEFCONTEXT_STR "defcontext="
> -#define LABELSUPP_STR "seclabel"
> +#define CONTEXT_STR         "context="
> +#define FSCONTEXT_STR       "fscontext="
> +#define ROOTCONTEXT_STR     "rootcontext="
> +#define DEFCONTEXT_STR      "defcontext="
> +#define LABELSUPP_STR       "seclabel"
> +#define LSM_CUSTOM_ARG_STR  "lsm_custom_arg="
>
>  struct netlbl_lsm_secattr;
>
> --
> 2.19.0.rc0.228.g281dcd1b4d0-goog
>


-- 
paul moore
www.paul-moore.com

^ permalink raw reply	[flat|nested] 11+ messages in thread
* Re: [PATCH] SELinux: allow other LSMs to use custom mount args
  2018-08-28 21:32 [PATCH] SELinux: allow other LSMs to use custom mount args Micah Morton
@ 2018-08-29 16:14   ` Casey Schaufler
  2018-08-29 16:14   ` Casey Schaufler
  1 sibling, 0 replies; 11+ messages in thread
From: Casey Schaufler @ 2018-08-29 16:14 UTC (permalink / raw)
  To: Micah Morton, linux-security-module; +Cc: SE Linux

On 8/28/2018 2:32 PM, Micah Morton wrote:
> The security_sb_copy_data LSM hook allows LSMs to copy custom string
> name/value args passed to mount_fs() into a temporary buffer (called
> "secdata") that will be accessible to LSM code during the
> security_sb_kern_mount hook further down in mount_fs(). Currently,
> SELinux effectively prevents any other LSMs from copying custom mount
> args into the temporary buffer (and being able to access them during
> security_sb_kern_mount), as it will fail with -EINVAL and print
> "SELinux:  unknown mount option" to the kernel message buffer if args it
> doesn't recognize are present in the temporary buffer when
> selinux_sb_kern_mount is called. This change adds an arg to the list of
> those accepted by SELinux during security_sb_kern_mount. SELinux won't
> do anything with this arg besides allow the name/value pair to be passed
> along to any other LSM that is stacked after SELinux.
>
> Developed on v4.18.
>
> Signed-off-by: Micah Morton <mortonm@chromium.org>

This needs to be generalized. Whatever security module you're
working on won't stack with any other module that uses mount
options, Smack in particular. Have you looked at the patch I've
been proposing as part of the general stacking work?

> ---
>  security/selinux/hooks.c            |  7 ++++++-
>  security/selinux/include/security.h | 11 ++++++-----
>  2 files changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 2b5ee5fbd652..e70ccc701eb8 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -445,6 +445,7 @@ enum {
>  	Opt_rootcontext = 4,
>  	Opt_labelsupport = 5,
>  	Opt_nextmntopt = 6,
> +	Opt_lsm_custom_arg = 7,
>  };
>  
>  #define NUM_SEL_MNT_OPTS	(Opt_nextmntopt - 1)
> @@ -455,6 +456,7 @@ static const match_table_t tokens = {
>  	{Opt_defcontext, DEFCONTEXT_STR "%s"},
>  	{Opt_rootcontext, ROOTCONTEXT_STR "%s"},
>  	{Opt_labelsupport, LABELSUPP_STR},
> +	{Opt_lsm_custom_arg, LSM_CUSTOM_ARG_STR "%s"},
>  	{Opt_error, NULL},
>  };
>  
> @@ -1156,6 +1158,8 @@ static int selinux_parse_opts_str(char *options,
>  			break;
>  		case Opt_labelsupport:
>  			break;
> +		case Opt_lsm_custom_arg:
> +			break;
>  		default:
>  			rc = -EINVAL;
>  			printk(KERN_WARNING "SELinux:  unknown mount option\n");
> @@ -2758,7 +2762,8 @@ static inline int selinux_option(char *option, int len)
>  		match_prefix(FSCONTEXT_STR, sizeof(FSCONTEXT_STR)-1, option, len) ||
>  		match_prefix(DEFCONTEXT_STR, sizeof(DEFCONTEXT_STR)-1, option, len) ||
>  		match_prefix(ROOTCONTEXT_STR, sizeof(ROOTCONTEXT_STR)-1, option, len) ||
> -		match_prefix(LABELSUPP_STR, sizeof(LABELSUPP_STR)-1, option, len));
> +		match_prefix(LABELSUPP_STR, sizeof(LABELSUPP_STR)-1, option, len) ||
> +		match_prefix(LSM_CUSTOM_ARG_STR, sizeof(LSM_CUSTOM_ARG_STR)-1, option, len));
>  }
>  
>  static inline void take_option(char **to, char *from, int *first, int len)
> diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h
> index 23e762d529fa..0ead836a0625 100644
> --- a/security/selinux/include/security.h
> +++ b/security/selinux/include/security.h
> @@ -59,11 +59,12 @@
>  #define SE_SBPROC		0x0200
>  #define SE_SBGENFS		0x0400
>  
> -#define CONTEXT_STR	"context="
> -#define FSCONTEXT_STR	"fscontext="
> -#define ROOTCONTEXT_STR	"rootcontext="
> -#define DEFCONTEXT_STR	"defcontext="
> -#define LABELSUPP_STR "seclabel"
> +#define CONTEXT_STR         "context="
> +#define FSCONTEXT_STR       "fscontext="
> +#define ROOTCONTEXT_STR     "rootcontext="
> +#define DEFCONTEXT_STR      "defcontext="
> +#define LABELSUPP_STR       "seclabel"
> +#define LSM_CUSTOM_ARG_STR  "lsm_custom_arg="
>  
>  struct netlbl_lsm_secattr;
>  

^ permalink raw reply	[flat|nested] 11+ messages in thread
* [PATCH] SELinux: allow other LSMs to use custom mount args
@ 2018-08-29 16:14   ` Casey Schaufler
  0 siblings, 0 replies; 11+ messages in thread
From: Casey Schaufler @ 2018-08-29 16:14 UTC (permalink / raw)
  To: linux-security-module

On 8/28/2018 2:32 PM, Micah Morton wrote:
> The security_sb_copy_data LSM hook allows LSMs to copy custom string
> name/value args passed to mount_fs() into a temporary buffer (called
> "secdata") that will be accessible to LSM code during the
> security_sb_kern_mount hook further down in mount_fs(). Currently,
> SELinux effectively prevents any other LSMs from copying custom mount
> args into the temporary buffer (and being able to access them during
> security_sb_kern_mount), as it will fail with -EINVAL and print
> "SELinux:  unknown mount option" to the kernel message buffer if args it
> doesn't recognize are present in the temporary buffer when
> selinux_sb_kern_mount is called. This change adds an arg to the list of
> those accepted by SELinux during security_sb_kern_mount. SELinux won't
> do anything with this arg besides allow the name/value pair to be passed
> along to any other LSM that is stacked after SELinux.
>
> Developed on v4.18.
>
> Signed-off-by: Micah Morton <mortonm@chromium.org>

This needs to be generalized. Whatever security module you're
working on won't stack with any other module that uses mount
options, Smack in particular. Have you looked at the patch I've
been proposing as part of the general stacking work?

> ---
>  security/selinux/hooks.c            |  7 ++++++-
>  security/selinux/include/security.h | 11 ++++++-----
>  2 files changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 2b5ee5fbd652..e70ccc701eb8 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -445,6 +445,7 @@ enum {
>  	Opt_rootcontext = 4,
>  	Opt_labelsupport = 5,
>  	Opt_nextmntopt = 6,
> +	Opt_lsm_custom_arg = 7,
>  };
>  
>  #define NUM_SEL_MNT_OPTS	(Opt_nextmntopt - 1)
> @@ -455,6 +456,7 @@ static const match_table_t tokens = {
>  	{Opt_defcontext, DEFCONTEXT_STR "%s"},
>  	{Opt_rootcontext, ROOTCONTEXT_STR "%s"},
>  	{Opt_labelsupport, LABELSUPP_STR},
> +	{Opt_lsm_custom_arg, LSM_CUSTOM_ARG_STR "%s"},
>  	{Opt_error, NULL},
>  };
>  
> @@ -1156,6 +1158,8 @@ static int selinux_parse_opts_str(char *options,
>  			break;
>  		case Opt_labelsupport:
>  			break;
> +		case Opt_lsm_custom_arg:
> +			break;
>  		default:
>  			rc = -EINVAL;
>  			printk(KERN_WARNING "SELinux:  unknown mount option\n");
> @@ -2758,7 +2762,8 @@ static inline int selinux_option(char *option, int len)
>  		match_prefix(FSCONTEXT_STR, sizeof(FSCONTEXT_STR)-1, option, len) ||
>  		match_prefix(DEFCONTEXT_STR, sizeof(DEFCONTEXT_STR)-1, option, len) ||
>  		match_prefix(ROOTCONTEXT_STR, sizeof(ROOTCONTEXT_STR)-1, option, len) ||
> -		match_prefix(LABELSUPP_STR, sizeof(LABELSUPP_STR)-1, option, len));
> +		match_prefix(LABELSUPP_STR, sizeof(LABELSUPP_STR)-1, option, len) ||
> +		match_prefix(LSM_CUSTOM_ARG_STR, sizeof(LSM_CUSTOM_ARG_STR)-1, option, len));
>  }
>  
>  static inline void take_option(char **to, char *from, int *first, int len)
> diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h
> index 23e762d529fa..0ead836a0625 100644
> --- a/security/selinux/include/security.h
> +++ b/security/selinux/include/security.h
> @@ -59,11 +59,12 @@
>  #define SE_SBPROC		0x0200
>  #define SE_SBGENFS		0x0400
>  
> -#define CONTEXT_STR	"context="
> -#define FSCONTEXT_STR	"fscontext="
> -#define ROOTCONTEXT_STR	"rootcontext="
> -#define DEFCONTEXT_STR	"defcontext="
> -#define LABELSUPP_STR "seclabel"
> +#define CONTEXT_STR         "context="
> +#define FSCONTEXT_STR       "fscontext="
> +#define ROOTCONTEXT_STR     "rootcontext="
> +#define DEFCONTEXT_STR      "defcontext="
> +#define LABELSUPP_STR       "seclabel"
> +#define LSM_CUSTOM_ARG_STR  "lsm_custom_arg="
>  
>  struct netlbl_lsm_secattr;
>  

^ permalink raw reply	[flat|nested] 11+ messages in thread
* Re: [PATCH] SELinux: allow other LSMs to use custom mount args
  2018-08-29 16:14   ` Casey Schaufler
@ 2018-08-29 21:44     ` Micah Morton
  -1 siblings, 0 replies; 11+ messages in thread
From: Micah Morton @ 2018-08-29 21:44 UTC (permalink / raw)
  To: casey; +Cc: linux-security-module, selinux

So are you saying that since another security module (i.e. Smack) has
this issue, SELinux ought not to fix this issue? I agree it would be
optimal to solve this problem in the general sense, but I don't see
why it shouldn't be solved for SELinux in the mean time. Seems like
that would give even more motivation to solve the problem in the
general sense down the road. I'm not sure which patch your talking
about, although I'm familiar with this post
https://lwn.net/Articles/635771/ and the linked patch:
https://lwn.net/Articles/636056/. Is there a more recent version since
2015?
On Wed, Aug 29, 2018 at 9:18 AM Casey Schaufler <casey@schaufler-ca.com> wrote:
>
> On 8/28/2018 2:32 PM, Micah Morton wrote:
> > The security_sb_copy_data LSM hook allows LSMs to copy custom string
> > name/value args passed to mount_fs() into a temporary buffer (called
> > "secdata") that will be accessible to LSM code during the
> > security_sb_kern_mount hook further down in mount_fs(). Currently,
> > SELinux effectively prevents any other LSMs from copying custom mount
> > args into the temporary buffer (and being able to access them during
> > security_sb_kern_mount), as it will fail with -EINVAL and print
> > "SELinux:  unknown mount option" to the kernel message buffer if args it
> > doesn't recognize are present in the temporary buffer when
> > selinux_sb_kern_mount is called. This change adds an arg to the list of
> > those accepted by SELinux during security_sb_kern_mount. SELinux won't
> > do anything with this arg besides allow the name/value pair to be passed
> > along to any other LSM that is stacked after SELinux.
> >
> > Developed on v4.18.
> >
> > Signed-off-by: Micah Morton <mortonm@chromium.org>
>
> This needs to be generalized. Whatever security module you're
> working on won't stack with any other module that uses mount
> options, Smack in particular. Have you looked at the patch I've
> been proposing as part of the general stacking work?
>
> > ---
> >  security/selinux/hooks.c            |  7 ++++++-
> >  security/selinux/include/security.h | 11 ++++++-----
> >  2 files changed, 12 insertions(+), 6 deletions(-)
> >
> > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> > index 2b5ee5fbd652..e70ccc701eb8 100644
> > --- a/security/selinux/hooks.c
> > +++ b/security/selinux/hooks.c
> > @@ -445,6 +445,7 @@ enum {
> >       Opt_rootcontext = 4,
> >       Opt_labelsupport = 5,
> >       Opt_nextmntopt = 6,
> > +     Opt_lsm_custom_arg = 7,
> >  };
> >
> >  #define NUM_SEL_MNT_OPTS     (Opt_nextmntopt - 1)
> > @@ -455,6 +456,7 @@ static const match_table_t tokens = {
> >       {Opt_defcontext, DEFCONTEXT_STR "%s"},
> >       {Opt_rootcontext, ROOTCONTEXT_STR "%s"},
> >       {Opt_labelsupport, LABELSUPP_STR},
> > +     {Opt_lsm_custom_arg, LSM_CUSTOM_ARG_STR "%s"},
> >       {Opt_error, NULL},
> >  };
> >
> > @@ -1156,6 +1158,8 @@ static int selinux_parse_opts_str(char *options,
> >                       break;
> >               case Opt_labelsupport:
> >                       break;
> > +             case Opt_lsm_custom_arg:
> > +                     break;
> >               default:
> >                       rc = -EINVAL;
> >                       printk(KERN_WARNING "SELinux:  unknown mount option\n");
> > @@ -2758,7 +2762,8 @@ static inline int selinux_option(char *option, int len)
> >               match_prefix(FSCONTEXT_STR, sizeof(FSCONTEXT_STR)-1, option, len) ||
> >               match_prefix(DEFCONTEXT_STR, sizeof(DEFCONTEXT_STR)-1, option, len) ||
> >               match_prefix(ROOTCONTEXT_STR, sizeof(ROOTCONTEXT_STR)-1, option, len) ||
> > -             match_prefix(LABELSUPP_STR, sizeof(LABELSUPP_STR)-1, option, len));
> > +             match_prefix(LABELSUPP_STR, sizeof(LABELSUPP_STR)-1, option, len) ||
> > +             match_prefix(LSM_CUSTOM_ARG_STR, sizeof(LSM_CUSTOM_ARG_STR)-1, option, len));
> >  }
> >
> >  static inline void take_option(char **to, char *from, int *first, int len)
> > diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h
> > index 23e762d529fa..0ead836a0625 100644
> > --- a/security/selinux/include/security.h
> > +++ b/security/selinux/include/security.h
> > @@ -59,11 +59,12 @@
> >  #define SE_SBPROC            0x0200
> >  #define SE_SBGENFS           0x0400
> >
> > -#define CONTEXT_STR  "context="
> > -#define FSCONTEXT_STR        "fscontext="
> > -#define ROOTCONTEXT_STR      "rootcontext="
> > -#define DEFCONTEXT_STR       "defcontext="
> > -#define LABELSUPP_STR "seclabel"
> > +#define CONTEXT_STR         "context="
> > +#define FSCONTEXT_STR       "fscontext="
> > +#define ROOTCONTEXT_STR     "rootcontext="
> > +#define DEFCONTEXT_STR      "defcontext="
> > +#define LABELSUPP_STR       "seclabel"
> > +#define LSM_CUSTOM_ARG_STR  "lsm_custom_arg="
> >
> >  struct netlbl_lsm_secattr;
> >
>

^ permalink raw reply	[flat|nested] 11+ messages in thread
* [PATCH] SELinux: allow other LSMs to use custom mount args
@ 2018-08-29 21:44     ` Micah Morton
  0 siblings, 0 replies; 11+ messages in thread
From: Micah Morton @ 2018-08-29 21:44 UTC (permalink / raw)
  To: linux-security-module

So are you saying that since another security module (i.e. Smack) has
this issue, SELinux ought not to fix this issue? I agree it would be
optimal to solve this problem in the general sense, but I don't see
why it shouldn't be solved for SELinux in the mean time. Seems like
that would give even more motivation to solve the problem in the
general sense down the road. I'm not sure which patch your talking
about, although I'm familiar with this post
https://lwn.net/Articles/635771/ and the linked patch:
https://lwn.net/Articles/636056/. Is there a more recent version since
2015?
On Wed, Aug 29, 2018 at 9:18 AM Casey Schaufler <casey@schaufler-ca.com> wrote:
>
> On 8/28/2018 2:32 PM, Micah Morton wrote:
> > The security_sb_copy_data LSM hook allows LSMs to copy custom string
> > name/value args passed to mount_fs() into a temporary buffer (called
> > "secdata") that will be accessible to LSM code during the
> > security_sb_kern_mount hook further down in mount_fs(). Currently,
> > SELinux effectively prevents any other LSMs from copying custom mount
> > args into the temporary buffer (and being able to access them during
> > security_sb_kern_mount), as it will fail with -EINVAL and print
> > "SELinux:  unknown mount option" to the kernel message buffer if args it
> > doesn't recognize are present in the temporary buffer when
> > selinux_sb_kern_mount is called. This change adds an arg to the list of
> > those accepted by SELinux during security_sb_kern_mount. SELinux won't
> > do anything with this arg besides allow the name/value pair to be passed
> > along to any other LSM that is stacked after SELinux.
> >
> > Developed on v4.18.
> >
> > Signed-off-by: Micah Morton <mortonm@chromium.org>
>
> This needs to be generalized. Whatever security module you're
> working on won't stack with any other module that uses mount
> options, Smack in particular. Have you looked at the patch I've
> been proposing as part of the general stacking work?
>
> > ---
> >  security/selinux/hooks.c            |  7 ++++++-
> >  security/selinux/include/security.h | 11 ++++++-----
> >  2 files changed, 12 insertions(+), 6 deletions(-)
> >
> > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> > index 2b5ee5fbd652..e70ccc701eb8 100644
> > --- a/security/selinux/hooks.c
> > +++ b/security/selinux/hooks.c
> > @@ -445,6 +445,7 @@ enum {
> >       Opt_rootcontext = 4,
> >       Opt_labelsupport = 5,
> >       Opt_nextmntopt = 6,
> > +     Opt_lsm_custom_arg = 7,
> >  };
> >
> >  #define NUM_SEL_MNT_OPTS     (Opt_nextmntopt - 1)
> > @@ -455,6 +456,7 @@ static const match_table_t tokens = {
> >       {Opt_defcontext, DEFCONTEXT_STR "%s"},
> >       {Opt_rootcontext, ROOTCONTEXT_STR "%s"},
> >       {Opt_labelsupport, LABELSUPP_STR},
> > +     {Opt_lsm_custom_arg, LSM_CUSTOM_ARG_STR "%s"},
> >       {Opt_error, NULL},
> >  };
> >
> > @@ -1156,6 +1158,8 @@ static int selinux_parse_opts_str(char *options,
> >                       break;
> >               case Opt_labelsupport:
> >                       break;
> > +             case Opt_lsm_custom_arg:
> > +                     break;
> >               default:
> >                       rc = -EINVAL;
> >                       printk(KERN_WARNING "SELinux:  unknown mount option\n");
> > @@ -2758,7 +2762,8 @@ static inline int selinux_option(char *option, int len)
> >               match_prefix(FSCONTEXT_STR, sizeof(FSCONTEXT_STR)-1, option, len) ||
> >               match_prefix(DEFCONTEXT_STR, sizeof(DEFCONTEXT_STR)-1, option, len) ||
> >               match_prefix(ROOTCONTEXT_STR, sizeof(ROOTCONTEXT_STR)-1, option, len) ||
> > -             match_prefix(LABELSUPP_STR, sizeof(LABELSUPP_STR)-1, option, len));
> > +             match_prefix(LABELSUPP_STR, sizeof(LABELSUPP_STR)-1, option, len) ||
> > +             match_prefix(LSM_CUSTOM_ARG_STR, sizeof(LSM_CUSTOM_ARG_STR)-1, option, len));
> >  }
> >
> >  static inline void take_option(char **to, char *from, int *first, int len)
> > diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h
> > index 23e762d529fa..0ead836a0625 100644
> > --- a/security/selinux/include/security.h
> > +++ b/security/selinux/include/security.h
> > @@ -59,11 +59,12 @@
> >  #define SE_SBPROC            0x0200
> >  #define SE_SBGENFS           0x0400
> >
> > -#define CONTEXT_STR  "context="
> > -#define FSCONTEXT_STR        "fscontext="
> > -#define ROOTCONTEXT_STR      "rootcontext="
> > -#define DEFCONTEXT_STR       "defcontext="
> > -#define LABELSUPP_STR "seclabel"
> > +#define CONTEXT_STR         "context="
> > +#define FSCONTEXT_STR       "fscontext="
> > +#define ROOTCONTEXT_STR     "rootcontext="
> > +#define DEFCONTEXT_STR      "defcontext="
> > +#define LABELSUPP_STR       "seclabel"
> > +#define LSM_CUSTOM_ARG_STR  "lsm_custom_arg="
> >
> >  struct netlbl_lsm_secattr;
> >
>

^ permalink raw reply	[flat|nested] 11+ messages in thread
* Re: [PATCH] SELinux: allow other LSMs to use custom mount args
  2018-08-29 21:44     ` Micah Morton
@ 2018-08-29 22:14       ` Casey Schaufler
  -1 siblings, 0 replies; 11+ messages in thread
From: Casey Schaufler @ 2018-08-29 22:14 UTC (permalink / raw)
  To: Micah Morton; +Cc: linux-security-module, selinux, Casey Schaufler

On 8/29/2018 2:44 PM, Micah Morton wrote:
> So are you saying that since another security module (i.e. Smack) has
> this issue, SELinux ought not to fix this issue?

Pretty much. More importantly, I'm saying that if you're going to
introduce a new security module that uses mount options, and you expect
this module to work with other modules (or even one other module) that
the problem is not with SELinux, it's with the infrastructure. A one
time hack to SELinux is not the right answer.

> I agree it would be
> optimal to solve this problem in the general sense, but I don't see
> why it shouldn't be solved for SELinux in the mean time.

Because that is a hack, not a solution. It won't work if you add a
third module with mount options to your stack.

> Seems like
> that would give even more motivation to solve the problem in the
> general sense down the road.

I've been posting a general mechanism here for at least the past year,
and will be posting another revision shortly after James updates the
security-next tree.

> I'm not sure which patch your talking
> about, although I'm familiar with this post
> https://lwn.net/Articles/635771/ and the linked patch:
> https://lwn.net/Articles/636056/. Is there a more recent version since
> 2015?

Yes.

> On Wed, Aug 29, 2018 at 9:18 AM Casey Schaufler <casey@schaufler-ca.com> wrote:
>> On 8/28/2018 2:32 PM, Micah Morton wrote:
>>> The security_sb_copy_data LSM hook allows LSMs to copy custom string
>>> name/value args passed to mount_fs() into a temporary buffer (called
>>> "secdata") that will be accessible to LSM code during the
>>> security_sb_kern_mount hook further down in mount_fs(). Currently,
>>> SELinux effectively prevents any other LSMs from copying custom mount
>>> args into the temporary buffer (and being able to access them during
>>> security_sb_kern_mount), as it will fail with -EINVAL and print
>>> "SELinux:  unknown mount option" to the kernel message buffer if args it
>>> doesn't recognize are present in the temporary buffer when
>>> selinux_sb_kern_mount is called. This change adds an arg to the list of
>>> those accepted by SELinux during security_sb_kern_mount. SELinux won't
>>> do anything with this arg besides allow the name/value pair to be passed
>>> along to any other LSM that is stacked after SELinux.
>>>
>>> Developed on v4.18.
>>>
>>> Signed-off-by: Micah Morton <mortonm@chromium.org>
>> This needs to be generalized. Whatever security module you're
>> working on won't stack with any other module that uses mount
>> options, Smack in particular. Have you looked at the patch I've
>> been proposing as part of the general stacking work?
>>
>>> ---
>>>  security/selinux/hooks.c            |  7 ++++++-
>>>  security/selinux/include/security.h | 11 ++++++-----
>>>  2 files changed, 12 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
>>> index 2b5ee5fbd652..e70ccc701eb8 100644
>>> --- a/security/selinux/hooks.c
>>> +++ b/security/selinux/hooks.c
>>> @@ -445,6 +445,7 @@ enum {
>>>       Opt_rootcontext = 4,
>>>       Opt_labelsupport = 5,
>>>       Opt_nextmntopt = 6,
>>> +     Opt_lsm_custom_arg = 7,
>>>  };
>>>
>>>  #define NUM_SEL_MNT_OPTS     (Opt_nextmntopt - 1)
>>> @@ -455,6 +456,7 @@ static const match_table_t tokens = {
>>>       {Opt_defcontext, DEFCONTEXT_STR "%s"},
>>>       {Opt_rootcontext, ROOTCONTEXT_STR "%s"},
>>>       {Opt_labelsupport, LABELSUPP_STR},
>>> +     {Opt_lsm_custom_arg, LSM_CUSTOM_ARG_STR "%s"},
>>>       {Opt_error, NULL},
>>>  };
>>>
>>> @@ -1156,6 +1158,8 @@ static int selinux_parse_opts_str(char *options,
>>>                       break;
>>>               case Opt_labelsupport:
>>>                       break;
>>> +             case Opt_lsm_custom_arg:
>>> +                     break;
>>>               default:
>>>                       rc = -EINVAL;
>>>                       printk(KERN_WARNING "SELinux:  unknown mount option\n");
>>> @@ -2758,7 +2762,8 @@ static inline int selinux_option(char *option, int len)
>>>               match_prefix(FSCONTEXT_STR, sizeof(FSCONTEXT_STR)-1, option, len) ||
>>>               match_prefix(DEFCONTEXT_STR, sizeof(DEFCONTEXT_STR)-1, option, len) ||
>>>               match_prefix(ROOTCONTEXT_STR, sizeof(ROOTCONTEXT_STR)-1, option, len) ||
>>> -             match_prefix(LABELSUPP_STR, sizeof(LABELSUPP_STR)-1, option, len));
>>> +             match_prefix(LABELSUPP_STR, sizeof(LABELSUPP_STR)-1, option, len) ||
>>> +             match_prefix(LSM_CUSTOM_ARG_STR, sizeof(LSM_CUSTOM_ARG_STR)-1, option, len));
>>>  }
>>>
>>>  static inline void take_option(char **to, char *from, int *first, int len)
>>> diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h
>>> index 23e762d529fa..0ead836a0625 100644
>>> --- a/security/selinux/include/security.h
>>> +++ b/security/selinux/include/security.h
>>> @@ -59,11 +59,12 @@
>>>  #define SE_SBPROC            0x0200
>>>  #define SE_SBGENFS           0x0400
>>>
>>> -#define CONTEXT_STR  "context="
>>> -#define FSCONTEXT_STR        "fscontext="
>>> -#define ROOTCONTEXT_STR      "rootcontext="
>>> -#define DEFCONTEXT_STR       "defcontext="
>>> -#define LABELSUPP_STR "seclabel"
>>> +#define CONTEXT_STR         "context="
>>> +#define FSCONTEXT_STR       "fscontext="
>>> +#define ROOTCONTEXT_STR     "rootcontext="
>>> +#define DEFCONTEXT_STR      "defcontext="
>>> +#define LABELSUPP_STR       "seclabel"
>>> +#define LSM_CUSTOM_ARG_STR  "lsm_custom_arg="
>>>
>>>  struct netlbl_lsm_secattr;
>>>

^ permalink raw reply	[flat|nested] 11+ messages in thread
* [PATCH] SELinux: allow other LSMs to use custom mount args
@ 2018-08-29 22:14       ` Casey Schaufler
  0 siblings, 0 replies; 11+ messages in thread
From: Casey Schaufler @ 2018-08-29 22:14 UTC (permalink / raw)
  To: linux-security-module

On 8/29/2018 2:44 PM, Micah Morton wrote:
> So are you saying that since another security module (i.e. Smack) has
> this issue, SELinux ought not to fix this issue?

Pretty much. More importantly, I'm saying that if you're going to
introduce a new security module that uses mount options, and you expect
this module to work with other modules (or even one other module) that
the problem is not with SELinux, it's with the infrastructure. A one
time hack to SELinux is not the right answer.

> I agree it would be
> optimal to solve this problem in the general sense, but I don't see
> why it shouldn't be solved for SELinux in the mean time.

Because that is a hack, not a solution. It won't work if you add a
third module with mount options to your stack.

> Seems like
> that would give even more motivation to solve the problem in the
> general sense down the road.

I've been posting a general mechanism here for at least the past year,
and will be posting another revision shortly after James updates the
security-next tree.

> I'm not sure which patch your talking
> about, although I'm familiar with this post
> https://lwn.net/Articles/635771/ and the linked patch:
> https://lwn.net/Articles/636056/. Is there a more recent version since
> 2015?

Yes.

> On Wed, Aug 29, 2018 at 9:18 AM Casey Schaufler <casey@schaufler-ca.com> wrote:
>> On 8/28/2018 2:32 PM, Micah Morton wrote:
>>> The security_sb_copy_data LSM hook allows LSMs to copy custom string
>>> name/value args passed to mount_fs() into a temporary buffer (called
>>> "secdata") that will be accessible to LSM code during the
>>> security_sb_kern_mount hook further down in mount_fs(). Currently,
>>> SELinux effectively prevents any other LSMs from copying custom mount
>>> args into the temporary buffer (and being able to access them during
>>> security_sb_kern_mount), as it will fail with -EINVAL and print
>>> "SELinux:  unknown mount option" to the kernel message buffer if args it
>>> doesn't recognize are present in the temporary buffer when
>>> selinux_sb_kern_mount is called. This change adds an arg to the list of
>>> those accepted by SELinux during security_sb_kern_mount. SELinux won't
>>> do anything with this arg besides allow the name/value pair to be passed
>>> along to any other LSM that is stacked after SELinux.
>>>
>>> Developed on v4.18.
>>>
>>> Signed-off-by: Micah Morton <mortonm@chromium.org>
>> This needs to be generalized. Whatever security module you're
>> working on won't stack with any other module that uses mount
>> options, Smack in particular. Have you looked at the patch I've
>> been proposing as part of the general stacking work?
>>
>>> ---
>>>  security/selinux/hooks.c            |  7 ++++++-
>>>  security/selinux/include/security.h | 11 ++++++-----
>>>  2 files changed, 12 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
>>> index 2b5ee5fbd652..e70ccc701eb8 100644
>>> --- a/security/selinux/hooks.c
>>> +++ b/security/selinux/hooks.c
>>> @@ -445,6 +445,7 @@ enum {
>>>       Opt_rootcontext = 4,
>>>       Opt_labelsupport = 5,
>>>       Opt_nextmntopt = 6,
>>> +     Opt_lsm_custom_arg = 7,
>>>  };
>>>
>>>  #define NUM_SEL_MNT_OPTS     (Opt_nextmntopt - 1)
>>> @@ -455,6 +456,7 @@ static const match_table_t tokens = {
>>>       {Opt_defcontext, DEFCONTEXT_STR "%s"},
>>>       {Opt_rootcontext, ROOTCONTEXT_STR "%s"},
>>>       {Opt_labelsupport, LABELSUPP_STR},
>>> +     {Opt_lsm_custom_arg, LSM_CUSTOM_ARG_STR "%s"},
>>>       {Opt_error, NULL},
>>>  };
>>>
>>> @@ -1156,6 +1158,8 @@ static int selinux_parse_opts_str(char *options,
>>>                       break;
>>>               case Opt_labelsupport:
>>>                       break;
>>> +             case Opt_lsm_custom_arg:
>>> +                     break;
>>>               default:
>>>                       rc = -EINVAL;
>>>                       printk(KERN_WARNING "SELinux:  unknown mount option\n");
>>> @@ -2758,7 +2762,8 @@ static inline int selinux_option(char *option, int len)
>>>               match_prefix(FSCONTEXT_STR, sizeof(FSCONTEXT_STR)-1, option, len) ||
>>>               match_prefix(DEFCONTEXT_STR, sizeof(DEFCONTEXT_STR)-1, option, len) ||
>>>               match_prefix(ROOTCONTEXT_STR, sizeof(ROOTCONTEXT_STR)-1, option, len) ||
>>> -             match_prefix(LABELSUPP_STR, sizeof(LABELSUPP_STR)-1, option, len));
>>> +             match_prefix(LABELSUPP_STR, sizeof(LABELSUPP_STR)-1, option, len) ||
>>> +             match_prefix(LSM_CUSTOM_ARG_STR, sizeof(LSM_CUSTOM_ARG_STR)-1, option, len));
>>>  }
>>>
>>>  static inline void take_option(char **to, char *from, int *first, int len)
>>> diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h
>>> index 23e762d529fa..0ead836a0625 100644
>>> --- a/security/selinux/include/security.h
>>> +++ b/security/selinux/include/security.h
>>> @@ -59,11 +59,12 @@
>>>  #define SE_SBPROC            0x0200
>>>  #define SE_SBGENFS           0x0400
>>>
>>> -#define CONTEXT_STR  "context="
>>> -#define FSCONTEXT_STR        "fscontext="
>>> -#define ROOTCONTEXT_STR      "rootcontext="
>>> -#define DEFCONTEXT_STR       "defcontext="
>>> -#define LABELSUPP_STR "seclabel"
>>> +#define CONTEXT_STR         "context="
>>> +#define FSCONTEXT_STR       "fscontext="
>>> +#define ROOTCONTEXT_STR     "rootcontext="
>>> +#define DEFCONTEXT_STR      "defcontext="
>>> +#define LABELSUPP_STR       "seclabel"
>>> +#define LSM_CUSTOM_ARG_STR  "lsm_custom_arg="
>>>
>>>  struct netlbl_lsm_secattr;
>>>

^ permalink raw reply	[flat|nested] 11+ messages in thread
* Re: [PATCH] SELinux: allow other LSMs to use custom mount args
  2018-08-29  4:58   ` Paul Moore
@ 2018-08-31 17:11     ` Stephen Smalley
  -1 siblings, 0 replies; 11+ messages in thread
From: Stephen Smalley @ 2018-08-31 17:11 UTC (permalink / raw)
  To: Paul Moore, mortonm; +Cc: linux-security-module, selinux

On 08/29/2018 12:58 AM, Paul Moore wrote:
> On Tue, Aug 28, 2018 at 5:32 PM Micah Morton <mortonm@chromium.org> wrote:
>> The security_sb_copy_data LSM hook allows LSMs to copy custom string
>> name/value args passed to mount_fs() into a temporary buffer (called
>> "secdata") that will be accessible to LSM code during the
>> security_sb_kern_mount hook further down in mount_fs(). Currently,
>> SELinux effectively prevents any other LSMs from copying custom mount
>> args into the temporary buffer (and being able to access them during
>> security_sb_kern_mount), as it will fail with -EINVAL and print
>> "SELinux:  unknown mount option" to the kernel message buffer if args it
>> doesn't recognize are present in the temporary buffer when
>> selinux_sb_kern_mount is called. This change adds an arg to the list of
>> those accepted by SELinux during security_sb_kern_mount. SELinux won't
>> do anything with this arg besides allow the name/value pair to be passed
>> along to any other LSM that is stacked after SELinux.
>>
>> Developed on v4.18.
>>
>> Signed-off-by: Micah Morton <mortonm@chromium.org>
>> ---
>>   security/selinux/hooks.c            |  7 ++++++-
>>   security/selinux/include/security.h | 11 ++++++-----
>>   2 files changed, 12 insertions(+), 6 deletions(-)
> 
> SELinux patches need to be sent to the SELinux mailing list (CC'd) for
> proper review.

Please also show us the user of this facility; we need to see both sides 
of the interface to fully assess it.  And that user has to be on a glide 
path to mainline; we don't add features for out-of-tree code.

WRT Casey's comments, I don't think that you necessarily have to deal 
with arbitrary stacking for this patch since the stacking support is not 
yet upstream, but it wouldn't hurt to consider whether you could solve 
the problem more generally.

> 
>> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
>> index 2b5ee5fbd652..e70ccc701eb8 100644
>> --- a/security/selinux/hooks.c
>> +++ b/security/selinux/hooks.c
>> @@ -445,6 +445,7 @@ enum {
>>          Opt_rootcontext = 4,
>>          Opt_labelsupport = 5,
>>          Opt_nextmntopt = 6,
>> +       Opt_lsm_custom_arg = 7,
>>   };
>>
>>   #define NUM_SEL_MNT_OPTS       (Opt_nextmntopt - 1)
>> @@ -455,6 +456,7 @@ static const match_table_t tokens = {
>>          {Opt_defcontext, DEFCONTEXT_STR "%s"},
>>          {Opt_rootcontext, ROOTCONTEXT_STR "%s"},
>>          {Opt_labelsupport, LABELSUPP_STR},
>> +       {Opt_lsm_custom_arg, LSM_CUSTOM_ARG_STR "%s"},
>>          {Opt_error, NULL},
>>   };
>>
>> @@ -1156,6 +1158,8 @@ static int selinux_parse_opts_str(char *options,
>>                          break;
>>                  case Opt_labelsupport:
>>                          break;
>> +               case Opt_lsm_custom_arg:
>> +                       break;
>>                  default:
>>                          rc = -EINVAL;
>>                          printk(KERN_WARNING "SELinux:  unknown mount option\n");
>> @@ -2758,7 +2762,8 @@ static inline int selinux_option(char *option, int len)
>>                  match_prefix(FSCONTEXT_STR, sizeof(FSCONTEXT_STR)-1, option, len) ||
>>                  match_prefix(DEFCONTEXT_STR, sizeof(DEFCONTEXT_STR)-1, option, len) ||
>>                  match_prefix(ROOTCONTEXT_STR, sizeof(ROOTCONTEXT_STR)-1, option, len) ||
>> -               match_prefix(LABELSUPP_STR, sizeof(LABELSUPP_STR)-1, option, len));
>> +               match_prefix(LABELSUPP_STR, sizeof(LABELSUPP_STR)-1, option, len) ||
>> +               match_prefix(LSM_CUSTOM_ARG_STR, sizeof(LSM_CUSTOM_ARG_STR)-1, option, len));
>>   }
>>
>>   static inline void take_option(char **to, char *from, int *first, int len)
>> diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h
>> index 23e762d529fa..0ead836a0625 100644
>> --- a/security/selinux/include/security.h
>> +++ b/security/selinux/include/security.h
>> @@ -59,11 +59,12 @@
>>   #define SE_SBPROC              0x0200
>>   #define SE_SBGENFS             0x0400
>>
>> -#define CONTEXT_STR    "context="
>> -#define FSCONTEXT_STR  "fscontext="
>> -#define ROOTCONTEXT_STR        "rootcontext="
>> -#define DEFCONTEXT_STR "defcontext="
>> -#define LABELSUPP_STR "seclabel"
>> +#define CONTEXT_STR         "context="
>> +#define FSCONTEXT_STR       "fscontext="
>> +#define ROOTCONTEXT_STR     "rootcontext="
>> +#define DEFCONTEXT_STR      "defcontext="
>> +#define LABELSUPP_STR       "seclabel"
>> +#define LSM_CUSTOM_ARG_STR  "lsm_custom_arg="
>>
>>   struct netlbl_lsm_secattr;
>>
>> --
>> 2.19.0.rc0.228.g281dcd1b4d0-goog
>>
> 
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread
* [PATCH] SELinux: allow other LSMs to use custom mount args
@ 2018-08-31 17:11     ` Stephen Smalley
  0 siblings, 0 replies; 11+ messages in thread
From: Stephen Smalley @ 2018-08-31 17:11 UTC (permalink / raw)
  To: linux-security-module

On 08/29/2018 12:58 AM, Paul Moore wrote:
> On Tue, Aug 28, 2018 at 5:32 PM Micah Morton <mortonm@chromium.org> wrote:
>> The security_sb_copy_data LSM hook allows LSMs to copy custom string
>> name/value args passed to mount_fs() into a temporary buffer (called
>> "secdata") that will be accessible to LSM code during the
>> security_sb_kern_mount hook further down in mount_fs(). Currently,
>> SELinux effectively prevents any other LSMs from copying custom mount
>> args into the temporary buffer (and being able to access them during
>> security_sb_kern_mount), as it will fail with -EINVAL and print
>> "SELinux:  unknown mount option" to the kernel message buffer if args it
>> doesn't recognize are present in the temporary buffer when
>> selinux_sb_kern_mount is called. This change adds an arg to the list of
>> those accepted by SELinux during security_sb_kern_mount. SELinux won't
>> do anything with this arg besides allow the name/value pair to be passed
>> along to any other LSM that is stacked after SELinux.
>>
>> Developed on v4.18.
>>
>> Signed-off-by: Micah Morton <mortonm@chromium.org>
>> ---
>>   security/selinux/hooks.c            |  7 ++++++-
>>   security/selinux/include/security.h | 11 ++++++-----
>>   2 files changed, 12 insertions(+), 6 deletions(-)
> 
> SELinux patches need to be sent to the SELinux mailing list (CC'd) for
> proper review.

Please also show us the user of this facility; we need to see both sides 
of the interface to fully assess it.  And that user has to be on a glide 
path to mainline; we don't add features for out-of-tree code.

WRT Casey's comments, I don't think that you necessarily have to deal 
with arbitrary stacking for this patch since the stacking support is not 
yet upstream, but it wouldn't hurt to consider whether you could solve 
the problem more generally.

> 
>> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
>> index 2b5ee5fbd652..e70ccc701eb8 100644
>> --- a/security/selinux/hooks.c
>> +++ b/security/selinux/hooks.c
>> @@ -445,6 +445,7 @@ enum {
>>          Opt_rootcontext = 4,
>>          Opt_labelsupport = 5,
>>          Opt_nextmntopt = 6,
>> +       Opt_lsm_custom_arg = 7,
>>   };
>>
>>   #define NUM_SEL_MNT_OPTS       (Opt_nextmntopt - 1)
>> @@ -455,6 +456,7 @@ static const match_table_t tokens = {
>>          {Opt_defcontext, DEFCONTEXT_STR "%s"},
>>          {Opt_rootcontext, ROOTCONTEXT_STR "%s"},
>>          {Opt_labelsupport, LABELSUPP_STR},
>> +       {Opt_lsm_custom_arg, LSM_CUSTOM_ARG_STR "%s"},
>>          {Opt_error, NULL},
>>   };
>>
>> @@ -1156,6 +1158,8 @@ static int selinux_parse_opts_str(char *options,
>>                          break;
>>                  case Opt_labelsupport:
>>                          break;
>> +               case Opt_lsm_custom_arg:
>> +                       break;
>>                  default:
>>                          rc = -EINVAL;
>>                          printk(KERN_WARNING "SELinux:  unknown mount option\n");
>> @@ -2758,7 +2762,8 @@ static inline int selinux_option(char *option, int len)
>>                  match_prefix(FSCONTEXT_STR, sizeof(FSCONTEXT_STR)-1, option, len) ||
>>                  match_prefix(DEFCONTEXT_STR, sizeof(DEFCONTEXT_STR)-1, option, len) ||
>>                  match_prefix(ROOTCONTEXT_STR, sizeof(ROOTCONTEXT_STR)-1, option, len) ||
>> -               match_prefix(LABELSUPP_STR, sizeof(LABELSUPP_STR)-1, option, len));
>> +               match_prefix(LABELSUPP_STR, sizeof(LABELSUPP_STR)-1, option, len) ||
>> +               match_prefix(LSM_CUSTOM_ARG_STR, sizeof(LSM_CUSTOM_ARG_STR)-1, option, len));
>>   }
>>
>>   static inline void take_option(char **to, char *from, int *first, int len)
>> diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h
>> index 23e762d529fa..0ead836a0625 100644
>> --- a/security/selinux/include/security.h
>> +++ b/security/selinux/include/security.h
>> @@ -59,11 +59,12 @@
>>   #define SE_SBPROC              0x0200
>>   #define SE_SBGENFS             0x0400
>>
>> -#define CONTEXT_STR    "context="
>> -#define FSCONTEXT_STR  "fscontext="
>> -#define ROOTCONTEXT_STR        "rootcontext="
>> -#define DEFCONTEXT_STR "defcontext="
>> -#define LABELSUPP_STR "seclabel"
>> +#define CONTEXT_STR         "context="
>> +#define FSCONTEXT_STR       "fscontext="
>> +#define ROOTCONTEXT_STR     "rootcontext="
>> +#define DEFCONTEXT_STR      "defcontext="
>> +#define LABELSUPP_STR       "seclabel"
>> +#define LSM_CUSTOM_ARG_STR  "lsm_custom_arg="
>>
>>   struct netlbl_lsm_secattr;
>>
>> --
>> 2.19.0.rc0.228.g281dcd1b4d0-goog
>>
> 
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread
end of thread, other threads:[~2018-08-31 17:11 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-08-28 21:32 [PATCH] SELinux: allow other LSMs to use custom mount args Micah Morton
2018-08-29  4:58 ` Paul Moore
2018-08-29  4:58   ` Paul Moore
2018-08-31 17:11   ` Stephen Smalley
2018-08-31 17:11     ` Stephen Smalley
2018-08-29 16:14 ` Casey Schaufler
2018-08-29 16:14   ` Casey Schaufler
2018-08-29 21:44   ` Micah Morton
2018-08-29 21:44     ` Micah Morton
2018-08-29 22:14     ` Casey Schaufler
2018-08-29 22:14       ` Casey Schaufler

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.