Elwyn: On behalf of the authors of draft-ietf-dhc-rfc3315bis, thanks much for the very thorough review (most of your comments look appropriate to apply). - Bernie -----Original Message----- From: Elwyn Davies [mailto:elwynd@xxxxxxxxxxxxxx] Sent: Thursday, January 18, 2018 1:00 PM To: gen-art@xxxxxxxx Cc: dhcwg@xxxxxxxx; ietf@xxxxxxxx; draft-ietf-dhc-rfc3315bis.all@xxxxxxxx Subject: Genart telechat review of draft-ietf-dhc-rfc3315bis-10 Reviewer: Elwyn Davies Review result: Almost Ready [This is a Last Call Review, not a telechat review]. 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 <https://trac.ietf.org/trac/gen/wiki/GenArtfaq>. Document: draft-ietf-dhc-rfc3315bis-10 Reviewer: Elwyn Davies Review Date: 2018-01-18 IETF LC End Date: 2018-01-24 IESG Telechat date: 2018-01-25 Summary: Amost ready. To the best of my abiity I have done a thorough check of the merging of the various earlier documents into the new -bis and the incorporation of the various Errata. All seems to be good from this point of view. I am slightly surprised that two other RFCs (7283 and 8213) were not subsumed into this document. Both are pure updates of 3315 with some important improvements and are pretty short in themselves. I also found that the new document did not do a good job on the interactions between relay-agents and servers. Particularly on the processing of options between relays and servers and details of reception of Relay-forward messages a little more detail would halp naive readers. Finally the draft has not made much of an effort to support possible alternative security algorithms - IETF policy now encourages protocols to deal with algorithm flexibility - DHCPv6 as it stands is pretty thoroughly wedded to HMAC-MD5 and would need some (relatively small) IANA improvements plus some thought to cope with different algorithms. There are also a fair number of nits, partly due to inconsistent cross referencing in s18. Major Issues: None Minor Issues: Non-incorporation of RFC 7283: Is there a good reason why the update of relay agent behaviour (which is apparently of general application across DHCPv6) is not incorporated into this document as with several other items? The update is short and would be readily addressed in s19 here. As it is one is left with another document with some key modifications left around rather than achieving the unification aim of this -bis. Non-incoporation of RFC 8213: Similarly regarding use of IPsec for server-relay agent comms. Definitions of T1 andT2: I find the definitions of T1 and T2 as replicated in many places to be a little confusing. T1 and T2 are actually time intervals rather than absolute times. I suggest a global substitution as follows: OLD: The time at which the client contacts the server NEW: The time interval after which the client would be expected to contact the server END s6: Might be worth pointing out the pitfalls of defining additional stateful services as had to be sorted out with RFC 7550. s11.2: [This is not explicitly an issue with this document, but affects its usage] The list of hardware types originally listed for ARP, is now well out of date - in particular it doesn't cover Ethernet interfaces faster than 10Mb/s. Perhas somebody could suggest some updates to Carlos Pignatoro (the designated expert). s12.2: > A client must create at least one distinct > IA_PD. Is it not the case that the majority of simple hosts are not concerned with IA_PD? I think this needs qualifying to say that if a client is interested in obtaining a prefix, then it has to create an IA_PD. s18.3, s18.3.11 and s19, Recording and/or reproduction of the relay address chain in servers: I found the lack of description of server handling of received messages embedded in relay-forward messages in s18.3 required considerable searching to understand what needed to be done, and there is no mention of the expectation that the relay address chain would (or at least might) need to be recorded to facilitate later generation of Reconfigure messages that are originated by the server. I think it would be helpful to add some words about this is in s18.3 as follows: ADD (probably as new para 6, et seq): All messages sent from clients may arrive encapsulated in Relay-forward messages. For example, if client C sent a message that was relayed by relay agent A to relay agent B and then to the server, the server would receive the following Relay-forward message to process and, in general, generate an appropriate Relay-reply to be returned to the client: msg-type: RELAY-FORWARD hop-count: 1 link-address: 0 peer-address: A Relay Message option, containing: msg-type: RELAY-FORWARD hop-count: 0 link-address: address from link to which C is attached peer-address: C Relay Message option: <client message> Figure xx: Relay-forward Example Thus the server MUST record the sequence of link addresses and peer addresses together with their associated hop count values to allow the response to the received message to be relayed through the same relay agents as the original client message (see Section 19.3). The server may also need to record this information in association with the client's DUID [I think?] in case it is needed to send a Reconfigure message at a later time unless the server has been configured with addresses that can be used to send Reconfigure messages (see Section 18.3.11). For messages that are not encapsulated in a Relay-forward message responses will be sent to the source address of the received message; this source address becomes the destination of the Relay-reply message in the case of Relay-forward messages (see Sections 18.3.10 and 18.3.11). END s19: The treatment of additonal options in communications between relay-agents and servers is extremely sketchy. It appears that extensive use is now made of this capability which was hardly envisaged when RFC 3315 was written. I think it would be helpful to include a little more information about how this capability is used and maybe pointers to a couple of examples in other RFCs that represent common usages. s20.4: Possibility of using alternative HMAC algorithms? Whilst it appears that HMAC-MD5 is still adequate and not significantly compromised, the draft does not make much provision for possible alternatives (there are no registries for alternative parameters in the authentication option (Section 21.11) and s20 is restricted to HMAC-MD5. Nits/editorial: General: Conventionally tables are given titles and numbers. s1, para 1: Might be worth adding in a second sentence to clarify the reason for the relay agents: "The basic operation of DHCP provides configuration for clients connected to the same link as the server." s1, para 4: s/provide only/deliver nothing but/ s1, para 4: Mention that the stateless mode was defined in RFC 3736. s1.1: It would be helpful to indicate that all the various Errata have been processed and point at Appendix A for complete listing of changes. s1.1, next to last sentence: s/errate filled/errata filed/ s1.1, last sentence: s/aforementioned/the aforementioned/ s3, para 1: The main IPv6 specification is now RFC 8200 rather than RFC 2460 [idnits gets very upset!] s3, para 2: s/address scope/address scopes/ s4.2: The term "encapsulated option" is used in several places in this draft and defined in [RFC7227]. It would be good to have a brief introduction here as well as a pointer to Section 9 of RFC 7227. s4.2, "binding": Probably worth notng that IA* and DUID are defined later in s4.2. s4.2, "relaying": Add "typically conected to different links". s4.2, "Singleton option": s/a the/a/ s5.3: OLD: The client then performs the earlier described two message exchange. NEW: The client then performs the two message exchange as described earlier. END s6.1, para 1, 2nd sentence: s/Stateless may be used/Stateless DHCP may be used/ s6.3, para 3: s/hence use/hence the use/ (2 places) s6.3, para 4: s/provided prefixes/provisioned with prefixes/ s6.5, para 1: Is the reference to RFC 3041 essential? RFC 4941 obsoletes it. [and idnits enquires] s6.6, para 3: s/to request larger prefix/to request a larger prefix/ s7.2: The references for the DHCP ports at https://www.iana.org/assignments/service-names-port-numbers/service-names-port-numbers.xhtml?&page=10 are currently associated with Jim Bound. Unfortunately Jim has passed on many years ago. I suggest asking IANA to update the references to this new RFC. Also the TCP port numbers are reserved for DHCP and are used in other DHCPv6 functions (e.g., RFC5460 but there is no allocation reference. ) ss9.1/9.2: What options might one get along with the encapsulated message? If so where are they processed? See the Minor Issue raised against s19 above. s11.2, para 1: The reference for hardware types probably ought to be RFC 5494 and it would handy to add in the IANA page address (https://www.iana.org/assignments/arp-parameters/arp-parameters.xhtml#arp-parameters-2). As mentioned in Minor Issues, relevant hardware types for higher speed Ethernets (and potentially other types) are missing from this list. s14.2: T1 and T2 are time intervals: s/time(s)/time interval(s)/g s15: It would help to indicate that specific values for the various retransmission parameters for each message type are given in the sections 18.2.x and use default values taken from the table in s7.6. Suggest adding after list of variables: ADD: Specfic values for each of these parameters relevant to the usages of the various messages are given in the sub-sections of Section 18.2 using values defined in the table in Section 7.6. The selection algorithm for RAND is common across all message transmissions. END s17.2, para 2: s/(ISP network)/(typically, connected to an ISP network)/ s18, Fig9: Would client always send request to non-selected server? And would it renew/release on non-selected server? s18 (multiple places): In many places the first reference to an option in a subsection is accompanied by a pointer to the part of s21 that defines it (Good!). However this is not consistent. I have picked up several cases where the reference is missing but there may be others. Please check and make it consistent. However, in the case of Client Identifier and Server Identifier options and the IA options, they appear so many times that it might be worth just adding a paragraph about the identifiers with the reference pointers in Section 18. s18.1: > For a rationale of this > approach, see Section 4.2 of [RFC7550]. I think the rationale is in s4.3 of RFC 7550. IMO I think I would have copied the rationale across - it is short and 7550 is being obsoleted. s18.2, para 3: The reference to RFC 7550 does not appear to add any value and since this doc is obsoleting RFC 7550 I think the ref should be removed. s18.2, para 5: s/Server Unicast option Section 21.12/Server Unicast option (see Section 21.12)/ s18.2.1, para 6: s/IA Prefix options/IA Prefix options (see Section 21.22)/ s18.2.1, para 10: s/descynronize/desynchronize/ s18.2.1, para 12 (after retransmit parameters): s/Rapid Commit option/Rapid Commit option (see Section 21.14)/ s18.2.3, para 5: s/IA Address options/IA Address options (see Section 21.6)/ s18.2.4, para 1: s/IA Address options/IA Address options (see Section 21.6)/, s/IA Prefix options/IA Prefix options (see Section 21.22)/ s18.2.9, para 8: s/The client MUST process SOL_MAX_RT and INF_MAX_RT options/The client MUST process SOL_MAX_RT and INF_MAX_RT options (see Sections 21.24 and 21.25)/ s18.2.10, para 2: (same as last comment) s18.2.10, para 6: s/Client FQDN option/Client FQDN option [RFC4704]/ s18.2.10.1: > - Record T1 and T2 times, if appropriate for the IA type. The T1 and T2 'times' received in the message are (as mentioned above) are actually intervals: The client needs to convert these to absolute times in some way (may of course be by setting an interval timer running!) but should the interval just run from the time of receipt of the message - or something more complicated? This should be noted here. s18.2.10.1, para 19 (I think): > - Sends a Renew/Rebind if any of the IAs are not in the Reply > message, but in this case the client MUST limit the rate at which > it sends these messages, to avoid the Renew/Rebind storm. > The term Renew/Rebind storm is used here just once and without definition. I am sure that I can see that one might happen but I think the circumstances envisioned need to be spelled out. Maybe already are at the end of s18.2.12? s18.3, para 5: s/Option Request option/Option Request option (see Section 21.7)/ ss18.3.1, 18.3.3, 18.3.5, para 1 in each case: s/regardless if/regardless of whether/ s18.3.2, para 6: s/Status Code option/Status Code option (see Section 21.13)/ s18.3.2, para 11: s/Reconfigure Accept option/Reconfigure Accept option (see Section21.20)/ s18.3.3, last para: s/Status Code option/Status Code option (see Section 21.13)/ s18.3.4, para 9: s/Status Code option/Status Code option (see Section 21.13)/ s18.3.5, para 8: s/Status Code option/Status Code option (see Section 21.13)/ s18.3.7, para 4: s/Status Code option/Status Code option (see Section 21.13)/ s18.3.8, para 4: s/Status Code option/Status Code option (see Section 21.13)/ s18.3.9, para 3: s/Reconfigure Accept option/Reconfigure Accept option (see Section 21.20)/ s18.3.9, para 4: s/Option Request option/Option Request option (see Section 21.7)/ s18.3.10, para 2: s/Interface-id option/Interface-id option (see Section 21.18)/ s18.3.11, para 2: Provide a pointer to Section 20 for DHCP Authentication. s18.2.11, para 5: I suggested in the Minor Issues that some text be added to s18 regarding collection of addressing information for use with Reconfigure messages. If this is done, a pointer back to this text would be useful here. s18.4, para 1: s/Server Unicast option/Server Unicast option (see Section 21.12)/ s18.4, para 2: s/Status Code option/Status Code option (see Section 21.13)/ s19.1, para 1: As mentioned above (in Minor Issues), I don't see why the essence of RFC 7283 is not incorporated into this document. s20.3: Does there need to be a registry for RDM values? s21: Including a general statement that general numeric values are encoded as unsigned integers would be helpful - this is not done consistently through the options at present. s21.4, last para: s/is taken to ("infinity")/is taken to mean "infinity"/ I think this para would be more useful as the third last para, i.e., moved up two paras to just after > The values in the T1 and T2 > fields are the number of seconds until T1 and T2. s21.8: The pref_value should be specified as an unsigned integer. s24: It would probably good to take on the allocations of well-known ports as owned by this document (see comment of s7.2) For future proofing of authentication, it might be desirable to start registries for some of the DHCP Authentication parameters. s24, Note (4) on Table 1: Need to make RFC8156 an informative ref. s27.1: RFC 7083 is being obsoleted... making it a normative reference is not allowed. s27.2: I would say that RFC 8213 needs to be normative unless (as I would recommend) the text is brought into this document, in which case it is unnnecessary.