Re: [Bro-Dev] Config Framework Feedback

2018-11-01 Thread Christian Kreibich
On 11/1/18 7:43 AM, Robin Sommer wrote:
> The oberservations / thoughts in this thread seem worth a ticket I'd
> say. We can refine this over time if the current semantics aren't
> quite ideal yet.

Okay Robin, I've created https://github.com/bro/bro/issues/201 for this.

Thanks,
-C.
___
bro-dev mailing list
bro-dev@bro.org
http://mailman.icsi.berkeley.edu/mailman/listinfo/bro-dev


Re: [Bro-Dev] Config Framework Feedback

2018-11-01 Thread Robin Sommer
The oberservations / thoughts in this thread seem worth a ticket I'd
say. We can refine this over time if the current semantics aren't
quite ideal yet.

Robin

On Tue, Oct 30, 2018 at 13:17 -0700, Christian Kreibich wrote:

> Hi folks,
> 
> I would agree that it takes a bit of experimentation to figure out 
> exactly when a change handler fires and how to reliably initialize or 
> update things based on an option's value.
> 
> Consider this:
> 
>module Foo;
> 
>export { option foo = F; }
> 
>function foo_handler(ID: string, foo_new: bool): bool
>{
>print fmt("New foo: %s", foo_new);
> 
># Update stuff here based on foo's value
># ...
> 
>return foo_new;
>}
> 
>event bro_init() {
>Option::set_change_handler("Foo::foo", foo_handler);
>}
> 
> ... foo_handler doesn't get called when you simply run the script 
> without redefing Config::config_files. When you do redef it, the handler 
> fires both when the config file sets foo to T, and when it sets it to F.
> 
> So you have to make sure that your initialization happens even when the 
> handler doesn't get called, and you cannot write your handler assuming 
> that the new value is actually different from the old one.
> 
> These arguably aren't bugs, but imo they do take getting used to.
> 
> Best,
> -C.
> ___
> bro-dev mailing list
> bro-dev@bro.org
> http://mailman.icsi.berkeley.edu/mailman/listinfo/bro-dev


-- 
Robin Sommer * Corelight, Inc. * ro...@corelight.com * www.corelight.com
___
bro-dev mailing list
bro-dev@bro.org
http://mailman.icsi.berkeley.edu/mailman/listinfo/bro-dev


Re: [Bro-Dev] Config Framework Feedback

2018-10-30 Thread Christian Kreibich
Hi folks,

I would agree that it takes a bit of experimentation to figure out 
exactly when a change handler fires and how to reliably initialize or 
update things based on an option's value.

Consider this:

   module Foo;

   export { option foo = F; }

   function foo_handler(ID: string, foo_new: bool): bool
   {
   print fmt("New foo: %s", foo_new);

   # Update stuff here based on foo's value
   # ...

   return foo_new;
   }

   event bro_init() {
   Option::set_change_handler("Foo::foo", foo_handler);
   }

... foo_handler doesn't get called when you simply run the script 
without redefing Config::config_files. When you do redef it, the handler 
fires both when the config file sets foo to T, and when it sets it to F.

So you have to make sure that your initialization happens even when the 
handler doesn't get called, and you cannot write your handler assuming 
that the new value is actually different from the old one.

These arguably aren't bugs, but imo they do take getting used to.

Best,
-C.
___
bro-dev mailing list
bro-dev@bro.org
http://mailman.icsi.berkeley.edu/mailman/listinfo/bro-dev


Re: [Bro-Dev] Config Framework Feedback

2018-10-30 Thread Hosom, Stephen M
I agree that more complex code on my side would definitely make this a 
non-issue.

In my case, I was setting up something in bro_init that would be used 
repeatedly throughout execution, and you're right--my code never handled option 
updates.


In this case, I opted to just use old redefable consts, as they will perform 
exactly as expected for any downstream users (changes require a bro restart).


Honestly, I only brought this up at all because I can definitely see a person 
new to scripting wondering why their options do not have the expected values at 
bro_init time and troubleshooting for hours.


Maybe it would be worthwhile to just add a note in the documentation for the 
config framework about it and leave it at that?


From: Azoff, Justin S 
Sent: Tuesday, October 30, 2018 3:41:00 PM
To: Hosom, Stephen M
Cc: bro-dev@bro.org
Subject: Re: [Bro-Dev] Config Framework Feedback

Message received from outside the Battelle network. Carefully examine it before 
you open any links or attachments.


> On Oct 30, 2018, at 2:09 PM, Hosom, Stephen M  wrote:
>
> I bumped into a limitation that was a little frustrating to work around with 
> the config framework.
>
>
> It is inconvenient that values stored in files read by adding to 
> Config::config_files are not available within the bro_init event. After 
> reviewing how the Config framework works, I understand why this is the case, 
> but it means that if you want to use configuration values on startup, you're 
> not guaranteed to be working with the anticipated value.
>
>
> I think the introduction of an event that ensures that configuration files 
> have been read by the time that the event fires might be worthwhile. I was 
> wondering if anyone had any thoughts on how to resolve/work-around this issue.

This is an issue, but probably not the one you are thinking of.  even if 
configuration files were fully read by the time bro_init was ran, you'd still 
need to handle the value changing at runtime.

If you need to run code when an option changes, you can use the 
Option::set_change_handler to make bro raise an event.  Using this instead of 
bro_init would fix the startup problem, and handle it changing at any time.

There's a test that uses this in testing/btest/core/option-priorities.bro and a 
few other files.


—
Justin Azoff



___
bro-dev mailing list
bro-dev@bro.org
http://mailman.icsi.berkeley.edu/mailman/listinfo/bro-dev


Re: [Bro-Dev] Config Framework Feedback

2018-10-30 Thread Azoff, Justin S

> On Oct 30, 2018, at 2:09 PM, Hosom, Stephen M  wrote:
> 
> I bumped into a limitation that was a little frustrating to work around with 
> the config framework.
> 
> 
> It is inconvenient that values stored in files read by adding to 
> Config::config_files are not available within the bro_init event. After 
> reviewing how the Config framework works, I understand why this is the case, 
> but it means that if you want to use configuration values on startup, you're 
> not guaranteed to be working with the anticipated value.
> 
> 
> I think the introduction of an event that ensures that configuration files 
> have been read by the time that the event fires might be worthwhile. I was 
> wondering if anyone had any thoughts on how to resolve/work-around this issue.

This is an issue, but probably not the one you are thinking of.  even if 
configuration files were fully read by the time bro_init was ran, you'd still 
need to handle the value changing at runtime.

If you need to run code when an option changes, you can use the 
Option::set_change_handler to make bro raise an event.  Using this instead of 
bro_init would fix the startup problem, and handle it changing at any time.

There's a test that uses this in testing/btest/core/option-priorities.bro and a 
few other files.


— 
Justin Azoff



___
bro-dev mailing list
bro-dev@bro.org
http://mailman.icsi.berkeley.edu/mailman/listinfo/bro-dev


[Bro-Dev] Config Framework Feedback

2018-10-30 Thread Hosom, Stephen M
I bumped into a limitation that was a little frustrating to work around with 
the config framework.


It is inconvenient that values stored in files read by adding to 
Config::config_files are not available within the bro_init event. After 
reviewing how the Config framework works, I understand why this is the case, 
but it means that if you want to use configuration values on startup, you're 
not guaranteed to be working with the anticipated value.


I think the introduction of an event that ensures that configuration files have 
been read by the time that the event fires might be worthwhile. I was wondering 
if anyone had any thoughts on how to resolve/work-around this issue.


known.dat:


Known::KnownServers 10.230.21.220,10.230.21.221

try-config.bro:


module Known;

redef Config::config_files += {"Known.dat"};

export {
option KnownServers: set[addr] = set();
}

event bro_init() {
print KnownServers;
}

event bro_done() {
print KnownServers;
}


=== output ===


bro -r stream-1.pcap ./try-config.bro

{


}

{

10.230.21.220,

10.230.21.221

}


Thanks,

Stephen

___
bro-dev mailing list
bro-dev@bro.org
http://mailman.icsi.berkeley.edu/mailman/listinfo/bro-dev