Re: [apps-review] apps-team review of draft-ietf-mif-dns-server-selection

[<[e-mail]>]

Thank you very much for your extensive review, I have replied and commented
inline. I cc’d mif, apps-ads, and apps-review – hope it is ok.

Completing review of -03 revision was just fine approach, as the -04 was
created only to share updates done based on feedback for -03 that was
received by yesterday evening my time. 

I will wait a while for additional input before uploading -05.

Best regards,

Teemu

-------
From: ext Murray S. Kucherawy [mailto:[e-mail]] 
Sent: 14. syyskuuta 2011 10:01
To: [e-mail];
[e-mail]
Cc: [e-mail]; [e-mail]
Subject: apps-team review of draft-ietf-mif-dns-server-selection

I have been selected as the Applications Area Review Team reviewer for this
draft (for background on apps-review, please see
http://www.apps.ietf.org/content/applications-area-review-team).

Please resolve these comments along with any other Last Call comments you
may receive. Please wait for direction from your document shepherd or AD
before posting a new version of the draft.

Document: draft-ietf-mif-dns-server-selection-03
Title: Improved DNS Server Selection for Multi-Homed Nodes
Reviewer: Murray S. Kucherawy
Review Date: September 13, 2011
IETF Last Call Date: N/A (not yet in IETF Last Call)
IESG Telechat Date: N/A (not scheduled)
Summary: This draft is almost ready for publication as a Standards Track RFC
but has a few issues that should be fixed before publication.

NOTE: This is an early review, so please ignore the parts of this
boilerplate that sound as though you’re in IETF Last Call.  Also, this is a
review of the -03 version of the draft.  A new version was published after I
had already completed a good part of my review, so I completed it based on
this version and not the new one.  I haven’t looked at the diffs between the
two.

Major Issues:
• None.

Minor Issues:
• The IANA Considerations section seems to be a little understated.  I would
suggest copying the one from RFC3646 and modifying it accordingly.

Teemu>Roger that, I updated my -05 candidate IANA considerations section to
look following (as of now):
--
   This memo requests IANA to assign two new option codes.  First option
   code is requested to be assigned for DHCPv4 DNS Server Selection
   option (TBD) from the DHCP option code space defined in section "New
   DHCP option codes" of RFC 2939.  Second option code is requested to
   be assigned to the DHCPv6 DNS Server Selection option (TBD) from the
   DHCPv6 option code space defined in section "IANA Considerations" of
   RFC 3315.
--

• The use of “domain” in the first paragraph of Section 1 is ambiguous.  The
word “domain” is unfortunately overloaded to mean management domain, DNS
domain, and probably some others, plus the usual dictionary definition.  I
recommend simply saying “a solution for IPv6.”  Don’t be surprised if a DNS
Directorate reviewer complains about this as well.

Teemu>It now says:”…multi-interfaced nodes and provides a solution”, as the
solution is proposed for both IPv4 and IPv6.

• The document in general makes use of the term “DNS suffix”.  It could be
that I’ve simply never encountered the term before, but it took me a while
of reading to realize this actually meant “domain name”.  I would suggest
clarifying this up front someplace, or changing it to “domain name”
throughout.

Teemu>It was changed to “domain” throughout. However, for section 4.1 (where
this is introduced) I added further clarification that “domain” is referring
to “DNS domain name suffixes”:
--
…..matching to specific DNS domain name suffixes or reverse (PTR record)
lookup
          requests matching to specific network addresses (later referred as
"domain" and "network").
--

• The last paragraph of Section 2.4 is confusing.  It says “After the
interface change[,] a host could have both positive and negative DNS cache
entries no longer valid on the new network interface.”  Should that
“both…and” be “either…or”?  I’m not clear on how a single node can have both
positive and negative DNS cache entries no longer valid on the new
interface, unless you mean some of its cache will be positive and wrong and
some of its cache will be negative and wrong (about different names) after
the move.  This should be clarified.

Teemu> Section 2.2..  It tries to say any cached entry may or may not be
valid after interface change – as you guessed. How about:
--
A thing worth noting is that network specific IP addresses can cause
problems also for a single-homed node, if the node retains DNS cache during
movement from one network to another. After the network change, a node may
have both wrong positive and wrong negative entries in its DNS cache.
--

• I found it strange to read RFC2119 normative language Section 4.1 which
describes an internal-use algorithm for collecting and organizing data
learned from DHCP and DHCPv6 to build up a set of rules for deciding which
nameserver(s) to use for which queries.  The same sort of thing appeared
later in Sections 4.5 and 4.6.  My understanding is that, since DHCP and
DHCPv6 are the wire protocols used and this section describes an
internal-use mechanism, these bits of the text shouldn’t include any RFC2119
language; there is nothing here that affects interoperability between two
nodes implementing this protocol.  If a node implementing this gets it
wrong, it only hurts itself; the wire protocols don’t break.  If this
section were to be separated into its own document, it would be
Informational, not Standards Track.  This isn’t a showstopper for me, but I
did find it unusual.

Teemu>The normative text for this algorithm is there mostly due concerns
that otherwise this option could open a new attack vector. By describing how
a node MUST handle the information it receives, the risk of attacks should
be decreased. The normative text is there also so that if a node claims
compliancy to RFC, network operator who provides such options (to build a
system) could expect a node to behave as is described. 

• There are a few places in the document, the Introduction and Section 4.1
for example, that talk about PTR lookup requests matching specific IPv6
prefixes.  However, the abstract mentions IPv4 support, and the mechanism is
also described for IPv4.  I suggest making a quick pass through the document
to be sure it doesn’t appear as if IPv6 is the focus and IPv4 was added in
as an after-thought.

Teemu>Well IPv4 was after-thought as I would have settled for IPv6-only but
WG wanted IPv4 option as well:-) The -04 version should have cleared several
of these "IPv6 focus" issues (by talking about network addresses instead of
prefixes), but I guess checking again would not hurt. On the other hand,
IPv6 really is the focus here, as multi-interface behavior with IPv4 anyway
is terrible mess due widespread usage of RFC1918 addressed networks (and
that's why I'm secretly hoping DHC WG would say they don’t want to use DHCP
option code for this option and hence this technology must be IPv6-only:-D.

• The normative language in Section 4.1 around DNSSEC seems to be the kind
of thing that should be left for local policy.  I can envision environments
where the client isn’t doing anything sensitive enough to demand DNSSEC, so
the first reply it gets from any nameserver it queried is probably fine. 
The same sort of thing reappears in Section 4.4.

Teemu>This document should already allow such local policy. Section 4.4.
describes possibility of "unless the node is specifically configured to do
so.". The DNSSEC requirement has been very hard on MIF WG mailing list
discussions in order to fight against creation of new attack vectors, hence
easing that requirement requires restarting of those discussions. See also
section 4.4 for cases where DNSSEC is not required - like the option 1:"The
server selection option is delivered across a secure, trusted channel."

• In Section 4.2, the paragraph starting “If the OPTION_DNS_SERVER_SELECT
contains…” was a little confusing to read.  It says in general that when
learning about DNS servers available, the node can update what it knows but
can’t forget what it’s learned, however it can ignore things it chooses to
ignore.  I found this difficult to follow.  Some expansion of this
paragraph, perhaps with examples, might help.

Teemu>I wonder if it would be actually wiser just to keep one instance of
the option contents in memory (from most trusted network interface?), and
ensure all legitimate DHCP servers have the same content for the same DNS
server option?

• At the end of Section 4.2, it says “…use of generic DHCPv6 Information
Refresh Time Option, as specified in [RFC4242], is envisaged.”  That wording
is odd; shouldn’t it be RECOMMENDED?  It’s too vague otherwise.

Teemu>Fine for me, changed on -05 candidate accordingly.

• Section 4.4 says a node MUST NOT use the feature being described here
unless specifically configured to do so.  That seems odd.  Is the intent to
have implementations force this feature off-by-default?

Teemu>Pretty much so, due security concerns. Please note that other SDOs or
organizations that create profiles may say that in their environments the
feature shall be on-by-default.

• I didn’t understand this sentence in Section 4.5: “When both options are
received from the same network interface …, the resolver MUST make the
decision which one to prefer based on preferences.”

Teemu>modified to:"...decision which one to prefer based on DNS preference
field value.". Better?

• Section 7 says “To ensure nodes’ routing … works correctly, network
administrators should also deploy related technologies for that purpose.” 
Are there any that can be suggested or referenced here?

Teemu>I actually removed that sentence on -04 as it is probably better to
tie different solution documents together in some other document (like at
draft-ietf-v6ops-ipv6-multihoming-without-ipv6nat ). The reference could
have been to RFC4191 and MIF DHCPv6 route option (at least).
 
• Appendix A is called “Best Current Practice”, but the first section starts
out “A possible current practice…”  Where’s the best one, and why is it
best?

Teemu>well.. changed title of Appendix A to "Possible alternative practices
for DNS server selection". Better?

Nits:
• There are a few places where I saw RFC2119 words used in non-normative
ways, such as the last sentence of Section 3.1 or the middle of Section 7. 
I would suggest considering alternate words like “might” in place of “may”
specifically to avoid such issues.

Teemu>You refer to section 7 comment regarding "are recommended to avoid..".
The section 7 was completely rewritten for -04 and is now using normative
ways. On section 3.1 you refer to "may be considered.."? Changed that to
might. I guess the doc needs reading pass to find possible other instances
of this issue...

• There’s a discussion point at the end of Section 4.1 that asks “What about
those DNS servers that instead of a negative answer always return positive
reply with an IP address of some captive portal?”  I don’t think software
implementing this specification can determine that via a DHCP query without
other supporting changes to the infrastructure.  I suggest not trying to
tackle this point.

Teemu>Thank you for commenting - and also supporting - on this issue. I
thought about this yesterday and also decided not trying to tackle this
point and removed the note from -04.... Node with DNSSEC implementation
would fail on validation on such cases.. but would still have to continue in
order to get to the captive portal....

• In Section 4.3, Figure 6, the order of “DNS-recusrive-name-server” and
“prf” in the description should be swapped to match the order in which they
appear in the diagram.

Teemu>Changed

• In Section 5, the explanation bullets under Figure 7 include two DHCPv6
replies.  The second one covered “example.com” and a specific IPv6 suffix,
but it’s not clear that there was no overlap with the first one (i.e., the
first one could’ve said the same thing).  It should be made clear that they
had disjoint replies.

Teemu>Ok.. changed to:
---
   2.  The node obtains domain 'domain1.example.com' and IPv6 network
       '0.8.b.d.0.1.0.0.2.ip6.arpa' for the new interface 1 from DHCPv6
       server
....
   5.  The node obtains domain 'domain2.example.com' and IPv6 network
       information, say '1.8.b.d.0.1.0.0.2.ip6.arpa' for the new
       interface 2 from DHCPv6 server
--- 
and  later in fig 8:
--
   1.  An application makes a request for resolving an FQDN, e.g.
       'private.domain2.example.com'
--

• In several places there are references to “chapters” instead of “sections”
or the “section” reference is not capitalized, which xml2rfc does for you. 
It seems then these are manual references rather than automatic ones, though
the document does claim to have been produced by xml2rfc.   You might make
your life easier by changing them to <xref…> tags.

Teemu>Fixed manually... will try to remember to update to automatic later:-D

• Section 10 might be improved by dividing separate topics into their own
subsections.  For example, the first three paragraphs could become Section
10.1, and the remaining two could go in 10.2 and 10.3.

Teemu>I will think about this (now running out of time to do edits while
replying)

• Appendix A.1 uses the term “hotspot” which is colloquial and might even be
trademarked.  I suggest “wireless network”.

Teemu>That part was removed at the same time the text talking about leaking
names was removed..

• The document is in need of a full edit pass with attention to corrections
of English grammar. 

Teemu>Yep, hopefully by someone who is native English speaker:-) (or
otherwise skilled).