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

rec: Fix cache handling of ECS queries with a source length of 0 #5515

Merged
merged 1 commit into from Aug 23, 2017

Conversation

Projects
None yet
3 participants
@rgacogne
Member

rgacogne commented Jul 12, 2017

Short description

The current behavior is to send a query without ECS information and to cache it as a non ECS-specific entry. Unfortunately doing so meant that we wouldn't send any further ECS-specific queries for
this (qname,qtype) since we could fallback to the non ECS-specific cached answer.

This commit adds a new parameter, ecs-scope-zero-addr, allowing to specify the address sent to ECS-whitelisted authoritative servers in that case. If unset, the default is to take the first usable (non-any) address in query-local-address then in query-local-address6, and finally to use 127.0.0.1 as a last resort.

Checklist

I have:

  • read the CONTRIBUTING.md document
  • compiled and tested this code
  • included documentation (including possible behaviour changes)
  • documented the code
  • added or modified regression test(s)
  • added or modified unit test(s)
}
if (!found) {
for (const auto& addr : g_localQueryAddresses6) {
if (!IsAnyAddress(addr)) {

This comment has been minimized.

@zeha

zeha Jul 12, 2017

Collaborator

what about link local addresses and other snowflakes?

@zeha

zeha Jul 12, 2017

Collaborator

what about link local addresses and other snowflakes?

This comment has been minimized.

@rgacogne

rgacogne Aug 22, 2017

Member

I agree this might be an issue, however this check is only intended to catch obvious configuration error since there is no way we can detect every one of them. My feeling is that the administrator setting query-local-address and/or query-local-address6 to something like a link-local address and adding entries to the ECS whitelist should also set ecs-scope-zero-addr. I might be wrong :)

@rgacogne

rgacogne Aug 22, 2017

Member

I agree this might be an issue, however this check is only intended to catch obvious configuration error since there is no way we can detect every one of them. My feeling is that the administrator setting query-local-address and/or query-local-address6 to something like a link-local address and adding entries to the ECS whitelist should also set ecs-scope-zero-addr. I might be wrong :)

rec: Fix cache handling of ECS queries with a source length of 0
The current behavior is to send a query without ECS information
and to cache it as a non ECS-specific entry. Unfortunately doing
so meant that we wouldn't send any further ECS-specific queries for
this (qname,qtype) since we could fallback to the non ECS-specific
cached answer.
This commit adds a new parameter, `ecs-scope-zero-addr`, allowing
to specify the address sent to ECS-whitelisted authoritative servers
in that case. If unset, the default is to take the first usable
(non-any) address in `query-local-address` then in `query-local-address6`,
and finally to use `127.0.0.1` as a last resort.
@rgacogne

This comment has been minimized.

Show comment
Hide comment
@rgacogne

rgacogne Aug 22, 2017

Member

Updated to the new docs format.

Member

rgacogne commented Aug 22, 2017

Updated to the new docs format.

@aerique aerique merged commit 44ff406 into PowerDNS:master Aug 23, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@aerique aerique removed the needs review label Aug 28, 2017

@rgacogne rgacogne deleted the rgacogne:rec-ecs-source-0 branch Sep 4, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment