Re: [IPsec] [Gen-art] Genart last call review of draft-ietf-ipsecme-split-dns-12

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 <tpauly@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.holmberg@ericsson.com <mailto:christer.holmberg@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-art@ietf.org
> https://www.ietf.org/mailman/listinfo/gen-art