Skip to contentSkip to conversation

Fix acct quoting in audit_log_acct_message())

PATCH Miloslav Trmac 4 participants 5 messages Mar 5, 2008

lore
Share & tools
lore
#0 lore raw

Tomas Mraz napsal(a):

Quoted 8 lines

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_-].

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.

Quoted 7 lines

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)'

Quoted 6 lines

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.

Quoted 11 lines

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.)
Mirek

#1 lore raw
Quoted 11 lines · Miloslav Trmac

Tomas Mraz napsal(a):

Quoted 8 lines

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.

Quoted 9 lines

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.

Quoted 9 lines
Quoted 9 lines

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:

Quoted 8 lines
Quoted 4 lines

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.

Quoted 5 lines
Quoted 5 lines

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

Quoted 8 lines
Quoted 6 lines

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.

Signature · 3 lines
Tomas Mraz
No matter how far down the wrong road you've gone, turn back.
                                              Turkish proverb
#2 lore raw
Quoted 5 lines
Quoted 5 lines
Quoted 3 lines

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_-].

Lets not invent YAES (Yet Another Encoding System). The world already has enough :-) There is value in sticking with known encodings, many programmers are instantly familiar with them and there is a raft of working code to support them. Off the top of my head I can think of:

1) backslash escapes with embedded octals
2) quoted printable
3) base64
4) xml entities

I think log file readability is high on the list of desired properties, that's why I would pick #1. If you're a sys-admin or a programmer you can read it in your sleep for virtually every escaped string likely to appear in audit data.

Quoted 4 lines

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.

Yes, I'm happy to just fix the encoding at this point, everything else is just icing on the cake and not necessary. If any changes other than fixing string encoding is going to imped acceptance then let's recognize reality and move on. The other things would be nice, but aren't needed. The string encoding has got to get fixed though.

Signature · 1 line
John Dennis <[email protected]>
#3 lore raw
Quoted 11 lines · John Dennis
Quoted 5 lines
Quoted 5 lines
Quoted 3 lines

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_-].

Lets not invent YAES (Yet Another Encoding System). The world already has enough :-) There is value in sticking with known encodings, many programmers are instantly familiar with them and there is a raft of working code to support them. Off the top of my head I can think of:

Quoted 4 lines

1) backslash escapes with embedded octals
2) quoted printable
3) base64
4) xml entities

5) url encoding - which is what I proposed and is most efficient and readable (well it is equivalent to quoted printable except it uses % instead of = which we cannot use as it has already a meaning as name value separator)

Signature · 3 lines
Tomas Mraz
No matter how far down the wrong road you've gone, turn back.
                                              Turkish proverb
#4 lore raw
Quoted 10 lines · On Wednesday 05 March 2008 10:21:29 Tomas Mraz wrote:
Quoted 9 lines

Lets not invent YAES (Yet Another Encoding System). The world already has enough :-) There is value in sticking with known encodings, many programmers are instantly familiar with them and there is a raft of working code to support them. Off the top of my head I can think of:

1) backslash escapes with embedded octals
2) quoted printable
3) base64
4) xml entities

Quoted 4 lines

5) url encoding - which is what I proposed and is most efficient and readable (well it is equivalent to quoted printable except it uses % instead of = which we cannot use as it has already a meaning as name value separator)

And if base64 is being suggested, why not:

6) ASCII encoded hex. Its compact, saves diskspace, easily encodes any utf-8 or ASCII string including punctuation and whitespace.

-Steve