[d-kernel] (без темы)

[d-kernel] (без темы)

[Vladimir D. Seleznev]

Hi!

This patch is intended for all kernel flavours that support AltHa.

Caps provide subsets of privilleges and should be covered by AltHa.

[d-kernel] [PATCH] AltHa: nosuid handles capabilities as well

[Vladimir D. Seleznev]

Signed-off-by: Vladimir D. Seleznev <vseleznv@altlinux.org>
---
 Documentation/admin-guide/LSM/AltHa.rst |  6 ++--
 security/altha/altha_lsm.c              | 37 ++++++++++++++++++++-----
 2 files changed, 33 insertions(+), 10 deletions(-)

diff --git a/Documentation/admin-guide/LSM/AltHa.rst b/Documentation/admin-guide/LSM/AltHa.rst
index be698709d3f0..beda40601c9e 100644
--- a/Documentation/admin-guide/LSM/AltHa.rst
+++ b/Documentation/admin-guide/LSM/AltHa.rst
@@ -3,7 +3,7 @@ AltHa
 ====
 
 AltHa is a Linux Security Module currently has three userspace hardening options:
-    * ignore SUID on binaries (with exceptions possible);
+    * ignore SUID and setcaps on binaries (with exceptions possible);
     * prevent running selected script interpreters in interactive mode;
     * disable open file unlinking in selected dirs.
     * enable kiosk mode
@@ -15,12 +15,12 @@ through sysctls in ``/proc/sys/kernel/altha``.
 
 NoSUID
 ============
-Modern Linux systems can be used with minimal (or even zero at least for OWL and ALT) usage of SUID programms, but in many cases in full-featured desktop or server systems there are plenty of them: uncounted and sometimes unnecessary. Privileged programms are always an attack surface, but mounting filesystems with ``nosuid`` flag doesn't provide enough granularity in SUID binaries management. This LSM module provides a single control point for all SUID binaries. When this submodule is enabled, SUID bits on all binaries except explicitly listed are system-wide ignored.
+Modern Linux systems can be used with minimal (or even zero at least for OWL and ALT) usage of SUID programms, but in many cases in full-featured desktop or server systems there are plenty of them: uncounted and sometimes unnecessary. Privileged programms are always an attack surface, but mounting filesystems with ``nosuid`` flag doesn't provide enough granularity in SUID binaries management. This LSM module provides a single control point for all SUID and setcap binaries. When this submodule is enabled, SUID and setcap bits on all binaries except explicitly listed are system-wide ignored.
 
 Sysctl parameters and defaults:
 
 * ``kernel.altha.nosuid.enabled = 0``, set to 1 to enable
-* ``kernel.altha.nosuid.exceptions =``, colon-separated list of enabled SUID binaries, for example: ``/bin/su:/usr/libexec/hasher-priv/hasher-priv``
+* ``kernel.altha.nosuid.exceptions =``, colon-separated list of enabled SUID and setcap binaries, for example: ``/bin/su:/usr/libexec/hasher-priv/hasher-priv``
 
 RestrScript
 ============
diff --git a/security/altha/altha_lsm.c b/security/altha/altha_lsm.c
index c670ad7ed458..5f0505a51644 100644
--- a/security/altha/altha_lsm.c
+++ b/security/altha/altha_lsm.c
@@ -11,6 +11,7 @@
 
 #include <linux/lsm_hooks.h>
 #include <linux/cred.h>
+#include <linux/capability.h>
 #include <linux/sysctl.h>
 #include <linux/binfmts.h>
 #include <linux/file.h>
@@ -237,10 +238,20 @@ int is_olock_dir(struct inode *inode)
 	return 0;
 }
 
+static int has_any_caps(struct cred *cred)
+{
+	return !cap_isclear(cred->cap_permitted) ||
+	       !cap_isclear(cred->cap_effective);
+
+	return 0;
+}
+
 /* Hooks */
 static int altha_bprm_creds_from_file(struct linux_binprm *bprm, struct file * fi)
 {
 	struct altha_list_struct *node;
+	int is_set_caps = 0;
+	char *set_uid_cap = "setuid";
 	/* when it's not a shebang issued script interpreter */
 	if (rstrscript_enabled && bprm->executable == bprm->interpreter) {
 		char *path_p;
@@ -267,11 +278,18 @@ static int altha_bprm_creds_from_file(struct linux_binprm *bprm, struct file * f
 		up_read(&interpreters_sem);
 		kfree(path_buffer);
 	}
-	if (unlikely(nosuid_enabled &&
-		     !uid_eq(bprm->cred->uid, bprm->cred->euid))) {
+	if (nosuid_enabled) {
 		char *path_p;
 		char *path_buffer;
 		uid_t cur_uid;
+		uid_t cur_euid = from_kuid(bprm->cred->user_ns, bprm->cred->euid);
+		/* Check for any caps for non-superuser. */
+		if (cur_euid != (uid_t) 0
+		   && has_any_caps(bprm->cred)) {
+			is_set_caps = 1;
+			set_uid_cap = "setcap";
+		} else if (uid_eq(bprm->cred->uid, bprm->cred->euid))
+			return 0;
 
 		path_buffer = kmalloc(PATH_MAX, GFP_KERNEL);
 		if (!path_buffer)
@@ -283,8 +301,8 @@ static int altha_bprm_creds_from_file(struct linux_binprm *bprm, struct file * f
 		list_for_each_entry(node, &nosuid_exceptions_list, list) {
 			if (strcmp(path_p, node->spath) == 0) {
 				pr_notice_ratelimited
-				    ("AltHa/NoSUID: %s permitted to setuid from %d\n",
-				     bprm->filename, cur_uid);
+				    ("AltHa/NoSUID: %s permitted to %s from %d\n",
+				     bprm->filename, set_uid_cap, cur_uid);
 				up_read(&nosuid_exceptions_sem);
 				kfree(path_buffer);
 				return 0;
@@ -292,9 +310,14 @@ static int altha_bprm_creds_from_file(struct linux_binprm *bprm, struct file * f
 		}
 		up_read(&nosuid_exceptions_sem);
 		pr_notice_ratelimited
-		    ("AltHa/NoSUID: %s prevented to setuid from %d\n",
-		     bprm->filename, cur_uid);
-		bprm->cred->euid = bprm->cred->uid;
+		    ("AltHa/NoSUID: %s prevented to %s from %d\n",
+		     bprm->filename, set_uid_cap, cur_uid);
+		if (is_set_caps) {
+			cap_clear(bprm->cred->cap_inheritable);
+			cap_clear(bprm->cred->cap_permitted);
+			cap_clear(bprm->cred->cap_effective);
+		} else
+			bprm->cred->euid = bprm->cred->uid;
 		kfree(path_buffer);
 	}
 	return 0;
-- 
2.33.2

Re: [d-kernel] (без темы) AltHa: nosuid handles capabilities as well

[Vitaly Chikunov]

Vladimir,

On Tue, Apr 26, 2022 at 09:47:32AM +0000, Vladimir D. Seleznev wrote:
> Hi!
> 
> This patch is intended for all kernel flavours that support AltHa.
> 
> Caps provide subsets of privilleges and should be covered by AltHa.

Было бы неплохо если бы были хотя бы какие-то тесты для этой feature.
Как например есть для kiosk. Не говоря уже о commit message.

Thanks,

> 
> _______________________________________________
> devel-kernel mailing list
> devel-kernel@lists.altlinux.org
> https://lists.altlinux.org/mailman/listinfo/devel-kernel

Re: [d-kernel] [PATCH] AltHa: nosuid handles capabilities as well

[Vitaly Chikunov]

Vladimir,

On Tue, Apr 26, 2022 at 09:47:33AM +0000, Vladimir D. Seleznev wrote:
> Signed-off-by: Vladimir D. Seleznev <vseleznv@altlinux.org>
> ---
>  Documentation/admin-guide/LSM/AltHa.rst |  6 ++--
>  security/altha/altha_lsm.c              | 37 ++++++++++++++++++++-----
>  2 files changed, 33 insertions(+), 10 deletions(-)
> 
> diff --git a/Documentation/admin-guide/LSM/AltHa.rst b/Documentation/admin-guide/LSM/AltHa.rst
> index be698709d3f0..beda40601c9e 100644
> --- a/Documentation/admin-guide/LSM/AltHa.rst
> +++ b/Documentation/admin-guide/LSM/AltHa.rst
> @@ -3,7 +3,7 @@ AltHa
>  ====
>  
>  AltHa is a Linux Security Module currently has three userspace hardening options:
> -    * ignore SUID on binaries (with exceptions possible);
> +    * ignore SUID and setcaps on binaries (with exceptions possible);
>      * prevent running selected script interpreters in interactive mode;
>      * disable open file unlinking in selected dirs.
>      * enable kiosk mode
> @@ -15,12 +15,12 @@ through sysctls in ``/proc/sys/kernel/altha``.
>  
>  NoSUID
>  ============
> -Modern Linux systems can be used with minimal (or even zero at least for OWL and ALT) usage of SUID programms, but in many cases in full-featured desktop or server systems there are plenty of them: uncounted and sometimes unnecessary. Privileged programms are always an attack surface, but mounting filesystems with ``nosuid`` flag doesn't provide enough granularity in SUID binaries management. This LSM module provides a single control point for all SUID binaries. When this submodule is enabled, SUID bits on all binaries except explicitly listed are system-wide ignored.
> +Modern Linux systems can be used with minimal (or even zero at least for OWL and ALT) usage of SUID programms, but in many cases in full-featured desktop or server systems there are plenty of them: uncounted and sometimes unnecessary. Privileged programms are always an attack surface, but mounting filesystems with ``nosuid`` flag doesn't provide enough granularity in SUID binaries management. This LSM module provides a single control point for all SUID and setcap binaries. When this submodule is enabled, SUID and setcap bits on all binaries except explicitly listed are system-wide ignored.
>  
>  Sysctl parameters and defaults:
>  
>  * ``kernel.altha.nosuid.enabled = 0``, set to 1 to enable
> -* ``kernel.altha.nosuid.exceptions =``, colon-separated list of enabled SUID binaries, for example: ``/bin/su:/usr/libexec/hasher-priv/hasher-priv``
> +* ``kernel.altha.nosuid.exceptions =``, colon-separated list of enabled SUID and setcap binaries, for example: ``/bin/su:/usr/libexec/hasher-priv/hasher-priv``
>  
>  RestrScript
>  ============
> diff --git a/security/altha/altha_lsm.c b/security/altha/altha_lsm.c
> index c670ad7ed458..5f0505a51644 100644
> --- a/security/altha/altha_lsm.c
> +++ b/security/altha/altha_lsm.c
> @@ -11,6 +11,7 @@
>  
>  #include <linux/lsm_hooks.h>
>  #include <linux/cred.h>
> +#include <linux/capability.h>
>  #include <linux/sysctl.h>
>  #include <linux/binfmts.h>
>  #include <linux/file.h>
> @@ -237,10 +238,20 @@ int is_olock_dir(struct inode *inode)
>  	return 0;
>  }
>  
> +static int has_any_caps(struct cred *cred)
> +{
> +	return !cap_isclear(cred->cap_permitted) ||
> +	       !cap_isclear(cred->cap_effective);
> +
> +	return 0;
> +}
> +
>  /* Hooks */
>  static int altha_bprm_creds_from_file(struct linux_binprm *bprm, struct file * fi)
>  {
>  	struct altha_list_struct *node;
> +	int is_set_caps = 0;
> +	char *set_uid_cap = "setuid";
>  	/* when it's not a shebang issued script interpreter */
>  	if (rstrscript_enabled && bprm->executable == bprm->interpreter) {
>  		char *path_p;
> @@ -267,11 +278,18 @@ static int altha_bprm_creds_from_file(struct linux_binprm *bprm, struct file * f
>  		up_read(&interpreters_sem);
>  		kfree(path_buffer);
>  	}
> -	if (unlikely(nosuid_enabled &&
> -		     !uid_eq(bprm->cred->uid, bprm->cred->euid))) {
> +	if (nosuid_enabled) {
>  		char *path_p;
>  		char *path_buffer;
>  		uid_t cur_uid;
> +		uid_t cur_euid = from_kuid(bprm->cred->user_ns, bprm->cred->euid);
> +		/* Check for any caps for non-superuser. */
> +		if (cur_euid != (uid_t) 0
> +		   && has_any_caps(bprm->cred)) {

Логика этого не понятна. Было бы неплохо чтоб она была пояснена в
комментариях чтоб и простые люди могли её понять.

Почему caps проверяются только для non-superuser? Ведь и superuser может
сбрасывать capabilities. У superuser может не быть каких-то capabilities
вплоть до никаких, кроме его uid 0. Следовательно, установка new
capabilities не должна зависеть от uid.

Далее, capabilities могут устанавливаться не все, но если в этой
проверке были обнаружены capabilities, то далее suid игнорируются
и не сбрасываются если они были. Что будет если установлены
одновременно и setcap, и suid?

Неплохо было бы, чтоб это все было пояснено в комментариях. Например,
почему не важно не сбрасывать suid при setcap.

Thanks,

> +			is_set_caps = 1;
> +			set_uid_cap = "setcap";
> +		} else if (uid_eq(bprm->cred->uid, bprm->cred->euid))
> +			return 0;
>  
>  		path_buffer = kmalloc(PATH_MAX, GFP_KERNEL);
>  		if (!path_buffer)
> @@ -283,8 +301,8 @@ static int altha_bprm_creds_from_file(struct linux_binprm *bprm, struct file * f
>  		list_for_each_entry(node, &nosuid_exceptions_list, list) {
>  			if (strcmp(path_p, node->spath) == 0) {
>  				pr_notice_ratelimited
> -				    ("AltHa/NoSUID: %s permitted to setuid from %d\n",
> -				     bprm->filename, cur_uid);
> +				    ("AltHa/NoSUID: %s permitted to %s from %d\n",
> +				     bprm->filename, set_uid_cap, cur_uid);
>  				up_read(&nosuid_exceptions_sem);
>  				kfree(path_buffer);
>  				return 0;
> @@ -292,9 +310,14 @@ static int altha_bprm_creds_from_file(struct linux_binprm *bprm, struct file * f
>  		}
>  		up_read(&nosuid_exceptions_sem);
>  		pr_notice_ratelimited
> -		    ("AltHa/NoSUID: %s prevented to setuid from %d\n",
> -		     bprm->filename, cur_uid);
> -		bprm->cred->euid = bprm->cred->uid;
> +		    ("AltHa/NoSUID: %s prevented to %s from %d\n",
> +		     bprm->filename, set_uid_cap, cur_uid);
> +		if (is_set_caps) {
> +			cap_clear(bprm->cred->cap_inheritable);
> +			cap_clear(bprm->cred->cap_permitted);
> +			cap_clear(bprm->cred->cap_effective);
> +		} else
> +			bprm->cred->euid = bprm->cred->uid;
>  		kfree(path_buffer);
>  	}
>  	return 0;
> -- 
> 2.33.2
> 
> _______________________________________________
> devel-kernel mailing list
> devel-kernel@lists.altlinux.org
> https://lists.altlinux.org/mailman/listinfo/devel-kernel

Re: [d-kernel] [PATCH] AltHa: nosuid handles capabilities as well

[Vladimir D. Seleznev]

On Wed, Apr 27, 2022 at 12:09:44AM +0300, Vitaly Chikunov wrote:
> Vladimir,
> 
> On Tue, Apr 26, 2022 at 09:47:33AM +0000, Vladimir D. Seleznev wrote:
> > Signed-off-by: Vladimir D. Seleznev <vseleznv@altlinux.org>
> > ---
> >  Documentation/admin-guide/LSM/AltHa.rst |  6 ++--
> >  security/altha/altha_lsm.c              | 37 ++++++++++++++++++++-----
> >  2 files changed, 33 insertions(+), 10 deletions(-)
> > 
> > diff --git a/Documentation/admin-guide/LSM/AltHa.rst b/Documentation/admin-guide/LSM/AltHa.rst
> > index be698709d3f0..beda40601c9e 100644
> > --- a/Documentation/admin-guide/LSM/AltHa.rst
> > +++ b/Documentation/admin-guide/LSM/AltHa.rst
> > @@ -3,7 +3,7 @@ AltHa
> >  ====
> >  
> >  AltHa is a Linux Security Module currently has three userspace hardening options:
> > -    * ignore SUID on binaries (with exceptions possible);
> > +    * ignore SUID and setcaps on binaries (with exceptions possible);
> >      * prevent running selected script interpreters in interactive mode;
> >      * disable open file unlinking in selected dirs.
> >      * enable kiosk mode
> > @@ -15,12 +15,12 @@ through sysctls in ``/proc/sys/kernel/altha``.
> >  
> >  NoSUID
> >  ============
> > -Modern Linux systems can be used with minimal (or even zero at least for OWL and ALT) usage of SUID programms, but in many cases in full-featured desktop or server systems there are plenty of them: uncounted and sometimes unnecessary. Privileged programms are always an attack surface, but mounting filesystems with ``nosuid`` flag doesn't provide enough granularity in SUID binaries management. This LSM module provides a single control point for all SUID binaries. When this submodule is enabled, SUID bits on all binaries except explicitly listed are system-wide ignored.
> > +Modern Linux systems can be used with minimal (or even zero at least for OWL and ALT) usage of SUID programms, but in many cases in full-featured desktop or server systems there are plenty of them: uncounted and sometimes unnecessary. Privileged programms are always an attack surface, but mounting filesystems with ``nosuid`` flag doesn't provide enough granularity in SUID binaries management. This LSM module provides a single control point for all SUID and setcap binaries. When this submodule is enabled, SUID and setcap bits on all binaries except explicitly listed are system-wide ignored.
> >  
> >  Sysctl parameters and defaults:
> >  
> >  * ``kernel.altha.nosuid.enabled = 0``, set to 1 to enable
> > -* ``kernel.altha.nosuid.exceptions =``, colon-separated list of enabled SUID binaries, for example: ``/bin/su:/usr/libexec/hasher-priv/hasher-priv``
> > +* ``kernel.altha.nosuid.exceptions =``, colon-separated list of enabled SUID and setcap binaries, for example: ``/bin/su:/usr/libexec/hasher-priv/hasher-priv``
> >  
> >  RestrScript
> >  ============
> > diff --git a/security/altha/altha_lsm.c b/security/altha/altha_lsm.c
> > index c670ad7ed458..5f0505a51644 100644
> > --- a/security/altha/altha_lsm.c
> > +++ b/security/altha/altha_lsm.c
> > @@ -11,6 +11,7 @@
> >  
> >  #include <linux/lsm_hooks.h>
> >  #include <linux/cred.h>
> > +#include <linux/capability.h>
> >  #include <linux/sysctl.h>
> >  #include <linux/binfmts.h>
> >  #include <linux/file.h>
> > @@ -237,10 +238,20 @@ int is_olock_dir(struct inode *inode)
> >  	return 0;
> >  }
> >  
> > +static int has_any_caps(struct cred *cred)
> > +{
> > +	return !cap_isclear(cred->cap_permitted) ||
> > +	       !cap_isclear(cred->cap_effective);
> > +
> > +	return 0;
> > +}
> > +
> >  /* Hooks */
> >  static int altha_bprm_creds_from_file(struct linux_binprm *bprm, struct file * fi)
> >  {
> >  	struct altha_list_struct *node;
> > +	int is_set_caps = 0;
> > +	char *set_uid_cap = "setuid";
> >  	/* when it's not a shebang issued script interpreter */
> >  	if (rstrscript_enabled && bprm->executable == bprm->interpreter) {
> >  		char *path_p;
> > @@ -267,11 +278,18 @@ static int altha_bprm_creds_from_file(struct linux_binprm *bprm, struct file * f
> >  		up_read(&interpreters_sem);
> >  		kfree(path_buffer);
> >  	}
> > -	if (unlikely(nosuid_enabled &&
> > -		     !uid_eq(bprm->cred->uid, bprm->cred->euid))) {
> > +	if (nosuid_enabled) {
> >  		char *path_p;
> >  		char *path_buffer;
> >  		uid_t cur_uid;
> > +		uid_t cur_euid = from_kuid(bprm->cred->user_ns, bprm->cred->euid);
> > +		/* Check for any caps for non-superuser. */
> > +		if (cur_euid != (uid_t) 0
> > +		   && has_any_caps(bprm->cred)) {
> 
> Логика этого не понятна. Было бы неплохо чтоб она была пояснена в
> комментариях чтоб и простые люди могли её понять.
> 
> Почему caps проверяются только для non-superuser? Ведь и superuser может
> сбрасывать capabilities. У superuser может не быть каких-то capabilities
> вплоть до никаких, кроме его uid 0. Следовательно, установка new
> capabilities не должна зависеть от uid.

Вероятно, сбрасывать caps следует и для процессов с euid == 0, а
проверять для них их наличие полагаю излишним.

> Далее, capabilities могут устанавливаться не все, но если в этой
> проверке были обнаружены capabilities, то далее suid игнорируются
> и не сбрасываются если они были. Что будет если установлены
> одновременно и setcap, и suid?

Не понял этой мысли, по коду caps проверяются только есть *не* был
обнаружен suid.

> Неплохо было бы, чтоб это все было пояснено в комментариях. Например,
> почему не важно не сбрасывать suid при setcap.
> 
> Thanks,
> 
> > +			is_set_caps = 1;
> > +			set_uid_cap = "setcap";
> > +		} else if (uid_eq(bprm->cred->uid, bprm->cred->euid))
> > +			return 0;
> >  
> >  		path_buffer = kmalloc(PATH_MAX, GFP_KERNEL);
> >  		if (!path_buffer)
> > @@ -283,8 +301,8 @@ static int altha_bprm_creds_from_file(struct linux_binprm *bprm, struct file * f
> >  		list_for_each_entry(node, &nosuid_exceptions_list, list) {
> >  			if (strcmp(path_p, node->spath) == 0) {
> >  				pr_notice_ratelimited
> > -				    ("AltHa/NoSUID: %s permitted to setuid from %d\n",
> > -				     bprm->filename, cur_uid);
> > +				    ("AltHa/NoSUID: %s permitted to %s from %d\n",
> > +				     bprm->filename, set_uid_cap, cur_uid);
> >  				up_read(&nosuid_exceptions_sem);
> >  				kfree(path_buffer);
> >  				return 0;
> > @@ -292,9 +310,14 @@ static int altha_bprm_creds_from_file(struct linux_binprm *bprm, struct file * f
> >  		}
> >  		up_read(&nosuid_exceptions_sem);
> >  		pr_notice_ratelimited
> > -		    ("AltHa/NoSUID: %s prevented to setuid from %d\n",
> > -		     bprm->filename, cur_uid);
> > -		bprm->cred->euid = bprm->cred->uid;
> > +		    ("AltHa/NoSUID: %s prevented to %s from %d\n",
> > +		     bprm->filename, set_uid_cap, cur_uid);
> > +		if (is_set_caps) {
> > +			cap_clear(bprm->cred->cap_inheritable);
> > +			cap_clear(bprm->cred->cap_permitted);
> > +			cap_clear(bprm->cred->cap_effective);
> > +		} else
> > +			bprm->cred->euid = bprm->cred->uid;
> >  		kfree(path_buffer);
> >  	}
> >  	return 0;
> > -- 
> > 2.33.2

-- 
   WBR,
   Vladimir D. Seleznev

[d-kernel] [PATCH] AltHa: nosuid handles capabilities as well

[Vladimir D. Seleznev]

* Documentation/admin-guide/LSM/AltHa.rst: Update documentation.
* security/altha/altha_lsm.c (altha_bprm_creds_from_file): Cover
capabilities.
(has_any_caps): New static function.

Signed-off-by: Vladimir D. Seleznev <vseleznv@altlinux.org>
---
 Documentation/admin-guide/LSM/AltHa.rst |  6 ++--
 security/altha/altha_lsm.c              | 46 ++++++++++++++++++++-----
 2 files changed, 41 insertions(+), 11 deletions(-)

diff --git a/Documentation/admin-guide/LSM/AltHa.rst b/Documentation/admin-guide/LSM/AltHa.rst
index be698709d3f0..beda40601c9e 100644
--- a/Documentation/admin-guide/LSM/AltHa.rst
+++ b/Documentation/admin-guide/LSM/AltHa.rst
@@ -3,7 +3,7 @@ AltHa
 ====
 
 AltHa is a Linux Security Module currently has three userspace hardening options:
-    * ignore SUID on binaries (with exceptions possible);
+    * ignore SUID and setcaps on binaries (with exceptions possible);
     * prevent running selected script interpreters in interactive mode;
     * disable open file unlinking in selected dirs.
     * enable kiosk mode
@@ -15,12 +15,12 @@ through sysctls in ``/proc/sys/kernel/altha``.
 
 NoSUID
 ============
-Modern Linux systems can be used with minimal (or even zero at least for OWL and ALT) usage of SUID programms, but in many cases in full-featured desktop or server systems there are plenty of them: uncounted and sometimes unnecessary. Privileged programms are always an attack surface, but mounting filesystems with ``nosuid`` flag doesn't provide enough granularity in SUID binaries management. This LSM module provides a single control point for all SUID binaries. When this submodule is enabled, SUID bits on all binaries except explicitly listed are system-wide ignored.
+Modern Linux systems can be used with minimal (or even zero at least for OWL and ALT) usage of SUID programms, but in many cases in full-featured desktop or server systems there are plenty of them: uncounted and sometimes unnecessary. Privileged programms are always an attack surface, but mounting filesystems with ``nosuid`` flag doesn't provide enough granularity in SUID binaries management. This LSM module provides a single control point for all SUID and setcap binaries. When this submodule is enabled, SUID and setcap bits on all binaries except explicitly listed are system-wide ignored.
 
 Sysctl parameters and defaults:
 
 * ``kernel.altha.nosuid.enabled = 0``, set to 1 to enable
-* ``kernel.altha.nosuid.exceptions =``, colon-separated list of enabled SUID binaries, for example: ``/bin/su:/usr/libexec/hasher-priv/hasher-priv``
+* ``kernel.altha.nosuid.exceptions =``, colon-separated list of enabled SUID and setcap binaries, for example: ``/bin/su:/usr/libexec/hasher-priv/hasher-priv``
 
 RestrScript
 ============
diff --git a/security/altha/altha_lsm.c b/security/altha/altha_lsm.c
index c670ad7ed458..d5863621212b 100644
--- a/security/altha/altha_lsm.c
+++ b/security/altha/altha_lsm.c
@@ -11,6 +11,7 @@
 
 #include <linux/lsm_hooks.h>
 #include <linux/cred.h>
+#include <linux/capability.h>
 #include <linux/sysctl.h>
 #include <linux/binfmts.h>
 #include <linux/file.h>
@@ -237,10 +238,19 @@ int is_olock_dir(struct inode *inode)
 	return 0;
 }
 
+static int has_any_caps(struct cred *cred)
+{
+	return !cap_isclear(cred->cap_permitted) ||
+	       !cap_isclear(cred->cap_effective);
+
+	return 0;
+}
+
 /* Hooks */
 static int altha_bprm_creds_from_file(struct linux_binprm *bprm, struct file * fi)
 {
 	struct altha_list_struct *node;
+	char *setuidcap_srt = "setuid";
 	/* when it's not a shebang issued script interpreter */
 	if (rstrscript_enabled && bprm->executable == bprm->interpreter) {
 		char *path_p;
@@ -267,11 +277,27 @@ static int altha_bprm_creds_from_file(struct linux_binprm *bprm, struct file * f
 		up_read(&interpreters_sem);
 		kfree(path_buffer);
 	}
-	if (unlikely(nosuid_enabled &&
-		     !uid_eq(bprm->cred->uid, bprm->cred->euid))) {
+	if (nosuid_enabled) {
 		char *path_p;
 		char *path_buffer;
-		uid_t cur_uid;
+		int is_setuid = 0, is_setcap = 0;
+		uid_t cur_uid, cur_euid;
+
+		is_setuid = !uid_eq(bprm->cred->uid, bprm->cred->euid);
+
+		if (!is_setuid)
+			is_setcap = has_any_caps(bprm->cred);
+
+		/*
+		 * If no suid but it has any caps, change message string from
+		 * setuid to setcap.
+		 */
+		if (is_setcap)
+			setuidcap_srt = "setcap";
+
+		/* If no suid and no caps detected, exit. */
+		if (!is_setuid && !is_setcap)
+			return 0;
 
 		path_buffer = kmalloc(PATH_MAX, GFP_KERNEL);
 		if (!path_buffer)
@@ -283,8 +309,8 @@ static int altha_bprm_creds_from_file(struct linux_binprm *bprm, struct file * f
 		list_for_each_entry(node, &nosuid_exceptions_list, list) {
 			if (strcmp(path_p, node->spath) == 0) {
 				pr_notice_ratelimited
-				    ("AltHa/NoSUID: %s permitted to setuid from %d\n",
-				     bprm->filename, cur_uid);
+				    ("AltHa/NoSUID: %s permitted to %s from %d\n",
+				     bprm->filename, setuidcap_srt, cur_uid);
 				up_read(&nosuid_exceptions_sem);
 				kfree(path_buffer);
 				return 0;
@@ -292,9 +318,13 @@ static int altha_bprm_creds_from_file(struct linux_binprm *bprm, struct file * f
 		}
 		up_read(&nosuid_exceptions_sem);
 		pr_notice_ratelimited
-		    ("AltHa/NoSUID: %s prevented to setuid from %d\n",
-		     bprm->filename, cur_uid);
-		bprm->cred->euid = bprm->cred->uid;
+		    ("AltHa/NoSUID: %s prevented to %s from %d\n",
+		     bprm->filename, setuidcap_srt, cur_uid);
+		if (cur_euid == (uid_t) 0)
+			bprm->cred->euid = bprm->cred->uid;
+		cap_clear(bprm->cred->cap_inheritable);
+		cap_clear(bprm->cred->cap_permitted);
+		cap_clear(bprm->cred->cap_effective);
 		kfree(path_buffer);
 	}
 	return 0;
-- 
2.33.3

[d-kernel] AltHa Covers capabilities

[Vladimir D. Seleznev]

Quickfix

[d-kernel] [PATCH] AltHa: nosuid handles capabilities as well

[Vladimir D. Seleznev]

* Documentation/admin-guide/LSM/AltHa.rst: Update documentation.
* security/altha/altha_lsm.c (altha_bprm_creds_from_file): Cover
capabilities.
(has_any_caps): New static function.

Signed-off-by: Vladimir D. Seleznev <vseleznv@altlinux.org>
---
 Documentation/admin-guide/LSM/AltHa.rst |  6 ++--
 security/altha/altha_lsm.c              | 46 ++++++++++++++++++++-----
 2 files changed, 41 insertions(+), 11 deletions(-)

diff --git a/Documentation/admin-guide/LSM/AltHa.rst b/Documentation/admin-guide/LSM/AltHa.rst
index be698709d3f0..beda40601c9e 100644
--- a/Documentation/admin-guide/LSM/AltHa.rst
+++ b/Documentation/admin-guide/LSM/AltHa.rst
@@ -3,7 +3,7 @@ AltHa
 ====
 
 AltHa is a Linux Security Module currently has three userspace hardening options:
-    * ignore SUID on binaries (with exceptions possible);
+    * ignore SUID and setcaps on binaries (with exceptions possible);
     * prevent running selected script interpreters in interactive mode;
     * disable open file unlinking in selected dirs.
     * enable kiosk mode
@@ -15,12 +15,12 @@ through sysctls in ``/proc/sys/kernel/altha``.
 
 NoSUID
 ============
-Modern Linux systems can be used with minimal (or even zero at least for OWL and ALT) usage of SUID programms, but in many cases in full-featured desktop or server systems there are plenty of them: uncounted and sometimes unnecessary. Privileged programms are always an attack surface, but mounting filesystems with ``nosuid`` flag doesn't provide enough granularity in SUID binaries management. This LSM module provides a single control point for all SUID binaries. When this submodule is enabled, SUID bits on all binaries except explicitly listed are system-wide ignored.
+Modern Linux systems can be used with minimal (or even zero at least for OWL and ALT) usage of SUID programms, but in many cases in full-featured desktop or server systems there are plenty of them: uncounted and sometimes unnecessary. Privileged programms are always an attack surface, but mounting filesystems with ``nosuid`` flag doesn't provide enough granularity in SUID binaries management. This LSM module provides a single control point for all SUID and setcap binaries. When this submodule is enabled, SUID and setcap bits on all binaries except explicitly listed are system-wide ignored.
 
 Sysctl parameters and defaults:
 
 * ``kernel.altha.nosuid.enabled = 0``, set to 1 to enable
-* ``kernel.altha.nosuid.exceptions =``, colon-separated list of enabled SUID binaries, for example: ``/bin/su:/usr/libexec/hasher-priv/hasher-priv``
+* ``kernel.altha.nosuid.exceptions =``, colon-separated list of enabled SUID and setcap binaries, for example: ``/bin/su:/usr/libexec/hasher-priv/hasher-priv``
 
 RestrScript
 ============
diff --git a/security/altha/altha_lsm.c b/security/altha/altha_lsm.c
index c670ad7ed458..cdc9abdac0df 100644
--- a/security/altha/altha_lsm.c
+++ b/security/altha/altha_lsm.c
@@ -11,6 +11,7 @@
 
 #include <linux/lsm_hooks.h>
 #include <linux/cred.h>
+#include <linux/capability.h>
 #include <linux/sysctl.h>
 #include <linux/binfmts.h>
 #include <linux/file.h>
@@ -237,10 +238,19 @@ int is_olock_dir(struct inode *inode)
 	return 0;
 }
 
+static int has_any_caps(struct cred *cred)
+{
+	return !cap_isclear(cred->cap_permitted) ||
+	       !cap_isclear(cred->cap_effective);
+
+	return 0;
+}
+
 /* Hooks */
 static int altha_bprm_creds_from_file(struct linux_binprm *bprm, struct file * fi)
 {
 	struct altha_list_struct *node;
+	char *setuidcap_srt = "setuid";
 	/* when it's not a shebang issued script interpreter */
 	if (rstrscript_enabled && bprm->executable == bprm->interpreter) {
 		char *path_p;
@@ -267,11 +277,27 @@ static int altha_bprm_creds_from_file(struct linux_binprm *bprm, struct file * f
 		up_read(&interpreters_sem);
 		kfree(path_buffer);
 	}
-	if (unlikely(nosuid_enabled &&
-		     !uid_eq(bprm->cred->uid, bprm->cred->euid))) {
+	if (nosuid_enabled) {
 		char *path_p;
 		char *path_buffer;
-		uid_t cur_uid;
+		int is_setuid = 0, is_setcap = 0;
+		uid_t cur_uid, cur_euid;
+
+		is_setuid = !uid_eq(bprm->cred->uid, bprm->cred->euid);
+
+		if (!is_setuid)
+			is_setcap = has_any_caps(bprm->cred);
+
+		/*
+		 * If no suid but it has any caps, change message string from
+		 * setuid to setcap.
+		 */
+		if (is_setcap)
+			setuidcap_srt = "setcap";
+
+		/* If no suid and no caps detected, exit. */
+		if (!is_setuid && !is_setcap)
+			return 0;
 
 		path_buffer = kmalloc(PATH_MAX, GFP_KERNEL);
 		if (!path_buffer)
@@ -283,8 +309,8 @@ static int altha_bprm_creds_from_file(struct linux_binprm *bprm, struct file * f
 		list_for_each_entry(node, &nosuid_exceptions_list, list) {
 			if (strcmp(path_p, node->spath) == 0) {
 				pr_notice_ratelimited
-				    ("AltHa/NoSUID: %s permitted to setuid from %d\n",
-				     bprm->filename, cur_uid);
+				    ("AltHa/NoSUID: %s permitted to %s from %d\n",
+				     bprm->filename, setuidcap_srt, cur_uid);
 				up_read(&nosuid_exceptions_sem);
 				kfree(path_buffer);
 				return 0;
@@ -292,9 +318,13 @@ static int altha_bprm_creds_from_file(struct linux_binprm *bprm, struct file * f
 		}
 		up_read(&nosuid_exceptions_sem);
 		pr_notice_ratelimited
-		    ("AltHa/NoSUID: %s prevented to setuid from %d\n",
-		     bprm->filename, cur_uid);
-		bprm->cred->euid = bprm->cred->uid;
+		    ("AltHa/NoSUID: %s prevented to %s from %d\n",
+		     bprm->filename, setuidcap_srt, cur_uid);
+		if (is_setuid)
+			bprm->cred->euid = bprm->cred->uid;
+		cap_clear(bprm->cred->cap_inheritable);
+		cap_clear(bprm->cred->cap_permitted);
+		cap_clear(bprm->cred->cap_effective);
 		kfree(path_buffer);
 	}
 	return 0;
-- 
2.33.3

Re: [d-kernel] [PATCH] AltHa: nosuid handles capabilities as well

[Vitaly Chikunov]

On Thu, Apr 28, 2022 at 12:42:52PM +0300, Vladimir D. Seleznev wrote:
> On Wed, Apr 27, 2022 at 12:09:44AM +0300, Vitaly Chikunov wrote:
> > Vladimir,
> > 
> > On Tue, Apr 26, 2022 at 09:47:33AM +0000, Vladimir D. Seleznev wrote:
> > > Signed-off-by: Vladimir D. Seleznev <vseleznv@altlinux.org>
> > > ---
> > >  Documentation/admin-guide/LSM/AltHa.rst |  6 ++--
> > >  security/altha/altha_lsm.c              | 37 ++++++++++++++++++++-----
> > >  2 files changed, 33 insertions(+), 10 deletions(-)
> > > 
> > > diff --git a/Documentation/admin-guide/LSM/AltHa.rst b/Documentation/admin-guide/LSM/AltHa.rst
> > > index be698709d3f0..beda40601c9e 100644
> > > --- a/Documentation/admin-guide/LSM/AltHa.rst
> > > +++ b/Documentation/admin-guide/LSM/AltHa.rst
> > > @@ -3,7 +3,7 @@ AltHa
> > >  ====
> > >  
> > >  AltHa is a Linux Security Module currently has three userspace hardening options:
> > > -    * ignore SUID on binaries (with exceptions possible);
> > > +    * ignore SUID and setcaps on binaries (with exceptions possible);
> > >      * prevent running selected script interpreters in interactive mode;
> > >      * disable open file unlinking in selected dirs.
> > >      * enable kiosk mode
> > > @@ -15,12 +15,12 @@ through sysctls in ``/proc/sys/kernel/altha``.
> > >  
> > >  NoSUID
> > >  ============
> > > -Modern Linux systems can be used with minimal (or even zero at least for OWL and ALT) usage of SUID programms, but in many cases in full-featured desktop or server systems there are plenty of them: uncounted and sometimes unnecessary. Privileged programms are always an attack surface, but mounting filesystems with ``nosuid`` flag doesn't provide enough granularity in SUID binaries management. This LSM module provides a single control point for all SUID binaries. When this submodule is enabled, SUID bits on all binaries except explicitly listed are system-wide ignored.
> > > +Modern Linux systems can be used with minimal (or even zero at least for OWL and ALT) usage of SUID programms, but in many cases in full-featured desktop or server systems there are plenty of them: uncounted and sometimes unnecessary. Privileged programms are always an attack surface, but mounting filesystems with ``nosuid`` flag doesn't provide enough granularity in SUID binaries management. This LSM module provides a single control point for all SUID and setcap binaries. When this submodule is enabled, SUID and setcap bits on all binaries except explicitly listed are system-wide ignored.
> > >  
> > >  Sysctl parameters and defaults:
> > >  
> > >  * ``kernel.altha.nosuid.enabled = 0``, set to 1 to enable
> > > -* ``kernel.altha.nosuid.exceptions =``, colon-separated list of enabled SUID binaries, for example: ``/bin/su:/usr/libexec/hasher-priv/hasher-priv``
> > > +* ``kernel.altha.nosuid.exceptions =``, colon-separated list of enabled SUID and setcap binaries, for example: ``/bin/su:/usr/libexec/hasher-priv/hasher-priv``
> > >  
> > >  RestrScript
> > >  ============
> > > diff --git a/security/altha/altha_lsm.c b/security/altha/altha_lsm.c
> > > index c670ad7ed458..5f0505a51644 100644
> > > --- a/security/altha/altha_lsm.c
> > > +++ b/security/altha/altha_lsm.c
> > > @@ -11,6 +11,7 @@
> > >  
> > >  #include <linux/lsm_hooks.h>
> > >  #include <linux/cred.h>
> > > +#include <linux/capability.h>
> > >  #include <linux/sysctl.h>
> > >  #include <linux/binfmts.h>
> > >  #include <linux/file.h>
> > > @@ -237,10 +238,20 @@ int is_olock_dir(struct inode *inode)
> > >  	return 0;
> > >  }
> > >  
> > > +static int has_any_caps(struct cred *cred)
> > > +{
> > > +	return !cap_isclear(cred->cap_permitted) ||
> > > +	       !cap_isclear(cred->cap_effective);
> > > +
> > > +	return 0;
> > > +}
> > > +
> > >  /* Hooks */
> > >  static int altha_bprm_creds_from_file(struct linux_binprm *bprm, struct file * fi)
> > >  {
> > >  	struct altha_list_struct *node;
> > > +	int is_set_caps = 0;
> > > +	char *set_uid_cap = "setuid";
> > >  	/* when it's not a shebang issued script interpreter */
> > >  	if (rstrscript_enabled && bprm->executable == bprm->interpreter) {
> > >  		char *path_p;
> > > @@ -267,11 +278,18 @@ static int altha_bprm_creds_from_file(struct linux_binprm *bprm, struct file * f
> > >  		up_read(&interpreters_sem);
> > >  		kfree(path_buffer);
> > >  	}
> > > -	if (unlikely(nosuid_enabled &&
> > > -		     !uid_eq(bprm->cred->uid, bprm->cred->euid))) {
> > > +	if (nosuid_enabled) {
> > >  		char *path_p;
> > >  		char *path_buffer;
> > >  		uid_t cur_uid;
> > > +		uid_t cur_euid = from_kuid(bprm->cred->user_ns, bprm->cred->euid);
> > > +		/* Check for any caps for non-superuser. */
> > > +		if (cur_euid != (uid_t) 0
> > > +		   && has_any_caps(bprm->cred)) {
> > 
> > Логика этого не понятна. Было бы неплохо чтоб она была пояснена в
> > комментариях чтоб и простые люди могли её понять.
> > 
> > Почему caps проверяются только для non-superuser? Ведь и superuser может
> > сбрасывать capabilities. У superuser может не быть каких-то capabilities
> > вплоть до никаких, кроме его uid 0. Следовательно, установка new
> > capabilities не должна зависеть от uid.
> 
> Вероятно, сбрасывать caps следует и для процессов с euid == 0, а
> проверять для них их наличие полагаю излишним.
> 
> > Далее, capabilities могут устанавливаться не все, но если в этой
> > проверке были обнаружены capabilities, то далее suid игнорируются
> > и не сбрасываются если они были. Что будет если установлены
> > одновременно и setcap, и suid?
> 
> Не понял этой мысли, по коду caps проверяются только есть *не* был
> обнаружен suid.

Почему?

> 
> > Неплохо было бы, чтоб это все было пояснено в комментариях. Например,
> > почему не важно не сбрасывать suid при setcap.
> > 
> > Thanks,
> > 
> > > +			is_set_caps = 1;
> > > +			set_uid_cap = "setcap";
> > > +		} else if (uid_eq(bprm->cred->uid, bprm->cred->euid))
> > > +			return 0;
> > >  
> > >  		path_buffer = kmalloc(PATH_MAX, GFP_KERNEL);
> > >  		if (!path_buffer)
> > > @@ -283,8 +301,8 @@ static int altha_bprm_creds_from_file(struct linux_binprm *bprm, struct file * f
> > >  		list_for_each_entry(node, &nosuid_exceptions_list, list) {
> > >  			if (strcmp(path_p, node->spath) == 0) {
> > >  				pr_notice_ratelimited
> > > -				    ("AltHa/NoSUID: %s permitted to setuid from %d\n",
> > > -				     bprm->filename, cur_uid);
> > > +				    ("AltHa/NoSUID: %s permitted to %s from %d\n",
> > > +				     bprm->filename, set_uid_cap, cur_uid);
> > >  				up_read(&nosuid_exceptions_sem);
> > >  				kfree(path_buffer);
> > >  				return 0;
> > > @@ -292,9 +310,14 @@ static int altha_bprm_creds_from_file(struct linux_binprm *bprm, struct file * f
> > >  		}
> > >  		up_read(&nosuid_exceptions_sem);
> > >  		pr_notice_ratelimited
> > > -		    ("AltHa/NoSUID: %s prevented to setuid from %d\n",
> > > -		     bprm->filename, cur_uid);
> > > -		bprm->cred->euid = bprm->cred->uid;
> > > +		    ("AltHa/NoSUID: %s prevented to %s from %d\n",
> > > +		     bprm->filename, set_uid_cap, cur_uid);
> > > +		if (is_set_caps) {
> > > +			cap_clear(bprm->cred->cap_inheritable);
> > > +			cap_clear(bprm->cred->cap_permitted);
> > > +			cap_clear(bprm->cred->cap_effective);
> > > +		} else
> > > +			bprm->cred->euid = bprm->cred->uid;
> > >  		kfree(path_buffer);
> > >  	}
> > >  	return 0;
> > > -- 
> > > 2.33.2
> 
> -- 
>    WBR,
>    Vladimir D. Seleznev
> _______________________________________________
> devel-kernel mailing list
> devel-kernel@lists.altlinux.org
> https://lists.altlinux.org/mailman/listinfo/devel-kernel

Re: [d-kernel] [PATCH] AltHa: nosuid handles capabilities as well

[Vladimir D. Seleznev]

On Fri, Apr 29, 2022 at 01:18:47AM +0300, Vitaly Chikunov wrote:
> On Thu, Apr 28, 2022 at 12:42:52PM +0300, Vladimir D. Seleznev wrote:
> > On Wed, Apr 27, 2022 at 12:09:44AM +0300, Vitaly Chikunov wrote:
> > > Vladimir,
> > > 
> > > On Tue, Apr 26, 2022 at 09:47:33AM +0000, Vladimir D. Seleznev wrote:
> > > > Signed-off-by: Vladimir D. Seleznev <vseleznv@altlinux.org>
> > > > ---
> > > >  Documentation/admin-guide/LSM/AltHa.rst |  6 ++--
> > > >  security/altha/altha_lsm.c              | 37 ++++++++++++++++++++-----
> > > >  2 files changed, 33 insertions(+), 10 deletions(-)
> > > > 
> > > > diff --git a/Documentation/admin-guide/LSM/AltHa.rst b/Documentation/admin-guide/LSM/AltHa.rst
> > > > index be698709d3f0..beda40601c9e 100644
> > > > --- a/Documentation/admin-guide/LSM/AltHa.rst
> > > > +++ b/Documentation/admin-guide/LSM/AltHa.rst
> > > > @@ -3,7 +3,7 @@ AltHa
> > > >  ====
> > > >  
> > > >  AltHa is a Linux Security Module currently has three userspace hardening options:
> > > > -    * ignore SUID on binaries (with exceptions possible);
> > > > +    * ignore SUID and setcaps on binaries (with exceptions possible);
> > > >      * prevent running selected script interpreters in interactive mode;
> > > >      * disable open file unlinking in selected dirs.
> > > >      * enable kiosk mode
> > > > @@ -15,12 +15,12 @@ through sysctls in ``/proc/sys/kernel/altha``.
> > > >  
> > > >  NoSUID
> > > >  ============
> > > > -Modern Linux systems can be used with minimal (or even zero at least for OWL and ALT) usage of SUID programms, but in many cases in full-featured desktop or server systems there are plenty of them: uncounted and sometimes unnecessary. Privileged programms are always an attack surface, but mounting filesystems with ``nosuid`` flag doesn't provide enough granularity in SUID binaries management. This LSM module provides a single control point for all SUID binaries. When this submodule is enabled, SUID bits on all binaries except explicitly listed are system-wide ignored.
> > > > +Modern Linux systems can be used with minimal (or even zero at least for OWL and ALT) usage of SUID programms, but in many cases in full-featured desktop or server systems there are plenty of them: uncounted and sometimes unnecessary. Privileged programms are always an attack surface, but mounting filesystems with ``nosuid`` flag doesn't provide enough granularity in SUID binaries management. This LSM module provides a single control point for all SUID and setcap binaries. When this submodule is enabled, SUID and setcap bits on all binaries except explicitly listed are system-wide ignored.
> > > >  
> > > >  Sysctl parameters and defaults:
> > > >  
> > > >  * ``kernel.altha.nosuid.enabled = 0``, set to 1 to enable
> > > > -* ``kernel.altha.nosuid.exceptions =``, colon-separated list of enabled SUID binaries, for example: ``/bin/su:/usr/libexec/hasher-priv/hasher-priv``
> > > > +* ``kernel.altha.nosuid.exceptions =``, colon-separated list of enabled SUID and setcap binaries, for example: ``/bin/su:/usr/libexec/hasher-priv/hasher-priv``
> > > >  
> > > >  RestrScript
> > > >  ============
> > > > diff --git a/security/altha/altha_lsm.c b/security/altha/altha_lsm.c
> > > > index c670ad7ed458..5f0505a51644 100644
> > > > --- a/security/altha/altha_lsm.c
> > > > +++ b/security/altha/altha_lsm.c
> > > > @@ -11,6 +11,7 @@
> > > >  
> > > >  #include <linux/lsm_hooks.h>
> > > >  #include <linux/cred.h>
> > > > +#include <linux/capability.h>
> > > >  #include <linux/sysctl.h>
> > > >  #include <linux/binfmts.h>
> > > >  #include <linux/file.h>
> > > > @@ -237,10 +238,20 @@ int is_olock_dir(struct inode *inode)
> > > >  	return 0;
> > > >  }
> > > >  
> > > > +static int has_any_caps(struct cred *cred)
> > > > +{
> > > > +	return !cap_isclear(cred->cap_permitted) ||
> > > > +	       !cap_isclear(cred->cap_effective);
> > > > +
> > > > +	return 0;
> > > > +}
> > > > +
> > > >  /* Hooks */
> > > >  static int altha_bprm_creds_from_file(struct linux_binprm *bprm, struct file * fi)
> > > >  {
> > > >  	struct altha_list_struct *node;
> > > > +	int is_set_caps = 0;
> > > > +	char *set_uid_cap = "setuid";
> > > >  	/* when it's not a shebang issued script interpreter */
> > > >  	if (rstrscript_enabled && bprm->executable == bprm->interpreter) {
> > > >  		char *path_p;
> > > > @@ -267,11 +278,18 @@ static int altha_bprm_creds_from_file(struct linux_binprm *bprm, struct file * f
> > > >  		up_read(&interpreters_sem);
> > > >  		kfree(path_buffer);
> > > >  	}
> > > > -	if (unlikely(nosuid_enabled &&
> > > > -		     !uid_eq(bprm->cred->uid, bprm->cred->euid))) {
> > > > +	if (nosuid_enabled) {
> > > >  		char *path_p;
> > > >  		char *path_buffer;
> > > >  		uid_t cur_uid;
> > > > +		uid_t cur_euid = from_kuid(bprm->cred->user_ns, bprm->cred->euid);
> > > > +		/* Check for any caps for non-superuser. */
> > > > +		if (cur_euid != (uid_t) 0
> > > > +		   && has_any_caps(bprm->cred)) {
> > > 
> > > Логика этого не понятна. Было бы неплохо чтоб она была пояснена в
> > > комментариях чтоб и простые люди могли её понять.
> > > 
> > > Почему caps проверяются только для non-superuser? Ведь и superuser может
> > > сбрасывать capabilities. У superuser может не быть каких-то capabilities
> > > вплоть до никаких, кроме его uid 0. Следовательно, установка new
> > > capabilities не должна зависеть от uid.
> > 
> > Вероятно, сбрасывать caps следует и для процессов с euid == 0, а
> > проверять для них их наличие полагаю излишним.
> > 
> > > Далее, capabilities могут устанавливаться не все, но если в этой
> > > проверке были обнаружены capabilities, то далее suid игнорируются
> > > и не сбрасываются если они были. Что будет если установлены
> > > одновременно и setcap, и suid?
> > 
> > Не понял этой мысли, по коду caps проверяются только есть *не* был
> > обнаружен suid.
> 
> Почему?

А зачем иначе? Тем более, что cap'ы сбрасываются безусловно.

-- 
   WBR,
   Vladimir D. Seleznev

Re: [d-kernel] [PATCH] AltHa: nosuid handles capabilities as well

[Vitaly Chikunov]

On Fri, Apr 29, 2022 at 02:24:16AM +0300, Vladimir D. Seleznev wrote:
> On Fri, Apr 29, 2022 at 01:18:47AM +0300, Vitaly Chikunov wrote:
> > On Thu, Apr 28, 2022 at 12:42:52PM +0300, Vladimir D. Seleznev wrote:
> > > On Wed, Apr 27, 2022 at 12:09:44AM +0300, Vitaly Chikunov wrote:
> > > > Vladimir,
> > > > 
> > > > On Tue, Apr 26, 2022 at 09:47:33AM +0000, Vladimir D. Seleznev wrote:
> > > > > Signed-off-by: Vladimir D. Seleznev <vseleznv@altlinux.org>
> > > > > ---
> > > > >  Documentation/admin-guide/LSM/AltHa.rst |  6 ++--
> > > > >  security/altha/altha_lsm.c              | 37 ++++++++++++++++++++-----
> > > > >  2 files changed, 33 insertions(+), 10 deletions(-)
> > > > > 
> > > > > diff --git a/Documentation/admin-guide/LSM/AltHa.rst b/Documentation/admin-guide/LSM/AltHa.rst
> > > > > index be698709d3f0..beda40601c9e 100644
> > > > > --- a/Documentation/admin-guide/LSM/AltHa.rst
> > > > > +++ b/Documentation/admin-guide/LSM/AltHa.rst
> > > > > @@ -3,7 +3,7 @@ AltHa
> > > > >  ====
> > > > >  
> > > > >  AltHa is a Linux Security Module currently has three userspace hardening options:
> > > > > -    * ignore SUID on binaries (with exceptions possible);
> > > > > +    * ignore SUID and setcaps on binaries (with exceptions possible);
> > > > >      * prevent running selected script interpreters in interactive mode;
> > > > >      * disable open file unlinking in selected dirs.
> > > > >      * enable kiosk mode
> > > > > @@ -15,12 +15,12 @@ through sysctls in ``/proc/sys/kernel/altha``.
> > > > >  
> > > > >  NoSUID
> > > > >  ============
> > > > > -Modern Linux systems can be used with minimal (or even zero at least for OWL and ALT) usage of SUID programms, but in many cases in full-featured desktop or server systems there are plenty of them: uncounted and sometimes unnecessary. Privileged programms are always an attack surface, but mounting filesystems with ``nosuid`` flag doesn't provide enough granularity in SUID binaries management. This LSM module provides a single control point for all SUID binaries. When this submodule is enabled, SUID bits on all binaries except explicitly listed are system-wide ignored.
> > > > > +Modern Linux systems can be used with minimal (or even zero at least for OWL and ALT) usage of SUID programms, but in many cases in full-featured desktop or server systems there are plenty of them: uncounted and sometimes unnecessary. Privileged programms are always an attack surface, but mounting filesystems with ``nosuid`` flag doesn't provide enough granularity in SUID binaries management. This LSM module provides a single control point for all SUID and setcap binaries. When this submodule is enabled, SUID and setcap bits on all binaries except explicitly listed are system-wide ignored.
> > > > >  
> > > > >  Sysctl parameters and defaults:
> > > > >  
> > > > >  * ``kernel.altha.nosuid.enabled = 0``, set to 1 to enable
> > > > > -* ``kernel.altha.nosuid.exceptions =``, colon-separated list of enabled SUID binaries, for example: ``/bin/su:/usr/libexec/hasher-priv/hasher-priv``
> > > > > +* ``kernel.altha.nosuid.exceptions =``, colon-separated list of enabled SUID and setcap binaries, for example: ``/bin/su:/usr/libexec/hasher-priv/hasher-priv``
> > > > >  
> > > > >  RestrScript
> > > > >  ============
> > > > > diff --git a/security/altha/altha_lsm.c b/security/altha/altha_lsm.c
> > > > > index c670ad7ed458..5f0505a51644 100644
> > > > > --- a/security/altha/altha_lsm.c
> > > > > +++ b/security/altha/altha_lsm.c
> > > > > @@ -11,6 +11,7 @@
> > > > >  
> > > > >  #include <linux/lsm_hooks.h>
> > > > >  #include <linux/cred.h>
> > > > > +#include <linux/capability.h>
> > > > >  #include <linux/sysctl.h>
> > > > >  #include <linux/binfmts.h>
> > > > >  #include <linux/file.h>
> > > > > @@ -237,10 +238,20 @@ int is_olock_dir(struct inode *inode)
> > > > >  	return 0;
> > > > >  }
> > > > >  
> > > > > +static int has_any_caps(struct cred *cred)
> > > > > +{
> > > > > +	return !cap_isclear(cred->cap_permitted) ||
> > > > > +	       !cap_isclear(cred->cap_effective);
> > > > > +
> > > > > +	return 0;
> > > > > +}
> > > > > +
> > > > >  /* Hooks */
> > > > >  static int altha_bprm_creds_from_file(struct linux_binprm *bprm, struct file * fi)
> > > > >  {
> > > > >  	struct altha_list_struct *node;
> > > > > +	int is_set_caps = 0;
> > > > > +	char *set_uid_cap = "setuid";
> > > > >  	/* when it's not a shebang issued script interpreter */
> > > > >  	if (rstrscript_enabled && bprm->executable == bprm->interpreter) {
> > > > >  		char *path_p;
> > > > > @@ -267,11 +278,18 @@ static int altha_bprm_creds_from_file(struct linux_binprm *bprm, struct file * f
> > > > >  		up_read(&interpreters_sem);
> > > > >  		kfree(path_buffer);
> > > > >  	}
> > > > > -	if (unlikely(nosuid_enabled &&
> > > > > -		     !uid_eq(bprm->cred->uid, bprm->cred->euid))) {
> > > > > +	if (nosuid_enabled) {
> > > > >  		char *path_p;
> > > > >  		char *path_buffer;
> > > > >  		uid_t cur_uid;
> > > > > +		uid_t cur_euid = from_kuid(bprm->cred->user_ns, bprm->cred->euid);
> > > > > +		/* Check for any caps for non-superuser. */
> > > > > +		if (cur_euid != (uid_t) 0
> > > > > +		   && has_any_caps(bprm->cred)) {
> > > > 
> > > > Логика этого не понятна. Было бы неплохо чтоб она была пояснена в
> > > > комментариях чтоб и простые люди могли её понять.
> > > > 
> > > > Почему caps проверяются только для non-superuser? Ведь и superuser может
> > > > сбрасывать capabilities. У superuser может не быть каких-то capabilities
> > > > вплоть до никаких, кроме его uid 0. Следовательно, установка new
> > > > capabilities не должна зависеть от uid.
> > > 
> > > Вероятно, сбрасывать caps следует и для процессов с euid == 0, а
> > > проверять для них их наличие полагаю излишним.
> > > 
> > > > Далее, capabilities могут устанавливаться не все, но если в этой
> > > > проверке были обнаружены capabilities, то далее suid игнорируются
> > > > и не сбрасываются если они были. Что будет если установлены
> > > > одновременно и setcap, и suid?
> > > 
> > > Не понял этой мысли, по коду caps проверяются только есть *не* был
> > > обнаружен suid.
> > 
> > Почему?
> 
> А зачем иначе? Тем более, что cap'ы сбрасываются безусловно.

А ты уже поменял код, но не написал про это изменение.

> 
> -- 
>    WBR,
>    Vladimir D. Seleznev
> _______________________________________________
> devel-kernel mailing list
> devel-kernel@lists.altlinux.org
> https://lists.altlinux.org/mailman/listinfo/devel-kernel

Re: [d-kernel] [PATCH] AltHa: nosuid handles capabilities as well

[Vitaly Chikunov]

Vladimir,

On Thu, Apr 28, 2022 at 02:59:55PM +0000, Vladimir D. Seleznev wrote:
> * Documentation/admin-guide/LSM/AltHa.rst: Update documentation.
> * security/altha/altha_lsm.c (altha_bprm_creds_from_file): Cover
> capabilities.
> (has_any_caps): New static function.
> 
> Signed-off-by: Vladimir D. Seleznev <vseleznv@altlinux.org>
> ---
>  Documentation/admin-guide/LSM/AltHa.rst |  6 ++--
>  security/altha/altha_lsm.c              | 46 ++++++++++++++++++++-----
>  2 files changed, 41 insertions(+), 11 deletions(-)
> 
> diff --git a/Documentation/admin-guide/LSM/AltHa.rst b/Documentation/admin-guide/LSM/AltHa.rst
> index be698709d3f0..beda40601c9e 100644
> --- a/Documentation/admin-guide/LSM/AltHa.rst
> +++ b/Documentation/admin-guide/LSM/AltHa.rst
> @@ -3,7 +3,7 @@ AltHa
>  ====
>  
>  AltHa is a Linux Security Module currently has three userspace hardening options:
> -    * ignore SUID on binaries (with exceptions possible);
> +    * ignore SUID and setcaps on binaries (with exceptions possible);
>      * prevent running selected script interpreters in interactive mode;
>      * disable open file unlinking in selected dirs.
>      * enable kiosk mode
> @@ -15,12 +15,12 @@ through sysctls in ``/proc/sys/kernel/altha``.
>  
>  NoSUID
>  ============
> -Modern Linux systems can be used with minimal (or even zero at least for OWL and ALT) usage of SUID programms, but in many cases in full-featured desktop or server systems there are plenty of them: uncounted and sometimes unnecessary. Privileged programms are always an attack surface, but mounting filesystems with ``nosuid`` flag doesn't provide enough granularity in SUID binaries management. This LSM module provides a single control point for all SUID binaries. When this submodule is enabled, SUID bits on all binaries except explicitly listed are system-wide ignored.
> +Modern Linux systems can be used with minimal (or even zero at least for OWL and ALT) usage of SUID programms, but in many cases in full-featured desktop or server systems there are plenty of them: uncounted and sometimes unnecessary. Privileged programms are always an attack surface, but mounting filesystems with ``nosuid`` flag doesn't provide enough granularity in SUID binaries management. This LSM module provides a single control point for all SUID and setcap binaries. When this submodule is enabled, SUID and setcap bits on all binaries except explicitly listed are system-wide ignored.
>  
>  Sysctl parameters and defaults:
>  
>  * ``kernel.altha.nosuid.enabled = 0``, set to 1 to enable
> -* ``kernel.altha.nosuid.exceptions =``, colon-separated list of enabled SUID binaries, for example: ``/bin/su:/usr/libexec/hasher-priv/hasher-priv``
> +* ``kernel.altha.nosuid.exceptions =``, colon-separated list of enabled SUID and setcap binaries, for example: ``/bin/su:/usr/libexec/hasher-priv/hasher-priv``
>  
>  RestrScript
>  ============
> diff --git a/security/altha/altha_lsm.c b/security/altha/altha_lsm.c
> index c670ad7ed458..cdc9abdac0df 100644
> --- a/security/altha/altha_lsm.c
> +++ b/security/altha/altha_lsm.c
> @@ -11,6 +11,7 @@
>  
>  #include <linux/lsm_hooks.h>
>  #include <linux/cred.h>
> +#include <linux/capability.h>
>  #include <linux/sysctl.h>
>  #include <linux/binfmts.h>
>  #include <linux/file.h>
> @@ -237,10 +238,19 @@ int is_olock_dir(struct inode *inode)
>  	return 0;
>  }
>  
> +static int has_any_caps(struct cred *cred)
> +{
> +	return !cap_isclear(cred->cap_permitted) ||
> +	       !cap_isclear(cred->cap_effective);
> +
> +	return 0;
> +}
> +
>  /* Hooks */
>  static int altha_bprm_creds_from_file(struct linux_binprm *bprm, struct file * fi)
>  {
>  	struct altha_list_struct *node;
> +	char *setuidcap_srt = "setuid";
>  	/* when it's not a shebang issued script interpreter */
>  	if (rstrscript_enabled && bprm->executable == bprm->interpreter) {
>  		char *path_p;
> @@ -267,11 +277,27 @@ static int altha_bprm_creds_from_file(struct linux_binprm *bprm, struct file * f
>  		up_read(&interpreters_sem);
>  		kfree(path_buffer);
>  	}
> -	if (unlikely(nosuid_enabled &&
> -		     !uid_eq(bprm->cred->uid, bprm->cred->euid))) {
> +	if (nosuid_enabled) {
>  		char *path_p;
>  		char *path_buffer;
> -		uid_t cur_uid;
> +		int is_setuid = 0, is_setcap = 0;
> +		uid_t cur_uid, cur_euid;


  CC      security/altha/altha_lsm.o
security/altha/altha_lsm.c: In function 'altha_bprm_creds_from_file':
security/altha/altha_lsm.c:284:32: error: unused variable 'cur_euid' [-Werror=unused-variable]
  284 |                 uid_t cur_uid, cur_euid;
      |                                ^~~~~~~~
cc1: all warnings being treated as errors
make[2]: *** [scripts/Makefile.build:277: security/altha/altha_lsm.o] Error 1
make[1]: *** [scripts/Makefile.build:540: security/altha] Error 2
make: *** [Makefile:1868: security] Error 2
make: *** Waiting for unfinished jobs....

> +
> +		is_setuid = !uid_eq(bprm->cred->uid, bprm->cred->euid);
> +
> +		if (!is_setuid)
> +			is_setcap = has_any_caps(bprm->cred);
> +
> +		/*
> +		 * If no suid but it has any caps, change message string from
> +		 * setuid to setcap.
> +		 */
> +		if (is_setcap)
> +			setuidcap_srt = "setcap";
> +
> +		/* If no suid and no caps detected, exit. */
> +		if (!is_setuid && !is_setcap)
> +			return 0;
>  
>  		path_buffer = kmalloc(PATH_MAX, GFP_KERNEL);
>  		if (!path_buffer)
> @@ -283,8 +309,8 @@ static int altha_bprm_creds_from_file(struct linux_binprm *bprm, struct file * f
>  		list_for_each_entry(node, &nosuid_exceptions_list, list) {
>  			if (strcmp(path_p, node->spath) == 0) {
>  				pr_notice_ratelimited
> -				    ("AltHa/NoSUID: %s permitted to setuid from %d\n",
> -				     bprm->filename, cur_uid);
> +				    ("AltHa/NoSUID: %s permitted to %s from %d\n",
> +				     bprm->filename, setuidcap_srt, cur_uid);
>  				up_read(&nosuid_exceptions_sem);
>  				kfree(path_buffer);
>  				return 0;
> @@ -292,9 +318,13 @@ static int altha_bprm_creds_from_file(struct linux_binprm *bprm, struct file * f
>  		}
>  		up_read(&nosuid_exceptions_sem);
>  		pr_notice_ratelimited
> -		    ("AltHa/NoSUID: %s prevented to setuid from %d\n",
> -		     bprm->filename, cur_uid);
> -		bprm->cred->euid = bprm->cred->uid;
> +		    ("AltHa/NoSUID: %s prevented to %s from %d\n",
> +		     bprm->filename, setuidcap_srt, cur_uid);
> +		if (is_setuid)
> +			bprm->cred->euid = bprm->cred->uid;
> +		cap_clear(bprm->cred->cap_inheritable);
> +		cap_clear(bprm->cred->cap_permitted);
> +		cap_clear(bprm->cred->cap_effective);
>  		kfree(path_buffer);
>  	}
>  	return 0;
> -- 
> 2.33.3
> 
> _______________________________________________
> devel-kernel mailing list
> devel-kernel@lists.altlinux.org
> https://lists.altlinux.org/mailman/listinfo/devel-kernel

Re: [d-kernel] [PATCH] AltHa: nosuid handles capabilities as well

[Vladimir D. Seleznev]

On Thu, May 05, 2022 at 06:55:05PM +0300, Vitaly Chikunov wrote:
> Vladimir,

Hi!

> On Thu, Apr 28, 2022 at 02:59:55PM +0000, Vladimir D. Seleznev wrote:
> [skip]
> 
>   CC      security/altha/altha_lsm.o
> security/altha/altha_lsm.c: In function 'altha_bprm_creds_from_file':
> security/altha/altha_lsm.c:284:32: error: unused variable 'cur_euid' [-Werror=unused-variable]
>   284 |                 uid_t cur_uid, cur_euid;
>       |                                ^~~~~~~~
> cc1: all warnings being treated as errors
> make[2]: *** [scripts/Makefile.build:277: security/altha/altha_lsm.o] Error 1
> make[1]: *** [scripts/Makefile.build:540: security/altha] Error 2
> make: *** [Makefile:1868: security] Error 2
> make: *** Waiting for unfinished jobs....

Hmm, interesting. My build is OK. I'll see on this tomorrow.

-- 
   WBR,
   Vladimir D. Seleznev

Re: [d-kernel] [PATCH] AltHa: nosuid handles capabilities as well

[Vitaly Chikunov]

Vladimir,

On Fri, May 06, 2022 at 02:45:36AM +0300, Vladimir D. Seleznev wrote:
> On Thu, May 05, 2022 at 06:55:05PM +0300, Vitaly Chikunov wrote:
> 
> > On Thu, Apr 28, 2022 at 02:59:55PM +0000, Vladimir D. Seleznev wrote:
> > [skip]
> > 
> >   CC      security/altha/altha_lsm.o
> > security/altha/altha_lsm.c: In function 'altha_bprm_creds_from_file':
> > security/altha/altha_lsm.c:284:32: error: unused variable 'cur_euid' [-Werror=unused-variable]
> >   284 |                 uid_t cur_uid, cur_euid;
> >       |                                ^~~~~~~~
> > cc1: all warnings being treated as errors
> > make[2]: *** [scripts/Makefile.build:277: security/altha/altha_lsm.o] Error 1
> > make[1]: *** [scripts/Makefile.build:540: security/altha] Error 2
> > make: *** [Makefile:1868: security] Error 2
> > make: *** Waiting for unfinished jobs....
> 
> Hmm, interesting. My build is OK. I'll see on this tomorrow.

While you will look at it tomorrow, please also add minimal tests, for
positive and negative case, also preferable make them self-contained
enough to be runnable in vm-run. Example of the test in
  security/kiosk/kiosk-test.sh

Thanks,

> 
> -- 
>    WBR,
>    Vladimir D. Seleznev
> _______________________________________________
> devel-kernel mailing list
> devel-kernel@lists.altlinux.org
> https://lists.altlinux.org/mailman/listinfo/devel-kernel

Re: [d-kernel] [PATCH] AltHa: nosuid handles capabilities as well

[Vladimir D. Seleznev]

On Mon, May 09, 2022 at 02:47:07PM +0300, Vitaly Chikunov wrote:
> Vladimir,

Hi!

> On Fri, May 06, 2022 at 02:45:36AM +0300, Vladimir D. Seleznev wrote:
> > On Thu, May 05, 2022 at 06:55:05PM +0300, Vitaly Chikunov wrote:
> > 
> > > On Thu, Apr 28, 2022 at 02:59:55PM +0000, Vladimir D. Seleznev wrote:
> > > [skip]
> > > 
> > >   CC      security/altha/altha_lsm.o
> > > security/altha/altha_lsm.c: In function 'altha_bprm_creds_from_file':
> > > security/altha/altha_lsm.c:284:32: error: unused variable 'cur_euid' [-Werror=unused-variable]
> > >   284 |                 uid_t cur_uid, cur_euid;
> > >       |                                ^~~~~~~~
> > > cc1: all warnings being treated as errors
> > > make[2]: *** [scripts/Makefile.build:277: security/altha/altha_lsm.o] Error 1
> > > make[1]: *** [scripts/Makefile.build:540: security/altha] Error 2
> > > make: *** [Makefile:1868: security] Error 2
> > > make: *** Waiting for unfinished jobs....
> > 
> > Hmm, interesting. My build is OK. I'll see on this tomorrow.
> 
> While you will look at it tomorrow,

I did look at it that day. When I built and tested it, it was
std-def-5.15.34-alt2 and it was successfully built. The same patch is
failing on 5.15.37-alt1. When I fixed it on 5.15.37-alt1, I found that
there are some bugs in the logic that should be fixed, and I postponed
it because of several reasons. Obviously I can't send a buggy patch.
I'll fix it as soon as I have time, hope it will be very soon.

I also notice that the first version of the patch, that I sent in
private mail, has no these issue, but we rejected it for other reasons.

> please also add minimal tests, for positive and negative case, also
> preferable make them self-contained enough to be runnable in vm-run.
> Example of the test in security/kiosk/kiosk-test.sh

Tests are good, but I consider it as a separate task, I would like to
not mixing them.

> Thanks,

-- 
   WBR,
   Vladimir D. Seleznev