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

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 <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>