search

armbues / ioc_parser

Tool to extract indicators of compromise from security reports in PDF format
MIT License
428 stars 171 forks source link

Added support for plain text from stdin, fixed namespace #3

Closed JesseBowling closed 9 years ago

JesseBowling commented 9 years ago

Thanks for your code. I have a need to extract IOC from plain text, so hacked up something to read stdin and parse it. I moved the default patterns.ini to ~/.patterns.ini. I also moved the argument parsing into main and removed the use of globals.

Feel free to use or reject as you see fit!

buffer commented 9 years ago

A couple of questions.

  1. I do not see the point of moving patterns.ini in the user home directory. What's the reason behind it?
  2. parse_stdin almost duplicates parse_file. I think creating a generic method called by both methods should be easier to maintain (and will make the code cleaner). Thoughts?
JesseBowling commented 9 years ago
  1. Personal preference; I prefer to have configuration files located somewhere standard so that regardless of where the script is located one can execute it without having to specify the patterns location. I.e., if ioc_parser.py is located in /usr/local/bin and I'm executing it from /case/bulkextract/, I don't have to add "-p /usr/local/bin/patterns.ini"....Pure commandline laziness. Making this default means I can still override if there's a need to use a different pattern file.
  2. Absolutely. This was a first stab, done to POC a problem I want to solve (which is actually extracting from email).

How about we reject this pull request, and I can start over with some smaller commits; thus I could submit the namespace changes in one, moving the patterns.ini in one, and rework the stdin format to really be one for "handle plain text files" that makes more use of the existing function.

Worth it to make a develop branch?

buffer commented 9 years ago

A nice trick exists in Python if you want to avoid situations like the one you describe in 1. I use it really frequently.

ini_file = os.path.join(os.path.dirname(os.path.abspath(file)), 'patterns.ini')

(file is not correctly visualized here so please read it as underscore-underscore file underscore-underscore)

and the path problem magically disappears.

Agree with your approach of rejecting the pull request and work on more granular ones. Armin?

armbues commented 9 years ago

I was going to reject because the default pattern location in home is not for everybody and would've complicated the installation process for all users. The trick using file sounds like a good idea.