feat: discover hostnames from TLS certificate SNI/CN/SAN data - #51
Open
bandrel wants to merge 10 commits into
Open
feat: discover hostnames from TLS certificate SNI/CN/SAN data#51bandrel wants to merge 10 commits into
bandrel wants to merge 10 commits into
Conversation
…certificate scanning
docs/ is not tracked on main, and these two files carry absolute local paths from the machine they were written on, same as 9ea2bb3. Add a docs/ entry to .gitignore this time so the pattern can't recur. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The TLS certificate hostname discovery bullet claimed the merged ip_hostname_map.json benefits "a future --resume". It doesn't: main() calls preprocess_targets() unconditionally, including on resume, and that rewrites ip_hostname_map.json from the target file alone with no merge of the existing file's contents, wiping out the cert-derived entries. A resume run's behavior is still correct because it re-derives the same cert hostnames from the still-cached nse_results/*.xml — re-derivation, not persistence — and that's deliberate: a persisted cert-derived hostname reaching create_hostname_target_file() could send nmap after a name-resolved address different from the one actually in scope (a commonName off a shared/CDN cert can resolve elsewhere entirely). Also documents that the new finding was added to _PER_HOST_DETAIL_TITLES, without which findings.txt would collapse the group with no per-host detail. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
_merge_ssl_cert_hostnames() builds the persisted ip_to_hostname map
that later feeds create_hostname_target_file(), unlike the display-only
paths (generate_findings(), _scan_extra_sql_ports()) that CLAUDE.md's
IPv4-only doctrine allows a find('address') fallback for. Remove that
fallback here so a dual-stacked host's IPv6 or MAC string can never
become a key in the map, closing the last uncovered branch in the
function and aligning it with the existing doctrine in one move.
Also add a test covering the `if not fname.endswith('.xml'): continue`
guard, which is load-bearing in production (nse_results/ legitimately
holds .coverage sidecars and .failed quarantine files) but was
previously untested. And update a stale comment in
_write_findings_txt to mention the new TLS certificate hostname list
case in _PER_HOST_DETAIL_TITLES.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
TLS services frequently reveal hostnames the operator never typed into the target file — via the certificate's
commonNameandsubjectAltName(SAN). SpooNMAP already runs thessl-certNSE script on common TLS ports for External scans (used today only to flag expired certs); this adds no new scanning, just parses output already collected.What changed
_extract_ssl_cert_hostnames()parsesssl-certoutput for the certificate'scommonName(anchored to theSubject:line, neverIssuer:) and eachSubject Alternative Name: DNS:entry — deduped, order-preserved, wildcards included._merge_ssl_cert_hostnames()walksnse_results/*.xmlafter the NSE script pass and fills gaps in the operator hostname map with the first non-wildcard name per host — never overwriting an operator-supplied hostname from the target file — then rewritesdiscovery/ip_hostname_map.json. Runs before both_aggregate_result_dir()andgenerate_findings()sospoonmap_output.*and the findings report reflect it for the current run.LOW-severity finding,TLS Certificate Hostname(s) Identified, listing every name found (wildcards included, since they're informative even though unused for targeting).ip_to_hostnamemap is not persisted forward across--resumeby design:preprocess_targets()rewritesip_hostname_map.jsonfrom the target file alone on every run. A resume re-derives the same cert hostnames from cachednse_results/*.xmlinstead. This is deliberate — a cert-derived hostname reachingcreate_hostname_target_file()could send nmap after a name-resolved address different from the one actually in scope (e.g. a shared/CDN certificate), so nothing carries the merged map into the next run's targeting.Verification
1208 passed, 5 skipped(skips are the documented environmental/root-gated ones) · 100% coverage onspoonmap.py(2542 stmts, 0 miss; floor is 95%) ·ruff check spoonmap.py tests/clean.Built via subagent-driven development: 4 tasks (extraction helper → merge/wiring → finding → docs), each with an independent spec-compliance + code-quality review; one fix round in Task 2 (a bare
.attrib['id']subscript that violated the file's defensive-parsing convention). A final whole-branch review then caught and a single fix wave closed:docs/despite an explicit prior decision (Fix broken wheel/sdist packaging, and harden CI (timeouts, Dependabot, root-only NSE job, workflow lint) #42-era) to keep them out of this public repo--resume(see the design note above)find('address')fallback in the new map-building code) was removed rather than tested, since a fallback there could let a dual-stacked host's IPv6/MAC string become a hostname-map key — this repo's IPv4-only doctrine sanctions that fallback only for display paths, not map-building ones.Docs
CLAUDE.md gains a "TLS certificate hostname discovery" bullet under Key Implementation Details, matching the file's existing dense, rationale-first documentation style.
🤖 Generated with Claude Code