Skip to content

nhrpd: gate source binding cache on S-bit and holding time per RFC2332 - #22951

Open
pawademao wants to merge 1 commit into
FRRouting:masterfrom
pawademao:fix/nhrp-11-rfcbin-resolution-request-source-binding-cache
Open

nhrpd: gate source binding cache on S-bit and holding time per RFC2332#22951
pawademao wants to merge 1 commit into
FRRouting:masterfrom
pawademao:fix/nhrp-11-rfcbin-resolution-request-source-binding-cache

Conversation

@pawademao

Copy link
Copy Markdown

Summary

nhrp_handle_resolution_req() unconditionally caches the source binding
from every Resolution Request CIE, ignoring two conditions mandated by
RFC 2332 Section 6.2.1:

  1. The S-bit (NHRP_FLAG_RESOLUTION_SOURCE_STABLE) must be set - without
    it, the binding is temporary and MUST NOT be retained.

  2. The CIE Holding Time must be greater than zero. A zero holding time
    means the binding should not be stored. The code also silently replaced
    zero with the interface default holdtime.

Impact

Transit/responder NHS nodes accumulate stale or unauthorized source
bindings, leading to incorrect shortcut paths, forwarding to wrong
NBMA addresses, or serving expired mappings.

Fix

Add an explicit gate before nhrp_cache_update_binding(): if the S-bit is
unset OR the original holding_time is zero, skip creating or updating the
cache entry for this CIE.

References

  • RFC 2332 Section 6.2.1
  • RFCAudit Bug 11 (RFCBin)

Signed-off-by: zhuxu 185172534zxxx@gmail.com

@greptile-apps

greptile-apps Bot commented Aug 6, 2026

Copy link
Copy Markdown

Greptile Summary

This PR makes NHRP Resolution Request source-binding caching conditional on both the source-stable flag and a nonzero advertised holding time.

  • Adds an RFC 2332 gate before source cache lookup and update.
  • Continues constructing a successful Resolution Reply when the source binding is not cacheable.
  • Prevents a zero holding time from being replaced with the interface default on this cache path.

Confidence Score: 5/5

The PR appears safe to merge, with the new guard limited to source-binding cache insertion and no identified regression in reply handling.

The flag and holding-time checks use the daemon’s established network-byte-order conventions, and the early continuation still reaches the common Resolution Reply construction after the CIE loop.

Important Files Changed

Filename Overview
nhrpd/nhrp_peer.c Adds a correctly scoped cache gate while preserving the existing Resolution Reply flow.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Receive Resolution Request CIE] --> B{S-bit set and holding time nonzero?}
    B -- Yes --> C[Create or update source binding cache]
    B -- No --> D[Skip source binding cache]
    C --> E[Construct Resolution Reply]
    D --> E
Loading

Reviews (1): Last reviewed commit: "nhrpd: gate source binding cache on S-bi..." | Re-trigger Greptile

Comment thread nhrpd/nhrp_peer.c
|| !cie->holding_time) {
debugf(NHRP_DEBUG_COMMON,
"shortcut res_rep: skipping source binding cache"
" (S-bit=%u, holding_time=%u)",

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this line is throwing up an error :-( --

===============================================
< WARNING: quoted string split across lines
< #535: FILE: /tmp/f1-801609/nhrp_peer.c:535:\n
Details at [https://ci1.netdef.org/browse/FRR-PULLREQ3-CHECKOUT-20491](https://ci1.netdef.org/browse/FRR-PULLREQ3-CHECKOUT-20491)

nhrp_handle_resolution_req() unconditionally caches the source
binding from every Resolution Request CIE, ignoring two conditions
mandated by RFC 2332 §6.2.1:

1. The S-bit (NHRP_FLAG_RESOLUTION_SOURCE_STABLE) must be set
   in the packet header.  Without this bit, the source binding
   is considered temporary and MUST NOT be retained.

2. The CIE Holding Time must be greater than zero.  A zero
   Holding Time means the binding should not be stored.

The code also silently replaced holding_time == 0 with the
interface default holdtime, converting "do not cache" into
"cache permanently."

Impact: transit/responder NHS nodes could accumulate stale or
unauthorized source bindings, leading to incorrect shortcut
paths, expired mappings, or forwarding to wrong NBMA addresses.

Fix: check both conditions before calling nhrp_cache_update_binding().
If the S-bit is unset or the original cie->holding_time is zero,
set CIE status to SUCCESS (the request itself is valid) but skip
creating or updating the cache entry.

Fixes: Bug 11 (RFCBin) — Resolution Request source binding cache
Signed-off-by: zhuxu <185172534zxxx@gmail.com>
@pawademao
pawademao force-pushed the fix/nhrp-11-rfcbin-resolution-request-source-binding-cache branch from 71aa033 to bb9bc17 Compare August 17, 2026 10:01
@github-actions github-actions Bot added the rebase PR needs rebase label Aug 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants