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

[Robert Wilton via Datatracker <[e-mail]>]

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


Please refer to https://www.ietf.org/about/groups/iesg/statements/handling-ballot-positions/ 
for more information about how to handle DISCUSS and COMMENT positions.


The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-ipsecme-add-ike/



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


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

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

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

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

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

(7) p 14, sec Appendix B.  Examples

I would suggest putting each of the two examples into its own subsection.

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.

(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')"

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

Regards,
Rob