Skip to content

Add recog verify tool#9

Merged
mkienow-r7 merged 21 commits intorapid7:masterfrom
mkienow-r7:feature/add-recog-verify
Oct 6, 2021
Merged

Add recog verify tool#9
mkienow-r7 merged 21 commits intorapid7:masterfrom
mkienow-r7:feature/add-recog-verify

Conversation

@mkienow-r7
Copy link
Copy Markdown
Contributor

@mkienow-r7 mkienow-r7 commented Sep 21, 2021

Description

Adds a recog fingerprint verify tool that is similar to recog_verify found in the Ruby implementation.

Other notable changes:

  • A potential breaking change for clients is the RecogMatcher.getExamples method no longer returns a Set<String>, but a Set<FingerprintExample>. The change was made to encapsulate details from the fingerprint file that were not currently stored in the object model.
  • Moved interpolate from the RecogMatchers.getMatches and RecogMatchers.getFirstMatch methods down to RecogMatcher.match.
  • Interpolation is applied to all parameter rather than being limited only to those that end with the CPE suffix .cpe23
  • Interpolation does not strip trailing colon character anymore
  • Created Maven modules for recog and recog-verify

Motivation and Context

These changes are proposed to facilitate the comparison of fingerprint content behavior between recog language implementations.

How Has This Been Tested?

  • New unit tests
  • mvn integration-test
  • Comparing the output of the Ruby recog_verify to com.rapid7.recog.verify.RecogVerifier. Of the 49 XML fingerprint files from rapid7/recog only 3 did not produce the same summary counts and that was a result of the java implementation not currently supporting base64 encoding. The three files with different summary counts are: ldap_searchresult.xml, snmp_sysdescr.xml, telnet_banners.xml.
    • ./bin/recog_verify -f d xml/http_servers.xml | tail -n 1
      SUMMARY: Test completed with 633 successful, 151 warnings, and 0 failures
      
    • mvn install
      mvn --projects recog-verify exec:java -Dexec.mainClass="com.rapid7.recog.verify.RecogVerifier" -Dexec.args="-f d xml/http_servers.xml" | tail -n 1
      SLF4J: Failed to load class "org.slf4j.impl.StaticLoggerBinder".
      SLF4J: Defaulting to no-operation (NOP) logger implementation
      SLF4J: See http://www.slf4j.org/codes.html#StaticLoggerBinder for further details.
      SUMMARY: Test completed with 633 successful, 151 warnings, and 0 failures
      

Types of changes

  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • I have updated the documentation accordingly (or changes are not required).
  • I have added tests to cover my changes (or new tests are not required).
  • All new and existing tests passed.

@mkienow-r7 mkienow-r7 force-pushed the feature/add-recog-verify branch from 794240e to a821cc2 Compare September 21, 2021 20:48
@mkienow-r7 mkienow-r7 marked this pull request as ready for review September 23, 2021 04:34
@mkienow-r7 mkienow-r7 force-pushed the feature/add-recog-verify branch from 844cfa4 to 64ba370 Compare September 27, 2021 16:39
@gschneider-r7
Copy link
Copy Markdown
Contributor

LGTM. 👍

Some considerations for post-merge:

  • When releasing a new version with this change, the version should bump to 0.8.0 or 1.0.0 and then continue to follow SemVer. This is due to the backwards-incompatible change on Examples.
  • Since the verifier is meant to run standalone, this functionality should probably be extracted to another module to reduce dependencies for consumers. That's probably not worth the effort right now, though; Perhaps save it for a 1.0.0 release. (i.e. make this into a maven multi-module project so consumers can select what modules to pull in - not to be confused with Java 9+ Module system).

gschneider-r7
gschneider-r7 previously approved these changes Sep 28, 2021
gschneider-r7
gschneider-r7 previously approved these changes Oct 5, 2021
Copy link
Copy Markdown
Contributor

@gschneider-r7 gschneider-r7 left a comment

Choose a reason for hiding this comment

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

LGTM 👍 A few more suggestions, but can be done separately.

  • Bump r7.recog.content.version to latest to ensure the integration test still passes on latest recog content
  • Fix the build status badge on the readme to use github actions (see adding-a-workflow-status-badge )

@mkienow-r7
Copy link
Copy Markdown
Contributor Author

LGTM 👍 A few more suggestions, but can be done separately.

  • Bump r7.recog.content.version to latest to ensure the integration test still passes on latest recog content
  • Fix the build status badge on the readme to use github actions (see adding-a-workflow-status-badge )

The integration test passed locally with the latest recog content version 2.3.21.

[INFO] --- maven-antrun-plugin:1.7:run (recog-download) @ recog-java ---
...
      [get] Getting: http://production.cf.rubygems.org/gems/recog-2.3.21.gem
...
[INFO] --- maven-failsafe-plugin:2.22.0:integration-test (default) @ recog-java ---
[INFO]
[INFO] -------------------------------------------------------
[INFO]  T E S T S
[INFO] -------------------------------------------------------
[INFO] Running com.rapid7.recog.RecogIntegration
[INFO] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.491 s - in com.rapid7.recog.RecogIntegration
[INFO]
[INFO] Results:
[INFO]
[INFO] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0

Copy link
Copy Markdown

@ekelly-1898 ekelly-1898 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 to me, we are not using any of the breaking changes in the Scan Engine so this should be easy enough to integrate.

@mkienow-r7 mkienow-r7 merged commit 1e89ea0 into rapid7:master Oct 6, 2021
@mkienow-r7 mkienow-r7 deleted the feature/add-recog-verify branch October 6, 2021 13:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants