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

TLS issuer and subject fingerprints #225

Merged
merged 36 commits into from
Apr 10, 2019
Merged

TLS issuer and subject fingerprints #225

merged 36 commits into from
Apr 10, 2019

Conversation

hdm
Copy link
Contributor

@hdm hdm commented Apr 4, 2019

This is a first run at fingerprints for TLS issuer and subject fields. I plan to keep updating this for a while, but would appreciate any feedback on the current state. Coverage-wise, this is about 50% of the systems that I am trying to match, but that doesn't include any Sonar scans.

@hdm
Copy link
Contributor Author

hdm commented Apr 4, 2019

I should have the rspec failures sorted out later today.

@tsellers-r7
Copy link
Contributor

Thanks HD. I'll try to take a look at this later today. @ me to catch my attention earlier

@hdm
Copy link
Contributor Author

hdm commented Apr 4, 2019

@tsellers-r7 thanks! it should be good for a first review

</fingerprint>


<!-- iDRAC with a MAC -->
Copy link
Contributor

Choose a reason for hiding this comment

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

for these (and perhaps other fingerprint groups) it would be better to put this differentiation in the description field as it will help more readily identify the difference between two fingerprints that would otherwise just be described as "Dell iDRAC Remote Access Controller"

<param pos="0" name="hw.vendor" value="Dell"/>
<param pos="0" name="hw.product" value="iDRAC"/>
<param pos="1" name="dell.service_tag"/>
</fingerprint>
Copy link
Contributor

Choose a reason for hiding this comment

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

minor, here and elsewhere there is some inconsistent indentation.

<param pos="0" name="os.product" value="Android" />
<param pos="0" name="hw.device" value="Media Server" />
<param pos="0" name="hw.vendor" value="ASUS" />
<param pos="0" name="hw.product" value="Nexus Player" />
Copy link
Contributor

Choose a reason for hiding this comment

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

minor, but the style regarding spaces before the close of a tag seems to vary in this file. I'd suggest a xmllint --format

<param pos="0" name="hw.vendor" value="APC" />
</fingerprint>

<fingerprint pattern="^CN=Temporary CA [a-fA-F0-9]{8}\-[a-fA-F0-9]{4}\-[a-fA-F0-9]{4}\-[a-fA-F0-9]{4}\-[a-fA-F0-9]{12},OU=Temporary CA">
Copy link
Contributor

Choose a reason for hiding this comment

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

Anchor at end with $?

xml/x509_issuers.xml Outdated Show resolved Hide resolved
@jhart-r7 jhart-r7 requested a review from tsellers-r7 April 5, 2019 21:12
@hdm
Copy link
Contributor Author

hdm commented Apr 6, 2019

The feedback provided so far should be resolved, thanks!

<param pos="0" name="os.vendor" value="VMWare"/>
<param pos="0" name="os.product" value="ESX"/>
<param pos="0" name="os.device" value="Hypervisor"/>
<param pos="0" name="os.cpe23" value="cpe:/o:vmware:esx:-"/>
Copy link
Contributor

Choose a reason for hiding this comment

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

Any indication that this is ESX cpe:/o:vmware:esx:- and not ESXi cpe:/o:vmware:esxi:-?

As a note, once this lands I'll try to add the ESXi host from my lab.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These were indeed ESXi not ESX, updating.

@tsellers-r7
Copy link
Contributor

@hdm - Unless you have anything you'd like to add to this before we land this I expect we will land this and release this afternoon.

@hdm
Copy link
Contributor Author

hdm commented Apr 10, 2019

Excellent, thanks! I have piles of more fingerprints, but happy to let this one land first.

@tsellers-r7
Copy link
Contributor

Up to you

@hdm
Copy link
Contributor Author

hdm commented Apr 10, 2019

Probably good to have this land first and then keep rolling things up for a bigger future PR (following any standardization changes were making, etc).

Copy link
Contributor

@jhart-r7 jhart-r7 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. Thank you again for the contribution!

@jhart-r7 jhart-r7 self-assigned this Apr 10, 2019
@jhart-r7 jhart-r7 merged commit 7962dbf into rapid7:master Apr 10, 2019
@hdm hdm deleted the feature/tls branch April 2, 2020 21:08
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.

3 participants