Re: [RFC PATCH ghak86 V1] audit: use audit_enabled as a boolean where convenient
On Thu, May 31, 2018 at 12:38 PM, Richard Guy Briggs wrote: > On 2018-05-31 11:48, Paul Moore wrote: >> On Thu, May 31, 2018 at 11:13 AM, Richard Guy Briggs wrote: >> > Most uses of audit_enabled don't care about the distinction between >> > AUDIT_ON and AUDIT_LOCKED, so using audit_enabled as a boolean makes >> > more sense and is easier to read. Most uses of audit_enabled treat it as >> > a boolean, so switch the remaining AUDIT_OFF usage to simply use >> > audit_enabled as a boolean where applicable. >> > >> > See: https://github.com/linux-audit/audit-kernel/issues/86 >> > >> > Signed-off-by: Richard Guy Briggs >> > --- >> > drivers/tty/tty_audit.c | 2 +- >> > include/net/xfrm.h | 2 +- >> > kernel/audit.c | 8 >> > net/netfilter/xt_AUDIT.c | 2 +- >> > net/netlabel/netlabel_user.c | 2 +- >> > 5 files changed, 8 insertions(+), 8 deletions(-) >> >> I'm not sure I like this idea. Yes, technically this change is >> functionally equivalent but I worry that this will increase the chance >> that non-audit folks will mistake audit_enabled as a true/false value >> when it is not. It might work now, but I worry about some subtle >> problem in the future. > > Would you prefer a patch to change the majority (18) of uses of > audit_enabled to be of the form "audit_enabled == AUDIT_OFF" (or > "audit_enabled != AUDIT_OFF")? > > I prefer the approach in this patch because it makes the code smaller > and significantly easier to read, but either way, I'd like all uses to > be consistent so that it is easier to read all the code similarly. > >> If you are bothered by the comparison to 0 (magic numbers), you could >> move the AUDIT_OFF/AUDIT_ON/AUDIT_LOCKED definitions into >> include/linux/audit.h and convert the "audit_enabled == 0" to >> "audit_enabled == AUDIT_OFF". > > I'd be fine doing that if you really dislike this patch's approach. Like I said, I'm don't really care for the boolean-like approach of this first patch. -- paul moore www.paul-moore.com -- Linux-audit mailing list Linux-audit@redhat.com https://www.redhat.com/mailman/listinfo/linux-audit
Re: [RFC PATCH ghak32 V2 01/13] audit: add container id
On 2018-05-17 17:00, Steve Grubb wrote:
> On Fri, 16 Mar 2018 05:00:28 -0400
> Richard Guy Briggs wrote:
>
> > Implement the proc fs write to set the audit container ID of a
> > process, emitting an AUDIT_CONTAINER record to document the event.
> >
> > This is a write from the container orchestrator task to a proc entry
> > of the form /proc/PID/containerid where PID is the process ID of the
> > newly created task that is to become the first task in a container,
> > or an additional task added to a container.
> >
> > The write expects up to a u64 value (unset: 18446744073709551615).
> >
> > This will produce a record such as this:
> > type=CONTAINER msg=audit(1519903238.968:261): op=set pid=596 uid=0
> > subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 auid=0
> > tty=pts0 ses=1 opid=596 old-contid=18446744073709551615 contid=123455
> > res=0
>
> The was one thing I was wondering about. Currently when we set the
> loginuid, the record is AUDIT_LOGINUID. The corollary is that when we
> set the container id, the event should be AUDIT_CONTAINERID or
> AUDIT_CONTAINER_ID.
>
> During syscall events, the path info is returned in a a record simply
> called AUDIT_PATH, cwd info is returned in AUDIT_CWD. So, rather than
> calling the record that gets attached to everything
> AUDIT_CONTAINER_INFO, how about simply AUDIT_CONTAINER.
>
>
> > The "op" field indicates an initial set. The "pid" to "ses" fields
> > are the orchestrator while the "opid" field is the object's PID, the
> > process being "contained". Old and new container ID values are given
> > in the "contid" fields, while res indicates its success.
> >
> > It is not permitted to self-set, unset or re-set the container ID. A
> > child inherits its parent's container ID, but then can be set only
> > once after.
> >
> > See: https://github.com/linux-audit/audit-kernel/issues/32
> >
> > Signed-off-by: Richard Guy Briggs
> > ---
> > fs/proc/base.c | 37
> > include/linux/audit.h | 16 +
> > include/linux/init_task.h | 4 ++-
> > include/linux/sched.h | 1 +
> > include/uapi/linux/audit.h | 2 ++
> > kernel/auditsc.c | 84
> > ++ 6 files changed, 143
> > insertions(+), 1 deletion(-)
> >
> > diff --git a/fs/proc/base.c b/fs/proc/base.c
> > index 60316b5..6ce4fbe 100644
> > --- a/fs/proc/base.c
> > +++ b/fs/proc/base.c
> > @@ -1299,6 +1299,41 @@ static ssize_t proc_sessionid_read(struct file
> > * file, char __user * buf, .read= proc_sessionid_read,
> > .llseek = generic_file_llseek,
> > };
> > +
> > +static ssize_t proc_containerid_write(struct file *file, const char
> > __user *buf,
> > + size_t count, loff_t *ppos)
> > +{
> > + struct inode *inode = file_inode(file);
> > + u64 containerid;
> > + int rv;
> > + struct task_struct *task = get_proc_task(inode);
> > +
> > + if (!task)
> > + return -ESRCH;
> > + if (*ppos != 0) {
> > + /* No partial writes. */
> > + put_task_struct(task);
> > + return -EINVAL;
> > + }
> > +
> > + rv = kstrtou64_from_user(buf, count, 10, &containerid);
> > + if (rv < 0) {
> > + put_task_struct(task);
> > + return rv;
> > + }
> > +
> > + rv = audit_set_containerid(task, containerid);
> > + put_task_struct(task);
> > + if (rv < 0)
> > + return rv;
> > + return count;
> > +}
> > +
> > +static const struct file_operations proc_containerid_operations = {
> > + .write = proc_containerid_write,
> > + .llseek = generic_file_llseek,
> > +};
> > +
> > #endif
> >
> > #ifdef CONFIG_FAULT_INJECTION
> > @@ -2961,6 +2996,7 @@ static int proc_pid_patch_state(struct seq_file
> > *m, struct pid_namespace *ns, #ifdef CONFIG_AUDITSYSCALL
> > REG("loginuid", S_IWUSR|S_IRUGO, proc_loginuid_operations),
> > REG("sessionid", S_IRUGO, proc_sessionid_operations),
> > + REG("containerid", S_IWUSR, proc_containerid_operations),
> > #endif
> > #ifdef CONFIG_FAULT_INJECTION
> > REG("make-it-fail", S_IRUGO|S_IWUSR,
> > proc_fault_inject_operations), @@ -3355,6 +3391,7 @@ static int
> > proc_tid_comm_permission(struct inode *inode, int mask) #ifdef
> > CONFIG_AUDITSYSCALL REG("loginuid", S_IWUSR|S_IRUGO,
> > proc_loginuid_operations), REG("sessionid", S_IRUGO,
> > proc_sessionid_operations),
> > + REG("containerid", S_IWUSR, proc_containerid_operations),
> > #endif
> > #ifdef CONFIG_FAULT_INJECTION
> > REG("make-it-fail", S_IRUGO|S_IWUSR,
> > proc_fault_inject_operations), diff --git a/include/linux/audit.h
> > b/include/linux/audit.h index af410d9..fe4ba3f 100644
> > --- a/include/linux/audit.h
> > +++ b/include/linux/audit.h
> > @@ -29,6 +29,7 @@
> >
> > #define AUDIT_INO_UNSET ((unsigned long)-1)
> > #define AUDIT_DEV_UNSET ((dev_t)-1)
> > +#define INVALID_CID AUDIT_CID_UNSET
> >
> > struct audit_sig_info {
> > uid_t
Re: [PATCH 8/8] ima: Differentiate auditing policy rules from "audit" actions
On 06/01/2018 04:13 PM, Paul Moore wrote:
On Fri, Jun 1, 2018 at 4:00 PM, Stefan Berger
wrote:
On 05/30/2018 07:34 PM, Richard Guy Briggs wrote:
On 2018-05-30 17:38, Stefan Berger wrote:
On 05/30/2018 05:22 PM, Paul Moore wrote:
On Wed, May 30, 2018 at 9:08 AM, Stefan Berger
wrote:
On 05/30/2018 08:49 AM, Richard Guy Briggs wrote:
On 2018-05-24 16:11, 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.
With this change we now call integrity_audit_msg_common() to get
common integrity auditing fields. This now produces the following
record when parsing an IMA policy rule:
type=UNKNOWN[1806] msg=audit(1527004216.690:311): action=dont_measure
\
fsmagic=0x9fa0 pid=1613 uid=0 auid=0 ses=2 \
subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 \
op=policy_update cause=parse_rule comm="echo"
exe="/usr/bin/echo" \
tty=tty2 res=1
Signed-off-by: Stefan Berger
---
include/uapi/linux/audit.h | 3 ++-
security/integrity/ima/ima_policy.c | 5 +++--
2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
index 4e61a9e05132..776e0abd35cf 100644
--- a/include/uapi/linux/audit.h
+++ b/include/uapi/linux/audit.h
@@ -146,7 +146,8 @@
#define AUDIT_INTEGRITY_STATUS1802 /* Integrity
enable
status */
#define AUDIT_INTEGRITY_HASH 1803 /* Integrity HASH type */
#define AUDIT_INTEGRITY_PCR 1804 /* PCR invalidation msgs
*/
-#define AUDIT_INTEGRITY_RULE 1805 /* policy rule */
+#define AUDIT_INTEGRITY_RULE 1805 /* IMA "audit" action policy
msgs */
+#define AUDIT_INTEGRITY_POLICY_RULE 1806 /* IMA policy rules */
#define AUDIT_KERNEL2000/* Asynchronous
audit
record. NOT A REQUEST. */
diff --git a/security/integrity/ima/ima_policy.c
b/security/integrity/ima/ima_policy.c
index 3aed25a7178a..a8ae47a386b4 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -634,7 +634,7 @@ static int ima_parse_rule(char *rule, struct
ima_rule_entry *entry)
int result = 0;
ab = integrity_audit_log_start(NULL, GFP_KERNEL,
- AUDIT_INTEGRITY_RULE);
+ AUDIT_INTEGRITY_POLICY_RULE);
Is it possible to connect this record to a syscall by replacing the
first parameter (NULL) by current->context?
We're likely going to need to "associate" this record (audit speak for
making the first parameter non-NULL) with others for the audit
container ID work. If you do it now, Richard's patches will likely
get a few lines smaller and that will surely make him a bit happier :)
Richard is also introducing a local context that we can then create and
use
instead of the NULL. Can we not use that then?
That is for records for which there is no syscall or user associated.
In fact there is another recent change that would be better to use than
current->audit_context, which is the function audit_context().
See commit cdfb6b3 ("audit: use inline function to get audit context").
Steven seems to say: "We don't want to add syscall records to everything.
That messes up schemas and existing code. The integrity events are 1
record
in size and should stay that way. This saves disk space and improves
readability."
We would have to fix current->context in this case since it is NULL. We
get
to this location by root cat'ing a policy or writing a policy filename
into
/sys/kernel/security/ima/policy.
Perhaps I'm missing something, but current in this case should point
to the process which is writing to the policy file, yes?
Yes, but current->context is NULL for some reason.
Is it always this way? If it isn't, which it should not be, we should
find out why. Well, we should find out why this is NULL anyways, since
it shouldn't be.
When someone writes a policy for IMA into securityfs, it's always NULL.
There's another location where IMA uses the current->audit_context, and
that's here:
https://elixir.bootlin.com/linux/latest/source/security/integrity/ima/ima_api.c#L323
At this location we sometimes see a (background) process with an
audit_context but in the majority of cases it's current->audit_context is
NULL. Starting a process as root or also non-root user, with the appropriate
IMA audit policy rules set, we always see a NULL audit_context here.
What does your audit configuration look like?
Depending on your configuration a NULL audit_context can be expected,
see audit_dummy_context(). I believe the default Fedora audit config
will leave you with a NULL audit_context for all processes. I also
believe that unless you explicitly set "audit=1" on the kernel command
line the init/systemd process will have a NULL audit_context (there
was actually a range of kernels where even setting "audit=1" woul
[PATCH ghak89 V2] 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
---
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..2678422 100644
--- a/include/uapi/linux/audit.h
+++ b/include/uapi/linux/audit.h
@@ -156,8 +156,9 @@
#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 at audit_log_start */
#define AUDIT_FILTER_FS0x06/* Apply rule at
__audit_inode_child */
+#define AUDIT_FILTER_TYPE AUDIT_FILTER_EXCLUDE /* obsolete misleading
naming */
#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 8/8] ima: Differentiate auditing policy rules from "audit" actions
On Fri, Jun 1, 2018 at 4:13 PM, Paul Moore wrote:
> On Fri, Jun 1, 2018 at 4:00 PM, Stefan Berger
> wrote:
>> On 05/30/2018 07:34 PM, Richard Guy Briggs wrote:
>>>
>>> On 2018-05-30 17:38, Stefan Berger wrote:
On 05/30/2018 05:22 PM, Paul Moore wrote:
>
> On Wed, May 30, 2018 at 9:08 AM, Stefan Berger
> wrote:
>>
>> On 05/30/2018 08:49 AM, Richard Guy Briggs wrote:
>>>
>>> On 2018-05-24 16:11, 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.
With this change we now call integrity_audit_msg_common() to get
common integrity auditing fields. This now produces the following
record when parsing an IMA policy rule:
type=UNKNOWN[1806] msg=audit(1527004216.690:311): action=dont_measure
\
fsmagic=0x9fa0 pid=1613 uid=0 auid=0 ses=2 \
subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 \
op=policy_update cause=parse_rule comm="echo"
exe="/usr/bin/echo" \
tty=tty2 res=1
Signed-off-by: Stefan Berger
---
include/uapi/linux/audit.h | 3 ++-
security/integrity/ima/ima_policy.c | 5 +++--
2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
index 4e61a9e05132..776e0abd35cf 100644
--- a/include/uapi/linux/audit.h
+++ b/include/uapi/linux/audit.h
@@ -146,7 +146,8 @@
#define AUDIT_INTEGRITY_STATUS1802 /* Integrity
enable
status */
#define AUDIT_INTEGRITY_HASH 1803 /* Integrity HASH type */
#define AUDIT_INTEGRITY_PCR 1804 /* PCR invalidation msgs
*/
-#define AUDIT_INTEGRITY_RULE 1805 /* policy rule */
+#define AUDIT_INTEGRITY_RULE 1805 /* IMA "audit" action policy
msgs */
+#define AUDIT_INTEGRITY_POLICY_RULE 1806 /* IMA policy rules */
#define AUDIT_KERNEL2000/* Asynchronous
audit
record. NOT A REQUEST. */
diff --git a/security/integrity/ima/ima_policy.c
b/security/integrity/ima/ima_policy.c
index 3aed25a7178a..a8ae47a386b4 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -634,7 +634,7 @@ static int ima_parse_rule(char *rule, struct
ima_rule_entry *entry)
int result = 0;
ab = integrity_audit_log_start(NULL, GFP_KERNEL,
- AUDIT_INTEGRITY_RULE);
+ AUDIT_INTEGRITY_POLICY_RULE);
>>>
>>> Is it possible to connect this record to a syscall by replacing the
>>> first parameter (NULL) by current->context?
>
> We're likely going to need to "associate" this record (audit speak for
> making the first parameter non-NULL) with others for the audit
> container ID work. If you do it now, Richard's patches will likely
> get a few lines smaller and that will surely make him a bit happier :)
Richard is also introducing a local context that we can then create and
use
instead of the NULL. Can we not use that then?
>>>
>>> That is for records for which there is no syscall or user associated.
>>>
>>> In fact there is another recent change that would be better to use than
>>> current->audit_context, which is the function audit_context().
>>> See commit cdfb6b3 ("audit: use inline function to get audit context").
>>>
Steven seems to say: "We don't want to add syscall records to everything.
That messes up schemas and existing code. The integrity events are 1
record
in size and should stay that way. This saves disk space and improves
readability."
>> We would have to fix current->context in this case since it is NULL. We
>> get
>> to this location by root cat'ing a policy or writing a policy filename
>> into
>> /sys/kernel/security/ima/policy.
>
> Perhaps I'm missing something, but current in this case should point
> to the process which is writing to the policy file, yes?
Yes, but current->context is NULL for some reason.
>>>
>>> Is it always this way? If it isn't, which it should not be, we should
>>> find out why. Well, we should find out why this is NULL anyways, since
>>> it shouldn't be.
>>
>>
>> When someone writes a policy for IMA into securityfs, it's always NULL.
>> There's another location where IMA uses the current->audit_context, and
>> that's here:
>>
>> https://elixir.bootlin.com/linux/latest/source/security/integrity/ima/ima_api.c#L323
>>
>>
Re: [RFC PATCH ghak89 V1] audit: rename FILTER_TYPE to FILTER_EXCL
On 2018-06-01 15:37, Steve Grubb wrote: > On Friday, June 1, 2018 3:12:15 PM EDT Richard Guy Briggs wrote: > > On 2018-06-01 15:03, Steve Grubb wrote: > > > On Friday, June 1, 2018 1:58:34 PM EDT Richard Guy Briggs wrote: > > > > On 2018-06-01 12:55, Steve Grubb wrote: > > > > > On Thursday, May 31, 2018 6:21:20 PM EDT Richard Guy Briggs wrote: > > > > > > On 2018-05-31 17:29, Steve Grubb wrote: > > > > > > > On Thursday, May 31, 2018 4:23:09 PM EDT Richard Guy Briggs > wrote: > > > > > > > > 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. > > > > > > > > > > > > > > Historically speaking, this is not why it is the way it is. But I > > > > > > > think > > > > > > > it > > > > > > > doesn't mean that you cannot do something like this: > > > > > > > > > > > > > > #define AUDIT_FILTER_EXCLUDEAUDIT_FILTER_TYPE > > > > > > > > > > > > I was originally hoping to do that, but that then causes a build > > > > > > error > > > > > > on any previous version of audit userspace. > > > > > > > > > > I cannot reproduce this. What error did you get? What version of gcc? > > > > > > > > I didn't even try to compile it since I'd predicted that there would be > > > > a symbol definition conflict. > > > > > > > > How did you not get a conflict with that definition also in the kernel > > > > header? > > > > > > It's an identical definition. That's OK. Changes to a definition is last > > > one wins - but you get a warning not an error. > > > > Do any distros compile with -Werror? > > Audit itself can't be compiled with -Werror as there are lots of warnings > about using string functions with unsigned chars. However, libaudit.h is used > in 20 or so packages and there is a chance one may have -Werror. But I think > its unlikely based on a recent project which involved looking over static > analysis results for a large chunk of the Fedora 27 repo. Out of 4730 source > packages, 84 had no compiler warnings. So, I'd say its next to impossible for > any distribution to make -Werror a blanket policy. Ok, I'll switch my patch to match your definition. Is there any plan to migrate the documentation to match? > -Steve - 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 -- Linux-audit mailing list Linux-audit@redhat.com https://www.redhat.com/mailman/listinfo/linux-audit
Re: [PATCH 8/8] ima: Differentiate auditing policy rules from "audit" actions
On Fri, Jun 1, 2018 at 4:00 PM, Stefan Berger
wrote:
> On 05/30/2018 07:34 PM, Richard Guy Briggs wrote:
>>
>> On 2018-05-30 17:38, Stefan Berger wrote:
>>>
>>> On 05/30/2018 05:22 PM, Paul Moore wrote:
On Wed, May 30, 2018 at 9:08 AM, Stefan Berger
wrote:
>
> On 05/30/2018 08:49 AM, Richard Guy Briggs wrote:
>>
>> On 2018-05-24 16:11, 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.
>>>
>>> With this change we now call integrity_audit_msg_common() to get
>>> common integrity auditing fields. This now produces the following
>>> record when parsing an IMA policy rule:
>>>
>>> type=UNKNOWN[1806] msg=audit(1527004216.690:311): action=dont_measure
>>> \
>>> fsmagic=0x9fa0 pid=1613 uid=0 auid=0 ses=2 \
>>> subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 \
>>> op=policy_update cause=parse_rule comm="echo"
>>> exe="/usr/bin/echo" \
>>> tty=tty2 res=1
>>>
>>> Signed-off-by: Stefan Berger
>>> ---
>>> include/uapi/linux/audit.h | 3 ++-
>>> security/integrity/ima/ima_policy.c | 5 +++--
>>> 2 files changed, 5 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
>>> index 4e61a9e05132..776e0abd35cf 100644
>>> --- a/include/uapi/linux/audit.h
>>> +++ b/include/uapi/linux/audit.h
>>> @@ -146,7 +146,8 @@
>>> #define AUDIT_INTEGRITY_STATUS1802 /* Integrity
>>> enable
>>> status */
>>> #define AUDIT_INTEGRITY_HASH 1803 /* Integrity HASH type */
>>> #define AUDIT_INTEGRITY_PCR 1804 /* PCR invalidation msgs
>>> */
>>> -#define AUDIT_INTEGRITY_RULE 1805 /* policy rule */
>>> +#define AUDIT_INTEGRITY_RULE 1805 /* IMA "audit" action policy
>>> msgs */
>>> +#define AUDIT_INTEGRITY_POLICY_RULE 1806 /* IMA policy rules */
>>> #define AUDIT_KERNEL2000/* Asynchronous
>>> audit
>>> record. NOT A REQUEST. */
>>> diff --git a/security/integrity/ima/ima_policy.c
>>> b/security/integrity/ima/ima_policy.c
>>> index 3aed25a7178a..a8ae47a386b4 100644
>>> --- a/security/integrity/ima/ima_policy.c
>>> +++ b/security/integrity/ima/ima_policy.c
>>> @@ -634,7 +634,7 @@ static int ima_parse_rule(char *rule, struct
>>> ima_rule_entry *entry)
>>> int result = 0;
>>> ab = integrity_audit_log_start(NULL, GFP_KERNEL,
>>> - AUDIT_INTEGRITY_RULE);
>>> + AUDIT_INTEGRITY_POLICY_RULE);
>>
>> Is it possible to connect this record to a syscall by replacing the
>> first parameter (NULL) by current->context?
We're likely going to need to "associate" this record (audit speak for
making the first parameter non-NULL) with others for the audit
container ID work. If you do it now, Richard's patches will likely
get a few lines smaller and that will surely make him a bit happier :)
>>>
>>> Richard is also introducing a local context that we can then create and
>>> use
>>> instead of the NULL. Can we not use that then?
>>
>> That is for records for which there is no syscall or user associated.
>>
>> In fact there is another recent change that would be better to use than
>> current->audit_context, which is the function audit_context().
>> See commit cdfb6b3 ("audit: use inline function to get audit context").
>>
>>> Steven seems to say: "We don't want to add syscall records to everything.
>>> That messes up schemas and existing code. The integrity events are 1
>>> record
>>> in size and should stay that way. This saves disk space and improves
>>> readability."
>>>
> We would have to fix current->context in this case since it is NULL. We
> get
> to this location by root cat'ing a policy or writing a policy filename
> into
> /sys/kernel/security/ima/policy.
Perhaps I'm missing something, but current in this case should point
to the process which is writing to the policy file, yes?
>>>
>>> Yes, but current->context is NULL for some reason.
>>
>> Is it always this way? If it isn't, which it should not be, we should
>> find out why. Well, we should find out why this is NULL anyways, since
>> it shouldn't be.
>
>
> When someone writes a policy for IMA into securityfs, it's always NULL.
> There's another location where IMA uses the current->audit_context, and
> that's here:
>
> https://elixir.bootlin.com/linux/latest/source/security/integrity/ima/ima_api.c#L323
>
> At this location we sometimes see a (background) process with an
> audit_context but in the majority of cases it's current->audit_context is
> NULL. Starting a process
Re: [RFC PATCH 2/2] [WIP] audit: allow other filter list types for AUDIT_DIR
On 2018-06-01 10:12, Ondrej Mosnacek wrote:
> 2018-05-31 22:52 GMT+02:00 Richard Guy Briggs :
> > On 2018-05-30 10:45, Ondrej Mosnacek wrote:
> >> This patch allows the AUDIR_DIR field to be used also with the exclude
> >> filter.
> >>
> >> Not-yet-signed-off-by: Ondrej Mosnacek
> >> ---
> >> kernel/audit.c | 5 +++--
> >> kernel/audit.h | 32 +++-
> >> kernel/audit_tree.c | 4 +++-
> >> kernel/auditfilter.c | 6 +-
> >> kernel/auditsc.c | 28
> >> 5 files changed, 42 insertions(+), 33 deletions(-)
> >>
> >> diff --git a/kernel/audit.c b/kernel/audit.c
> >> index e7478cb58079..aac5b6ecc11d 100644
> >> --- a/kernel/audit.c
> >> +++ b/kernel/audit.c
> >> @@ -1333,7 +1333,8 @@ static int audit_receive_msg(struct sk_buff *skb,
> >> struct nlmsghdr *nlh)
> >> if (!audit_enabled && msg_type != AUDIT_USER_AVC)
> >> return 0;
> >>
> >> - err = audit_filter(msg_type, AUDIT_FILTER_USER);
> >> + // FIXME: do we need to pass the context here?
> >> + err = audit_filter(msg_type, AUDIT_FILTER_USER, NULL);
> >> if (err == 1) { /* match or error */
> >> err = 0;
> >> if (msg_type == AUDIT_USER_TTY) {
> >> @@ -1754,7 +1755,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_TYPE, ctx)))
> >> return NULL;
> >>
> >> /* NOTE: don't ever fail/sleep on these two conditions:
> >> diff --git a/kernel/audit.h b/kernel/audit.h
> >> index 214e14948370..43cfeba25802 100644
> >> --- a/kernel/audit.h
> >> +++ b/kernel/audit.h
> >> @@ -324,13 +324,43 @@ extern void audit_kill_trees(struct list_head *list);
> >> #define audit_kill_trees(list) BUG()
> >> #endif
> >>
> >> +struct audit_tree_refs {
> >> + struct audit_tree_refs *next;
> >> + struct audit_chunk *c[31];
> >> +};
> >> +
> >> +/* A utility function to match tree refs: */
> >> +static inline int match_tree_refs(struct audit_context *ctx, struct
> >> audit_tree *tree)
> >> +{
> >> +#ifdef CONFIG_AUDIT_TREE
> >> + struct audit_tree_refs *p;
> >> + int n;
> >> + if (!tree)
> >> + return 0;
> >> + /* full ones */
> >> + for (p = ctx->first_trees; p != ctx->trees; p = p->next) {
> >> + for (n = 0; n < 31; n++)
> >> + if (audit_tree_match(p->c[n], tree))
> >> + return 1;
> >> + }
> >> + /* partial */
> >> + if (p) {
> >> + for (n = ctx->tree_count; n < 31; n++)
> >> + if (audit_tree_match(p->c[n], tree))
> >> + return 1;
> >> + }
> >> +#endif
> >> + return 0;
> >> +}
> >> +
> >> extern char *audit_unpack_string(void **bufp, size_t *remain, size_t len);
> >>
> >> extern pid_t audit_sig_pid;
> >> extern kuid_t audit_sig_uid;
> >> extern u32 audit_sig_sid;
> >>
> >> -extern int audit_filter(int msgtype, unsigned int listtype);
> >> +extern int audit_filter(int msgtype, unsigned int listtype,
> >> + struct audit_context *ctx);
> >>
> >> #ifdef CONFIG_AUDITSYSCALL
> >> extern int audit_signal_info(int sig, struct task_struct *t);
> >> diff --git a/kernel/audit_tree.c b/kernel/audit_tree.c
> >> index 67e6956c0b61..d4d36389c3d7 100644
> >> --- a/kernel/audit_tree.c
> >> +++ b/kernel/audit_tree.c
> >> @@ -675,9 +675,11 @@ void audit_trim_trees(void)
> >>
> >> int audit_make_tree(struct audit_krule *rule, char *pathname, u32 op)
> >> {
> >> + if (krule->listnr != AUDIT_FILTER_EXIT &&
> >> + krule->listnr != AUDIT_FILTER_TYPE)
> >> + return -EINVAL;
> >>
> >> if (pathname[0] != '/' ||
> >> - rule->listnr != AUDIT_FILTER_EXIT ||
> >> op != Audit_equal ||
> >> rule->inode_f || rule->watch || rule->tree)
> >> return -EINVAL;
> >> diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
> >> index 6db9847ca031..e1d70cb77ea3 100644
> >> --- a/kernel/auditfilter.c
> >> +++ b/kernel/auditfilter.c
> >> @@ -1309,7 +1309,7 @@ int audit_compare_dname_path(const char *dname,
> >> const char *path, int parentlen)
> >> return strncmp(p, dname, dlen);
> >> }
> >>
> >> -int audit_filter(int msgtype, unsigned int listtype)
> >> +int audit_filter(int msgtype, unsigned int listtype, struct audit_context
> >> *ctx)
> >> {
> >> struct audit_entry *e;
> >> int ret = 1; /* Audit by default */
> >> @@ -1363,6 +1363,10 @@ int audit_filter(int msgtype, unsigned int listtype)
> >> if (f->op == Audit_not_equal)
> >> result = !result;
> >>
Re: [PATCH 8/8] ima: Differentiate auditing policy rules from "audit" actions
On 05/30/2018 07:34 PM, Richard Guy Briggs wrote:
On 2018-05-30 17:38, Stefan Berger wrote:
On 05/30/2018 05:22 PM, Paul Moore wrote:
On Wed, May 30, 2018 at 9:08 AM, Stefan Berger
wrote:
On 05/30/2018 08:49 AM, Richard Guy Briggs wrote:
On 2018-05-24 16:11, 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.
With this change we now call integrity_audit_msg_common() to get
common integrity auditing fields. This now produces the following
record when parsing an IMA policy rule:
type=UNKNOWN[1806] msg=audit(1527004216.690:311): action=dont_measure \
fsmagic=0x9fa0 pid=1613 uid=0 auid=0 ses=2 \
subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 \
op=policy_update cause=parse_rule comm="echo" exe="/usr/bin/echo" \
tty=tty2 res=1
Signed-off-by: Stefan Berger
---
include/uapi/linux/audit.h | 3 ++-
security/integrity/ima/ima_policy.c | 5 +++--
2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
index 4e61a9e05132..776e0abd35cf 100644
--- a/include/uapi/linux/audit.h
+++ b/include/uapi/linux/audit.h
@@ -146,7 +146,8 @@
#define AUDIT_INTEGRITY_STATUS1802 /* Integrity enable
status */
#define AUDIT_INTEGRITY_HASH 1803 /* Integrity HASH type */
#define AUDIT_INTEGRITY_PCR 1804 /* PCR invalidation msgs */
-#define AUDIT_INTEGRITY_RULE 1805 /* policy rule */
+#define AUDIT_INTEGRITY_RULE 1805 /* IMA "audit" action policy
msgs */
+#define AUDIT_INTEGRITY_POLICY_RULE 1806 /* IMA policy rules */
#define AUDIT_KERNEL2000/* Asynchronous audit
record. NOT A REQUEST. */
diff --git a/security/integrity/ima/ima_policy.c
b/security/integrity/ima/ima_policy.c
index 3aed25a7178a..a8ae47a386b4 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -634,7 +634,7 @@ static int ima_parse_rule(char *rule, struct
ima_rule_entry *entry)
int result = 0;
ab = integrity_audit_log_start(NULL, GFP_KERNEL,
- AUDIT_INTEGRITY_RULE);
+ AUDIT_INTEGRITY_POLICY_RULE);
Is it possible to connect this record to a syscall by replacing the
first parameter (NULL) by current->context?
We're likely going to need to "associate" this record (audit speak for
making the first parameter non-NULL) with others for the audit
container ID work. If you do it now, Richard's patches will likely
get a few lines smaller and that will surely make him a bit happier :)
Richard is also introducing a local context that we can then create and use
instead of the NULL. Can we not use that then?
That is for records for which there is no syscall or user associated.
In fact there is another recent change that would be better to use than
current->audit_context, which is the function audit_context().
See commit cdfb6b3 ("audit: use inline function to get audit context").
Steven seems to say: "We don't want to add syscall records to everything.
That messes up schemas and existing code. The integrity events are 1 record
in size and should stay that way. This saves disk space and improves
readability."
We would have to fix current->context in this case since it is NULL. We get
to this location by root cat'ing a policy or writing a policy filename into
/sys/kernel/security/ima/policy.
Perhaps I'm missing something, but current in this case should point
to the process which is writing to the policy file, yes?
Yes, but current->context is NULL for some reason.
Is it always this way? If it isn't, which it should not be, we should
find out why. Well, we should find out why this is NULL anyways, since
it shouldn't be.
When someone writes a policy for IMA into securityfs, it's always NULL.
There's another location where IMA uses the current->audit_context, and
that's here:
https://elixir.bootlin.com/linux/latest/source/security/integrity/ima/ima_api.c#L323
At this location we sometimes see a (background) process with an
audit_context but in the majority of cases it's current->audit_context
is NULL. Starting a process as root or also non-root user, with the
appropriate IMA audit policy rules set, we always see a NULL
audit_context here.
- 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
--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit
Re: [RFC PATCH ghak89 V1] audit: rename FILTER_TYPE to FILTER_EXCL
On Friday, June 1, 2018 3:12:15 PM EDT Richard Guy Briggs wrote: > On 2018-06-01 15:03, Steve Grubb wrote: > > On Friday, June 1, 2018 1:58:34 PM EDT Richard Guy Briggs wrote: > > > On 2018-06-01 12:55, Steve Grubb wrote: > > > > On Thursday, May 31, 2018 6:21:20 PM EDT Richard Guy Briggs wrote: > > > > > On 2018-05-31 17:29, Steve Grubb wrote: > > > > > > On Thursday, May 31, 2018 4:23:09 PM EDT Richard Guy Briggs wrote: > > > > > > > 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. > > > > > > > > > > > > Historically speaking, this is not why it is the way it is. But I > > > > > > think > > > > > > it > > > > > > doesn't mean that you cannot do something like this: > > > > > > > > > > > > #define AUDIT_FILTER_EXCLUDEAUDIT_FILTER_TYPE > > > > > > > > > > I was originally hoping to do that, but that then causes a build > > > > > error > > > > > on any previous version of audit userspace. > > > > > > > > I cannot reproduce this. What error did you get? What version of gcc? > > > > > > I didn't even try to compile it since I'd predicted that there would be > > > a symbol definition conflict. > > > > > > How did you not get a conflict with that definition also in the kernel > > > header? > > > > It's an identical definition. That's OK. Changes to a definition is last > > one wins - but you get a warning not an error. > > Do any distros compile with -Werror? Audit itself can't be compiled with -Werror as there are lots of warnings about using string functions with unsigned chars. However, libaudit.h is used in 20 or so packages and there is a chance one may have -Werror. But I think its unlikely based on a recent project which involved looking over static analysis results for a large chunk of the Fedora 27 repo. Out of 4730 source packages, 84 had no compiler warnings. So, I'd say its next to impossible for any distribution to make -Werror a blanket policy. -Steve -- Linux-audit mailing list Linux-audit@redhat.com https://www.redhat.com/mailman/listinfo/linux-audit
Re: [RFC PATCH ghak89 V1] audit: rename FILTER_TYPE to FILTER_EXCL
On 2018-06-01 15:03, Steve Grubb wrote: > On Friday, June 1, 2018 1:58:34 PM EDT Richard Guy Briggs wrote: > > On 2018-06-01 12:55, Steve Grubb wrote: > > > On Thursday, May 31, 2018 6:21:20 PM EDT Richard Guy Briggs wrote: > > > > On 2018-05-31 17:29, Steve Grubb wrote: > > > > > On Thursday, May 31, 2018 4:23:09 PM EDT Richard Guy Briggs wrote: > > > > > > 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. > > > > > > > > > > Historically speaking, this is not why it is the way it is. But I > > > > > think > > > > > it > > > > > doesn't mean that you cannot do something like this: > > > > > > > > > > #define AUDIT_FILTER_EXCLUDEAUDIT_FILTER_TYPE > > > > > > > > I was originally hoping to do that, but that then causes a build error > > > > on any previous version of audit userspace. > > > > > > I cannot reproduce this. What error did you get? What version of gcc? > > > > I didn't even try to compile it since I'd predicted that there would be > > a symbol definition conflict. > > > > How did you not get a conflict with that definition also in the kernel > > header? > > It's an identical definition. That's OK. Changes to a definition is last one > wins - but you get a warning not an error. Do any distros compile with -Werror? > -Steve - 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 -- Linux-audit mailing list Linux-audit@redhat.com https://www.redhat.com/mailman/listinfo/linux-audit
Re: [RFC PATCH ghak89 V1] audit: rename FILTER_TYPE to FILTER_EXCL
On Friday, June 1, 2018 1:58:34 PM EDT Richard Guy Briggs wrote: > On 2018-06-01 12:55, Steve Grubb wrote: > > On Thursday, May 31, 2018 6:21:20 PM EDT Richard Guy Briggs wrote: > > > On 2018-05-31 17:29, Steve Grubb wrote: > > > > On Thursday, May 31, 2018 4:23:09 PM EDT Richard Guy Briggs wrote: > > > > > 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. > > > > > > > > Historically speaking, this is not why it is the way it is. But I > > > > think > > > > it > > > > doesn't mean that you cannot do something like this: > > > > > > > > #define AUDIT_FILTER_EXCLUDEAUDIT_FILTER_TYPE > > > > > > I was originally hoping to do that, but that then causes a build error > > > on any previous version of audit userspace. > > > > I cannot reproduce this. What error did you get? What version of gcc? > > I didn't even try to compile it since I'd predicted that there would be > a symbol definition conflict. > > How did you not get a conflict with that definition also in the kernel > header? It's an identical definition. That's OK. Changes to a definition is last one wins - but you get a warning not an error. -Steve -- Linux-audit mailing list Linux-audit@redhat.com https://www.redhat.com/mailman/listinfo/linux-audit
Re: [RFC PATCH ghak89 V1] audit: rename FILTER_TYPE to FILTER_EXCL
On 2018-06-01 12:55, Steve Grubb wrote: > On Thursday, May 31, 2018 6:21:20 PM EDT Richard Guy Briggs wrote: > > On 2018-05-31 17:29, Steve Grubb wrote: > > > On Thursday, May 31, 2018 4:23:09 PM EDT Richard Guy Briggs wrote: > > > > 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. > > > > > > Historically speaking, this is not why it is the way it is. But I think > > > it > > > doesn't mean that you cannot do something like this: > > > > > > #define AUDIT_FILTER_EXCLUDEAUDIT_FILTER_TYPE > > > > I was originally hoping to do that, but that then causes a build error > > on any previous version of audit userspace. > > I cannot reproduce this. What error did you get? What version of gcc? I didn't even try to compile it since I'd predicted that there would be a symbol definition conflict. How did you not get a conflict with that definition also in the kernel header? > -Steve - 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 -- Linux-audit mailing list Linux-audit@redhat.com https://www.redhat.com/mailman/listinfo/linux-audit
Re: [RFC PATCH ghak89 V1] audit: rename FILTER_TYPE to FILTER_EXCL
On Thursday, May 31, 2018 6:21:20 PM EDT Richard Guy Briggs wrote: > On 2018-05-31 17:29, Steve Grubb wrote: > > On Thursday, May 31, 2018 4:23:09 PM EDT Richard Guy Briggs wrote: > > > 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. > > > > Historically speaking, this is not why it is the way it is. But I think > > it > > doesn't mean that you cannot do something like this: > > > > #define AUDIT_FILTER_EXCLUDEAUDIT_FILTER_TYPE > > I was originally hoping to do that, but that then causes a build error > on any previous version of audit userspace. I cannot reproduce this. What error did you get? What version of gcc? Thanks, -Steve -- 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-05-31 22:52 GMT+02:00 Richard Guy Briggs :
> On 2018-05-30 10:45, Ondrej Mosnacek wrote:
>> This patch allows the AUDIR_DIR field to be used also with the exclude
>> filter.
>>
>> Not-yet-signed-off-by: Ondrej Mosnacek
>> ---
>> kernel/audit.c | 5 +++--
>> kernel/audit.h | 32 +++-
>> kernel/audit_tree.c | 4 +++-
>> kernel/auditfilter.c | 6 +-
>> kernel/auditsc.c | 28
>> 5 files changed, 42 insertions(+), 33 deletions(-)
>>
>> diff --git a/kernel/audit.c b/kernel/audit.c
>> index e7478cb58079..aac5b6ecc11d 100644
>> --- a/kernel/audit.c
>> +++ b/kernel/audit.c
>> @@ -1333,7 +1333,8 @@ static int audit_receive_msg(struct sk_buff *skb,
>> struct nlmsghdr *nlh)
>> if (!audit_enabled && msg_type != AUDIT_USER_AVC)
>> return 0;
>>
>> - err = audit_filter(msg_type, AUDIT_FILTER_USER);
>> + // FIXME: do we need to pass the context here?
>> + err = audit_filter(msg_type, AUDIT_FILTER_USER, NULL);
>> if (err == 1) { /* match or error */
>> err = 0;
>> if (msg_type == AUDIT_USER_TTY) {
>> @@ -1754,7 +1755,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_TYPE, ctx)))
>> return NULL;
>>
>> /* NOTE: don't ever fail/sleep on these two conditions:
>> diff --git a/kernel/audit.h b/kernel/audit.h
>> index 214e14948370..43cfeba25802 100644
>> --- a/kernel/audit.h
>> +++ b/kernel/audit.h
>> @@ -324,13 +324,43 @@ extern void audit_kill_trees(struct list_head *list);
>> #define audit_kill_trees(list) BUG()
>> #endif
>>
>> +struct audit_tree_refs {
>> + struct audit_tree_refs *next;
>> + struct audit_chunk *c[31];
>> +};
>> +
>> +/* A utility function to match tree refs: */
>> +static inline int match_tree_refs(struct audit_context *ctx, struct
>> audit_tree *tree)
>> +{
>> +#ifdef CONFIG_AUDIT_TREE
>> + struct audit_tree_refs *p;
>> + int n;
>> + if (!tree)
>> + return 0;
>> + /* full ones */
>> + for (p = ctx->first_trees; p != ctx->trees; p = p->next) {
>> + for (n = 0; n < 31; n++)
>> + if (audit_tree_match(p->c[n], tree))
>> + return 1;
>> + }
>> + /* partial */
>> + if (p) {
>> + for (n = ctx->tree_count; n < 31; n++)
>> + if (audit_tree_match(p->c[n], tree))
>> + return 1;
>> + }
>> +#endif
>> + return 0;
>> +}
>> +
>> extern char *audit_unpack_string(void **bufp, size_t *remain, size_t len);
>>
>> extern pid_t audit_sig_pid;
>> extern kuid_t audit_sig_uid;
>> extern u32 audit_sig_sid;
>>
>> -extern int audit_filter(int msgtype, unsigned int listtype);
>> +extern int audit_filter(int msgtype, unsigned int listtype,
>> + struct audit_context *ctx);
>>
>> #ifdef CONFIG_AUDITSYSCALL
>> extern int audit_signal_info(int sig, struct task_struct *t);
>> diff --git a/kernel/audit_tree.c b/kernel/audit_tree.c
>> index 67e6956c0b61..d4d36389c3d7 100644
>> --- a/kernel/audit_tree.c
>> +++ b/kernel/audit_tree.c
>> @@ -675,9 +675,11 @@ void audit_trim_trees(void)
>>
>> int audit_make_tree(struct audit_krule *rule, char *pathname, u32 op)
>> {
>> + if (krule->listnr != AUDIT_FILTER_EXIT &&
>> + krule->listnr != AUDIT_FILTER_TYPE)
>> + return -EINVAL;
>>
>> if (pathname[0] != '/' ||
>> - rule->listnr != AUDIT_FILTER_EXIT ||
>> op != Audit_equal ||
>> rule->inode_f || rule->watch || rule->tree)
>> return -EINVAL;
>> diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
>> index 6db9847ca031..e1d70cb77ea3 100644
>> --- a/kernel/auditfilter.c
>> +++ b/kernel/auditfilter.c
>> @@ -1309,7 +1309,7 @@ int audit_compare_dname_path(const char *dname, const
>> char *path, int parentlen)
>> return strncmp(p, dname, dlen);
>> }
>>
>> -int audit_filter(int msgtype, unsigned int listtype)
>> +int audit_filter(int msgtype, unsigned int listtype, struct audit_context
>> *ctx)
>> {
>> struct audit_entry *e;
>> int ret = 1; /* Audit by default */
>> @@ -1363,6 +1363,10 @@ int audit_filter(int msgtype, unsigned int listtype)
>> if (f->op == Audit_not_equal)
>> result = !result;
>> break;
>> + case AUDIT_DIR:
>> + if (ctx)
>> + result = match_tree_refs(ctx,
>> e->rule.tree);
>
> I don't see why you need to send a context to audit_filter, since the
> rest of the function assumes the cur
