Subject: [secdir] secdir review of draft-ietf-roll-unaware-leaves

From: Carl Wallace <[email-masked]>
Date: Wed, 09 Dec 2020 07:42:42 -0500

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.

---

From: "Pascal Thubert (pthubert)" <[email-masked]>
Date: Wed, 9 Dec 2020 18:43:32 +0000

Hello Carl

Many thanks for your deep review!

I submitted the proposed changes as rev 24 so you can investigate what I did in the diffs:
https://www.ietf.org/rfcdiff?url2=draft-ietf-roll-unaware-leaves-24

Let us see below 😊

> -----Original Message-----
> From: Carl Wallace <[email-masked]>
> Sent: mercredi 9 décembre 2020 13:43
> To: [email-masked]; [email-masked]; [email-masked]
> Subject: secdir review of draft-ietf-roll-unaware-leaves
> 
> 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.

Yes, section 3 and 4 are informative, to provide the context that we use in this spec. 
This way we can have references inside the document itself, to make the reading easier; from your review I realize it is still far from perfect.
So I changed the intro text to be clearer on the non-normative aspect of those sections:
"
   *  Section 3 and Section 4 present in a non-normative fashion the
      salient aspects of RPL and 6LoWPAN ND, respectively, that are
      leveraged in this specification to provide connectivity to a 6LN
      acting as a RUL across a RPL network.
"

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

Well, that was an earlier comment that I applied here. I find that the comment was correct though, see https://tools.ietf.org/html/rfc6687#section-4.2
"

   Path Stretch:  The path stretch metric encompasses the stretch factor
      for both hop distance and ETX path cost (as defined in the
      Terminology section).  The hop distance stretch, which is
      determined as the difference between the number of hops taken by a
      packet while following a route built via RPL and the number of
      hops taken by shortest path routing (using link ETX as the
      metric), is computed.  The ETX path cost stretch is also provided.
"
It was harder to find a good text like the above in RFC 6550. 
RFC 6550 focuses on the operation but does not insist that much on the consequences of using say, storing vs. non storing mode.
The idea was to leave that to books or informational applicability statements and keep the draft as short as possible. Which apparently we did not do swell enough either; the size of the RFC has played against it in the past, e.g., homenet.

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

True, the idea was that this section should not be normative. I removed the uppercase and indicated more in section 9.2. Then I added text in sections 9.2.1 and 9.2.2.

The text in 4.3.1 becomes:
"
   A 6LR that provides reachability services for a RUL in a RPL network
   as specified in this document includes a 6CIO in its RA messages and
   set the L, P and E flags as prescribed by [RFC8505], more in
   Section 9.2.
"

Then we now find:
"
9.2.1.  Perspective of the 6LN Acting as RUL

   This specification does not alter the operation of a 6LoWPAN ND-
   compliant 6LN/RUL, which is expected to operate as follows:

   1.  The 6LN selects a 6LR that provides reachability services for a
       RUL.  This is signaled a 6CIO in the RA messages with the L, P
       and E flags set to 1 as prescribed by [RFC8505].
"
And 
"
9.2.2.  Perspective of the 6LR Acting as Border Router

   A 6LR that provides reachability services for a RUL in a RPL network
   as specified in this document MUST include a 6CIO in its RA messages
   and set the L, P and E flags to 1 as prescribed by [RFC8505].
"

> 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.
 
Well, the problem is that the "expected" behavior is normatively imposed by another spec, RFC 8200 or RFC 8505 for the most part.

We avoided the normative paraphrasing with this trick. That was one of the deep exercises we made during A-D review.

For the sake of completeness I scanned the doc again. I did not find a place where I thought I could do this replacement.

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

Hey, that's when a partial information is misinformation. The full sentence is 

"
   To terminate the IP-in-IP tunnel, the RUL, as
   an IPv6 Host, must be able to decapsulate the tunneled packet and
   either drop the inner packet if it is not the final destination, or
   pass it to the upper layer for further processing.

"
It does not say " the RUL MUST be able to decapsulate " at all. There is a condition "To terminate the IP-in-IP tunnel", and then that's not normative text. So there's no inconsistency but maybe a hard text to read.
So maybe what your point is simply to replace "must" by "would" to clarify the conditional intention? 

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

Then again that's a normative paraphrase that we try to avoid. But I can quote.	

All in all we'd end up with

"
5.2.  Support of IPv6 Encapsulation

   Section 2.1 of [USEofRPLinfo] defines the rules for tunneling either
   to the final destination (e.g., a RUL) or to its attachment Router
   (designated as 6LR).  In order to terminate the IP-in-IP tunnel, the
   RUL, as an IPv6 Host, would have to be capable of decapsulating the
   tunneled packet and either drop the encapsulated packet if it is not
   the final destination, or pass it to the upper layer for further
   processing.  As indicated in section 4.1 of [USEofRPLinfo], this is
   not mandated by [RFC8504], so the Root typically terminates the IP-
   in-IP tunnel at the parent 6LR.  It is thus not necessary for a RUL
   to support IP-in-IP decapsulation.

"

> - What does "if the ’F’ flag is reset" mean? Does it just mean set to 0?

Yes. This used to be a clear language but apparently the fashion has changed. 
But damnit. In my old book a flag is up (set) or down (reset or unset). Arguable, yes, a bit is set to 0 or set to 1.
But OK if that's the most accepted terminology, let's use "set to 0" and "set to 1".
So I went through the doc and made the change (that's many times). I find it a lot less readable.

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

True, but the point is that -as a consequence of the meaning being unknown - " the size of the ROVR is undetermined and this node cannot validate the ROVR "

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

OK, the intention was to say that the target option is still forwarded as is even if there is an unknown the ROVR size. I think that the text says that and that your question is about the reverse logic. So I read that the intention is not to change the text.
There could be padding, and that padding does not have to be reproduced as is. An implementation may rebuild the DAOs from RIB. A legacy implementation may not parse beyond the target prefix and rebuild the DAOs without ROVR. What we are specifying now is to memorize the option and reproduce it as is if the ROVRSz is not understood, to ensure we propagate it all. This protects the future but opens the risk of a side channel.

So I added to the security considerations:
"
   Section 6.1 indicates that when the length of the ROVR field is
   unknown, the RPL Target Option must be passed on as received in RPL
   storing Mode.  This creates a possible opening for using DAO messages
   as a covert channel.  Note that DAO messages are rare and the
   overusing that channel could be detected.  An implementation SHOULD
   notify the network management when a RPL Target Option is receives
   with an unknown ROVR field size, to ensure that the situation is 
   known to the network administrator.
"

> - Should there be a "prefix length field MUST indicate 128 bits when F flag is set"
> instead of ignoring the length and assuming 128?

No. The prefix length must indicate the prefix length. The 'F' flag indicates that the IID is provided as well.

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

Yes, done.

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

The paragraph just above says:
"

   This specification extends the use of the DCO message to the Non-
   Storing MOP, whereby the DCO is sent end-to-end by the Root directly
   to the RAN that injected the DAO message for the considered target.
   In that case, intermediate nodes do not need to support
   [EFFICIENT-NPDAO]; they forward the DCO message as a plain IPv6
   packet between the Root and the RAN.
"
For RULs, the serving 6LR is the RAN

I added the first sentence below:
"
In the case of a RUL, the 6LR that serves the RUL acts as the RAN that receives
the Non-Storing DCO.
This specification leverages the Non-Storing DCO between the Root and the 6LR
 that serves as attachment Router for a RUL. A 6LR and a Root that support this
 specification MUST implement the Non-Storing DCO.
"

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

I changed reset throughout to "set to 0"

> 
> 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.
> 
Great catch. Setting the "T" flag to 1 is actually mandated in RFC 8505:
"
   o  A node that supports this specification MUST provide a TID field
      in the EARO and set the T flag to indicate the presence of the TID
      (see Section 5.2).
"

The uppercase was a mistake. I fixed to
"
   compliant 6LN/RUL, which is expected to operate as follows:

   1.  The 6LN selects a 6LR that provides reachability services for a
       RUL.  This is signaled a 6CIO in the RA messages with the L, P
       and E flags set to 1 as prescribed by [RFC8505].

   2.  The 6LN obtains an IPv6 global address, either using Stateless
       Address Autoconfiguration (SLAAC) [RFC4862] based on a Prefix
       Information Option (PIO) [RFC4861] found in an RA message, or
       some other means, such as DHCPv6 [RFC8415].

   3.  Once it has formed an address, the 6LN registers its address and
       refreshes its registration periodically, early enough within the
       Lifetime of the previous Address Registration, as prescribed by
       [RFC6775], to refresh the NCE before the lifetime indicated in
       the EARO expires.  It sets the T Flag to 1 as prescribed in
       [RFC8505]. The TID is incremented each time and wraps in a
       lollipop fashion (see section 5.2.1 of [RFC8505], which is fully
       compatible with section 7.2 of [RFC6550]).

"

Again, many thanks for your review!

Please let me know if we are good, and keep safe;

Pascal