Re: bgpd: stop with the announce confusion

2018-06-07 Thread Theo de Raadt
Job Snijders  wrote:

> On Thu, Jun 07, 2018 at 12:14:07PM +0200, Claudio Jeker wrote:
> > > It would be helpful during upgrades if it's possible to write some
> > > configurations that work the same on both the old and new versions.
> > > That way the configuration can be changed to a version which will
> > > still work before you get to the point where you can't simply revert
> > > to the old config file). I think if the new version would accept
> > > but ignore "announce all", that would be enough.
> > 
> > Yes, I can add a dummy "announce *" handler. In general people should be
> > already able to write filters that work before and after the change.
> > Happy to add a bit of code to ease the change but it also comes with a
> > price that users may not adjust their configs because they don't fail to
> > load.
> 
> I'd prefer if only "announce all" is allowed as a no-op (but a warning
> about the directive's deprecation is printed), for "announce self" and
> "announce default-route" just throw an error and refuse to start the
> daemon.

Accepting it, but doing a syslog... for about a year or so, would ensure
noone gets hooped.  bgp speakers are quite often remote...



Re: bgpd: stop with the announce confusion

2018-06-07 Thread Job Snijders
On Thu, Jun 07, 2018 at 12:14:07PM +0200, Claudio Jeker wrote:
> > It would be helpful during upgrades if it's possible to write some
> > configurations that work the same on both the old and new versions.
> > That way the configuration can be changed to a version which will
> > still work before you get to the point where you can't simply revert
> > to the old config file). I think if the new version would accept
> > but ignore "announce all", that would be enough.
> 
> Yes, I can add a dummy "announce *" handler. In general people should be
> already able to write filters that work before and after the change.
> Happy to add a bit of code to ease the change but it also comes with a
> price that users may not adjust their configs because they don't fail to
> load.

I'd prefer if only "announce all" is allowed as a no-op (but a warning
about the directive's deprecation is printed), for "announce self" and
"announce default-route" just throw an error and refuse to start the
daemon.

Kind regards,

Job



Re: bgpd: stop with the announce confusion

2018-06-07 Thread Claudio Jeker
On Thu, Jun 07, 2018 at 12:14:07PM +0200, Claudio Jeker wrote:
> On Thu, Jun 07, 2018 at 10:20:17AM +0100, Stuart Henderson wrote:
> > On 2018/06/07 10:26, Claudio Jeker wrote:
> > > On Wed, Jun 06, 2018 at 11:04:56PM +0200, Claudio Jeker wrote:
> > > > So the announce keyword in bgpd is massivly overloaded.
> > > > It is one of the most common things new bgpd users are unsure about.
> > > > These are all possible announce options:
> > > > 
> > > > announce (IPv4|IPv6) (none|unicast|vpn)
> > > > announce as-4byte (yes|no)
> > > > announce capabilities (yes|no)
> > > > announce refresh (yes|no)
> > > > announce restart (yes|no)
> > > > and
> > > > announce (all|none|self|default-route)
> > > > 
> > > > Especially the last one is a bit confusing because it influences the way
> > > > the filter work. This was good in the beginning of bgpd but now I think
> > > > the time is right to move on. Filtering in general got more important 
> > > > and
> > > > using large-communities as a method to tag prefixes so that they can be
> > > > matched against those later is important.
> > > > People are easily confused by `announce IPv4 unicast` vs `announce all`
> > > > and so on.
> > > > 
> > > > The following diff does a few things.
> > > > a) it removes the `announce (all|none|self|default-route)` version
> > > > b) `announce none` is now `export none`
> > > > c) `announce default-route` is now `export default-route`
> > > > d) the examples file is adjusted accordingly and also the network
> > > >statements are no setting a large-community which is used by the
> > > >filters.
> > > > e) the filters are split in input and output and to a lesser extent
> > > >ibgp vs ebgp.
> > > > f) since anounce self is gone the filters will now default deny (which 
> > > > is
> > > >not a big thing and has the benefit that at start all routes show up 
> > > > in
> > > >Adj-RIB-In first).
> > > > 
> > > > In general people using `announce all` can just remove the line. When
> > > > using `annouce self` more care is needed since the filters may need 
> > > > extra
> > > > adjustment. `announce none` and `announce default-route` are simple
> > > > renames, every thing else remains the same.
> > > > 
> > > 
> > > To be more precise what needs to be changed in your config.
> > > 
> > > - neighbors using 'announce none' just rename it to 'export none'
> > > - neighbors using 'announce default-route' just rename it to 'export
> > >   default-route'
> > > 
> > > For all other cases 'announce self' and 'announce all' needs to be removed
> > > and filters need to be put in place instead. A good example is in
> > > /etc/example/bgpd.conf. In short you want to permit ibgp in and out and
> > > for ebgp you want to only allow reasonable things in and only your
> > > announcements out. How is this done best is by tagging announcements with
> > > a community. I use large-community here because they are more expressive.
> > > 
> > > # tag announcements with a community
> > > network 10.0.1.0/24 set large-community $ASN:1:1
> > > 
> > > ... neighbor configs ...
> > > 
> > > # first inbound
> > > allow from ibgp
> > > # the example filters out a lot of bad prefixes here which is good
> > > # In general you don't trust the outside world so this is where you should
> > > # filter the most.
> > > allow from ebgp inet prefixlen 8 - 24
> > > allow from ebgp inet6 prefixlen 16 - 48
> > > # scrub our community range from ebgp
> > > match from ebgp set large-community delete $ASN:*:*
> > > 
> > > # now outbound filters
> > > # again ibgp is generally allowed
> > > allow to ibgp
> > > # for ebgp make sure to not leak any unintended networks so best is to
> > > # just allow the networks we tagged.
> > > allow to ebgp large-community $ASN:1:1
> > > # another option is to go eplicit by network
> > > allow to ebgp prefix 10.0.1.0/24
> > > 
> > > For a pure ibgp router the basic rules can be simplified to:
> > >allow from ibgp
> > >allow to ibgp
> > > 
> > > While most configs probably have inbound filters they may lack outbound
> > > filters. It is possible to check the effect of this change by adding
> > >deny from any
> > >deny to any
> > > at the start of your filters. `bgpctl show rib out nei myPeerName` is a
> > > great way to see what is announced before and after.
> > > 
> > > See this as an oportunity to bring your bgpd filter up to current
> > > standards.
> > > -- 
> > > :wq Claudio
> > > 
> > 
> > It would be helpful during upgrades if it's possible to write some
> > configurations that work the same on both the old and new versions.
> > That way the configuration can be changed to a version which will
> > still work before you get to the point where you can't simply revert
> > to the old config file). I think if the new version would accept
> > but ignore "announce all", that would be enough.
> 
> Yes, I can add a dummy "announce *" handler. In general people should be
> already able to write filters that work before 

Re: bgpd: stop with the announce confusion

2018-06-07 Thread Job Snijders
On Wed, Jun 06, 2018 at 11:04:56PM +0200, Claudio Jeker wrote:
> The following diff does a few things.
> a) it removes the `announce (all|none|self|default-route)` version
> b) `announce none` is now `export none`
> c) `announce default-route` is now `export default-route`
> d) the examples file is adjusted accordingly and also the network
>statements are no setting a large-community which is used by the
>filters.
> e) the filters are split in input and output and to a lesser extent
>ibgp vs ebgp.
> f) since anounce self is gone the filters will now default deny (which is
>not a big thing and has the benefit that at start all routes show up in
>Adj-RIB-In first).

OK job@

I built a release & tested on a few routers.

I like Stuart's suggestion, if the directive "announce all" is made a
no-op in 6.4 (and removed in 6.5), I can produce configs that work on
both 6.3 and 6.4, making the upgrade easier.

Kind regards,

Job



Re: bgpd: stop with the announce confusion

2018-06-07 Thread Claudio Jeker
On Thu, Jun 07, 2018 at 10:20:17AM +0100, Stuart Henderson wrote:
> On 2018/06/07 10:26, Claudio Jeker wrote:
> > On Wed, Jun 06, 2018 at 11:04:56PM +0200, Claudio Jeker wrote:
> > > So the announce keyword in bgpd is massivly overloaded.
> > > It is one of the most common things new bgpd users are unsure about.
> > > These are all possible announce options:
> > > 
> > > announce (IPv4|IPv6) (none|unicast|vpn)
> > > announce as-4byte (yes|no)
> > > announce capabilities (yes|no)
> > > announce refresh (yes|no)
> > > announce restart (yes|no)
> > > and
> > > announce (all|none|self|default-route)
> > > 
> > > Especially the last one is a bit confusing because it influences the way
> > > the filter work. This was good in the beginning of bgpd but now I think
> > > the time is right to move on. Filtering in general got more important and
> > > using large-communities as a method to tag prefixes so that they can be
> > > matched against those later is important.
> > > People are easily confused by `announce IPv4 unicast` vs `announce all`
> > > and so on.
> > > 
> > > The following diff does a few things.
> > > a) it removes the `announce (all|none|self|default-route)` version
> > > b) `announce none` is now `export none`
> > > c) `announce default-route` is now `export default-route`
> > > d) the examples file is adjusted accordingly and also the network
> > >statements are no setting a large-community which is used by the
> > >filters.
> > > e) the filters are split in input and output and to a lesser extent
> > >ibgp vs ebgp.
> > > f) since anounce self is gone the filters will now default deny (which is
> > >not a big thing and has the benefit that at start all routes show up in
> > >Adj-RIB-In first).
> > > 
> > > In general people using `announce all` can just remove the line. When
> > > using `annouce self` more care is needed since the filters may need extra
> > > adjustment. `announce none` and `announce default-route` are simple
> > > renames, every thing else remains the same.
> > > 
> > 
> > To be more precise what needs to be changed in your config.
> > 
> > - neighbors using 'announce none' just rename it to 'export none'
> > - neighbors using 'announce default-route' just rename it to 'export
> >   default-route'
> > 
> > For all other cases 'announce self' and 'announce all' needs to be removed
> > and filters need to be put in place instead. A good example is in
> > /etc/example/bgpd.conf. In short you want to permit ibgp in and out and
> > for ebgp you want to only allow reasonable things in and only your
> > announcements out. How is this done best is by tagging announcements with
> > a community. I use large-community here because they are more expressive.
> > 
> > # tag announcements with a community
> > network 10.0.1.0/24 set large-community $ASN:1:1
> > 
> > ... neighbor configs ...
> > 
> > # first inbound
> > allow from ibgp
> > # the example filters out a lot of bad prefixes here which is good
> > # In general you don't trust the outside world so this is where you should
> > # filter the most.
> > allow from ebgp inet prefixlen 8 - 24
> > allow from ebgp inet6 prefixlen 16 - 48
> > # scrub our community range from ebgp
> > match from ebgp set large-community delete $ASN:*:*
> > 
> > # now outbound filters
> > # again ibgp is generally allowed
> > allow to ibgp
> > # for ebgp make sure to not leak any unintended networks so best is to
> > # just allow the networks we tagged.
> > allow to ebgp large-community $ASN:1:1
> > # another option is to go eplicit by network
> > allow to ebgp prefix 10.0.1.0/24
> > 
> > For a pure ibgp router the basic rules can be simplified to:
> >allow from ibgp
> >allow to ibgp
> > 
> > While most configs probably have inbound filters they may lack outbound
> > filters. It is possible to check the effect of this change by adding
> >deny from any
> >deny to any
> > at the start of your filters. `bgpctl show rib out nei myPeerName` is a
> > great way to see what is announced before and after.
> > 
> > See this as an oportunity to bring your bgpd filter up to current
> > standards.
> > -- 
> > :wq Claudio
> > 
> 
> It would be helpful during upgrades if it's possible to write some
> configurations that work the same on both the old and new versions.
> That way the configuration can be changed to a version which will
> still work before you get to the point where you can't simply revert
> to the old config file). I think if the new version would accept
> but ignore "announce all", that would be enough.

Yes, I can add a dummy "announce *" handler. In general people should be
already able to write filters that work before and after the change.
Happy to add a bit of code to ease the change but it also comes with a
price that users may not adjust their configs because they don't fail to
load.
 
> Remember that "announce self" is implicit for ebgp neighbours unless
> there's another "announce" keyword, so people might be using 

Re: bgpd: stop with the announce confusion

2018-06-07 Thread Stuart Henderson
On 2018/06/07 10:26, Claudio Jeker wrote:
> On Wed, Jun 06, 2018 at 11:04:56PM +0200, Claudio Jeker wrote:
> > So the announce keyword in bgpd is massivly overloaded.
> > It is one of the most common things new bgpd users are unsure about.
> > These are all possible announce options:
> > 
> > announce (IPv4|IPv6) (none|unicast|vpn)
> > announce as-4byte (yes|no)
> > announce capabilities (yes|no)
> > announce refresh (yes|no)
> > announce restart (yes|no)
> > and
> > announce (all|none|self|default-route)
> > 
> > Especially the last one is a bit confusing because it influences the way
> > the filter work. This was good in the beginning of bgpd but now I think
> > the time is right to move on. Filtering in general got more important and
> > using large-communities as a method to tag prefixes so that they can be
> > matched against those later is important.
> > People are easily confused by `announce IPv4 unicast` vs `announce all`
> > and so on.
> > 
> > The following diff does a few things.
> > a) it removes the `announce (all|none|self|default-route)` version
> > b) `announce none` is now `export none`
> > c) `announce default-route` is now `export default-route`
> > d) the examples file is adjusted accordingly and also the network
> >statements are no setting a large-community which is used by the
> >filters.
> > e) the filters are split in input and output and to a lesser extent
> >ibgp vs ebgp.
> > f) since anounce self is gone the filters will now default deny (which is
> >not a big thing and has the benefit that at start all routes show up in
> >Adj-RIB-In first).
> > 
> > In general people using `announce all` can just remove the line. When
> > using `annouce self` more care is needed since the filters may need extra
> > adjustment. `announce none` and `announce default-route` are simple
> > renames, every thing else remains the same.
> > 
> 
> To be more precise what needs to be changed in your config.
> 
> - neighbors using 'announce none' just rename it to 'export none'
> - neighbors using 'announce default-route' just rename it to 'export
>   default-route'
> 
> For all other cases 'announce self' and 'announce all' needs to be removed
> and filters need to be put in place instead. A good example is in
> /etc/example/bgpd.conf. In short you want to permit ibgp in and out and
> for ebgp you want to only allow reasonable things in and only your
> announcements out. How is this done best is by tagging announcements with
> a community. I use large-community here because they are more expressive.
> 
> # tag announcements with a community
> network 10.0.1.0/24 set large-community $ASN:1:1
> 
> ... neighbor configs ...
> 
> # first inbound
> allow from ibgp
> # the example filters out a lot of bad prefixes here which is good
> # In general you don't trust the outside world so this is where you should
> # filter the most.
> allow from ebgp inet prefixlen 8 - 24
> allow from ebgp inet6 prefixlen 16 - 48
> # scrub our community range from ebgp
> match from ebgp set large-community delete $ASN:*:*
> 
> # now outbound filters
> # again ibgp is generally allowed
> allow to ibgp
> # for ebgp make sure to not leak any unintended networks so best is to
> # just allow the networks we tagged.
> allow to ebgp large-community $ASN:1:1
> # another option is to go eplicit by network
> allow to ebgp prefix 10.0.1.0/24
> 
> For a pure ibgp router the basic rules can be simplified to:
>allow from ibgp
>allow to ibgp
> 
> While most configs probably have inbound filters they may lack outbound
> filters. It is possible to check the effect of this change by adding
>deny from any
>deny to any
> at the start of your filters. `bgpctl show rib out nei myPeerName` is a
> great way to see what is announced before and after.
> 
> See this as an oportunity to bring your bgpd filter up to current
> standards.
> -- 
> :wq Claudio
> 

It would be helpful during upgrades if it's possible to write some
configurations that work the same on both the old and new versions.
That way the configuration can be changed to a version which will
still work before you get to the point where you can't simply revert
to the old config file). I think if the new version would accept
but ignore "announce all", that would be enough.

Remember that "announce self" is implicit for ebgp neighbours unless
there's another "announce" keyword, so people might be using it without
realising. It would not be very good if those people (who are probably
not dealing with BGP often) started redistributing all their transit
routes to peers ;)



Re: bgpd: stop with the announce confusion

2018-06-07 Thread Claudio Jeker
On Wed, Jun 06, 2018 at 11:04:56PM +0200, Claudio Jeker wrote:
> So the announce keyword in bgpd is massivly overloaded.
> It is one of the most common things new bgpd users are unsure about.
> These are all possible announce options:
> 
> announce (IPv4|IPv6) (none|unicast|vpn)
> announce as-4byte (yes|no)
> announce capabilities (yes|no)
> announce refresh (yes|no)
> announce restart (yes|no)
> and
> announce (all|none|self|default-route)
> 
> Especially the last one is a bit confusing because it influences the way
> the filter work. This was good in the beginning of bgpd but now I think
> the time is right to move on. Filtering in general got more important and
> using large-communities as a method to tag prefixes so that they can be
> matched against those later is important.
> People are easily confused by `announce IPv4 unicast` vs `announce all`
> and so on.
> 
> The following diff does a few things.
> a) it removes the `announce (all|none|self|default-route)` version
> b) `announce none` is now `export none`
> c) `announce default-route` is now `export default-route`
> d) the examples file is adjusted accordingly and also the network
>statements are no setting a large-community which is used by the
>filters.
> e) the filters are split in input and output and to a lesser extent
>ibgp vs ebgp.
> f) since anounce self is gone the filters will now default deny (which is
>not a big thing and has the benefit that at start all routes show up in
>Adj-RIB-In first).
> 
> In general people using `announce all` can just remove the line. When
> using `annouce self` more care is needed since the filters may need extra
> adjustment. `announce none` and `announce default-route` are simple
> renames, every thing else remains the same.
> 

To be more precise what needs to be changed in your config.

- neighbors using 'announce none' just rename it to 'export none'
- neighbors using 'announce default-route' just rename it to 'export
  default-route'

For all other cases 'announce self' and 'announce all' needs to be removed
and filters need to be put in place instead. A good example is in
/etc/example/bgpd.conf. In short you want to permit ibgp in and out and
for ebgp you want to only allow reasonable things in and only your
announcements out. How is this done best is by tagging announcements with
a community. I use large-community here because they are more expressive.

# tag announcements with a community
network 10.0.1.0/24 set large-community $ASN:1:1

... neighbor configs ...

# first inbound
allow from ibgp
# the example filters out a lot of bad prefixes here which is good
# In general you don't trust the outside world so this is where you should
# filter the most.
allow from ebgp inet prefixlen 8 - 24
allow from ebgp inet6 prefixlen 16 - 48
# scrub our community range from ebgp
match from ebgp set large-community delete $ASN:*:*

# now outbound filters
# again ibgp is generally allowed
allow to ibgp
# for ebgp make sure to not leak any unintended networks so best is to
# just allow the networks we tagged.
allow to ebgp large-community $ASN:1:1
# another option is to go eplicit by network
allow to ebgp prefix 10.0.1.0/24

For a pure ibgp router the basic rules can be simplified to:
   allow from ibgp
   allow to ibgp

While most configs probably have inbound filters they may lack outbound
filters. It is possible to check the effect of this change by adding
   deny from any
   deny to any
at the start of your filters. `bgpctl show rib out nei myPeerName` is a
great way to see what is announced before and after.

See this as an oportunity to bring your bgpd filter up to current
standards.
-- 
:wq Claudio