Implement RFC 7871 EDNS Client Subnet (ECS) #88
Conversation
|
Sorry for the delay, I got nerd sniped into another larger project after getting most of this done. Updated and rebased! Please re-review. |
|
Thanks @rfinnie ! |
rfinnie
force-pushed
the
ecs
branch
4 times, most recently
from
7fea025
to
1cadad4
Compare
dohproxy/server_protocol.py
Outdated
| if original_ecs_option is not None: | ||
| options.append(original_ecs_option) |
There was a problem hiding this comment.
this would be invalid as it would put an ECS query option in the response.
I think the logic would be something along:
if original_ecs_option is None:
pop_ecs_from_response()
e.g, if we were not provided an ECS optiopn in the query, we would take it out of the response, otherwise we leave it untouched.
There was a problem hiding this comment.
Thanks. That logic was from earlier on when trying to deal with a scenario where the client had ECS set but we were modifying ECS anyway, which of course we don't actually want to do. I was able to simplify the logic now and not need to track the original ECS option, just by passing from set_dns_ecs() whether our ECS was actually set or not. The logic should be sound now.
dohproxy/server_protocol.py
Outdated
| dnsr.edns = dnsq.edns | ||
| dnsr.options = options |
There was a problem hiding this comment.
same here, we only need to "reset" EDNS if we modified it in the first place.
There was a problem hiding this comment.
Agreed, it's now part of "if dnsr is not None and we_set_ecs".
In a nutshell, ECS is like X-Forwarded-For for DNS, and lets DNS resolvers and proxies tell authoritative servers where the request originated (down to a /24 v4 or /56 v6 network). This implementation preserves existing (and hence should be more specific) ECS options.
In a nutshell, ECS is like X-Forwarded-For for DNS, and lets DNS
resolvers and proxies tell authoritative servers where the request
originated (down to a /24 v4 or /56 v6 network). IMHO this should
be the default, but I've left it as an option.