Re: [IPsec] Robert Wilton's Discuss on draft-ietf-ipsecme-add-ike-11: (with DISCUSS and COMMENT)

["Rob Wilton (rwilton)" <[e-mail]>]

Hi Med,

> -----Original Message-----
> From: [e-mail] <[e-mail]>
> Sent: 27 April 2023 10:48
> To: Rob Wilton (rwilton) <[e-mail]>; The IESG <[e-mail]>
> Cc: [e-mail]; [e-mail];
> [e-mail]; [e-mail]
> Subject: RE: Robert Wilton's Discuss on draft-ietf-ipsecme-add-ike-11: (with
> DISCUSS and COMMENT)
> 
> Hi Rob,
> 
> Thanks for the review.
> 
> Please see inline.
> 
> Cheers,
> Med
> 
> 
> Orange Restricted
> 
> > -----Message d'origine-----
> > De : Robert Wilton via Datatracker <[e-mail]>
> > Envoyé : jeudi 27 avril 2023 11:19
> > À : The IESG <[e-mail]>
> > Cc : [e-mail]; [e-mail];
> > [e-mail]; [e-mail]; [e-mail]
> > Objet : Robert Wilton's Discuss on draft-ietf-ipsecme-add-ike-11:
> > (with DISCUSS and COMMENT)
> >
> > Robert Wilton has entered the following ballot position for
> > draft-ietf-ipsecme-add-ike-11: Discuss
> >
> > When responding, please keep the subject line intact and reply to
> > all email addresses included in the To and CC lines. (Feel free to
> > cut this introductory paragraph, however.)
> >
> > ------------------------------------------------------------------
> > ----
> > DISCUSS:
> > ------------------------------------------------------------------
> > ----
> >
> > Hi,
> >
> > Thanks for this document.
> >
> > This should be a trivial discuss to resolve, and only flagging it
> > as a discuss
> > because I think that it makes the spec unclear (or wrong):
> >
> > (1) p 4, sec 3.1.  ENCDNS_IP* Configuration Payload Attributes
> >
> >    *  IP Address(es) (variable) - Includes one or more IP
> > addresses that
> >       can be used to reach the encrypted DNS resolver identified
> > by the
> >       Authentication Domain Name.  For ENCDNS_IP4 this field
> > contains
> >       one or more 4-octet IPv4 addresses, and for ENCDNS_IP6 this
> > field
> >       contains one or more 16-octet IPv6 addresses.
> >
> > Shouldn't this be zero or more IP addresses?  Otherwise, the
> > example that only
> > contains a domain and no IP address appears to be invalid.
> >
> 
> [Med] That text is correct. The field is present only when there is an IP address
> to convey; otherwise the field is skipped. The presence is indicated by this field:
> 
> Num Addresses (1 octet) - Indicates the number of enclosed IPv4 (for
> ENCDNS_IP4) or IPv6 (for ENCDNS_IP6) addresses.
[Rob Wilton (rwilton)] 

I've just rechecked it, and I still don't find the text clear in that section that this field is optional.  I think that some more words are required somewhere :-)

E.g., the overall length field is defined like this:

   *  Length (2 octets, unsigned integer) - Length of the enclosed data
      in octets.  In particular, this field is set to:

      -  0 if the Configuration payload has types CFG_REQUEST (if no
         specific DNS resolver is requested) or CFG_ACK.  If the
         'Length' field is set to 0, then later fields shown in Figure 1
         are not present.

ADN Length is defined as:

   *  ADN Length (1 octet) - Indicates the length of the "Authentication
      Domain Name" field in octets.  When set to '0', this means that no
      ADN is enclosed in the attribute.

Whereas, Num Addresses is defined as:

   *  Num Addresses (1 octet) - Indicates the number of enclosed IPv4
      (for ENCDNS_IP4) or IPv6 (for ENCDNS_IP6) addresses.  This value
      MUST NOT be set to 0 if the Configuration payload is of type
      CFG_REPLY or CFG_SET.

- This doesn't indicate that 0 addresses allowed (which might be okay), but it also doesn't indicate that the IP Addresses field is absent if there are no addresses.

As in I would still read section 3.1 in its entirety as needing as least 1 IP address to be specified unless ADN Length is set to 0.

Thanks,
Rob


> 
> >
> > ------------------------------------------------------------------
> > ----
> > COMMENT:
> > ------------------------------------------------------------------
> > ----
> >
> > Minor level comments:
> >
> > (2) p 0, sec
> >
> >    This document specifies new Internet Key Exchange Protocol
> > Version 2
> >    (IKEv2) Configuration Payload Attribute Types to assign DNS
> > resolvers
> >    that support encrypted DNS protocols, such as DNS-over-HTTPS
> > (DoH),
> >    DNS-over-TLS (DoT), and DNS-over-QUIC (DoQ).
> >
> > Are there any updates needed to RFC 9061 needed to cover
> > manageability
> > aspects/updates of the attributes defined in this draft?  Note,
> > I'm not
> > requesting that they be added to this draft, but instead, I want
> > to check if
> > there is any need or plan to address them.
> 
> Med: Not AFAIK.
> 
> 
> >
> > (3) p 2, sec 2.  Terminology
> >
> >    Do53:  refers to unencrypted DNS.
> >
> > This term only turns up a few (3 times) in this doc, and its not
> > clear to me
> > that it improves its readability.  I didn't know what it meant,
> > possibly just
> > referencing to "Unencrypted DNS" would be better for the wider
> > audience?
> >
> 
> [Med] Do53 is widely used but without a reference. I prefer to maintain in this
> section. Thanks.
> 
> > (4) p 3, sec 3.1.  ENCDNS_IP* Configuration Payload Attributes
> >
> >       -  0 if the Configuration payload has types CFG_REQUEST (if
> > no
> >          specific DNS resolver is requested) or CFG_ACK.  If the
> >          'Length' field is set to 0, then later fields shown in
> > Figure 1
> >          are not present.
> >
> > I found this text unclear & confusing when combined with the
> > following two
> > paragraphs.  I would suggest rewording the first sentence to
> > something like:
> >
> >    0, if the Configuration payload has (i) type CFG_REQUEST and no
> >    specific DNS resolver is requested or (ii) type CFG_ACK.
> >
> 
> [Med] OK.
> 
> 
> > (5) p 13, sec Appendix A.  Sample Deployment Scenarios
> >
> > Readability may be slightly improved by adding a sentence here to
> > explain what
> > the purpose of this section is.
> 
> [Med] We have a sentence in the intro:
> 
>    Sample use cases are described in Appendix A.  The Configuration
>    Payload Attribute Types defined in this document are not specific to
>    these deployments, but can also be used in other deployment contexts.
>    It is out of the scope of this document to provide a comprehensive
>    list of deployment contexts.
> 
> Will see if we can make a change.
> 
> >
> > (6) p 14, sec Appendix A.  Sample Deployment Scenarios
> >
> >    Enterprise networks are susceptible to internal and external
> > attacks.
> >    To minimize that risk all enterprise traffic is encrypted
> >    (Section 2.1 of [I-D.arkko-farrell-arch-model-t]).
> >
> > Would "SHOULD be encrypted" be better than "is encrypted"? Or,
> > alternatively,
> > "Encrypting all internal enterprise traffic minimizes the risks of
> > attacks
> > (Section 2.1 of [I-D.arkko-farrell-arch-model-t]).
> >
> 
> [Med] We removed that sentence as per a comment from Paul.
> 
> 
> > (7) p 14, sec Appendix B.  Examples
> >
> > I would suggest putting each of the two examples into its own
> > subsection.
> >
> 
> [Med] OK.
> 
> > Nit level comments:
> >
> > (8) p 4, sec 3.1.  ENCDNS_IP* Configuration Payload Attributes
> >
> >       -  0 if the Configuration payload has types CFG_REQUEST (if
> > no
> >          specific DNS resolver is requested) or CFG_ACK.  If the
> >          'Length' field is set to 0, then later fields shown in
> > Figure 1
> >          are not present.
> >       -  (4 + Length of the ADN + N * 4 + Length of SvcParams) for
> >          ENCDNS_IP4 attributes if the Configuration payload has
> > types
> >          CFG_REQUEST or CFG_REPLY or CFG_SET; N being the number
> > of
> >          included IPv4 addresses ('Num addresses').
> >
> > Possibly "(4 + 'Length of the ADN' + (N * 4) + Length of
> > SvcParams)", and
> > similarly for IPv6, would be more explicit.
> >
> 
> [Med] OK
> 
> > (9) p 5, sec 3.2.  ENCDNS_DIGEST_INFO Configuration Payload
> > Attribute
> >
> >    *  Length (2 octets, unsigned integer) - Length of the enclosed
> > data
> >       in octets.  This field MUST be set to "2 + 2 * number of
> > included
> >       hash algorithm identifiers".
> >
> > For clarity, I suggest: "2 + (2 * 'number of included
> >       hash algorithm identifiers')"
> >
> 
> [Med] OK
> 
> > (10) p 6, sec 3.2.  ENCDNS_DIGEST_INFO Configuration Payload
> > Attribute
> >
> >    *  Length (2 octets, unsigned integer) - Length of the enclosed
> > data
> >       in octets.  This field MUST be set to "2 + 2 * number of
> > included
> >       hash algorithm identifiers".
> >    *  Num Hash Algs (1 octet) - Indicates the number of included
> > hash
> >       algorithm identifiers.  This field MUST be set to "(Length -
> >       2)/2".
> >
> > I suggest, included 'hash algorithm identifiers'.
> >
> 
> [Med] OK
> 
> > Regards,
> > Rob
> >
> >
> 
> ________________________________________________________________
> _________________________________________________________
> 
> Ce message et ses pieces jointes peuvent contenir des informations
> confidentielles ou privilegiees et ne doivent donc
> pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce
> message par erreur, veuillez le signaler
> a l'expediteur et le detruire ainsi que les pieces jointes. Les messages
> electroniques etant susceptibles d'alteration,
> Orange decline toute responsabilite si ce message a ete altere, deforme ou
> falsifie. Merci.
> 
> This message and its attachments may contain confidential or privileged
> information that may be protected by law;
> they should not be distributed, used or copied without authorisation.
> If you have received this email in error, please notify the sender and delete this
> message and its attachments.
> As emails may be altered, Orange is not liable for messages that have been
> modified, changed or falsified.
> Thank you.