Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ENH: Shodan parser: handle invalid hostnames, move _common_keys out of class, adjust apply_mapping #2117

Merged
3 commits merged into from
Oct 29, 2021

Conversation

monoidic
Copy link
Contributor

@monoidic monoidic commented Sep 30, 2021

This PR makes some changes to the Shodan parser, namely:

  • Instead of keeping track of extra.ftp.<something>.parameters, FTP parameters are collected together into extra.ftp.features as a list of said features, reducing field count
  • rsync.modules is collected
  • conversion functions can raise NoValueException with a string argument to signify that the conversion would not succeed, such as in the case of a single IP address being given in hostnames, which would then be passed into source.reverse_dns and fail to validate as a FQDN
  • _common_keys is moved out of the class
  • _dict_dict_to_obj_list is introduced, for converting a string-to-dict mapping into a list of dicts with the previous key as an attribute of the dict; this can be useful for preventing issues where, when feeding the data into aggregating tools, you'd end up with many more fields than necessary, e.g vulns.CVE-2010-0001.cvss, CVE-2010-0002.cvss etc.
  • _get_first to get the first item from a list, with NoValueException raised on empty lists
  • _get_first_hostname to handle the first valid FQDN from a list of hostnames for hostnames in the Shodan banner, if there is one, and gives NoValueException otherwise
  • ssl.cert.serial and ssl.dhparams.generator, which may return both integers and strings, are converted to strings
  • Changes to apply_mapping, such as reducing needless loop iterations, removing a big try-except, and adding the NoValueException handling described above
  • Stops falsy values (False, 0) besides None from being filtered out

@monoidic monoidic marked this pull request as ready for review October 4, 2021 08:43
@codecov-commenter
Copy link

codecov-commenter commented Oct 11, 2021

Codecov Report

Merging #2117 (c812b4b) into develop (2da82cf) will increase coverage by 0.01%.
The diff coverage is 86.66%.

@@             Coverage Diff             @@
##           develop    #2117      +/-   ##
===========================================
+ Coverage    75.90%   75.92%   +0.01%     
===========================================
  Files          440      440              
  Lines        23573    23605      +32     
  Branches      3150     3151       +1     
===========================================
+ Hits         17894    17922      +28     
- Misses        4947     4952       +5     
+ Partials       732      731       -1     
Impacted Files Coverage Δ
intelmq/bots/parsers/shodan/parser.py 90.09% <86.20%> (-1.46%) ⬇️
intelmq/tests/bots/parsers/shodan/test_parser.py 100.00% <100.00%> (ø)

Copy link
Contributor

@waldbauer-certat waldbauer-certat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good for me

@ghost ghost added this to the 3.1.0 milestone Oct 29, 2021
@ghost ghost self-assigned this Oct 29, 2021
@ghost ghost added component: bots feature Indicates new feature requests or new features labels Oct 29, 2021
@ghost ghost merged commit d28c82b into certtools:develop Oct 29, 2021
@ghost
Copy link

ghost commented Oct 29, 2021

Thanks!

I add a changelog entry in 58b8423, heavily based on your PR description above.

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: bots feature Indicates new feature requests or new features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants