Fix DNS CAA check for IP scans and subdomains#3041
Conversation
|
Thanks! Always appreciated to take action. There are a few points which need to be addressed:
IP addresses can appear in SAN, so it is not possible to just remove this. Famous examples are 1.1.1.1 or 8.8.8.8 , RFC https://www.rfc-editor.org/info/rfc5280/#section-4.2.1.6 is much older. I am tending to rather leaving like it is (preferred) or a check whether the certificate contains an IPv4 or IPv6 address in the SAN. In general though I believe it is a user error of the tester: if someone uses the IP address for testing, one should expect at least the certificate section to contain false output. If there are couple to virtual hosts, you're just testing the wrong host.
You have a point here. (works for me though, not sure whether it's me DNS setup that doesn't add the local domain). Maybe it's better to move L10282 before to L10279.
Let me sleep over that. Is the question how far the tool should help beginners. If this finds it's way into the dev version, it should be not phrased as long, fileout() is missing, it's at the wrong place and there's an additional CR I don't understand
|
|
Really appreciate the feedback! My COO always says "Read the RFC"🥲😂 On your first point, I slept on it and did some deep diving and I've come to fully agree with you there. My solution is a bit too blunt. I'll look at deferring back. For the trailing dot, that's a really good catch. I didn't think of that. I'll update that later today. Completely fair, as part of the update I'll shorten the message,use fileout(), fix the placement and remove the extra CR, oversight on my end there. 🙃 |
- Skip CAA lookup entirely when NODE is an IP address; show "not checked (IP address scan)" instead of spuriously querying IP octets as domain labels and reporting "not offered" - Force FQDN (trailing dot) on the initial caa_node before the walk loop so dig does not apply the resolv.conf search domain to the first query, which could return a false result - Add a visible warning in the scan header when scanning by IP address, noting that trust/CAA and other domain-specific checks may be unreliable and the user should rescan with the hostname
|
I believe I've addressed everything, tested against 6 different targets and am happy with the results. Did an amendment rather than submitting a new PR.
|

For context, I'm a penetration tester and a few folks at the company have pointed out CAA false positives in our reports because of testssl. Also IP scans not returning 100% reliable output. Decided to have a go at fixes and submit this PR :)
Following changes made:
Describe your changes
Please refer to an issue here or describe the change thoroughly in your PR.
What is your pull request about?
If it's a code change please check the boxes which are applicable
help()