On 2017-09-08 13:02, Paul Moore wrote: > On Tue, Sep 5, 2017 at 2:46 AM, Richard Guy Briggs <[email protected]> wrote: > > The audit subsystem is adding a BPRM_FCAPS record when auditing setuid > > application execution (SYSCALL execve). This is not expected as it was > > supposed to be limited to when the file system actually had capabilities > > in an extended attribute. It lists all capabilities making the event > > really ugly to parse what is happening. The PATH record correctly > > records the setuid bit and owner. Suppress the BPRM_FCAPS record on > > set*id. > > > > See: https://github.com/linux-audit/audit-kernel/issues/16 > > > > The first to eighth just massage the logic to make it easier to > > understand. Some of them could be squashed together. > > > > The patch that resolves this issue is the ninth. > > > > It would be possible to address the original issue with a change of > > "!uid_eq(new->euid, root_uid) || !uid_eq(new->uid, root_uid)" > > to > > "!(uid_eq(new->euid, root_uid) || uid_eq(new->uid, root_uid))" > > but it took me long enough to understand this logic that I don't think > > I'd be doing any favours by leaving it this difficult to understand. > > > > The final patch attempts to address all the conditions that need logging > > based on mailing list conversations, recoginizing there is probably some > > duplication in the logic. > > > > Passes: (ltp 20170516) > > ./runltp -f syscalls -s cap > > ./runltp -f securebits > > ./runltp -f cap_bounds > > ./runltp -f filecaps > > make TARGETS=capabilities kselftest (when run locally, fails over > > nfs) > > > > v4 > > rebase on kees' 4.13 commoncap changes > > minor local func renames > > > > v3 > > refactor into several sub-functions > > convert most macros to inline funcs > > > > v2 > > use macros to clarify intent of calculations > > fix original logic error > > address additional audit logging conditions > > > > Richard Guy Briggs (10): > > capabilities: factor out cap_bprm_set_creds privileged root > > capabilities: intuitive names for cap gain status > > capabilities: rename has_cap to has_fcap > > capabilities: use root_priveleged inline to clarify logic > > capabilities: use intuitive names for id changes > > capabilities: move audit log decision to function > > capabilities: remove a layer of conditional logic > > capabilities: invert logic for clarity > > capabilities: fix logic for effective root or real root > > capabilities: audit log other surprising conditions > > > > security/commoncap.c | 179 > > ++++++++++++++++++++++++++++++++------------------ > > 1 files changed, 114 insertions(+), 65 deletions(-) > > I took a quick look at this latest revision and aside from some > disagreements on style/formatting it looks okayish to me. However, I > am going to walk back my previous I-can-take-this-via-the-audit-tree > comments, I think this probably should go in via the capabilities > (Serge) and/or linux-security (James) tree.
"okayish"? That sounds positive. :-) Can you offer a clear ack or reviewed-by? > paul moore - RGB -- Richard Guy Briggs <[email protected]> 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 -- Linux-audit mailing list [email protected] https://www.redhat.com/mailman/listinfo/linux-audit
