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.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
> ip...@ietf.org <mailto:ip...@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

Reply via email to