Follow CNAMES on all lookups by default #397

phillip-stephens · 2024-07-16T15:43:38Z

Resolves #347

Description

Bring the logic from ALookups module into all iterative lookups.
Add a flag to disable this behavior: --follow-cnames=false

Testing

I found 3 domains that use CNAMES. I couldn't add a CNAME chain to zdns-testing.com for repeatable tests because Google Cloud "packs" the CNAME's + A records into a single answer to reduce round trips on resolution:

$ dig -t A cname-stop1.zdns-testing.com            
...
;; ANSWER SECTION:
cname-stop1.zdns-testing.com. 300 IN    CNAME   cname-stop2.zdns-testing.com.
cname-stop2.zdns-testing.com. 300 IN    CNAME   cname-stop3.zdns-testing.com.
cname-stop3.zdns-testing.com. 300 IN    CNAME   zdns-testing.com.
zdns-testing.com.       300     IN      A       3.4.5.6
zdns-testing.com.       300     IN      A       1.2.3.4
zdns-testing.com.       300     IN      A       2.3.4.5

Therefore, testing with Google Cloud doesn't work.

echo "www.wwno.org\nwww.wander-plus.com\nwww.microjpm.com" | ./zdns A --iterative

main Repo - Note how only CNAMES are returned, no A records

{"data":{"answers":[{"answer":"trvlbuddy.myshopify.com.","class":"IN","name":"www.wander-plus.com","ttl":3600,"type":"CNAME"}],"protocol":"udp","resolver":"97.74.103.44:53"},"name":"www.wander-plus.com","status":"NOERROR","timestamp":"2024-07-16T15:43:02Z"}
{"data":{"answers":[{"answer":"grove1.prod.npr.psdops.com.","class":"IN","name":"www.wwno.org","ttl":1800,"type":"CNAME"}],"protocol":"udp","resolver":"64.125.77.13:53"},"name":"www.wwno.org","status":"NOERROR","timestamp":"2024-07-16T15:43:02Z"}
{"data":{"answers":[{"answer":"microjpm.webnode.page.","class":"IN","name":"www.microjpm.com","ttl":3600,"type":"CNAME"}],"protocol":"udp","resolver":"144.217.207.158:53"},"name":"www.microjpm.com","status":"NOERROR","timestamp":"2024-07-16T15:43:03Z"}

Phillip/347-cnames

$ echo "www.wwno.org\nwww.wander-plus.com\nwww.microjpm.com" | ./zdns A --iterative
{"data":{"answers":[{"answer":"trvlbuddy.myshopify.com.","class":"IN","name":"www.wander-plus.com","ttl":3600,"type":"CNAME"},{"answer":"shops.myshopify.com.","class":"IN","name":"trvlbuddy.myshopify.com","ttl":3600,"type":"CNAME"},{"answer":"23.227.38.74","class":"IN","name":"shops.myshopify.com","ttl":3600,"type":"A"}],"protocol":"udp","resolver":"162.159.24.4:53"},"name":"www.wander-plus.com","status":"NOERROR","timestamp":"2024-07-26T15:33:10-04:00"}
{"data":{"answers":[{"answer":"microjpm.webnode.page.","class":"IN","name":"www.microjpm.com","ttl":3600,"type":"CNAME"},{"answer":"projects-lb.webnode.io.","class":"IN","name":"microjpm.webnode.page","ttl":360,"type":"CNAME"},{"answer":"3.79.173.192","class":"IN","name":"projects-lb.webnode.io","ttl":60,"type":"A"},{"answer":"18.185.25.67","class":"IN","name":"projects-lb.webnode.io","ttl":60,"type":"A"}],"protocol":"udp","resolver":"205.251.193.149:53"},"name":"www.microjpm.com","status":"NOERROR","timestamp":"2024-07-26T15:33:10-04:00"}
{"data":{"answers":[{"answer":"grove1.prod.npr.psdops.com.","class":"IN","name":"www.wwno.org","ttl":1800,"type":"CNAME"},{"answer":"d3qldz0dq4fdfs.cloudfront.net.","class":"IN","name":"grove1.prod.npr.psdops.com","ttl":300,"type":"CNAME"},{"answer":"18.238.25.33","class":"IN","name":"d3qldz0dq4fdfs.cloudfront.net","ttl":60,"type":"A"},{"answer":"18.238.25.94","class":"IN","name":"d3qldz0dq4fdfs.cloudfront.net","ttl":60,"type":"A"},{"answer":"18.238.25.107","class":"IN","name":"d3qldz0dq4fdfs.cloudfront.net","ttl":60,"type":"A"},{"answer":"18.238.25.114","class":"IN","name":"d3qldz0dq4fdfs.cloudfront.net","ttl":60,"type":"A"}],"protocol":"udp","resolver":"205.251.197.83:53"},"name":"www.wwno.org","status":"NOERROR","timestamp":"2024-07-26T15:33:11-04:00"}

CNAME with External Resolver

Most recursive resolvers will handle CNAMES for us:

echo "www.wander-plus.com" | ./zdns A | jq -c 
{"data":{"additionals":[{"flags":"","type":"EDNS0","udpsize":4096,"version":0}],"answers":[{"answer":"trvlbuddy.myshopify.com.","class":"IN","name":"www.wander-plus.com","ttl":3569,"type":"CNAME"},{"answer":"shops.myshopify.com.","class":"IN","name":"trvlbuddy.myshopify.com","ttl":3569,"type":"CNAME"},{"answer":"23.227.38.74","class":"IN","name":"shops.myshopify.com","ttl":2087,"type":"A"}],"protocol":"udp","resolver":"192.168.1.1:53"},"name":"www.wander-plus.com","status":"NOERROR","timestamp":"2024-07-26T15:35:07-04:00"}

But to prove that we're handling the CNAME traversal, we can point ZDNS to an authoritative nameserver for wander-plus and watch the logs:

$ echo "www.wander-plus.com" | ./zdns A --name-servers="97.74.103.44" --verbosity=5 | jq -c
INFO[0000] using local addresses: [192.168.1.243]       
INFO[0000] for non-iterative lookups, using nameservers: 97.74.103.44:53 
INFO[0000] no name server provided for external lookup, using  random external name server: 97.74.103.44:53 
DEBU[0000] DEPTH 00:[MIEKG-IN: starting a C/DNAME following lookup for  www.wander-plus.com  ( 1 )] 
DEBU[0000] DEPTH 01:    [MIEKG-IN: following external lookup for  www.wander-plus.com  ( 1 )] 
DEBU[0000] DEPTH 01:    [****WIRE LOOKUP***  A   www.wander-plus.com   97.74.103.44:53] 
DEBU[0000] DEPTH 01:    [MIEKG-OUT: following external lookup for  www.wander-plus.com  ( 1 ) with  1  attempts: status:  NOERROR  , err:  <nil>] 
DEBU[0000] DEPTH 01:    [MIEKG-IN: following external lookup for  trvlbuddy.myshopify.com  ( 1 )] 
DEBU[0000] DEPTH 01:    [****WIRE LOOKUP***  A   trvlbuddy.myshopify.com   97.74.103.44:53] 
DEBU[0000] DEPTH 01:    [MIEKG-OUT: following external lookup for  trvlbuddy.myshopify.com  ( 1 ) with  1  attempts: status:  REFUSED  , err:  <nil>] 
{"data":{"additionals":[{"flags":"","type":"EDNS0","udpsize":1232,"version":0}],"answers":[{"answer":"trvlbuddy.myshopify.com.","class":"IN","name":"www.wander-plus.com","ttl":3600,"type":"CNAME"}],"protocol":"udp","resolver":"97.74.103.44:53"},"name":"www.wander-plus.com","status":"NOERROR","timestamp":"2024-07-26T15:41:16-04:00"}

We can see that when the authoritative name server returned the CNAME, we attempted to follow it. Unfortunately, that nameserver isn't authoritative for the domain pointed to by the CNAME, but it shows the logic works. You can also see we return the last good status/result, similar to dig.

$ dig -t A www.wander-plus.com @97.74.103.44 
...
;; QUESTION SECTION:
;www.wander-plus.com.           IN      A

;; ANSWER SECTION:
www.wander-plus.com.    3600    IN      CNAME   trvlbuddy.myshopify.com.

DNAME Testing

See integration test

Turns out that DNS nameservers will perform CNAME Synthesis with DNAMES (for backwards compatibility for devices that don't support it) and our coreDNS resolver at esrg.stanford.edu is no different.

$ echo "a.zdns-dname.esrg.stanford.edu" | ./zdns A 
{"data":{"additionals":[{"flags":"","type":"EDNS0","udpsize":1232,"version":0}],"answers":[{"answer":"zdns-testing.com.","class":"IN","name":"zdns-dname.esrg.stanford.edu","ttl":86400,"type":"DNAME"},{"answer":"a.zdns-testing.com.","class":"IN","name":"a.zdns-dname.esrg.stanford.edu","ttl":86400,"type":"CNAME"},{"answer":"21.9.87.65","class":"IN","name":"a.zdns-testing.com","ttl":300,"type":"A"}],"protocol":"udp","resolver":"1.1.1.1:53"},"name":"a.zdns-dname.esrg.stanford.edu","status":"NOERROR","timestamp":"2024-07-22T17:41:26-04:00"}

~/zdns on  phillip/347-cnames! ⌚ 17:41:26
$ echo "a.zdns-dname.esrg.stanford.edu" | ./zdns A --iterative
{"data":{"answers":[{"answer":"zdns-testing.com.","class":"IN","name":"zdns-dname.esrg.stanford.edu","ttl":86400,"type":"DNAME"},{"answer":"a.zdns-testing.com.","class":"IN","name":"a.zdns-dname.esrg.stanford.edu","ttl":86400,"type":"CNAME"},{"answer":"21.9.87.65","class":"IN","name":"a.zdns-testing.com","ttl":300,"type":"A"}],"protocol":"udp","resolver":"216.239.32.108:53"},"name":"a.zdns-dname.esrg.stanford.edu","status":"NOERROR","timestamp":"2024-07-22T17:41:43-04:00"}

To test the DNAME following, I inserted code to drop all CNAME answers manually. In this way, we'd only get the correct output if DNAMES were being followed.

$ echo "a.zdns-dname.esrg.stanford.edu" | ./zdns A --iterative
{"data":{"answers":[{"answer":"zdns-testing.com.","class":"IN","name":"zdns-dname.esrg.stanford.edu","ttl":86400,"type":"DNAME"},{"answer":"21.9.87.65","class":"IN","name":"a.zdns-testing.com","ttl":300,"type":"A"}],"protocol":"udp","resolver":"216.239.32.108:53"},"name":"a.zdns-dname.esrg.stanford.edu","status":"NOERROR","timestamp":"2024-07-22T17:45:23-04:00"}

I couldn't think of a reliable way to automate this testing, it doesn't seem that CoreDNS supports turning off CNAME Synthesis for DNAME's.

External Lookups

Here we can see when doing an external lookup against dns-01.esrg.stanford.edu, DNAMEs are being followed.

$ echo "a.zdns-dname.esrg.stanford.edu,171.67.68.25" | ./zdns A --verbosity 5 
INFO[0000] using local addresses: [10.216.70.2]         
INFO[0000] for non-iterative lookups, using nameservers: 1.1.1.1:53 
DEBU[0000] DEPTH 00:[MIEKG-IN: starting a following iterative lookup for  a.zdns-dname.esrg.stanford.edu  ( 1 )] 
DEBU[0000] DEPTH 01:    [MIEKG-IN: following external lookup for  a.zdns-dname.esrg.stanford.edu  ( 1 )] 
DEBU[0000] DEPTH 01:    [****WIRE LOOKUP***  A   a.zdns-dname.esrg.stanford.edu   171.67.68.25:53] 
DEBU[0000] DEPTH 01:    [MIEKG-OUT: following external lookup for  a.zdns-dname.esrg.stanford.edu  ( 1 ) with  1  attempts: status:  NOERROR  , err:  <nil>] 
DEBU[0000] DEPTH 01:    [MIEKG-IN: following external lookup for  a.zdns-testing.com  ( 1 )] 
DEBU[0000] DEPTH 01:    [****WIRE LOOKUP***  A   a.zdns-testing.com   171.67.68.25:53] 
DEBU[0000] DEPTH 01:    [MIEKG-OUT: following external lookup for  a.zdns-testing.com  ( 1 ) with  1  attempts: status:  SERVFAIL  , err:  <nil>] 
{"data":{"additionals":[{"flags":"","type":"EDNS0","udpsize":1232,"version":0}],"protocol":"udp","resolver":"171.67.68.25:53"},"name":"a.zdns-dname.esrg.stanford.edu","status":"SERVFAIL","timestamp":"2024-07-22T17:54:37-04:00"}

(We first lookup a.zdns-dname.esrg.stanford.edu, and then query for a.zdns-testing.com, which the name server isn't authoritative for but it tests we're following it correctly.

phillip-stephens · 2024-07-16T20:03:10Z

@zakird I hit the build-and-large-scan-test too many times so now it's timing out. Other than that, this is ready for review, passing the tests I can think of.

zakird · 2024-07-18T15:22:30Z

src/cli/cli.go

@@ -159,6 +160,7 @@ func init() {
 	rootCmd.PersistentFlags().BoolVar(&GC.CheckingDisabled, "checking-disabled", false, "Sends DNS packets with the CD bit set")
 	rootCmd.PersistentFlags().BoolVar(&GC.RecycleSockets, "recycle-sockets", true, "Create long-lived unbound UDP socket for each thread at launch and reuse for all (UDP) queries")
 	rootCmd.PersistentFlags().BoolVar(&GC.NameServerMode, "name-server-mode", false, "Treats input as nameservers to query with a static query rather than queries to send to a static name server")
+	rootCmd.PersistentFlags().BoolVar(&GC.ShouldFollowCNAMEs, "follow-cnames", true, "Follow CNAMEs/DNAMEs in the iterative lookup process, not applicable to non-iterative lookups")


Why isn't this applicable to non-iterative lookups? Shouldn't we follow the name there too if that's what we get back? You could use alookup in non-iterative mode, so I think this is a step backwards in terms of functionality.

I was under the impression that in non-iterative mode we leave all resolution decisions up to the name server in question. We just return whatever it gives us.

But what you're describing sounds valid as well, and the user could figure out what queries we made (if we followed a CNAME) by looking at the trace, so this does seem like strictly better functionality.

The intent of the alookup module was that it did do that unlike the plain A lookup, which returns the raw response.

zakird · 2024-07-18T16:40:46Z

src/cli/worker_manager.go

@@ -172,6 +176,7 @@ func populateResolverConfig(gc *CLIConf, flags *pflag.FlagSet) *zdns.ResolverCon
 	// copy nameservers to resolver config
 	config.ExternalNameServers = gc.NameServers
 	config.LookupAllNameServers = gc.LookupAllNameServers
+	config.ShouldFollowCNAMEs = gc.ShouldFollowCNAMEs


This is named inconsistent with other options, would remove the "Should" prefix

zakird · 2024-07-18T16:42:00Z

While many servers do support CNAME Synthesis, I think it would be best to directly support DNAME's since synthesis was primarily designed for backwards compatibility and I think that researchers will want to know if they were actually just following a DNAME.

phillip-stephens · 2024-07-18T17:03:51Z

Sounds good, I'll add DNAME support and CNAME following on non-iterative queries!

phillip-stephens · 2024-07-22T21:55:10Z

@zakird When you have a chance, this is ready for review. I've updated the PR description, we're following DNAMEs now and I've tested external lookups.

phillip-stephens · 2024-07-22T21:57:21Z

I figure I'll wait to get rid of alookup until we have #353 done and can perform A and AAAA at the same time.

zakird · 2024-07-23T00:45:22Z

It looks like

    "protocol": "",
    "resolver": ""

Are empty in the new code, despite being there in the old one. Can you look into?

phillip-stephens · 2024-07-23T14:17:55Z

It looks like
    "protocol": "",
    "resolver": ""
Are empty in the new code, despite being there in the old one. Can you look into?

@zakird This definitely was the case with an older version of the code in this branch, but I believe I fixed it. Can you run git pull and try with latest since I can't reproduce with the latest code.

echo "cloudflare.com" | ./zdns --local-addr "171.67.71.210" --name-servers "1.1.1.1" A
{"data":{"additionals":[{"flags":"","type":"EDNS0","udpsize":1232,"version":0}],"answers":[{"answer":"104.16.133.229","class":"IN","name":"cloudflare.com","ttl":253,"type":"A"},{"answer":"104.16.132.229","class":"IN","name":"cloudflare.com","ttl":253,"type":"A"}],"protocol":"udp","resolver":"1.1.1.1:53"},"name":"cloudflare.com","status":"NOERROR","timestamp":"2024-07-23T14:17:15Z"}

$ echo "cloudflare.com" | ./zdns A --iterative --local-addr "171.67.71.210" --name-servers "1.1.1.1"
{"data":{"answers":[{"answer":"104.16.133.229","class":"IN","name":"cloudflare.com","ttl":300,"type":"A"},{"answer":"104.16.132.229","class":"IN","name":"cloudflare.com","ttl":300,"type":"A"}],"protocol":"udp","resolver":"162.159.0.33:53"},"name":"cloudflare.com","status":"NOERROR","timestamp":"2024-07-23T14:17:02Z"}

zakird · 2024-07-23T17:48:22Z

I'm not sure that I follow. This is what's in the body of the PR:

{"data":{"answers":[{"answer":"grove1.prod.npr.psdops.com.","class":"IN","name":"www.wwno.org","ttl":1800,"type":"CNAME"}],"protocol":"udp","resolver":"64.125.77.13:53"},"name":"www.wwno.org","status":"NOERROR","timestamp":"2024-07-16T15:43:02Z"}
{"data":{"answers":[{"answer":"microjpm.webnode.page.","class":"IN","name":"www.microjpm.com","ttl":3600,"type":"CNAME"}],"protocol":"udp","resolver":"144.217.207.158:53"},"name":"www.microjpm.com","status":"NOERROR","timestamp":"2024-07-16T15:43:03Z"}

which does have this? If mainline doesn't do this, I don't follow how we ended up with that output.

phillip-stephens · 2024-07-23T18:02:31Z

Ohhhh, thought you were seeing this behavior on the branch. You're correct, I forgot to update the PR description. Just updated!

phillip-stephens added 10 commits July 16, 2024 10:26

CNAME following works for test lookups, integration tests failing tho

3855a7c

fixed return type so integration tests pass

69b5e22

add comments and clean up

9f92473

add flag --follow-cnames

6bd2d4c

add DNAME integration test

314e6e7

add DNAME comments

a75b540

add verbosity to test GH runner

d97a074

remove verbosity flag for debugging

911d5f1

sigh...didn't add iterative flag to test

57396f9

mistakenly misnamed the zdns exe in the test

a65a415

phillip-stephens marked this pull request as ready for review July 16, 2024 20:02

phillip-stephens requested a review from a team as a code owner July 16, 2024 20:02

phillip-stephens requested a review from zakird July 16, 2024 20:02

zakird and others added 3 commits July 16, 2024 13:45

Merge branch 'main' into phillip/347-cnames

753f3eb

add cname loop handling

b9755a2

lint

f545f59

phillip-stephens assigned zakird Jul 18, 2024

zakird requested changes Jul 18, 2024

View reviewed changes

zakird reviewed Jul 18, 2024

View reviewed changes

phillip-stephens changed the title ~~Follow CNAMES on iterative lookups~~ Follow CNAMES on all lookups by default Jul 18, 2024

phillip-stephens added 7 commits July 22, 2024 10:18

Merge branch 'main' into phillip/347-cnames

0f77035

follow CNAMEs on external lookups too

154d587

fixed bugs

31c1235

handle dnames, untested

11b021b

fixed bugs with dname handling

6774e67

PR comments

4511213

Merge remote-tracking branch 'origin/main' into phillip/347-cnames

4c94d48

phillip-stephens requested a review from zakird July 23, 2024 14:34

Merge branch 'main' into phillip/347-cnames

40e4a0e

phillip-stephens added 8 commits July 25, 2024 17:01

Merge branch 'main' into phillip/347-cnames

a68cc45

removed guard code that prevented following cnames with external lookups

b083d1d

return last good status when following CNAMEs, like dig

59be41a

remove unneeded guard

016d1b4

return last good result/status if following cnames, fixes tests

2b1c35a

return last good status if we hit error traversing cnames

f8d9e32

Merge branch 'main' into phillip/347-cnames

95f83f8

Merge branch 'main' into phillip/347-cnames

52b271e

zakird approved these changes Jul 31, 2024

View reviewed changes

zakird merged commit 22c38f1 into main Jul 31, 2024
3 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Follow CNAMES on all lookups by default #397

Follow CNAMES on all lookups by default #397

phillip-stephens commented Jul 16, 2024 •

edited

Loading

phillip-stephens commented Jul 16, 2024

zakird Jul 18, 2024

phillip-stephens Jul 18, 2024

zakird Jul 18, 2024

zakird Jul 18, 2024

zakird commented Jul 18, 2024

phillip-stephens commented Jul 18, 2024

phillip-stephens commented Jul 22, 2024

phillip-stephens commented Jul 22, 2024

zakird commented Jul 23, 2024

phillip-stephens commented Jul 23, 2024 •

edited

Loading

zakird commented Jul 23, 2024 •

edited

Loading

phillip-stephens commented Jul 23, 2024

Follow CNAMES on all lookups by default #397

Follow CNAMES on all lookups by default #397

Conversation

phillip-stephens commented Jul 16, 2024 • edited Loading

Description

Testing

CNAME with External Resolver

DNAME Testing

External Lookups

phillip-stephens commented Jul 16, 2024

zakird Jul 18, 2024

Choose a reason for hiding this comment

phillip-stephens Jul 18, 2024

Choose a reason for hiding this comment

zakird Jul 18, 2024

Choose a reason for hiding this comment

zakird Jul 18, 2024

Choose a reason for hiding this comment

zakird commented Jul 18, 2024

phillip-stephens commented Jul 18, 2024

phillip-stephens commented Jul 22, 2024

phillip-stephens commented Jul 22, 2024

zakird commented Jul 23, 2024

phillip-stephens commented Jul 23, 2024 • edited Loading

zakird commented Jul 23, 2024 • edited Loading

phillip-stephens commented Jul 23, 2024

phillip-stephens commented Jul 16, 2024 •

edited

Loading

phillip-stephens commented Jul 23, 2024 •

edited

Loading

zakird commented Jul 23, 2024 •

edited

Loading