search

sethhall / bro-mqtt

MQTT analyzer for Bro
BSD 3-Clause "New" or "Revised" License
4 stars 1 forks source link

Script directory included twice when installed with bro-pkg #2

Open lilyinstarlight opened 6 years ago

lilyinstarlight commented 6 years ago

Since this is a plugin that includes the 'scripts' directory, the scripts are already included by Bro at startup.[1] Setting script_dir in 'bro-pkg.meta' causes them to be included twice when installed via bro-pkg and Bro gives redefinition errors at https://paste.fooster.io/nqvvja.

The solution I found is to either unset the script_dir variable (which is valid[2]) or set it to scripts/Bro/MQTT to load the 'mqtt_notify.bro' script which is currently not loaded.

Following that second option reveals, however, that the mqtt_notify.bro script was not updated when the mqtt_connect event was updated and the script no longer runs without changing the parameters to use MQTT::ConnectMsg.

You can tell Bro includes the 'scripts' directory by default for plugins by the 'loaded_scripts.log' output at https://paste.fooster.io/yddhwb.

I would like to make a pull request which fixes these issues and loads the corrected mqtt_notify.bro script if you think that logic is sound and is something you want.

[1] https://github.com/bro/bro/blob/master/src/plugin/Manager.cc#L176 [2] https://bro-package-manager.readthedocs.io/en/stable/package.html#script-dir-field

Thanks and have a nice day!

sethhall commented 6 years ago

Arg! Feel free to make a pull request. You're totally right and I made this mistake once before too.