* [PATCH 04/10] LSM: SafeSetID: refactor safesetid_security_capable()
@ 2019-04-10 16:55 Micah Morton
2019-04-10 17:13 ` Kees Cook
0 siblings, 1 reply; 3+ messages in thread
From: Micah Morton @ 2019-04-10 16:55 UTC (permalink / raw)
To: jmorris, keescook, casey, linux-security-module; +Cc: Jann Horn, Micah Morton
From: Jann Horn <jannh@google.com>
At the moment, safesetid_security_capable() has two nested conditional
blocks, and one big comment for all the logic. Chop it up and reduce the
amount of indentation.
Signed-off-by: Jann Horn <jannh@google.com>
Signed-off-by: Micah Morton <mortonm@chromium.org>
---
security/safesetid/lsm.c | 41 +++++++++++++++++++++++++---------------
1 file changed, 26 insertions(+), 15 deletions(-)
diff --git a/security/safesetid/lsm.c b/security/safesetid/lsm.c
index 15cd13b5a211..ab429e1816c5 100644
--- a/security/safesetid/lsm.c
+++ b/security/safesetid/lsm.c
@@ -55,21 +55,32 @@ static int safesetid_security_capable(const struct cred *cred,
int cap,
unsigned int opts)
{
- if (cap == CAP_SETUID &&
- setuid_policy_lookup(cred->uid, INVALID_UID) != SIDPOL_DEFAULT) {
- if (!(opts & CAP_OPT_INSETID)) {
- /*
- * Deny if we're not in a set*uid() syscall to avoid
- * giving powers gated by CAP_SETUID that are related
- * to functionality other than calling set*uid() (e.g.
- * allowing user to set up userns uid mappings).
- */
- pr_warn("Operation requires CAP_SETUID, which is not available to UID %u for operations besides approved set*uid transitions\n",
- __kuid_val(cred->uid));
- return -1;
- }
- }
- return 0;
+ /* We're only interested in CAP_SETUID. */
+ if (cap != CAP_SETUID)
+ return 0;
+
+ /*
+ * If CAP_SETUID is currently used for a set*uid() syscall, we want to
+ * let it go through here; the real security check happens later, in the
+ * task_fix_setuid hook.
+ */
+ if ((opts & CAP_OPT_INSETID) != 0)
+ return 0;
+
+ /*
+ * If no policy applies to this task, allow the use of CAP_SETUID for
+ * other purposes.
+ */
+ if (setuid_policy_lookup(cred->uid, INVALID_UID) == SIDPOL_DEFAULT)
+ return 0;
+
+ /*
+ * Reject use of CAP_SETUID for functionality other than calling
+ * set*uid() (e.g. setting up userns uid mappings).
+ */
+ pr_warn("Operation requires CAP_SETUID, which is not available to UID %u for operations besides approved set*uid transitions\n",
+ __kuid_val(cred->uid));
+ return -1;
}
/*
--
2.21.0.392.gf8f6787159e-goog
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH 04/10] LSM: SafeSetID: refactor safesetid_security_capable()
2019-04-10 16:55 [PATCH 04/10] LSM: SafeSetID: refactor safesetid_security_capable() Micah Morton
@ 2019-04-10 17:13 ` Kees Cook
2019-05-07 15:01 ` Micah Morton
0 siblings, 1 reply; 3+ messages in thread
From: Kees Cook @ 2019-04-10 17:13 UTC (permalink / raw)
To: Micah Morton
Cc: James Morris, Casey Schaufler, linux-security-module, Jann Horn
On Wed, Apr 10, 2019 at 9:55 AM Micah Morton <mortonm@chromium.org> wrote:
>
> From: Jann Horn <jannh@google.com>
>
> At the moment, safesetid_security_capable() has two nested conditional
> blocks, and one big comment for all the logic. Chop it up and reduce the
> amount of indentation.
>
> Signed-off-by: Jann Horn <jannh@google.com>
> Signed-off-by: Micah Morton <mortonm@chromium.org>
Reviewed-by: Kees Cook <keescook@chromium.org>
-Kees
> ---
> security/safesetid/lsm.c | 41 +++++++++++++++++++++++++---------------
> 1 file changed, 26 insertions(+), 15 deletions(-)
>
> diff --git a/security/safesetid/lsm.c b/security/safesetid/lsm.c
> index 15cd13b5a211..ab429e1816c5 100644
> --- a/security/safesetid/lsm.c
> +++ b/security/safesetid/lsm.c
> @@ -55,21 +55,32 @@ static int safesetid_security_capable(const struct cred *cred,
> int cap,
> unsigned int opts)
> {
> - if (cap == CAP_SETUID &&
> - setuid_policy_lookup(cred->uid, INVALID_UID) != SIDPOL_DEFAULT) {
> - if (!(opts & CAP_OPT_INSETID)) {
> - /*
> - * Deny if we're not in a set*uid() syscall to avoid
> - * giving powers gated by CAP_SETUID that are related
> - * to functionality other than calling set*uid() (e.g.
> - * allowing user to set up userns uid mappings).
> - */
> - pr_warn("Operation requires CAP_SETUID, which is not available to UID %u for operations besides approved set*uid transitions\n",
> - __kuid_val(cred->uid));
> - return -1;
> - }
> - }
> - return 0;
> + /* We're only interested in CAP_SETUID. */
> + if (cap != CAP_SETUID)
> + return 0;
> +
> + /*
> + * If CAP_SETUID is currently used for a set*uid() syscall, we want to
> + * let it go through here; the real security check happens later, in the
> + * task_fix_setuid hook.
> + */
> + if ((opts & CAP_OPT_INSETID) != 0)
> + return 0;
> +
> + /*
> + * If no policy applies to this task, allow the use of CAP_SETUID for
> + * other purposes.
> + */
> + if (setuid_policy_lookup(cred->uid, INVALID_UID) == SIDPOL_DEFAULT)
> + return 0;
> +
> + /*
> + * Reject use of CAP_SETUID for functionality other than calling
> + * set*uid() (e.g. setting up userns uid mappings).
> + */
> + pr_warn("Operation requires CAP_SETUID, which is not available to UID %u for operations besides approved set*uid transitions\n",
> + __kuid_val(cred->uid));
> + return -1;
> }
>
> /*
> --
> 2.21.0.392.gf8f6787159e-goog
>
--
Kees Cook
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH 04/10] LSM: SafeSetID: refactor safesetid_security_capable()
2019-04-10 17:13 ` Kees Cook
@ 2019-05-07 15:01 ` Micah Morton
0 siblings, 0 replies; 3+ messages in thread
From: Micah Morton @ 2019-05-07 15:01 UTC (permalink / raw)
To: Kees Cook; +Cc: James Morris, Casey Schaufler, linux-security-module, Jann Horn
Ready for merge.
On Wed, Apr 10, 2019 at 10:14 AM Kees Cook <keescook@chromium.org> wrote:
>
> On Wed, Apr 10, 2019 at 9:55 AM Micah Morton <mortonm@chromium.org> wrote:
> >
> > From: Jann Horn <jannh@google.com>
> >
> > At the moment, safesetid_security_capable() has two nested conditional
> > blocks, and one big comment for all the logic. Chop it up and reduce the
> > amount of indentation.
> >
> > Signed-off-by: Jann Horn <jannh@google.com>
> > Signed-off-by: Micah Morton <mortonm@chromium.org>
>
> Reviewed-by: Kees Cook <keescook@chromium.org>
>
> -Kees
>
> > ---
> > security/safesetid/lsm.c | 41 +++++++++++++++++++++++++---------------
> > 1 file changed, 26 insertions(+), 15 deletions(-)
> >
> > diff --git a/security/safesetid/lsm.c b/security/safesetid/lsm.c
> > index 15cd13b5a211..ab429e1816c5 100644
> > --- a/security/safesetid/lsm.c
> > +++ b/security/safesetid/lsm.c
> > @@ -55,21 +55,32 @@ static int safesetid_security_capable(const struct cred *cred,
> > int cap,
> > unsigned int opts)
> > {
> > - if (cap == CAP_SETUID &&
> > - setuid_policy_lookup(cred->uid, INVALID_UID) != SIDPOL_DEFAULT) {
> > - if (!(opts & CAP_OPT_INSETID)) {
> > - /*
> > - * Deny if we're not in a set*uid() syscall to avoid
> > - * giving powers gated by CAP_SETUID that are related
> > - * to functionality other than calling set*uid() (e.g.
> > - * allowing user to set up userns uid mappings).
> > - */
> > - pr_warn("Operation requires CAP_SETUID, which is not available to UID %u for operations besides approved set*uid transitions\n",
> > - __kuid_val(cred->uid));
> > - return -1;
> > - }
> > - }
> > - return 0;
> > + /* We're only interested in CAP_SETUID. */
> > + if (cap != CAP_SETUID)
> > + return 0;
> > +
> > + /*
> > + * If CAP_SETUID is currently used for a set*uid() syscall, we want to
> > + * let it go through here; the real security check happens later, in the
> > + * task_fix_setuid hook.
> > + */
> > + if ((opts & CAP_OPT_INSETID) != 0)
> > + return 0;
> > +
> > + /*
> > + * If no policy applies to this task, allow the use of CAP_SETUID for
> > + * other purposes.
> > + */
> > + if (setuid_policy_lookup(cred->uid, INVALID_UID) == SIDPOL_DEFAULT)
> > + return 0;
> > +
> > + /*
> > + * Reject use of CAP_SETUID for functionality other than calling
> > + * set*uid() (e.g. setting up userns uid mappings).
> > + */
> > + pr_warn("Operation requires CAP_SETUID, which is not available to UID %u for operations besides approved set*uid transitions\n",
> > + __kuid_val(cred->uid));
> > + return -1;
> > }
> >
> > /*
> > --
> > 2.21.0.392.gf8f6787159e-goog
> >
>
>
> --
> Kees Cook
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-05-07 15:01 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-04-10 16:55 [PATCH 04/10] LSM: SafeSetID: refactor safesetid_security_capable() Micah Morton
2019-04-10 17:13 ` Kees Cook
2019-05-07 15:01 ` Micah Morton
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox