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

[Christer Holmberg]

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 "



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
https://www.ietf.org/mailman/listinfo/ipsec

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

[Tommy Pauly]

Hi Christer,

Thanks for the review! Some responses inline.

Best,
Tommy

> On Aug 16, 2018, at 11:25 PM, Christer Holmberg 
>  wrote:
> 
> Reviewer: Christer Holmberg
> Review result: Ready with Nits
> 
> I am the assigned Gen-ART reviewer for this draft. The General Area
> Review Team (Gen-ART) reviews all IETF documents being processed
> by the IESG for the IETF Chair.  Please treat these comments just
> like any other last call comments.
> 
> For more information, please see the FAQ at
> 
> .
> 
> Document: draft-ietf-ipsecme-split-dns-12
> Reviewer: Christer Holmberg
> Review Date: 2018-08-16
> IETF LC End Date: 2018-08-24
> IESG Telechat date: Not scheduled for a telechat
> 
> Summary: The document is well written, and almost ready for publication. I 
> have
> a couple of questions that I would like the authors to address.
> 
> Major issues: N/A
> 
> 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.


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

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

> 
> 
> ___
> IPsec mailing list
> IPsec@ietf.org
> https://www.ietf.org/mailman/listinfo/ipsec

___
IPsec mailing list
IPsec@ietf.org
https://www.ietf.org/mailman/listinfo/ipsec

[IPsec] Genart last call review of draft-ietf-ipsecme-split-dns-12

[Christer Holmberg]

Reviewer: Christer Holmberg
Review result: Ready with Nits

I am the assigned Gen-ART reviewer for this draft. The General Area
Review Team (Gen-ART) reviews all IETF documents being processed
by the IESG for the IETF Chair.  Please treat these comments just
like any other last call comments.

For more information, please see the FAQ at

.

Document: draft-ietf-ipsecme-split-dns-12
Reviewer: Christer Holmberg
Review Date: 2018-08-16
IETF LC End Date: 2018-08-24
IESG Telechat date: Not scheduled for a telechat

Summary: The document is well written, and almost ready for publication. I have
a couple of questions that I would like the authors to address.

Major issues: N/A

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?

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?

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?


___
IPsec mailing list
IPsec@ietf.org
https://www.ietf.org/mailman/listinfo/ipsec