staticd: in route config, reject keywords as ifname - #20311
Conversation
|
I think we need to add this to the staticd documentation. |
| "segments", | ||
| "nexthop-vrf", | ||
| NULL /*End sentinel*/ | ||
| }; |
There was a problem hiding this comment.
This seems a bit fragile given these are all coming from inside DEFPY's defined elsewhere in this file. I'm curious if @eqvinox can take a look and see if there's some way we can leverage the cli parsing logic to disallow these automatically from the CLI command definition.
If I understand correctly this is coming from defs that look like:
...
<INTERFACE|Null0>
[{tag (1-4294967295)
|(1-255)$distance
|vrf NAME
...
I'm wondering if the CLI parser can automatically handle this.
BTW, is vrf foobar actually accepted as well? From my read here it's allowing tag NUM b/c tag is consumed by INTERFACE and then the number is consumed by (1-255)$distance, but there's no generic WORD in that follow on selection so I would expect vrf foobar to fail to parse. If that logic holds I think the only keywords that will trigger this bug are ones followed by a number from 1 to 255, as it's the (1-255)$distance atom that's causing us all the pain. If distance had had a keyword like distance i don't think we'd have this issue. :)
There was a problem hiding this comment.
yes, the 'vrf XXX' tokens work - the matching is willing to use the more-specific match on the keyword 'vrf' followed by another word. but the lower-level match that would accept 'vrf' as an interface name will be rejected.
as you say, it's brittle to have to encode the ambiguous keywords, and it would be better to have an explicit "ifname YYY" instead of the positional matching. let's all try to squash examples like this as we review cli change proposals going forward...?
906852f to
a5bd6b7
Compare
|
rebased to newer master |
|
yes, sure, I've pushed a small doc update too.
|
4026428 to
8852762
Compare
|
rebased to newer master |
8852762 to
cdd24d0
Compare
|
rebased to newer master |
cdd24d0 to
f9b5e44
Compare
|
rebased to newer master |
| NULL /*End sentinel*/ | ||
| }; | ||
|
|
||
| for (cp = invalid_names; cp != NULL && *cp != NULL; cp++) |
There was a problem hiding this comment.
This is a case sensitive strmatch, but we use case insenstive on line 142. Do we care?
There was a problem hiding this comment.
It looks to me like the token-matching is case-sensitive, so "TAG" isn't the same as "tag" in the problem config commands.
Reject cli keywords from the various 'ip route' configs if the vty code interprets them as interface names. Signed-off-by: Mark Stapp <mjs@cisco.com>
Add a note to the docs that static route config keywords are not valid ifnames. Signed-off-by: Mark Stapp <mjs@cisco.com>
f9b5e44 to
9569593
Compare
|
rebased and added the list of keywords to the user doc |
4 participants
Reject cli keywords from the various 'ip route' configs if the vty code interprets them as interface names; some examples are "tag", "label", "color". Here's an example: