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 Thanks, Tommy > On Aug 19, 2018, at 1:39 PM, Christer Holmberg > <[email protected]> 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 > [email protected] <mailto:[email protected]> > https://www.ietf.org/mailman/listinfo/ipsec > <https://www.ietf.org/mailman/listinfo/ipsec>
_______________________________________________ IPsec mailing list [email protected] https://www.ietf.org/mailman/listinfo/ipsec
