[dhcwg] Genart telechat review of draft-ietf-dhc-rfc3315bis-10

[Elwyn Davies <[e-mail]>]

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.