MISP / misp-modules

Modules for expansion services, enrichment, import and export in MISP and other tools.
http://misp.github.io/misp-modules
GNU Affero General Public License v3.0
345 stars 234 forks source link

Server -m option fails to load and run custom modules #489

Open chrisinmtown opened 3 years ago

chrisinmtown commented 3 years ago

This issue requests repair of misp-modules startup option -m.

We have developed a custom module that queries an internal proprietary system to enrich details of attributes on the MISP web UI. That module is in the python virtual environment used by misp-modules server, but not in the site-packages/misp_modules directory. I tried to launch misp-modules with a command line something like misp-modules -m misp.cymru but get this error:

AttributeError: module 'misp.cymru' has no attribute 'register'
chrisinmtown commented 3 years ago

The error happens in the code here:

https://github.com/MISP/misp-modules/blob/509e5ac979245693be63575ef3bf015aac80be23/misp_modules/__init__.py#L264

This line tries to call the register() function on the freshly loaded module. No existing module defines a register function, so I don't know how to write it. I think this code is wrong. I see that the development case (args.devel) handles args.m totally differently starting here:

https://github.com/MISP/misp-modules/blob/509e5ac979245693be63575ef3bf015aac80be23/misp_modules/__init__.py#L244

However that branch of code sets no value for global loaded_modules so I don't think it works either :(

I'm trying to figure out the 'moduletype' value that is set by this code and what it should be. I watched the code process existing modules and it seems the type string is just 'modules', if I'm reading this correctly:

% python3 -m pdb misp_modules/__init__.py  
> /Users/me/git/github/MISP.misp-modules/misp_modules/__init__.py(22)<module>()
-> import os
(Pdb) b 140
Breakpoint 1 at /Users/me/git/github/MISP.misp-modules/misp_modules/__init__.py:140
(Pdb) c
No module named '__main__.modules'; '__main__' is not a package
No module named '__main__.helpers'; '__main__' is not a package
2021-04-28 14:09:50,331 - misp-modules - INFO - Launch MISP modules server from current directory.
2021-04-28 14:09:50,355 - misp-modules - INFO - Helpers loaded cache.py 
2021-04-28 14:09:50,361 - misp-modules - INFO - MISP modules cisco_firesight_manager_ACL_rule_export imported
> /Users/me/git/github/MISP.misp-modules/misp_modules/__init__.py(140)load_modules()
-> mhandlers['type:' + modulename] = moduletype
(Pdb) p modulename, moduletype
('cisco_firesight_manager_ACL_rule_export', 'modules')

I suspect moduletype is supposed to be one of expansion, import, export; but that does not seem to be happening. To be fair, I'm probably invoking the misp-modules system incorrectly, please correct me.

chrisinmtown commented 3 years ago

Here's a change I propose to make option -m load & run custom modules that are outside the misp_modules package. I expect the get_module_type_name function needs to be extended tho. Please check it carefully.

diff --git a/misp_modules/__init__.py b/misp_modules/__init__.py
index 21e3db3..28f3efe 100644
--- a/misp_modules/__init__.py
+++ b/misp_modules/__init__.py
@@ -218,6 +218,20 @@ def _launch_from_current_dir():
     return load_modules(modulesdir)

+def get_module_type_name(module: str) -> tuple:
+    """
+    Return tuple with module type, module name
+    by splitting the qualified name on '.'
+
+    :param module: Dotted name like misp.foo.bar
+    """
+    if '.' in module:
+        splitted = module.split(".")
+        return (splitted[-2], splitted[-1])
+    else:
+        return (module, module)
+
+
 def main():
     global mhandlers
     global loaded_modules
@@ -236,17 +250,15 @@ def main():
     listen = args.l
     if args.devel:
         log = init_logger(level=True)
-        log.info('Launch MISP modules server in developement mode. Enable debug, load a list of modules is -m is used.')
+        log.info('Launch MISP modules server in development mode. Enable debug, load a list of modules if -m is used.')
         if args.m:
             mhandlers = {}
-            modules = []
+            loaded_modules = []
             for module in args.m:
-                splitted = module.split(".")
-                modulename = splitted[-1]
-                moduletype = splitted[2]
+                (moduletype, modulename) = get_module_type_name(module)
                 mhandlers[modulename] = importlib.import_module(module)
                 mhandlers['type:' + modulename] = moduletype
-                modules.append(modulename)
+                loaded_modules.append(modulename)
                 log.info('MISP modules {0} imported'.format(modulename))
         else:
             mhandlers, loaded_modules = _launch_from_current_dir()
@@ -260,8 +272,11 @@ def main():
             mhandlers, loaded_modules = _launch_from_current_dir()

         for module in args.m:
-            mispmod = importlib.import_module(module)
-            mispmod.register(mhandlers, loaded_modules)
+            (moduletype, modulename) = get_module_type_name(module)
+            mhandlers[modulename] = importlib.import_module(module)
+            mhandlers['type:' + modulename] = moduletype
+            loaded_modules.append(modulename)
+            log.info('MISP modules {0} imported'.format(modulename))

     service = [(r'/modules', ListModules), (r'/query', QueryModule)]
github-germ commented 3 years ago

@chrisinmtown -- I tested your version of __init__.py with MISP 2.4.141, and it worked for me. I placed my custom expansion enrichment module script in .../site-packages/mypkg/mymod.py and started with misp-modules -s -m mypkg.mymod and the module was discovered with enrichment working in the WebUI.

One other small change, this usual import in a module script:

Let's hope that a misp-module expert can contribute that missing piece soon as I'm ready to use this nice improvement. Thanks!

adulau commented 3 years ago

That's a cool idea. I'll have a look and update misp-modules. Thanks a lot for the idea and contribution.

chrisinmtown commented 3 years ago

Hi @adulau glad you'll find time for this. I cannot take credit for a new idea here, just trying to use an existing misp-modules feature. Please let me know if I understood the -m option correctly and if my proposal needs more work.

chrisinmtown commented 3 years ago

good day Alexandre @adulau please let me know if it would help move this along if I get approval & submit a PR with the proposed change. We'd very much like to have this solution in the main branch. Thanks.

adulau commented 3 years ago

Good news. Monday we release an updated version of misp-modules along with MISP. Then I can work to integrate your request in the next release.

chrisinmtown commented 3 years ago

Glad to hear it. Please let me know how I can help. If my proposed change meets your intent, I will work on getting approval to submit a pull request. If my change does not meet your design goals, please advise how to improve it.

github-germ commented 1 year ago

I guess this was never accomplished?

adulau commented 1 year ago

Oops need to check if I ever pushed the branch with the test code.

github-germ commented 1 year ago

@adulau HNY'23... checking back on old issues. Close as fixed or wontdo? Thx.