[PATCH] kernel: Conditionally support non-root users, groups and capabilities

[PATCH] kernel: Conditionally support non-root users, groups and capabilities

[Iulia Manda]

There are a lot of embedded systems that run most or all of their functionality
in init, running as root:root. For these systems, supporting multiple users is
not necessary.

This patch adds a new symbol, CONFIG_NON_ROOT, that makes support for non-root
users, non-root groups, and capabilities optional.

When this symbol is not defined, UID and GID are zero in any possible case
and processes always have all capabilities.

Also, the following syscalls are compiled out: setuid, setregid, setgid,
setreuid, setresuid, getresuid, setresgid, getresgid, setgroups, getgroups,
setfsuid, setfsgid, capget, capset.

This change saves about 25 KB on a defconfig build.

Bloat-o-meter output:
add/remove: 7/66 grow/shrink: 21/421 up/down: 1701/-27172 (-25471)

Signed-off-by: Iulia Manda <iulia.manda21@gmail.com>
Reviewed-by: Josh Triplett <josh@joshtriplett.org>
---
 include/linux/capability.h |   12 ++++++++++++
 include/linux/uidgid.h     |   12 ++++++++++++
 init/Kconfig               |   19 ++++++++++++++++++-
 kernel/capability.c        |    6 ++++++
 kernel/groups.c            |    4 ++++
 kernel/sys.c               |    2 ++
 kernel/sys_ni.c            |   14 ++++++++++++++
 7 files changed, 68 insertions(+), 1 deletion(-)

diff --git a/include/linux/capability.h b/include/linux/capability.h
index aa93e5e..d8791d2 100644
--- a/include/linux/capability.h
+++ b/include/linux/capability.h
@@ -211,8 +211,20 @@ extern bool has_ns_capability(struct task_struct *t,
 extern bool has_capability_noaudit(struct task_struct *t, int cap);
 extern bool has_ns_capability_noaudit(struct task_struct *t,
 				      struct user_namespace *ns, int cap);
+#ifdef CONFIG_NON_ROOT
 extern bool capable(int cap);
 extern bool ns_capable(struct user_namespace *ns, int cap);
+#else
+static inline bool capable(int cap)
+{
+	return true;
+}
+
+static inline bool ns_capable(struct user_namespace *ns, int cap)
+{
+	return true;
+}
+#endif /* CONFIG_NON_ROOT */
 extern bool capable_wrt_inode_uidgid(const struct inode *inode, int cap);
 extern bool file_ns_capable(const struct file *file, struct user_namespace *ns, int cap);
 
diff --git a/include/linux/uidgid.h b/include/linux/uidgid.h
index 2d1f9b6..22bd1fa 100644
--- a/include/linux/uidgid.h
+++ b/include/linux/uidgid.h
@@ -29,6 +29,7 @@ typedef struct {
 #define KUIDT_INIT(value) (kuid_t){ value }
 #define KGIDT_INIT(value) (kgid_t){ value }
 
+#ifdef CONFIG_NON_ROOT
 static inline uid_t __kuid_val(kuid_t uid)
 {
 	return uid.val;
@@ -38,6 +39,17 @@ static inline gid_t __kgid_val(kgid_t gid)
 {
 	return gid.val;
 }
+#else
+static inline uid_t __kuid_val(kuid_t uid)
+{
+	return 0;
+}
+
+static inline gid_t __kgid_val(kgid_t gid)
+{
+	return 0;
+}
+#endif
 
 #define GLOBAL_ROOT_UID KUIDT_INIT(0)
 #define GLOBAL_ROOT_GID KGIDT_INIT(0)
diff --git a/init/Kconfig b/init/Kconfig
index 9afb971..dc5bfd4 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -394,6 +394,7 @@ endchoice
 
 config BSD_PROCESS_ACCT
 	bool "BSD Process Accounting"
+	select NON_ROOT
 	help
 	  If you say Y here, a user level program will be able to instruct the
 	  kernel (via a special system call) to write process accounting
@@ -420,6 +421,7 @@ config BSD_PROCESS_ACCT_V3
 config TASKSTATS
 	bool "Export task/process statistics through netlink"
 	depends on NET
+	select NON_ROOT
 	default n
 	help
 	  Export selected statistics for tasks/processes through the
@@ -1140,6 +1142,7 @@ config CHECKPOINT_RESTORE
 
 menuconfig NAMESPACES
 	bool "Namespaces support" if EXPERT
+	depends on NON_ROOT
 	default !EXPERT
 	help
 	  Provides the way to make tasks work with different objects using
@@ -1352,11 +1355,25 @@ menuconfig EXPERT
 
 config UID16
 	bool "Enable 16-bit UID system calls" if EXPERT
-	depends on HAVE_UID16
+	depends on HAVE_UID16 && NON_ROOT
 	default y
 	help
 	  This enables the legacy 16-bit UID syscall wrappers.
 
+config NON_ROOT
+	bool "Multiple users, groups and capabilities support" if EXPERT
+	default y
+	help
+	  This option enables support for non-root users, groups and
+	  capabilities.
+
+	  If you say N here, all processes will run with UID 0, GID 0, and all
+	  possible capabilities.  Saying N here also compiles out support for
+	  system calls related to UIDs, GIDs, and capabilities, such as setuid,
+	  setgid, and capset.
+
+	  If unsure, say Y here.
+
 config SGETMASK_SYSCALL
 	bool "sgetmask/ssetmask syscalls support" if EXPERT
 	def_bool PARISC || MN10300 || BLACKFIN || M68K || PPC || MIPS || X86 || SPARC || CRIS || MICROBLAZE || SUPERH
diff --git a/kernel/capability.c b/kernel/capability.c
index 989f5bf..bead84a 100644
--- a/kernel/capability.c
+++ b/kernel/capability.c
@@ -35,6 +35,7 @@ static int __init file_caps_disable(char *str)
 }
 __setup("no_file_caps", file_caps_disable);
 
+#ifdef CONFIG_NON_ROOT
 /*
  * More recent versions of libcap are available from:
  *
@@ -279,6 +280,7 @@ error:
 	abort_creds(new);
 	return ret;
 }
+#endif
 
 /**
  * has_ns_capability - Does a task have a capability in a specific user ns
@@ -360,6 +362,7 @@ bool has_capability_noaudit(struct task_struct *t, int cap)
 	return has_ns_capability_noaudit(t, &init_user_ns, cap);
 }
 
+#ifdef CONFIG_NON_ROOT
 /**
  * ns_capable - Determine if the current task has a superior capability in effect
  * @ns:  The usernamespace we want the capability in
@@ -385,6 +388,7 @@ bool ns_capable(struct user_namespace *ns, int cap)
 	return false;
 }
 EXPORT_SYMBOL(ns_capable);
+#endif
 
 /**
  * file_ns_capable - Determine if the file's opener had a capability in effect
@@ -411,6 +415,7 @@ bool file_ns_capable(const struct file *file, struct user_namespace *ns,
 }
 EXPORT_SYMBOL(file_ns_capable);
 
+#ifdef CONFIG_NON_ROOT
 /**
  * capable - Determine if the current task has a superior capability in effect
  * @cap: The capability to be tested for
@@ -426,6 +431,7 @@ bool capable(int cap)
 	return ns_capable(&init_user_ns, cap);
 }
 EXPORT_SYMBOL(capable);
+#endif
 
 /**
  * capable_wrt_inode_uidgid - Check nsown_capable and uid and gid mapped
diff --git a/kernel/groups.c b/kernel/groups.c
index 664411f..94f2c89 100644
--- a/kernel/groups.c
+++ b/kernel/groups.c
@@ -190,6 +190,7 @@ int set_current_groups(struct group_info *group_info)
 
 EXPORT_SYMBOL(set_current_groups);
 
+#ifdef CONFIG_NON_ROOT
 SYSCALL_DEFINE2(getgroups, int, gidsetsize, gid_t __user *, grouplist)
 {
 	const struct cred *cred = current_cred();
@@ -213,6 +214,7 @@ SYSCALL_DEFINE2(getgroups, int, gidsetsize, gid_t __user *, grouplist)
 out:
 	return i;
 }
+#endif
 
 bool may_setgroups(void)
 {
@@ -227,6 +229,7 @@ bool may_setgroups(void)
  *	without another task interfering.
  */
 
+#ifdef CONFIG_NON_ROOT
 SYSCALL_DEFINE2(setgroups, int, gidsetsize, gid_t __user *, grouplist)
 {
 	struct group_info *group_info;
@@ -251,6 +254,7 @@ SYSCALL_DEFINE2(setgroups, int, gidsetsize, gid_t __user *, grouplist)
 
 	return retval;
 }
+#endif
 
 /*
  * Check whether we're fsgid/egid or in the supplemental group..
diff --git a/kernel/sys.c b/kernel/sys.c
index a8c9f5a..bfe532b 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -319,6 +319,7 @@ out_unlock:
  * SMP: There are not races, the GIDs are checked only by filesystem
  *      operations (as far as semantic preservation is concerned).
  */
+#ifdef CONFIG_NON_ROOT
 SYSCALL_DEFINE2(setregid, gid_t, rgid, gid_t, egid)
 {
 	struct user_namespace *ns = current_user_ns();
@@ -809,6 +810,7 @@ change_okay:
 	commit_creds(new);
 	return old_fsgid;
 }
+#endif /* CONFIG_NON_ROOT */
 
 /**
  * sys_getpid - return the thread group id of the current process
diff --git a/kernel/sys_ni.c b/kernel/sys_ni.c
index 5adcb0a..7995ef5 100644
--- a/kernel/sys_ni.c
+++ b/kernel/sys_ni.c
@@ -159,6 +159,20 @@ cond_syscall(sys_uselib);
 cond_syscall(sys_fadvise64);
 cond_syscall(sys_fadvise64_64);
 cond_syscall(sys_madvise);
+cond_syscall(sys_setuid);
+cond_syscall(sys_setregid);
+cond_syscall(sys_setgid);
+cond_syscall(sys_setreuid);
+cond_syscall(sys_setresuid);
+cond_syscall(sys_getresuid);
+cond_syscall(sys_setresgid);
+cond_syscall(sys_getresgid);
+cond_syscall(sys_setgroups);
+cond_syscall(sys_getgroups);
+cond_syscall(sys_setfsuid);
+cond_syscall(sys_setfsgid);
+cond_syscall(sys_capget);
+cond_syscall(sys_capset);
 
 /* arch-specific weak syscall entries */
 cond_syscall(sys_pciconfig_read);
-- 
1.7.10.4

Re: [PATCH] kernel: Conditionally support non-root users, groups and capabilities

[Casey Schaufler]

On 1/20/2015 3:33 PM, Iulia Manda wrote:
> There are a lot of embedded systems that run most or all of their functionality
> in init, running as root:root. For these systems, supporting multiple users is
> not necessary.
>
> This patch adds a new symbol, CONFIG_NON_ROOT, that makes support for non-root
> users, non-root groups, and capabilities optional.
>
> When this symbol is not defined, UID and GID are zero in any possible case
> and processes always have all capabilities.
>
> Also, the following syscalls are compiled out: setuid, setregid, setgid,
> setreuid, setresuid, getresuid, setresgid, getresgid, setgroups, getgroups,
> setfsuid, setfsgid, capget, capset.
>
> This change saves about 25 KB on a defconfig build.
>
> Bloat-o-meter output:
> add/remove: 7/66 grow/shrink: 21/421 up/down: 1701/-27172 (-25471)
>
> Signed-off-by: Iulia Manda <iulia.manda21@gmail.com>
> Reviewed-by: Josh Triplett <josh@joshtriplett.org>

Authoritative LSM hooks were loudly rejected in or about 1999.
One of the primary reasons they were rejected was because you could
use them do exactly what this patch does, which is to remove the basic
Linux security policy. If attitudes have changed sufficiently that
removing the "classic" security behavior is now deemed acceptable,
I propose that we reintroduce the option of authoritative LSM hooks
instead. That would give you all this saving, and probably more.


> ---
>  include/linux/capability.h |   12 ++++++++++++
>  include/linux/uidgid.h     |   12 ++++++++++++
>  init/Kconfig               |   19 ++++++++++++++++++-
>  kernel/capability.c        |    6 ++++++
>  kernel/groups.c            |    4 ++++
>  kernel/sys.c               |    2 ++
>  kernel/sys_ni.c            |   14 ++++++++++++++
>  7 files changed, 68 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/capability.h b/include/linux/capability.h
> index aa93e5e..d8791d2 100644
> --- a/include/linux/capability.h
> +++ b/include/linux/capability.h
> @@ -211,8 +211,20 @@ extern bool has_ns_capability(struct task_struct *t,
>  extern bool has_capability_noaudit(struct task_struct *t, int cap);
>  extern bool has_ns_capability_noaudit(struct task_struct *t,
>  				      struct user_namespace *ns, int cap);
> +#ifdef CONFIG_NON_ROOT
>  extern bool capable(int cap);
>  extern bool ns_capable(struct user_namespace *ns, int cap);
> +#else
> +static inline bool capable(int cap)
> +{
> +	return true;
> +}
> +
> +static inline bool ns_capable(struct user_namespace *ns, int cap)
> +{
> +	return true;
> +}
> +#endif /* CONFIG_NON_ROOT */
>  extern bool capable_wrt_inode_uidgid(const struct inode *inode, int cap);
>  extern bool file_ns_capable(const struct file *file, struct user_namespace *ns, int cap);
>  
> diff --git a/include/linux/uidgid.h b/include/linux/uidgid.h
> index 2d1f9b6..22bd1fa 100644
> --- a/include/linux/uidgid.h
> +++ b/include/linux/uidgid.h
> @@ -29,6 +29,7 @@ typedef struct {
>  #define KUIDT_INIT(value) (kuid_t){ value }
>  #define KGIDT_INIT(value) (kgid_t){ value }
>  
> +#ifdef CONFIG_NON_ROOT
>  static inline uid_t __kuid_val(kuid_t uid)
>  {
>  	return uid.val;
> @@ -38,6 +39,17 @@ static inline gid_t __kgid_val(kgid_t gid)
>  {
>  	return gid.val;
>  }
> +#else
> +static inline uid_t __kuid_val(kuid_t uid)
> +{
> +	return 0;
> +}
> +
> +static inline gid_t __kgid_val(kgid_t gid)
> +{
> +	return 0;
> +}
> +#endif
>  
>  #define GLOBAL_ROOT_UID KUIDT_INIT(0)
>  #define GLOBAL_ROOT_GID KGIDT_INIT(0)
> diff --git a/init/Kconfig b/init/Kconfig
> index 9afb971..dc5bfd4 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -394,6 +394,7 @@ endchoice
>  
>  config BSD_PROCESS_ACCT
>  	bool "BSD Process Accounting"
> +	select NON_ROOT
>  	help
>  	  If you say Y here, a user level program will be able to instruct the
>  	  kernel (via a special system call) to write process accounting
> @@ -420,6 +421,7 @@ config BSD_PROCESS_ACCT_V3
>  config TASKSTATS
>  	bool "Export task/process statistics through netlink"
>  	depends on NET
> +	select NON_ROOT
>  	default n
>  	help
>  	  Export selected statistics for tasks/processes through the
> @@ -1140,6 +1142,7 @@ config CHECKPOINT_RESTORE
>  
>  menuconfig NAMESPACES
>  	bool "Namespaces support" if EXPERT
> +	depends on NON_ROOT
>  	default !EXPERT
>  	help
>  	  Provides the way to make tasks work with different objects using
> @@ -1352,11 +1355,25 @@ menuconfig EXPERT
>  
>  config UID16
>  	bool "Enable 16-bit UID system calls" if EXPERT
> -	depends on HAVE_UID16
> +	depends on HAVE_UID16 && NON_ROOT
>  	default y
>  	help
>  	  This enables the legacy 16-bit UID syscall wrappers.
>  
> +config NON_ROOT
> +	bool "Multiple users, groups and capabilities support" if EXPERT
> +	default y
> +	help
> +	  This option enables support for non-root users, groups and
> +	  capabilities.
> +
> +	  If you say N here, all processes will run with UID 0, GID 0, and all
> +	  possible capabilities.  Saying N here also compiles out support for
> +	  system calls related to UIDs, GIDs, and capabilities, such as setuid,
> +	  setgid, and capset.
> +
> +	  If unsure, say Y here.
> +
>  config SGETMASK_SYSCALL
>  	bool "sgetmask/ssetmask syscalls support" if EXPERT
>  	def_bool PARISC || MN10300 || BLACKFIN || M68K || PPC || MIPS || X86 || SPARC || CRIS || MICROBLAZE || SUPERH
> diff --git a/kernel/capability.c b/kernel/capability.c
> index 989f5bf..bead84a 100644
> --- a/kernel/capability.c
> +++ b/kernel/capability.c
> @@ -35,6 +35,7 @@ static int __init file_caps_disable(char *str)
>  }
>  __setup("no_file_caps", file_caps_disable);
>  
> +#ifdef CONFIG_NON_ROOT
>  /*
>   * More recent versions of libcap are available from:
>   *
> @@ -279,6 +280,7 @@ error:
>  	abort_creds(new);
>  	return ret;
>  }
> +#endif
>  
>  /**
>   * has_ns_capability - Does a task have a capability in a specific user ns
> @@ -360,6 +362,7 @@ bool has_capability_noaudit(struct task_struct *t, int cap)
>  	return has_ns_capability_noaudit(t, &init_user_ns, cap);
>  }
>  
> +#ifdef CONFIG_NON_ROOT
>  /**
>   * ns_capable - Determine if the current task has a superior capability in effect
>   * @ns:  The usernamespace we want the capability in
> @@ -385,6 +388,7 @@ bool ns_capable(struct user_namespace *ns, int cap)
>  	return false;
>  }
>  EXPORT_SYMBOL(ns_capable);
> +#endif
>  
>  /**
>   * file_ns_capable - Determine if the file's opener had a capability in effect
> @@ -411,6 +415,7 @@ bool file_ns_capable(const struct file *file, struct user_namespace *ns,
>  }
>  EXPORT_SYMBOL(file_ns_capable);
>  
> +#ifdef CONFIG_NON_ROOT
>  /**
>   * capable - Determine if the current task has a superior capability in effect
>   * @cap: The capability to be tested for
> @@ -426,6 +431,7 @@ bool capable(int cap)
>  	return ns_capable(&init_user_ns, cap);
>  }
>  EXPORT_SYMBOL(capable);
> +#endif
>  
>  /**
>   * capable_wrt_inode_uidgid - Check nsown_capable and uid and gid mapped
> diff --git a/kernel/groups.c b/kernel/groups.c
> index 664411f..94f2c89 100644
> --- a/kernel/groups.c
> +++ b/kernel/groups.c
> @@ -190,6 +190,7 @@ int set_current_groups(struct group_info *group_info)
>  
>  EXPORT_SYMBOL(set_current_groups);
>  
> +#ifdef CONFIG_NON_ROOT
>  SYSCALL_DEFINE2(getgroups, int, gidsetsize, gid_t __user *, grouplist)
>  {
>  	const struct cred *cred = current_cred();
> @@ -213,6 +214,7 @@ SYSCALL_DEFINE2(getgroups, int, gidsetsize, gid_t __user *, grouplist)
>  out:
>  	return i;
>  }
> +#endif
>  
>  bool may_setgroups(void)
>  {
> @@ -227,6 +229,7 @@ bool may_setgroups(void)
>   *	without another task interfering.
>   */
>  
> +#ifdef CONFIG_NON_ROOT
>  SYSCALL_DEFINE2(setgroups, int, gidsetsize, gid_t __user *, grouplist)
>  {
>  	struct group_info *group_info;
> @@ -251,6 +254,7 @@ SYSCALL_DEFINE2(setgroups, int, gidsetsize, gid_t __user *, grouplist)
>  
>  	return retval;
>  }
> +#endif
>  
>  /*
>   * Check whether we're fsgid/egid or in the supplemental group..
> diff --git a/kernel/sys.c b/kernel/sys.c
> index a8c9f5a..bfe532b 100644
> --- a/kernel/sys.c
> +++ b/kernel/sys.c
> @@ -319,6 +319,7 @@ out_unlock:
>   * SMP: There are not races, the GIDs are checked only by filesystem
>   *      operations (as far as semantic preservation is concerned).
>   */
> +#ifdef CONFIG_NON_ROOT
>  SYSCALL_DEFINE2(setregid, gid_t, rgid, gid_t, egid)
>  {
>  	struct user_namespace *ns = current_user_ns();
> @@ -809,6 +810,7 @@ change_okay:
>  	commit_creds(new);
>  	return old_fsgid;
>  }
> +#endif /* CONFIG_NON_ROOT */
>  
>  /**
>   * sys_getpid - return the thread group id of the current process
> diff --git a/kernel/sys_ni.c b/kernel/sys_ni.c
> index 5adcb0a..7995ef5 100644
> --- a/kernel/sys_ni.c
> +++ b/kernel/sys_ni.c
> @@ -159,6 +159,20 @@ cond_syscall(sys_uselib);
>  cond_syscall(sys_fadvise64);
>  cond_syscall(sys_fadvise64_64);
>  cond_syscall(sys_madvise);
> +cond_syscall(sys_setuid);
> +cond_syscall(sys_setregid);
> +cond_syscall(sys_setgid);
> +cond_syscall(sys_setreuid);
> +cond_syscall(sys_setresuid);
> +cond_syscall(sys_getresuid);
> +cond_syscall(sys_setresgid);
> +cond_syscall(sys_getresgid);
> +cond_syscall(sys_setgroups);
> +cond_syscall(sys_getgroups);
> +cond_syscall(sys_setfsuid);
> +cond_syscall(sys_setfsgid);
> +cond_syscall(sys_capget);
> +cond_syscall(sys_capset);
>  
>  /* arch-specific weak syscall entries */
>  cond_syscall(sys_pciconfig_read);

Re: [PATCH] kernel: Conditionally support non-root users, groups and capabilities

[Josh Triplett]

On Tue, Jan 20, 2015 at 04:05:00PM -0800, Casey Schaufler wrote:
> On 1/20/2015 3:33 PM, Iulia Manda wrote:
> > There are a lot of embedded systems that run most or all of their functionality
> > in init, running as root:root. For these systems, supporting multiple users is
> > not necessary.
> >
> > This patch adds a new symbol, CONFIG_NON_ROOT, that makes support for non-root
> > users, non-root groups, and capabilities optional.
> >
> > When this symbol is not defined, UID and GID are zero in any possible case
> > and processes always have all capabilities.
> >
> > Also, the following syscalls are compiled out: setuid, setregid, setgid,
> > setreuid, setresuid, getresuid, setresgid, getresgid, setgroups, getgroups,
> > setfsuid, setfsgid, capget, capset.
> >
> > This change saves about 25 KB on a defconfig build.
> >
> > Bloat-o-meter output:
> > add/remove: 7/66 grow/shrink: 21/421 up/down: 1701/-27172 (-25471)
> >
> > Signed-off-by: Iulia Manda <iulia.manda21@gmail.com>
> > Reviewed-by: Josh Triplett <josh@joshtriplett.org>
> 
> Authoritative LSM hooks were loudly rejected in or about 1999.
> One of the primary reasons they were rejected was because you could
> use them do exactly what this patch does, which is to remove the basic
> Linux security policy. If attitudes have changed sufficiently that
> removing the "classic" security behavior is now deemed acceptable,
> I propose that we reintroduce the option of authoritative LSM hooks
> instead. That would give you all this saving, and probably more.

Wouldn't authoritative LSM hooks require *adding* the necessary hook
logic, along with a hook module implementing such a policy?  Unless
you're suggesting that compiling in LSM hooks without any providers
would result in this behavior by default, which seems rather
questionable.

Also note that this is compiling out the entire family of UID/GID system
calls, which LSM hooks could not do.

In any case, I see two major problems with authoritative LSM hooks that
this patch avoids:

First, simplicity: I doubt authoritative LSM hooks could match this
diffstat:

> >  include/linux/capability.h |   12 ++++++++++++
> >  include/linux/uidgid.h     |   12 ++++++++++++
> >  init/Kconfig               |   19 ++++++++++++++++++-
> >  kernel/capability.c        |    6 ++++++
> >  kernel/groups.c            |    4 ++++
> >  kernel/sys.c               |    2 ++
> >  kernel/sys_ni.c            |   14 ++++++++++++++
> >  7 files changed, 68 insertions(+), 1 deletion(-)

Second, code size reduction: In addition to the concern above about
adding hooks rather than removing code, this patch allows
constant-folding away huge amounts of code, which any kind of "hook"
mechanism would have a hard time doing.  This patch lets the compiler do
almost all of the work.  Notice the "66 shrink" and "421 down" in the
bloat-o-meter summary.

The intent here is not to open the door to arbitrary replacement
security policies.  The intent is to simply add a compile-time option to
compile *out* security policies entirely, for systems that will not only
never call setuid but in many cases never even call fork.

- Josh Triplett

Re: [PATCH] kernel: Conditionally support non-root users, groups and capabilities

[Casey Schaufler]

On 1/20/2015 4:50 PM, Josh Triplett wrote:
> On Tue, Jan 20, 2015 at 04:05:00PM -0800, Casey Schaufler wrote:
>> On 1/20/2015 3:33 PM, Iulia Manda wrote:
>>> There are a lot of embedded systems that run most or all of their functionality
>>> in init, running as root:root. For these systems, supporting multiple users is
>>> not necessary.
>>>
>>> This patch adds a new symbol, CONFIG_NON_ROOT, that makes support for non-root
>>> users, non-root groups, and capabilities optional.
>>>
>>> When this symbol is not defined, UID and GID are zero in any possible case
>>> and processes always have all capabilities.
>>>
>>> Also, the following syscalls are compiled out: setuid, setregid, setgid,
>>> setreuid, setresuid, getresuid, setresgid, getresgid, setgroups, getgroups,
>>> setfsuid, setfsgid, capget, capset.
>>>
>>> This change saves about 25 KB on a defconfig build.
>>>
>>> Bloat-o-meter output:
>>> add/remove: 7/66 grow/shrink: 21/421 up/down: 1701/-27172 (-25471)
>>>
>>> Signed-off-by: Iulia Manda <iulia.manda21@gmail.com>
>>> Reviewed-by: Josh Triplett <josh@joshtriplett.org>
>> Authoritative LSM hooks were loudly rejected in or about 1999.
>> One of the primary reasons they were rejected was because you could
>> use them do exactly what this patch does, which is to remove the basic
>> Linux security policy. If attitudes have changed sufficiently that
>> removing the "classic" security behavior is now deemed acceptable,
>> I propose that we reintroduce the option of authoritative LSM hooks
>> instead. That would give you all this saving, and probably more.
> Wouldn't authoritative LSM hooks require *adding* the necessary hook
> logic, along with a hook module implementing such a policy?  Unless
> you're suggesting that compiling in LSM hooks without any providers
> would result in this behavior by default, which seems rather
> questionable.

The proposed default configuration would have included hooks implementing the
traditional UNIX/Linux discretionary access controls and Super User. Proposed
alternative configurations allowed the addition of POSIX capabilities, B&L MAC
and that new kid on the block, FLASK.

Your configuration would have required trivial "return success" policy functions,
much like what you find in security/capability.c today.

> Also note that this is compiling out the entire family of UID/GID system
> calls, which LSM hooks could not do.

Which breaks backward compatibility.
 

> In any case, I see two major problems with authoritative LSM hooks that
> this patch avoids:
>
> First, simplicity: I doubt authoritative LSM hooks could match this
> diffstat:
>
>>>  include/linux/capability.h |   12 ++++++++++++
>>>  include/linux/uidgid.h     |   12 ++++++++++++
>>>  init/Kconfig               |   19 ++++++++++++++++++-
>>>  kernel/capability.c        |    6 ++++++
>>>  kernel/groups.c            |    4 ++++
>>>  kernel/sys.c               |    2 ++
>>>  kernel/sys_ni.c            |   14 ++++++++++++++
>>>  7 files changed, 68 insertions(+), 1 deletion(-)

The size of the diffstat is no indication of the value of a change.
You're using a trivial amount of ifdefing to implement a very significant change.
If you removed the code you're blocking out your diffstat would be much indicative of
the impact, and much larger.

> Second, code size reduction: In addition to the concern above about
> adding hooks rather than removing code, this patch allows
> constant-folding away huge amounts of code, which any kind of "hook"
> mechanism would have a hard time doing.  This patch lets the compiler do
> almost all of the work.  Notice the "66 shrink" and "421 down" in the
> bloat-o-meter summary.

Yes, we all know (or should) that you can reduce the size of a change using

	#ifdef 0
	... 58,455 line of code ...
	#endif

instead of removing the 58,455 lines of code.

> The intent here is not to open the door to arbitrary replacement
> security policies.  The intent is to simply add a compile-time option to
> compile *out* security policies entirely, for systems that will not only
> never call setuid but in many cases never even call fork.

You are opening the door to creating a "Linux" kernel that does not
behave like Linux. This has been seriously considered and flat out rejected
in the past. It was not a matter of the mechanism used to achieve the
change in policy, it was in fact the ability to change the policy.

Again, attitudes may have changed since the turn of the century. If they
have, we should do a real job of allowing the existing policy to be
changed rather than whacking away code with the ifdef chainsaw.

> - Josh Triplett
>

Re: [PATCH] kernel: Conditionally support non-root users, groups and capabilities

[Josh Triplett]

On Tue, Jan 20, 2015 at 05:23:06PM -0800, Casey Schaufler wrote:
> On 1/20/2015 4:50 PM, Josh Triplett wrote:
> > On Tue, Jan 20, 2015 at 04:05:00PM -0800, Casey Schaufler wrote:
> >> On 1/20/2015 3:33 PM, Iulia Manda wrote:
> >>> There are a lot of embedded systems that run most or all of their functionality
> >>> in init, running as root:root. For these systems, supporting multiple users is
> >>> not necessary.
> >>>
> >>> This patch adds a new symbol, CONFIG_NON_ROOT, that makes support for non-root
> >>> users, non-root groups, and capabilities optional.
> >>>
> >>> When this symbol is not defined, UID and GID are zero in any possible case
> >>> and processes always have all capabilities.
> >>>
> >>> Also, the following syscalls are compiled out: setuid, setregid, setgid,
> >>> setreuid, setresuid, getresuid, setresgid, getresgid, setgroups, getgroups,
> >>> setfsuid, setfsgid, capget, capset.
> >>>
> >>> This change saves about 25 KB on a defconfig build.
> >>>
> >>> Bloat-o-meter output:
> >>> add/remove: 7/66 grow/shrink: 21/421 up/down: 1701/-27172 (-25471)
> >>>
> >>> Signed-off-by: Iulia Manda <iulia.manda21@gmail.com>
> >>> Reviewed-by: Josh Triplett <josh@joshtriplett.org>
> >> Authoritative LSM hooks were loudly rejected in or about 1999.
> >> One of the primary reasons they were rejected was because you could
> >> use them do exactly what this patch does, which is to remove the basic
> >> Linux security policy. If attitudes have changed sufficiently that
> >> removing the "classic" security behavior is now deemed acceptable,
> >> I propose that we reintroduce the option of authoritative LSM hooks
> >> instead. That would give you all this saving, and probably more.
> > Wouldn't authoritative LSM hooks require *adding* the necessary hook
> > logic, along with a hook module implementing such a policy?  Unless
> > you're suggesting that compiling in LSM hooks without any providers
> > would result in this behavior by default, which seems rather
> > questionable.
> 
> The proposed default configuration would have included hooks implementing the
> traditional UNIX/Linux discretionary access controls and Super User. Proposed
> alternative configurations allowed the addition of POSIX capabilities, B&L MAC
> and that new kid on the block, FLASK.
> 
> Your configuration would have required trivial "return success" policy functions,
> much like what you find in security/capability.c today.

It's entirely possible that kernel developers are willing to
accept a CONFIG_NON_ROOT=n policy but are still opposed to authoritative
LSM hooks.  Not every mechanism needs to be expanded and abstracted to
cover the most general possible form.  On the contrary, simple solutions
tend to go over better unless there's a strong push for a whole family
of use cases that require a more complex solution.  And I haven't seen
any kind of push for authoritative LSM hooks for years.

I appreciate that you'd be interested in an opportunity to revive
authoritative LSM hooks, and you're welcome to write the much larger
patch doing so.  That's not by itself an argument against a much simpler
solution with existing code that solves the use case at hand.

On top of that, unless that mechanism supported selecting a single such
(lack of) policy at compile time and turning it into static inlines,
even trivial policy functions would not actually get compiled away into
nothing along with all the hundreds of conditionals calling them.

> > Also note that this is compiling out the entire family of UID/GID system
> > calls, which LSM hooks could not do.
> 
> Which breaks backward compatibility.

Like half of the other options under CONFIG_EXPERT, as well as some that
aren't.  If you turn off Kconfig options designed to disable syscalls,
you'll get a kernel that returns -ENOSYS for those syscalls, as
requested.  That doesn't break backward compatibility; leave the option
enabled if you need those syscalls.

> > In any case, I see two major problems with authoritative LSM hooks that
> > this patch avoids:
> >
> > First, simplicity: I doubt authoritative LSM hooks could match this
> > diffstat:
> >
> >>>  include/linux/capability.h |   12 ++++++++++++
> >>>  include/linux/uidgid.h     |   12 ++++++++++++
> >>>  init/Kconfig               |   19 ++++++++++++++++++-
> >>>  kernel/capability.c        |    6 ++++++
> >>>  kernel/groups.c            |    4 ++++
> >>>  kernel/sys.c               |    2 ++
> >>>  kernel/sys_ni.c            |   14 ++++++++++++++
> >>>  7 files changed, 68 insertions(+), 1 deletion(-)
> 
> The size of the diffstat is no indication of the value of a change.
> You're using a trivial amount of ifdefing to implement a very significant change.

(Please note that I'm not the author of the change, just a proponent and
reviewer.  I don't wish to take credit for someone else's work here,
though I'm happy to shoulder blame/flame for the idea.)

And yes, indeed, this is a significant change with a mercifully small
diffstat.  The ones with much larger diffstats tend to get rather poor
responses, and suggestions that the compiler should be made to do more
of the work by providing appropriate inline stubs.  This is the first
time I've seen a complaint that a change is too *small*.

> > Second, code size reduction: In addition to the concern above about
> > adding hooks rather than removing code, this patch allows
> > constant-folding away huge amounts of code, which any kind of "hook"
> > mechanism would have a hard time doing.  This patch lets the compiler do
> > almost all of the work.  Notice the "66 shrink" and "421 down" in the
> > bloat-o-meter summary.
> 
> Yes, we all know (or should) that you can reduce the size of a change using

I'm not talking about reducing the size of the *change* here.  I'm
talking about reducing the size of the compiled code.  The full
bloat-o-meter output shows hundreds of functions each becoming smaller,
generally because blocks like "if (!capable(...)) { ... }" went away
entirely.

> > The intent here is not to open the door to arbitrary replacement
> > security policies.  The intent is to simply add a compile-time option to
> > compile *out* security policies entirely, for systems that will not only
> > never call setuid but in many cases never even call fork.
> 
> You are opening the door to creating a "Linux" kernel that does not
> behave like Linux. This has been seriously considered and flat out rejected
> in the past.

Reiterating my explanation from the last time this came up:

"""it's been our standard practice for ages.  We started down that road
long, long ago, when we first introduced Kconfig and optional/modular
features.  /dev/* are user-facing interfaces, yet you can compile them
out or make them modular.  /sys/* and/proc/* are user-facing interfaces,
yet you can compile part or all of them out.  Filesystem names passed to
mount are user-facing interfaces, yet you can compile them out.  (Not
just things like ext4; think FUSE or overlayfs, which some applications
will build upon and require.)  Some prctls are optional, new syscalls
like BPF or inotify or process_vm_{read,write}v are optional, hardware
interfaces are optional, control groups are optional, containers and
namespaces are optional, checkpoint/restart is optional, KVM is
optional, kprobes are optional, kmsg is optional, /dev/port is optional,
ACL support is optional, USB support (as used by libusb) is optional,
sound interfaces are optional, GPU interfaces are optional, even futexes
are optional."""

A fair bit of POSIX and other standard interfaces can already be
compiled out.

So what's this about "not behaving like Linux"?  Linux is whatever lives
in linux.git; it's a lot more capable these days, and that doesn't just
mean *adding* features.  The alternative to a tinier Linux isn't a
larger Linux, it's non-Linux embedded OSes that behave *nothing* like
Linux because they're *not Linux*.

> Again, attitudes may have changed since the turn of the century.  If
> they have, we should do a real job of allowing the existing policy to
> be changed

To the extent there was ever a policy about not compiling out code, that
policy was changed long ago.  If you're looking to change policies about
authoritative LSM hooks and what security frameworks can do:

return -ETOOMANYWINDMILLS;

- Josh Triplett

Re: [PATCH] kernel: Conditionally support non-root users, groups and capabilities

[One Thousand Gnomes]

On Wed, 21 Jan 2015 01:33:08 +0200
Iulia Manda <iulia.manda21@gmail.com> wrote:

> There are a lot of embedded systems that run most or all of their functionality
> in init, running as root:root. For these systems, supporting multiple users is
> not necessary.

We probably shouldn't encourage such poor design ;-)

The proposed patch generates a whole mass of ifdefs all over the place.
If it's going to be done move all the functions in question together
somewhere logical and give them a single ifdef or a file of their own.

It also doesn't appear to be dropping all it should - why can't you
simply not compile in groups.c for example ? If you can't then it says
the patch is far from complete at this point.

Alan

Re: [PATCH] kernel: Conditionally support non-root users, groups and capabilities

[josh]

On Wed, Jan 21, 2015 at 02:52:19PM +0000, One Thousand Gnomes wrote:
> The proposed patch generates a whole mass of ifdefs all over the place.
> If it's going to be done move all the functions in question together
> somewhere logical and give them a single ifdef or a file of their own.

I assume the header-file ifdefs are not problematic.  The functions in
kernel/sys.c are already grouped together in a single block and needed
only one ifdef.  So I'd assume the main problem is the multiple ifdefs
in capability.c and groups.c?  Might be possible to consolidate those,
sure.

> It also doesn't appear to be dropping all it should - why can't you
> simply not compile in groups.c for example ? If you can't then it says
> the patch is far from complete at this point.

Making groups.c entirely optional has been on the todo list for a while;
it's rather harder than just dropping the syscalls, as some of its
other functions are exported to the rest of the kernel as well, but it's
doable.

- Josh Triplett