Apologies to everyone for it having taken so long to address your comments.

I'm finally going through and trying to capture and incorporate comments.
Because we've taken so long to release an updated draft, making sure
we are capturing all the points if a bit tricky. In order to try avoid
having things slip through the cracks I'm going to try simply work
through the comments in order, releasing (to Github) version as I go.


On Wed, Dec 31, 2014 at 2:31 PM, 神明達哉 <jin...@wide.ad.jp> wrote:
> I've read the draft and have the following comments.
>
> - Section 5
>
>    o  SOURCE NETMASK, unsigned octet representing the length of the
>       netmask pertaining to the query.  In replies, it mirrors the same
>       value as in the requests.  It can be set to 0 to disable client-
>       based lookups, in which case the ADDRESS field MUST be absent.
>
>   I suspect the term "NETMASK" could be misleading or at least
>   inaccurate.  Technically, a netmask can be an arbitrary bit pattern
>   to represent a "mask", so its "length" may not always be
>   well-defined.  Also, IPv6 doesn't support the concept of
>   non-contiguous "netmask".  This definition seems to implicitly
>   assume contiguous netmasks, and define the term "NETMASK" as the
>   prefix length of an address.  I'm fine with the assumption as
>   non-contiguous IPv4 netmasks can be reasonably considered a thing of
>   past.  But in that case I'd suggest using the term "PREFIX-LENGTH",
>   noting that this specification does not support non-contiguous IPv4
>   netmasks.
>
>   Same for SCOPE NETMASK.

DONE.

Yes. NETMASK was a poor choice early on in the document life. Fixed it
to PREFIX-LENGTH (and borrowed the definition from RFC4291).

>
> - Section 5
>
>    o  SCOPE NETMASK, unsigned octet representing the length of the
>       [...]
>       It might or might not match SOURCE NETMASK; it could be shorter or
>       longer.
>
>   "match" sounds awkward since the "SOURCE NETMASK" is defined as a
>   length of bits (again, I'd suggest renaming it "SOURCE
>   PREFIX-LENGTH").  I suggest: s/match/be equal to/ (and perhaps
>   s/shorter or longer/smaller or larger/)

DONE.
I renamed to PREFIX-LENGTH and update to "be equal to". I left the
"longer or shorter", and this is (IMO) more common usage.

>
> - Section 6.1
>
>    A Stub Resolver MAY generate DNS queries with an edns-client-subnet
>    option with SOURCE NETMASK set to 0 (i.e. 0.0.0.0/0) to indicate that
>
>   I'd suggest: s/i.e./e.g./ since this may also be an IPv6 address (in
>   which case FAMILY is set to 2)

DONE.
Good point, thank you.

I updated this to be explicit ("(i.e. 0.0.0.0/0 for IPv4 or ::/0 for
IPv6) "). This seemed cleaner than the example form.

>
> - Section 6.3
>
>    fields, as detailed below.  Note that the additional and authority
>    sections from a DNS response message are specifically excluded here.
>    Any information cached from these sections MUST NOT be tied to a
>    network.
>
>   What if the "optimized" reply is a negative one for some specific
>   client addresses (while it's positive for some other addresses)?
>   The authoritative server would (or might) return an SOA in the
>   authority section, expecting the Recursive Resolver to use it for
>   negative cache specific to the corresponding client address.
>   Or are we simply excluding the support for negative optimized
>   replies?  (If so, I think it would be worth noting explicitly).

I think that we should just be excluding NXD - this fits in with my
view of how ECS should work (and what NXD "means"). Noting that this
explicitly.
Actually, I don't really know what the right answer here is - if I've
claimed that the best address for 192.0.2.0/24 is 10.10.10.10, can I
now claim that the name doesn't exist for users at 172.16.5.2?! Or,
can I return 10.10.10.10 to uses at 192.0.2.0/24 and then return
SERVFAIL for other folk? Both feel wrong - perhaps we need to say that
any error (defined later :-P) applies to all entries?!
I've created an issue in github to track this:
https://github.com/wkumari/draft-vandergaast-edns-client-subnet/issues/1


>
> - Section 6.3
>
>    If the address of the client is within any of the networks in the
>    cache, then the cached response MUST be returned as usual.  If the
>    address of the client matches multiple networks in the cache, the
>    entry with the longest SCOPE NETMASK value MUST be returned, as with
>    most route-matching algorithms.
>
>   If I understand this (and Section 6.3 in general), the following
>   "suboptimal" scenario could happen:
>   - The Authoritative Server is configured with two prefixes for
>     optimized responses: 2001:db8::/32 and 2001:db8:2::/48
>   - The Recursive Server sends a query with SOURCE of 2001:db8:1::/48
>   - The Authoritative Server finds the best matching prefix for the
>     SOURCE is 2001:db8::/32 and returns a response corresponding to
>     it, setting SCOPE NETMASK to 32
>   - The Recursive Server receives the response and caches it
>   - The Recursive Server receives a query from 2001:db8:2::1, and
>     finds it has a matching cache (with prefix being 2001:db8::/32)
>     and uses the cached response to answer the query, even if it could
>     get the specifically optimized response for 2001:db8:2::/48 from
>     the Authoritative Server.
>
>   Is my understanding correct?  If so, is this a problem to resolve or
>   something we need to accept?

Yup, very good point.

A "good" implementation of the server could note that 2001:db8:2::/48
is a subnet of 2001:db8::/32 and warn the user that the /32 may elide
the /48. It could even break the /32 into shorter prexies (all with
the same answer as the /32, apart from the more specific /48). I do
not think that it is our place to specify which the implementation
chooses, but I've noted that implementations MUST document what they
do.

>
> - Section 6.3: likewise, what about the following scenario?
>   - The Authoritative Server is configured with two prefixes for
>     optimized responses: 2001:db8::1/48 and 2001:db8:2::/48
>   - The Recursive Server receives a query from 2001:db8:3::1 and
>     sends its own query to the Authoritative Server with
>     SOURCE=2001:db8::/32, SCOPE NETMASK=48
>   - The Authoritative Server finds the two /48 prefixes match the
>     source, and returns a response using one of them.
>   - The Recursive Server receives the response and caches it.
>   - The Recursive Server receives a query from 2001:db8:3::1 again.
>     It doesn't match the cached SCOPED address, so it sends its own
>     query to the authoritative server again.
>   - This will be always the case for 2001:db8:3::1, and the cached
>     information is effectively useless, making the Recursive Server
>     busier.
>
> - Section 6.3
>
>    Any reply containing an edns-client-subnet option considered invalid
>    should be treated as if no edns-client-subnet option was specified at
>    all.
>
>   What specifically does "considered invalid" mean?  And, depending on
>   that, shouldn't the reply rather be discarded in such a case?

Addressed.

Not a clue -- I think it was just cruft from an earlier version of the
doc. There are many things that could conceivably make an ECS reply
"invalid" (for example, a PREFIX-LENGTH of /33 for an IPv4 address),
but seeing as the second paragraph of Section 6.3 contains "If the
FAMILY, SOURCE PREFIX-LENGTH, and SOURCE PREFIX-LENGTH bits of ADDRESS
in the reply don't match the fields in the corresponding request, ..."
 I don't think I need to list sanity checks here. If the recursive
sent a bizarre request, it need to deal with figuring out what to do
with the reply... so I just removed this sentence and we can re-add it
if we figure out what corner case it was trying to cover.


>
> - Section 8
>
>    The presence or absence of an [RFC6891] EDNS0 OPT resource record
>    containing an edns-client-subnet option in a DNS query does not
>    change the usage of the resource records and mechanisms used to
>    provide data origin authentication and data integrity to the DNS, as
>    described in [RFC4033], [RFC4034] and [RFC4035].  OPT records are not
>    signed.
>
>   While what's described in this section is not incorrect, I suspect
>   it's not really fair not to mention that intended deployments (i.e.,
>   CDN-like ones) with this option are not really friendly with DNSSEC
>   at best; even if it's not impossible to provide a valid signature
>   for every "optimized" response in theory, it would involve many
>   non-trivial technical challenges such as on-the-fly signing.
>
>   If these challenges make CDN-like operators unwilling to deploy
>   DNSSEC and this option makes the optimization even more attractive,
>   then this option could help hinder wider deployment of DNSSEC.
>   I'm not saying this option is evil and should be banned just because
>   of this, but I'd expect this kind of consideration for this section
>   to be more helpful and fair.

CDNs already face this issue - the availability of ECS does not (IMO)
materially change how many different answers CDNs provide, it just
allows them to better choose which of the N sets of responses get to
which client.

>
> - Section 10.1
>
>    Users who wish their full IP address to be hidden can include an
>    edns-client-subnet option specifying the wildcard address 0.0.0.0/0
>    (i.e.  FAMILY set to 1 (IPv4), SOURCE NETMASK to 0 and no ADDRESS).
>
>   What if the user (stub resolver)'s address is IPv6?  Should it still
>   set FAMILY to 1?

DONE.

I updated this to just be "Users who wish their full IP address to be
hidden can include an edns-client-subnet option specifying the
wildcard address (i.e. SOURCE PREFIX-LENGTH of 0)."


>
> - Section 10.2
>
>    With multiple queries for the same name in flight, the attacker has a
>    higher chance of success in sending a matching response (with the
>    address 0.0.0.0/0 to get it cached for many hosts).
>
>   Another IPv4-centric description.  This should probably be, e.g.
>   "with the SCOPE NETMASK being 0, meaning an empty prefix".

DONE.
... and thanks for the suggested text.

>
> - Section 12
>
>    1.   A stub resolver SR with IP address 192.0.2.37 tries to resolve
>         www.example.com, by forwarding the query to the Recursive
>         Resolver R from IP address IP, asking for recursion.
>
>   I guess the intent was: s/Resolver R/Resolver RNS/.  And some other
>   "R"s in the section will also have to be changed to "RNS".

DONE.
Thank you.

>
> - Section 12
>
>         *  ADDRESS, set to 0xC0 0x00 0x02 0xE0, copied from the request.
>
>   This is not a verbatim copy from the request, so I suggest:
>   "extended from the request".

DONE.

>
> Editorial nits
>
> - Section 3: A very minor point, but this draft seems to always use
>   the term "Section" to refer to a numbered portion of RFCs/drafts,
>   such as "Section 5" or "Section 6.1", so to be consistent "chapter"
>   would better be "Section".
>
>    Authoritative Nameserver:  A nameserver that has authority over one
>       or more DNS zones.  These are normally not contacted by clients
>       directly but by Recursive Resolvers.  Described in [RFC1035]
>       chapter 6.

DONE.

Well spotted.

>
> - Section 6.3: s/.././
>
>       to queries originating from the network covered by the ADDRESS and
>       SCOPE NETMASK..

DONE.
Thank you.

W

>
> --
> JINMEI, Tatuya
>
> _______________________________________________
> DNSOP mailing list
> DNSOP@ietf.org
> https://www.ietf.org/mailman/listinfo/dnsop



-- 
I don't think the execution is relevant when it was obviously a bad
idea in the first place.
This is like putting rabid weasels in your pants, and later expressing
regret at having chosen those particular rabid weasels and that pair
of pants.
   ---maf

_______________________________________________
DNSOP mailing list
DNSOP@ietf.org
https://www.ietf.org/mailman/listinfo/dnsop

Reply via email to