Skip to content

bgpd: fix md5 password unset on dynamic nbr#20740

Merged
ton31337 merged 1 commit intoFRRouting:masterfrom
chiragshah6:bgp_dev6
Feb 10, 2026
Merged

bgpd: fix md5 password unset on dynamic nbr#20740
ton31337 merged 1 commit intoFRRouting:masterfrom
chiragshah6:bgp_dev6

Conversation

@chiragshah6
Copy link
Member

When a password is applied on peer-group associated to dynamic neighbor listen range.

1) Per peer (/32)  MD5 entry is set on the listen socket for each group member
     /* Attempt to install password on socket. */
     if (!BGP_CONNECTION_SU_UNSPEC(member->connection) &&
         bgp_md5_set(member->connection) < 0)
         ret = BGP_ERR_TCPSIG_FAILED;
2) Per dynamic listen range prefix (/24) MD5 entry on the listen socket.

    for (ALL_LIST_ELEMENTS_RO(peer->group->listen_range[AFI_IP], ln, lr))
        bgp_md5_set_prefix(peer->bgp, lr, password);
    for (ALL_LIST_ELEMENTS_RO(peer->group->listen_range[AFI_IP6], ln, lr))
        bgp_md5_set_prefix(peer->bgp, lr, password);

When no neighbor password is applied to the peer‑group, the configuration correctly removes the prefix‑range (/24) MD5 entry, but the stale per‑peer (/32) entry remains. Because the kernel always prefers the more specific (/32) rule, the affected peer’s TCP SYN packets (sent without MD5) are silently dropped.

The fix is to ensure that the peer‑delete path also removes the dynamic neighbor’s per‑peer MD5 entry, allowing the cleanup to complete correctly.

Signed-off-by: Chirag Shah chirag@nvidia.com

When a password is applied on peer-group associated to
dynamic neighbor listen range.
1) Per peer (/32)  MD5 entry is set on the listen socket for
each group member
     /* Attempt to install password on socket. */
     if (!BGP_CONNECTION_SU_UNSPEC(member->connection) &&
         bgp_md5_set(member->connection) < 0)
         ret = BGP_ERR_TCPSIG_FAILED;
2) Per dynamic listen range prefix (/24) MD5 entry on the
listen socket.

    for (ALL_LIST_ELEMENTS_RO(peer->group->listen_range[AFI_IP], ln, lr))
        bgp_md5_set_prefix(peer->bgp, lr, password);
    for (ALL_LIST_ELEMENTS_RO(peer->group->listen_range[AFI_IP6], ln, lr))
        bgp_md5_set_prefix(peer->bgp, lr, password);

When no neighbor <pg> password is applied to the peer‑group,
the configuration correctly removes the prefix‑range (/24) MD5 entry,
but the stale per‑peer (/32) entry remains. Because the kernel always
prefers the more specific (/32) rule, the affected peer’s
TCP SYN packets (sent without MD5) are silently dropped.

The fix is to ensure that the peer‑delete path also removes
the dynamic neighbor’s per‑peer MD5 entry, allowing the cleanup to complete correctly.

Ticket: #4875599

Signed-off-by: Chirag Shah <chirag@nvidia.com>
@greptile-apps
Copy link

greptile-apps bot commented Feb 10, 2026

Greptile Overview

Greptile Summary

This change updates peer_delete() in bgpd/bgpd.c to always remove the per-peer (/32) TCP-MD5 socket option when a peer with PEER_FLAG_PASSWORD is deleted, as long as it’s not an accept-peer, the connection has a concrete address (not AF_UNSPEC), and the peer isn’t a peer-group configuration node.

This aligns the delete path with how dynamic neighbors derived from listen ranges are programmed (per-peer MD5 entries in addition to per-prefix listen-range entries), and prevents stale /32 MD5 rules from persisting after the peer-group password is removed—otherwise the kernel prefers the more-specific rule and drops non-MD5 SYNs.

Confidence Score: 5/5

  • This PR is safe to merge with minimal risk.
  • The change is narrowly scoped to the peer deletion cleanup condition and matches the intended behavior described in the PR (removing stale per-peer TCP-MD5 state). No additional control flow or data structure changes are introduced, and the call is still gated on non-accept peers, a concrete remote address, and non-group nodes.
  • No files require special attention.

Important Files Changed

Filename Overview
bgpd/bgpd.c Adjusts peer_delete() MD5 cleanup to always call bgp_md5_unset() for non-accept peers with a concrete remote address, including dynamic neighbors, preventing stale /32 TCP-MD5 rules.

Sequence Diagram

sequenceDiagram
  autonumber
  participant CLI as vtysh/CLI
  participant BGP as bgpd peer_delete()
  participant NET as bgp_network bgp_md5_unset()
  participant K as Kernel TCP-MD5

  CLI->>BGP: delete peer / dynamic member
  BGP->>BGP: peer_delete(peer)
  alt peer has password flag
    BGP->>BGP: free peer->password
    alt non-accept peer && concrete remote address && not a peer-group node
      BGP->>NET: bgp_md5_unset(peer->connection)
      NET->>K: setsockopt TCP_MD5SIG with NULL key
      K-->>NET: result
      NET-->>BGP: result
    else condition not met
      BGP-->>BGP: skip per-peer MD5 cleanup
    end
  end
  BGP-->>CLI: peer removed
Loading

@ton31337
Copy link
Member

@Mergifyio backport dev/10.6 stable/10.5 stable/10.4 stable/10.3 stable/10.2

@mergify
Copy link

mergify bot commented Feb 10, 2026

backport dev/10.6 stable/10.5 stable/10.4 stable/10.3 stable/10.2

✅ Backports have been created

Details

@ton31337 ton31337 merged commit 41b32cc into FRRouting:master Feb 10, 2026
25 checks passed
donaldsharp added a commit that referenced this pull request Feb 10, 2026
bgpd: fix md5 password unset on dynamic nbr (backport #20740)
donaldsharp added a commit that referenced this pull request Feb 10, 2026
bgpd: fix md5 password unset on dynamic nbr (backport #20740)
donaldsharp added a commit that referenced this pull request Feb 10, 2026
bgpd: fix md5 password unset on dynamic nbr (backport #20740)
donaldsharp added a commit that referenced this pull request Feb 10, 2026
bgpd: fix md5 password unset on dynamic nbr (backport #20740)
donaldsharp added a commit that referenced this pull request Feb 10, 2026
bgpd: fix md5 password unset on dynamic nbr (backport #20740)
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