Giter Site home page Giter Site logo

zeek / zeek Goto Github PK

View Code? Open in Web Editor NEW
5.9K 350.0 1.2K 157.69 MB

Zeek is a powerful network analysis framework that is much different from the typical IDS you may know.

Home Page: https://www.zeek.org

License: Other

Makefile 0.05% Shell 0.54% CMake 1.00% C++ 55.95% C 0.02% Python 0.12% Lex 0.37% Yacc 0.58% Zeek 41.18% Dockerfile 0.12% Batchfile 0.01% JavaScript 0.04% Go 0.01%
bro network-monitoring pcap security nsm dfir zeek

zeek's Introduction

Zeek Logo

The Zeek Network Security Monitor

A powerful framework for network traffic analysis and security monitoring.

Key FeaturesDocumentationGetting StartedDevelopmentLicense

Follow us on Twitter at @zeekurity.

Coverage Status Build Status

Slack Discourse

Key Features

  • In-depth Analysis Zeek ships with analyzers for many protocols, enabling high-level semantic analysis at the application layer.

  • Adaptable and Flexible Zeek's domain-specific scripting language enables site-specific monitoring policies and means that it is not restricted to any particular detection approach.

  • Efficient Zeek targets high-performance networks and is used operationally at a variety of large sites.

  • Highly Stateful Zeek keeps extensive application-layer state about the network it monitors and provides a high-level archive of a network's activity.

Getting Started

The best place to find information about getting started with Zeek is our web site www.zeek.org, specifically the documentation section there. On the web site you can also find downloads for stable releases, tutorials on getting Zeek set up, and many other useful resources.

You can find release notes in NEWS, and a complete record of all changes in CHANGES.

To work with the most recent code from the development branch of Zeek, clone the master git repository:

git clone --recursive https://github.com/zeek/zeek

With all dependencies in place, build and install:

./configure && make && sudo make install

Write your first Zeek script:

# File "hello.zeek"

event zeek_init()
    {
    print "Hello World!";
    }

And run it:

zeek hello.zeek

For learning more about the Zeek scripting language, try.zeek.org is a great resource.

Development

Zeek is developed on GitHub by its community. We welcome contributions. Working on an open source project like Zeek can be an incredibly rewarding experience and, packet by packet, makes the Internet a little safer. Today, as a result of countless contributions, Zeek is used operationally around the world by major companies and educational and scientific institutions alike for securing their cyber infrastructure.

If you're interested in getting involved, we collect feature requests and issues on GitHub here and you might find these to be a good place to get started. More information on Zeek's development can be found here, and information about its community and mailing lists (which are fairly active) can be found here.

License

Zeek comes with a BSD license, allowing for free use with virtually no restrictions. You can find it here.

Tooling

We use the following tooling to help discover issues to fix, amongst a number of others.

zeek's People

Contributors

0xxon avatar 1wilkens avatar amazingpp avatar awelzel avatar bbannier avatar ckreibich avatar cstruck avatar dnthayer avatar eladsolomon-ms avatar fatemabw avatar flyingwithjerome avatar grigorescu avatar j-gras avatar jbencteux avatar jsiwek avatar jsoref avatar justinazoff avatar leres avatar mauropalumbo75 avatar mavam avatar maxkellermann avatar neverlord avatar pbcullen avatar rsmmr avatar sethhall avatar srunnels avatar timwoj avatar vpax avatar ynadji avatar zeek-bot avatar

Stargazers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

Watchers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

zeek's Issues

Interpreter exceptions cause memory leaks

Moved from https://bro-tracker.atlassian.net/browse/BIT-831

Created by Johanna Amann at 2012-06-08T18:51:11.569-0500:

The following bro script apparently triggers a memory-leak in the print statement.

event HTTP::log_http(rec: HTTP::Info)
{
	print fmt("%s %s", rec$md5, rec);
}

To reproduce run bro using 2009-M57-day11-18.trace. pprof output is attached.

Comment by Robin Sommer at 2012-06-11T10:21:02.731-0500:

This seems potentially significant. Anyone up for taking a look?

Comment by Jon Siwek at 2012-06-11T10:24:15.970-0500:

Replying to [robin|comment:1]:
This seems potentially significant. Anyone up for taking a look?

Yeah, I think I know what's going on, might have a patch shortly.

Comment by Jon Siwek at 2012-06-11T15:00:35.602-0500:

What's happening is the reference to rec$md5 does not first check whether that optional field is initialized (e.g. rec?$md5), so when it isn't, an internal InterpreterException gets thrown and there's no cleanup of heap allocations during the stack unwind. I'm not sure that catching, cleaning up allocated resources, and rethrowing that exception every place it can occur is going to be the greatest way to fix this. E.g. currently that exception only gets thrown from a couple implementations of UnaryExpr::Fold, but that's called from a couple implementations of Expr::Eval, which maybe used in ~70 places, then still the callers of those places also potentially need to handle the exception to do cleanup, etc.

Would it be better to wait and fix this through overhauling memory management to use smart pointers? That's planned, right? Any script that tries to reference a missing field should be corrected, anyway (these errors are logged in reporter.log).

Comment by Robin Sommer at 2012-06-18T13:58:43.261-0500:

Hmmm … That indeed sounds like a bigger problem. The exception is a relatively recent addition, before that Bro would just abort with an internal error for many runtime errors. So not much we can do right now.

Regarding smart pointers, yes, in principle, but that's a big task, with potential to introduce very subtle problems. So while it's on the roadmap, I'm actually reluctant to tackle it soon. Also, perhaps this can be taken core of by eventually compiling Bro scripts, which will remove lots of the relevant code anyway long-term. But that's all something for a longer discussion.

I'm leaving this ticket open, but I'm removing the milestone.

Comment by Johanna Amann at 2015-10-19T15:38:07.706-0500:

Closing - we will not be able to fix this anytime in the forseeable future because of the current internal architecture. Fixing this would be part of a major rewrite of the scripting functionality of Bro--and we will not forget about it; this is one of the well-known current gotchas of Bro.

Attachment https://bro-tracker.atlassian.net/secure/attachment/10217/bug.bro:

event HTTP::log_http(rec: HTTP::Info)
{
    print fmt("%s %s", rec$md5, rec);
}

incorrect type in default attribute segfaults

Moved from https://bro-tracker.atlassian.net/browse/BIT-1656

Created by Justin Azoff at 2016-07-29T11:03:45.745-0500:

user error, but surprised about the segfault.

$ bro --version
bro version 2.4-715
$ cat foo.bro
global recent_global_view_keys: set[string, string] &create_expire=10secs &default=0;
$ bro foo.bro
error in ./foo.bro, line 1: arithmetic mixed with non-arithmetic (set[string,string] and 0)
error in ./foo.bro, line 1: &default value has inconsistent type (0 and set[string,string])
Segmentation fault: 11

Building plugins without source tree doesn't honor --enable-debug

When building a Bro plugin with a Bro build tree around, cmake/BroPluginDynamic.cmake inspects Bro's cmake cache to infer whether to do a debug build. Without a build tree, however, it always does a release build.

I think we should add --build-mode to bro-config that outputs either release or debug, and then have the plugin build system pick up on that.

Type-checking is missing for some &attributes

Moved from https://bro-tracker.atlassian.net/browse/BIT-1981

Created by Vern Paxson at 2018-09-05T05:46:51.797-0500:

The appended script [^attr-type-check.bro] doesn't generate any complaints about the non-sensical attributes; and the one attribute that does make sense, &default, doesn't work, this script generates an error complaining about a's value not being set.

Comment by Jon Siwek at 2018-09-05T11:05:07.461-0500:

I'd probably say even &default doesn't make sense to permit there? One probably would initialize a default value via = assignment instead?

Comment by Vern Paxson at 2018-09-05T14:16:54.280-0500:

You mean why use "global a: count &default = 5" instead of "global a = 5"? Yeah, I agree. There are maybe microcases where there's a semantic difference (like when assigning to an &optional record field), but that's about it.

Comment by Jon Siwek at 2018-09-05T14:50:27.638-0500:

Yeah, that's all I meant: &default doesn't seem useful in the case of of a global.

Attachment https://bro-tracker.atlassian.net/secure/attachment/21473/attr-type-check.bro:

global a: count
        &default = 10
        &default = 9
        &optional
        &log
        &add_func = function(): count { return 3; };
print a;

Sum of two vectors of int

Moved from https://bro-tracker.atlassian.net/browse/BIT-1760

Created by François at 2016-11-25T16:33:09.248-0600:

There is a bug when adding two vectors.
It works for vector of count, but returns a vector filled iwth zeros for vector of int.

Her is my test case :

event bro_init()
{
local c1 : vector of count = { 1, 2 };
local c2 : vector of count = { 3, 4 };
local c3 = c1 + c2;
print c3;

local v1 : vector of int = { 1, 2 };
local v2 : vector of int = { 3, 4 };
local v3 = v1 + v2;
print v3;
}

Output is :

[4, 6]
[0, 0]

Comment by François at 2016-11-25T16:58:29.761-0600:

It works with the following change :

local v1 : vector of int = { +1, +2 };
local v2 : vector of int = { +3, +4 };
local v3 = v1 + v2;

Well, quite strange !

Comment by Daniel Thayer at 2016-11-25T17:10:23.412-0600:

This might be related to #159

Comment by François at 2016-11-25T17:17:14.343-0600:

I think so.

This sample also works fine :
local v1 : vector of int = { -1, 2 };
local v2 : vector of int = { 3, 4 };
local v3 = v1 + v2;

I think 1 is understood like a count, and -1 and +1 are understood like int.
{ 1, 2} is understood like a vector of count and { -1, 2} a vector of int.

Comment by François at 2016-11-25T17:49:27.654-0600:

In BinaryExpr::Fold() :

  • At the beginning of this method :

    • type of the current element is used : InternalTypeTag it = v1->Type()->InternalType();
    • For a value of 1, i.e. count : is_integral = 0 and is_unsigned = 1, DO_FOLD is called and computes u3.
    • For a value of +1, i.e. int : is_integral = 1 and is_unsigned = 0, Do_FOLD is called and computes i3.
  • At the end of this method :

    • yield type of the vector is used : ret_type = ret_type->YieldType();
    • for a vector of int : return new Val(i3, ret_type->Tag());
    • for a vector of count : return new Val(u3, ret_type->Tag());
  1. So if we have an element of type count in a vector of int, it does not work : u3 is computed, but i3 (=0) is assigned.

  2. This is also true if we have an element of type int into a vector of count : i3 is computed, but u3 (=0) is assigned.

Test case for this second case :
local v1 : vector of count = { +1, +2 };
local v2 : vector of count = { +3, +4 };
local v3 = v1 + v2;

But of course the initial problem is in the computation of types when initializating the vector.

Patterns don't currently work in the input framework

The input framework doesn't currently support the pattern type, likely as a consequence of the former lack of support of dynamic patterns. When trying to use patterns in an input framework type, one gets:

error: Incompatible type "pattern" in type definition for for field "foo" in ReaderFrontend
error: Input stream foobar: Problem unrolling

The type list here is likely a good start: https://github.com/bro/bro/blob/master/src/input/Manager.cc#L797

broxygen ignores some doc comments

Moved from https://bro-tracker.atlassian.net/browse/BIT-1483

Created by Daniel Thayer at 2015-09-21T13:59:16.368-0500:

Tried adding a documentation comment to an enum type, but broxygen ignores it.
It seems the problem is the name "Event" (if I rename the type name, then
the documentation comment is no longer ignored).
Example from frameworks/input/main.bro:

export {

## This line is ignored.
type Event: enum {
    EVENT_NEW = 0,
    EVENT_CHANGED = 1,
    EVENT_REMOVED = 2,
};

Comment by Jon Siwek at 2018-09-05T14:29:24.309-0500:

Can't reproduce in master branch.

Comment by Daniel Thayer at 2018-09-06T10:20:13.260-0500:

I added a documentation comment in commit d1ed09b,
and the comment does not appear in the HTML:
https://www.bro.org/sphinx-git/scripts/base/frameworks/input/main.bro.html

Comment by Daniel Thayer at 2018-09-06T10:51:14.503-0500:

The comment also does not appear in
build/doc/broxygen_script_output/base/frameworks/input/main.bro.rst

to_json mishandles strings with both \ and unprintable characters

Moved from https://bro-tracker.atlassian.net/browse/BIT-1959

Created by Zack Weinberg at 2018-07-25T12:46:57.194-0500:

The to_json function (implemented in base/utils/json.bro) escapes unprintable characters using the clean primitive, and then manually escapes \ and " with regexes. This is wrong on two levels: the transformation performed by clean is irreversible, and \x escapes are not part of the JSON standard. For instance, consider

@load base/utils/json
print to_json("\"\\\x81");

Running this test program with bro -b -C test.bro will produce the output "\"\\\\x81". Because of the irreversible clean transformation, this output could correspond to either the original three-byte string (hexdumped, 22 5C 81) or a six-byte string containing two backslashes and the literal string x81 (hexdumped, 22 5C 5C 78 38 31.) Because \x escapes are not part of the JSON standard, it is not enough to replace clean and the inner gsub with escape_string; that would produce "\\\x81", which is unambiguous, but also unparseable.

The ideal output would be "\"\\\u0081". I'm not sure how to accomplish this, considering that gsub does not appear to implement any way of referring to capture groups from the replacement string. For now I'm going to change json.bro to read

		case "string":
		return cat("\"", gsub(escape_string(v), /\"/, "\\\""), "\"");

and postprocess the JSON with a more powerful regex engine before trying to parse it.

(There is also the headache of dealing with strings with U+0000 in them, but I think it would be fair to declare that Not Your Problem.)

Comment by Zack Weinberg at 2018-07-25T12:55:06.083-0500:

For the record, the construct that doesn't seem to be possible with gsub, but is possible with a "more powerful regex engine", is

perl -pe 's/(?<!\\)((?:\\\\)*\\)x/${1}u00/g'

Comment by Robin Sommer at 2018-08-01T12:45:08.143-0500:

to_json() should probably become a bif so that we can write it in C++ and "do the right thing". This is part of a broader question of potentially making Bro more JSON-friendly. Having well-defined JSON de-/serialization of Bro values would be nice.

And yeah, our regexps don't support capture groups. That's another item on the roadmap: consider replacing Bro's engine with one of the modern libraries; that needs some thought & research.

Misleading error messages parsing scripts with dangling functions

Moved from https://bro-tracker.atlassian.net/browse/BIT-1899

Created by Jim Mellander at 2018-01-30T16:01:00.896-0600:

If a function is left open inadvertantly at the end of a script by omitting the final closing brace, a misleading error message is generated about the next script being loaded. Example: leaving the final brace off of non-cluster.bro (sumstats), has resulted inr in the following message:

error in //bro/base/frameworks/tunnels/./main.bro, line 8: syntax error, at or near "module"

presumably due to the function still being open when the next policy script is loaded. Wouldn't it be more reasonable to check at the end of each script when loaded that there are no dangling functions, expressions, etc. ????

Leverage CAF's flow control to limit communication load

CAF provides flow control with back pressure, and Bro should be able to leverage that for making sender-side decisions on when to throttle messages. The most basic approach would probably be a simple policy to declare outgoing events that can be dropped if the channel becomes overloaded.

Improve signaling of deprecated code

Moved from https://bro-tracker.atlassian.net/browse/BIT-1474

Created by Matthias Vallentin at 2015-09-06T19:42:56.730-0500:

Based on discussion in BIT-1411, we need a better mechanism for deprecating code. In particular, Vern suggested the attribute &deprecate=expr syntax where expr would be an expression that evaluates at the point of use of the deprecated construct and would show the user a hint on how to migrate.

Comment by Vern Paxson at 2015-09-06T19:51:49.476-0500:

One thing to add is the need for some sort of option/context such that users can control when the message about the deprecated variable gets displayed.

Comment by Jon Siwek at 2015-09-08T09:13:35.862-0500:

I had already done "&deprecated" (as part of 2.4 I believe). But don't think I allowed it to be assigned an expression which could contain more info or had a global flag to disable the warnings. (Just trying to provide input on what's already available, but both those ideas do seem like useful features to add).

Comment by Matthias Vallentin at 2015-09-08T20:28:32.036-0500:

Ah, I missed that. That's definitely something we can work with. Let's keep this ticket open for now to track the enhancement with evaluated expressions.

initialization of "int" record fields fails for unsigned integers

Moved from https://bro-tracker.atlassian.net/browse/BIT-1759

Created by Daniel Thayer at 2016-11-24T12:09:43.312-0600:

The following script demonstrates how initialization is not consistent with
assignment for the "int" type:

type myrecord: record {
    ii: int;
};

global globalfail: myrecord &redef;
redef globalfail = [$ii = 7];

event bro_init()
{
    local fail1 = myrecord($ii = 7);
    local fail2: myrecord = record($ii = 7);
    local fail3: myrecord = [$ii = 7];

    local ok1: myrecord;
    ok1$ii = 7;
}

Running this script gives the following result:
error in ./test.bro, line 6 and count: type clash for field "ii" ((coerce [$ii=7] to myrecord) and count)
error in ./test.bro, line 6: bad record initializer ((coerce [$ii=7] to error))
error in ./test.bro, line 10 and count: type clash for field "ii" ((coerce [$ii=7] to myrecord) and count)
error in ./test.bro, line 11 and count: type clash for field "ii" ((coerce [$ii=7] to myrecord) and count)
error in ./test.bro, line 12 and count: type clash for field "ii" ((coerce [$ii=7] to myrecord) and count)

Changing the "7" to "+7" in the script for all occurrences of "7" except the last one makes the script work as expected.

Comment by Jon Siwek at 2018-09-13T13:25:53.081-0500:

Another example, for reference, in BIT-1762.

Add new string functions

Picking up BIT-1754 again:

List of functions to implement:

  • str.count - Return the number of non-overlapping occurrences of substring sub in the range
  • str.find - Return the lowest index in the string where substring sub is found within the slice s[start:end].
  • str.lstrip - Return a copy of the string with leading characters removed. The chars argument is a string specifying the set of characters to be removed.
  • str.rfind - Return the highest index in the string where substring sub is found
  • str.rstrip - Return a copy of the string with trailing characters removed. The chars argument is a string specifying the set of characters to be removed
  • str.strip - Return a copy of the string with the leading and trailing characters removed. The chars argument is a string specifying the set of characters to be removed

Notes from @sethhall

We'd merge these in if you implemented them. Are you still planning on doing so? I have a couple of notes too...

  • count can be done with the built in find_all function
  • find can currently be implemented with the built in strstr function.
  • lstrip, rstrip, can be implement with the sub function.

misc/dump-events only dumps events handled by other scripts

Moved from https://bro-tracker.atlassian.net/browse/BIT-1218

Created by Vlad Grigorescu at 2014-07-11T20:06:01.469-0500:

misc/dump-events is a very handy script, and I often use it as a script writing tool. If I have a PCAP, I run it with misc/dump-events to get a quick sense of which events fire on it, and how many times each event fires. This helps me pick out the best event to handle.

The issue is that events that aren't handled elsewhere don't get reported, as unhandled events aren't generated. Would it be possible to have dump-events (or perhaps dump-all-events) pretend like all events are handled, to get a more complete event listing?

Problem with arguments in multi-workers environment

Hi,

I've got a problem in multi-worker environment.
I've pasted code and result in the following screenshot.

This is the bug:
capture_bro_ref_bug_2_code
with the result:
capture_bro_ref_bug_2_result

As you can see, only the last value of the type my_rec is printed.
This is working properly if my_rec is a basic type (like string or count).
This is inspired by notice framework code.

There is the a workaround:
capture_bro_ref_bug_1_code
with the result:
capture_bro_ref_bug_1_result

But with very complex record, this is not easy to do this.
And it's seems to highlight a problem in communication between workers.

Am I right, or is there another means to do this ?

Thank for your reply.

Radius Attribute 66

Per RFC 2868 section 3.3 (https://tools.ietf.org/html/rfc2868#section-3.3) specifically the "String" section (https://tools.ietf.org/html/rfc2868#page-6)
says that the Radius attribute 66 can be an FQDN or an IP Address.

Also, you may note the Cisco doc on Attribute 66
https://www.cisco.com/c/en/us/td/docs/ios-xml/ios/sec_usr_radatt/configuration/xe-16/sec-usr-radatt-xe-16-book/sec-rad-att-66-tunnel.html#reference_BBF6BA0BA03045838A307FA0AE1AFD40

The current implementation for the Bro Radius protocol is an "addr". I would recommend changing it to a string.
https://github.com/bro/bro/blob/a33d2d13bfa2788593d1ba2fa2988b4496c17193/scripts/base/protocols/radius/main.bro#L88

Bug in Table Value Type Verification

Moved from https://bro-tracker.atlassian.net/browse/BIT-1859

Created by Aaron Eppert at 2017-10-21T11:39:24.735-0500:

event bro_init(){
    local service_table : table[string, count] of count = {["www", 80] = "Internal Web Server", 
                                                           ["dns1", 53] = "Internal DNS 1",
                                                           ["dns2", 53] = "Internal DNS 2", 
                                                           ["dhcp-for-wifi", 443] = "DHCP Management interface for WiFi"};

    print service_table;
}

The above snippet works fine, but notice the values of the table are strings not counts. However, if we change any table value to a count, an error is raised.

event bro_init(){
    local service_table : table[string, count] of count = {["www", 80] = "Internal Web Server", 
                                                           ["dns1", 53] = "Internal DNS 1",
                                                           ["dns2", 53] = 3, 
                                                           ["dhcp-for-wifi", 443] = "DHCP Management interface for WiFi"};

    print service_table;
}

Forward declarations of events don't work inside a module namespace

Moved from https://bro-tracker.atlassian.net/browse/BIT-71

Created by Robin Sommer at 2009-02-20T13:43:57.000-0600:

Forward declarations of events aren't correctly resolved when used inside a module namespace. The worst thing about this is that they fail silently: there's no error message, the handler is just not executed.

The example below never prints anything. Once the module statement is removed, everything works fine though.

module Foo;

global bar: event();

event bar()
    {
    print "bar";
    }

event new_connection(c: connection)
    {
    event bar();
    }

Comment by Jon Siwek at 2015-03-16T14:38:28.976-0500:

Other examples of the problem referenced in BIT-984.

Comment by Robin Sommer at 2016-07-08T18:46:57.988-0500:

I don't see an easy fix for this. Unfortunately event IDs are handled somewhat differently than other IDs, but the global prototype cannot adjust to that because the parser doesn't know that it's an event before it's too late.

Closing, as there's a work-around by using fully scoped event names (see BIT-984).

find-filtered-trace could be a bit smarter

Moved from https://bro-tracker.atlassian.net/browse/BIT-1919

Created by Vern Paxson at 2018-04-05T17:04:44.097-0500:

I fed Bro a pcap filtered to port 53 traffic. It had a bunch of UDP activity, but the TCP activity was just scans. This led to "The analyzed trace file was determined to contain only TCP control packets, which may indicate it's been pre-filtered. By default, Bro reports the missing segments for this type of trace, but the 'detect_filtered_trace' option may be toggled if that's not desired. being generated - misleading in this context. In particular, the trace doesn't contain only TCP control packets; rather, of the TCP packets it contained, those were only control packets.

Seems the script could be smarter and if it sees UDP w/ payload, then not warn.

type of vector loop index is int, not count

The following code snippet prints "int", not "count": for ( i in vector("a") ) print type_name(i);, even though the documentation sez

A vector is like a table, except it’s always indexed by a count

The problem is the use of basetype(TYPE_INT) in ForStmt::ForStmt.

File analyzer API doesn't have a good way to pass arguments to instantiated analyzers

When adding a file analyzer with add_analyzer, there's no good way to pass arguments to the new analyzer instance (say, if it were an analyzer to extract certain tags from a data stream, I would to want tell it which tags to extract). There's a record AnalyzerArgs but that's kind of an odd thing: right now it has two callbacks for the DATA_EVENT analyzer; but it's passed in for analyzers of any kind. One could extend that type with further arguments for other analyzers, but having one type for all analyzers doesn't seem like the right mechanisms.

SSH Analyzer won't trigger proper logging of auth_success for successful "none" method authentications

Our Bro instance has been firing some SSH::Watched_Country_Login notices where the corresponding SSH log entry does not have auth_success set. That is, our standard tab separated SSH logs are showing auth_success as '-' instead of 'T'.

This seems to be happening only for successful "none" method authentications.

Looking at the SSH Analyzer code (SSH.cc), I think the problem is a missing call to BifEvent::generate_ssh_auth_attempted() in SSH_Analyzer::ProcessEncrypted() when the successful "none" method authentication is detected.

Specifically:

		// If our user can authenticate via the "none" method, this
		// packet will be a SSH_MSG_USERAUTH_SUCCESS, which has a
		// fixed (decrypted) size of 8 bytes (1 for content
		// pad-aligned to 8-byte boundaries). relative_len would be
		// -16.
		if ( ! userauth_failure_size && (len + 16 == service_accept_size) )
			{
			auth_decision_made = true;
			if ( ssh_auth_successful )
				BifEvent::generate_ssh_auth_successful(interp->bro_analyzer(), interp->bro_analyzer()->Conn(), true);
			return;
			}

should probably become:

		// If our user can authenticate via the "none" method, this
		// packet will be a SSH_MSG_USERAUTH_SUCCESS, which has a
		// fixed (decrypted) size of 8 bytes (1 for content
		// pad-aligned to 8-byte boundaries). relative_len would be
		// -16.
		if ( ! userauth_failure_size && (len + 16 == service_accept_size) )
			{
			auth_decision_made = true;
			if ( ssh_auth_attempted )
				BifEvent::generate_ssh_auth_attempted(interp->bro_analyzer(), interp->bro_analyzer()->Conn(), true);
			if ( ssh_auth_successful )
				BifEvent::generate_ssh_auth_successful(interp->bro_analyzer(), interp->bro_analyzer()->Conn(), true);
			return;
			}

This matches the structure of the code for when normal successful authentications are detected by:

// ...or a success packet.
		if ( len - service_accept_size == -16 )
			{
			auth_decision_made = true;
			if ( ssh_auth_attempted )
				BifEvent::generate_ssh_auth_attempted(interp->bro_analyzer(), interp->bro_analyzer()->Conn(), true);
			if ( ssh_auth_successful )
				BifEvent::generate_ssh_auth_successful(interp->bro_analyzer(), interp->bro_analyzer()->Conn(), false);
			return;
			}
}

In bro/scripts/base/protocols/ssh/main.bro, the ssh_auth_attempted() is what sets c$ssh$auth_success and ultimately triggers successful authentications to be logged. If ssh_auth_attempted() is never run, then the SSH log entry is created in connection_state_remove() with c$ssh$auth_success unset.

Since SSH::Watched_Country_Login fires from ssh_auth_successful(), the notice is fired even though the SSH log entry ends up showing auth_success as '-'.

I diagnosed the issue by adding the auth_method_none value from ssh_auth_successful() to the SSH::Watched_Country_Login notice msg and saw that anytime we had an auth_method_none = T, the SSH log entry showed auth_success as '-'.

Bad subnet masks should not manifest as internal errors

An statement like print 1.2.3.4/33; errors out with internal error in <stdin>, line 1: Bad IPAddr(v4) IPPrefix length : 33. This is both excessive and makes it hard to pinpoint what lead to the problem. It should instead generate a regular run-time error.

sets that (ultimately) contain sets do not do proper lookups/deletes

Moved from https://bro-tracker.atlassian.net/browse/BIT-1949

Created by Vern Paxson at 2018-06-29T12:12:29.674-0500:

A set that ultimately(1) contains other sets can miscompute membership. For example, the attached script, if run 100 times, will fail to delete the given set member about 15% of the time. This appears to be due to randomizing the hash functions, because in related testing I was able to get consistent behavior by loading the same seeds each time.

When fixing this problem, if set operations like union/intersection have been implemented (per a pending branch), then be sure to test them too.

(1) We don't currently support sets-of-sets. Would be good to fix this at the same time too.

Attachment https://bro-tracker.atlassian.net/secure/attachment/21450/set-containing-sets-index-bug.bro:

type r: record {
    b: set[count];
};

global a: set[r];

add a[record($b=set(1,3,5))];
add a[record($b=set(2,4,6))];

print a;

delete a[record($b=set(5,3,1))];

print a;

mysql analyzer logs tls as binary

Occasionally we see 500k binary lines being written to the mysql.log whenever a tls connection is in use.

cmd+arg ends up being something like

unknown-242     \x01\x00\x00\xee\x03\x03...+500k more of data

I can see the plaintext part of a x509 cert in there as well.

I think vlad had this mostly fixed in topic/vladg/mysql_tls, but that broke normal query logging.

Bro crash with packet filter definition

Moved from https://bro-tracker.atlassian.net/browse/BIT-1640

Created by Johanna Amann at 2016-07-03T09:35:49.480-0500:

Reported via email; crash with:

@load base/frameworks/packet-filter
redef PacketFilter::restricted_filter = PacketFilter::port_to_bpf(80/tcp);

backtrace:

* thread #1: tid = 0x1f713, 0x000000010001aa60 bro`Ref(o=0x0000000000000000) + 16 at Obj.h:210, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x1c)
    frame #0: 0x000000010001aa60 bro`Ref(o=0x0000000000000000) + 16 at Obj.h:210
   207 	
   208 	inline void Ref(BroObj* o)
   209 		{
-> 210 		if ( ++o->ref_cnt <= 1 )
   211 			bad_ref(0);
   212 		if ( o->ref_cnt == INT_MAX )
   213 			bad_ref(1);
(lldb) bt
* thread #1: tid = 0x1f713, 0x000000010001aa60 bro`Ref(o=0x0000000000000000) + 16 at Obj.h:210, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x1c)
  * frame #0: 0x000000010001aa60 bro`Ref(o=0x0000000000000000) + 16 at Obj.h:210
    frame #1: 0x000000010001b31f bro`BroType::Ref(this=0x0000000000000000) + 31 at Type.h:250
    frame #2: 0x000000010001b2b9 bro`Val::Val(this=0x00000001061c80f0, t=0x0000000000000000) + 73 at Val.h:377
    frame #3: 0x000000010001b213 bro`EnumVal::EnumVal(this=0x00000001061c80f0, i=1, t=0x0000000000000000) + 35 at Val.h:955
    frame #4: 0x000000010001a983 bro`EnumVal::EnumVal(this=0x00000001061c80f0, i=1, t=0x0000000000000000) + 35 at Val.h:956
    frame #5: 0x0000000100112981 bro`map_conn_type(tp=TRANSPORT_TCP) + 193 at bro.bif:3120
    frame #6: 0x0000000100112c41 bro`BifFunc::bro_get_port_transport_proto(frame=0x00000001061c8020, BiF_ARGS=0x00000001061c8090) + 113 at bro.bif:3171
    frame #7: 0x000000010010565f bro`BuiltinFunc::Call(this=0x0000000102b68d60, args=0x00000001061c8090, parent=0x00000001061c8020) const + 1183 at Func.cc:586
    frame #8: 0x00000001000f2e75 bro`CallExpr::Eval(this=0x0000000102e181e0, f=0x00000001061c8020) const + 421 at Expr.cc:4544
    frame #9: 0x00000001000e5a7d bro`AssignExpr::Eval(this=0x0000000102e18380, f=0x00000001061c8020) const + 93 at Expr.cc:2400
    frame #10: 0x00000001001fae44 bro`ExprStmt::Exec(this=0x0000000102e18500, f=0x00000001061c8020, flow=0x00007fff5fbf48b0) const + 84 at Stmt.cc:352
    frame #11: 0x000000010020245b bro`StmtList::Exec(this=0x0000000102e17c20, f=0x00000001061c8020, flow=0x00007fff5fbf48b0) const + 219 at Stmt.cc:1699
    frame #12: 0x0000000100103b2b bro`BroFunc::Call(this=0x0000000102e18fc0, args=0x00000001061c79e0, parent=0x0000000000000000) const + 3179 at Func.cc:403
    frame #13: 0x00000001000f2e75 bro`CallExpr::Eval(this=0x00000001061c7f10, f=0x0000000000000000) const + 421 at Expr.cc:4544
    frame #14: 0x00000001000d6f8a bro`Expr::InitVal(this=0x00000001061c7f10, t=0x0000000101f8af30, aggr=0x0000000000000000) const + 122 at Expr.cc:130
    frame #15: 0x000000010023ab25 bro`init_val(init=0x00000001061c7f10, t=0x0000000101f8af30, aggr=0x0000000000000000) + 69 at Var.cc:16
    frame #16: 0x000000010023876f bro`make_var(id=0x00000001051b1d10, t=0x0000000101f8af30, c=INIT_FULL, init=0x00000001061c7f10, attr=0x0000000000000000, dt=VAR_REDEF, do_init=1) + 2223 at Var.cc:190
    frame #17: 0x0000000100237eba bro`add_global(id=0x00000001051b1d10, t=0x0000000000000000, c=INIT_FULL, init=0x00000001061c7f10, attr=0x0000000000000000, dt=VAR_REDEF) + 74 at Var.cc:228
    frame #18: 0x0000000100013159 bro`yyparse() + 29161 at parse.y:1070
    frame #19: 0x000000010003b750 bro`main(argc=4, argv=0x00007fff5fbff670) + 11776 at main.cc:892
    frame #20: 0x00007fff93c695ad libdyld.dylib`start + 1

Comment by Johanna Amann at 2016-07-05T13:31:13.305-0500:

Ok, I figured out what is going on here. The problem is that get_port_transport_proto (called in PacketFilter::port_to_bpf) indirectly calls:

return new EnumVal(1, transport_proto);

transport_proto is one of the variables that is populated in init_net_var (NetVar.cc). This happens only after yyparse() is done. init_net_var cannot be moved before yyparse, because it needs the types that are generated during parsing.

And with that I am a bit unsure what we should do here - basically, using functions in redefs will work in many cases, but will cause a crash anytime one of the netvars is used internally. I don't really think we use this ourselves - would it perhaps make sense to just prohibit function calls in redefs?

Comment by Johanna Amann at 2016-07-05T15:05:37.624-0500:

I just talked to Robin for a few minutes - a possible solution to this problem would be to split parsing into two steps - parse init_bare first, which populates the records, then call init_net_var, then parse the rest. I will try to look if this is possible...

FR: Protosigs port range support

Moved from https://bro-tracker.atlassian.net/browse/BIT-1752

Created by jamesl at 2016-11-02T12:44:46.416-0500:

Topic says it...several protocols go over large ranges [Apple|https://support.apple.com/en-us/HT202944]. For example, Apples RTP and RTCP go from 16403-16472. I'd either have to a) put in all 69 entries, or just not specify port range. Would be nice if the Signature Framework supported ranges, to even include comma separated ranges. Thank you.

rpath problem with librocksdb on macOS

I've got RocksDB installed via MacPorts in /opt/local. I'm building Bro without any special configure options, and with prefix /opt/bro-demo. Configure finds RocksDB fine and the build links against it. Then I do make install. Now when I run the resulting binary /opt/bro-demo/bin/bro binary, I get:

dyld: Library not loaded: librocksdb.5.14.dylib
  Referenced from: /opt/bro-demo/bin/bro
  Reason: image not found
Abort trap: 6

I can fix that by setting DYLD_LIBRARY_PATH to /opt/local/lib. However, that environment variable doesn't propagate to subprocess on macOS, meaning that when btest spawns bro from that location, it runs into the same problem.

I believe rocksdb is the only library that has that problem, and it's probably because of how it's linker path differs from others:

# otool -L  /opt/bro-demo/bin/bro 
/opt/bro-demo/bin/bro:
	/usr/lib/libpcap.A.dylib (compatibility version 1.0.0, current version 1.0.0)
	/opt/local/lib/libssl.1.0.0.dylib (compatibility version 1.0.0, current version 1.0.0)
	/opt/local/lib/libcrypto.1.0.0.dylib (compatibility version 1.0.0, current version 1.0.0)
	/usr/lib/libresolv.9.dylib (compatibility version 1.0.0, current version 1.0.0)
	/opt/local/lib/libz.1.dylib (compatibility version 1.0.0, current version 1.2.11)
	@rpath/libbroker.0.dylib (compatibility version 0.0.0, current version 0.0.0)
	@rpath/libcaf_core.0.16.0.dylib (compatibility version 0.16.0, current version 0.16.0)
	@rpath/libcaf_io.0.16.0.dylib (compatibility version 0.16.0, current version 0.16.0)
	@rpath/libcaf_openssl.0.16.0.dylib (compatibility version 0.16.0, current version 0.16.0)
	librocksdb.5.14.dylib (compatibility version 0.0.0, current version 0.0.0)
	/usr/lib/libc++.1.dylib (compatibility version 1.0.0, current version 400.9.0)
	/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1252.50.4)

incorrect module identification in error message

Moved from https://bro-tracker.atlassian.net/browse/BIT-1937

Created by Vern Paxson at 2018-06-12T12:03:41.818-0500:

The attached script reports an error "line 8: no such field in record (y::f$z)" when instead it should be "line 8: no such field in record (x::f$z)" (i.e., x:: rather than y:: ).

Attachment https://bro-tracker.atlassian.net/secure/attachment/21448/b.bro:

module x;
export { type foo: record { a: count; }; }

module y;

function bar(f: x::foo)
    {
    print f$z;
    }

Compiling with musl (Alpine)

There are a few issues that prevent Bro from being compiled on Alpine (or any system using musl-libc). It looks like there have been several community efforts to get this working, like the one here: https://github.com/blacktop/docker-bro/tree/master/2.5/patches.

On Alpine, I had to install some extra packages to get bro to build:

fts-dev and linux-headers

I don't think that the patches required to compile with musl would be terribly controversial.

The bigger deal, is that as far as I can tell, is that DNS resolution doesn't work.

virt-brodev01:~# cat foo.bro 
event bro_init()
	{
	when ( local h = lookup_hostname("www.google.com") ) { print h; }
	}

virt-brodev01:~# nslookup www.google.com
Name:      www.google.com
Address 1: 172.217.4.100 ord36s04-in-f4.1e100.net
Address 2: 2607:f8b0:4009:800::2004 ord36s04-in-x04.1e100.net

virt-brodev01:~# /usr/local/bro/bin/bro -i eth0 foo.bro 
warning in /usr/local/bro/share/bro/base/init-bare.bro, line 1: problem initializing NB-DNS: no valid nameservers in resolver config
listening on eth0

warning: can't issue DNS request
warning: can't issue DNS request
{
0.0.0.0
}
^C1542721228.497591 received termination signal

logging issue with smb file read and write

As i saw in bro SMB scripts (base/protocols/smb/main.bro), bro doesn't log FILE_READ and FILE_WRITE actions. Is there any special reason for not doing it?

## The file actions which are logged.
option logged_file_actions: set[Action] = {
	FILE_OPEN,
	FILE_RENAME,
	FILE_DELETE,

	PRINT_OPEN,
	PRINT_CLOSE,
};

Bro segfaults when reporter.log is unwritable

Bro segfaults on os-x (mojave) when reporter.log is unwritable (e.g. because owned by a different user).

Backtrace varies a bit - always in some destructor. Example:

$ lldb -- bro config.bro
(lldb) target create "bro"
r
Current executable set to 'bro' (x86_64).
(lldb) settings set -- target.run-args  "config.bro"
(lldb) r
Process 64202 launched: '/Users/johanna/bro/install-master/bin/bro' (x86_64)
warning: ./config/Input::READER_CONFIG: Init: cannot open ./config
Process 64202 stopped
* thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0xe)
    frame #0: 0x00000001003b301f bro`plugin::Manager::HookQueueEvent(Event*) const [inlined] std::__1::__list_imp<std::__1::pair<int, plugin::Plugin*>, std::__1::allocator<std::__1::pair<int, plugin::Plugin*> > >::begin(this=0x0000000000000006) at list:598
   595 	#if _LIBCPP_DEBUG_LEVEL >= 2
   596 	        return iterator(__end_.__next_, this);
   597 	#else
-> 598 	        return iterator(__end_.__next_);
   599 	#endif
   600 	    }
   601 	    _LIBCPP_INLINE_VISIBILITY
Target 0: (bro) stopped.
(lldb) bt
* thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0xe)
  * frame #0: 0x00000001003b301f bro`plugin::Manager::HookQueueEvent(Event*) const [inlined] std::__1::__list_imp<std::__1::pair<int, plugin::Plugin*>, std::__1::allocator<std::__1::pair<int, plugin::Plugin*> > >::begin(this=0x0000000000000006) at list:598
    frame #1: 0x00000001003b301f bro`plugin::Manager::HookQueueEvent(Event*) const [inlined] std::__1::list<std::__1::pair<int, plugin::Plugin*>, std::__1::allocator<std::__1::pair<int, plugin::Plugin*> > >::begin(this=0x0000000000000006 size=0) at list:910
    frame #2: 0x00000001003b301f bro`plugin::Manager::HookQueueEvent(this=0x0000000103500110, event=0x0000000103358570) const at Manager.cc:655
    frame #3: 0x00000001000e77de bro`EventMgr::QueueEvent(this=0x0000000100ac82f0, event=0x0000000103358570) at Event.cc:113
    frame #4: 0x000000010003cf9e bro`EventMgr::QueueEvent(this=0x0000000100ac82f0, h=0x00007ffeefbfccc0, vl=0x000000010333cd20, src=0, aid=0, mgr=0x0000000000000000, obj=0x0000000000000000) at Event.h:61
    frame #5: 0x00000001001cbfc2 bro`Reporter::DoLog(this=0x0000000106d9d490, prefix="error", event=EventHandlerPtr @ 0x00007ffeefbfccc0, out=0x00007fff91f66240, conn=0x0000000000000000, addl=0x0000000000000000, location=true, time=true, postfix=0x0000000000000000, fmt="%s: %s", ap=0x00007ffeefbfcce0) at Reporter.cc:497
    frame #6: 0x00000001001ce2a7 bro`Reporter::Error(this=0x0000000106d9d490, fmt="%s: %s") at Reporter.cc:102
    frame #7: 0x00000001002c7de9 bro`threading::ReporterMessage::Process(this=0x000000010aa51f90) at MsgThread.cc:148
    frame #8: 0x00000001002c4083 bro`threading::Manager::Process(this=0x00000001035000a0) at Manager.cc:134
    frame #9: 0x00000001002c1e06 bro`threading::Manager::Terminate(this=0x00000001035000a0) at Manager.cc:30
    frame #10: 0x00000001002c1cf0 bro`threading::Manager::~Manager(this=0x00000001035000a0) at Manager.cc:20
    frame #11: 0x00000001002c25f5 bro`threading::Manager::~Manager(this=0x00000001035000a0) at Manager.cc:18
    frame #12: 0x00000001002c2619 bro`threading::Manager::~Manager(this=0x00000001035000a0) at Manager.cc:18
    frame #13: 0x0000000100825f40 bro`iosource::Manager::~Manager(this=0x00000001061a5e60) at Manager.cc:27
    frame #14: 0x00000001008263f5 bro`iosource::Manager::~Manager(this=0x00000001061a5e60) at Manager.cc:23
    frame #15: 0x000000010003ce8f bro`terminate_bro() at main.cc:384
    frame #16: 0x0000000100042509 bro`main(argc=2, argv=0x00007ffeefbff628) at main.cc:1252
    frame #17: 0x00007fff5fec8085 libdyld.dylib`start + 1
    frame #18: 0x00007fff5fec8085 libdyld.dylib`start + 1
(lldb)

config.bro is a simple Bro file written to generate an reporter output:

redef Config::config_files += { "./config" };

The file config does not exist. reporter.log is owned by root and not writeable by the process running Bro.

consolidate redundant frame/call stack tracking

Both these seem to accomplish the same thing:

https://github.com/bro/bro/blob/f31204c7c55a87479f36b7f1195c8f5323330904/src/Func.cc#L53
https://github.com/bro/bro/blob/f31204c7c55a87479f36b7f1195c8f5323330904/src/Frame.cc#L10

The later is maybe more general so can try consolidating to that.

Another enhancement may be to wrap accesses/modifications to that frame stack in a new thread-safe API/structure. Reason being that it's otherwise difficult for plugins to passively observe/sample the script-layer call stack (can't use a BIF because that changes call stack, can't use hooks in main thread because it's also important to know how much time is not spent in script-layer, so naturally would turn to requiring thread-safe access from external plugin thread).

Segmentation fault (core dumped) if an expire_func is missing a return <interval>

Moved from https://bro-tracker.atlassian.net/browse/BIT-1925

Created by Aashish Sharma at 2018-04-20T14:38:50.128-0500:

I know this error/crash is introduced because of lame coding practice but nevertheless it results in a segfault and a coredump.

If an expire_function is missing a return time interval bro would segfault.

The attached script demonstrates this.

the issue becomes very subtle and difficult to identify/debug if one has complicated expire_func where different conditions return different times for extending or deleting values in the table. The segfault may not happen for days then. Lack of backtrace and any other indicator results in identifying and debugging this issue very hard.

[^test-expire-crash.bro]

Attachment https://bro-tracker.atlassian.net/secure/attachment/21446/test-expire-crash.bro:

export { 


        global expire_worker_stats: function(t: table[addr] of count, idx: addr): interval ;
        global worker_stats: table[addr] of count 
                 &create_expire=5 secs  &expire_func=expire_worker_stats ;
}

function expire_worker_stats(t: table[addr] of count, idx: addr): interval
{

        print fmt("expire_worker_stats: 0 secs:  %s", t[idx]); 
    
    ### comment it out for normal run
        #return 0 secs;
}


event connection_state_remove(c: connection)
{
        local orig = c$id$orig_h ;

        if (orig !in worker_stats)
        {
                worker_stats[orig] = 0 ; 
        }

        worker_stats[orig] += 1 ; 
}

option keyword not working with BifConst

I went through our scripts to identify what variables we're currently redef-ing, in order to convert those to use the new option keyword. It looks like any variable that's accessed via BifConst results in an error when it's converted to an option.

For example, SMB::pipe_filenames is accessed via BifConst here:

smb2-com-create.pac: BifConst::SMB::pipe_filenames->AsTable()->Lookup(filename->CheckString()) )

And when I change the export block in init-bare to:

option SMB::pipe_filenames: set[string] = {};

I get the following error:

internal error: internal variable SMB::pipe_filenames is not constant

Is there something intrinsically different about variables that are accessed via BifConst, or can this behavior be fixed?

-f silently fails if base/frameworks/packet-filter isn't loaded

Moved from https://bro-tracker.atlassian.net/browse/BIT-1407

Created by Vern Paxson at 2015-05-30T15:30:45.970-0500:

I know we've been through this before (though searching the tickets in Jira, I couldn't find the thread). But to revisit: the "-f filter" option silently does nothing if base/frameworks/packet-filter isn't loaded (so the scenario here is using -b to suppress its automatic loading). This can lead to seriously confusing behavior. It would be preferable if there's either an error message indicating that the option won't be supported, or if it forced loading of packet-filter.

Comment by Robin Sommer at 2015-06-01T10:11:09.695-0500:

Yeah, I can see that. I think the main problem is the interaction
between the command-line option and script, something that's rare
(i.e., that the command-line option is tight that closely to a script
being loaded). I would actually suggest we remove the command-line
option altogether and instead work with a global: "bro -i eth0
PacketFilter::filter=XXXX" (I believe we have a global with that
effect already, otherwise we could add it).

Robin

Comment by Seth Hall at 2015-06-01T10:19:09.748-0500:

We could alternately take the additional step of moving command line argument processing into Bro scripts. I don’t know how exactly we’d solve this particular problem, but it should be reasonably solvable that way.

.Seth

--
Seth Hall
International Computer Science Institute
(Bro) because everyone has a network
http://www.bro.org/

Comment by Robin Sommer at 2015-06-01T10:35:09.583-0500:

If we had that, the '-f' switch would probably be added by the packet
filter script, so it'd be available only when loaded.

I'd like to move argument processing into script-land as well.
Generally, however, I think it would still be good to avoid arguments
controlling script behaviour as much as possible. Main reason is
argument inflation: -f has been historically around but there are
plenty other scripts that in principle could take command-line
arguments as well, which would get messy. And we have the X=Y syntax
already to take care of that.

Comment by Vern Paxson at 2015-06-01T13:33:29.022-0500:

While there's an appeal to processing arguments in script-land, because some arguments control basic script processing (e.g., -p), I'm not sure this can be done in a coherent fashion without some significant under-the-hood kludges.

Regarding replacing -f with PacketFilter::filter=XXXX, yuck - I wouldn't want to have to remember that, and there's no ready way for a user to discover this.

I'd be happy settling for -f warning (or exiting) with a statement that without the packet filtering framework loaded, it's a no-op. Though I have a forboding that you're going to tell me that that's actually hard to implement :-P.

Comment by Vern Paxson at 2015-06-01T13:35:33.426-0500:

Also, regarding the policy script adding the -f flag: how would the user discover that that's what they need to do (have the policy script loaded) to get the flag? I can see quite a bit of perplexment if Bro just tells the user that the flag doesn't exist without explaining why, if they don't have the script loaded.

Malformed .bro script segfaults

Moved from https://bro-tracker.atlassian.net/browse/BIT-1708

Created by Justin Azoff at 2016-09-29T17:34:29.238-0500:

Was playing around with running the bro parser through afl-fuzz and found one crash in the parser:

root@bro-manager:~/crashes.1# xxd ./id\:000031*
00000000: 00ec ecec 2f22 ff6e 6e69 746c 7465 7175  ..../".nnitltequ
00000010: 6103 e83b 5b2f 007f ecec 40ff 6e69 746c  a..;[/[email protected]
00000020: 7f00                                     ..
root@bro-manager:~/crashes.1# bro --version
bro version 2.5-beta-debug
root@bro-manager:~/crashes.1# bro ./id\:000031*
error in ././id:000031,sig:11,src:000335,op:int16,pos:17,val:be:+1000, line 1: unrecognized character -
error in ././id:000031,sig:11,src:000335,op:int16,pos:17,val:be:+1000, line 1: unrecognized character - �
error in ././id:000031,sig:11,src:000335,op:int16,pos:17,val:be:+1000, line 1: unrecognized character - �
error in ././id:000031,sig:11,src:000335,op:int16,pos:17,val:be:+1000, line 1: unrecognized character - �
error in ././id:000031,sig:11,src:000335,op:int16,pos:17,val:be:+1000, line 1: unrecognized character -
error in ././id:000031,sig:11,src:000335,op:int16,pos:17,val:be:+1000, line 1: unrecognized character -
error in ././id:000031,sig:11,src:000335,op:int16,pos:17,val:be:+1000, line 1: unrecognized character - �
error in ././id:000031,sig:11,src:000335,op:int16,pos:17,val:be:+1000, line 1: unrecognized character - �
error in ././id:000031,sig:11,src:000335,op:int16,pos:17,val:be:+1000, line 1: unrecognized character - @
error in ././id:000031,sig:11,src:000335,op:int16,pos:17,val:be:+1000, line 1: unrecognized character - �
Segmentation fault

The backtrace is

Program received signal SIGSEGV, Segmentation fault.
0x00000000008600d2 in EquivClass::UniqueChar (this=0x18bd288, sym=-24) at /root/src/bro-2.5-beta/src/EquivClass.cc:170
170                     bck[fwd[sym]] = bck[sym];
(gdb) bt
#0  0x00000000008600d2 in EquivClass::UniqueChar (this=0x18bd288, sym=-24) at /root/src/bro-2.5-beta/src/EquivClass.cc:170
#1  0x00000000008bea1b in NFA_State::NFA_State (this=0x2e0e380, arg_sym=-24, ec=0x18bd288) at /root/src/bro-2.5-beta/src/NFA.cc:28
#2  0x00000000007f201b in RE_parse () at re-parse.y:206
#3  0x00000000008d32f7 in Specific_RE_Matcher::Compile (this=0x18bd140, lazy=0) at /root/src/bro-2.5-beta/src/RE.cc:111
#4  0x00000000008d42f8 in RE_Matcher::Compile (this=0x3b7ad50, lazy=0) at /root/src/bro-2.5-beta/src/RE.cc:425
#5  0x00000000007f58f9 in yyparse () at parse.y:699
#6  0x000000000080e09f in main (argc=2, argv=0x7fffffffe548) at /root/src/bro-2.5-beta/src/main.cc:854
(gdb)

and the file is attached.

Comment by Justin Azoff at 2016-09-29T17:42:07.258-0500:

Thanks to the wonders of the afl-tmin 'test case minimizer' tool, that found the smallest possible test case that crashes:

root@bro-manager:~/crashes# xxd minified.bro
00000000: 2fe8 2f                                  /./
root@bro-manager:~/crashes# bro --version
bro version 2.5-beta-debug
root@bro-manager:~/crashes# bro minified.bro
Segmentation fault

Comment by Justin Azoff at 2016-09-29T17:55:23.351-0500:

Also got it down to a one liner:

# bro misc/dump-events DumpEvents::include=/$'\xe8'/
Segmentation fault

(that include var was the first redeffable regex I could find)

(Base64 encoded) Attachment https://bro-tracker.atlassian.net/secure/attachment/18901/id_000031.bro:

AOzs7C8i/25uaXRsdGVxdWED6DtbLwB/7OxA/25pdGx/AA==

(Base64 encoded) Attachment https://bro-tracker.atlassian.net/secure/attachment/18902/minified.bro:

L+gv

Really prioritize sigaction() in setsignal() implementation

The setsignal() function in setsignal.c prioritizes sigaction() for setting signal handlers, but cmake/CheckFunctions.cmake will cause it to never be used in the case when sigset() is also available.

sigset() is considered obsolete and sigaction() should be prioritized, so should change that CMake script.

Maybe something to try in the 2.7 dev cycle.

known_* scripts rewrite for broker

Moved from https://bro-tracker.atlassian.net/browse/BIT-1521

Created by Justin Azoff at 2015-11-03T09:03:40.517-0600:

known services script does

        if ( ! addr_matches_host(id$resp_h, service_tracking) ||
             "ftp-data" in c$service || # don't include ftp data sessions
             ("DNS" in c$service && c$resp$size == 0) ) # for dns, require that the server talks.
                return;

but should probably also ignore gridftp-data. Probably a good idea to add a set of services that behave like ftp for it to check.

Comment by Jeannette Dopheide at 2016-01-07T13:42:21.914-0600:

Due 1/14

Comment by Justin Azoff at 2016-01-13T08:53:46.970-0600:

Hmm, this may be a little harder than I thought. c$service is a set, and bro doesn't have a bif for comparing one set to another set.. i can do it with a loop like this, but is there a better way?

function intersects(a: set[string], b: set[string]): bool{
	for (one in a) {
		if (one in b) {
			return T;
		}
	}
	return F;
}

even though c$service is a set, it should only have 1 or 2 strings in it, so it is not a large loop.

Comment by Seth Hall at 2016-01-14T08:37:46.000-0600:

Yeah, it would be great to be able to do set intersections in a performant way, but I think your assessment is right that this set should be short enough in all cases to go ahead and just do the loop here.

Comment by Justin Azoff at 2016-01-14T09:07:03.193-0600:

Hmm.. it seems a little odd to put the intersects function in the known-services script, is there a better place for it?

Comment by Seth Hall at 2016-01-14T09:13:08.239-0600:


Don’t put a set intersection function as a general utility function. We probably shouldn’t be offering that generally right now since it should be added to the language in a way that performs better. Since you’re only using it in a single location, just do a loop.

Comment by Justin Azoff at 2016-01-14T10:06:51.126-0600:

Gah. It looks like coming up with a test case for this will be a pain. The existing pcap doesn't get detected as gridftp-data since the transfer size is too small:

jazoff@Justins-MacBook-Air  /tmp/b  $ bro -r ~/src/bro/testing/btest//Traces/globus-url-copy.trace local base/protocols/ftp/gridftp  'Known::service_tracking=ALL_HOSTS'
WARNING: No Site::local_nets have been defined.  It's usually a good idea to define your local networks.
20000, 1

jazoff@Justins-MacBook-Air  /tmp/b  $ cat conn.log |bro-cut id.orig_h id.orig_p id.resp_h id.resp_p service
192.168.57.103	60108	192.168.57.101	2811	ssl,gridftp,ftp
192.168.57.103	35391	192.168.57.101	55968	ssl

jazoff@Justins-MacBook-Air  /tmp/b  $ cat known_services.log |bro-cut  host port_num service
192.168.57.101	2811	FTP
192.168.57.101	55968	SSL

Filtering the ephemeral ssl service running on port 55968 is what I am trying to accomplish here.

Comment by Johanna Amann at 2016-01-14T12:34:51.267-0600:

If only the testcase is a problem - couldn't you just redef GridFTP::size_threshold for the test?

Comment by Justin Azoff at 2016-01-14T13:45:37.537-0600:

Ah, yes that helped the protocol detection.. though I think it shows a bug in known services in general:

$ bro -r ~/src/bro/testing/btest//Traces/globus-url-copy.trace local base/protocols/ftp/gridftp  'Known::service_tracking=ALL_HOSTS' 'GridFTP::size_threshold=1'
WARNING: No Site::local_nets have been defined.  It's usually a good idea to define your local networks.

$ cat conn.log |bro-cut id.orig_h id.orig_p id.resp_h id.resp_p service192.168.57.103	60108	192.168.57.101	2811	gridftp,ssl,ftp
192.168.57.103	35391	192.168.57.101	55968	ssl,gridftp-data

$ cat known_services.log |bro-cut  host port_num service
192.168.57.101	2811	FTP
192.168.57.101	55968	SSL

Some of this is due to how it keeps track of services by ip,port. Since ssl is always detected first, that is the one that gets logged.

It looks even if it was changed to ip,port,service gridftp may not show up because it never makes it into known services.

The gridftp analyzer does

    add c$service["gridftp-data"];

But this doesn't trigger a protocol_confirmation (even though it would be too late anyway). since the (ip,port) would have been logged as ssl.

So, I think known-services:

  • Needs to keep track of things by (ip,port,service)
  • Should possibly wait until a connection is closed and it has all the facts before trying to log the service.

If I remove the protocol_confirmation event and use simply:

event connection_state_remove(c: connection) &priority=-5
    {
    known_services_done(c);
    }

It mostly works:

$ cat known_services.log |bro-cut  host port_num service
192.168.57.101	2811	FTP,SSL,gridftp
192.168.57.101	55968	gridftp-data,SSL

I'm not sure if it should log them once per line, and if we should do something about the mismatch in case.

Comment by Justin Azoff at 2016-02-17T11:13:51.009-0600:

topic/jazoff/ticket1521 contains a branch that I believe fixes most of the issues with known-services.

I think there may still be one outstanding bug (but it is something that is broken worse in the current code).

The current code tracks services by (addr,port). If no service is detected on a port it will log it as (ip, port, empty). If a service is later detected on that port, nothing will be logged.

This branch WILL log it, but it will also log twice in the opposite order, which is possibly not desired.

So, this will work and is an improvement:

ip, port, empty
# time passes
ip, port, HTTP

But it may also log

ip, port, HTTP
# time passes
ip, port, empty

To fix that it would need to keep track of a separate (ip, port) set that had a non empty service detected. Once something like HTTP was detected the (ip, port) would be added, and then it would skip logging (ip, port, empty)

Comment by Seth Hall at 2016-02-17T11:22:43.141-0600:

What if you change known_services to...

table[addr, port] of set[string]

Then you could avoid the case you laid out here, which I suspect in production would be pretty annoying.

Comment by Justin Azoff at 2016-02-17T14:19:35.172-0600:

Ok, I implemented that. I'm working on generating some pcaps for these test cases. One thing I think i ran into is I think this check (that was in the older version too) is not quite right:

# Handle the connection ending in case no protocol was ever detected.
event connection_state_remove(c: connection) &priority=-5
        {
        if (c$resp$state == TCP_ESTABLISHED )
                event known_services_done(c);
        }

It seems (in my testing) that TCP_CLOSED occurs much more often. I think this check intends to filter connections that completed a handshake, but is not really doing so. I honestly have no idea why a connection would be TCP_ESTABLISHED inside connection_state_removed.

This is a similar issue as in BIT-1535.. how to determine if a connection succeeded.

Vlad also mentioned that starting a timer for each protocol confirmation might not be the best idea. We could rely 100% on connection_state_removed if we did not mind delaying a known_service log entry until the connection closed. I think if I can fix that if statement, the timers could be removed and the entire thing could be greatly simplified.

Vlad also brought up something I was thinking about as well that c$service should probably be an ordered vector.. since now that I fixed the multiple-service-logging issue you see things like

FTP,SSL,gridftp
gridftp,FTP,SSL
SSL,gridftp,FTP

or

SSL,SMTP

but i believe chronologically they should all be

FTP,SSL,gridftp
SMTP,SSL

Comment by Adam Slagell at 2016-05-06T14:57:52.439-0500:

This is close enough we should push out the partial fix for 2.5

Comment by Robin Sommer at 2016-06-17T13:49:26.900-0500:

Will change quite a bit anyways with Broker integration.

Comment by Adam Slagell at 2016-06-22T10:14:12.701-0500:

Status Justin?

Comment by Justin Azoff at 2016-06-22T10:26:38.701-0500:

I believe we decided in the meeting last week that even though what I have is an improvement, since all of the known_* scripts need to be rewritten for broker it probably isn't worth changing a bunch of stuff for 2.5, only to then change it again for the next release. I will focus more on getting all the known_ scripts updated and fixing the bugs across the board.

Comment by Adam Slagell at 2016-06-22T10:37:34.730-0500:

Ah, I was not at that meeting. So are we doing anything for 2.5 here?

Comment by Adam Slagell at 2016-09-28T06:56:58.614-0500:

Justin, you fixed some of this for 2.5, but it seems like there is a broader issue than gridftp. Should this ticket be closed out, and a new one opened up for the 2.6 known_* script rewrite?

Comment by Justin Azoff at 2016-09-28T07:24:34.010-0500:

Yeah, all the known scripts will need a full rewrite once broker is integrated. I have some ideas on how to write a 'known' framework to make logging things once and only once easier.

Comment by Justin Azoff at 2017-02-21T12:53:46.306-0600:

It looks like that script is just broken. The main software script that logs new software versions does:

    ts[info$name] = info;
    Log::write(Software::LOG, info);

and then the version changes script is doing

    local old = ts[rec$name]

But at that point old and rec are the same exact thing. It's possible to fix this, it just can't use the log_software event because at that point the "old" version has already been overwritten.

Another issue with the script is that the 'tracked' variable has a create expire of only 24h, so if the server version is only seen every 48 hours, or if bro is restarted it won't know the version changed.

Newer features in Broker should allow interesting version changes to be tracked using persistent data stores. That would really fix the issue. There are similar things that need to be re-written for better tracking known hosts/known services/known certs.

On Feb 21, 2017, at 12:50 PM, fatema bannatwala <[email protected]> wrote:

I was going through the version-changes.bro script, thinking of adding some software
to track the version changes, but realized that there is no comparison done between the
old version tracked and the version detected in "rec: Info" of log_software event.

Hence, was thinking to add a condition to check it before the notice is raised for the version
change, like following:
( or I might be missing something regarding the functionality of the script. :/)

event log_software(rec: Info)
	{
	local ts = tracked[rec$host];
	
	if ( rec$name in ts )
		{
		local old = ts[rec$name];
	
		# Is it a potentially interesting version change?
		if ( rec$name in interesting_version_changes )
			{  
			   
			   if (software_fmt_version(old$version) != software_fmt_version(rec$version))
		             {	local msg = fmt("%.6f %s switched from %s to %s (%s)",
					network_time(), rec$software_type,
					software_fmt_version(old$version),
					software_fmt(rec), rec$software_type);
		    	    NOTICE([$note=Software_Version_Change, $src=rec$host,
			        $msg=msg, $sub=software_fmt(rec)]);
		             }
			}
		}
	}

Any thoughts? anybody using this script to track software changes?

Comment by Jon Siwek at 2018-08-14T11:08:21.264-0500:

I removed the 2.6 tag from this since it almost seems out of scope now, but still relevant later. i.e. first step of porting to Broker for 2.6 is done, but next step for a later release is to consider doing any of the changes that Justin was had worked on. Or let me know if there's strong opinion on still needing to do any changes for 2.6.

Comment by Justin Azoff at 2018-08-14T12:06:53.846-0500:

I don't know about the timeline, but known-services still needs fixing for multi-protocol services.

topic/jazoff/ticket1521 still has the pre-broker fixed known-services.bro along with a testing/btest/Traces/ssl-and-ssh-using-sslh.trace test pcap.

I didn't try changing c$service to a vector though, it's probably the right thing to do but that may break a bunch of other scripts.

It looks like you already fixed the software version changes script at some point as part of the broker work.

for-loops that iterate over key-value pairs

Moved from https://bro-tracker.atlassian.net/browse/BIT-1879

Created by Jon Siwek at 2017-12-08T13:45:51.144-0600:

Should investigate what it would take to have for-loops that iterate over keys and values simultaneously rather than just over keys. Since the value is already available in the core for free when iterating over table entries, it just makes it more convenient for the programmer and it also avoids the extra overhead of having to do an extra lookup from script-land in the cases where one needs the value.

Bro plugins should support a patch version (x.y.z)

Moved from https://bro-tracker.atlassian.net/browse/BIT-1985

Created by Jon Zeolla at 2018-09-06T09:01:40.307-0500:

Since the project and downstream projects such as bro-pkg seems to align with semver, plugins should be able to have a MAJOR.MINOR.PATCH version. I suggest adding an optional PATCH version.

A quick review pointed me to the following, but I'm sure there will be some downstream changes and btests to put together:

https://github.com/bro/bro/blob/b99be6458b3fe83284b5262ca0391bf3cc99f7c2/src/plugin/Plugin.h#L66-L82
https://github.com/bro/bro/blob/b99be6458b3fe83284b5262ca0391bf3cc99f7c2/doc/devel/plugins.rst
https://github.com/bro/bro/blob/b99be6458b3fe83284b5262ca0391bf3cc99f7c2/CMakeLists.txt#L61-L65
https://github.com/bro/bro/blob/b99be6458b3fe83284b5262ca0391bf3cc99f7c2/src/plugin/Plugin.cc#L442-L449

My apologies if this is a duplicate, did a quick search and couldn't find it but was on pretty slow Internet at the time.

Tricky config framework semantics

At least for new-comers to the framework it can be tricky to consistently trigger initialization/update code given a config 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);
}

Here, 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 they do take getting used to. Perhaps worth coming up with some design patterns or additional documentation. If we do want to change functionality, we could consider adding a mechanism that lets the user specify that they'd like to see the change handler triggered once at startup, and that they'd like to skip repeat invocations with the same value.

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.