Page MenuHomeVyOS Platform

bgp large-community-list regex validation incomplete
Open, HighPublicBUG

Description

FRR supports using regex patterns to specify large communities. At the moment this is implemented in src/validators/bgp-large-community-list; However, the code has several issues:

  • It assumes GLOB matching (regex "12345:*:23"), even though FRR uses regex substring match, i.e., this setting would actually match 12345:23.* (and technically 12345::23.*; but that won't happen). This was introduced in https://github.com/vyos/vyos-1x/pull/1025
  • The code only allows for '*' in the second field, i.e., to match 12345:.*:.*, one has to set 12345:*:.* (thereby matching 12345:.* effectively)
  • The code strictly enforces three seperating ":", which are not strictly needed for a regex (even though that leap _could_ be made if documented)

In general, the current matching code severely restricts the capabilities of this feature in FRR.

The problem with validation is that _technically_ one would want to make sure that the supplied regex matches something resembling a large community; However, in absence of that, technically anything looking like a bgp-regex should be fine (see: https://docs.frrouting.org/en/latest/bgp.html?highlight=large-community#bgp-regular-expressions ; Still, _technically_ this seems to be more limited than POSIX 1003.2)

I took a quick shot at how i'd make this look assuming pure POSIX 1003.2; If someone finds a nice lib in python that implements proper BGP regex, that would be a drop-in replacement for re.compile, i guess.

https://github.com/vyos/vyos-1x/compare/current...ichdasich:vyos-1x:bgp_large_community_validator_new

Details

Difficulty level
Normal (likely a few hours)
Version
1.4
Why the issue appeared?
Will be filled on close
Is it a breaking change?
Behavior change
Issue type
Bug (incorrect behavior)

Event Timeline

Just put this on a live system, and it behaves as intended (so far). Special meaning of _ would certainly have to be added to the check, i guess, but that needs further delving into bgp-regex syntax.

dmbaturin added a project: VyOS 1.5 Circinus.

Honestly? Rather not. Even though i have been testing this extensively, there needs to be some discussion re: old-config-impact (+some testing); Similarly, _ideally_ this would not just go for standard python regex, but instead try to figure out what frr _acutally_ uses. So, while a poc, i think this needs a bit more work.