[GIT PULL] Audit patches for v4.18
Hi Linus, Another reasonable chunk of audit changes for v4.18, thirteen patches in total. The thirteen patches can mostly be broken down into one of four categories: general bug fixes, accessor functions for audit state stored in the task_struct, negative filter matches on executable names, and extending the (relatively) new seccomp logging knobs to the audit subsystem. The main driver for the accessor functions from Richard are the changes we're working on to associate audit events with containers, but I think they have some standalone value too so I figured it would be good to get them in now. The seccomp/audit patches from Tyler apply the seccomp logging improvements from a few releases ago to audit's seccomp logging; starting with this patchset the changes in /proc/sys/kernel/seccomp/actions_logged should apply to both the standard kernel logging and audit. As usual, everything passes the audit-testsuite and it happens to merge cleanly with your tree. Please pull, thanks. -Paul -- The following changes since commit 60cc43fc888428bb2f18f08997432d426a243338: Linux 4.17-rc1 (2018-04-15 18:24:20 -0700) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/audit.git tags/audit-pr-20180605 for you to fetch changes up to 5b71388663c0920848c0ee7de946970a2692b76d: audit: Fix wrong task in comparison of session ID (2018-05-21 14:27:43 -0400) audit/stable-4.18 PR 20180605 Ondrej Mosnáček (2): audit: allow not equal op for audit by executable audit: Fix wrong task in comparison of session ID Richard Guy Briggs (7): audit: add syscall information to FEATURE_CHANGE records audit: convert sessionid unset to a macro audit: use inline function to get audit context audit: use inline function to set audit context audit: use new audit_context access funciton for seccomp_actions_logged audit: normalize loginuid read access audit: use existing session info function Tyler Hicks (4): seccomp: Separate read and write code for actions_logged sysctl seccomp: Configurable separator for the actions_logged string seccomp: Audit attempts to modify the actions_logged sysctl seccomp: Don't special case audited processes when logging Documentation/userspace-api/seccomp_filter.rst | 7 -- include/linux/audit.h | 39 --- include/net/xfrm.h | 4 +- include/uapi/linux/audit.h | 1 + init/init_task.c | 3 +- kernel/audit.c | 6 +- kernel/audit_watch.c | 2 +- kernel/auditfilter.c | 6 +- kernel/auditsc.c | 135 - kernel/fork.c | 2 +- kernel/seccomp.c | 126 --- net/bridge/netfilter/ebtables.c| 2 +- net/core/dev.c | 18 ++-- net/netfilter/x_tables.c | 2 +- net/netlabel/netlabel_user.c | 2 +- security/integrity/ima/ima_api.c | 2 +- security/integrity/integrity_audit.c | 2 +- security/lsm_audit.c | 2 +- security/selinux/hooks.c | 7 +- security/selinux/selinuxfs.c | 6 +- security/selinux/ss/services.c | 12 +-- 21 files changed, 242 insertions(+), 144 deletions(-) -- paul moore www.paul-moore.com -- Linux-audit mailing list Linux-audit@redhat.com https://www.redhat.com/mailman/listinfo/linux-audit
[RFC PATCH ghak86 V1] audit: eliminate audit_enabled magic number comparison
Remove comparison of audit_enabled to magic numbers outside of audit. Related: https://github.com/linux-audit/audit-kernel/issues/86 Signed-off-by: Richard Guy Briggs --- drivers/tty/tty_audit.c | 2 +- include/linux/audit.h| 5 - include/net/xfrm.h | 2 +- kernel/audit.c | 3 --- net/netfilter/xt_AUDIT.c | 2 +- net/netlabel/netlabel_user.c | 2 +- 6 files changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/tty/tty_audit.c b/drivers/tty/tty_audit.c index e30aa6b..50f567b 100644 --- a/drivers/tty/tty_audit.c +++ b/drivers/tty/tty_audit.c @@ -92,7 +92,7 @@ static void tty_audit_buf_push(struct tty_audit_buf *buf) { if (buf->valid == 0) return; - if (audit_enabled == 0) { + if (audit_enabled == AUDIT_OFF) { buf->valid = 0; return; } diff --git a/include/linux/audit.h b/include/linux/audit.h index 69c7847..9334fbe 100644 --- a/include/linux/audit.h +++ b/include/linux/audit.h @@ -117,6 +117,9 @@ struct audit_field { extern void audit_log_session_info(struct audit_buffer *ab); +#define AUDIT_OFF 0 +#define AUDIT_ON 1 +#define AUDIT_LOCKED 2 #ifdef CONFIG_AUDIT /* These are defined in audit.c */ /* Public API */ @@ -202,7 +205,7 @@ static inline int audit_log_task_context(struct audit_buffer *ab) static inline void audit_log_task_info(struct audit_buffer *ab, struct task_struct *tsk) { } -#define audit_enabled 0 +#define audit_enabled AUDIT_OFF #endif /* CONFIG_AUDIT */ #ifdef CONFIG_AUDIT_COMPAT_GENERIC diff --git a/include/net/xfrm.h b/include/net/xfrm.h index 7f2e31a..ce995a1 100644 --- a/include/net/xfrm.h +++ b/include/net/xfrm.h @@ -734,7 +734,7 @@ static inline struct audit_buffer *xfrm_audit_start(const char *op) { struct audit_buffer *audit_buf = NULL; - if (audit_enabled == 0) + if (audit_enabled == AUDIT_OFF) return NULL; audit_buf = audit_log_start(audit_context(), GFP_ATOMIC, AUDIT_MAC_IPSEC_EVENT); diff --git a/kernel/audit.c b/kernel/audit.c index e7478cb..8442c65 100644 --- a/kernel/audit.c +++ b/kernel/audit.c @@ -83,9 +83,6 @@ #define AUDIT_INITIALIZED 1 static int audit_initialized; -#define AUDIT_OFF 0 -#define AUDIT_ON 1 -#define AUDIT_LOCKED 2 u32audit_enabled = AUDIT_OFF; bool audit_ever_enabled = !!AUDIT_OFF; diff --git a/net/netfilter/xt_AUDIT.c b/net/netfilter/xt_AUDIT.c index f368ee6..af883f1 100644 --- a/net/netfilter/xt_AUDIT.c +++ b/net/netfilter/xt_AUDIT.c @@ -72,7 +72,7 @@ static bool audit_ip6(struct audit_buffer *ab, struct sk_buff *skb) struct audit_buffer *ab; int fam = -1; - if (audit_enabled == 0) + if (audit_enabled == AUDIT_OFF) goto errout; ab = audit_log_start(NULL, GFP_ATOMIC, AUDIT_NETFILTER_PKT); if (ab == NULL) diff --git a/net/netlabel/netlabel_user.c b/net/netlabel/netlabel_user.c index 2f328af..4676f5b 100644 --- a/net/netlabel/netlabel_user.c +++ b/net/netlabel/netlabel_user.c @@ -101,7 +101,7 @@ struct audit_buffer *netlbl_audit_start_common(int type, char *secctx; u32 secctx_len; - if (audit_enabled == 0) + if (audit_enabled == AUDIT_OFF) return NULL; audit_buf = audit_log_start(audit_context(), GFP_ATOMIC, type); -- 1.8.3.1 -- Linux-audit mailing list Linux-audit@redhat.com https://www.redhat.com/mailman/listinfo/linux-audit
Re: [PATCH v3 4/4] ima: Differentiate auditing policy rules from "audit" actions
On Tue, Jun 5, 2018 at 10:15 AM, Mimi Zohar wrote: > Hi Paul, > > On Mon, 2018-06-04 at 20:21 -0400, Paul Moore wrote: >> On Mon, Jun 4, 2018 at 4:54 PM, Stefan Berger >> wrote: >> > The AUDIT_INTEGRITY_RULE is used for auditing IMA policy rules and >> > the IMA "audit" policy action. This patch defines >> > AUDIT_INTEGRITY_POLICY_RULE to reflect the IMA policy rules. >> > >> > Since we defined a new message type we can now also pass the >> > audit_context and get an associated SYSCALL record. This now produces >> > the following records when parsing IMA policy's rules: >> >> Aaand now I see you included the current->audit_context pointer I >> mentioned in my comments for 3/4 ;) >> >> So basically this should be fine, although I should point out that you >> do not need to define a new message type to associate records >> together. The fact that we don't associate all connected records is >> basically a bug. >> >> Anyway, patches 3/4 and 4/4 look good to me. Considering this is >> likely going in during the *next* merge window, I would ask that you >> convert from "current->audit_context" to "audit_context()" as soon as >> this merge window closes. >> >> Thanks! > > Thanks, Paul. I'd like to start queueing patches for the next open > window now, instead of scrambling later. Can I add your Ack now, and > remember to make this change when rebasing? Sure, go ahead and add my ACK to both 3/4 and 4/4 as long as you double pinky swear you'll do the audit_context() fix-up during the merge :) Acked-by: Paul Moore -- paul moore www.paul-moore.com -- Linux-audit mailing list Linux-audit@redhat.com https://www.redhat.com/mailman/listinfo/linux-audit
[PATCH ghak89 V3] audit: rename FILTER_TYPE to FILTER_EXCLUDE
The AUDIT_FILTER_TYPE name is vague and misleading due to not describing where or when the filter is applied and obsolete due to its available filter fields having been expanded. Userspace has already renamed it from AUDIT_FILTER_TYPE to AUDIT_FILTER_EXCLUDE without checking if it already exists. The userspace maintainer assures that as long as it is set to the same value it will not be a problem since the userspace code does not treat compiler warnings as errors. If this policy changes then checks if it already exists can be added at the same time. See: https://github.com/linux-audit/audit-kernel/issues/89 Signed-off-by: Richard Guy Briggs --- v3: - Move macros together, update comment v2: - Change from AUDIT_FILTER_EXCL to AUDIT_FILTER_EXCLUDE --- include/uapi/linux/audit.h | 3 ++- kernel/audit.c | 2 +- kernel/auditfilter.c | 10 +- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h index 04f9bd2..6cae130 100644 --- a/include/uapi/linux/audit.h +++ b/include/uapi/linux/audit.h @@ -156,7 +156,8 @@ #define AUDIT_FILTER_ENTRY 0x02/* Apply rule at syscall entry */ #define AUDIT_FILTER_WATCH 0x03/* Apply rule to file system watches */ #define AUDIT_FILTER_EXIT 0x04/* Apply rule at syscall exit */ -#define AUDIT_FILTER_TYPE 0x05/* Apply rule at audit_log_start */ +#define AUDIT_FILTER_EXCLUDE 0x05/* Apply rule before record creation */ +#define AUDIT_FILTER_TYPE AUDIT_FILTER_EXCLUDE /* obsolete misleading naming */ #define AUDIT_FILTER_FS0x06/* Apply rule at __audit_inode_child */ #define AUDIT_NR_FILTERS 7 diff --git a/kernel/audit.c b/kernel/audit.c index 3a18e59..513a10e 100644 --- a/kernel/audit.c +++ b/kernel/audit.c @@ -1754,7 +1754,7 @@ struct audit_buffer *audit_log_start(struct audit_context *ctx, gfp_t gfp_mask, if (audit_initialized != AUDIT_INITIALIZED) return NULL; - if (unlikely(!audit_filter(type, AUDIT_FILTER_TYPE))) + if (unlikely(!audit_filter(type, AUDIT_FILTER_EXCLUDE))) return NULL; /* NOTE: don't ever fail/sleep on these two conditions: diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c index eaa3201..261843d 100644 --- a/kernel/auditfilter.c +++ b/kernel/auditfilter.c @@ -264,7 +264,7 @@ static inline struct audit_entry *audit_to_entry_common(struct audit_rule_data * case AUDIT_FILTER_TASK: #endif case AUDIT_FILTER_USER: - case AUDIT_FILTER_TYPE: + case AUDIT_FILTER_EXCLUDE: case AUDIT_FILTER_FS: ; } @@ -337,7 +337,7 @@ static int audit_field_valid(struct audit_entry *entry, struct audit_field *f) { switch(f->type) { case AUDIT_MSGTYPE: - if (entry->rule.listnr != AUDIT_FILTER_TYPE && + if (entry->rule.listnr != AUDIT_FILTER_EXCLUDE && entry->rule.listnr != AUDIT_FILTER_USER) return -EINVAL; break; @@ -931,7 +931,7 @@ static inline int audit_add_rule(struct audit_entry *entry) /* If any of these, don't count towards total */ switch(entry->rule.listnr) { case AUDIT_FILTER_USER: - case AUDIT_FILTER_TYPE: + case AUDIT_FILTER_EXCLUDE: case AUDIT_FILTER_FS: dont_count = 1; } @@ -1013,7 +1013,7 @@ int audit_del_rule(struct audit_entry *entry) /* If any of these, don't count towards total */ switch(entry->rule.listnr) { case AUDIT_FILTER_USER: - case AUDIT_FILTER_TYPE: + case AUDIT_FILTER_EXCLUDE: case AUDIT_FILTER_FS: dont_count = 1; } @@ -1369,7 +1369,7 @@ int audit_filter(int msgtype, unsigned int listtype) break; } if (result > 0) { - if (e->rule.action == AUDIT_NEVER || listtype == AUDIT_FILTER_TYPE) + if (e->rule.action == AUDIT_NEVER || listtype == AUDIT_FILTER_EXCLUDE) ret = 0; break; } -- 1.8.3.1 -- Linux-audit mailing list Linux-audit@redhat.com https://www.redhat.com/mailman/listinfo/linux-audit
Re: [PATCH v3 4/4] ima: Differentiate auditing policy rules from "audit" actions
Hi Paul, On Mon, 2018-06-04 at 20:21 -0400, Paul Moore wrote: > On Mon, Jun 4, 2018 at 4:54 PM, Stefan Berger > wrote: > > The AUDIT_INTEGRITY_RULE is used for auditing IMA policy rules and > > the IMA "audit" policy action. This patch defines > > AUDIT_INTEGRITY_POLICY_RULE to reflect the IMA policy rules. > > > > Since we defined a new message type we can now also pass the > > audit_context and get an associated SYSCALL record. This now produces > > the following records when parsing IMA policy's rules: > > Aaand now I see you included the current->audit_context pointer I > mentioned in my comments for 3/4 ;) > > So basically this should be fine, although I should point out that you > do not need to define a new message type to associate records > together. The fact that we don't associate all connected records is > basically a bug. > > Anyway, patches 3/4 and 4/4 look good to me. Considering this is > likely going in during the *next* merge window, I would ask that you > convert from "current->audit_context" to "audit_context()" as soon as > this merge window closes. > > Thanks! Thanks, Paul. I'd like to start queueing patches for the next open window now, instead of scrambling later. Can I add your Ack now, and remember to make this change when rebasing? Mimi > > > type=UNKNOWN[1807] msg=audit(1527888965.738:320): action=audit \ > > func=MMAP_CHECK mask=MAY_EXEC res=1 > > type=UNKNOWN[1807] msg=audit(1527888965.738:320): action=audit \ > > func=FILE_CHECK mask=MAY_READ res=1 > > type=SYSCALL msg=audit(1527888965.738:320): arch=c03e syscall=1 \ > > success=yes exit=17 a0=1 a1=55bcfcca9030 a2=11 a3=7fcc1b55fb38 \ > > items=0 ppid=1567 pid=1601 auid=0 uid=0 gid=0 euid=0 suid=0 \ > > fsuid=0 egid=0 sgid=0 fsgid=0 tty=tty2 ses=2 comm="echo" \ > > exe="/usr/bin/echo" \ > > subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 key=(null) > > > > Signed-off-by: Stefan Berger > > --- > > include/uapi/linux/audit.h | 1 + > > security/integrity/ima/ima_policy.c | 4 ++-- > > 2 files changed, 3 insertions(+), 2 deletions(-) > > > > diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h > > index 65d9293f1fb8..cb358551376b 100644 > > --- a/include/uapi/linux/audit.h > > +++ b/include/uapi/linux/audit.h > > @@ -148,6 +148,7 @@ > > #define AUDIT_INTEGRITY_PCR1804 /* PCR invalidation msgs */ > > #define AUDIT_INTEGRITY_RULE 1805 /* policy rule */ > > #define AUDIT_INTEGRITY_EVM_XATTR 1806 /* New EVM-covered xattr */ > > +#define AUDIT_INTEGRITY_POLICY_RULE 1807 /* IMA policy rules */ > > > > #define AUDIT_KERNEL 2000/* Asynchronous audit record. NOT A > > REQUEST. */ > > > > diff --git a/security/integrity/ima/ima_policy.c > > b/security/integrity/ima/ima_policy.c > > index bc99713dfe57..f7230db217a7 100644 > > --- a/security/integrity/ima/ima_policy.c > > +++ b/security/integrity/ima/ima_policy.c > > @@ -652,8 +652,8 @@ static int ima_parse_rule(char *rule, struct > > ima_rule_entry *entry) > > bool uid_token; > > int result = 0; > > > > - ab = integrity_audit_log_start(NULL, GFP_KERNEL, > > - AUDIT_INTEGRITY_RULE); > > + ab = integrity_audit_log_start(current->audit_context, GFP_KERNEL, > > + AUDIT_INTEGRITY_POLICY_RULE); > > > > entry->uid = INVALID_UID; > > entry->fowner = INVALID_UID; > > -- > > 2.13.6 > > > -- Linux-audit mailing list Linux-audit@redhat.com https://www.redhat.com/mailman/listinfo/linux-audit
Re: [RFC PATCH 2/2] [WIP] audit: allow other filter list types for AUDIT_DIR
2018-06-05 0:19 GMT+02:00 Paul Moore : > On Fri, Jun 1, 2018 at 4:05 PM, Richard Guy Briggs wrote: >> On 2018-06-01 10:12, Ondrej Mosnacek wrote: > > ... > >>> audit_receive_msg -- this function doesn't work with context at all, >>> so I wasn't sure if audit_filter should consider it being NULL or if >>> it should get it from the current task. My hunch is the second option >>> is the right one, but the first one is safer (AUDIT_DIR will simply >>> never be checked here). I don't have such insight into the logic of >>> audit_context's lifetime, so I need someone to tell me what makes more >>> sense here. > > Given the nature of audit_receive_msg(), would it ever match on an > AUDIT_DIR field? I don't think it would since there aren't really any > vfs accesses that occur in the source of sending a netlink message > down to the kernel ... am I missing something? Probably not, but we still need to decide whether to pass the current task's context or not. Both options (NULL and audit_context()) seem to be benign for now, but I need you to pick one of those that you prefer so I can send a final patch. > >> That is starting to work with context. The recent FEATURE_CHANGE patch >> to connect records of the same event uses current->audit_context (now >> audit_context()) from audit_log_feature_change() called from >> audit_set_feature() called from audit_receive_msg(). >> >> There is also a work in progress to convert all the CONFIG_CHANGE >> records over. I'm just trying to track down all the instances. > > This will be a nice improvement. > > -- > paul moore > www.paul-moore.com -- Ondrej Mosnacek Associate Software Engineer, Security Technologies Red Hat, Inc. -- Linux-audit mailing list Linux-audit@redhat.com https://www.redhat.com/mailman/listinfo/linux-audit
Re: [RFC PATCH 1/2] audit: allow other filter list types for AUDIT_EXE
2018-06-04 22:41 GMT+02:00 Paul Moore : > On Wed, May 30, 2018 at 4:45 AM, Ondrej Mosnacek wrote: >> This patch removes the restriction of the AUDIT_EXE field to only >> SYSCALL filter and teaches audit_filter to recognize this field. >> >> This makes it possible to write rule lists such as: >> >> auditctl -a exit,always [some general rule] >> # Filter out events with executable name /bin/exe1 or /bin/exe2: >> auditctl -a exclude,always -F exe=/bin/exe1 >> auditctl -a exclude,always -F exe=/bin/exe2 >> >> See: https://github.com/linux-audit/audit-kernel/issues/54 >> >> Signed-off-by: Ondrej Mosnacek >> --- >> kernel/auditfilter.c | 7 +-- >> 1 file changed, 5 insertions(+), 2 deletions(-) > > Thanks for your patience Ondrej. > > Having reflected a bit on things from the recent IMA audit discussion, > my current thinking is to go ahead and merge this patch into > audit/next once the merge window closes. OK, feel free to merge it independently of the DIR patch, I sent them in series because they need to be applied in order (otherwise there would be merge conflicts). > >> diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c >> index eaa320148d97..6db9847ca031 100644 >> --- a/kernel/auditfilter.c >> +++ b/kernel/auditfilter.c >> @@ -428,8 +428,6 @@ static int audit_field_valid(struct audit_entry *entry, >> struct audit_field *f) >> case AUDIT_EXE: >> if (f->op != Audit_not_equal && f->op != Audit_equal) >> return -EINVAL; >> - if (entry->rule.listnr != AUDIT_FILTER_EXIT) >> - return -EINVAL; >> break; >> } >> return 0; >> @@ -1360,6 +1358,11 @@ int audit_filter(int msgtype, unsigned int listtype) >> f->type, f->op, >> f->lsm_rule, NULL); >> } >> break; >> + case AUDIT_EXE: >> + result = audit_exe_compare(current, >> e->rule.exe); >> + if (f->op == Audit_not_equal) >> + result = !result; >> + break; >> default: >> goto unlock_and_return; >> } >> -- >> 2.17.0 > > -- > paul moore > www.paul-moore.com -- Ondrej Mosnacek Associate Software Engineer, Security Technologies Red Hat, Inc. -- Linux-audit mailing list Linux-audit@redhat.com https://www.redhat.com/mailman/listinfo/linux-audit
[PATCH ghak82 v3] audit: Fix extended comparison of GID/EGID
The audit_filter_rules() function in auditsc.c used the in_[e]group_p() functions to check GID/EGID match, but these functions use the current task's credentials, while the comparison should use the credentials of the task given to audit_filter_rules() as a parameter (tsk). Note that we can use group_search(cred->group_info, ...) as a replacement for both in_group_p and in_egroup_p as these functions only compare the parameter to cred->fsgid/egid and then call group_search. In fact, the usage of in_group_p was even more incorrect: it compares to cred->fsgid (which is usually equal to cred->egid) and not cred->gid. GitHub issue: https://github.com/linux-audit/audit-kernel/issues/82 Fixes: 37eebe39c973 ("audit: improve GID/EGID comparation logic") Signed-off-by: Ondrej Mosnacek --- kernel/auditsc.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/kernel/auditsc.c b/kernel/auditsc.c index ceb1c4596c51..518a3336a697 100644 --- a/kernel/auditsc.c +++ b/kernel/auditsc.c @@ -494,20 +494,20 @@ static int audit_filter_rules(struct task_struct *tsk, result = audit_gid_comparator(cred->gid, f->op, f->gid); if (f->op == Audit_equal) { if (!result) - result = in_group_p(f->gid); + result = groups_search(cred->group_info, f->gid); } else if (f->op == Audit_not_equal) { if (result) - result = !in_group_p(f->gid); + result = !groups_search(cred->group_info, f->gid); } break; case AUDIT_EGID: result = audit_gid_comparator(cred->egid, f->op, f->gid); if (f->op == Audit_equal) { if (!result) - result = in_egroup_p(f->gid); + result = groups_search(cred->group_info, f->gid); } else if (f->op == Audit_not_equal) { if (result) - result = !in_egroup_p(f->gid); + result = !groups_search(cred->group_info, f->gid); } break; case AUDIT_SGID: -- 2.17.1 -- Linux-audit mailing list Linux-audit@redhat.com https://www.redhat.com/mailman/listinfo/linux-audit