Policy route wildcard validation

Development
1

I filed an issue a little while ago that the policy route wildcards are not properly matching pod interface. I’ve worked out a solution, but I want to clarify a couple of things.

  1. I’ve made it so that any wildcard is accepted even if no current interfaces matches. This is because during boot, pod interfaces are created after applying the policy routes so some form of deferred binding is needed and it didn’t seem correct to have diverging behaviors for other interfaces. Is this acceptable?
  2. The current regex validation for interfaces with wildcards confuses me: “\<some prefix\>([0-9]?)(*?)(.+)?”. I’m not sure this really matches the patterns of interface names but it is also pretty restrictive on the wildcard front. Should this be updated to something more forgiving or is this the desired validation?
2

There’s 2 issues there; checking if the interface exists and strictly enforcing a naming schema.

I’ve mentioned this dozens of times at this point and it seems to fall on deaf ears, but I’ll try one last time.

As it relates to nftables config, there is generally no reason to have strict validation of the interface name. Outside of things like netdev hooks, it doesn’t care about a naming schema or whether an interface exists. As long as no illegal characters are inserted, it’ll insert a rule. If I remove the constraints I can just do this:

VyOS Config:

set policy route TEST interface 'idontexist'
set policy route TEST rule 10 set table '10'
set policy route TEST rule 10 source address '1.2.3.4'

Nftables Config:

vyos@vyos# sudo nft list table vyos_mangle
table ip vyos_mangle {
        chain VYOS_PBR_PREROUTING {
                type filter hook prerouting priority mangle; policy accept;
                iifname "idontexist" counter packets 0 bytes 0 jump VYOS_PBR_UD_TEST
        }

        chain VYOS_PBR_POSTROUTING {
                type filter hook postrouting priority mangle; policy accept;
        }

        chain VYOS_PBR_UD_TEST {
                ip saddr 1.2.3.4 counter packets 0 bytes 0 meta mark set 0x7ffffff5 return comment "ipv4-route-TEST-10"
        }
}

IP Rule:

vyos@vyos# sudo ip rule show
0:      from all lookup local
10:     from all fwmark 0x7ffffff5 lookup 10
32766:  from all lookup main
32767:  from all lookup default

Nothing in that requires that an interface follow a naming scheme, or that it exists on the system. Having a restrictive constraint here buys nothing, and just causes issues like this and others.

Having a constraint that ensures only valid characters is better and simpler. Something like:

must start with a letter 
can have lower and upper case letters
can have numbers 
can have '-, _, or .'
can have a '*' but only as the last character 
must not exceed 15 characters

This single regex would replace the existing constraints used:

Current constraints:

<!-- include start from constraint/interface-name-with-wildcard.xml.i -->
<regex>(bond|br|dum|en|ersp|eth|gnv|ifb|ipoe|lan|l2tp|l2tpeth|macsec|peth|ppp|pppoe|pptp|sstp|sstpc|tun|veth|vpptap|vpptun|vti|vtun|vxlan|wg|wlan|wwan)([0-9]?)(\*?)(.+)?|lo</regex>
<regex>(pod-[-_a-zA-Z0-9]{1,11})</regex>
<validator name="file-path --lookup-path /sys/class/net --directory"/>
<!-- include end -->
3

Please forgive me, but I’ve not been a member of the community very long. I’m not trying to ignore advice (current or former), I’m trying fix an issue with minimal impact.

As I noted in my initial message, I don’t think the current constraints make a lot of sense. If dropping all of the prefix and existence validation will make it through code review I’m happy to do it.

1 Like
4

No, you’ve been a good community member and have contributed a few times; you’re good!

That wasn’t a commentary on your post/question/suggestion. It was a comment on the overuse of aggressive constraints for underlying services that don’t require it (nftables, frr, etc…). There’s been a large number of tasks created due to it, but the practice remains.

5

I appreciate it.

Any advice on the way forward. Should I remove prefix validation and existence checks?

6

If it were up to me, I’d have a single regex constraint like this:

^(?=.{1,15}$)[A-Za-z][A-Za-z0-9_.-]*\*?$
7

This topic was automatically closed 60 days after the last reply. New replies are no longer allowed.