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

recog_standardize: handle missing files, detect removed identifiers, unify device type tracking #438

Merged
merged 7 commits into from
Apr 8, 2022

Conversation

TomSellers
Copy link
Contributor

@TomSellers TomSellers commented Apr 5, 2022

Description

This PR fixes #384 and makes a few other changes.

Changes

  1. Adds error handling so that if a particular identifiers file is missing the script will return an empty string so that the contents can be populated from the existing fingerprint database corpus.

  2. Adds logic to detect and notify the user when values have been removed from an indicators list. This is useful when performing cleanups or widespread changes of values. No flag is required to enable this behavior, it works just like the previous detection of new values.

  3. Unifies *.device identifiers in indicators/device.txt and removes the existing os.device and hw.device files. This has already helped identify a few issues which I have addressed.

Example output after deleting 3 valid entries from the device.txt and adding one bonus entry of Sample Device Type Thingy.

$ ruby bin/recog_standardize xml/*.xml -w
DEVICE REMOVED VALUE: Sample Device Type Thingy
DEVICE NEW VALUE: DSL Modem
DEVICE NEW VALUE: DSLAM
DEVICE NEW VALUE: DSU/CSU
$ echo $?                                
1

$ ruby bin/recog_standardize xml/*.xml   
$ echo $?                             
0

How Has This Been Tested?

ruby bin/recog_standardize xml/*.xml and reviewing the changes

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

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 self-assigned this Apr 5, 2022
@mkienow-r7
Copy link
Contributor

What do you think about changes that would be compatible with the recog_standardize use in tools/dev/hooks/pre-commit? The new reset flag won't provide a focused alert for the user.

@TomSellers TomSellers changed the title recog_standardize: handle missing files, allow indicator reset recog_standardize: handle missing files, detect removed identifiers, unify device type tracking Apr 6, 2022
@TomSellers
Copy link
Contributor Author

@mkienow-r7 - I've reworked this PR so that it should work just fine with the commit hook. Now detection of values that need to be removed is handled on every run and a console line which includes the text REMOVED VALUE is emitted. I've reworked this description.

@@ -5672,11 +5672,11 @@ Copyright (c) 1995-2005 by Cisco Systems
=======================================================================-->

<fingerprint pattern="^PARADYNE T1 DSU/CSU; model: ([^;]+); S/W Release: ([^;]+);">
<description>Paradyne CSU/DSU</description>
<description>Paradyne DSU/CSU</description>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here and below. Simple typo as can been seen in the example. Changed to align with correct usage elsewhere.

<param pos="0" name="os.device" value="HiPath"/>
<param pos="0" name="os.family" value="WAP"/>
<param pos="0" name="os.family" value="HiPath"/>
<param pos="0" name="os.device" value="WAP"/>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was a typo that switched the values between the family and device params.

@TomSellers
Copy link
Contributor Author

@mkienow-r7
I went ahead and merged upstream/main into this PR to ensure that there were no issues.
Glad to see master -> main, thanks for doing that.

Copy link
Contributor

@mkienow-r7 mkienow-r7 left a comment

Choose a reason for hiding this comment

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

Nice work improving recog_standardize and cleaning up the identifiers. Thank you for the contribution @TomSellers!

@mkienow-r7 mkienow-r7 merged commit 8c58d72 into rapid7:main Apr 8, 2022
mkienow-r7 added a commit to mkienow-r7/recog-ruby that referenced this pull request Apr 15, 2022
@todb-r7 todb-r7 mentioned this pull request Dec 9, 2022
3 tasks
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.

recog_standard crashes when identifier files are missing.
2 participants