-
Notifications
You must be signed in to change notification settings - Fork 199
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
Fix wrong values in snmp_sysdescr.xml #286
Conversation
👍 LGTM |
Thanks for the contribution. We don't currently have anything else that uses |
As a note, I see that it is listed in https://github.com/rapid7/recog/blob/master/identifiers/hw_device.txt and that we have one occurrence of it's use; Lines 1316 to 1325 in c0deef7
|
well, that's incorrect. there is this one: Line 1316 in c2beb75
the specificity is not so important, but Monitoring is not the proper classification for HMI devices, while they are usually old Windows box |
I made a comment on that one about a split second before you did. Make sense, particularly if this is a firmware image and not stock Windows with some service running on it. This seems like it'd be a good time to had If it is correct I can follow this up with a PR to add them if needed. |
Sound like a good Idea. |
actually did some further research on these devices. turned out this is running WindowsCE! should fix that as well. LMK if you want me to push the change as well. e.g. here: Line 6322 in c2beb75
actually os.family correct mapping will be |
@nstylepro - Yes, if you could add these changes that'd be great. Thanks! |
review again :) |
@@ -6318,10 +6318,12 @@ Copyright (c) 1995-2005 by Cisco Systems | |||
<example>Siemens, SIMATIC HMI, KTP600 Basic Mono PN, 6AV6647-0AB11-3AX0, HW:1, FW:V01.06.00</example> | |||
<example>Siemens, SIMATIC HMI, KTP600 Basic color PN, 6AV6 647-0AD11-3AX0, HW:1, FW:V11.00.02.00</example> | |||
<param pos="0" name="os.vendor" value="Siemens"/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
os.vendor
here and elsewhere should be Microsoft.
hw.vendor
should, I'm guessing, be Siemens
?
Can you group the os.*
and hw.*
together?
<param pos="2" name="os.version"/> | ||
<param pos="0" name="os.family" value="Windows"/> | ||
<param pos="0" name="hw.family" value="Simatic S7"/> | ||
<param pos="1" name="hw.product"/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes from os.product
to hw.product
are what broke the tests. You can change the field in the example and retest using rspec
I'll land this and fix the issues. Thanks much for the feedback and pr @nstylepro |
FYI, here is a PR with the changes: #291 |
Changed wrong values of 2 fingerprints