Subject: [Last-Call] Genart last call review of draft-ietf-bier-te-arch-10

From: Robert Sparks via Datatracker <[email-masked]>
Date: Sat, 07 Aug 2021 15:42:02 -0700

Reviewer: Robert Sparks
Review result: Ready with Nits

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-bier-te-arch-10
Reviewer: Robert Sparks
Review Date: 2021-08-07
IETF LC End Date: 2021-08-11
IESG Telechat date: Not scheduled for a telechat

Summary: Essentially ready for publication as a Proposed Standard RFC, but with
nits, and possibly a minor issue, to address before publication.

Possible minor issue:

The Security Considerations section of this document could discuss an
exponential amplification attack by way of misconfiguration using the DNC bit.
If you have the following configuration (best read in a fixed-width font):

BFRA: p1 -> forward_connected(DNC) BFRC
      p2 -> forward_connected(DNC) BFRD
      p3 -> local_decap

BFRB: p1 -> forward_connected(DNC) BFRC
      p2 -> forward_connected(DNC) BFRD
      p3 -> local_decap

BFRC: p1 -> forward_connected(DNC) BFRA
      p2 -> forward_connected(DNC) BFRB
      p3 -> local_decap

BFRD: p1 -> forward_connected(DNC) BFRA
      p2 -> forward_connected(DNC) BFRB
      p3 -> local_decap

A single packet with bits (p1,p2,p3) introduced to any of the 4 BFR will
produce an exponentially increasing amount of internal traffic, and traffic
sent to whatever the local_decaped packets is configured for.

         p3              p3
        BFRA p1 ---- p1 BFRC
              \     /
               \   /
                 X
               /   \
              /     \
        BFRB p2 ---- p2 BFRD
         p3              p3

This may be easy to achieve accidentally if following the suggestion for
creating a bidirectional ring described in section 5.1.6.

Larger Nits:

The first example uses terms defined later in the document (like local_decap).
It would help the reader to say that right before the example.

Section 2.3 list 2 item 2. What is a "reference option"? This first sentence of
this item should be simplified, possibly by breaking it into two or more.

The first sentence of section 2.4 does not parse. Perhaps "BIER-TE forwarding
is designed to make it easy to build or program common forwarding hardware."
but I'm lost at "with BIER". Please clarify.

In section 3.2, there is a nested list that is introduced as "functionality"
but later referred to as "steps". Please provide a clearer description, and
make it obvious that these are the steps that the subsections (such as 3.2.1)
refer to.Is the outer list the steps 3.2.1 refers to?

Section 3.2 list item 2, sublist item 3. "Install state on the BFIR to
imposition the desired BIER packet header(s) for packets of the overlay flow"
does not parse. Please clarify.

Please clarify the last sentence (starting "See also") of 3.2.1.2. I don't know
what you are really trying to say but "solution describes interaction" doesn't
make sense.

At 3.2.1.4, what does "out-of-scope FRR procedures" mean? I suggest just
dropping "out-of-scope". If that's not right, more clarification is needed.

Section 3.4 second paragraph last sentence: Did you mean "BIER specific
routing" rather than "BFER specific routing"?

Section 4.1, Check that you really meant BFR-id = SI * 2^BSL + BP in paragraph
three.

Section 5.1, second paragraph: The list of sections '(4.1, ... 4.8)' didn't
track a document change. I think you mean to point to '(5.1.1, ...'. But the
list is unnecessary - I suggest deleting it.

In section 5.1.6, I think you are using L3 to mean different things in the
first paragraph and in the example.

Section 5.1.9 after Figure 17, you point to figure 20, but I think you really
meant figure 17. (It's interesting that figure 17 and 20 are essentially the
same, perhaps the document could be simplified).

Section 5.3.5: Where did this 20%..80% range come from? The last sentence is
unclear.

Section 5.3.6.2: paragraph 3. The sentence starting "This is much worse" is
complex. Please break it into several.

Section 5.3.7 First paragraph. This long sentence fails to parse. Please
simplify it.

Section 7 paragraph 6 (beginning "For additional,"): This sentence is very hard
to parse. Please simplify it.

Smaller Nits:

Overview: Expand BIFT on first use.

The Overview claims that the reader is expected to be familiar with both
RFC8279 and RFC 8296, but only lists the first as a normative reference.
Consider making RFC 8296 normative, or adjusting the introduction text.

First bullet in overview: "reference forwarding examples" -> "forwarding
examples"

Last bullet in overview: "sections of document" -> "sections of the document"

In 2.1, "To send in addition to BFR6 via BFR4 also a copy to BFR3" is hard to
parse. Perhaps: "To send a copy to BFR3 in addition to BFR6 via BFR4".

Section 3.2.1: Why is "Notwithstanding other option," necessary. I suggest
deleting it.

Section 3.2.1: "without any active dynamic components" is odd. Perhaps replace
the sentence with "Automated configuration of the BIER control plane is not
required. An operator can manually configure the BIER control plane via CLI on
the BFRs."

Section 3.3: "constitutes of the following components" is obtuse. Perhaps you
could replace it with "consists of"?

Section 3:3: I suggest replacing "This is done to inhibit that packets can
loop" with "This inhibits loop detection."

Section 4.6: I suggest replacing "support to configure" with "support
configuring" and "support to have" with "support having".

Section 4.6 (and other places) Saying "clear DNC", that is clear Do-Not-Clear
is dissonant. It might help avoid confusion to say "unset DNC", or replace
"with clear DNC flag" with "with the DNC flag net set".

Section 5.1.7: I suggest replacing "allows to use" with "allows using"

Section 5.3.4: Why is complex quoted in 'how "complex" the Tree Engineering is'?

Section 5.3.4: "Communications between BFIR flow" -> "Communications between
the BFIR flow"

Section 5.3.6.1, paragraph 3 first sentence: You say "over time" three times in
this sentence. Please simply it.

Section 5.3.6.1, paragraph 3, last sentence: I suggest changing "consider to
use" to "consider using"

---

From: Toerless Eckert <[email-masked]>
Date: Fri, 13 Aug 2021 19:17:37 +0200

Thanks, Robert

Let me quickly reply on your looping example, and Cc bier list,
because the problem as you describe does not exist thanks to how
BIER-TE handles DNC, but maybe you and others want to chime in whether
or not your example of an attempted attack by misconfiguration and
what it results in would be good to add to the text anyhow, and
if you think so, then where (security considerations or any
other place you think it fits). Will be happy to add it for the
next rev, if more people than you think it would be helpfull.

inline...

> creating a bidirectional ring described in section 5.1.6.

 c0(p1,p2,p3) -> BFRA -> c1(p1) -> BFRC <- ttl expiry -> BFRA
                      -> c2(p2) -> BFRD <- ttl expiry -> BFRB
                      -> receive

Aka: The DNC flag on an adjacency will only keep the bit set on
the copy made for this bit, not on the other copies. The
forwarding pseudocode shows this by clearing all bits with
adjacencies first and then it sets back the bit for only
the adjacency when it has DNC flag.

In result, there will be 2 TTL expiring microloops,
one for  p1 between BFRC,BFRA and one for p2 between BFRD,BFRB,
and of course, these can overload links.  But there will be no
duplicate packets received.

This is something you could always do as well for ASM/SSM IP multicast
and BIER by a misconfiguration attack against RPF (aka: multicast static
routing misconfig attack) or with a single loop (as there is no replication)
for IP Unicast by attacking with a static route misconfig. 

Of course, for BIER, IP multicast or IP unicast one could argue that
you may not provide these routing configuration options, but all routers
i know have them. And hopefully at some point we even have YANG
nodels representing such configuration options (static (m)routes).

Cheers
    Toerless

-- 
---
[email-masked]

---

From: Robert Sparks <[email-masked]>
Date: Fri, 13 Aug 2021 12:30:56 -0500

On 8/13/21 12:17 PM, Toerless Eckert wrote:
> the copy made for this bit, not on the other copies.

Got it.

I don't feel strongly about adding additional text in this case. If you 
do, I would prefer framing it as accidental misconfiguration rather than 
malicious.

> The

---

From: Alvaro Retana <[email-masked]>
Date: Thu, 19 Aug 2021 17:40:46 +0000

On August 13, 2021 at 1:31:29 PM, Robert Sparks wrote:

...
> do, I would prefer framing it as accidental misconfiguration rather than

FWIW, I agree with Robert.  Also, there's text in the Security
Considerations related to the risk of misconfigurations.

Alvaro.

---

From: Lars Eggert <[email-masked]>
Date: Tue, 24 Aug 2021 12:13:30 +0300

Robert, thank you for your review; I am expecting the authors to reply. I have entered a No Objection ballot for this document.

Lars

> On 2021-8-8, at 1:42, Robert Sparks via Datatracker <[email-masked]> wrote:
> 
> Reviewer: Robert Sparks
> last-call mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/last-call