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

Alvaro Retana <aretana.ietf@gmail.com> Wed, 09 June 2021 19:40 UTC

Return-Path: <aretana.ietf@gmail.com>
X-Original-To: idr@ietfa.amsl.com
Delivered-To: idr@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id A20B53A237B; Wed, 9 Jun 2021 12:40:01 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -2.097
X-Spam-Level:
X-Spam-Status: No, score=-2.097 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, FREEMAIL_FROM=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, UNPARSEABLE_RELAY=0.001, URIBL_BLOCKED=0.001] autolearn=ham autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (2048-bit key) header.d=gmail.com
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id htr_nBUAobZL; Wed, 9 Jun 2021 12:39:56 -0700 (PDT)
Received: from mail-ej1-x632.google.com (mail-ej1-x632.google.com [IPv6:2a00:1450:4864:20::632]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id C817C3A237C; Wed, 9 Jun 2021 12:39:55 -0700 (PDT)
Received: by mail-ej1-x632.google.com with SMTP id ci15so40157929ejc.10; Wed, 09 Jun 2021 12:39:55 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:mime-version:date:message-id:subject:to:cc :content-transfer-encoding; bh=nVd3wJqGRx3q1O2M797QdeVL3ZcUNiyDbv4YSDdM0Qs=; b=iOuj/t+mu3eQJ7zzJkY/bGF4Ai7SBvOjVCssdKc7HGe00+qcdCV/1SdjITTQVmsnWQ 7NpaxTxOkbnuKGcM+k3ouzxdzRnSWLRiABQMRDqba0Nnsrd34JRxGreEUYb8Z9+zRzPy yu2xthNsjBTlumRsR49enPNt9eSEpexEJBQnuVo+vVHuUTBY1ut4v5HRmLyRmfywHfto o2tShiUKCwmOpdXCk2LmmKk6O/tNlmjd9l8k8rNvoitC/Fo6jc/vCa2dRzM8+5k3PJ5i XuYLbGgSRUGnA5qvt9cER6Dm2uOFZ0ujQUCJ8Ak5tIrNjHZ1XCv6VNvu0VodJ+tmx4e1 k3fQ==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:mime-version:date:message-id:subject:to:cc :content-transfer-encoding; bh=nVd3wJqGRx3q1O2M797QdeVL3ZcUNiyDbv4YSDdM0Qs=; b=NjE1QD9Ypm4GPkNm6f2Hmn4mg7LLIEORMfcfOczpIm/3q5ZRnsscGFx+vjJOi02sQ8 pAEiH0tzyeHzDODy1IOp0ZC6CNDnmHehV3Adw4C2CjNVFL0YJn4M6JjWFPuaaczYHrkp 1i9SyO3Y5Hz2b1LTAf/2/W1MX+urZgtmG/bxEff96HuqGqBCO/eZMuLHRUFGkVCS6xDp wM/zYw/QD+Y1zE05mZU/fTjF1dMJVD2vQwJyehhAphSnM8wPEZxcDjUwVlVfyzR4h4Sf y1Y2fucjWQ0QRjGOdEwbs/rsBDZUTzTnk77OpwlAvDhspcpjKS+RUFVViz9Bz7GV1Fx2 pE+A==
X-Gm-Message-State: AOAM530DeCnaBsW3482cM0nrO4aM7xYSY+RTOnD8qxT75dGVHREPTlje nehEn3ItnUPOkKKbDSjPkJx3y2mONAeZ9MuYux4lWsDahaU=
X-Google-Smtp-Source: ABdhPJyFl+nBiMXcJW2VRyMpnHl7+He5yiJiDl8eYIPAAFkcGvJj6QJ2ZhTykOonghytcYLMYP65XiHoTpBoCu8zmP4=
X-Received: by 2002:a17:906:fcbb:: with SMTP id qw27mr1326404ejb.478.1623267591569; Wed, 09 Jun 2021 12:39:51 -0700 (PDT)
Received: from 1058052472880 named unknown by gmailapi.google.com with HTTPREST; Wed, 9 Jun 2021 12:39:50 -0700
From: Alvaro Retana <aretana.ietf@gmail.com>
MIME-Version: 1.0
Date: Wed, 09 Jun 2021 12:39:50 -0700
Message-ID: <CAMMESszyrdFsaYYtvTo_0jHnsWGsbc-0aSthUi7pmNUt+U+GsA@mail.gmail.com>
To: draft-ietf-idr-bgp-open-policy@ietf.org
Cc: Susan Hares <shares@ndzh.com>, "idr-chairs@ietf.org" <idr-chairs@ietf.org>, IDR List <idr@ietf.org>, "grow@ietf.org grow@ietf.org" <grow@ietf.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: <https://mailarchive.ietf.org/arch/msg/idr/K3-50yGFey0jSJo63J2xBLDj71Y>
Subject: [Idr] AD Review of draft-ietf-idr-bgp-open-policy-15
X-BeenThere: idr@ietf.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Inter-Domain Routing <idr.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/idr>, <mailto:idr-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/idr/>
List-Post: <mailto:idr@ietf.org>
List-Help: <mailto:idr-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/idr>, <mailto:idr-request@ietf.org?subject=subscribe>
X-List-Received-Date: Wed, 09 Jun 2021 19:40:02 -0000

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


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


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.


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


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


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.


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


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


[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


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.


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

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:


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.


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.


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

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.

This issue is not specific to Customers, so add the extra text after
the roles are defined.


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

Suggestion>
   Customer: MAY advertise any route learned from a Customer, or locally
   originated, to a Provider.  All other routes MUST NOT be propagated.


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


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.


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.


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


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


[?] "their configuration is not part of BGP itself"  I don't
understand what this phrase is intended to mean.


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.


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.


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

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

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


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


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 ad follows:

   o  Code: 9
   ...


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


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


209	5.  Role correctness

[minor] This section talks about the validation fo the capability.
Please make it a sub-section of §4.


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.


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.

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.

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.


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


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.


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


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


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


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


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


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.


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


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


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


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.


[major] There is no corresponding ingress policy that checks the ASN
to match routes received from Providers or RS.  (#2 only talks about
Peers)


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


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.


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.


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


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

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

Whichever the case, it should be clearly called out somewhere.  [This
point is closely related to Mach's rtg-dir review.]


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.

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.


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.


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


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


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


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


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


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


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 un the Capability Codes registry -- you need to be
explicit about that.


[major] The paragraph indicates two different allocation procedures:
Standard Action, and then others for different parts of the range.
Which is it?


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


[minor] Please add a table here to show the values.  Pointing at Table
4 is not enough because it doesn't include the unassigned range.


[minor] Also, please include a table showing the ranges and the
allocation policy, if more than one.


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


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


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


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.

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


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


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.

[End of Review -15]