Updates from RumbleDiscovery/recog fork

pbarry25 commented 2 years ago

Description

Updates from the RumbleDiscovery/recog fork. Mostly fingerprints, with addition of the ability to use filesystem-based examples for long examples.

Motivation and Context

Looking to merge RumbleDiscovery/recog fork updates back to R7's repo, for community benefit.

How Has This Been Tested?

recog_match, recog_verify, rake spec as we went
rake tests prior to PR creation here

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)

Checklist:

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

tsellers-r7 commented 2 years ago

@pbarry25 - Can you please

Delete the contents of all txt files in identifiers/
Rebuild them from the XML fingerprints by running ruby bin/recog_standardize -w
Run ruby bin/recog_cleanup

That will help remove some of the extra entries in the identifiers files as well as remove some of the end of file line changes.

Thanks much.

EDIT: Since step 1 is more work than it really should be, I opened an issue to make it a little easier: https://github.com/rapid7/recog/issues/384

pbarry25 commented 2 years ago

Appreciate the feedback, @tsellers-r7 and @mkienow-r7! I'll circle back to it this weekend, for sure.

I did rebuild the identifiers/*.txt files, @tsellers-r7, and also ran the cleanup on the XML files.

tsellers-r7 commented 2 years ago

@pbarry25 - Thanks for doing that. You may also want to run the CPE update script.

https://github.com/rapid7/recog/blob/master/CONTRIBUTING.md#updating-cpes

That will allow you to see which CPEs are stripped out of the current fingerprints and determine you want to make adjustments so that they are generated.

Sometimes they can't be generated if either the vendor or product don't exist in the NIST database.

if you aren't attached to them or you think there are many CPE changes unrelated to your PR we can run the cleanup after this is landed.

CC @mkienow-r7

tsellers-r7 commented 2 years ago

@pbarry25 - I think I'm done with the first review pass. Thanks tons for putting this up. Big PRs are always.. fun but we will get it sorted out.

mkienow-r7 commented 2 years ago

I found an issue in recog_verify with the external example support and opened RumbleDiscovery/recog#5 to resolve it.

pbarry25 commented 2 years ago

Thanks again for the thorough review and patience, @tsellers-r7 and @mkienow-r7! 🙇 I'm new to the NIST/CPE logic/process here and I don't want to mess that up for the related feedback, I'll try to get that done and make a final push to this PR today.

EDIT 11/11/2021: was able to run update_cpes.py locally yesterday (also surfaced a 'duplicate' value error I had missed, nice!), am reconciling the change-set from that with our fork and the remaining one or two feedbacks.

pbarry25 commented 2 years ago

Hey @tsellers-r7 and @mkienow-r7, I pushed what I believe is all the things addressed outside of the discussed follow-on PR (couple of items) and a cpe remap for phoenixcontact. I believe we'll need to make a code update to our side before fully dropping the <vendor>.serial_number completely, but I added hw.serial_number alongside the vendor one (when I noticed it) for now. This all passed rake spec and rake tests and the recog_verify gauntlet for me, whew! Happy to hear any additional feedback y'all have. Thank you!

mkienow-r7 commented 2 years ago

The Java Verify failure is unrelated to your changes. I merged a recog-java enhancement today that adds base64 example support. I have some small fingerprint updates that need to be merged.

pbarry25 commented 2 years ago

TYVM, @tsellers-r7!

rapid7 / recog