Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix CNAME handling a bit #19

Merged
merged 1 commit into from
Jan 20, 2017
Merged

Fix CNAME handling a bit #19

merged 1 commit into from
Jan 20, 2017

Conversation

tianon
Copy link
Owner

@tianon tianon commented Jan 20, 2017

If there's a CNAME in the list of RRs to return, always put it in Answer (not Extra), regardless of the Question type.

Additionally, when we perform recursive queries, our recursive query is what needs to be in the "name" field for the reply.

Before:

$ dig ha.pool.sks-keyservers.net

; <<>> DiG 9.11.0-P2 <<>> ha.pool.sks-keyservers.net
;; global options: +cmd
;; Got answer:
;; ->>HEADER<<- opcode: QUERY, status: NOERROR, id: 6632
;; flags: qr rd; QUERY: 1, ANSWER: 1, AUTHORITY: 0, ADDITIONAL: 1
;; WARNING: recursion requested but not available

;; QUESTION SECTION:
;ha.pool.sks-keyservers.net.	IN	A

;; ANSWER SECTION:
ha.pool.sks-keyservers.net. 0	IN	A	172.18.0.7

;; ADDITIONAL SECTION:
ha.pool.sks-keyservers.net. 0	IN	CNAME	haproxy-sks.docker.

;; Query time: 1 msec
;; SERVER: 172.18.42.1#53(172.18.42.1)
;; WHEN: Fri Jan 20 12:56:19 PST 2017
;; MSG SIZE  rcvd: 144

After:

$ dig ha.pool.sks-keyservers.net

; <<>> DiG 9.11.0-P2 <<>> ha.pool.sks-keyservers.net
;; global options: +cmd
;; Got answer:
;; ->>HEADER<<- opcode: QUERY, status: NOERROR, id: 53265
;; flags: qr rd; QUERY: 1, ANSWER: 2, AUTHORITY: 0, ADDITIONAL: 0
;; WARNING: recursion requested but not available

;; QUESTION SECTION:
;ha.pool.sks-keyservers.net.	IN	A

;; ANSWER SECTION:
ha.pool.sks-keyservers.net. 0	IN	CNAME	haproxy-sks.docker.
haproxy-sks.docker.	0	IN	A	172.18.0.7

;; Query time: 1 msec
;; SERVER: 172.18.42.1#53(172.18.42.1)
;; WHEN: Fri Jan 20 13:07:19 PST 2017
;; MSG SIZE  rcvd: 136

cc @yosifkit @paultag (this should fix GnuPG 2's libdns handling of our CNAME records)

If there's a CNAME in the list of RRs to return, _always_ put it in Answer (not Extra), regardless of the Question type.

Additionally, when we perform recursive queries, our recursive query is what needs to be in the "name" field for the reply.

Before:

```console
$ dig ha.pool.sks-keyservers.net

; <<>> DiG 9.11.0-P2 <<>> ha.pool.sks-keyservers.net
;; global options: +cmd
;; Got answer:
;; ->>HEADER<<- opcode: QUERY, status: NOERROR, id: 6632
;; flags: qr rd; QUERY: 1, ANSWER: 1, AUTHORITY: 0, ADDITIONAL: 1
;; WARNING: recursion requested but not available

;; QUESTION SECTION:
;ha.pool.sks-keyservers.net.	IN	A

;; ANSWER SECTION:
ha.pool.sks-keyservers.net. 0	IN	A	172.18.0.7

;; ADDITIONAL SECTION:
ha.pool.sks-keyservers.net. 0	IN	CNAME	haproxy-sks.docker.

;; Query time: 1 msec
;; SERVER: 172.18.42.1#53(172.18.42.1)
;; WHEN: Fri Jan 20 12:56:19 PST 2017
;; MSG SIZE  rcvd: 144
```

After:

```console
$ dig ha.pool.sks-keyservers.net

; <<>> DiG 9.11.0-P2 <<>> ha.pool.sks-keyservers.net
;; global options: +cmd
;; Got answer:
;; ->>HEADER<<- opcode: QUERY, status: NOERROR, id: 53265
;; flags: qr rd; QUERY: 1, ANSWER: 2, AUTHORITY: 0, ADDITIONAL: 0
;; WARNING: recursion requested but not available

;; QUESTION SECTION:
;ha.pool.sks-keyservers.net.	IN	A

;; ANSWER SECTION:
ha.pool.sks-keyservers.net. 0	IN	CNAME	haproxy-sks.docker.
haproxy-sks.docker.	0	IN	A	172.18.0.7

;; Query time: 1 msec
;; SERVER: 172.18.42.1#53(172.18.42.1)
;; WHEN: Fri Jan 20 13:07:19 PST 2017
;; MSG SIZE  rcvd: 136
```
@tianon
Copy link
Owner Author

tianon commented Jan 20, 2017

Here's an example record I created on Cloudflare to help me figure out what behavior rawdns ought to have here:

$ dig sks.infosiftr.net


; <<>> DiG 9.11.0-P2 <<>> sks.infosiftr.net
;; global options: +cmd
;; Got answer:
;; ->>HEADER<<- opcode: QUERY, status: NOERROR, id: 55513
;; flags: qr rd ra; QUERY: 1, ANSWER: 7, AUTHORITY: 0, ADDITIONAL: 1

;; OPT PSEUDOSECTION:
; EDNS: version: 0, flags:; udp: 512
;; QUESTION SECTION:
;sks.infosiftr.net.		IN	A

;; ANSWER SECTION:
sks.infosiftr.net.	86	IN	CNAME	ha.pool.sks-keyservers.net.
ha.pool.sks-keyservers.net. 386	IN	A	79.143.214.213
ha.pool.sks-keyservers.net. 386	IN	A	104.236.209.43
ha.pool.sks-keyservers.net. 386	IN	A	67.205.148.255
ha.pool.sks-keyservers.net. 386	IN	A	37.191.238.78
ha.pool.sks-keyservers.net. 386	IN	A	178.32.66.144
ha.pool.sks-keyservers.net. 386	IN	A	208.113.128.181

;; Query time: 45 msec
;; SERVER: 172.18.42.1#53(172.18.42.1)
;; WHEN: Fri Jan 20 13:07:18 PST 2017
;; MSG SIZE  rcvd: 179

@tianon
Copy link
Owner Author

tianon commented Jan 20, 2017

Proof in the pudding:

root@87e7a50d4fcf:/# gpg --keyserver ha.pool.sks-keyservers.net --recv-keys 'B42F 6819 007F 00F8 8E36  4FD4 036A 9C25 BF35 7DD4'
gpg: directory '/root/.gnupg' created
gpg: new configuration file '/root/.gnupg/dirmngr.conf' created
gpg: new configuration file '/root/.gnupg/gpg.conf' created
gpg: keybox '/root/.gnupg/pubring.kbx' created
gpg: /root/.gnupg/trustdb.gpg: trustdb created
gpg: key 036A9C25BF357DD4: public key "Tianon Gravi <[email protected]>" imported
gpg: no ultimately trusted keys found
gpg: Total number processed: 1
gpg:               imported: 1
root@87e7a50d4fcf:/# gpg --version
gpg (GnuPG) 2.1.17
libgcrypt 1.7.5-beta
Copyright (C) 2016 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <https://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.

Home: /root/.gnupg
Supported algorithms:
Pubkey: RSA, ELG, DSA, ECDH, ECDSA, EDDSA
Cipher: IDEA, 3DES, CAST5, BLOWFISH, AES, AES192, AES256, TWOFISH,
        CAMELLIA128, CAMELLIA192, CAMELLIA256
Hash: SHA1, RIPEMD160, SHA256, SHA384, SHA512, SHA224
Compression: Uncompressed, ZIP, ZLIB, BZIP2

@tianon tianon merged commit 3494a13 into master Jan 20, 2017
@tianon tianon deleted the always-answer-cname branch January 20, 2017 21:38
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.

1 participant