feat: add -mx-resolve flag to resolve MX hosts to IPs in one pass (#972)#978
Open
ChrisJr404 wants to merge 3 commits into
Open
feat: add -mx-resolve flag to resolve MX hosts to IPs in one pass (#972)#978ChrisJr404 wants to merge 3 commits into
ChrisJr404 wants to merge 3 commits into
Conversation
…ojectdiscovery#972) When doing recon you usually want the IPs that back the MX hostnames, not just the hostnames themselves — and you don't want to have to pipe the output back into dnsx to get them. Add a new -mx-resolve / -mxr flag that runs the existing MX query then follows up with an A lookup for each returned MX hostname, deduping the resolved addresses across hosts. The IPs are emitted as a new "MX_IP" record type so existing -mx output is unchanged for users who don't pass the new flag, and -mx-resolve implies -mx so a single flag gets you everything. Hosts that fail to resolve (NXDOMAIN, transient error) are silently dropped — the MX line still lists the hostname so the gap is visible.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughAdds a new CLI option ChangesMX Hostname Resolution Feature
Sequence Diagram(s)sequenceDiagram
autonumber
participant Worker
participant DNSx as dnsx.Lookup
participant Output
Worker->>Worker: receive dnsData with MX hostnames
alt MXResolve enabled
Worker->>DNSx: rate-limited A lookup per MX hostname
DNSx-->>Worker: return IPs or error
Worker->>Worker: aggregate & deduplicate IPs
Worker->>Output: emit MX_IP records for resolved IPs
end
Worker->>Output: emit MX hostname records (unchanged)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/runner/options.go`:
- Line 134: The help text for the mx-resolve flag is inaccurate:
options.MXResolve description claims it resolves "A/AAAA" but the implementation
still uses dnsx.Lookup which only returns A records; update either the behavior
or the description. Either change the flag text passed to flagSet.BoolVarP to
say "resolve MX hostnames to A records (implies -mx)" to match dnsx.Lookup, or
modify the MX resolution code path to call a resolver that returns both A and
AAAA (e.g., replace dnsx.Lookup with a DNS API that returns IPv6 as well or add
dnsx.LookupAAAA/dnsx.LookupIP calls) so options.MXResolve truly returns A and
AAAA; make this change where options.MXResolve is consumed and where dnsx.Lookup
is invoked.
In `@internal/runner/runner.go`:
- Around line 812-815: resolveMXHosts currently performs per-hostname DNS
queries outside the request limiter, so -mx-resolve can exceed RPS and misreport
stats; update resolveMXHosts (or its caller) to execute all DNS lookups via the
existing r.limiter (e.g., acquire/release or submit lookups through the same
throttled lookup helper used elsewhere) so each MX hostname lookup is counted
and rate-limited, and ensure the same change is applied to the other
MX-resolution block referenced (the calls around lines 851-873) that use
r.options.MXResolve and r.outputRecordType.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5bd2fdb7-3b66-44cb-89be-548ffb1bdebd
📒 Files selected for processing (2)
internal/runner/options.gointernal/runner/runner.go
- resolveMXHosts now calls r.limiter.Take() before each follow-up A lookup so -mx-resolve respects -rl the same way the main worker does. - The flag's help text said 'A/AAAA records' but dnsx.Lookup only returns A records today; tightened to 'A records' so the help matches the implementation. AAAA support is a worthwhile follow-up but needs a new helper in libs/dnsx — out of scope for this PR.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/runner/runner.go (1)
779-816:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
MX_IPoutput is silently suppressed when-filter-typeis active.The
responseTypeFilterMapblock (lines 779–790) exits early withcontinuebefore the MX-resolve block at lines 812–816 is reached. As a result,dnsx -mx-resolve -filter-type mxwill emitMXhostnames but never the derivedMX_IPrecords.🛠️ Proposed fix
if len(r.options.responseTypeFilterMap) > 0 { r.outputRecordType(domain, dnsData.A, "A", dnsData.CDNName, dnsData.ASN) r.outputRecordType(domain, dnsData.AAAA, "AAAA", dnsData.CDNName, dnsData.ASN) r.outputRecordType(domain, dnsData.CNAME, "CNAME", dnsData.CDNName, dnsData.ASN) r.outputRecordType(domain, dnsData.MX, "MX", dnsData.CDNName, dnsData.ASN) + if r.options.MXResolve { + if mxIPs := r.resolveMXHosts(dnsData.MX); len(mxIPs) > 0 { + r.outputRecordType(domain, mxIPs, "MX_IP", dnsData.CDNName, dnsData.ASN) + } + } r.outputRecordType(domain, dnsData.NS, "NS", dnsData.CDNName, dnsData.ASN) ... continue }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/runner/runner.go` around lines 779 - 816, The responseTypeFilterMap branch exits early and therefore never performs MX host resolution for MX_IP; update the block that calls r.outputRecordType(..., "MX", ...) to also, when r.options.MXResolve is true, call r.resolveMXHosts(dnsData.MX) and emit the derived IPs via r.outputRecordType(domain, mxIPs, "MX_IP", dnsData.CDNName, dnsData.ASN) if any are returned (apply the same dedupe/length checks used in the MXResolve logic below); this keeps MX and MX_IP output consistent whether -filter-type is active or not.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/runner/runner.go`:
- Around line 857-875: The MX resolver is performing auxiliary A lookups via
r.dnsx.Lookup(host) in Runner.resolveMXHosts but never increments the stats
counters, so -mx-resolve -stats undercounts requests; update the code to call
the stats increment (the same method used in wildcard.go) immediately before
each r.dnsx.Lookup(host) so the live requests counter reflects these follow-up A
queries; also consider adding the resulting lookup count to the overall total
(the variable managed in prepareInput) or at minimum document that total is a
lower bound if you choose only to increment the live requests counter.
---
Outside diff comments:
In `@internal/runner/runner.go`:
- Around line 779-816: The responseTypeFilterMap branch exits early and
therefore never performs MX host resolution for MX_IP; update the block that
calls r.outputRecordType(..., "MX", ...) to also, when r.options.MXResolve is
true, call r.resolveMXHosts(dnsData.MX) and emit the derived IPs via
r.outputRecordType(domain, mxIPs, "MX_IP", dnsData.CDNName, dnsData.ASN) if any
are returned (apply the same dedupe/length checks used in the MXResolve logic
below); this keeps MX and MX_IP output consistent whether -filter-type is active
or not.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8c754f9a-c119-47fd-9640-ca6267addd5e
📒 Files selected for processing (2)
internal/runner/options.gointernal/runner/runner.go
✅ Files skipped from review due to trivial changes (1)
- internal/runner/options.go
… counter
CodeRabbit flagged two gaps in the -mx-resolve path:
1. The response-type-filter branch in worker() returned before reaching
the MX-resolve block, so 'dnsx -mx-resolve -rtf <type>' silently
dropped the derived MX_IP records. Mirror the unfiltered branch by
calling resolveMXHosts inside the filter branch as well.
2. resolveMXHosts performs auxiliary A-record lookups via r.dnsx.Lookup
but never incremented the live 'requests' counter, so '-mx-resolve
-stats' undercounted total queries. Bump the counter alongside each
additional lookup, matching the pattern in InputWorker.
Neo - PR Security ReviewCaution Neo couldn't finish analyzing this pull request during this run. Please run the review again. Comment |
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.
Closes #972.
Background
When doing recon you usually want the IPs that back each MX hostname, not just the hostnames themselves. The current workflow is to pipe
dnsx -mx's output back into anotherdnsx(ordig) call:That's a brittle two-step pipeline that breaks when MX outputs ever change format and forces an extra DNS round-trip per pass.
Change
Add a new
-mx-resolve/-mxrflag that runs the existing MX query then follows up with an A lookup for each returned MX hostname, deduping the resolved addresses across hosts. The IPs are emitted as a newMX_IPrecord type so existing-mxoutput is unchanged for users who don't pass the new flag, and-mx-resolveimplies-mxso a single flag gets you everything.Hosts that fail to resolve (NXDOMAIN, transient error, empty MX list) are silently dropped — the MX line still lists the hostname so the gap is visible to the operator.
Verification
go build ./...andgo vet ./internal/runner/both clean.Notes
+37 / 0lines across two files. No public API change. Setting-mx-resolvewithout-mxworks (we auto-enable-mx); setting both is a no-op of the implication.-respoutput. JSON output already serialises the underlyingdnsDataso MX hostnames remain there; the resolved IPs aren't yet wired into the JSON shape — happy to follow up with amx_ipJSON field if you'd like that as a separate PR (it's an additive schema change so worth its own discussion).-mxrbecause-mxis already taken; mirrors the-resp/-reshortening pattern.Summary by CodeRabbit