Cdn check with domain and with dns response#377
Cdn check with domain and with dns response#377Mzack9999 merged 6 commits intoprojectdiscovery:devfrom
Conversation
echo www.gap.com | ./dnsx -cdn -json | jq
_ __ __
__| | _ __ ___ \ \/ /
/ _' || '_ \ / __| \ /
| (_| || | | |\__ \ / \
\__,_||_| |_||___//_/\_\
projectdiscovery.io
[INF] Current dnsx version 1.1.4 (latest)
{
"host": "www.gap.com",
"ttl": 2795,
"resolver": [
"1.0.0.1:53"
],
"a": [
"104.104.158.228"
],
"cname": [
"www.gap.com.edgekey.net",
"e12405.x.akamaiedge.net"
],
"all": [
"www.gap.com.\t2795\tIN\tCNAME\twww.gap.com.edgekey.net.",
"www.gap.com.edgekey.net.\t20795\tIN\tCNAME\te12405.x.akamaiedge.net.",
"e12405.x.akamaiedge.net.\t20\tIN\tA\t104.104.158.228",
"\n;; OPT PSEUDOSECTION:\n; EDNS: version 0; flags:; udp: 1232"
],
"status_code": "NOERROR",
"timestamp": "2023-05-19T13:24:09.8923071-03:00",
"cdn": true,
"cdn-name": "akamai"
} |
Mzack9999
left a comment
Mzack9999
left a comment
There was a problem hiding this comment.
@brenocss This is an excellent idea. The data in the response might not contain A or AAAA records since the user defines the question types. Probably we need a hybrid approach:
- If
A|AAAArecords are available, then we check the response - Otherwise, we proceed with the previous direct check
What do you think?
|
This fixes a bug as well where a site use multi-cdn's. The first DNS query will pickup one CDN response, while that second call to |
|
@Mzack9999 did you find this use case? |
|
merge conflict |
Neo - PR Security ReviewNo security issues found Highlights
Comment |
|
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)
WalkthroughThe change reuses existing DNS response data for CDN detection when Changes
Sequence DiagramsequenceDiagram
participant Runner as Runner
participant DNSX as DNSX
participant CDN as CDN Detector
participant DNS as DNS Resolver
Runner->>DNSX: QueryMultiple(domain)
DNSX->>DNS: Resolve DNS records
DNS-->>DNSX: Return dnsData (A/AAAA/CNAME or not)
DNSX-->>Runner: Return dnsData
alt dnsData has A/AAAA/CNAME
Runner->>DNSX: CdnCheckRespData(dnsData)
DNSX->>CDN: CheckDNSResponse(dnsData)
CDN-->>DNSX: Return IsCDNIP, CDNName, CDNType
DNSX-->>Runner: CDN detection result
else dnsData lacks relevant records
Runner->>DNSX: CdnCheck(domain)
DNSX->>CDN: Perform fresh CDN probe (IP-based)
CDN-->>DNSX: Return IsCDNIP, CDNName, CDNType
DNSX-->>Runner: CDN detection result (set dnsData.CDNType="cdn" if IsCDNIP)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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. 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: 1
🧹 Nitpick comments (1)
internal/runner/runner.go (1)
685-688: Consider logging CDN check errors for debugging.Both CDN check paths discard errors with
_. While CDN detection is non-critical, logging failures at debug level would help troubleshoot issues when CDN information is unexpectedly missing.💡 Suggested improvement
// reuse existing DNS response to avoid redundant lookups and ensure // consistency between reported records and CDN detection - dnsData.IsCDNIP, dnsData.CDNName, dnsData.CDNType, _ = r.dnsx.CdnCheckRespData(dnsData.DNSData) + var err error + dnsData.IsCDNIP, dnsData.CDNName, dnsData.CDNType, err = r.dnsx.CdnCheckRespData(dnsData.DNSData) + if err != nil { + gologger.Debug().Msgf("CDN check failed for %s: %v", domain, err) + } } else { // fall back to a fresh lookup when the response lacks A/AAAA/CNAME records - dnsData.IsCDNIP, dnsData.CDNName, _ = r.dnsx.CdnCheck(domain) + var err error + dnsData.IsCDNIP, dnsData.CDNName, err = r.dnsx.CdnCheck(domain) + if err != nil { + gologger.Debug().Msgf("CDN check failed for %s: %v", domain, err) + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/runner/runner.go` around lines 685 - 688, The CDN check calls drop errors; update the two calls to CdnCheckRespData and CdnCheck to capture the error and emit a debug-level log including the domain, the function name (CdnCheckRespData or CdnCheck), and the error (e.g., using r.logger.Debugf("CDN check %s failed for %s: %v", "CdnCheckRespData", domain, err)). Keep the existing assignment to dnsData.IsCDNIP/dnsData.CDNName/dnsData.CDNType but ensure you don’t change return handling beyond logging the captured err.
🤖 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 682-689: The fallback path leaves dnsData.CDNType unset because
CdnCheck returns (bool, string, error) while CdnCheckRespData returns the CDN
type; update CdnCheck to return the CDN type as a third return value (e.g.,
(bool, string, string, error)), update the caller in runner.go to assign
dnsData.IsCDNIP, dnsData.CDNName, dnsData.CDNType from r.dnsx.CdnCheck(domain),
and propagate the signature change to all other CdnCheck callers so CDNType is
consistently populated; alternatively, if changing the signature is undesirable,
explicitly set dnsData.CDNType to a sensible default (e.g., empty string or
"ip-only") in the fallback branch to avoid leaving it unset.
---
Nitpick comments:
In `@internal/runner/runner.go`:
- Around line 685-688: The CDN check calls drop errors; update the two calls to
CdnCheckRespData and CdnCheck to capture the error and emit a debug-level log
including the domain, the function name (CdnCheckRespData or CdnCheck), and the
error (e.g., using r.logger.Debugf("CDN check %s failed for %s: %v",
"CdnCheckRespData", domain, err)). Keep the existing assignment to
dnsData.IsCDNIP/dnsData.CDNName/dnsData.CDNType but ensure you don’t change
return handling beyond logging the captured err.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a062524f-6f7a-46a8-b716-aa9f1c525bab
📒 Files selected for processing (4)
dnsxinternal/runner/runner.golibs/dnsx/cdn.golibs/dnsx/dnsx.go
5 participants
I have modified the cdncheck function to accept domains However, I believe it is better to use the second function that was created, which takes a dnsResponse as input. This reduces the number of DNS requests.
Summary by CodeRabbit