Last Call Review of draft-ietf-roll-unaware-leaves-23
review-ietf-roll-unaware-leaves-23-secdir-lc-wallace-2020-12-10-00

Request Review of draft-ietf-roll-unaware-leaves
Requested rev. no specific revision (document currently at 30)
Type Last Call Review
Team Security Area Directorate (secdir)
Deadline 2020-12-09
Requested 2020-11-13
Authors Pascal Thubert, Michael Richardson
Draft last updated 2020-12-10
Completed reviews Iotdir Last Call review of -13 by Shwetha Bhandari (diff)
Rtgdir Last Call review of -23 by Julien Meuric (diff)
Iotdir Last Call review of -23 by Peter Van der Stok (diff)
Secdir Last Call review of -23 by Carl Wallace (diff)
Genart Last Call review of -24 by Elwyn Davies (diff)
Assignment Reviewer Carl Wallace 
State Completed
Review review-ietf-roll-unaware-leaves-23-secdir-lc-wallace-2020-12-10
Posted at https://mailarchive.ietf.org/arch/msg/secdir/dmLRgWOu6PcYcRdn8FpjnYOwbRU
Reviewed rev. 23 (document currently at 30)
Review result Has Nits
Review completed: 2020-12-09

Review
review-ietf-roll-unaware-leaves-23-secdir-lc-wallace-2020-12-10

I reviewed this document as part of the Security Directorate's ongoing effort to review all IETF documents being processed by the IESG.  These comments were written primarily for the benefit of the Security Area Directors.  Document authors, document editors, and WG chairs should treat these comments just like any other IETF Last Call comments.

This specification updates RFC6550, RFC6775, and RFC8505, to provide routing services to RPL Unaware Leaves that implement 6LoWPAN ND and the extensions therein. The changes described in the draft largely consist of defining some previously undefined reserved flags (including corresponding inclusion of ROVR in existing message), redefining some status messages and extending use of an existing message for additional mode of operation. Some questions and comments are below. These could all be categorized minor nits focused on improving clarity.

General
- The document feels long given the magnitude of changes. There are some explanatory sections that may be better off left as references to the normative specs. As someone unfamiliar with ROLL, I found reconciling explanatory text here with source docs difficult in spots.

Section 1
- In the introduction, the reference to RFC6687 in the first sentence of the third paragraph seems misplaced. While the term 'path stretch' appears in that document, the concept being referenced doesn't seem to match and may be better served by a reference to section 3.1 of RFC 6550.

Section 4.3.1
- The last sentence in section 4.3.1 probably belongs in (or should be repeated) in Section 8. It seems odd to feature fresh standards language in a section that is providing background but not in the section enhancing the referenced doc.

Section 5.1, 5.3 and 5.4
- There are a number of "is expected" instances that may benefit from being written as SHOULD/SHOULD NOT or MUST/MUST NOT.

Section 5.2
- Language is unclear on whether decapsulation is required by a RUL. The statement "the RUL, as an IPv6 Host, must be able to decapsulate the tunneled packet" is inconsistent with the last statement in the paragraph. Maybe change first statement to "If a RUL supports terminating an IP-in-IP tunnel...".
- The statement "the Root terminates" may benefit from a SHOULD. The sentence establishes when a SHOULD would not apply already. [USEofRPLinfo] has a SHOULD when making this same point in 4.1. Replacing this entire paragraph with the fourth paragraph in section 4.1 may be the right thing to do.

Section 6.1:
- What does "if the ’F’ flag is reset" mean? Does it just mean set to 0?
- In the ROVRsz definition, why would values above 4 result in size being unknown? Maybe this should say the meaning is unknown for values above 4. 
- It states "an implementation SHOULD propagate the whole Target Option" when ROVRsz is greater than 4. In what cases should the whole target option not be propagated? 
- Should there be a "prefix length field MUST indicate 128 bits when F flag is set" instead of ignoring the length and assuming 128? 
- Should this section require bits not claimed by this spec to be set to zero as in 6550? This assumes this spec is only claiming 5 of 8 bits. That may be worth clarifying as well.

Section 7
- What does "A 6LR and a Root that support this specification MUST implement the Non-Storing DCO" mean? The only definition of "non-storing DCO" appears to be in the previous paragraph, which does not apply to the 6LR.

Section 9
- What does 'reset' mean in second paragraph? It seems to mean "not set".

Section 9.2.1
- This section begins by noting no changes are defined for the described process that is defined by various other specs. The section describes many requirements including some that look to be new, i.e., first paragraph below the bulleted list.