rapid7 / recog

Pattern recognition for hosts, services, and content
Other
662 stars 195 forks source link

Scripted update to add missing example attributes #400

Closed dabdine closed 2 years ago

dabdine commented 2 years ago

Description

Adds missing example attributes to <example> elements in recog fingerprint XML files, greatly reducing recog_verify warnings.

This is only round 1. Subsequent rounds will target some hand modifications to target fingerprints missing examples altogether. Additionally, there is one warning that is not fixed with this patch due to it being an edge case: The fingerprint regex conditionally matches a version, but the only example does not supply one. That will be fixed by hand as well in a subsequent PR.

The script that was used to make these changes is here: https://gist.github.com/dabdine/6c641407e805ab33530ec67459a94c3a

Motivation and Context

Clean up warnings so that more strict enforcement of quality can eventually be turned on.

How Has This Been Tested?

Ran recog_verify

Types of changes

Checklist:

dabdine commented 2 years ago

Fails for golang, works for C/JRuby & Java. The failing cases are because of captured junk text in a service version. I'll tweak the regex to see if I can get the behavior to work consistently across go/ruby/Java.

dabdine commented 2 years ago

Regarding the comments about bugs above, I'm simply noting them for some hand mods later. I don't intend to fix those as part of this (automated) patch

mkienow-r7 commented 2 years ago

Regarding the comments about bugs above, I'm simply noting them for some hand mods later. I don't intend to fix those as part of this (automated) patch

Do you intend to make those fixes or is that an exercise left to the reader? :)

Is there a reason for not making those corrections as part of this PR or a PR against your PR branch?

dabdine commented 2 years ago

@mkienow-r7 i'm open to suggestions! I was going to fix some of these today assuming this patch lands. I think it'd be better to isolate the hand-crafted fixes from this patch though and do it post-merge

mkienow-r7 commented 2 years ago

@dabdine It seems I had things a little mixed up from the verify workflow failure the other day. I agree, and I'm fine with collaborative post-merge corrections.

mkienow-r7 commented 2 years ago

It's really nice to see the warning count drop dramatically. Thank you for the contribution @dabdine!