On Sat, Nov 3, 2018 at 9:14 PM Vern Paxson <vern(a)corelight.com> wrote:
Thanks for the pointers & thoughts! A quick
question, more in a bit:
To better understand the existing behavior,
here's the commit that
introduced this (specifically with regards to conn_id):
https://github.com/bro/bro/commit/38a1aa5a346d10de32f9b40e0869cdb48a98974b
...
> Note that for nested record types, the inner fields must likewise
> be declared with &log. Consequently, conn_id is now declared with
> &log in bro.init.
Does your understanding of this accord with the current behavior when
running on testing/btest/scripts/base/frameworks/logging/attr.bro ?
The test suite result has it not logging Log$id, even though it's of
type conn_id, which has &log. (For my new version, it does log it.)
Hmm. I had to think about this for a bit, and I think it does agree with
the commit message. It's rather subtle, but because the message talks about
how the fields "must likewise be declared with &log," I can see how the
expectation would be that *both* the conn_id declaration in init-bare and
the usage in your record need to have the &log keyword to be logged.
However, before reading that commit message, that was not my expectation
for how Bro would behave.
I've been playing around with this a bit more, and I think that what's
described in the commit message is not the current behavior. Specifically,
the following seem to behave the same:
type conn_id: record {
orig_h: addr;
orig_p: port;
resp_h: addr;
resp_p: port;
} &log;
type conn_id: record {
orig_h: addr &log;
orig_p: port &log;
resp_h: addr &log;
resp_p: port &log;
};
This example demonstrates that all fields are still logged:
http://try.bro.org/#/trybro/saved/275829
In my mind, if the keyword is applied to a record, I would expect any new
fields added to that record to also be logged. However, if I use conn_id as
defined in init-bare (with &log applied to the record), and I redef conn_id
as follows, it will not log the new field:
redef record conn_id += {
nolog: bool &optional;
}
I believe that applying &log to a record is just shorthand to applying it
individually to all fields on that record, whenever you define or redef
that record.
Simply put, I think the current behavior is not correct, and that we should
take this opportunity to determine what the behavior *should* be.
--Vlad