Re: [PATCH V3 09/10] capabilities: fix logic for effective root or real root
On 2017-08-24 11:29, Serge E. Hallyn wrote:
Quoting Richard Guy Briggs (rgb(a)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 <rgb(a)redhat.com>
Reviewed-by: Serge Hallyn <serge(a)hallyn.com>
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 <rgb(a)redhat.com>
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