Linux-Security-Module Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 1/2] LSM: Signal to SafeSetID when in set*gid syscall
@ 2020-07-20 18:11 Micah Morton
  2020-07-21  2:11 ` Serge E. Hallyn
  2020-07-27 18:44 ` James Morris
  0 siblings, 2 replies; 4+ messages in thread
From: Micah Morton @ 2020-07-20 18:11 UTC (permalink / raw)
  To: linux-security-module
  Cc: keescook, casey, paul, stephen.smalley.work, serge, jmorris,
	Thomas Cedeno, Micah Morton

From: Thomas Cedeno <thomascedeno@google.com>

For SafeSetID to properly gate set*gid() calls, it needs to know whether
ns_capable() is being called from within a sys_set*gid() function or is
being called from elsewhere in the kernel. This allows SafeSetID to deny
CAP_SETGID to restricted groups when they are attempting to use the
capability for code paths other than updating GIDs (e.g. setting up
userns GID mappings). This is the identical approach to what is
currently done for CAP_SETUID.

Signed-off-by: Thomas Cedeno <thomascedeno@google.com>
Signed-off-by: Micah Morton <mortonm@chromium.org>
---
 kernel/sys.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/kernel/sys.c b/kernel/sys.c
index 00a96746e28a..55e0c86772ab 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -373,7 +373,7 @@ long __sys_setregid(gid_t rgid, gid_t egid)
 	if (rgid != (gid_t) -1) {
 		if (gid_eq(old->gid, krgid) ||
 		    gid_eq(old->egid, krgid) ||
-		    ns_capable(old->user_ns, CAP_SETGID))
+		    ns_capable_setid(old->user_ns, CAP_SETGID))
 			new->gid = krgid;
 		else
 			goto error;
@@ -382,7 +382,7 @@ long __sys_setregid(gid_t rgid, gid_t egid)
 		if (gid_eq(old->gid, kegid) ||
 		    gid_eq(old->egid, kegid) ||
 		    gid_eq(old->sgid, kegid) ||
-		    ns_capable(old->user_ns, CAP_SETGID))
+		    ns_capable_setid(old->user_ns, CAP_SETGID))
 			new->egid = kegid;
 		else
 			goto error;
@@ -432,7 +432,7 @@ long __sys_setgid(gid_t gid)
 	old = current_cred();
 
 	retval = -EPERM;
-	if (ns_capable(old->user_ns, CAP_SETGID))
+	if (ns_capable_setid(old->user_ns, CAP_SETGID))
 		new->gid = new->egid = new->sgid = new->fsgid = kgid;
 	else if (gid_eq(kgid, old->gid) || gid_eq(kgid, old->sgid))
 		new->egid = new->fsgid = kgid;
@@ -744,7 +744,7 @@ long __sys_setresgid(gid_t rgid, gid_t egid, gid_t sgid)
 	old = current_cred();
 
 	retval = -EPERM;
-	if (!ns_capable(old->user_ns, CAP_SETGID)) {
+	if (!ns_capable_setid(old->user_ns, CAP_SETGID)) {
 		if (rgid != (gid_t) -1        && !gid_eq(krgid, old->gid) &&
 		    !gid_eq(krgid, old->egid) && !gid_eq(krgid, old->sgid))
 			goto error;
@@ -871,7 +871,7 @@ long __sys_setfsgid(gid_t gid)
 
 	if (gid_eq(kgid, old->gid)  || gid_eq(kgid, old->egid)  ||
 	    gid_eq(kgid, old->sgid) || gid_eq(kgid, old->fsgid) ||
-	    ns_capable(old->user_ns, CAP_SETGID)) {
+	    ns_capable_setid(old->user_ns, CAP_SETGID)) {
 		if (!gid_eq(kgid, old->fsgid)) {
 			new->fsgid = kgid;
 			if (security_task_fix_setgid(new,old,LSM_SETID_FS) == 0)
-- 
2.28.0.rc0.105.gf9edc3c819-goog

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 1/2] LSM: Signal to SafeSetID when in set*gid syscall
  2020-07-20 18:11 [PATCH 1/2] LSM: Signal to SafeSetID when in set*gid syscall Micah Morton
@ 2020-07-21  2:11 ` Serge E. Hallyn
  2020-07-27 18:44 ` James Morris
  1 sibling, 0 replies; 4+ messages in thread
From: Serge E. Hallyn @ 2020-07-21  2:11 UTC (permalink / raw)
  To: Micah Morton
  Cc: linux-security-module, keescook, casey, paul,
	stephen.smalley.work, serge, jmorris, Thomas Cedeno

On Mon, Jul 20, 2020 at 11:11:56AM -0700, Micah Morton wrote:
> From: Thomas Cedeno <thomascedeno@google.com>
> 
> For SafeSetID to properly gate set*gid() calls, it needs to know whether
> ns_capable() is being called from within a sys_set*gid() function or is
> being called from elsewhere in the kernel. This allows SafeSetID to deny
> CAP_SETGID to restricted groups when they are attempting to use the
> capability for code paths other than updating GIDs (e.g. setting up
> userns GID mappings). This is the identical approach to what is
> currently done for CAP_SETUID.
> 
> Signed-off-by: Thomas Cedeno <thomascedeno@google.com>
> Signed-off-by: Micah Morton <mortonm@chromium.org>

I see that only safesetid is using that now anyway.

Reviewed-by: Serge Hallyn <serge@hallyn.com>

> ---
>  kernel/sys.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/kernel/sys.c b/kernel/sys.c
> index 00a96746e28a..55e0c86772ab 100644
> --- a/kernel/sys.c
> +++ b/kernel/sys.c
> @@ -373,7 +373,7 @@ long __sys_setregid(gid_t rgid, gid_t egid)
>  	if (rgid != (gid_t) -1) {
>  		if (gid_eq(old->gid, krgid) ||
>  		    gid_eq(old->egid, krgid) ||
> -		    ns_capable(old->user_ns, CAP_SETGID))
> +		    ns_capable_setid(old->user_ns, CAP_SETGID))
>  			new->gid = krgid;
>  		else
>  			goto error;
> @@ -382,7 +382,7 @@ long __sys_setregid(gid_t rgid, gid_t egid)
>  		if (gid_eq(old->gid, kegid) ||
>  		    gid_eq(old->egid, kegid) ||
>  		    gid_eq(old->sgid, kegid) ||
> -		    ns_capable(old->user_ns, CAP_SETGID))
> +		    ns_capable_setid(old->user_ns, CAP_SETGID))
>  			new->egid = kegid;
>  		else
>  			goto error;
> @@ -432,7 +432,7 @@ long __sys_setgid(gid_t gid)
>  	old = current_cred();
>  
>  	retval = -EPERM;
> -	if (ns_capable(old->user_ns, CAP_SETGID))
> +	if (ns_capable_setid(old->user_ns, CAP_SETGID))
>  		new->gid = new->egid = new->sgid = new->fsgid = kgid;
>  	else if (gid_eq(kgid, old->gid) || gid_eq(kgid, old->sgid))
>  		new->egid = new->fsgid = kgid;
> @@ -744,7 +744,7 @@ long __sys_setresgid(gid_t rgid, gid_t egid, gid_t sgid)
>  	old = current_cred();
>  
>  	retval = -EPERM;
> -	if (!ns_capable(old->user_ns, CAP_SETGID)) {
> +	if (!ns_capable_setid(old->user_ns, CAP_SETGID)) {
>  		if (rgid != (gid_t) -1        && !gid_eq(krgid, old->gid) &&
>  		    !gid_eq(krgid, old->egid) && !gid_eq(krgid, old->sgid))
>  			goto error;
> @@ -871,7 +871,7 @@ long __sys_setfsgid(gid_t gid)
>  
>  	if (gid_eq(kgid, old->gid)  || gid_eq(kgid, old->egid)  ||
>  	    gid_eq(kgid, old->sgid) || gid_eq(kgid, old->fsgid) ||
> -	    ns_capable(old->user_ns, CAP_SETGID)) {
> +	    ns_capable_setid(old->user_ns, CAP_SETGID)) {
>  		if (!gid_eq(kgid, old->fsgid)) {
>  			new->fsgid = kgid;
>  			if (security_task_fix_setgid(new,old,LSM_SETID_FS) == 0)
> -- 
> 2.28.0.rc0.105.gf9edc3c819-goog

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 1/2] LSM: Signal to SafeSetID when in set*gid syscall
  2020-07-20 18:11 [PATCH 1/2] LSM: Signal to SafeSetID when in set*gid syscall Micah Morton
  2020-07-21  2:11 ` Serge E. Hallyn
@ 2020-07-27 18:44 ` James Morris
  2020-08-04 21:13   ` [PATCH v2 1/2] LSM: Signal to SafeSetID when setting group IDs Micah Morton
  1 sibling, 1 reply; 4+ messages in thread
From: James Morris @ 2020-07-27 18:44 UTC (permalink / raw)
  To: Micah Morton
  Cc: linux-security-module, keescook, casey, paul,
	stephen.smalley.work, serge, Thomas Cedeno

On Mon, 20 Jul 2020, Micah Morton wrote:

> From: Thomas Cedeno <thomascedeno@google.com>
> 
> For SafeSetID to properly gate set*gid() calls, it needs to know whether
> ns_capable() is being called from within a sys_set*gid() function or is
> being called from elsewhere in the kernel. This allows SafeSetID to deny
> CAP_SETGID to restricted groups when they are attempting to use the
> capability for code paths other than updating GIDs (e.g. setting up
> userns GID mappings). This is the identical approach to what is
> currently done for CAP_SETUID.
> 
> Signed-off-by: Thomas Cedeno <thomascedeno@google.com>
> Signed-off-by: Micah Morton <mortonm@chromium.org>


Acked-by: James Morris <jamorris@linux.microsoft.com>


-- 
James Morris
<jmorris@namei.org>


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v2 1/2] LSM: Signal to SafeSetID when setting group IDs
  2020-07-27 18:44 ` James Morris
@ 2020-08-04 21:13   ` Micah Morton
  0 siblings, 0 replies; 4+ messages in thread
From: Micah Morton @ 2020-08-04 21:13 UTC (permalink / raw)
  To: linux-security-module
  Cc: keescook, casey, paul, stephen.smalley.work, serge, jmorris,
	Thomas Cedeno, Micah Morton

From: Thomas Cedeno <thomascedeno@google.com>

For SafeSetID to properly gate set*gid() calls, it needs to know whether
ns_capable() is being called from within a sys_set*gid() function or is
being called from elsewhere in the kernel. This allows SafeSetID to deny
CAP_SETGID to restricted groups when they are attempting to use the
capability for code paths other than updating GIDs (e.g. setting up
userns GID mappings). This is the identical approach to what is
currently done for CAP_SETUID.

NOTE: We also add signaling to SafeSetID from the setgroups() syscall,
as we have future plans to restrict a process' ability to set
supplementary groups in addition to what is added in this series for
restricting setting of the primary group.

Signed-off-by: Thomas Cedeno <thomascedeno@google.com>
Signed-off-by: Micah Morton <mortonm@chromium.org>
---
Change since last patch: signal from setgroups() syscall as well as
set*gid() syscalls.
 kernel/capability.c |  2 +-
 kernel/groups.c     |  2 +-
 kernel/sys.c        | 10 +++++-----
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/kernel/capability.c b/kernel/capability.c
index 1444f3954d75..6cfbfba65b9b 100644
--- a/kernel/capability.c
+++ b/kernel/capability.c
@@ -418,7 +418,7 @@ EXPORT_SYMBOL(ns_capable_noaudit);
 /**
  * ns_capable_setid - Determine if the current task has a superior capability
  * in effect, while signalling that this check is being done from within a
- * setid syscall.
+ * setid or setgroups syscall.
  * @ns:  The usernamespace we want the capability in
  * @cap: The capability to be tested for
  *
diff --git a/kernel/groups.c b/kernel/groups.c
index 6ee6691f6839..fe7e6385530e 100644
--- a/kernel/groups.c
+++ b/kernel/groups.c
@@ -178,7 +178,7 @@ bool may_setgroups(void)
 {
 	struct user_namespace *user_ns = current_user_ns();
 
-	return ns_capable(user_ns, CAP_SETGID) &&
+	return ns_capable_setid(user_ns, CAP_SETGID) &&
 		userns_may_setgroups(user_ns);
 }
 
diff --git a/kernel/sys.c b/kernel/sys.c
index 00a96746e28a..55e0c86772ab 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -373,7 +373,7 @@ long __sys_setregid(gid_t rgid, gid_t egid)
 	if (rgid != (gid_t) -1) {
 		if (gid_eq(old->gid, krgid) ||
 		    gid_eq(old->egid, krgid) ||
-		    ns_capable(old->user_ns, CAP_SETGID))
+		    ns_capable_setid(old->user_ns, CAP_SETGID))
 			new->gid = krgid;
 		else
 			goto error;
@@ -382,7 +382,7 @@ long __sys_setregid(gid_t rgid, gid_t egid)
 		if (gid_eq(old->gid, kegid) ||
 		    gid_eq(old->egid, kegid) ||
 		    gid_eq(old->sgid, kegid) ||
-		    ns_capable(old->user_ns, CAP_SETGID))
+		    ns_capable_setid(old->user_ns, CAP_SETGID))
 			new->egid = kegid;
 		else
 			goto error;
@@ -432,7 +432,7 @@ long __sys_setgid(gid_t gid)
 	old = current_cred();
 
 	retval = -EPERM;
-	if (ns_capable(old->user_ns, CAP_SETGID))
+	if (ns_capable_setid(old->user_ns, CAP_SETGID))
 		new->gid = new->egid = new->sgid = new->fsgid = kgid;
 	else if (gid_eq(kgid, old->gid) || gid_eq(kgid, old->sgid))
 		new->egid = new->fsgid = kgid;
@@ -744,7 +744,7 @@ long __sys_setresgid(gid_t rgid, gid_t egid, gid_t sgid)
 	old = current_cred();
 
 	retval = -EPERM;
-	if (!ns_capable(old->user_ns, CAP_SETGID)) {
+	if (!ns_capable_setid(old->user_ns, CAP_SETGID)) {
 		if (rgid != (gid_t) -1        && !gid_eq(krgid, old->gid) &&
 		    !gid_eq(krgid, old->egid) && !gid_eq(krgid, old->sgid))
 			goto error;
@@ -871,7 +871,7 @@ long __sys_setfsgid(gid_t gid)
 
 	if (gid_eq(kgid, old->gid)  || gid_eq(kgid, old->egid)  ||
 	    gid_eq(kgid, old->sgid) || gid_eq(kgid, old->fsgid) ||
-	    ns_capable(old->user_ns, CAP_SETGID)) {
+	    ns_capable_setid(old->user_ns, CAP_SETGID)) {
 		if (!gid_eq(kgid, old->fsgid)) {
 			new->fsgid = kgid;
 			if (security_task_fix_setgid(new,old,LSM_SETID_FS) == 0)
-- 
2.28.0.163.g6104cc2f0b6-goog


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, back to index

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-20 18:11 [PATCH 1/2] LSM: Signal to SafeSetID when in set*gid syscall Micah Morton
2020-07-21  2:11 ` Serge E. Hallyn
2020-07-27 18:44 ` James Morris
2020-08-04 21:13   ` [PATCH v2 1/2] LSM: Signal to SafeSetID when setting group IDs Micah Morton

Linux-Security-Module Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-security-module/0 linux-security-module/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-security-module linux-security-module/ https://lore.kernel.org/linux-security-module \
		linux-security-module@vger.kernel.org
	public-inbox-index linux-security-module

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-security-module


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git