changing Notice::policy mechanism

I think the construct we need to solve the problem already exists purely in the script-layer:

    type TriggerFunc: function(n: Notice::Info): bool;
    global Notice::policy: vector of set[TriggerFunc];

That doesn't have the attribute that I think we likely want, namely an
explicit "switch" on the particular notice type.

Sepaking of which, maybe we don't want an exact split-case-by-case but
rather something a bit broader. Here I'm thinking of policies that I'd
like to apply to any Notice of n different types; it's important to not
have to replicate that code, but instead just list the n types and the
associated predicate.

    Vern

I think the construct we need to solve the problem already exists purely in the script-layer:

   type TriggerFunc: function(n: Notice::Info): bool;
   global Notice::policy: vector of set[TriggerFunc];

That doesn't have the attribute that I think we likely want, namely an
explicit "switch" on the particular notice type.

Right, your switch/case idea reminded me of that optimization. Would a table work for that? So revising the example:

    type TriggerType: enum;
    type TriggerFunc: function(n: Notice::Info): bool;
    type PolicyHandlers: vector of set[TriggerFunc];
    global Notice::policy: table[TriggerType] of PolicyHandlers &redef;

Again, the issue probably being the cumbersome syntax of manipulation/redefinitions (if it's even possible right now) that's expected to be done by the user.

Sepaking of which, maybe we don't want an exact split-case-by-case but
rather something a bit broader. Here I'm thinking of policies that I'd
like to apply to any Notice of n different types; it's important to not
have to replicate that code, but instead just list the n types and the
associated predicate.

Think the above example supports that if the common code is just implemented as a TriggerFunc and then inserted in to the PolicyHandlers for the desired TriggerTypes.

    Jon

This is getting at the heart of what I think I want. Being able to define a notice type or notice types and apply an action is the end result, but how you implement the support for that in the script layer is currently too complicated. Optimally, I think it would be nice if people never had to use the action notice policy and that was why I wrote those "cheater" variables… Notice::ignored_types, Notice::emailed_types, Notice::alarmed_types, etc. The problem is, is that I think that no matter what "cheater" variables we create people will always need to be more expressive (that's been my experience with the few variables I already created).

What I'm pressing for is that make that next step more obvious because it would be a familiar Bro experience. Also, the implementation of that support in the script layer would be a lot clearer. Right now, there is a lot of code just to support the policy mechanism in the notice framework.

  .Seth

Here's an idea that combines a lot of the previous discussion -- how about we implement the capabilities of a "switch" statement, except we use a new type of function to do it. So there would then be function types designated by keywords "function", "event", and "switch". The "switch" would be like a "function" in that it can be called synchronously and doesn't have to be scheduled like an "event", but it's also like an "event" in that you can implement multiple bodies for the same "switch" identifier and designate &priority for each. Additionally, a new attribute called &case can be used with "switch" functions, and the argument to it is evaluated each time the function is called to determine whether or not to actually execute a given body. Also, if the body of a "switch" function returns as a result of a "break" statement, then that aborts the execution of the "switch" function and no more bodies get processed.

So an example of how it works in code form would be...

type TriggerType: enum {
    Good,
    Bad,
    Ugly,
};

type MyInfo: record {
    trigger: TriggerType;
};

# this declaration of the switch function would be optional (same as how functions work now)
global trigger: switch(n: MyInfo);

switch trigger(n: MyInfo) &case=(n$trigger==Bad) &priority=-5
    {
    this_stuff_never_happens();
    }

switch trigger(n: MyInfo) &case=(n$trigger==Bad) &priority=10
    {
    do_stuff();
    break;
    }

switch trigger(n: MyInfo) &case=(n$trigger==Ugly) &priority=1
    {
    do_other_stuff();
    }

event bro_init()
    {
    local rec: MyInfo = [$a=13, $trigger=Bad]);
    trigger(rec);
    # rec can be inspected here for meaningful modifications
    }

That should give everything we want:

(1) case expressions allow conditional application of logic
(2) logic can be prioritized
(3) logic can be short-circuited
(4) easy to understand how a user can refine/insert new logic for a priority/condition of their choosing
(5) its use should be generalizable and not specific to "policy" evaluation

One downside might be that it's not the typical "switch" people could be familiar with, but I don't think it's that confusing to take previous knowledge of how "switch" statements work when trying to understand how to use "switch" functions. I also don't think this would preclude the later implementation of "switch" in the usual statement form, it's just that implementing it using functions satisfies the "refinement" requirement more easily.

    Jon

Jon and I chatted about this for a while and I *really* don't like the &case attribute and I don't like the switch keyword much either. I think that making this switch-like is confusing because it's not really that much like a switch, it's more like a ladder of function bodies (I don't know of a comparison to make with a feature of another language). I think that the &case attribute would be confusing too because if you had larger conditions it would start to get unmanageable quickly and then you fall back to doing a block condition at the top of the code block which is already a pretty well established convention in Bro scripts, like this…

if ( n$note != Some_Notice )
  return;

Instead of using the "policy" keyword, how does everyone feel about "rule"? It would probably be confusing to people coming from Snort and Suricata, but I'm not overly concerned about that. To try and make my case, I got notice policies from a couple of sites. These are real world, actively used notice policies.

Current style at site 1…

redef Notice::policy += {
      [$action = Notice::ACTION_BLOCK,
       $pred(n: Notice::Info) = { return n$note == SSH::Password_Guessing && n?$src && !Site::is_local_addr(n$src) && !Site::is_private_addr(n$src) && !(n$src in external_ssh_scanners_whitelist || n$src in planetlab_hosts); } ]
};

redef Notice::policy += {
      [$action = Notice::ACTION_EMAIL,
       $pred(n: Notice::Info) = { return n$note == SSH::Password_Guessing && n?$src && ( Site::is_local_addr(n$src) || Site::is_private_addr(n$src) ) && !(n$src in internal_ssh_scanners_whitelist || n$src in planetlab_hosts); } ]
};

redef Notice::policy += {
      [$action = Notice::ACTION_EMAIL,
       $pred(n: Notice::Info) = { return n$note == HTTP::Malware_Hash_Registry_Match; } ]
};

redef Notice::policy += {
      [$action = Notice::ACTION_EMAIL,
       $pred(n: Notice::Info) = { return n$note == FTP::Site_Exec_Success; } ]
};

redef Notice::policy += {
      [$action = Notice::ACTION_EMAIL,
       $pred(n: Notice::Info) = { return n$note == SSH::Watched_Country_Login; } ]
};

redef Notice::policy += {
      [$action = Notice::ACTION_EMAIL, $suppress_for = 1 day,
       $pred(n: Notice::Info) = { return n$note == Software::Vulnerable_Java; } ]
};

New proposed style at site 1…

rule Notice::policy(n: Notice::Info)
  {
  if ( n$note == SSH::Password_Guessing &&
       n?$src &&
       !Site::is_local_addr(n$src) &&
       !Site::is_private_addr(n$src) &&
       !(n$src in external_ssh_scanners_whitelist || n$src in planetlab_hosts) )
    {
    add n$actions[Notice::ACTION_BLOCK];
    add n$actions[Notice::ACTION_EMAIL];
    }
  }

rule Notice::policy(n: Notice::Info)
  {
  if ( n$note == HTTP::Malware_Hash_Registry_Match )
    add n$actions[Notice::ACTION_EMAIL];

  else if ( n$note == FTP::Site_Exec_Success )
    add n$actions[Notice::ACTION_EMAIL];
  
  else if ( n$note == SSH::Watched_Country_Login )
    add n$actions[Notice::ACTION_EMAIL];

  else if ( n$note == Software::Vulnerable_Java )
    {
    n$suppress_for = 1day;
    add n$actions[Notice::ACTION_EMAIL];
    }
  }

Current style at site 2…

const dont_email_types: set[Notice::Type] = {
    SSL::Invalid_Server_Cert,
    SSH::Login,
    HTTP::MD5,
};

redef Notice::policy += {
        [$pred(n: Notice::Info) = { return (n$note in Notice::block_types); },
         $action = ACTION_BLOCK,
         $priority = 10],
};

redef Notice::policy += { [$action = Notice::ACTION_ALARM, $priority = 0,
                           $pred(n: Notice::Info) = { return (n$note !in dont_email_types);}
                          ] };
    
redef Notice::policy += { [$action = Notice::ACTION_LOG, $priority = 10, $halt=T,
                           $pred(n: Notice::Info) = { return (n$note == HTTP::SQL_Injection_Victim && !Site::is_local_addr(n$src));}
                          ] };

redef Notice::policy += { [$action = Notice::ACTION_LOG, $priority = 10, $halt=T,
                           $pred(n: Notice::Info) = { return (n$note == HTTP::SQL_Injection_Attacker && Site::is_local_addr(n$src));}
                          ] };

New proposed style at site 2...

const dont_email_types: set[Notice::Type] = {
    SSL::Invalid_Server_Cert,
    SSH::Login,
    HTTP::MD5,
};

redef Notice::emailed_types += {
    #DNS::EXTERNAL_FOREIGN_DNS,
    HTTP::IncorrectFileTypeBadHost,
    HTTP::SQL_Injection_Victim,
};

rule Notice::policy(n: Notice::Info) &priority=10
  {
  if ( n$note in Notice::ipblocker_types )
    add n$actions[ACTION_BLOCK];
  }

rule Notice::policy(n: Notice::Info) &priority=0
  {
  if ( n$note !in dont_email_types)
    {
    add n$actions[ACTION_ALARM];
    }
  }

rule Notice::policy(n: Notice::Info) &priority=10
  {
  if ( (n$note == HTTP::SQL_Injection_Victim && !Site::is_local_addr(n$src)) ||
       (n$note == HTTP::SQL_Injection_Attacker && Site::is_local_addr(n$src)) )
       {
       n$action = ACTION_LOG;
       return F;
       }
  return T;
  }

I think the new style is a lot clearer and easier to work with in both cases, but I assume this will still require more discussion. :slight_smile:

  .Seth

I still haven't fully caught up with this thread but I veto this
suggestion. :slight_smile: Rule is already overloaded (the Snort association, and
also Bro already has "rules" (indeed in the Snort sense).

Robin

Here's another variant. Don't know yet if I like it but we could make
use of type overloading for finding the right cases:

    with (n: Notice::Info)
    {
        if ( xxx )
            break; # short circuit
    }

    with (n: Notice::Info)
    {
        if ( yyy )
            do_someting();
    }

    [...]
    local n: Notice::Info;
    switch on n;
    [...]

"switch on" runs all "with"s that have a matching argument type.
Predicated need to be written explictly as if() stmts.

There might be nicer syntax than "with" and "switch on".

Robin

More variants…

It could be like synchronous signal handling with "raise" and "trap" keywords.

   trap trigger(n: Notice::Info) &priority=5
   {
   if ( xxx )
      break; # short circuit
   }

   trap trigger(n: Notice::Info) &priority=-5
   {
   if ( yyy )
      do_someting();
   }

   [...]
   local n: Notice::Info;
   raise trigger(n);
   […]

It could be like exception handling with "throw", and "catch" keywords.

   catch trigger(n: Notice::Info) &priority=5
   {
   if ( xxx )
      throw; # rethrow to continue handling
   }

   catch trigger(n: Notice::Info) &priority=-5
   {
   if ( yyy )
      do_someting();
   # handling stops here because it's not rethrown
   }

   [...]
   local n: Notice::Info;
   throw trigger(n);
   […]

    Jon

Oh, I kind of like this. To me it even makes the break-ing more obvious since there are a series of traps being executed. I've been thinking of these more and more like an easy to use hooking mechanism and I think raise and trap fit in with that.

Actually, what about just "hook"?

  hook trigger(n: Notice::Info) &priority=5
  {
  if ( xxx )
     break; # short circuit
  }

  hook trigger(n: Notice::Info) &priority=-5
  {
  if ( yyy )
     do_someting();
  }

  [...]
  local n: Notice::Info;
  hook trigger(n);
  […]

  .Seth

Now it looks like HILTI. :slight_smile:

Actually I like this best of all yet.

Robin

Is everyone that cares to speak up ok with "hook"?

Seems to make sense to me since I think what we're talking about here will be used as a way to synchronously hook into processing in various places (in a less syntactically verbose manner than the current approach in the notice framework). Notice and file analysis hooking is essentially trying to hook into those processing pipelines to change how they are dealt with.

  .Seth