Skip to content

Fix DNS CAA check for IP scans and subdomains#3041

Open
grhza wants to merge 1 commit into
testssl:3.3devfrom
grhza:3.3dev
Open

Fix DNS CAA check for IP scans and subdomains#3041
grhza wants to merge 1 commit into
testssl:3.3devfrom
grhza:3.3dev

Conversation

@grhza
Copy link
Copy Markdown

@grhza grhza commented May 21, 2026

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:

  • 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

Describe your changes

Please refer to an issue here or describe the change thoroughly in your PR.

What is your pull request about?

  • [ X] Bug fix
  • [X ] Improvement
  • New feature (adds functionality)
  • Breaking change (bug fix, feature or improvement that would cause existing functionality to not work as expected)
  • Typo fix
  • Documentation update
  • Update of other files

If it's a code change please check the boxes which are applicable

  • [X ] For the main program: My edits contain no tabs, indentation is five spaces and any line endings do not contain any blank chars
  • [X ] I've read CONTRIBUTING.md and Coding_Convention.md
  • [X ] I have tested this fix or improvement against >=2 hosts and I couldn't spot a problem
  • I have tested this new feature against >=2 hosts which show this feature and >=2 host which does not (in order to avoid side effects) . I couldn't spot a problem
  • For the new feature I have made corresponding changes to the documentation and / or to help()
  • If it's a bigger change: I added myself to CREDITS.md (alphabetical order) and the change to CHANGELOG.md

@drwetter
Copy link
Copy Markdown
Collaborator

Thanks! Always appreciated to take action. There are a few points which need to be addressed:

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"

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.

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

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.

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

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

image

@grhza
Copy link
Copy Markdown
Author

grhza commented May 24, 2026

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
@grhza
Copy link
Copy Markdown
Author

grhza commented May 25, 2026

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.

  • CAA / IP: Left the existing IP early-exit as-is per your preference

  • FQDN trailing dot: Kept the pre-loop enforcement added before the walk and removed the now-redundant in-loop duplicate functionally equivalent to moving it before the get_caa_rr_record call, just slightly cleaner.

  • IP warning: Shortened the message, added fileout(), removed the stray outln, and moved the block above the rDNS line so it renders on its own line without the extra CR

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.

2 participants