Ballot for draft-ietf-roll-enrollment-priority
Discuss
Yes
No Objection
No Record
Summary: Has 5 DISCUSSes. Needs 3 more YES or NO OBJECTION positions to pass.
Section 5: From this section I understand that secured layer 2 is common, adding security at layer 3 has not been implemented (specified in RFC6550 - circa 2012). The only asserted mitigations for layer 2 security, which I would call 'red switch vulnerabilities' has not been implemented. But if layer 3 security were to be added, it would fix all the vulnerabilities? Section 5, para 2: The mix of encryption and privacy is not clear. Does encryption guarantee privacy (not always the case), it classically provides confidentiality. Please clarify how wireless, layer 2 encryption protects the security and privacy of this extension. Section 5, para 6: There is no mention in RFC9031 about rekeying to remove a node from the network. It is used as a mechanism to update the parameter sets. Please clarify.
Thanks to Rifaat Shekh-Yusef for their secdir review. I support Mahesh's discuss. General: This draft is hard to parse, what is the objective and what is being specified? In addition, there are many typos. Section 1, last paragraph: Put this para first. It actually says what the draft is about. Section 1.1, para 4: The last sentence isn't quite right? 'the introduction of a lot of traffic from could be part' maybe remove 'from'? Section 1.1, para 6: There are two uses of the term 'Join', and one of them is commonly called 'onboarding', however this draft chooses to use the word 'enrollment'. This adds more confusion. Section 4, para 1: There are no management protocols for RPL, so there is no way to establish a default to allow a node to join. What's the plan? Section 4, para 2: How does firmware update come into play here? Section 4, para 3: The term 'metric container' is used twice in this draft, only in this section. What is it?
# Éric Vyncke INT AD comments for draft-ietf-roll-enrollment-priority-18 CC @evyncke Thank you for the work put into this document. Please find below some blocking DISCUSS points (easy to address), some non-blocking COMMENT points/nits (replies would be appreciated even if only for my own education). Special thanks to Ines Robles for the shepherd's concise write-up including the WG consensus *BUT* it lacks the justification of the intended status. Please note that Toerless Eckert is the IoT directorate reviewer (at my request) and you may want to consider this review as well when it will be available (no need to wait for it though): https://datatracker.ietf.org/doc/draft-ietf-roll-enrollment-priority/reviewrequest/24749/ I hope that this review helps to improve the document, Regards, -éric Note: this ballot comments follow the Markdown syntax of https://github.com/mnot/ietf-comments/tree/main, i.e., they can be processed by a tool to create github issues. ## DISCUSS (blocking) As noted in https://datatracker.ietf.org/doc/statement-iesg-handling-ballot-positions-20220121/, a DISCUSS ballot is a request to have a discussion on the points below; I really think that the document would be improved with a change here, but can be convinced otherwise. ### Section 3.2 A trivial fix, but `and MAY increment the Min Priority, if it is less than 0x7f` is ambiguous whether the 'if' must be checked before or after the increment operation. ### Section 4.1 Why not a "MUST" in `SHOULD assume a default value of 0x40`? This would sound sensible and also fits the interoperation requirement of BCP14 (see also https://datatracker.ietf.org/doc/statement-iesg-statement-on-clarifying-the-use-of-bcp-14-key-words/). ### Section 6 s/This entry should be called Minimum Enrollment Priority/This entry *has a meaning of 'Minimum Enrollment Priority'*/
## COMMENTS (non-blocking) ### 6tisch The text in section 1 and in the abstract seems to limit the applicability to 6tisch, but the text is unclear whether there are (or in the future) other use cases. ### Section 1 Please add an informative reference to `IEEE 802.15.4 Enhanced Beacon`. Suggest: s/This document introduces/This document *specifies*/ as it is PS. ### Section 1.1 s/option described here/option *specified* here/ as it is PS. ### Section 3.2 Guidance will be welcome by the implementers about `if the change is considered important (i.e., it is expected to propagate in the DODAG quickly)`.
Section 3.2, Option Processing:
299 > The contents of the option MUST be generated by the DODAG Root. A
300 > 6LR MAY change only the Version Number (in lollipop fashion), and MAY
301 > increment the Min Priority, if it is less than 0x7f.
...
323 > A 6LR, which would otherwise be willing to act as a _Join Proxy_,
324 > will examine the locally adopted value of minimum enrollment priority
325 > and to that number add any additional local consideration (such as
326 > upstream congestion, number of NCE slots available, etc.).
...
333 > Note that the calculated local value _vl_ does _not_ update the value
334 > _vr_ in the option.
I can't tell whether this describes one mechanism or two. Line 301 says a 6LR
MAY increment the Min Priority carried in the option -- which I read as
something a 6LR does when it regenerates the option in the DIOs it forwards
to its own children, since Section 3.1 (lines 286-288) says a 6LR "uses the
contents of this option from whichever parent it selects as the basis for
the option that it sends." But lines 323-326 describe a 6LR adding its own
"additional local consideration" to the adopted minimum enrollment priority,
and line 333 then says explicitly that this calculated local value does
_not_ update the value in the option that gets propagated.
If lines 301 and 323-326 are describing the same increment, the document
contradicts itself about whether it propagates. If they're describing two
different things -- a deliberate, version-bumped increment that propagates,
versus an ephemeral local overlay used only for this node's own Join Proxy
gate that never leaves the node -- then the text needs to say so, because as
written it uses near-identical language for both ("increment the Min
Priority" vs. "add any additional local consideration" to "the minimum
enrollment priority") with nothing to tell an implementer which rule
applies when.
Compounding this, the variable name _vl_ is reused for two different
quantities: at line 311-312 it means "the Version Number [the 6LR] has last
adopted locally," and at line 333 it means the locally-calculated minimum
enrollment priority from lines 323-326. Untangling which _vl_ is meant
requires the reader to already have resolved the ambiguity above.
I think this section could do with a rewrite to name the two mechanisms
separately (if two are intended) and to stop reusing _vl_ for two different
values.
440 > Nevertheless, a node that is attacked and has malware placed on it 441 > creates vulnerabilities in the same way such an attack on any node 442 > involved in Internet routing protocol does. The re-keying provisions 443 > of [RFC9031] exist to permit an operator to remove such nodes from 444 > the network. RFC 9031's re-keying (Section 8.4.3) rotates the network-wide key material; it doesn't have a primitive that specifically ejects one already-joined node. A compromised node effectively loses the ability to participate once it's excluded from a re-key, but "remove such nodes from the network" reads as though CoJP has a dedicated eviction mechanism, which as far as I can tell it doesn't. Worth tightening this sentence so it doesn't overstate what RFC 9031 provides. ---------------------------------------------------------------------- NIT ---------------------------------------------------------------------- All comments below are about very minor potential issues that you may choose to address in some way - or ignore - as you see fit. Some were flagged by automated tools (via https://github.com/larseggert/ietf-reviewtool), so there will likely be some false positives. There is no need to let me know what you did with these suggestions. Section 1.1, Motivation and Overview: 124 > There are a variety of local metrics which a 6LowPAN Router (6LR) 125 > [RFC6066] can use to determine if it should provide the _Join Proxy_ RFC 6066 is the TLS extensions RFC; the 6LR definition is RFC 6606, which is what Section 2 (line 205) cites correctly for the same term. Gunter Van de Velde flagged this in his ballot comment; I agree with him. s/[RFC6066]/[RFC6606]/ --- 159 > basis for the proxy priority described in [RFC9032]. 160 > Section Section 3.1 explains how these fields affect the Trickle 161 > Timer. s/Section Section 3.1/Section 3.1/ --- Section 3.1, Option Format: 292 > minimum enrollment priority to a lower value MAY be considered 293 > "inconsistent", or a node MAY wait re-transmit according to the s/MAY wait re-transmit/MAY wait to re-transmit/ --- Section 3.2, Option Processing: 330 > The resulting minimum enrollment priority, if less than 0x7f, should 331 > enable the _Join Proxy_ function. This is the only lowercase "should" amid an otherwise consistently RFC 2119-keyword section; worth confirming whether SHOULD was intended here or whether the lowercase is deliberate. --- Section 4, Operational Considerations: 338 > The RPL ecosystem has not included a management protocols to date. A 339 > future mechanisms, such as [I-D.ietf-roll-capabilities] could enable s/a management protocols/a management protocol/ s/A future mechanisms/A future mechanism/ --- 369 > the absence of of this metric, a 6LR will need to base decisions on s/absence of of this metric/absence of this metric/ --- Section 4.1, Incremental deployment Considerations: 372 > This document therefore establishes that a 6LRs that support this 373 > option but do not receive it via any path SHOULD assume a default s/a 6LRs that support this option/6LRs that support this option/ --- Section 1.1, Motivation and Overview: 142 > nodes are not yet known to be trustworthy, the introduction of a lot 143 > of traffic from could be part of a denial of service attack. "traffic from could be" reads like a dropped word. s/traffic from could be/traffic could be/ --- Section 7, Acknowledgements: 457 > This has been reviewed by Charlie Perkins, Rifaat Shehk-Yusek, Dave 458 > Thaler, and Thomas Watteyne. The SECDIR reviewer's name is Rifaat Shekh-Yusef. s/Rifaat Shehk-Yusek/Rifaat Shekh-Yusef/ --- 460 > Huimin She contributed text about expressing the DODAG size. Ketan 461 > Talaulika was the responsible AD and provided many editorial s/Ketan Talaulika/Ketan Talaulikar/ --- Capitalization of "DODAG Root" / "DODAG root" is inconsistent throughout Section 3 (e.g., line 250 "the DODAG root" vs. line 299 "the DODAG Root"); worth picking one and using it consistently.
# IESG review of draft-ietf-roll-enrollment-priority-18 CC @MikeBishop ## Discuss ### Section 2, paragraph 2 ``` The term 6LR means 6LowPAN Router, and is defined in [RFC6606]. It refers to a router that forwards packets in a 6LowPAN network. ``` Given how pervasively 6LR is used throughout this document, is RFC6606 in fact only an Informative reference? It's unclear to me that one could fully implement this specification without consulting it.
## Comments
### Section 1.1, paragraph 2
```
There are a variety of local metrics which a 6LowPAN Router (6LR)
[RFC6066] can use to determine if it should provide the _Join Proxy_
```
It is unlikely that "TLS Extension Definitions" are the intended citation.
As others have noted, I'd guess you meant to reference RFC6606 again, and
RFC6066 can be removed from this document's references entirely.
### Section 1.1, paragraph 1
These reasons seem like reasons a node would not announce itself.
Perhaps adjust the first sentence to that orientation?
### Section 2, paragraph 6
This disambiguation between competing uses of the same English word is
important, and thank you for including it. However, I find the number-prefix
notation a bit confusing. Perhaps something like "The term \"Join\" has been
used in two different senses in related documents. First, [RFC9031]..."
### Section 3.1, paragraph 4
The Figure shows a Length field which does not appear in the text.
### Section 3.1, paragraph 11
Does this warrant a SHOULD/MUST?
### Section 3.1, paragraph 11
How does the root handle a DODAG size greater than the one representable in this
many bits (491k)? We may think it's unlikely today, but we should still say what
happens.
### Section 3.2, paragraph 4
This section is inconsistent about whether vl/vr are the contents of the option
or the Version Number from that option. Consider clearly stating one definition
up front, before using them in the process.
### Section 3.2, paragraph 9
It's not clear to me how to reconcile vl-does-not-update-vr with the
6LR's option (in the first paragraph) to update Version Number and Min Priority.
Consider separately stating how a 6LR decides what to send downstream.
### Section 4.1, paragraph 2
Is there a rationale for why this value provides "the best reaction"?
## Nits
All comments below are about very minor potential issues that you may choose to
address in some way - or ignore - as you see fit. Some were flagged by
automated tools (via https://github.com/larseggert/ietf-reviewtool), so there
will likely be some false positives. There is no need to let me know what you
did with these suggestions.
### Typos
#### "Abstract", paragraph 1
```
- at any given time. Currently, Join Proxies (6LowPAN Routers) make
- ^
+ at any given time. Currently, Join Proxies (6LoWPAN Routers) make
+ ^
```
#### Section 1, paragraph 1
```
- to utilize a nearby 6LowPAN Router as a Join Proxy.
- ^
+ to utilize a nearby 6LoWPAN Router as a Join Proxy.
+ ^
```
#### Section 1.1, paragraph 2
```
- There are a variety of local metrics which a 6LowPAN Router (6LR)
- ^
+ There are a variety of local metrics which a 6LoWPAN Router (6LR)
+ ^
```
#### Section 1.1, paragraph 4
```
- nodes are not yet known to be trustworthy, the introduction of a lot
- ^
- of traffic from could be part of a denial of service attack.
+ nodes are not yet known to be trustworthy; the introduction of a lot
+ ^
+ of traffic from them could be part of a denial of service attack.
+ +++++
```
#### Section 1.1, paragraph 7
```
- Section Section 3.1 explains how these fields affect the Trickle
- --------
```
#### Section 2, paragraph 2
```
- The term 6LR means 6LowPAN Router, and is defined in [RFC6606]. It
- ^
- refers to a router that forwards packets in a 6LowPAN network.
- ^
+ The term 6LR means 6LoWPAN Router, and is defined in [RFC6606]. It
+ ^
+ refers to a router that forwards packets in a 6LoWPAN network.
+ ^
```
#### Section 4, paragraph 1
```
- in the node is via a custom firmware load, or a vendor proprietary
- ^
+ in the node is via a custom firmware load, or a vendor-proprietary
+ ^
```
#### Section 7, paragraph 1
```
- This has been reviewed by Charlie Perkins, Rifaat Shehk-Yusek, Dave
- ^
+ This has been reviewed by Charlie Perkins, Rifaat Shehk-Yusef, Dave
+ ^
```
#### Section 7, paragraph 2
```
- Talaulika was the responsible AD and provided many editorial
+ Talaulikar was the responsible AD and provided many editorial
+ +
```
### Section 1.1, paragraph 7
"occupancy of the bandwidth" => "bandwidth utilization"?
### Section 3.1, paragraph 13
Perhaps: ..."inconsistent"; if not, the node retransmits...
### Outdated references
Document references `draft-ietf-roll-dao-projection`, but that has been
published as `RFC9914{quote}.
### Grammar/style
#### Section 1.1, paragraph 3
```
from them could be part of a denial of service attack. This extension inclu
^^^^^^^^^^^^^^^^^
```
It appears that hyphens are missing.
#### Section 3.2, paragraph 6
```
ecosystem has not included a management protocols to date. A future mechanis
^^^^^^^^^^^^^^^^^^^^^^
```
The plural noun "protocols" cannot be used with the article "a".
#### Section 3.2, paragraph 6
```
nagement protocols to date. A future mechanisms, such as [I-D.ietf-roll-capa
^^^^^^^^^^^^^^^^^^^
```
The plural noun "mechanisms" cannot be used with the article "A". Did you mean
"A future mechanism" or "future mechanisms"?
#### Section 4, paragraph 1
```
IO messages. In effect, the 6LR's sub-tree below a node without support for t
^^^^^^^^
```
This word is normally spelled as one.
#### Section 4, paragraph 1
```
nrollment priority. In the absence of of this metric, a 6LR will need to bas
^^^^^
```
Possible typo: you repeated a word.
Hi Michael, Rahul, Pascal, and Konrad,
Thank you of the effort put into this document. Please find below some comments points for discussions.
# Assess the effectiveness and determinism of the approach
It is not clear to me to what extent disseminating this option will solve the congestion cases that motivated this work (for values !=127) and how exposing the proxy functions is surgically controlled vs. letting the decision to be local to a 6LR. It would be helpful to share, for example, simulation work (if any). At a minimum, having some examples would be helpful.
For example, the document says:
This document introduces Routing Protocol for Low-Power and Lossy
Networks (RPL) extensions to ensure that enrollment remains orderly,
prevents localized congestion at specific Join Proxies,
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Likewise, and from an operational standpoint and taking into account this is supposed to be a control mechanism, it is not clear from the current spec whether the outcome of triggering option dissemination with some specific values is determinist. This is even exacerbated as the priority value is inferred from some other values that may not reflect the exact network conditions.
# Inferring size
CURRENT:
The DODAG Size can be measured by the Root based on the DAO activity.
In such a case, it represents the number of routes not the number of
nodes, and can thus be used to infer the load only in a network where
each node advertises roughly the same number of addresses and
generates roughly the same amount of traffic.
How the last part is known to the root?
# Important changes
CURRENT:
Whenever the DODAG root changes the values of the minimum enrollment
priority or DODAG Size in the option, it MUST also increment the
value of Version Number. Moreover, if the change is considered
important (i.e., it is expected to propagate in the DODAG quickly),
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Maybe I’m misreading the base spec, but I prefer to check.
Setting that bit does not mean immediate sending as RFC6206 has this condition in step 4:
4. At time t, Trickle transmits if and only if the counter c is less than the redundancy constant k.
# First instance
CURRENT:
Upon receiving the option, a 6LR first checks the value of the
Version Number field in the option, _vr_, versus the value of the
Version Number it has last adopted locally, _vl_.
The behavior reasons as if an instance was already received. The text is silent for the processing of the first instance (and that how vl should be set for such cases).
# Unit and practicalities
CURRENT:
A 6LR, which would otherwise be willing to act as a _Join Proxy_,
will examine the locally adopted value of minimum enrollment priority
and to that number add any additional local consideration (such as
upstream congestion, number of NCE slots available, etc.).
As there is no unit or weight to evaluate the impact of each of these local conditions, how this update is supposed to be done? This is important as having distinct behaviors may lead to random behavior by local nodes and thus lead to suboptimal behavior vs acting to graft new nodes.
# 404 CURRENT: Source for this draft and an issue tracker can be found at https://github.com/roll-wg/voucher. FYI, I wanted to use the repo to send some nits in a PR but I got a 404 error. # One Extension or Many Extensions Abstract: This document introduces RPL extensions to ensure that enrollment remains orderly, prevents localized congestion at specific Join Vs. Section 1: This extension includes a mechanism to allow the network operator to send a signal that no new nodes are expected at that time, Please make sure the document is consistent. # Updates CURRENT: This new metric, the minimum enrollment priority, is updated by each 6LR to reflect conditions in that 6LR. My reading of the spec is that the update is conditional, while this overview sentence suggests this will always be the casese # Stability impact and Oscillation phenomenon CURRENT: It can also use this information to move between DODAGs in order to help keep the relative sizes balanced. I guess this behavior may lead to instability if a node changes its attachment so frequently or if the network conditions are so. Maybe have some guards under the operational considerations section to flag this. # Join flavors CURRENT: The term (1)"Join" has been used in documents such as [RFC9031] to denote the activity of a new node authenticating itself to the network to obtain authorization to become a member of the network. … I’m afraid all this discussion need some rework as I failed to digest the subtleties covered here, paragraphs right after, and the actual use in the document. # T bit ## The description of what can be characterized as important is missing. ## Please fix this part as well OLD: timer [RFC6206], Section 4.2 reset at the node [RFC6550], Section 8.3. NEW: timer ([RFC6206], Section 4.2) reset at the node ([RFC6550], Section 8.3). # EB CURRENT: Min Priority The minimum enrollment priority. This is a 7-bit field providing a base value for the Enhanced Beacon Join priority. Please add a reference to where Enhanced Beacon Join priority is defined. # A bit can be set to 0 or 1 OLD: * Otherwise, the 6LR MUST adopt the contents of the option (i.e., the values of Version Number, Min Priority, DODAG Size, and the T bit) as its local ones. Moreover, if _vl_ was smaller than _vr_ (in the lollipop counter order) and the T bit in the received option was set, then the 6LR MUST reset its DIO trickle timer. ^^^^^^^^^^^^^^ NEW: * Otherwise, the 6LR MUST adopt the contents of the option (i.e., the values of Version Number, Min Priority, DODAG Size, and the T bit) as its local ones. Moreover, if _vl_ was smaller than _vr_ (in the lollipop counter order) and the T bit in the received option was set to 1, then the 6LR MUST reset its DIO trickle timer. # Nits ## OLD: These reasons include low available battery power, already high committed network bandwidth, and lack of available free memory for Neighbor Cache Entry (NCE) slots [RFC4861], Section 5.1. NEW: These reasons include low available battery power, already high committed network bandwidth, and lack of available free memory for Neighbor Cache Entry (NCE) slots ([RFC4861], Section 5.1). ## OLD: These unknown nodes are not yet known to be trustworthy, the introduction of a lot of traffic from could be part of a NEW: These nodes are not yet known to be trustworthy, the introduction of a lot of traffic from such nodes could be part of a ## OLD: Section Section 3.1 explains how these fields affect the Trickle Timer. NEW: Section 3.1 explains how these fields affect the Trickle Timer. ## OLD: This minimum enrollment priority is used by each 6LR node to determine whether or not it will operate as a _Join Proxy_ for nodes that want to enroll. NEW: This minimum enrollment priority is used by each 6LR node to determine whether it will operate as a _Join Proxy_ for nodes that want to enroll. ## Broken sentence. Please check CURRENT: Updates to the option propagate through the network according to the trickle algorithm. [RFC6206] Other than the minimum enrollment ## OLD: The terms DAO, DODAG, DODAG root, DIO, trickle timer are from [RFC6550]. NEW: The terms DAO, DODAG, DODAG root, DIO, and trickle timer are from [RFC6550]. ## OLD: the absence of of this metric, NEW: the absence of this metric, ## Please check this part CURRENT: that a 6LRs that support this … Cheers, Med
Thanks to Rifaat Shekh-Yusef for their secdir review. I support Deb's discuss about the security issues.
I support the DISCUSS position of Mahesh Jethanandani. ** Section 1.1. Editorial. There are a variety of local metrics which a 6LowPAN Router (6LR) [RFC6066] can use to determine if it should provide the _Join Proxy_ function. These reasons include low available battery power, already high committed network bandwidth, and lack of available free memory for Neighbor Cache Entry (NCE) slots [RFC4861], Section 5.1. The second sentence doesn’t seem to follow from the first. The second sentence seems like it is a list of when a 6LR should NOT provider the join proxy function. Perhaps, s/can use to determine if it should provide/can use to determine if it should not provide/ ** Section 4 The RPL ecosystem has not included a management protocols to date. A future mechanisms, such as [I-D.ietf-roll-capabilities] could enable assessment and configuration of node features. How can [I-D.ietf-roll-capabilities] be considered a future mechanism? It is an expired draft from 2021. The work appears abandoned. Does it need to be mentioned? ** Section 4. RPL nodes therefore need to come with sensible defaults that allow a node to join a DODAG. As an implementor, what would make a default configuration “sensible”? ** Section 5. In most deployments involving wireless technology, layer 2 is always encrypted using a layer-2 specific technology, and so privacy of this option is available. What are the security properties being ensured? Encryption is mentioned as a technique and then “privacy” is mentioned as a property? Is this intended to discuss confidentiality?
I strongly support Deb's DISCUSS. Minor note (no reply needed, just something I ran into while re-reading this): in Section 1.1, you say "[ignoring typo already over-reported] Section 3.1 explains how these fields affect the Trickle Timer" which is true, but Section 3.2 also has the following, which is important enough that I would recommend saying "Sections 3.1 and 3.2" since 3.2 isn't long so as to make that requirement more obvious. > Moreover, if vl was smaller than vr (in the lollipop counter order) > and the T bit in the received option was set, then the 6LR MUST > reset its DIO trickle timer