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

plugin/file: Use NXDOMAIN response if CNAME target is NXDOMAIN #4303

Merged
merged 4 commits into from
Dec 9, 2020

Conversation

chrisohaver
Copy link
Member

@chrisohaver chrisohaver commented Nov 20, 2020

Signed-off-by: Chris O'Haver cohaver@infoblox.com

1. Why is this pull request needed and what does it do?

As cited in #4288, per https://tools.ietf.org/html/rfc6604#section-3, when following a CNAME chain the response code sent to the client should be the same response code received when resolving the target of the final link in the chain. Strictly this means we should be passing through the exact Rcode received when looking up the final target. But since file uses an alternate response code datatype file.Result which does not map 1:1 to DNS response codes, making that happen might require some larger scope refactoring. So for now, just addressing response codes that do map.

2. Which issues (if any) are related?

fixes #4288

3. Which documentation changes (if any) need to be made?

4. Does this introduce a backward incompatible change or deprecation?

Signed-off-by: Chris O'Haver <cohaver@infoblox.com>
Signed-off-by: Chris O'Haver <cohaver@infoblox.com>
@miekg
Copy link
Member

miekg commented Nov 21, 2020

looks good. Maybe add a little test?

Signed-off-by: Chris O'Haver <cohaver@infoblox.com>
Signed-off-by: Chris O'Haver <cohaver@infoblox.com>
@chrisohaver
Copy link
Member Author

looks good. Maybe add a little test?

Thanks, test added.

@chrisohaver chrisohaver changed the title plugin/file: Use NXDOMAIN response if CNAME target it NXDOMAIN plugin/file: Use NXDOMAIN response if CNAME target is NXDOMAIN Nov 25, 2020
@miekg miekg merged commit 6bbb48d into coredns:master Dec 9, 2020
@chrisohaver chrisohaver deleted the file-cnames branch January 9, 2021 14:45
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.

Non-existent CNAME target in the same zone should be returned with NXDOMAIN instead of NOERROR rcode
2 participants