Subject: Re: [GROW] AD Review of draft-ietf-idr-bgp-open-policy-15

From: Alexander Azimov <[email-masked]>
Date: Tue, 10 Aug 2021 19:22:52 +0300

Dear Alvaro,

Thank you once again for your very careful review and detailed comments.

We tried our best to carefully consider all your comments, incorporated
changes accordingly in version -16, and uploaded it.

https://datatracker.ietf.org/doc/html/draft-ietf-idr-bgp-open-policy-16

Our (authors’) responses to your specific comments are inline below and
marked with [AA/KS].

Before jumping to the long list of inline comments I'd like to highlight
that while policy specification is better worded now, their principles
remain unchanged.

So, the existing implementations remain practically unaffected except for
the minor consideration of the presence of multiple OTCs per UPDATE.

The inline comments took 21 pages in Word. The last line should read: [End
of Review -15]

If that does not show, then it means the text got cut off somewhere. Let us
know.

Alexander / Sriram

------------------------------------------

[Idr] AD Review of draft-ietf-idr-bgp-open-policy-15

Alvaro Retana <[email-masked]> Wed, 09 June 2021 19:40 UTC

Dear authors:

Thank you for your work on addressing the route leaks problem!

I think the document still needs a lot of work.  In general, the

precision in the language is not what I would expect from a Standards

Track document.  Also, I believe the specification is not complete.

Please see details inline.

Instead of returning the document to the WG, and because it is short,

I am including a long list of suggestions below.  Given all the

proposed changes, we will eventually need to run a new WGLC explicitly

including the grow WG -- I have cc'd them in this message as well.

Thanks!

Alvaro.

[Line numbers from idnits.]

...

14                   Route Leak Prevention using Roles in Update and Open
messages

15                                 draft-ietf-idr-bgp-open-policy-15

[minor] s/Update and Open/UPDATE and OPEN BGP

[AA/KS] Fixed.

[major] Throughout the document the concept of "sending prefixes" (or

sometimes announcing them) is used -- I understand that these are

common phrases.  However, that is not the terminology used in rfc4271.

Please use "route advertisement" (and variations) instead.  Note that

"propagated" is also ok.

[AA/KS] Fixed.

17               Abstract

19                 Route leaks are the propagation of BGP prefixes which
violate

20                 assumptions of BGP topology relationships; e.g. passing
a route

21                 learned from one lateral peer to another lateral peer or
a transit

22                 provider, passing a route learned from one transit
provider to

23                 another transit provider or a lateral peer.  Existing
approaches to

24                 leak prevention rely on marking routes by operator
configuration,

25                 with no check that the configuration corresponds to that
of the eBGP

26                 neighbor, or enforcement that the two eBGP speakers
agree on the

27                 relationship.  This document enhances BGP OPEN to
establish agreement

28                 of the (peer, customer, provider, Route Server, Route
Server client)

29                 relationship of two neighboring eBGP speakers to enforce
appropriate

30                 configuration on both sides.  Propagated routes are then
marked with

31                 an Only to Customer (OTC) attribute according to the
agreed

32                 relationship, allowing both prevention and detection of
route leaks.

[minor]

OLD>

   This document enhances BGP OPEN to establish agreement of the (peer,

   customer, provider, Route Server, Route Server client)  relationship of

   two neighboring eBGP speakers to enforce appropriate configuration on
both

   sides. Propagated routes are then marked with an Only to Customer (OTC)

   attribute according to the agreed relationship, allowing both prevention

   and detection of route leaks.

Suggestion>

   This document enhances the BGP OPEN message to establish an agreement of

   the relationship between two eBGP speakers in order to enforce
appropriate

   configuration on both sides.  Propagated routes are then marked according

   to the agreed relationship, allowing both prevention and detection of
route

   leaks.

[AA/KS] Done.

...

94               1.  Introduction

96                 A BGP route leak occurs when a route is learned from a
transit

97                 provider or lateral peer and then announced to another
provider or

98                 lateral peer.  See [RFC7908].  These are usually the
result of

99                 misconfigured or absent BGP route filtering or lack of
coordination

100              between two eBGP speakers.

[nit] s/peer.  See [RFC7908]./peer [RFC7908].

[AA/KS] Fixed.

[nit] s/between two eBGP speakers./between autonomous systems.

[AA/KS] Fixed.

102              The mechanism proposed in

103              [I-D.ietf-grow-route-leak-detection-mitigation] uses large-

104              communities to perform detection and mitigation of route
leaks.

105              While signaling using communities is easy to implement and
deploy

106              quickly, it normally relies on operator-maintained policy

107              configuration, which is vulnerable to misconfiguration
[Streibelt].

108              The community signal can also be stripped at the ISP
boundaries.

[major] This paragraph says that another proposal uses a different

mechanism to solve the same problem...but that it has issues.  I am

not a fan of comparing solutions, much less ones that are still in

progress (even if the authors overlap).  Please remove this paragraph.

Having said that, I-D.ietf-grow-route-leak-detection-mitigation points

back at this document as a way to help automate the process described

there.  Are there any specific dependencies on that draft from this

one?  I don't think so, but if there are, that would be the reason to

reference it.

I am assuming that any interaction will be included in

I-D.ietf-grow-route-leak-detection-mitigation -- and that it won't

change this document.  That other ID is still under discussion.

[AA/KS] The paragraphs with comparison with the other draft has been
removed.

110              This document provides configuration automation using 'BGP
roles',

111              which are negotiated using a new BGP Capability Code in
OPEN message

112              (see Section 4 in [RFC5492]).  Either or both BGP speakers
MAY be

113              configured to require that this capability be agreed for
the BGP OPEN

114              to succeed.

[nit] s/in OPEN message/in the OPEN message

[AA/KS] Fixed.

[minor] s/see Section 4 in [RFC5492]/[RFC5492]

[AA/KS] Fixed.

[major] "MAY be configured to require"   This phrase sounds like a

contradiction: it is optional to require something.  Let's leave the

normative language to the normative parts of this document.  s/MAY/may

[AA/KS] Fixed.

116              A new optional, transitive BGP Path Attribute Only to
Customer (OTC)

117              is specified that SHOULD be automatically configured using
BGP roles.

118              This attribute prevents networks from creating leaks, and
detects

119              leaks created by third parties.

[major] Again, let's keep normative language for later.

Suggestion>

   A new optional, transitive BGP Path Attribute, called Only to Customer

   (OTC), is specified in Section 6.  It prevents networks from creating

   leaks, and detects leaks created by third parties.

[AA/KS] Fixed. New text as follows:

   This document provides configuration automation using BGP Roles,

   which are negotiated using a BGP Role Capability in the OPEN message

   [RFC5492].

...

124            2.  Peering Relationships

126              Despite the use of terms such as "customer", "peer", etc.
in this

127              document, these are not necessarily business relationships
based on

128              payment agreements.  These terms are used to represent
restrictions

129              on BGP route propagation, sometimes known as the
Gao-Rexford model

130              [Gao].  The following is a list of various roles in eBGP
peering and

131              the corresponding rules for route propagation:

[major] Given the normative language used below, it should be made

clear that the names of the roles are defined for and apply only to

the discussion in this document.  I don't want there to be confusion

related to general terms like "peer".

It would be ideal if the roles were identified by their

capitalization, to differentiate between "Peer" (the role) and "peer"

(as in the BGP speaker at the other side of the session).  Please

review the document and make the necessary changes.

[AA/KS]  s/peer/Peer/g + all other roles are also capitalized; see revised
Sections 2 and 3. Added a Terminology Section 1.1 where these roles (Peer,
Provider, etc.), local/remote AS, and “route is ineligible” are clearly
defined.

Suggestion>

   The terms defined and used in this document (see below) don't necessarily

   represent business relationships based on payment agreements.  These
terms

   are used to represent restrictions on BGP route propagation, sometimes

   known as the Gao-Rexford model [Gao].  The following is a list of the
roles

   for eBGP peering and their corresponding rules for route propagation:

[AA/KS] Done.

133              Provider:  MAY send to a customer all available prefixes.

[major] The option is not to send all or nothing, but the fact that a

Provider can send anything (from a default, to a subset, to all) to a

Customer, right?

Suggestion>

   Provider:  MAY propagate any available route to a Customer.

[AA/KS]  Applied, also s/propagate/advertise/. See revised Sec. 2.

135              Customer:  MAY send to a provider prefixes which the sender

136                 originates and prefixes learned from any of their
customers.  A

137                 customer MUST NOT send to a provider prefixes learned
from its

138                 peers, from other providers, or from Route Servers.

[major] "MAY send to a provider...prefixes learned from any of their
customers."

This got me thinking of the fact that the role (Customer) is for a

specific eBGP session, but an AS may have multiple eBGP sessions with

multiple roles, which is where the part about "learned from any of

their customers" comes from.  Please add some text at the start of

this section (maybe in a separate paragraph after the first couple of

sentences above) to explain.

[AA/KS] Done. Please see revised Section 2. Also, we have added “…on each
eBGP session between autonomous systems (ASes)” in relevant places -- in
the Intro section, Sections 3 and 5.

[major] "MUST NOT send...prefixes learned from..."  Again, given that

the roles are session-based, how do you expect this requirement to be

satisfied?  IOW, how would an eBGP session in routerA know that a

specific prefix was learned from a Peer (or not) at routerB on the

other side of the network?

[AA/KS] There are some details that need to be understood. If a route was
advertised by a Peer, Provider, or RS and it is not marked with OTC, then
the ingress router adds an OTC Attribute with the AS number of the sender
(remote AS). See revised Section 4. The mere presence of an OTC indicates
to the egress router that the route was learned from a Peer, Provider, or
RS and it can be advertised only to customers. The absence of an OTC
Attribute indicates to the egress router that the route was either
advertised by a Customer or originated by the local AS. If a route received
from a Customer at an ingress router has an OTC Attribute, then the route
is rejected by ingress policy. So, a customer route or a locally originated
route never has OTC attached when received at the egress router. Now, this
is explained well in the revised Section 4 in the updated draft.

[AA/KS] We have added new wording in Section 4 to explain the above for the
reader’s benefit. This new wording appears immediately after the egress
policy description.

I believe that current practice of using communities to identify where

the routes came from is a straight-forward solution to this issue.

Please add something to that effect, as an example of how the

requirement can be met -- but also declare the specific mechanism out

of scope.

[AA/KS] That internal (i.e., intra-AS) mechanism is subsumed in the OTC
mechanism. Please see the explanation provided above.

This issue is not specific to Customers, so add the extra text after

the roles are defined.

[AA/KS] We think here you are taking about the mention of “multiple eBGP
sessions with

multiple roles”. As stated above, we took care of this generally by adding
“…on each eBGP session between autonomous systems (ASes)” a few times in
the Intro section and in Sections 3 and 5.

[minor] The only role not listed is RS-Client.  I'm assuming that

prefixes learned from a RS-Client also MUST NOT be advertised, right?

[AA/KS] Yes, RS-Client is not listed (in what a Customer MAY propagate).
The assumption is that an RS does not have a Provider (transit provider) by
definition. An RS treats all ASes connected to it as RS-Clients. So, a
local AS with a Customer Role is not an RS and thus does not have
RS-Clients.

Suggestion>

   Customer: MAY advertise any route learned from a Customer, or locally

   originated, to a Provider.  All other routes MUST NOT be propagated.

[AA/KS] Fixed.

140              Route Server (RS):  MAY send to an Route Server client
(RS-client)

141                 all available prefixes.

[] Suggestion>

   Route Server (RS):  MAY propagate any available route to a Route Server

   Client (RS-Client).

[AA/KS] Fixed.

143              RS-client:  MAY send to an RS prefixes which the sender
originates

144                 and prefixes learned from its customers.  An RS-client
MUST NOT

145                 send to an RS prefixes learned from its peers or
providers, or

146                 from another RS.

[] Suggestion>

   RS-Client: MAY advertise any route learned from a Customer or locally

   originated to a Route Server.  All other routes MUST NOT be propagated.

[AA/KS] Fixed

148              Peer:  MAY send to a peer prefixes which the sender
originates and

149                 prefixes learned from its customers.  A peer MUST NOT
send to a

150                 peer prefixes learned from other peers, from its
providers, or

151                 from RS(s).

[] Suggestion>

   Peer: MAY advertise any route learned from a Customer or locally

   originated to a Peer.  All other routes MUST NOT be propagated.

[AA/KS] Fixed

153              Of course, any BGP speaker may apply policy to reduce what
is

154              announced, and a recipient may apply policy to reduce the
set of

155              routes they accept.  Violation of the above rules may
result in route

156              leaks and MUST NOT be allowed.  Automatic enforcement of
these rules

157              should significantly reduce route leaks that may otherwise
occur due

158              to manual configuration mistakes.  While enforcing the
above rules

159              will address most BGP peering scenarios, their
configuration is not

160              part of BGP itself; therefore, configuration of ingress
and egress

161              prefix filters is still strongly advised.

[nit] s/Of course, any BGP/A BGP

[AA/KS] Fixed.

[major] "MUST NOT be allowed"  I don't know how this requirement is

enforced: the sender should advertise what is specified.  Adding a

normative requirement to the options and requirements listed doesn't

contribute...

Suggestion: remove the whole sentence; there is some redundancy with

the next one.

[AA/KS] s/Violation of the above rules may result in route leaks and MUST
NOT be allowed./Violation of the above rules may result in route leaks./

[?] "their configuration is not part of BGP itself"  I don't

understand what this phrase is intended to mean.

[AA/KS]  Yes, confusing. Removed the phrase. Replaced the paragraph with
this shorter and clearer one:

   A BGP speaker may apply policy to reduce what is announced, and a

   recipient may apply policy to reduce the set of routes they accept.

   Violation of the above rules may result in route leaks.  Automatic

   enforcement of these rules should significantly reduce route leaks

   that may otherwise occur due to manual configuration mistakes.

163            3.  BGP Role

[] This section should be placed *before* §2 (Peering Relationships),

so that the roles are described before the advertisement rules.  Some

of the text in the first paragraph of §2 (about business

relationships) should be moved here.

[AA/KS] We were following the sequence of introduction -> motivation &
definitions -> solution specification. And the idea behind §2 was to
describe motivation and then go to the solution with BGP Roles. The way the
wording has been revised in these sections should make this flow even
better. Please see revised Sections 2 and 3.

165              BGP Role is a new configuration option that is configured
on a per-

166              session basis.  BGP Roles reflect the agreement between
two BGP

167              speakers about their relationship.  One of the Roles
described below

168              SHOULD be configured on each eBGP session between ISPs
that carry

169              IPv4 and(or) IPv6 unicast prefixes.

[minor] "BGP Role is a new configuration option that is configured on

a per-session basis."

I don't think that this document should deal with specifics of what

could be configured in an implementation -- but with the function of

the roles.

Suggestion>

   The BGP Role characterizes the relationship between the eBGP speakers

   forming a session.

[AA/KS] The updated paragraph (at the top of Section 3) reads as follows:

   The BGP Role characterizes the relationship between the eBGP speakers

   forming a session.  BGP Role is configured on a per-session basis.

   An eBGP speaker SHOULD configure the BGP Role locally based on the

   local AS's knowledge of its Role.  The only exception is when the

   eBGP connection is complex (see Section 5).  BGP Roles are mutually

   confirmed using the BGP Role Capability (described in Section 3.1) on

   each eBGP session between autonomous systems (ASes).  One of the

   Roles described below SHOULD be configured at the local AS for each

   eBGP session with a neighbor (remote AS) (see definitions in

   Section 1.1).

[major] "One of the Roles described below SHOULD be configured on each

eBGP session between ISPs that carry IPv4 and(or) IPv6 unicast

prefixes."

I have several questions:

-  When is it ok to not use a role?  Why is the configuration

recommended and not required?

[AA/KS] The exception is the ‘complex’ relations case that is described in
the additional considerations section. See the updated paragraph at the top
of Section 3 (copied above).

- Only between ISPs?  What about Customers?  Are Route Servers

considered ISPs?  At minimum, you should describe the term some more.

This should also scope the applicability.

[AA/KS] The ISP term was used very loosely.  Correction: s/ISP/AS/. Please
see the updated paragraph (copied above).

- Only unicast routes?  Why?   This probably goes to the definition of

a route leak and where it happens.  rfc7908 focuses on what has been

seen on the Internet -- but I'm sure that as long as the same types of

relationships/topologies, the same type of issues can occur.  I'm not

asking that you explore that, but maybe a quick mention (in the

Introduction) that the main focus/applicability is the Internet, then

it won't be necessary to mention AFs.

[AA/KS] Per your suggestion, we have inserted this sentence in the Intro
section:  “The main focus/applicability is the Internet (IPv4 and IPv6
route announcements).” We make only one other mention of IPv4 and IPv6 at
the end of Section 4 as follows:

   The described ingress and egress policies are applicable only for

   unicast IPv4 and IPv6 address families and MUST not affect other

   address families by default.

...

184              Since BGP Role reflects the relationship between two BGP
speakers, it

185              could also be used for other purposes besides route leak
mitigation.

[] Like what?  I'm not disagreeing, but it seems like this sentence is

unnecessary.

[AA/KS] For example, one can imagine configuring local preference based on
BGP Roles configuration. But since it is out of the scope of this document,
we have removed the whole sentence.

187            4.  BGP Role Capability

189              The TLV (type, length, value) of the BGP Role capability
are:

191              o  Type - <TBD1>;

[major] Capabilities are not exactly defined as TLVs.

Suggestion>

   The BGP Role Capability is defined as follows:

   o  Code: 9

   ...

[AA/KS]  Fixed

...

197                                 +-------+---------------------+

198                                 | Value | Role name           |

199                                 +-------+---------------------+

200                                 |   0   | Sender is Provider  |

201                                 |   1   | Sender is RS        |

202                                 |   2   | Sender is RS-client |

203                                 |   3   | Sender is Customer  |

204                                 |   4   | Sender is Peer      |

205                                 +-------+---------------------+

[minor] The description of the Value says that the Role corresponds to

the speaker/sender.  There's no need to use "Sender is" for every

entry.  Also, the "Role name" is simply Provider, RS, etc.

[AA/KS] We now use the terms local AS /remote AS per your suggestion
instead of sender/receiver. Hence, in the table, s/Role name/Role name (for
the local AS)/

Also, we got rid of “Sender is”. Please see the new Table 1 in the updated
draft.

207                               Table 1: Predefined BGP Role Values

[major] It is not a given that a single capability has to be included

-- rfc5492 includes the case where a multiple of the same can be

present.

Suggestion>

   A BGP Speaker MUST NOT advertise multiple versions of the BGP Role

   Capability.  If a BGP Speaker receives multiple copies of the BGP Role

   Capability.... ???

[AA/KS]  We have added the suggested text at the end of Section 3.1 as
follows:

       If BGP Role is locally configured, the eBGP speaker MUST advertise

   BGP Role Capability in the BGP OPEN message.  An eBGP speaker MUST

   NOT advertise multiple versions of the BGP Role Capability.

[AA/KS] For the second part of your suggestion, we have also added the
following in Section 3.2:

   If an eBGP speaker receives multiple but identical BGP Role

   Capabilities with the same value in each, then the speaker MUST

   consider it to be a single BGP Role Capability and proceed [RFC5492].

   If multiple BGP Role Capabilities are received and not all of them

   have the same value, then the BGP speaker MUST reject the connection

   using the Role Mismatch Notification (code 2, subcode 8).

209            5.  Role correctness

[minor] This section talks about the validation of the capability.

Please make it a sub-section of §4.

[AA/KS]  We have restructured the sections as follows:

   3.  BGP Role  . . . . . . . . . . . . . . . . . . . . . . . . . .   4

     3.1.  BGP Role Capability . . . . . . . . . . . . . . . . . . .   5

     3.2.  Role Correctness  . . . . . . . . . . . . . . . . . . . .   5

211              Section 3 described how BGP Role encodes the relationship
between two

212              eBGP speakers.  But the mere presence of BGP Role doesn't

213              automatically guarantee role agreement between two BGP
peers.

[minor] The encoding is defined in §4.

[AA/KS]  Fixed. Please see the top 2 paragraphs of Sec. 3.2.

215              To enforce correctness, the BGP Role check is applied with
a set of

216              constraints on how speakers' BGP Roles MUST correspond.
Of course,

217              each speaker MUST announce and accept the BGP Role
capability in the

218              BGP OPEN message exchange.

[major] "MUST announce and accept"

This requirement contradicts the default setting of "strict mode": if

false, then the speaker cannot be expected to both send and receive

the capability.

[AA/KS] We got rid of the cited phrase. We also got rid of the
contradiction. At the bottom of Sec 3.1, it now says:

   If BGP Role is locally configured, the eBGP speaker MUST advertise

   BGP Role Capability in the BGP OPEN message.  An eBGP speaker MUST

   NOT advertise multiple versions of the BGP Role Capability.

I think that we could eliminate §5.1 by bringing the intent of the

text up to this section.  Suggestion>

   A BGP Speaker that supports the BGP Role Capability SHOULD include it in

   the OPEN Message.  If the BGP Role Capability is sent, but one is not

   received, then the connection SHOULD be rejected using the Role Mismatch

   Notification (code 2, subcode 8); this mode of operation is called
"strict

   mode". For backwards compatibility, if the BGP speaker does not receive

   the capability from its peer, it MAY proceed with session establishment

   without consideration for the roles defined here; this SHOULD be the

   default mode of operation.

[AA/KS] Great suggestion. We have followed it. Previous Section 5 is now
Section 3.2. Previous Section 5.1 has been subsumed in Section 3.2 per your
suggestion. Section 3.2 is now well-organized including dealing with error
condition with multiple versions of the BGP Role Capability.

Note that in this suggestion I used the "Role Mismatch Notification",

but not receiving the capability is not really a role mismatch -- its

a "missing capability" error.  This type of error is not currently

defined.  It is up to you whether you want to define it here or not.

[AA/KS] Using the Role Mismatch Notification here seems the prudent choice.
No need to define another error type.

220              If a speaker receives a BGP Role capability, it MUST check
the value

221              of the received capability (i.e., the sender's role) with
its own BGP

222              Role.  The allowed pairings are as follows:

[major] "MUST check"   The required action is not to check, but to

make sure that the advertised values match one of the pairs below.

Suggestion (using also text from the last paragraph in this section)>

   If the BGP Role Capability is sent and one is also received from the
peer,

   the roles must correspond to the relationships in Table 2.  If the roles

   do not correspond, the BGP Speaker MUST reject the connection using the
Role

   Mismatch Notification (code 2, subcode 8).

[AA/KS] Applied.  See 2nd para in Sec. 3.2.

224                               +---------------+-----------------+

225                               | Sender's Role | Receiver's Role |

226                               +---------------+-----------------+

227                               | Provider      | Customer        |

228                               | Customer      | Provider        |

229                               | RS            | RS-client       |

230                               | RS-client     | RS              |

231                               | Peer          | Peer            |

232                               +---------------+-----------------+

[major] I find the language of the local router checking the "received

capability (i.e., the sender's role)" confusing since I would have

expected the local router to be the sender.  To avoid confusion, it

might be clearer to call the roles "local" and "remote".  I realize

that in the end the point of view doesn't matter much as long as a

router picks one -- but this is a standards track specification and it

should be as precise as possible.

[AA/KS] Fixed. We now use ‘local AS’/ ‘remote AS’ terminology per your
advice. We describe it upfront in Section 1.1 (Terminology). In the table
(cited above), we did the following: s/Sender’s Role/Local AS Role/ and
s/Receiver's Role/Remote AS Role/. See Table 2 in Section 3.2.

...

236              If the role of the receiving speaker for the eBGP session
in

237              consideration is included in Table 1 and the observed Role
pair is

238              not in the above table, then the receiving speaker MUST
reject the

239              eBGP connection, send a Role Mismatch Notification (code
2, subcode

240              <TBD2>), and also send a Connection Rejected Notification
[RFC4486]

241              (Notification with error code 6, subcode 5).

[] I moved this text to the suggested paragraph before Table 2.  Also,

note that only one Notification can be sent at a time.

[AA/KS] Fixed. Suggestions followed. Please see the revised paragraph
before Table 2. xxx

...

255            6.  BGP Only to Customer (OTC) Attribute

257              Newly defined here, the Only to Customer (OTC) is an
optional, 4

258              bytes long, transitive BGP Path attribute with the Type
Code <TBD3>.

259              The purpose of this attribute is to guarantee that once a
route is

260              sent to customer, peer, or RS-client, it will subsequently
go only to

261              customers.  The value of OTC is an AS number determined by
policy as

262              described below.  The semantics and usage of the OTC
attribute are

263              made clear by the ingress and egress policies described
below.

[] Suggestion>

   The Only to Customer (OTC) Attribute is an optional transitive path

   attribute with Attribute Type Code 35 and a length of 4 octets.  The

   attribute value is an AS number (ASN) determined by the policy described

   below.

[AA/KS] We applied your wording, but kept the phrase related to the purpose
of the attribute.  Please see the first paragraph of Section 4.

[major] What should a receiver do if the length is not set to 4?

[AA/KS] Added the following wording in the renumbered Section 4:

   If an eBGP speaker receives an UPDATE with an OTC Attribute with a

   length different from 4 octets, then the UPDATE SHALL be considered

   malformed.  If malformed, the UPDATE message SHALL be handled using

   the approach of "treat-as-withdraw" [RFC7606].

[AA/KS] In the above, we are following the approach suggested in RFC 7606
for malformed BGP attributes.

265              The following ingress policy applies to the OTC attribute:

267              1.  If a route with OTC attribute is received from a
Customer or RS-

268                  client, then it is a route leak and MUST be rejected.

[nit] s/with OTC attribute/with the OTC attribute/g

[AA/KS]  Fixed as follows:

   1.  If a route with the OTC Attribute is received from a Customer or

       RS-Client, then it is a route leak and MUST be considered

       ineligible (see Section 1.1).

[major] "MUST be rejected"  What *exactly* does this mean in terms of

BGP?  There is no "rejection" action -- so I'm guessing that

"rejected" means that the route has to be ignored, not used, and not

propagated.  In line with rfc4271/§9.1.1, would "the route MUST be

considered ineligible" be appropriate?

[AA/KS] Changed to ‘considered ineligible’. We also define the term in the
new Section 1.1 (Terminology).

270              2.  If a route with OTC attribute is received from a Peer
and its

271                  value is not equal to the sending neighbor's
Autonomous System

272                  (AS) number, then it is a route leak and MUST be
rejected.

[major] Can multiple OTC attributes be sent?  What should the receiver

do in that case?  I'm also thinking about cases where attributes may

be added by multiple ASes.

[AA/KS] We have added this wording towards the end in Section 4 (updated
draft):

   Correct implementation of the procedures specified in this document

   is not expected to result in the presence of multiple OTC Attributes

   in an UPDATE.  However, if an eBGP speaker receives multiple OTC

   Attributes with a route, then the only difference in the processing

   is in Step 2 of the ingress policy.

[AA/KS] Also, Step 2 of the ingress policy now reads:

   2.  If a route with the OTC Attribute is received from a Peer and at

       least one of the OTC Attributes has a value that is not equal to

       the remote (i.e.,  Peer's) AS number, then it is a route leak and

       MUST be considered ineligible.

...

278              The egress policy MUST be:

[major] "MUST be"   Given that the clauses (below) also contain

normative language, this "MUST" is unnecessary.

s/.../The following egress policy applies to the OTC attribute:

[AA/KS]  Fixed as follows:

   The following egress policy applies to the processing of the OTC

   Attribute:

280              1.  A route with the OTC attribute set MUST NOT be sent to
Providers,

281                  Peers, or RS(s).

[major] This rule seems to contradict the last one on ingress, which

expects the OTC attribute from Peers.  ??

[AA/KS] There is no contradiction. We have worded it better as follows (we
have interchanged steps 1 and 2 in the revised draft):

   2.  If a route already contains the OTC Attribute, it MUST NOT be

       propagated to Providers, Peers, or RS(s).

[AA/KS] Explanation: An ingress router at the local AS expects to receive
routes with the OTC attribute set when the remote AS is a Provide, Peer, or
RS. When the remote is a Peer, the ASN in the OTC MUST match the ASN of the
remote AS for the route to be eligible (for route selection). If the remote
is a Provider or RS, then ASN value in the OTC need not match the ASN of
the remote AS for eligibility. When a route is received at ingress from a
Provider, RS, or Peer, and the OTC is absent, then an OTC is added at the
ingress with ASN value equal to that of the remote AS. So, whoever sets the
OTC (remote AS or local AS) when route is propagated from
Provider->Customer, Peer->Peer, or RS->RS-client, the OTC will have the
same value. The egress router does not forward a route with OTC already set
(at a prior AS or at ingress) to a Provider, Peer, or RC; such a route is
sent only to the customers. So, the ingress policy is responsible for
detecting leaks, egress policy for preventing leaks, and both of them
perform OTC marking. The detection may take place multiple hops away.

A shorter version of the explanation above is now included in Section 4
immediately after the egress policy.

[minor] s/A route with the OTC attribute set MUST NOT be/The OTC

Attribute MUST NOT be included in routes

[AA/KS:] No! That is not correct. When sending a locally-originated or
Customer route to a Peer, an OTC with value equal to the local ASN is
added. Note: A Customer or RS-Client are not adding the OTC attribute when
advertising a route to its Provider or RS, respectively.

283              2.  If route is sent to a Customer or Peer, or an
RS-client (when the

284                  sender is an RS) and the OTC attribute is not present,
then it

285                  MUST be added with value equal to AS number of the
sender.

[] Suggestion>

   If a route to be sent to a Customer, Peer, or an RS-client (when the

   sender is an RS), and the OTC attribute is not present, it MUST be added

   with a value equal to the AS number of the sender.

[AA/KS] Yes, done. Worded it as follows (note: we have interchanged steps 1
and 2 in the revised draft):

   1.  If a route is to be advertised to a Customer, Peer, or RS-Client

       (when the sender is an RS), and the OTC Attribute is not present,

       then an OTC Attribute MUST be added with a value equal to the AS

       number of the local AS.

[major] There is no corresponding ingress policy that checks the ASN

to match routes received from Providers or RS.  (#2 only talks about

Peers)

[AA/KS:] There is no need. The presence of the OTC attribute on a route
received from a Provider or RS does not require any action to be taken on
ingress concerning the OTC. The OTC attribute may have been set by the
neighbor (remote AS) or an earlier AS in the path. There is no need to
check the OTC value. The mere presence of an OTC Attribute is a signal for
the egress router that the route can be sent only to Customers (i.e., MUST
NOT be sent to a Provider or Peer). However, the absence of an OTC
Attribute on a route received from a Provider, Peer, or RS requires that
the OTC Attribute be added on ingress with an ASN value equal to that of
the sending neighbor (remote AS). This is to cover for a Provider, Peer, or
RS that is not upgraded to do OTC.

...

289            7.  Enforcement

291              Having the relationship unequivocally agreed between the
two peers in

292              BGP OPEN is critical; BGP implementations MUST enforce the

293              relationship/role establishment rules (see Section 5) in
order to

294              ameliorate operator policy configuration errors (if any).

[] The suggested text in §5 eliminates the need for this paragraph.

[AA/KS] Yes. In fact, the whole ‘Enforcement’ section is gone/partly
subsumed in other sections.

296              Similarly, the application of that relationship on prefix
propagation

297              using OTC MUST be enforced by the BGP implementations, and
not

298              exposed to user misconfiguration.

[] This is a very important point: there are no exceptions to the

policies in §6.  Please add some text to that effect in §6.

Suggestion>

   The operator MUST NOT have the ability to modify the policies defined in

   this section.

[AA/KS] Applied. This sentence is moved to the renumbered Section 4 at the
end .

300              As opposed to communities, BGP attributes may not be
generally

301              modified or stripped by the operator; BGP router
implementations

302              enforce such treatment.  This is the desired property for
the OTC

303              marking.  Hence, this document specifies OTC as an
attribute.

[] This text goes back to comparing solutions...not needed.

[AA/KS]  Yes. The text is removed.

305            8.  Additional Considerations

...

315              No Roles SHOULD be configured on a 'complex' BGP session
(assuming it

316              is not segregated) and in that case, OTC MUST be set by
configuration

317              on a per-prefix basis.  However, there are no built-in
measures to

318              check correctness of OTC use if BGP Role is not configured.

[major] "OTC MUST be set by configuration"  I think this phrase

contradicts the statement in §7 about "OTC...not exposed to user

misconfiguration": both behaviors (allowing configuration, and not)

cannot be required at the same time.  Perhaps all the MUSTs involved

can be changed to SHOULDs.  ??

[AA/KS] The sentence about "OTC...not exposed to user misconfiguration" was
not needed and has been removed. We have also reworded the above cited
paragraph. The revised paragraph reads as follows (“Additional
Considerations” is now Sec. 5):

   No Roles SHOULD be configured on a 'complex' eBGP session (assuming

   it is not segregated) and in that case, the OTC Attribute processing

   MUST be done relying on configuration on a per-prefix basis.  Also,

   in this case, the per-prefix peering configuration MUST follow the

   same definitions of peering relations as described in Section 2.

   However, in this case, there are no built-in measures to check

   correctness of the per-prefix peering configuration.

[major] "...there are no built-in measures to check correctness of OTC

use if BGP Role is not configured."

- If the role is not configured, but the receiving router supports

OTC, what should it do?  It sounds that §6 only applies then the BGP

Role Capability is successfully negotiated -- is that true?

[AA/KS] First, the sentence has been revised. Please see the revised
paragraph copied above starting with ‘No Roles …’.

[AA/KS] Regarding when §6 (now §4) applies, given the updated draft, there
is no issue or confusion anymore. The following revised wording (#1 and #2)
fixes it (this takes care of Mach’s comments also):

   1. Paragraph at the top of Section 3:

   The BGP Role characterizes the relationship between the eBGP speakers

   forming a session.  BGP Role is configured on a per-session basis.

   An eBGP speaker SHOULD configure the BGP Role locally based on the

   local AS's knowledge of its Role.  The only exception is when the

   eBGP connection is complex (see Section 5).  BGP Roles are mutually

   confirmed using the BGP Role Capability (described in Section 3.1) on

   each eBGP session between autonomous systems (ASes).  One of the

   Roles described below SHOULD be configured at the local AS for each

   eBGP session with a neighbor (remote AS) (see definitions in

   Section 1.1).

   2. Paragraph in Section 3.2:

   If the BGP Role Capability is sent, but one is not received, then the

   connection MAY be rejected using the Role Mismatch Notification (code

   2, subcode 8); this mode of operation is called the "strict mode".

   For backward compatibility, if the BGP speaker does not receive the

   capability from its peer, it SHOULD ignore the absence of BGP Role

   Capability and proceed with session establishment; this SHOULD be the

   default non-strict mode of operation.  In this case, the locally

   configured BGP Role is used for the procedures described in

   Section 4.

[AA/KS]  Summary: Whether in strict or default mode, BGP Role is
configured, except it may not be mutually confirmed in the default mode.

[AA/KS] The policies in §4 (was §6) apply even in the case of complex
relationships based on local configuration of the per-prefix eBGP peering
configuration (see 2nd paragraph in Section 5).

- OTOH, if the role is configured and "strict mode" is *not* used, it

sounds as if §6 only applies if "strict mode" is used?  Is that true,

or should the receiver, who configured the role, but didn't receive

the Capability, and now received the attribute use the policy in §6?

[AA/KS] This issue goes away in light of the significant wording changes
noted above.

Whichever the case, it should be clearly called out somewhere.  [This

point is closely related to Mach's rtg-dir review.]

[AA/KS] Yes. As stated above, this issue goes away in light of the wording
changes/clarifications (that were also agreed to with Mach).

320              The incorrect setting of BGP Roles and/or OTC attributes
may affect

321              prefix propagation.  Further, this document doesn't
specify any

322              special handling of incorrect/private ASNs in OTC
attribute; such

323              errors should not happen with proper configuration.

[minor] "special handling of incorrect/private ASNs in OTC"   §6 says

that the ASN has to match the neighbor's.  That should take care of

the number not being the right one.

[AA/KS] There is no assumption that OTC value always equals to neighbor’s
ASN. It could be set multiple hops away and propagated Provider-to-Customer
(downward) on each of the hops. The revised paragraph at the end in §4 (was
§6) reads:

   The incorrect setting of BGP Roles and/or OTC Attributes may affect

   prefix propagation.  Further, this document does not specify any

   special handling of incorrect AS numbers in the OTC Attribute.  Such

   errors should not happen with proper configuration.

What about confederations?  I understand the intent here is to prevent

leaks in the Internet, so a private ASN shouldn't show up there.  §5

hints at the BGP Role Capability being applicable only to eBGP, but it

doesn't normatively constrain it to "normal" eBGP sessions.  Should

it?  I think it should.  If you do too, what should a receiver do if

the Capability is received over iBGP or an eBGP session that is not

"normal" -- ignore sounds like a good thing.

[AA/KS] Yes, “ignore” -- meaning do not act on it and let the OTC Attribute
propagate -- seems correct. IMO, the OTC processing at the ingress/egress
of the confederation will happen correctly as per this draft (RFC). So,
nothing specific to confederations needs to be said. Please advise.

325              As the BGP Role reflects the peering relationship between
neighbors,

326              it might have other uses beyond the route leak solution
discussed so

327              far.  For example, BGP Role might affect route priority,
or be used

328              to distinguish borders of a network if a network consists
of multiple

329              ASs.  Though such uses may be worthwhile, they are not the
goal of

330              this document.  Note that such uses would require local
policy

331              control.

[major] Please don't speculate about other uses!  If there are other

uses, then they should be specified.

[AA/KS] Yes, agree. We have removed the paragraph.

333              The use of BGP Roles are specified for unicast IPv4 and
IPv6 address

334              families.  While BGP roles can be configured on other
address

335              families its applicability for these cases is out of scope
of this

336              document.

[major] Ok.  So this means that the receiver should look at the

Multiprotocol capability (if present) and make sure that the proper

AFI/SAFI are included.  This needs to be specified in §4.  What should

the receiver do if the Capability is received for a different, or

additional, AFI/SAFI?  Or does all this mean that the policy in §6

simply doesn't apply to other AFI/SAFIs?  What should a receiver do if

the OTC attribute is received on a different AFI/SAFI?

[AA/KS] BGP Role capability isn’t negotiated on the AFI/SAFI granularity.
Per your earlier suggestion, we added the following sentence upfront in the
Section 1: “The main focus/applicability is the Internet (IPv4 and IPv6
unicast route advertisements).” So, we have now deleted the above quoted
sentence. We also added the following in Section 4 (last paragraph):

   The described ingress and egress policies are applicable only for

   unicast IPv4 and IPv6 address families and MUST not affect other

   address families by default.

[AA/KS] Let us know if you feel that the above resolves the issue about
AFI/SAFI.

338              As BGP role configuration results in automatic creation of
inbound/

339              outbound filters, existence of roles should be treated as
existence

340              of Import and Export policy [RFC8212].

[minor] "BGP role configuration results in..."   The configuration

doesn't result in anything -- the successful negotiation of the

capability does.  Same comment for "existence of roles".

[AA/KS]  See below.

[major] "should be treated as existence of Import and Export policy

[RFC8212]"  Should this text use normative language?  SHOULD/MUST?  Is

this a recommendation or a requirement?  If recommended, when would it

be ok to not consider the negotiation to satisfy rfc8212?

[] Was this interaction with rfc8212 considered by the grow WG?

[AA/KS]  We have removed the paragraph. RFC 8212 only says that there must
exist Explicit Import/Export Policies. It doesn’t list any specific ones.
It allows the vendor to choose what they consider qualify for such
policies.

342            9.  IANA Considerations

344              This document defines a new Capability Codes option [to be
removed

345              upon publication:
https://www.iana.org/assignments/capability-codes/

346              capability-codes.xhtml ] [RFC5492], named "BGP Role" with
an assigned

347              value <TBD1>.  The length of this capability is 1.

[minor] s/Capability Codes option/BGP Capability

[AA/KS] Fixed. We have revised the IANA Considerations (Section 6) to read
substantially better.

[minor] s/, named "BGP Role" with an assigned value <TBD1>./named "BGP

Role".  IANA has assigned value 9.

[AA/KS] Fixed

[minor] The length is already specified in §4; no need to repeat that here.

[AA/KS] Fixed

349              The BGP Role capability includes a Value field, for which
IANA is

350              requested to create and maintain a new sub-registry called
"BGP Role

351              Value".  Assignments consist of Value and corresponding
Role name.

352              Initially this registry is to be populated with the data
contained in

353              Table 1 found in Section 4.  Future assignments may be
made by a

354              Standard Action procedure [RFC8126].  The allocation
policy for new

355              entries up to and including value 127 is "Expert Review"
[RFC8126].

356              The allocation policy for values 128 through 251 is "First
Come First

357              Served".  The values from 252 through 255 are for
"Experimental Use".

[major] Where should this sub-registry be located?  I'm assuming that

it would be placed in the Capability Codes registry -- you need to be

explicit about that.

[AA/KS] Fixed. Please see the new wording.

[major] The paragraph indicates two different allocation procedures:

Standard Action, and then others for different parts of the range.

Which is it?

[AA/KS] It is best to assign the values only through IETF Review. The IANA
section is updated accordingly.

[major] If there will be an "Expert Review" range, you will need to

provide specific guidance to the experts.  Keep in mind that Expert

Review doesn't require WG approval (or even discussion), unless the

guidance is specific about that.  Take a look at

draft-ietf-idr-bgp-ls-registry for an example of guidance that

includes the WG -- if that's what you want.

[AA/KS] Please see comment above. This is all fixed.

[minor] Please add a table here to show the values.  Pointing at Table 1

is not enough because it doesn't include the unassigned range.

[AA/KS] Done. New Table 3 is added.

[minor] Also, please include a table showing the ranges and the

allocation policy, if more than one.

[AA/KS] Done. Table 3 is newly added.

359              This document defines a new subcode, "Role Mismatch" with
an assigned

360              value <TBD2> in the OPEN Message Error subcodes registry
[to be

361              removed upon publication:
http://www.iana.org/assignments/bgp- <https://www.iana.org/assignments/bgp->

362              parameters/bgp-parameters.xhtml#bgp-parameters-6]
[RFC4271].

[major] s/a new subcode, "Role Mismatch" with an assigned value <TBD2>

in the OPEN Message Error subcodes registry [...] [RFC4271]./the "Role

Mismatch" Message Error subcode.  IANA has assigned value 8 [...].

[AA/KS] Fixed.

364              This document defines a new optional, transitive BGP Path
Attributes

365              option, named "Only to Customer (OTC)" with an assigned
value <TBD3>

366              [To be removed upon publication:
http://www.iana.org/assignments/bgp- <https://www.iana.org/assignments/bgp->

367              parameters/bgp-parameters.xhtml#bgp-parameters-2]
[RFC4271].  The

368              length of this attribute is four bytes.

[major] s/a new optional, transitive BGP Path Attributes option, named

"Only to Customer (OTC)" with an assigned value <TBD3> [...]

[RFC4271]./the "Only to Customer (OTC)" BGP Path Attribute [...].

IANA has assigned code value 35.

[AA/KS] Fixed

[minor] The length is already specified in §6; no need to repeat that here.

[AA/KS] Fixed

370            10.  Security Considerations

372              This document proposes a mechanism for prevention of route
leaks that

373              are the result of BGP policy misconfiguration.

375              A misconfiguration in OTC setup may affect prefix
propagation.  But

376              the automation that is provided by BGP roles should make
such

377              misconfiguration unlikely.

[minor] Please add references to rfc4271 and rfc4272.

[AA/KS] Done

[major] There are multiple security issues that a rogue router

(whether it is due to misconfiguration or on purpose) can take

advantage of.  Prefix propagation, both in the "route will go further"

point of view, as well as when the "route will not go as far".  Please

elaborate a little on the differences.

Also, there are cases, with strict mode on for example, where the

session may not even be established.  Please include these risks in

this section.

[AA/KS] We have carefully revised the whole Security Considerations section
to incorporate the above comments.

...

405            11.2.  Informative References

...

419              [RFC7908]  Sriram, K., Montgomery, D., McPherson, D.,
Osterweil, E.,

420                         and B. Dickson, "Problem Definition and
Classification of

421                         BGP Route Leaks", RFC 7908, DOI
10.17487/RFC7908, June

422                         2016, <https://www.rfc-editor.org/info/rfc7908>.

[major] The definition of a route leak is the base for this work, so

this reference should be Normative.

[AA/KS] Fixed

424              [RFC8126]  Cotton, M., Leiba, B., and T. Narten,
"Guidelines for

425                         Writing an IANA Considerations Section in
RFCs", BCP 26,

426                         RFC 8126, DOI 10.17487/RFC8126, June 2017,

427                         <https://www.rfc-editor.org/info/rfc8126>.

[major] This reference should be Normative.

[AA/KS] Fixed

[End of Review -15]

вт, 15 июн. 2021 г. в 00:47, Alvaro Retana <[email-masked]>:

> On June 12, 2021 at 5:12:37 PM, Alexander Azimov wrote:
>
>
> Hi!
>
> ...
> > May I ask you to give more details on this part of your comment?
> > > Also, I believe the specification is not complete.
> > Can you amplify what parts of mechanics are missing?
>
> There are some comments in §4, 6, 7...about the use of multiple
> capabilities, error handling, multiple OTC attributes, incomplete
> policies, use of the OTC attribute if the capabilities were not
> successfully negotiated, etc.
>
> THT -- thanks!
>
> Alvaro.
>

-- 
Best regards,
Alexander Azimov

---

From: Alvaro Retana <[email-masked]>
Date: Tue, 10 Aug 2021 18:40:29 +0200

On August 10, 2021 at 12:23:09 PM, Alexander Azimov wrote:

Alexander:

Hi!

> Our (authors’) responses to your specific comments are inline below and

Thanks for the update!  I'll put this in my queue of things to review. :-)

> Before jumping to the long list of inline comments I'd like to highlight that
> while policy specification is better worded now, their principles remain
> unchanged.

Good!  The intent was to make the specification clear, not to change it.

Thanks!

Alvaro.

---

From: Alvaro Retana <[email-masked]>
Date: Wed, 18 Aug 2021 09:01:25 -0700

Alexander/Sriram:

The document as improved significantly!  Thank you for the hard work!!

I have just a couple of responses inline, and have appended a quick
review of -16.  In general, most of the remaining items should be easy
to address.  The only significant issue I want to discuss some more is
related to confederations (see below).  All these comments can be
considered last call comments.

> including the grow WG -- I have cc'd them in this message as well.

Sue: Please issue a WGLC (cc'ing grow) for this document.  A week
should be enough.

Once the WGLC is done and we have an updated document I will start the IETF LC.

Thanks!

Alvaro.

...
...
[AA/KS] There are some details that need to be understood. If a route was
[AA/KS] advertised by a Peer, Provider, or RS and it is not marked with OTC,
[AA/KS] then the ingress router adds an OTC Attribute with the AS number of
[AA/KS] the sender (remote AS). See revised Section 4. The mere presence of
[AA/KS] an OTC indicates to the egress router that the route was learned from
[AA/KS] a Peer, Provider, or RS and it can be advertised only to customers.
[AA/KS] The absence of an OTC Attribute indicates to the egress router that
[AA/KS] the route was either advertised by a Customer or originated by the
[AA/KS] local AS. If a route received from a Customer at an ingress router has
[AA/KS] an OTC Attribute, then the route is rejected by ingress policy. So, a
[AA/KS] customer route or a locally originated route never has OTC attached
[AA/KS] when received at the egress router. Now, this is explained well in the
[AA/KS] revised Section 4 in the updated draft.

Please add something like that (shorter) to §2.  Suggestion>

   As specified in §4, the Only to Customer (OTC) Attribute is used to
   identify all the routes in the AS that have been received from a Peer,
   Provider or RS.

...
[AA/KS] Yes, “ignore” -- meaning do not act on it and let the OTC Attribute
[AA/KS] propagate -- seems correct. IMO, the OTC processing at the ingress/
[AA/KS] egress of the confederation will happen correctly as per this draft
[AA/KS] (RFC). So, nothing specific to confederations needs to be said.
[AA/KS] Please advise.

The question is this:  §4 says (talking about egress policy) that "an
OTC Attribute MUST be added with a value equal to the AS number of the
local AS".  Should a router that sends an UPDATE to a member of the
confederation in a different ASN include the OTC?

Note that rfc5065 says this:

   A member of a BGP confederation MUST use its Member-AS Number in all
   transactions with peers that are members of the same confederation as
   the local BGP speaker.

...and rfc4271 defines eBGP simply as:

   A peer in a different AS is referred to as an external peer, while a
   peer in the same AS is referred to as an internal peer.  Internal BGP
   and external BGP are commonly abbreviated as IBGP and EBGP.

So, if a confederation peer is in a different ASN, then it is an eBGP
peer.  The egress policy should then result in an OTC with the a
Member-ASN.  Which results in the route not being propagated to a
Provider (assuming the confederation is a Customer).

If the OTC is ignored (as suggested above) and the route is advertised
to the Provider, who will not use the route anyway because it has an
OTC.

This assumes that the intra-confederation eBGP sessions are configured
to use BGP Roles.  Yes, I think this won't be too common...but it
could happen.

Thinking out loud -- I see a couple of possibilities:

1- Say that the rules in §4 don't apply if there is a private ASN in
it.  But this cconstrains other applications, specially ones where the
Customer doesn't have its own ASN.

2- Require that any OTCs added inside a confederation be removed
before a route is advertised externally.  This path is against this
text:

   Once the OTC Attribute has been set, it MUST be preserved unchanged.

Suggestion>
   A BGP speaker that advertises a route to a BGP speaker located in a
   neighboring autonomous system that is not a member of the local
   confederation [rfc5065] MUST remove any OTC Attributes whose value
   matches an ASN in the removed AS_CONFED_SEQUENCE or AS_CONFED_SET in
   the AS_PATH.  Otherwise, the OTC Attribute MUST be preserved unchanged.

rfc5065 would need to be a Normative reference.

[End Comments -15]

[Start of Review -16]

...
124	1.1.  Terminology

126	   In the rest of this document, the term "Peer" is used to refer to a
127	   "lateral peer" for simplicity.  Also, the terms Provider and Customer
128	   are used to refer to a transit provider and a transit customer,
129	   respectively.  Further, the terms RS and RS-Client are used to refer
130	   to a Route Server and its client, respectively.

[major] "lateral peer" is used in several places but not explicitly
defined.  What is a lateral peer?  How is it different from a
"regular" peer?  I looked at rfc7909 but couldn't find an explicit
definition there either -- did I miss it?  If so, then there's no need
to add anything else. :-)

...
173	3.  BGP Role

175	   The BGP Role characterizes the relationship between the eBGP speakers
176	   forming a session.  BGP Role is configured on a per-session basis.
177	   An eBGP speaker SHOULD configure the BGP Role locally based on the
178	   local AS's knowledge of its Role.  The only exception is when the
179	   eBGP connection is complex (see Section 5).  BGP Roles are mutually
180	   confirmed using the BGP Role Capability (described in Section 3.1) on
181	   each eBGP session between autonomous systems (ASes).  One of the
182	   Roles described below SHOULD be configured at the local AS for each
183	   eBGP session with a neighbor (remote AS) (see definitions in
184	   Section 1.1).

[minor] There's some redundant text...

Suggestion>
   The BGP Role characterizes the relationship between the eBGP
   speakers forming a session.  One of the Roles described below
   SHOULD be configured at the local AS for each eBGP session (see
   definitions in Section 1.1) based on the local AS's knowledge
   of its Role.  The only exception is when the eBGP connection is
   'complex' (see Section 5).  BGP Roles are mutually confirmed
   using the BGP Role Capability (described in Section 3.1) on
   each eBGP session.

...
227	3.2.  Role Correctness
...
251	   If the BGP Role Capability is sent, but one is not received, then the
252	   connection MAY be rejected using the Role Mismatch Notification (code
253	   2, subcode 8); this mode of operation is called the "strict mode".
254	   For backward compatibility, if the BGP speaker does not receive the
255	   capability from its peer, it SHOULD ignore the absence of BGP Role
256	   Capability and proceed with session establishment; this SHOULD be the
257	   default non-strict mode of operation.  In this case, the locally
258	   configured BGP Role is used for the procedures described in
259	   Section 4.

[major] "strict mode"

The text is not strict about "strict mode": rejecting the session is
optional.  You also added this text to the Security Considerations:

   Setting the strict mode of operation for BGP Role negotiation as the
   default may result in a situation where the eBGP session will not
   come up after a software update.  Such an implementation of this
   document is strongly discouraged.

Even if strict mode is not the default, configuring it may still
result in the same problem.   If discouraged then why even talk about
it?

Suggestion>

   For backward compatibility, if the BGP Role Capability is sent but
   one is not received, the BGP Speaker SHOULD ignore the absence of
   the BGP Role Capability and proceed with session establishment.
   The locally configured BGP Role is used for the procedures described
   in Section 4.

   ... later on in the security section ...

   Strictly requiring the presence of the BGP Role Capability may result
   in a situation where the eBGP session will not come up after a software
   update or configuration change.  Such an implementation of this document
   is strongly discouraged.

Note that this suggestion simplifies the text (no more "MAY...strict"
"contradiction") while not changing the functionality.

If the strict requirement is not good, is there a reason to not always
require the backwards compatible behavior?  If implementations already
reset sessions then I guess we can keep the "SHOULD".

261	   If an eBGP speaker receives multiple but identical BGP Role
262	   Capabilities with the same value in each, then the speaker MUST
263	   consider it to be a single BGP Role Capability and proceed [RFC5492].
264	   If multiple BGP Role Capabilities are received and not all of them
265	   have the same value, then the BGP speaker MUST reject the connection
266	   using the Role Mismatch Notification (code 2, subcode 8).

[major] s/the speaker MUST...[RFC5492]/the speaker must...[RFC5492]

This behavior is already specified in rfc5492, no need to specify it here again.

...
271	4.  BGP Only to Customer (OTC) Attribute
...
316	   The OTC Attribute may be set by the egress policy of remote AS or by
317	   the ingress policy of local AS.  In both scenarios, the OTC value
318	   will be the same.  This makes the scheme more robust and benefits
319	   early adopters.

[nit] s/egress policy of remote AS or by the ingress policy of local
AS/egress policy of the remote AS or by the ingress policy of the
local AS

...
334	   The described ingress and egress policies are applicable only for
335	   unicast IPv4 and IPv6 address families and MUST not affect other
336	   address families by default.  The operator MUST NOT have the ability
337	   to modify the policies defined in this section.

[major] s/MUST not/MUST NOT

[major] "MUST not affect other address families by default"   Maybe
"MUST NOT be applied to other address families by default" ??

339	5.  Additional Considerations
...
349	   No Roles SHOULD be configured on a 'complex' eBGP session (assuming
350	   it is not segregated) and in that case, the OTC Attribute processing
351	   MUST be done relying on configuration on a per-prefix basis.  Also,
352	   in this case, the per-prefix peering configuration MUST follow the
353	   same definitions of peering relations as described in Section 2.
354	   However, in this case, there are no built-in measures to check
355	   correctness of the per-prefix peering configuration.

[major] This paragraph requires the use of OTC in cases where a Role
is not configured.  I don't feel comfortable with that because it is
equivalent to mandating behavior for eBGP sessions that don't support
this document.  I would prefer it if this was worded as a
recommendation (non-normative).

Suggestion>
   No Roles SHOULD be configured on a 'complex' eBGP session (assuming
   it is not segregated).  An operator may want to achieve an equivalent
   outcome by configuring policies on a per-prefix basis to follow the
   definitions of peering relations as described in Section 2.  However,
   in this case, there are no built-in measures to check correctness of
   the per-prefix peering configuration.

357	   The incorrect setting of BGP Roles and/or OTC Attributes may affect
358	   prefix propagation.  Further, this document does not specify any
359	   special handling of incorrect AS numbers in the OTC Attribute.  Such
360	   errors should not happen with proper configuration.

[] Let's take out the last sentence.  The rest of the paragraph
implies that the value of the ASN doesn't matter/shouldn't be checked
(because it could be set elsewhere)...but calling it an error gives it
the importance that it doesn't have.

362	6.  IANA Considerations

364	   IANA has registered a new BGP Capability described in Section 3.1 in
365	   the "Capability Codes" registry's "IETF Review" range [RFC5492].  The
366	   description for the new capability is "BGP Role".  IANA has assigned
367	   the value 9 [to be removed upon publication:
368	   https://www.iana.org/assignments/capability-codes/capability-
369	   codes.xhtml].  This document is the reference for the new capability.

[nit] s/BGP Capability described in Section 3.1/BGP Capability (Section 3.1)

...
406	7.  Security Considerations
...
428	   Removing the OTC Attribute or changing its value can limit the
429	   opportunity of route leak detection.  Such activity can be done on
430	   purpose as part of a Man in the Middle (MITM) attack.  For example,
431	   an AS can remove the OTC Attribute on a received route and then leak
432	   the route to its transit provider.  Such malicious activity cannot be
433	   prevented without cryptographically signing the BGP UPDATE [RFC8205]
434	   or out of band detection [I-D.ietf-sidrops-aspa-verification], but
435	   such schemes are beyond the scope of this document.

[] As you may know, the IETF has been discussing the use of more
inclusive terminology.  Please consider changing this phrase: "on
purpose as part of a Man in the Middle (MITM) attack."

Suggestions:
"on purpose by a rogue router."
"on purpose as part of an on-path attack."

https://www.ietf.org/about/groups/iesg/statements/on-inclusive-language/

[major] "Such malicious activity cannot be prevented without
cryptographically signing the BGP UPDATE [RFC8205] or out of band
detection [I-D.ietf-sidrops-aspa-verification], but such schemes are
beyond the scope of this document."

BGPSec doesn't protect against the removal of the OTC -- or protect
any other attribute (except for the AS_PATH).

As far as I can tell, I-D.ietf-sidrops-aspa-verification doesn't
protect against the removal of the OTC either, and could only detect
anomalies related to the Customer/Peer(s) relationship.

The point here is the the potential mitigation would either not work
in this case or only provide a partial solution.  I think we would be
better off simply recognizing that this threat is not new in BGP and
that it may affect any attribute.

...
448	8.1.  Normative References
...
460	   [RFC4272]  Murphy, S., "BGP Security Vulnerabilities Analysis",
461	              RFC 4272, DOI 10.17487/RFC4272, January 2006,
462	              <https://www.rfc-editor.org/info/rfc4272>.

[major] This reference can be informative.

[EoR -16]