[bess] Re: Secdir last call review of draft-ietf-bess-evpn-mh-split-horizon-09

[Yaron Sheffer <[e-mail]>]

Looks good. Thank you Jorge!

 

From: Jorge Rabadan (Nokia) <[e-mail]>
Date: Monday, 8 July 2024 at 18:23
To: Yaron Sheffer <[e-mail]>, [e-mail] <[e-mail]>
Cc: [e-mail] <[e-mail]>, [e-mail] <[e-mail]>, [e-mail] <[e-mail]>
Subject: Re: Secdir last call review of draft-ietf-bess-evpn-mh-split-horizon-09

Hi Yaron,

 

Thanks very much for the review. Version 10 has just been published, addressing your comments.

 

Please find in-line some responses to your comments, with [jorge].

Thank you!

Jorge

 

From: Yaron Sheffer via Datatracker <[e-mail]>
Date: Sunday, June 30, 2024 at 9:17 AM
To: [e-mail] <[e-mail]>
Cc: [e-mail] <[e-mail]>, [e-mail] <[e-mail]>, [e-mail] <[e-mail]>
Subject: Secdir last call review of draft-ietf-bess-evpn-mh-split-horizon-09

CAUTION: This is an external email. Please be very careful when clicking links or opening attachments. See the URL nok.it/ext for additional information.



Reviewer: Yaron Sheffer
Review result: Has Nits

This document defines a way to explicitly share the multi-homing split horizon
procedures over BGP, for a large variety of NVO use cases. This reviewer is not
familiar with the EVPN ecosystem, and comments below may reflect my own
ignorance.

In general: the document is clear, and does not appear to create any novel
security risks.

2.1: the first two paragraphs are duplicates.

[jorge] good catch. Removed the first one, thanks.

2.2: "A received A-D per ES route where Single-Active and SHT bits are
different from zero MUST be treat-as-withdraw" - IIUC, this is very fragile
behavior, where an incorrect flag results in the entire path being removed. Why
does this behavior make more sense than simply ignoring the SHT bits?

[jorge] If the single-active bit is set, then the Ethernet Segment only supports one split horizon type and there is no need to signal anything in the SHT. Receiving something different from 00 may mean that the advertising node has a bug and it may intend to do something wrong. Since the split horizon function deals with loop/packet duplication, which may be harmful in most cases, we thought we should be strict about this, hence the treat-as-withdraw behavior. Hope it is ok.

2.2: For the Geneve use case: is the value "10" always valid, or is it only
valid if the ESI-Label is present? The text is unclear.

[jorge] good point. Changed to:

“A value of 10 indicates the intent to use ESI Label-based Split Horizon, and it is only valid if an Ethernet option TLV with non-zero Source-ID is present”

4: "The security considerations relevant to multihoming" - is it clear to all
readers which security considerations these are? Are you referring to the
entirety of Sec. 19 of RFC7432?

[jorge] I think it is better to change it to :

“All the security considerations described in RFC7432 are applicable to this document.”

4: "unauthorized changes to the SHT configuration by an attacker should not
cause traffic disruption" - when various kinds of misconfiguration are "treat
as withdraw", how does that NOT cause traffic disruption? I would assume that
when the route is withdrawn, eventually traffic is disrupted.

[jorge] the assumption is that the misconfigurations are options allowed in this document. Only the options that are invalid may cause the treat-as-withdraw behavior. I tried to clarify with this modified paragraph:

“Apart from this risk, this document describes procedures to ensure that all Provider Edge (PE) devices or Network Virtualization Edges (NVEs) connected to the same Ethernet Segment (ES) agree on a common SHT method, with a fallback to a default behavior in case of a mismatch in the SHT bits being advertised by any two PEs or NVEs in the Ethernet Segment. Consequently, unauthorized changes to the SHT configuration by an attacker on a single PE or NVE of the Ethernet Segment should not cause traffic disruption (as long as the SHT value is valid as per this document) but may result in alterations to forwarding behavior.”

 

 

7: the Contributors section is empty.

[jorge] removed. Thanks!