fix 4bASN bug ;add interface and enh support for bird

[KusakabeShi]

1. Do not generate (comm.ext, "131072..4199999999") if router_asn is 4 byte

When I use 4bytes router AS(114514) with this client.yml

It will geneeate this bird configs:

Which cause this error:

Because we can't put two 4byte value into the extended community

So I add a check at templates/bird/macros.j2 line L74, if there are any 4 byte value in the comm.ext, it will skip the line.

Router AS1145: Router AS114514:

2. Add interface and extended next hop section support for bird

client.yml

Result:

• Note: extended next hop only works properly at bird >= 2.0.8

[pierky]

Hi @KusakabeShi.

thanks for filing this PR.

Extended communities and 32bit ASN for router server

Regarding the extended BGP communities issue, I definitely overlooked this part on the templates. It's a corner case that is hit when a 32bit ASN is used for the router server. This is somehow already handled in ARouteServer's configure command, which prompts the user with the following question after a 32bit ASN is given for the router server:

Place-holder 16-bit ASN for BGP communities
===========================================

Since the ASN used for the route server is a 32-bit value, features and services
offered to clients using (standard) BGP communities need to be configured using
a placeholder 16 bit ASN. Please provide the value of the 16 bit ASN to be used
as placeholder (range: 64512-65534).

16-bit placeholder ASN used for BGP communities (default: 65534)

So when a 32bit ASN is provided for the route server, the tool implements a workaround to use a "fake" placeholder ASN for the extended communities.

Even though it is a corner case, I'll add some checks in the code to prompt the user when a similar case is found. I'll also merge your commit where you implemented the fix on the templates, and most likely I'll extend the same approach to OpenBGPD.

Interface and extended next-hop

For what regards the addition of interface and extended next-hop, I'm not sure whether that should be considered as a "core" feature to be added to the tool, or whether it should be implemented using the capabilities that the tool offers for site-specific customizations.

Most likely they could already be implemented using include_local_file and hooks. It'd be great if you could confirm whether you attempted to get a working config using those customization features and if you faced any issues that blocked you on that.

authorized_addresses_list

This is a bit unclear to me how you used it in the templates. If I'm correct I don't see this new config knob being used in any template on this branch. Would you mind expanding a bit more on this?

Thanks again!

[KusakabeShi]

authorized_addresses_list

About the authorized_addresses_list part, it's a bug of bird that we can't access the bgp_next_hop attrubute if we establish the session via ipv6 link local, we can only get :: in the filter.

So that all routes will be filtered by this reason except we set the policy to authorized_addresses and spacify the authorized_addresses_list to ::

But we can't set authorized_addresses_list in generial.yml, we have to put it at every client in client.yml

This patch: https://github.com/pierky/arouteserver/pull/104#discussion_r839154391 allows us set the authorized_addresses_list attribute in the generial.yml

Interface

I think spacify the interface for the bgp session can improve the security of our session and bird support it, it's nice if we support it, too.

Extended next hop

We can use include_local_file to customize protocol setting, but we can't use it to cuztomize channel setting. It's not in the channel block. https://github.com/pierky/arouteserver/blob/79fb649e51c1f76a608c33164a0d1fd4860cd75f/templates/bird/clients.j2#L474

Maybe we can generialize this? Allow users define custom protocol setting and channel setting in the client.cfg option.

[pierky]

Hello @KusakabeShi,

thanks for the clarification.

I think I've addressed the 32bit ext comms issue in the 32bit_asn_ext_comms branch. The CI pipeline is running now, but all the preliminary checks all look good so far.

Regarding the authorized_addresses_list, I see now what your idea was, thanks for clarifying it. Since this seems more like a BIRD limitation, I think the solution you proposed is a bit too aggressive, since it would impact the entire "data model". Also, it's using an already-existing option that is designed to do kind of the opposite of what it would do in this situation. Like adding :: on it would basically defeat the entire next-hop validation that is using that option today. At the moment I'm more inclined towards implementing a workaround in the BIRD templates only. That would restrict the scope of the workaround just to the affected platform (not sure if OpenBGPD is also affected, I'll check it as part of the work). Also, the meaning of authorized_addresses_list would not be overloaded with something that seems a bit far from its actual goal.

I'm thinking to something along these lines:

function next_hop_is_valid_for_{{ client.id }}()
{
    # Checks if NEXT_HOP is one of those allowed for routes announced by {{ client.id }}
{% if client.ip|ipaddr_ver == 6 and (client.ip|lower).startswith("fe80::") %}
    # The client address is an IPv6 link-local one; we can't check it because [description of the issue].
    return true;
{% endif %}
...

Does it make sense to you?

For what concerns the addition of the interface and extended next-hop I'll revisit it as soon as I'll have addressed these other two points (ext comms with 32bit ASNs and next-hop validation for link-local sessions). I think I'll try to expand the customization features a bit more, making them more generic, to allow users to inject custom snippets of config that could help achieving what you needed here.

[KusakabeShi]

Extended next hop & interface

Yes, we can add interface section by include_local_file. But it's global, we can't customize it for each member in the IX...

How about we add this?

authorized_addresses_list

It sounds great, Because it's BIRD limitation, we can handle this internally until BIRD fix it. Shall we show a warning to users that nexthop validation will be ignored?

By the way, interface section is required if the client IP is ipv6 link local address.

[pierky]

Hi @KusakabeShi,

some progress here, please check e3e5bff279355497f6c92a5a647e1bd533dad43e if you would like to see what I've been working on. It's a way to define custom options at client-definition level, very close to what you proposed here I think. It's in the custom_options at the moment, CI/CD running for it.

The 32bit ext comms fix was merged into the dev branch in the meantime. I'll keep you posted.

[pierky]

@KusakabeShi, I've just pushed a new branch to implement the next-hop check bypass for IPv6 link-local addresses, commit 6b895627dbd7cb397d06d6854e56635f22d59267. CI/CD is running right now.

With this commit, if everything goes well, I plan to close this PR: could you please check if what was done here satisfies your needs? Thanks

[KusakabeShi]

There is a minor issue which may confuse user: the channel configuration and protocol configuration are different.

But here: ipv4/ipv6/any make users thought they are same thing.

For example, cost is a channel configuration, I can put it in ipv4 channel and ipv6 channel at the same time, but I can't put it in protocol configuration.

[KusakabeShi]

btw1: I'm confusing about how make authorized_addresses_list available from general.yml impact the entire "data model"?

I think "For any option, general.yml is the default option for every client. Options in clients.yml can overwrite it" is more consistent rather than " but some options are not available from general.yml"

btw2, how about make the custom_options available from general.yml?

[pierky]

Hello,

I see your point about the confusion between protocol and channel in BIRD 2 for what regards custom_options. I've made some changes under c268d6bf75c74711c4db5c73b9e614187be8d9b1, where an extra level of granularity is offered. Mind checking if that makes more sense for you?

btw1: I'm confusing about how make authorized_addresses_list available from general.yml impact the entire "data model"?

authorized_addresses_list is used to describe something that is very specific to the client to which it applies, it is used to filter the NEXT_HOP of the routes announced by each client, that's why in my opinion should not be set at general level.

What's the use case for which you would like to have a value of authorized_addresses_list that is shared by all the clients? The one originally reported in this issue was to work around a limitation of BIRD, which has been addressed in a different way already here.

I think "For any option, general.yml is the default option for every client. Options in clients.yml can overwrite it" is more consistent rather than " but some options are not available from general.yml" btw2, how about make the custom_options available from general.yml?

I think I can answer both points saying that general.yml is not really meant to be a "superset" of all the options which are eventually used to configure clients. It's more like a general policy for the route server: for certain parameters of that policy it makes sense to use values from general.yml to also configure clients, but broadly speaking is not supposed to be used as the template on top of which build more specific clients definitions.

I think that both cases could be addressed by having an external source of truth or macro-driven-file populating the clients.yml file with all the desired custom options.

[KusakabeShi]

Emm, yes. In the most case, authorized_addresses_list is a client specfic config.
But I think custom_options can be both client level and general level? depends on how we want to add options to clients.

[pierky]

Hello @KusakabeShi,

I've moved forward with the current solution and merged the dev branch into master, which will eventually generate v1.15.0 once the CI/CD pipeline will complete its job.

I see your point about making general.yml kind of a template to set clients options, but at the same time I prefer to keep the two entities separate, with different "logical blocks and scopes". What you mentioned above can be easily achieved using an external utility that manipulates YML files or generates them from a source of truth. Hope you don't mind too much about this 😉.

Many many thanks for the work you've done here and all the feedback you provided, they greatly helped to this new release.