From: Maria Matejka <mq@ucw.cz>
Date: Wed, 20 Aug 2025 13:34:31 +0000 (+0200)
Subject: BGP: Fix TCP-AO single key rejection
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=e6291e5b92a17fdd6a010f2c1423b70fd4149ae4;p=thirdparty%2Fbird.git

BGP: Fix TCP-AO single key rejection

When one key fails but others are working OK, do not shut down the BGP,
just disable that one key. We intended to do it this way but it somehow
slipped through.

Also added key cleanup in cases where the key addition fails for just
some sockets but not for all.
---

diff --git a/proto/bgp/bgp.c b/proto/bgp/bgp.c
index 47b48bc8c..862756f2a 100644
--- a/proto/bgp/bgp.c
+++ b/proto/bgp/bgp.c
@@ -163,7 +163,8 @@ bgp_open(struct bgp_proto *p)
    * interface notifier. By default, listen to nothing.
    *
    * Also dynamically spawned protocol do not need a listening socket,
-   * they already have their parent's one. */
+   * they already have their parent's one and the requests are actually
+   * ignored when looking for the accepting protocol. */
   if (p->cf->ipatt || p->cf->c.parent)
     return 0;
 
@@ -351,6 +352,11 @@ bgp_sk_add_ao_key(struct bgp_proto *p, sock *sk, struct bgp_ao_key *key, const c
   return rv;
 }
 
+static int
+bgp_sk_delete_ao_key(struct bgp_proto *p, sock *sk, struct bgp_ao_key *key,
+		     struct bgp_ao_key *backup, int current_key_id, int rnext_key_id,
+		     const char *kind);
+
 static int
 bgp_enable_ao_key(struct bgp_proto *p, struct bgp_ao_key *key)
 {
@@ -359,27 +365,45 @@ bgp_enable_ao_key(struct bgp_proto *p, struct bgp_ao_key *key)
   BGP_TRACE(D_EVENTS, "Adding TCP-AO key %d/%d", key->key.send_id, key->key.recv_id);
 
   /* Handle listening sockets */
-  struct bgp_listen_request *blr; node *nxt;
+  struct bgp_listen_request *blr, *failed = NULL; node *nxt;
   WALK_LIST2(blr, nxt, p->listen, pn)
     if (bgp_sk_add_ao_key(p, blr->sock->sk, key, "listening") < 0)
     {
-      key->failed = 1;
-      return -1;
+      failed = blr;
+      goto failA;
     }
 
-  key->active = 1;
-
   /* Handle incoming socket */
   if (p->incoming_conn.sk)
     if (bgp_sk_add_ao_key(p, p->incoming_conn.sk, key, "session (in)") < 0)
-      return -1;
+      goto failA;
 
   /* Handle outgoing socket */
   if (p->outgoing_conn.sk)
     if (bgp_sk_add_ao_key(p, p->outgoing_conn.sk, key, "session (out)") < 0)
-      return -1;
+      goto failB;
 
+  key->active = 1;
   return 0;
+
+failB:
+  /* Cleanup incoming socket */
+  if (p->incoming_conn.sk)
+    bgp_sk_delete_ao_key(p, p->incoming_conn.sk, key, NULL, -1, -1, "session (in)");
+
+failA:
+  /* Cleanup listening sockets */
+  WALK_LIST2(blr, nxt, p->listen, pn)
+  {
+    if (blr == failed)
+      break;
+
+    bgp_sk_delete_ao_key(p, blr->sock->sk, key, NULL, -1, -1, "listening");
+  }
+
+  /* Mark as failed */
+  key->failed = 1;
+  return -1;
 }
 
 struct bgp_active_keys {
@@ -432,25 +456,28 @@ bgp_disable_ao_key(struct bgp_proto *p, struct bgp_ao_key *key, struct bgp_activ
 
   BGP_TRACE(D_EVENTS, "Deleting TCP-AO key %d/%d", key->key.send_id, key->key.recv_id);
 
+  /* Try to disable everywhere even if first fails */
+  int rv = 0;
+
   /* Handle listening socket */
   struct bgp_listen_request *blr; node *nxt;
   WALK_LIST2(blr, nxt, p->listen, pn)
     if (bgp_sk_delete_ao_key(p, blr->sock->sk, key, NULL, -1, -1, "listening") < 0)
-      return -1;
+      rv = -1;
 
   key->active = 0;
 
   /* Handle incoming socket */
   if (p->incoming_conn.sk && info)
     if (bgp_sk_delete_ao_key(p, p->incoming_conn.sk, key, info->backup, info->in_current, info->in_rnext, "session (in)") < 0)
-      return -1;
+      rv = -1;
 
   /* Handle outgoing socket */
   if (p->outgoing_conn.sk && info)
     if (bgp_sk_delete_ao_key(p, p->outgoing_conn.sk, key, info->backup, info->out_current, info->out_rnext, "session (out)") < 0)
-      return -1;
+      rv = -1;
 
-  return 0;
+  return rv;
 }
 
 static int
@@ -558,8 +585,7 @@ bgp_enable_ao_keys(struct bgp_proto *p)
   ASSERT(!p->incoming_conn.sk && !p->outgoing_conn.sk);
 
   WALK_LIST_(struct bgp_ao_key, key, p->ao.keys)
-    if (bgp_enable_ao_key(p, key) < 0)
-      goto fail;
+    bgp_enable_ao_key(p, key);
 
   p->ao.best_key = bgp_select_best_ao_key(p);
 
@@ -679,7 +705,7 @@ bgp_reconfigure_ao_keys(struct bgp_proto *p, const struct bgp_config *cf)
     WALK_LIST_DELSAFE(key, key2, old_keys)
       bgp_remove_ao_key(p, key, &info);
 
-    /* If some key removals failed */
+    /* If some key removals failed, restart */
     if (!EMPTY_LIST(old_keys))
       return 0;
   }
@@ -687,8 +713,7 @@ bgp_reconfigure_ao_keys(struct bgp_proto *p, const struct bgp_config *cf)
   /* Enable new keys */
   WALK_LIST_(struct bgp_ao_key, key, p->ao.keys)
     if (!key->active && !key->failed)
-      if (bgp_enable_ao_key(p, key) < 0)
-	return 0;
+      bgp_enable_ao_key(p, key);
 
   /* Update RNext key */
   if (bgp_update_rnext_ao_key(p) < 0)