Christer, thank you for your review. Tommy, thank you for addressing Christer’s comments. I entered a No Objection ballot.
Alissa > On Oct 22, 2018, at 4:26 PM, Tommy Pauly <tpa...@apple.com> wrote: > > Hi Christer, > > Thanks again for the review. I've addressed all three comments below in an > update to the draft: > > https://tools.ietf.org/html/draft-ietf-ipsecme-split-dns-13 > <https://tools.ietf.org/html/draft-ietf-ipsecme-split-dns-13> > https://tools.ietf.org/rfcdiff?url2=draft-ietf-ipsecme-split-dns-13.txt > <https://tools.ietf.org/rfcdiff?url2=draft-ietf-ipsecme-split-dns-13.txt> > > Thanks, > Tommy > >> On Aug 19, 2018, at 1:39 PM, Christer Holmberg >> <christer.holmb...@ericsson.com <mailto:christer.holmb...@ericsson.com>> >> wrote: >> >> Hi Tommy, >> >> Please see inline. >> >> >> Minor issues: >> >> Q1: >> >>>> Section 3.1 contains some SHOULD-do statements, e.g.,: >>>> >>>> "the initiator SHOULD also include one or more INTERNAL_IP4_DNS and >>>> INTERNAL_IP6_DNS attributes in the CFG_REQUEST" >>>> >>>> "the initiator SHOULD also include one or more INTERNAL_DNS_DOMAIN >>>> attributes >>>> in the CFG_REQUEST." >>>> >>>> Is there a reason for not using MUST instead of SHOULD? >>> >>> In general, the CFG_REQUEST attributes are a bit loose—they're hints more >>> than requirements. >>> >>> From section 3.15.1 of RFC7296: >>> >>> The CFG_REQUEST and CFG_REPLY pair allows an IKE endpoint to request >>> information from its peer. If an attribute in the CFG_REQUEST >>> Configuration payload is not zero-length, it is taken as a suggestion >>> for that attribute. The CFG_REPLY Configuration payload MAY return >>> that value, or a new one. It MAY also add new attributes and not >>> include some requested ones. Unrecognized or unsupported attributes >>> MUST be ignored in both requests and responses. >>> >>> So, the CFG_REPLY MUST have a DNS server to go along with the DNS domain, >>> but I left the SHOULD in spirit >>> of the fact that the CFG_REQUEST is more of a suggestion. >>> >>> That being said, if others in the WG would like to see this be a MUST, I'm >>> fine with that as well. I don't think we >>> should have the responder error out if it doesn't see both, however. >> >> Well, if it is only a suggestion, then I guess my question is why use >> something as strong as SHOULD :) SHOULD basically means >> MUST-unless-you-have-a-good-reason to. >> >> In general, is providing suggestions a SHOULD, or is it only for the >> attributes above? >> >> Anyway, if you want to have a SHOULD (or even a MUST) I won't object. But, >> when I see a SHOULD, I always ask about the background :) >> >> >> Q2: >> >>>> Section 3.2 says: >>>> >>>> "the initiator SHOULD behave as if Split DNS configurations are not >>>> supported >>>> by the server." >>>> >>>> Again, is there a reason for not using MUST? >>> >>> This one could be a MUST. The one exception I could see is if the initiator >>> was statically configured with some split DNS domains to use as split >>> domains >>> In case the responder didn't provide any in the CFG_REPLY, it should still >>> use those and not send all DNS queries inside the tunnel. I wouldn't want >>> this >>> MUST to disable the static configuration workarounds that implementations >>> have done prior to allowing split DNS to be negotiated. >> >> Could you say: >> >> "the initiator MUST behave as if a Split DNS configurations are not >> supported, unless <insert text above the statically configuration case >> above>" >> >> >> >> Nits/editorial comments: >> >> Q3: >> >>>> Is there a need for the "Background" section? Since the text is related to >>>> what >>>> is described in the "Introduction", could the the text be moved there? >>> >>> The main new points that the Background section adds on top of the >>> Introduction are: >>> - The prior art for split DNS in IKEv1 >>> - The fact that this is currently mainly seen in enterprise VPN deployments >>> >>> These points could indeed be moved to the introduction. I had felt they fit >>> better as "background" since they're >>> not essential to the description of the protocol, but give context that is >>> relevant now (and may be less so in the future). >> >> The first sections of both the Introduction and the Background sections talk >> about split DNS: >> >> "Split DNS is a common configuration for secure tunnels, such as >> Virtual Private Networks in which host machines private to an >> organization can only be resolved using internal DNS resolvers" >> >> "Split DNS is a common configuration for enterprise VPN deployments, >> in which one or more private DNS domains are only accessible and >> resolvable via an IPsec based VPN connection." >> >> So, isn't Split DNS already covered by the Introduction? What extra does the >> Background text bring? >> >> The second paragraph of the Background could be placed at the end of the >> Introduction, in my opinion. >> >> Regards, >> >> Christer >> >> _______________________________________________ >> IPsec mailing list >> IPsec@ietf.org <mailto:IPsec@ietf.org> >> https://www.ietf.org/mailman/listinfo/ipsec >> <https://www.ietf.org/mailman/listinfo/ipsec> > _______________________________________________ > Gen-art mailing list > gen-...@ietf.org > https://www.ietf.org/mailman/listinfo/gen-art
_______________________________________________ IPsec mailing list IPsec@ietf.org https://www.ietf.org/mailman/listinfo/ipsec
