-
Notifications
You must be signed in to change notification settings - Fork 1.4k
babeld: fix RFC violations in babel message parser #20339
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
This commit fix various RFC violations found in `message.c`. 1. In Update message, this commit add a check to ignore the whole enclosing TLV if the mandatory bit of a sub-TLV is set. This commit also add a check to ensure that if AE is 3, Omitted must be zero. 2. In IHU message, this commit add similar mandatory bit check for sub-TLVs. 3. Add checks for `interval != 0` in various locations as RFC states that interval must not be zero. 4. Remove `Reserved == 0` checks from various locations, as RFC states that they must be ignored on recepton. 5. Add checks for `router_id` that ensure they must not be all zeros or all ones. 6. Add sub-TLV handling logic to various message types. Signed-off-by: spademomo <[email protected]>
riw777
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good
|
need to fix lint failures ... |
|
Thank you for you review! I will fix the lint issue soon. |
| nh = neigh->address; | ||
| } | ||
|
|
||
| // Add check that if AE (message[2]) is 3 (link-local IPv6), then Omitted (message[5]) must be 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we drop this (most likely for AI-assisted coding) comment?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And is this related to rfc8966 previous (mandatory bit) handling?
| flog_err(EC_BABEL_PACKET, | ||
| "Received router id with all-ones value."); | ||
| goto fail; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The have_router_id flag is set to 1 before validation checks at lines 684-693. When validation fails (all-zero or all-ones router_id), the code exits via goto fail, but have_router_id remains 1 with an invalid router_id value. This causes subsequent MESSAGE_UPDATE messages in the same packet to incorrectly fail validation because they see have_router_id = 1 but router_id contains an invalid value, even though the original MESSAGE_ROUTER_ID TLV was rejected.
This commit fix various RFC violations found in
message.c.interval != 0in various locations as RFC states that interval must not be zero.Reserved == 0checks from various locations, as RFC states that they must be ignored on recepton.router_idthat ensure they must not be all zeros or all ones.