Babel: Keep separate auth PC counters for unicast and multicast

The babel protocol normally sends all its messages as multicast packets,
but the protocol specification allows most messages to be sent as either
unicast or multicast, and the two can be mixed freely. In particular, the
babeld implementation can be configured to unicast updates to all peers
instead of sending them as unicast.

Daniel discovered that this can cause problems with the packet counter
checks in the MAC extension due to packet reordering. This happens on WiFi
networks where clients have power save enabled (which is quite common in
infrastructure networks): in this case, the access point will buffer all
multicast traffic and only send it out along with its beacons, leading to a
maximum buffering in default Linux-based access point configuration of up
to 200 ms.

This means that a Babel sender that mixes unicast and multicast messages
can have the unicast messages overtake the multicast messages because of
this buffering; when authentication is enabled, this causes the receiver to
discard the multicast message when it does arrive because it now has a
packet counter value less than the unicast message that arrived before it.
Daniel observed that this happens frequently enough that Babel ceases to
work entirely when runner over a WiFi network.

The issue has been described in draft-ietf-babel-mac-relaxed, which is
currently pending RFC publication. That also describes two mitigation
mechanisms: Keeping separate PC counters for unicast and multicast, and
using a reorder window for PC values. This patch implements the former as
that is the simplest, and resolves the particular issue seen on WiFi.

Thanks to Daniel Gröber for the bugreport.

Minor changes from committer.

3 files changed, 25 insertions, 7 deletions

diff --git a/proto/babel/babel.c b/proto/babel/babel.c
index a9a08e31..6842d61b 100644
--- a/proto/babel/babel.c
+++ b/proto/babel/babel.c
@@ -1566,7 +1566,8 @@ babel_auth_check_pc(struct babel_iface *ifa, struct babel_msg_auth *msg)
     n->auth_index_len = msg->index_len;
     memcpy(n->auth_index, msg->index, msg->index_len);
 
-    n->auth_pc = msg->pc;
+    n->auth_pc_unicast = msg->pc;
+    n->auth_pc_multicast = msg->pc;
     n->auth_passed = 1;
 
     return 1;
@@ -1585,16 +1586,30 @@ babel_auth_check_pc(struct babel_iface *ifa, struct babel_msg_auth *msg)
     return 0;
   }
 
-  /* (6) Index matches; only accept if PC is greater than last */
-  if (n->auth_pc >= msg->pc)
+  /*
+   * (6) Index matches; only accept if PC is greater than last. We keep separate
+   * counters for unicast and multicast because multicast packets can be delayed
+   * significantly on wireless networks (enough to be received out of order).
+   * Separate counters are safe because the packet destination address is part
+   * of the MAC pseudo-header (so unicast packets can't be replayed as multicast
+   * and vice versa).
+   */
+  u32 auth_pc = msg->unicast ? n->auth_pc_unicast : n->auth_pc_multicast;
+  if (auth_pc >= msg->pc)
   {
     LOG_PKT_AUTH("Authentication failed for %I on %s - "
-		 "lower packet counter (rcv %u, old %u)",
-                 msg->sender, ifa->ifname, msg->pc, n->auth_pc);
+		 "lower %s packet counter (rcv %u, old %u)",
+                 msg->sender, ifa->ifname,
+		 msg->unicast ? "unicast" : "multicast",
+		 msg->pc, auth_pc);
     return 0;
   }
 
-  n->auth_pc = msg->pc;
+  if (msg->unicast)
+    n->auth_pc_unicast = msg->pc;
+  else
+    n->auth_pc_multicast = msg->pc;
+
   n->auth_passed = 1;
 
   return 1;
diff --git a/proto/babel/babel.h b/proto/babel/babel.h
index 6699127e..dcd303e1 100644
--- a/proto/babel/babel.h
+++ b/proto/babel/babel.h
@@ -229,7 +229,8 @@ struct babel_neighbor {
   u16 next_hello_seqno;
   uint last_hello_int;
 
-  u32 auth_pc;
+  u32 auth_pc_unicast;
+  u32 auth_pc_multicast;
   u8 auth_passed;
   u8 auth_index_len;
   u8 auth_index[BABEL_AUTH_INDEX_LEN];
@@ -407,6 +408,7 @@ struct babel_msg_auth {
   u8 challenge_seen;
   u8 challenge_len;
   u8 challenge[BABEL_AUTH_MAX_NONCE_LEN];
+  u8 unicast;
 };
 
 static inline int babel_sadr_enabled(struct babel_proto *p)
diff --git a/proto/babel/packets.c b/proto/babel/packets.c
index 28bf9f63..61c94cc5 100644
--- a/proto/babel/packets.c
+++ b/proto/babel/packets.c
@@ -1704,6 +1704,7 @@ babel_read_pc(struct babel_tlv *hdr, union babel_msg *m UNUSED,
   state->auth.pc_seen = 1;
   state->auth.index_len = index_len;
   state->auth.index = tlv->index;
+  state->auth.unicast = state->is_unicast;
   state->current_tlv_endpos += index_len;
 
   return PARSE_SUCCESS;