No subject

aud_{delete,get,put}_{event,hdr,subj,obj}: The use of optional records is
poorly defined.  I used the index essentially as a counter into a list of
records, which seems consistent with the treatment of the first record,
and offhand comments about other records.  Is that what the authors had in
mind?  When _put_ and _get_ are interleaved, how should _NEXT_ITEM and 
_FIRST_ITEM, etc, behave?  (I did most of the obvious things)

aud_get_{event,hdr,subj,obj}_info: the sequential access is not very well
defined, as well as the ordering behavior.  I added an additional field to
the aud_info_t struct to allow a retrieval by AUD_NEXT_ITEM to return the
item_id, allowing iteration on unknown fields in a useful way.  This also
makes the implementation of aud_dup_record() easier as it can just wrap an
interation through each record for copying purposes.  It also wasn't clear
to me who owned the data pointed to by items retrieved by aud_get_*_info:
can that just be a copy of a pointer to an internal structure?  Should the
draft specify that it is a local copy or a shared copy?

aud_{read,write}: the ability to implement non-blocking behavior seems to
effectively require atomic reading of a complete record, or maintaining
state that is hard to deal with.  At least, if select is to be used to
tell when a record is ready on the descriptor if it is to a regular file.
I ended up just ignoring the non-blocking case because I could not really
think of a useful way for it to work.  Aud_write is also supposed to fix
up records missing items that are required, where possible--presumably
while maintaining the ordering requirements described in aud_put_hdr_info?

aud_free: I am not sure I like that it requires me to keep track of all
the pointers I hand out, and their types :-).

aud_get_id (etc): that convert audit ids to string form.  A obvious
implementation is to use getpwnam and getpwid to map between usernames and
uid's.  However, those calls are not thread-safe in most implementations,
as they use static variables for returns.  

aud_copy_int: the size argument I mentioned previously would be nice to
bound the storage.  I implement a version with a size argument, and a
version without that calls the prior with an argument of 0 having it
ignore the size bound.  When dealing with potentially untrusted (or just
corrupted) records, this is probably a good idea.

In the aud_item_t definition, the binary blob pointer is intended to point
to an array of the type.  This works fine for all the types there except
for the string/string array.  The draft points out that strings should
just be null-terminated strings, and the pointer will be a char*.
However, it doesn't describe how to handle an AUD_TYPE_STRING_ARRAY.  If
the pointed to item includes pointers, then the byte data cannot just be
relocated, nor size bounded in advance through use of the length field of
the aud_info_t.  My temptation would be to just pack the strings
sequentially, with the length bound indicating where the end of the block
of data was.  However, that is not defined, and some of the important
interesting audit events (exec :-) require the use of string arrays.
Making them relocatable (i.e., no pointers inside the block) would be
good.

In general: thread safety is not mentioned; I can imagine a threaded
intrusion detection program listening on /dev/audit, ammong other things.
An explicit statement simply that mutual exclusion is required on access
to all audit records (on a per-aud_rec_t basis), and must be handled by
the caller?

Also in general: the subject and object components of an audit record seem
poorly defined--or at least, when to use them with regards to a
particular event isn't in the event-specific section.

> > Another lack of a feature is a standard flattened file format
> > for audit records.
> 
> Yup. The one proposal presented which could have addressed this
> included a truely reprehensible API. Rather than keep the good bits
> and chuck the bad, the whole thing was rejected and any attempts to
> bring back the good was squashed because of the ABI. 

Once I finish up my first run at an implementation (probably this
weekend), leaving aside the kernel hooks, I'll stick it up on a web page
if anyone wants to run through the format I've been using.  It's
essentially the obvious: a recursive marshalling of each structure and its
children, sequentially onto a byte stream.  Then a parsing out again
afterwards.

Although it took me a while to understand where the draft was going, it
actually isn't too badly off considering the description of its writing
that you have given :-).  All it needs to do is define a structure and
delivery mechanism, and then the details of the events.  And it does that
fairly well.  Post-processing is a seperate issue, but one that the API
need not define.  My only criticism is really that it is flexible enough
that performance is a concern :-).  A multi-dimmensional record for a
single kernel syscall is almost too much.  Fortunatelly most of the events
specified don't take advantage of the flexibility.

Assuming I can get permission to redistribute man pages based on the spec,
I'll stick them online also.

  Robert N Watson 

robert at fledge.watson.org              http://www.watson.org/~robert/
PGP key fingerprint: 03 01 DD 8E 15 67 48 73  25 6D 10 FC EC 68 C1 1C

Carnegie Mellon University            http://www.cmu.edu/
TIS Labs at Network Associates, Inc.  http://www.tis.com/
Safeport Network Services             http://www.safeport.com/

To Unsubscribe: send mail to majordomo at cyrus.watson.org
with "unsubscribe posix1e" in the body of the message