Re: [PATCH] Fix acct quoting in audit_log_acct_message())

[Tomas Mraz <tmraz@redhat.com>]

On Wed, 2008-03-05 at 14:55 +0100, Miloslav Trmac wrote:
> Tomas Mraz napsal(a):
> > This proposal is just for starting the discussion.
> > 
> > 1. Messages contain <name>=<value> pairs separated by spaces.
> > 2. All <names> are just alphanumeric sequences.
> > 3. Values can be either:
> >  a) byte sequences with the following special characters encoded as %XX
> > where XX is hexadecimal value of the encoded byte. Special characters
> > are: bytes with value <= 0x20 or >= 0x7F, '%', '(', ')', and '='.
> Perhaps we should reserve more characters for future features - at least
> '"', '\'' and '\\', maybe everything but [a-zA-Z0-9_-].
Too much reserved characters would mean higher inefficiency of encoding.
But you're right that some more could be useful.

>  From the previous thread - the currently used hexadecimal format is
> good for non-ASCII data (2 characters per byte instead of 3 bytes);  It
> probably won't be better for most messages - perhaps it should be left
> as a third alternative, e.g. \xaa55abcdef.
> 
> One more proposal:
> 4. If a value is undefined, the name=value pair is not present.  Special
>     values ("?", "(null)", "") are never used to represent unknown
>     field values.
That complicates the message generation. I'd prefer just defining one
special null value for all fields and stick to it. The special value
would have the same meaning as if the name=value pair was missing
completely.

> >  b) recursively embedded messages enclosed in '(' and ')' parentheses.
> 
> > type=USER_START msg=audit(1204632061.112:32361): user pid=10902 uid=0
> > auid=0 subj=system_u:system_r:crond_t:s0-s0:c0.c1023
> > msg='op=PAM:session_open acct=root exe="/usr/sbin/crond" (hostname=?,
> > addr=?, terminal=cron res=success)'
> > 
> > becomes:
> > 
> > type=USER_START msg=(audit=1204632061.112:3236 src=user pid=10902 uid=0
> > auid=0 subj=system_u:system_r:crond_t:s0-s0:c0.c1023
> > msg=(op=PAM:session_open acct=root exe=/usr/sbin/crond hostname=? addr=?
> > terminal=cron res=success))
> [Should there be only one trailing )? ]  Using "msg" for both the kernel
> and user-space part is ambiguous - perhaps "kmsg"/"umsg" or just
> "k"/"u"?  Or, preferably, don't nest the kernel fields at all - the
> nesting carries no information.
The nesting might be again useful for easy generation of messages, but
sometimes pure concatenation might be enough.

> > type=AVC msg=audit(1204601533.621:32307): avc:  denied  { read write }
> > for  pid=9822 comm="tmpwatch" path="socket:[14038]" dev=sockfs ino=14038
> > scontext=system_u:system_r:tmpreaper_t:s0-s0:c0.c1023
> > tcontext=system_u:system_r:crond_t:s0-s0:c0.c1023 tclass=tcp_socket
> > 
> > becomes:
> > 
> > type=AVC msg=(audit=1204601533.621:32307 src=avc kind=denied
> > acts=read:write pid=9822 comm=tmpwatch path=socket:[14038] dev=sockfs
> > ino=14038 scontext=system_u:system_r:tmpreaper_t:s0-s0:c0.c1023
> > tcontext=system_u:system_r:crond_t:s0-s0:c0.c1023 tclass=tcp_socket)
> (auparse already defines names for some of the fields, the names should
> be reused.)
Yes.

But as Eric said the format of the AVC messages will not change. But
then it doesn't make much sense to me to change the format thoroughly.
Perhaps just changing the string enconding to be non ambiguous as John
Dennis proposed would be enough.
-- 
Tomas Mraz
No matter how far down the wrong road you've gone, turn back.
                                              Turkish proverb