Geo Location Plugin

Hello All,

I recently have been looking at how Bro's plugins work. I was able to
recreate the rot13 functionality from the online documentation and
wanted to do something a little more challenging in hopes of learning
more about plugin development.

On a suggestion from Seth, I've been attempting to move the geoIP
stuff, which is currently baked into the core, into a self contained
plugin. I've gotten Bro and my plugin to compile together (mostly by
simply moving existing code into their proper files within
aux/bro-aux/plugin-support/Geo). However, I cannot seem to find the
plugin listed in 'bro -NN' and I cannot use the data types or
functions my bif defines. I'm hoping someone has a suggestion or a tip
on troubleshooting the interaction between plugins and the core. Many
thanks,

-AK

If you haven't yet, try compiling Bro with --enable-debug (and then
recompiling your plugin as well). Then run with "-B plugins" and see
if debug.log says anything helpful.

Robin

I can at least now see why my plugin isn’t working as excepted. Thanks Robin.

-AK

Great, let me know if it's anything that the plugin documentation
could make more clear.

Robin

Thanks Robin. Everything on the “Writing Bro Plugins” page is clear although the debugging section seems thin to me. Then again, I’ve have never compiled Bro with debugging enabled before.

I’m running into issues with my plugin’s shared object. Bro is complaining about an undefined symbol (likely due to a bug in my source). Are there plans to expand the Types section of that page? I think I’m not declaring a scriptland record type correctly.

-AK

Hi:

A mismatched function definition in .h / implementation in .cc usually causes this issue for me. Maybe try running the mangled symbol that's reported as missing through c++filt to see what it really is (if you haven't already) and use nm or the like to make sure the plugin library file contains that symbol?

-Gilbert

I ran into a similar issue recently with the approxidate plugin I've been working on. Jon reminded me that I need to do this...

#ifdef __cplusplus
extern "C" {
#endif
unsigned long approxidate_relative(const char *date, const struct timeval *tv);
#ifdef __cplusplus
}
#endif

  .Seth

Thanks Robin. Everything on the "Writing Bro Plugins" page is clear
although the debugging section seems thin to me. Then again, I've have
never compiled Bro with debugging enabled before.

Ok, I'll try to extend that a bit more, it's written mainly from the
perspective of adding debugging output to your plugin; not in terms of
leveraging the existing output to see why something's not working.

Are there plans to expand the Types section of that page?

I would like to, but it's quite a bit of work to fill in the TODOs, so
honestly I don't see that coming very soon. The problem is that most
of the missing TODOs are actually not really a matter per se of
writing a plugin or not, but require text on how to write the
corresponding code for Bro in general (in other words, even before
moving to plugins, much of that would have been quite similar). For
now, the best way to figure this out is to look at existing code,
i.e., for the builtin-in stuff other *.bif files.

I think I'm not declaring a scriptland record type correctly.

Feel free to post error messages here.

Robin

Thanks all. After moving declarations around, nm shows all my expected
symbols to be defined. I'm now receiving "internal error in
/usr/local/bro/share/bro/base/init-bare.bro, line 1: internal type
geo_location missing". It seems this error is being caused by the
check on the return value of lookup_ID() done within the
internal_type() function in Var.cc. From what I can tell, I don't have

geo_location = internal_type("geo_location")->AsRecordType();

in the right location. This line is from the init_net_var() function
from NetVar.cc, which gets called by main.cc. I thought it might be a
clash in name/module spaces and tried using the init-plugin script
with some unique values, but I still receive the same error. I'm not
sure if it matters but I when I run "bro -NNb" I can see my inactive
dynamic plugin, so I know Bro is aware of it. I currently have
everything sitting in a single .bif file. Would it be useful to post
that?

-AK

From what I can tell, I don't have

geo_location = internal_type("geo_location")->AsRecordType();

in the right location. This line is from the init_net_var() function
from NetVar.cc, which gets called by main.cc.

Maybe that can just be completely removed if Bro proper no longer relies on that type since all the related functionality is now provided by the plugin?

everything sitting in a single .bif file. Would it be useful to post
that?

How about putting the entire plugin source directory in a github repo? That should make it easy for others to start poking at the same code as you.

- Jon

Agreed. Any code related to GeoIP should move into the plugin, including types.

  .Seth

I moved that line from NetVar to my plugin source. I also moved some things from bifs to the plugin source (honestly, just copied and pasted). Posting just the plugin directory won’t show which lines in the core I moved to the plugin. Should I make a branch off master? What are the general conventions for doing this?

-AK

Yes, you'd make a branch of master and make your changes there and build your plugin against that branch. When your plugin is merged, it will also require a merge of your branch against Bro as well.

  .Seth

Oh, also: https://www.bro.org/development/howtos/process.html

  .Seth

One question here: are we prepated to ask people to install a plugin
if they want geoip functionality? I'm asking because while generally I
think it's the right way going forward to move external dependencies
into plugins, this one we have offered for a while so, in some way,
not having it out of the box is a regression (unless we decide to
build (some) plugins by default, which currently isn't happening).

Robin

I think that is reasonable for some things.

I think having some plugins distributed and built with Bro is reasonable, too. As long as functionality and interfaces don’t change current users shouldn’t notice a difference.
How eventually moving the plugin to a package manager will affect users is something else to consider.

-AK

+1 that core plugins be built / distributed with bro.

Re: package / plugin management, coming from a Java EE background, one of the things I don’t like about some of the OSGi software (read: highly modular, plugin-driven) I’ve worked with is how confusing it can get to track versions of each specific bundle / plugin to know how / when to update them. Binding a specific set of core plugin versions to application releases mitigates some of that complexity in my experience. In other words, I’d be a bit hesitant to agree that bro should individually package core plugins and offer them independently through a repo, just because it will probably require a great deal of care to update those (since there could be e.g. side effects / dependencies that aren’t immediately obvious).

An external repository / package manager seems like a good fit for non-core plugins, though.

-Gilbert

I think having some plugins distributed and built with Bro is reasonable, too. As long as functionality and interfaces don’t change current users shouldn’t notice a difference.
How eventually moving the plugin to a package manager will affect users is something else to consider.

-AK

That makes sense. The next logical question would be, what makes a plugin special enough to be included the core? I think GeoIP, being legacy like Robin mentioned, qualifies.

-AK

Yep. This is going to just remain as-always a careful balance between providing extra functionality and keeping the number of dependencies low for Bro.

  .Seth