Subject: Re: [Idr] AD Review of draft-ietf-idr-ix-bgp-route-server-09

From: Nick Hilliard <[email-masked]>
Date: Fri, 29 Apr 2016 23:44:04 +0100

Hi Alvaro,

[the hex numbers are git commit tags for
github.com/fooelisa/draft-jasinska-ix-bgp-route-server, to make it
easier for the co-authors to follow what's going on.]

First, thanks for the comments - much appreciated.  And apologies for
the delay in replying.

To give you some background, this is one Internet Draft of a set of two:
draft-ietf-idr-ix-bgp-route-server was intended to be a standards track
document which outlines the differences between a standard BGP
implementation and a route server implementation. Its sister draft
(draft-ietf-grow-ix-bgp-route-server-operations) is an information draft
which deals with operational issues.

We deliberately steered clear of dealing with operational issues in this
draft because its focus was on the bgp protocol. Several of the comments
you make below are probably more relevant to
draft-ietf-grow-ix-bgp-route-server-operations. We've marked them as such.

Otherwise, I've submitted draft-ietf-idr-ix-bgp-route-server-10 with
your suggestions.

Alvaro Retana (aretana) wrote:
> 1.  Should this document be marked as updating rfc4271?  There are
> several places in Section 2 where the behavior is different than what
> is specified in rfc4271.  Given that the route server functionality
> is an optional enhancement, I don't think this document should be
> marked as updating rfc4271 (I.e. It changes the behavior only for the
> functionality specified here).  I'm assuming the authors/WG agree
> since there is no indication of an update.  Please include a clear
> statement (maybe in the Introduction) to the fact that the mechanism
> is optional.

added: "Route server functionality is not mandatory in BGP
implementations" to the introduction. [#0b70f51]

> 2.  Section 2.1. (Client UPDATE Messages). "The route server SHOULD
> forward UPDATE messages where appropriate…to its clients."  When is
> it appropriate?  I'm guessing that you mean something like "where
> local policy permits"..  Please be clear.

reworded [#4d0643a]

> 3.  Section 2.2.2. (AS_PATH Attribute)  Not prepending the route
> server's AS_PATH brings up a couple of issues/questions:
> *   RFC4271 talks about prepending the local ASN (when advertising to
> an EBGP peer), and it doesn't mention any exceptions.  This document
> includes an exception.  Clearly indicate the modifications to Section
> 5.1.2. (AS_PATH) of RFC4271.
> *   Section 6.3. (UPDATE Message Error Handling) of rfc4271 says that
> when an "…UPDATE message is received from an external peer, the local
> system MAY check whether the leftmost…AS in the AS_PATH attribute is
> equal to the autonomous system number of the peer that sent the
> message", and MUST send a NOTIFICATION if it doesn't.
> *   Please be explicit about the required behavior for both the route
> server, and the clients.

Cleared up 5.1.2 and 6.3 differences for both sender and receiver [#14334e5]

> *   Even though this check is optional in RFC4271, mandating that is
> not be done could be interpreted as a security issue.  Please include
> some text around it (and mitigation) in the Security Considerations
> section.
> *   Independent of the check, not including an ASN in the AS_PATH
> could (in the general case) result in loops; which I also interpret
> as a potential security issue.  Please include some text about this
> too — a mitigation option is clearly to not check the first ASN only
> when known route servers are peers.

Noted.  An extra MUST has been added to the spec to allow rsclients to
disable the check, and it's mentioned in the security section that this
check should only be disabled when the rsclient operator is sure that
the peer is a RS. (#f235abf)

> 4.  Section 2.2.4. (Communities Attributes) says that Communities
> "SHOULD NOT be modified, processed or removed.  However, if such an
> attribute is intended for processing by the route server itself, it
> MAY be modified or removed."  How does the route server know if the
> sender intended for a community to be processed by it, or not?
> Please add details.

I've reworded this as follows:

>    [...] SHOULD NOT be modified, processed or removed, except as
>    defined by local policy.  If a Communities Attribute is intended for
>    processing by the route server itself, as determined by local policy,
>    it MAY be modified or removed.

#99b0810

> 5.  Section 3. (Security Considerations)
> *   Besides the points above, because clients rely on the route
> server to implement outbound route filtering for them, there can be a
> privacy issue/route leak scenario where the route server can send
> routes to clients that it shouldn't have.  Not much can be done to
> mitigate this because it will mostly be due to a bad implementation
> or simply a "bad operator".
> *   You should point at relevant sections of RFC7454. 6.

I've added a reference to RFC7454 but am a bit unhappy about it because
this is more relevant to the route server operations draft than this
draft.  Like any BGP speaker, a route server can cause a huge amount of
damage is misconfigured or badly coded, and these problems are not even
slightly limited to prefix filtering issues. (#2df7be3)

> Minor:
>
> 1.  Please be consistent:
> *   s/exterior BGP/external BGP..or even just EBGP (yes, a nit..but
> that's how rfc4271 calls EBGP)

oops, fixed (#36fa650)

> *   s/best path selection process/Decision Process

I've changed most of these (#3611255).

> *   s/IX/IXP

Gah, missed that.

> 2.  Section 2.2.3. (MULTI_EXIT_DISC Attribute) specifically calls out
> the MED, but you already said in Section 2.2. (Attribute
> Transparency) that all optional attributes "SHOULD NOT be updated by
> the route server…and SHOULD be passed on to other route server
> clients."  Is there anything special about the MED that I'm missing?
> If not, then you should be able to delete the section.

This was added to make it clear to implementers that of all the
transitive attributes, the NH, AS_PATH and MED are the most important.
There are lots of transitive attributes defined for BGP, but we
enumerated those three specifically because if any of them is left out,
route server functionality will be broken.

> 3.  In Section 2.3.1. (Path Hiding on a Route Server) policy
> implementation is described in terms of outbound route filtering.
> The description of the example says that if "AS1's policy prevents
> AS2's path from being accepted, then AS1 would never receive a path
> to this prefix".  To be consistent with the outbound filtering
> description, you might want to change that text to something like
> "AS2's policy prevents AS1 from receiving the path, …"

Right, good point.  That was written from the point of view of the IXP
operator who writes the outbound policy for AS1, which is really AS2's
policy, not AS1's.  (#2437a52)

Nick