From mboxrd@z Thu Jan 1 00:00:00 1970 From: rgb@redhat.com (Richard Guy Briggs) Date: Thu, 24 Aug 2017 12:44:35 -0400 Subject: [PATCH V3 09/10] capabilities: fix logic for effective root or real root In-Reply-To: <20170824162946.GI10515@mail.hallyn.com> References: <0d9646956d9b2d99e8699c009de21f14fa592e7a.1503459890.git.rgb@redhat.com> <20170824162946.GI10515@mail.hallyn.com> Message-ID: <20170824164435.GJ24692@madcap2.tricolour.ca> To: linux-security-module@vger.kernel.org List-Id: linux-security-module.vger.kernel.org On 2017-08-24 11:29, Serge E. Hallyn wrote: > Quoting Richard Guy Briggs (rgb at redhat.com): > > 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: 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 > > Reviewed-by: Serge Hallyn > > I wonder whether, > > > --- > > security/commoncap.c | 6 +++--- > > 1 files changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/security/commoncap.c b/security/commoncap.c > > index eb2da69..49cce06 100644 > > --- a/security/commoncap.c > > +++ b/security/commoncap.c > > @@ -540,7 +540,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 root > > For clarity, what do you think about adding "(because fcaps were not used)"? Possibly. Is it possible to become root without fcaps other than logging in on a console as root from the get-go? But I see your point. Even if su or sudo were used to gain root, it would have been on a previous operation and not the immediate one being audited. The intention behind the change in the comment wording was to emphasize that the original comment hand-waved a bit about effective root vs real root without being explicit that it could be one or the other rather than requiring both, which affected the logic used to express it. > > * 3) root is supposed to have all caps (SECURE_NOROOT) > > * Since this is just a normal root execing a process. > > * > > @@ -553,8 +553,8 @@ 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.7.1 - RGB -- Richard Guy Briggs Sr. S/W Engineer, Kernel Security, Base Operating Systems Remote, Ottawa, Red Hat Canada IRC: rgb, SunRaycer Voice: +1.647.777.2635, Internal: (81) 32635 -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo at vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html From mboxrd@z Thu Jan 1 00:00:00 1970 From: Richard Guy Briggs Subject: Re: [PATCH V3 09/10] capabilities: fix logic for effective root or real root Date: Thu, 24 Aug 2017 12:44:35 -0400 Message-ID: <20170824164435.GJ24692@madcap2.tricolour.ca> References: <0d9646956d9b2d99e8699c009de21f14fa592e7a.1503459890.git.rgb@redhat.com> <20170824162946.GI10515@mail.hallyn.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Content-Disposition: inline In-Reply-To: <20170824162946.GI10515@mail.hallyn.com> Sender: owner-linux-security-module@vger.kernel.org To: "Serge E. Hallyn" Cc: linux-security-module@vger.kernel.org, linux-audit@redhat.com, Andy Lutomirski , "Serge E. Hallyn" , Kees Cook , James Morris , Eric Paris , Paul Moore , Steve Grubb List-Id: linux-audit@redhat.com On 2017-08-24 11:29, Serge E. Hallyn wrote: > Quoting Richard Guy Briggs (rgb@redhat.com): > > 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: 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 > > Reviewed-by: Serge Hallyn > > I wonder whether, > > > --- > > security/commoncap.c | 6 +++--- > > 1 files changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/security/commoncap.c b/security/commoncap.c > > index eb2da69..49cce06 100644 > > --- a/security/commoncap.c > > +++ b/security/commoncap.c > > @@ -540,7 +540,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 root > > For clarity, what do you think about adding "(because fcaps were not used)"? Possibly. Is it possible to become root without fcaps other than logging in on a console as root from the get-go? But I see your point. Even if su or sudo were used to gain root, it would have been on a previous operation and not the immediate one being audited. The intention behind the change in the comment wording was to emphasize that the original comment hand-waved a bit about effective root vs real root without being explicit that it could be one or the other rather than requiring both, which affected the logic used to express it. > > * 3) root is supposed to have all caps (SECURE_NOROOT) > > * Since this is just a normal root execing a process. > > * > > @@ -553,8 +553,8 @@ 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.7.1 - RGB -- Richard Guy Briggs Sr. S/W Engineer, Kernel Security, Base Operating Systems Remote, Ottawa, Red Hat Canada IRC: rgb, SunRaycer Voice: +1.647.777.2635, Internal: (81) 32635