What is the bug

What is the bug

[Burn Alting]

All,

Consider the following raw audit event ...

        node=fedora20.swtf.dyndns.org type=CONFIG_CHANGE
        msg=audit(1390028319.573:20803): auid=4294967295 ses=4294967295
        subj=system_u:system_r:auditctl_t:s0 op="remove rule"
        key="time-change" list=4 res=1

When the auparse library parses this event event, it does not correctly
parse the 'op' value and so both auparse_get_field_str() and
auparse_interpret_field() both return '"remove' rather than 'remove
rule'.

Now, I seem to recollect an earlier e-mail that would suggest the bug is
in kernel/auditfilter.c:audit_receive_filter() as it calls
audit_log_rule_change() with the string "add rule" or "remove rule". One
assumes we need to perhaps either
a. replace the space with a hyphen in these arguments, or
b. in kernel/auditfilter.c:audit_log_rule_change() replace the call
	audit_log_string(ab, action);
with
	audit_log_untrustedstring(ab, action); 

If this is the case, then is there any appetite to have these bugs fixed
on the next update to the kernel audit code?


Thanks in advance

Burn

Re: What is the bug

[Steve Grubb]

Hello,

On Sat, 18 Jan 2014 20:02:37 +1100
Burn Alting <burn@swtf.dyndns.org> wrote:
> Consider the following raw audit event ...
> 
>         node=fedora20.swtf.dyndns.org type=CONFIG_CHANGE
>         msg=audit(1390028319.573:20803): auid=4294967295
> ses=4294967295 subj=system_u:system_r:auditctl_t:s0 op="remove rule"
>         key="time-change" list=4 res=1
> 
> When the auparse library parses this event event, it does not
> correctly parse the 'op' value and so both auparse_get_field_str() and
> auparse_interpret_field() both return '"remove' rather than 'remove
> rule'.

Correct. I have pointed this out for years and no one has wanted to fix
it. The hex-encoding should only be used on fields that a user can
influence, like file names. Since op= is always filled in by actual
audit code - which is trusted, it should never _need_ encoding.
Anywhere there is an op= and the field has blanks in it, it should be
reformatted to have a dash between the words rather than a space. So,
you would have remove-rule in your example. Untrusted string should
never be used for this.

> Now, I seem to recollect an earlier e-mail that would suggest the bug
> is in kernel/auditfilter.c:audit_receive_filter() as it calls
> audit_log_rule_change() with the string "add rule" or "remove rule".
> One assumes we need to perhaps either
> a. replace the space with a hyphen in these arguments, or
> b. in kernel/auditfilter.c:audit_log_rule_change() replace the call
> 	audit_log_string(ab, action);
> with
> 	audit_log_untrustedstring(ab, action); 
> 
> If this is the case, then is there any appetite to have these bugs
> fixed on the next update to the kernel audit code?

Yes please. I have been wanting this fixed for years. Grep all the auit
code for this. I seem to recall problems in the ipsec and IMA code.

Thanks!

-Steve

Re: What is the bug

[Eric Paris]

What's the bugzilla?

On Sat, 2014-01-18 at 08:53 -0500, Steve Grubb wrote:
> Hello,
> 
> On Sat, 18 Jan 2014 20:02:37 +1100
> Burn Alting <burn@swtf.dyndns.org> wrote:
> > Consider the following raw audit event ...
> > 
> >         node=fedora20.swtf.dyndns.org type=CONFIG_CHANGE
> >         msg=audit(1390028319.573:20803): auid=4294967295
> > ses=4294967295 subj=system_u:system_r:auditctl_t:s0 op="remove rule"
> >         key="time-change" list=4 res=1
> > 
> > When the auparse library parses this event event, it does not
> > correctly parse the 'op' value and so both auparse_get_field_str() and
> > auparse_interpret_field() both return '"remove' rather than 'remove
> > rule'.
> 
> Correct. I have pointed this out for years and no one has wanted to fix
> it. The hex-encoding should only be used on fields that a user can
> influence, like file names. Since op= is always filled in by actual
> audit code - which is trusted, it should never _need_ encoding.
> Anywhere there is an op= and the field has blanks in it, it should be
> reformatted to have a dash between the words rather than a space. So,
> you would have remove-rule in your example. Untrusted string should
> never be used for this.
> 
> > Now, I seem to recollect an earlier e-mail that would suggest the bug
> > is in kernel/auditfilter.c:audit_receive_filter() as it calls
> > audit_log_rule_change() with the string "add rule" or "remove rule".
> > One assumes we need to perhaps either
> > a. replace the space with a hyphen in these arguments, or
> > b. in kernel/auditfilter.c:audit_log_rule_change() replace the call
> > 	audit_log_string(ab, action);
> > with
> > 	audit_log_untrustedstring(ab, action); 
> > 
> > If this is the case, then is there any appetite to have these bugs
> > fixed on the next update to the kernel audit code?
> 
> Yes please. I have been wanting this fixed for years. Grep all the auit
> code for this. I seem to recall problems in the ipsec and IMA code.
> 
> Thanks!
> 
> -Steve
> 
> --
> Linux-audit mailing list
> Linux-audit@redhat.com
> https://www.redhat.com/mailman/listinfo/linux-audit

Re: What is the bug

[Burn Alting]

Eric,

I haven't listed one yet, but I will in the next day or two. I will
identify every string and file involved before doing so. I would only
ask, what kernel version should I use as my reference?

Rgds

On Mon, 2014-01-20 at 15:25 -0500, Eric Paris wrote:
> What's the bugzilla?
> 
> On Sat, 2014-01-18 at 08:53 -0500, Steve Grubb wrote:
> > Hello,
> > 
> > On Sat, 18 Jan 2014 20:02:37 +1100
> > Burn Alting <burn@swtf.dyndns.org> wrote:
> > > Consider the following raw audit event ...
> > > 
> > >         node=fedora20.swtf.dyndns.org type=CONFIG_CHANGE
> > >         msg=audit(1390028319.573:20803): auid=4294967295
> > > ses=4294967295 subj=system_u:system_r:auditctl_t:s0 op="remove rule"
> > >         key="time-change" list=4 res=1
> > > 
> > > When the auparse library parses this event event, it does not
> > > correctly parse the 'op' value and so both auparse_get_field_str() and
> > > auparse_interpret_field() both return '"remove' rather than 'remove
> > > rule'.
> > 
> > Correct. I have pointed this out for years and no one has wanted to fix
> > it. The hex-encoding should only be used on fields that a user can
> > influence, like file names. Since op= is always filled in by actual
> > audit code - which is trusted, it should never _need_ encoding.
> > Anywhere there is an op= and the field has blanks in it, it should be
> > reformatted to have a dash between the words rather than a space. So,
> > you would have remove-rule in your example. Untrusted string should
> > never be used for this.
> > 
> > > Now, I seem to recollect an earlier e-mail that would suggest the bug
> > > is in kernel/auditfilter.c:audit_receive_filter() as it calls
> > > audit_log_rule_change() with the string "add rule" or "remove rule".
> > > One assumes we need to perhaps either
> > > a. replace the space with a hyphen in these arguments, or
> > > b. in kernel/auditfilter.c:audit_log_rule_change() replace the call
> > > 	audit_log_string(ab, action);
> > > with
> > > 	audit_log_untrustedstring(ab, action); 
> > > 
> > > If this is the case, then is there any appetite to have these bugs
> > > fixed on the next update to the kernel audit code?
> > 
> > Yes please. I have been wanting this fixed for years. Grep all the auit
> > code for this. I seem to recall problems in the ipsec and IMA code.
> > 
> > Thanks!
> > 
> > -Steve
> > 
> > --
> > Linux-audit mailing list
> > Linux-audit@redhat.com
> > https://www.redhat.com/mailman/listinfo/linux-audit
> 
> 

Re: What is the bug

[Eric Paris]

I didn't mean to make that statement at you, it was in reference to:

> On Sat, 2014-01-18 at 08:53 -0500, Steve Grubb wrote:
> > > I have pointed this out for years and no one has wanted to fix
> > > it.

Please though, do file a bug, as if you don't, I think you can reset
assured, it will be forgotten 'for years'.  If you a RHEL customer,
please go through support.  If not, you can file against a recent
upstream kernel.

-Eric

On Tue, 2014-01-21 at 08:05 +1100, Burn Alting wrote:
> Eric,
> 
> I haven't listed one yet, but I will in the next day or two. I will
> identify every string and file involved before doing so. I would only
> ask, what kernel version should I use as my reference?
> 
> Rgds
> 
> On Mon, 2014-01-20 at 15:25 -0500, Eric Paris wrote:
> > What's the bugzilla?
> > 
> > On Sat, 2014-01-18 at 08:53 -0500, Steve Grubb wrote:
> > > Hello,
> > > 
> > > On Sat, 18 Jan 2014 20:02:37 +1100
> > > Burn Alting <burn@swtf.dyndns.org> wrote:
> > > > Consider the following raw audit event ...
> > > > 
> > > >         node=fedora20.swtf.dyndns.org type=CONFIG_CHANGE
> > > >         msg=audit(1390028319.573:20803): auid=4294967295
> > > > ses=4294967295 subj=system_u:system_r:auditctl_t:s0 op="remove rule"
> > > >         key="time-change" list=4 res=1
> > > > 
> > > > When the auparse library parses this event event, it does not
> > > > correctly parse the 'op' value and so both auparse_get_field_str() and
> > > > auparse_interpret_field() both return '"remove' rather than 'remove
> > > > rule'.
> > > 
> > > Correct. I have pointed this out for years and no one has wanted to fix
> > > it. The hex-encoding should only be used on fields that a user can
> > > influence, like file names. Since op= is always filled in by actual
> > > audit code - which is trusted, it should never _need_ encoding.
> > > Anywhere there is an op= and the field has blanks in it, it should be
> > > reformatted to have a dash between the words rather than a space. So,
> > > you would have remove-rule in your example. Untrusted string should
> > > never be used for this.
> > > 
> > > > Now, I seem to recollect an earlier e-mail that would suggest the bug
> > > > is in kernel/auditfilter.c:audit_receive_filter() as it calls
> > > > audit_log_rule_change() with the string "add rule" or "remove rule".
> > > > One assumes we need to perhaps either
> > > > a. replace the space with a hyphen in these arguments, or
> > > > b. in kernel/auditfilter.c:audit_log_rule_change() replace the call
> > > > 	audit_log_string(ab, action);
> > > > with
> > > > 	audit_log_untrustedstring(ab, action); 
> > > > 
> > > > If this is the case, then is there any appetite to have these bugs
> > > > fixed on the next update to the kernel audit code?
> > > 
> > > Yes please. I have been wanting this fixed for years. Grep all the auit
> > > code for this. I seem to recall problems in the ipsec and IMA code.
> > > 
> > > Thanks!
> > > 
> > > -Steve
> > > 
> > > --
> > > Linux-audit mailing list
> > > Linux-audit@redhat.com
> > > https://www.redhat.com/mailman/listinfo/linux-audit
> > 
> > 
> 
>