linux-kernel - [PATCH GHAK16 V5 09/10] capabilities: fix logic for effective root or real root

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <84e457662ed6918af35f5678c3cdd0250d693fa7.1507769413.git.rgb@redhat.com>
Date:   Wed, 11 Oct 2017 20:57:13 -0400
From:   Richard Guy Briggs <rgb@...hat.com>
To:     linux-security-module@...r.kernel.org, linux-audit@...hat.com,
        linux-kernel@...r.kernel.org
Cc:     Richard Guy Briggs <rgb@...hat.com>,
        Kees Cook <keescook@...omium.org>,
        Andy Lutomirski <luto@...nel.org>,
        "Serge E . Hallyn" <serge.hallyn@...ntu.com>,
        James Morris <james.l.morris@...cle.com>,
        Paul Moore <pmoore@...hat.com>,
        Steve Grubb <sgrubb@...hat.com>, Eric Paris <eparis@...hat.com>
Subject: [PATCH GHAK16 V5 09/10] capabilities: fix logic for effective root or real root

Now that the logic is inverted, it is much easier to see that both real
root and effective root conditions had to be met to avoid printing the
BPRM_FCAPS record with audit syscalls.  This meant that any setuid root
applications would print a full BPRM_FCAPS record when it wasn't
necessary, cluttering the event output, since the SYSCALL and PATH
records indicated the presence of the setuid bit and effective root user
id.

Require only one of effective root or real root to avoid printing the
unnecessary record.

Ref: commit 3fc689e96c0c ("Add audit_log_bprm_fcaps/AUDIT_BPRM_FCAPS")
See: https://github.com/linux-audit/audit-kernel/issues/16

Signed-off-by: Richard Guy Briggs <rgb@...hat.com>
Reviewed-by: Serge Hallyn <serge@...lyn.com>
Acked-by: James Morris <james.l.morris@...cle.com>
Acked-by: Kees Cook <keescook@...omium.org>
Acked-by: Paul Moore <paul@...l-moore.com>
---
 security/commoncap.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/security/commoncap.c b/security/commoncap.c
index 0bd94d3..ad7536d 100644
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -770,7 +770,7 @@ static inline bool __is_setgid(struct cred *new, const struct cred *old)
  *
  * We do not bother to audit if 3 things are true:
  *   1) cap_effective has all caps
- *   2) we are root
+ *   2) we became root *OR* are were already root
  *   3) root is supposed to have all caps (SECURE_NOROOT)
  * Since this is just a normal root execing a process.
  *
@@ -783,8 +783,7 @@ static inline bool nonroot_raised_pE(struct cred *cred, kuid_t root)
 
 	if (__cap_grew(effective, ambient, cred) &&
 	    !(__cap_full(effective, cred) &&
-	      __is_eff(root, cred) &&
-	      __is_real(root, cred) &&
+	      (__is_eff(root, cred) || __is_real(root, cred)) &&
 	      root_privileged()))
 		ret = true;
 	return ret;
-- 
1.8.3.1