From b174cc0abc0a9d7e84cc6fae46d9e19b714fbcfb Mon Sep 17 00:00:00 2001
From: "Ondrej Zajicek (work)" <santiago@crfreenet.org>
Date: Sun, 6 Jun 2021 15:22:59 +0200
Subject: [PATCH] Babel: Add MAC authentication support - update

Some cleanups and bugfixes to the previous patch, including:

 - Fix rate limiting in index mismatch check

 - Fix missing BABEL_AUTH_INDEX_LEN in auth_tx_overhead computation

 - Fix missing auth_tx_overhead recalculation during reconfiguration

 - Fix pseudoheader construction in babel_auth_sign() (sport vs fport)

 - Fix typecasts for ptrdiffs in log messages

 - Make auth log messages similar to corresponding RIP/OSPF ones

 - Change auth log messages for events that happen during regular
   operation to debug messages

 - Switch meaning of babel_auth_check*() functions for consistency
   with corresponding RIP/OSPF ones

 - Remove requirement for min/max key length, only those required by
   given MAC code are enforced
---
 doc/bird.sgml         |   9 +-
 proto/babel/babel.c   | 104 ++++++++++-------
 proto/babel/babel.h   |   7 +-
 proto/babel/config.Y  |  22 ++--
 proto/babel/packets.c | 261 ++++++++++++++++++++----------------------
 5 files changed, 205 insertions(+), 198 deletions(-)

diff --git a/doc/bird.sgml b/doc/bird.sgml
index 8235740e..15085874 100644
--- a/doc/bird.sgml
+++ b/doc/bird.sgml
@@ -1827,8 +1827,8 @@ protocol babel [<name>] {
 			accept to "&lt;date&gt;";
 			from "&lt;date&gt;";
 			to "&lt;date&gt;";
-			algorithm ( hmac sha1 | hmac sha256 | hmac sha384 | hmac
-	sha512 | blake2s | blake2b );
+			algorithm ( hmac sha1 | hmac sha256 | hmac sha384 |
+	hmac sha512 | blake2s128 | blake2s256 | blake2b256 | blake2b512 );
 		};
 	};
 }
@@ -1932,13 +1932,12 @@ protocol babel [<name>] {
       authentication is selected, a key must be specified with the
       <cf/password/ configuration option. Default: none.
 
-      <tag><label id="babel-password">password "<m/text/"</tag> Specifies a
-      password used for authentication. See the <ref id="proto-pass"
+      <tag><label id="babel-password">password "<m/text/"</tag>
+      Specifies a password used for authentication. See the <ref id="proto-pass"
       name="password"> common option for a detailed description. The Babel
       protocol will only accept HMAC-based algorithms or one of the Blake
       algorithms, and the length of the supplied password string must match the
       key size used by the selected algorithm.
-
 </descrip>
 
 <sect1>Attributes
diff --git a/proto/babel/babel.c b/proto/babel/babel.c
index 82ba7da1..c322acb6 100644
--- a/proto/babel/babel.c
+++ b/proto/babel/babel.c
@@ -1428,94 +1428,113 @@ babel_auth_init_neighbor(struct babel_neighbor *n)
 }
 
 static void
-babel_auth_send_challenge(struct babel_iface *ifa, struct babel_neighbor *n)
+babel_auth_send_challenge_request(struct babel_iface *ifa, struct babel_neighbor *n)
 {
   struct babel_proto *p = ifa->proto;
   union babel_msg msg = {};
 
-  TRACE(D_PACKETS, "Sending AUTH challenge to %I on %s",
+  TRACE(D_PACKETS, "Sending challenge request to %I on %s",
 	n->addr, ifa->ifname);
 
   random_bytes(n->auth_nonce, BABEL_AUTH_NONCE_LEN);
   n->auth_nonce_expiry = current_time() + BABEL_AUTH_CHALLENGE_TIMEOUT;
   n->auth_next_challenge = current_time() + BABEL_AUTH_CHALLENGE_INTERVAL;
 
-  msg.type = BABEL_TLV_CHALLENGE_REQ;
+  msg.type = BABEL_TLV_CHALLENGE_REQUEST;
   msg.challenge.nonce_len = BABEL_AUTH_NONCE_LEN;
   msg.challenge.nonce = n->auth_nonce;
 
   babel_send_unicast(&msg, ifa, n->addr);
 }
 
+static void
+babel_auth_send_challenge_reply(struct babel_iface *ifa, struct babel_neighbor *n, struct babel_msg_auth *rcv)
+{
+  struct babel_proto *p = ifa->proto;
+  union babel_msg msg = {};
+
+  TRACE(D_PACKETS, "Sending challenge reply to %I on %s",
+	n->addr, ifa->ifname);
+
+  n->auth_next_challenge_reply = current_time() + BABEL_AUTH_CHALLENGE_INTERVAL;
+
+  msg.type = BABEL_TLV_CHALLENGE_REPLY;
+  msg.challenge.nonce_len = rcv->challenge_len;
+  msg.challenge.nonce = rcv->challenge;
+
+  babel_send_unicast(&msg, ifa, n->addr);
+}
+
 int
 babel_auth_check_pc(struct babel_iface *ifa, struct babel_msg_auth *msg)
 {
   struct babel_proto *p = ifa->proto;
   struct babel_neighbor *n;
 
-  TRACE(D_PACKETS, "Handling MAC check from %I on %s",
-        msg->sender,  ifa->ifname);
-
-  /* We create the neighbour entry at this point because it makes it easier to
+  /*
+   * We create the neighbour entry at this point because it makes it easier to
    * rate limit challenge replies; this is explicitly allowed by the spec (see
-   *  Section 4.3).
+   * Section 4.3).
    */
   n = babel_get_neighbor(ifa, msg->sender);
 
-  if (msg->challenge_seen && n->auth_next_challenge_reply <= current_time())
-  {
-    union babel_msg resp = {};
-    TRACE(D_PACKETS, "Sending MAC challenge response to %I", msg->sender);
-    resp.type = BABEL_TLV_CHALLENGE_REPLY;
-    resp.challenge.nonce_len = msg->challenge_len;
-    resp.challenge.nonce = msg->challenge;
-    n->auth_next_challenge_reply = current_time() + BABEL_AUTH_CHALLENGE_INTERVAL;
-    babel_send_unicast(&resp, ifa, msg->sender);
-  }
+  /* (3b) Handle challenge request */
+  if (msg->challenge_seen && (n->auth_next_challenge_reply <= current_time()))
+    babel_auth_send_challenge_reply(ifa, n, msg);
 
-  if (msg->index_len > BABEL_AUTH_INDEX_LEN || !msg->pc_seen)
+  /* (4a) If PC TLV is missing, drop the packet */
+  if (!msg->pc_seen)
   {
-    LOG_PKT_AUTH("Invalid index or no PC from %I on %s",
+    LOG_PKT_AUTH("Authentication failed for %I on %s - missing or invalid PC",
                  msg->sender, ifa->ifname);
-    return 1;
+    return 0;
   }
 
-  /* On successful challenge, update PC and index to current values */
+  /* (4b) On successful challenge, update PC and index to current values */
   if (msg->challenge_reply_seen &&
-      n->auth_nonce_expiry &&
-      n->auth_nonce_expiry >= current_time() &&
+      (n->auth_nonce_expiry > current_time()) &&
       !memcmp(msg->challenge_reply, n->auth_nonce, BABEL_AUTH_NONCE_LEN))
   {
     n->auth_index_len = msg->index_len;
     memcpy(n->auth_index, msg->index, msg->index_len);
-    n->auth_pc = msg->pc;
-  }
 
-  /* If index differs, send challenge */
-  if ((n->auth_index_len != msg->index_len ||
-      memcmp(n->auth_index, msg->index, msg->index_len)) &&
-      n->auth_next_challenge <= current_time())
-  {
-    LOG_PKT_AUTH("Index mismatch from %I on %s; sending challenge",
-                 msg->sender, ifa->ifname);
-    babel_auth_send_challenge(ifa, n);
+    n->auth_pc = msg->pc;
+    n->auth_expiry = current_time() + BABEL_AUTH_NEIGHBOR_TIMEOUT;
+    n->auth_passed = 1;
+
     return 1;
   }
 
-  /* Index matches; only accept if PC is greater than last */
+  /* (5) If index differs, send challenge and drop the packet */
+  if ((n->auth_index_len != msg->index_len) ||
+      memcmp(n->auth_index, msg->index, msg->index_len))
+  {
+    TRACE(D_PACKETS, "Index mismatch for packet from %I via %s",
+	  msg->sender, ifa->ifname);
+
+    if (n->auth_next_challenge <= current_time())
+      babel_auth_send_challenge_request(ifa, n);
+
+    return 0;
+  }
+
+  /* (6) Index matches; only accept if PC is greater than last */
   if (n->auth_pc >= msg->pc)
   {
-    LOG_PKT_AUTH("Packet counter too low from %I on %s",
-                 msg->sender, ifa->ifname);
-    return 1;
+    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);
+    return 0;
   }
 
   n->auth_pc = msg->pc;
   n->auth_expiry = current_time() + BABEL_AUTH_NEIGHBOR_TIMEOUT;
   n->auth_passed = 1;
-  return 0;
+
+  return 1;
 }
 
+
 /*
  *	Babel interfaces
  */
@@ -1854,7 +1873,9 @@ babel_reconfigure_iface(struct babel_proto *p, struct babel_iface *ifa, struct b
   ifa->next_hop_ip4 = ipa_nonzero(new->next_hop_ip4) ? new->next_hop_ip4 : addr4;
   ifa->next_hop_ip6 = ipa_nonzero(new->next_hop_ip6) ? new->next_hop_ip6 : ifa->addr;
 
-  if (new->auth_type != BABEL_AUTH_NONE && old->auth_type != new->auth_type)
+  babel_iface_update_buffers(ifa);
+
+  if ((new->auth_type != BABEL_AUTH_NONE) && (new->auth_type != old->auth_type))
     babel_auth_reset_index(ifa);
 
   if (ipa_zero(ifa->next_hop_ip4) && p->ip4_channel)
@@ -1866,9 +1887,6 @@ babel_reconfigure_iface(struct babel_proto *p, struct babel_iface *ifa, struct b
   if (ifa->next_regular > (current_time() + new->update_interval))
     ifa->next_regular = current_time() + (random() % new->update_interval);
 
-  if ((new->tx_length != old->tx_length) || (new->rx_buffer != old->rx_buffer))
-    babel_iface_update_buffers(ifa);
-
   if (new->check_link != old->check_link)
     babel_iface_update_state(ifa);
 
diff --git a/proto/babel/babel.h b/proto/babel/babel.h
index ef5b4a5d..1e946f19 100644
--- a/proto/babel/babel.h
+++ b/proto/babel/babel.h
@@ -61,8 +61,9 @@
 #define BABEL_OVERHEAD		(IP6_HEADER_LENGTH+UDP_HEADER_LENGTH)
 #define BABEL_MIN_MTU		(512 + BABEL_OVERHEAD)
 
-#define BABEL_AUTH_NONE		0
+#define BABEL_AUTH_NONE			0
 #define BABEL_AUTH_MAC			1
+
 #define BABEL_AUTH_NONCE_LEN		10	/* we send 80 bit nonces */
 #define BABEL_AUTH_MAX_NONCE_LEN	192	/* max allowed by spec */
 #define BABEL_AUTH_INDEX_LEN		32	/* max size in spec */
@@ -82,9 +83,9 @@ enum babel_tlv_type {
   BABEL_TLV_UPDATE		= 8,
   BABEL_TLV_ROUTE_REQUEST	= 9,
   BABEL_TLV_SEQNO_REQUEST	= 10,
-  BABEL_TLV_MAC		= 16,
+  BABEL_TLV_MAC			= 16,
   BABEL_TLV_PC			= 17,
-  BABEL_TLV_CHALLENGE_REQ	= 18,
+  BABEL_TLV_CHALLENGE_REQUEST	= 18,
   BABEL_TLV_CHALLENGE_REPLY	= 19,
   BABEL_TLV_MAX
 };
diff --git a/proto/babel/config.Y b/proto/babel/config.Y
index 5e0710b5..05210fa4 100644
--- a/proto/babel/config.Y
+++ b/proto/babel/config.Y
@@ -103,26 +103,24 @@ babel_iface_finish:
   {
     struct password_item *pass;
     uint len = 0, i = 0;
+
     WALK_LIST(pass, *BABEL_IFACE->passwords)
     {
       /* Set default crypto algorithm (HMAC-SHA256) */
       if (!pass->alg)
 	pass->alg = ALG_HMAC_SHA256;
 
-      if (pass->alg & ALG_HMAC) {
-        if (pass->length < mac_type_length(pass->alg) ||
-            pass->length > mac_type_block_size(pass->alg))
-          cf_error("key length %d is not between output size %d and block size %d for algorithm %s",
-                   pass->length, mac_type_length(pass->alg),
-                   mac_type_block_size(pass->alg), mac_type_name(pass->alg));
-      } else if (!(pass->alg == ALG_BLAKE2S_128 || pass->alg == ALG_BLAKE2S_256 ||
-                   pass->alg == ALG_BLAKE2B_256 || pass->alg == ALG_BLAKE2B_512)) {
-	cf_error("Only HMAC and Blake algorithms are supported");
-      }
+      if (!((pass->alg & ALG_HMAC) ||
+	    (pass->alg == ALG_BLAKE2S_128) ||
+	    (pass->alg == ALG_BLAKE2S_256) ||
+	    (pass->alg == ALG_BLAKE2B_256) ||
+	    (pass->alg == ALG_BLAKE2B_512)))
+	cf_error("Only HMAC and Blake2 algorithms are supported");
 
       len += mac_type_length(pass->alg);
       i++;
     }
+
     BABEL_IFACE->mac_num_keys = i;
     BABEL_IFACE->mac_total_len = len;
   }
@@ -146,9 +144,9 @@ babel_iface_item:
  | NEXT HOP IPV4 ipa { BABEL_IFACE->next_hop_ip4 = $4; if (!ipa_is_ip4($4)) cf_error("Must be an IPv4 address"); }
  | NEXT HOP IPV6 ipa { BABEL_IFACE->next_hop_ip6 = $4; if (!ipa_is_ip6($4)) cf_error("Must be an IPv6 address"); }
  | AUTHENTICATION NONE { BABEL_IFACE->auth_type = BABEL_AUTH_NONE; }
- | AUTHENTICATION MAC { BABEL_IFACE->auth_type = BABEL_AUTH_MAC; }
+ | AUTHENTICATION MAC { BABEL_IFACE->auth_type = BABEL_AUTH_MAC; BABEL_IFACE->auth_permissive = 0; }
  | AUTHENTICATION MAC PERMISSIVE { BABEL_IFACE->auth_type = BABEL_AUTH_MAC; BABEL_IFACE->auth_permissive = 1; }
- | password_list	{ }
+ | password_list
  ;
 
 babel_iface_opts:
diff --git a/proto/babel/packets.c b/proto/babel/packets.c
index 703d4026..f13410e2 100644
--- a/proto/babel/packets.c
+++ b/proto/babel/packets.c
@@ -13,6 +13,7 @@
 #include "babel.h"
 #include "lib/mac.h"
 
+
 struct babel_pkt_header {
   u8 magic;
   u8 version;
@@ -112,6 +113,12 @@ struct babel_subtlv_source_prefix {
   u8 addr[0];
 } PACKED;
 
+struct babel_tlv_mac {
+  u8 type;
+  u8 length;
+  u8 mac[0];
+} PACKED;
+
 struct babel_tlv_pc {
   u8 type;
   u8 length;
@@ -119,19 +126,13 @@ struct babel_tlv_pc {
   u8 index[0];
 } PACKED;
 
-struct babel_tlv_mac {
-  u8 type;
-  u8 length;
-  u8 mac[0];
-} PACKED;
-
 struct babel_tlv_challenge {
   u8 type;
   u8 length;
   u8 nonce[0];
 } PACKED;
 
-struct babel_mac_pseudohdr {
+struct babel_mac_pseudoheader {
   u8 src_addr[16];
   u16 src_port;
   u8 dst_addr[16];
@@ -234,7 +235,7 @@ struct babel_write_state {
     if ((loop_pos > end) || (loop_pos + tlv->length > end))             \
     {                                                                   \
       LOG_PKT("Bad TLV from %I via %s type %d pos %d - framing error",  \
-	      saddr, ifname, tlv->type, (byte *)tlv - (byte *)start);   \
+	      saddr, ifname, tlv->type, (int) ((byte *)tlv - (byte *)start)); \
       frame_err = 1;                                                    \
       break;                                                            \
     }
@@ -306,11 +307,13 @@ put_ip6_ll(void *p, ip6_addr addr)
   put_u32(p+4, _I3(addr));
 }
 
+
 /*
  *      Authentication-related functions
  */
+
 uint babel_auth_write_challenge(struct babel_tlv *hdr, union babel_msg *msg, struct babel_write_state *state, uint max_len);
-int babel_auth_add_tlvs(struct babel_iface *ifa, struct babel_tlv *tlv, int max_len);
+int babel_auth_add_tlvs(struct babel_iface *ifa, struct babel_tlv *tlv, uint max_len);
 int babel_auth_sign(struct babel_iface *ifa, ip_addr dest);
 int babel_auth_check(struct babel_iface *ifa,
                      ip_addr saddr, u16 sport,
@@ -395,15 +398,17 @@ static const struct babel_tlv_data tlv_data[BABEL_TLV_MAX] = {
     babel_write_seqno_request,
     babel_handle_seqno_request
   },
-  [BABEL_TLV_CHALLENGE_REQ] = {
-    sizeof(struct babel_tlv),
+  [BABEL_TLV_CHALLENGE_REQUEST] = {
+    sizeof(struct babel_tlv_challenge),
     NULL,
     babel_auth_write_challenge,
+    NULL
   },
   [BABEL_TLV_CHALLENGE_REPLY] = {
-    sizeof(struct babel_tlv),
+    sizeof(struct babel_tlv_challenge),
     NULL,
     babel_auth_write_challenge,
+    NULL
   },
 };
 
@@ -421,7 +426,7 @@ static const struct babel_tlv_data source_prefix_tlv_data = {
 
 static const struct babel_tlv_data *get_packet_subtlv_data(u8 type)
 {
-  switch(type)
+  switch (type)
   {
   case BABEL_SUBTLV_SOURCE_PREFIX:
     return &source_prefix_tlv_data;
@@ -1128,7 +1133,7 @@ babel_read_source_prefix(struct babel_tlv *hdr, union babel_msg *msg,
   if (tlv->plen == 0)
     return PARSE_ERROR;
 
-  switch(msg->type)
+  switch (msg->type)
   {
   case BABEL_TLV_UPDATE:
     /* Wildcard updates with source prefix MUST be silently ignored */
@@ -1278,6 +1283,7 @@ babel_write_tlv(struct babel_tlv *hdr,
   return tlv_data[msg->type].write_tlv(hdr, msg, state, max_len);
 }
 
+
 /*
  *	Packet RX/TX functions
  */
@@ -1343,7 +1349,7 @@ babel_write_queue(struct babel_iface *ifa, list *queue)
     sl_free(p->msg_slab, msg);
   }
 
-  pos += babel_auth_add_tlvs(ifa, (struct babel_tlv *) pos, end-pos);
+  pos += babel_auth_add_tlvs(ifa, (struct babel_tlv *) pos, end - pos);
 
   uint plen = pos - (byte *) pkt;
   put_u16(&pkt->length, plen - sizeof(struct babel_pkt_header));
@@ -1423,7 +1429,7 @@ babel_enqueue(union babel_msg *msg, struct babel_iface *ifa)
 
 /**
  * babel_process_packet - process incoming data packet
- * @ifa: Interface packet was received on.
+ * @ifa: Interface packet was received on
  * @pkt: Pointer to the packet data
  * @len: Length of received packet
  * @saddr: Address of packet sender
@@ -1483,7 +1489,7 @@ babel_process_packet(struct babel_iface *ifa,
   TRACE(D_PACKETS, "Packet received from %I via %s",
         saddr, ifa->iface->name);
 
-  if (babel_auth_check(ifa, saddr, sport, daddr, dport, pkt, end, len-plen))
+  if (!babel_auth_check(ifa, saddr, sport, daddr, dport, pkt, end, len - plen))
     return;
 
   init_list(&msgs);
@@ -1506,7 +1512,7 @@ babel_process_packet(struct babel_iface *ifa,
     else /* PARSE_ERROR */
     {
       LOG_PKT("Bad TLV from %I via %s type %d pos %d - parse error",
-	      saddr, ifa->iface->name, tlv->type, (byte *)tlv - (byte *)pkt);
+	      saddr, ifa->iface->name, tlv->type, (int) ((byte *)tlv - (byte *)pkt));
       sl_free(p->msg_slab, msg);
       break;
     }
@@ -1638,15 +1644,21 @@ babel_read_pc(struct babel_tlv *hdr, union babel_msg *m UNUSED,
 {
   struct babel_tlv_pc *tlv = (void *) hdr;
 
-  if (!state->auth.pc_seen)
-  {
-    state->auth.pc_seen = 1;
-    state->auth.pc = get_u32(&tlv->pc);
-    state->auth.index_len = TLV_OPT_LENGTH(tlv);
-    state->auth.index = tlv->index;
-  }
+  /* RFC 8967 4.3 (3) - If multiple PCs are found, only the first one is used */
+  if (state->auth.pc_seen)
+    return PARSE_IGNORE;
 
-  return PARSE_IGNORE;
+  uint index_len = TLV_OPT_LENGTH(tlv);
+  if (index_len > BABEL_AUTH_INDEX_LEN)
+    return PARSE_IGNORE;
+
+  state->auth.pc = get_u32(&tlv->pc);
+  state->auth.pc_seen = 1;
+  state->auth.index_len = index_len;
+  state->auth.index = tlv->index;
+  state->current_tlv_endpos += index_len;
+
+  return PARSE_SUCCESS;
 }
 
 static const struct babel_tlv_data pc_tlv_data = {
@@ -1661,20 +1673,18 @@ babel_read_challenge_req(struct babel_tlv *hdr, union babel_msg *m UNUSED,
   struct babel_tlv_challenge *tlv = (void *) hdr;
 
   if (!state->is_unicast)
-  {
-    DBG("Ignoring non-unicast challenge request from %I\n", state->saddr);
-    return PARSE_IGNORE;
-  }
-
-  if (tlv->length > BABEL_AUTH_MAX_NONCE_LEN)
     return PARSE_IGNORE;
 
-  state->auth.challenge_len = tlv->length;
-  if (state->auth.challenge_len)
-    memcpy(state->auth.challenge, tlv->nonce, state->auth.challenge_len);
+  uint nonce_len = TLV_OPT_LENGTH(tlv);
+  if (nonce_len > BABEL_AUTH_MAX_NONCE_LEN)
+    return PARSE_IGNORE;
+
+  state->auth.challenge_len = nonce_len;
+  bmemcpy(state->auth.challenge, tlv->nonce, nonce_len);
   state->auth.challenge_seen = 1;
+  state->current_tlv_endpos += nonce_len;
 
-  return PARSE_IGNORE;
+  return PARSE_SUCCESS;
 }
 
 static const struct babel_tlv_data challenge_req_tlv_data = {
@@ -1688,13 +1698,18 @@ babel_read_challenge_reply(struct babel_tlv *hdr, union babel_msg *m UNUSED,
 {
   struct babel_tlv_challenge *tlv = (void *) hdr;
 
-  if (tlv->length != BABEL_AUTH_NONCE_LEN || state->auth.challenge_reply_seen)
+  if (state->auth.challenge_reply_seen)
     return PARSE_IGNORE;
 
-  state->auth.challenge_reply_seen = 1;
-  memcpy(state->auth.challenge_reply, tlv->nonce, BABEL_AUTH_NONCE_LEN);
+  uint nonce_len = TLV_OPT_LENGTH(tlv);
+  if (nonce_len != BABEL_AUTH_NONCE_LEN)
+    return PARSE_IGNORE;
 
-  return PARSE_IGNORE;
+  memcpy(state->auth.challenge_reply, tlv->nonce, BABEL_AUTH_NONCE_LEN);
+  state->auth.challenge_reply_seen = 1;
+  state->current_tlv_endpos += nonce_len;
+
+  return PARSE_SUCCESS;
 }
 
 static const struct babel_tlv_data challenge_reply_tlv_data = {
@@ -1705,11 +1720,11 @@ static const struct babel_tlv_data challenge_reply_tlv_data = {
 static const struct babel_tlv_data *
 get_auth_tlv_data(u8 type)
 {
-  switch(type)
+  switch (type)
   {
   case BABEL_TLV_PC:
     return &pc_tlv_data;
-  case BABEL_TLV_CHALLENGE_REQ:
+  case BABEL_TLV_CHALLENGE_REQUEST:
     return &challenge_req_tlv_data;
   case BABEL_TLV_CHALLENGE_REPLY:
     return &challenge_reply_tlv_data;
@@ -1720,7 +1735,7 @@ get_auth_tlv_data(u8 type)
 
 uint
 babel_auth_write_challenge(struct babel_tlv *hdr, union babel_msg *m,
-                           struct babel_write_state *state UNUSED,uint max_len)
+                           struct babel_write_state *state UNUSED, uint max_len)
 {
   struct babel_tlv_challenge *tlv = (void *) hdr;
   struct babel_msg_challenge *msg = &m->challenge;
@@ -1731,36 +1746,28 @@ babel_auth_write_challenge(struct babel_tlv *hdr, union babel_msg *m,
     return 0;
 
   TLV_HDR(tlv, msg->type, len);
-  memcpy(tlv->nonce, msg->nonce, msg->nonce_len);
+  bmemcpy(tlv->nonce, msg->nonce, msg->nonce_len);
 
   return len;
 }
 
-static int
-babel_mac_hash(struct password_item *pass,
-               struct babel_mac_pseudohdr *phdr,
-               byte *pkt, uint pkt_len,
-               byte *buf, uint *buf_len)
+static void
+babel_mac_fill(struct password_item *pass,
+	       struct babel_mac_pseudoheader *phdr,
+	       byte *pkt, uint pkt_len,
+	       byte *mac)
 {
   struct mac_context ctx;
 
-  if (mac_type_length(pass->alg) > *buf_len)
-    return 1;
-
   mac_init(&ctx, pass->alg, pass->password, pass->length);
   mac_update(&ctx, (byte *)phdr, sizeof(*phdr));
   mac_update(&ctx, (byte *)pkt, pkt_len);
-
-  *buf_len = mac_get_length(&ctx);
-  memcpy(buf, mac_final(&ctx), *buf_len);
-
+  memcpy(mac, mac_final(&ctx), mac_get_length(&ctx));
   mac_cleanup(&ctx);
-
-  return 0;
 }
 
 static void
-babel_mac_build_phdr(struct babel_mac_pseudohdr *phdr,
+babel_mac_build_phdr(struct babel_mac_pseudoheader *phdr,
                      ip_addr saddr, u16 sport,
                      ip_addr daddr, u16 dport)
 {
@@ -1778,61 +1785,56 @@ babel_auth_check_mac(struct babel_iface *ifa, byte *pkt,
                      ip_addr saddr, u16 sport,
                      ip_addr daddr, u16 dport)
 {
-  uint hash_len = (uint)(trailer - pkt);
   struct babel_proto *p = ifa->proto;
+  uint pkt_len = (uint)(trailer - pkt);
   byte *end = trailer + trailer_len;
   btime now_ = current_real_time();
-  struct babel_mac_pseudohdr phdr;
-  struct password_item *pass;
-  struct babel_tlv *tlv;
 
-  if (trailer_len < sizeof(*tlv))
+  if (trailer_len < sizeof(struct babel_tlv))
   {
-    LOG_PKT_AUTH("No MAC signature on packet from %I on %s",
+    LOG_PKT_AUTH("Authentication failed for %I on %s - no MAC signature",
                  saddr, ifa->ifname);
-    return 1;
+    return 0;
   }
 
+  struct babel_mac_pseudoheader phdr;
   babel_mac_build_phdr(&phdr, saddr, sport, daddr, dport);
 
+  struct password_item *pass;
   WALK_LIST(pass, *ifa->cf->passwords)
   {
-    byte mac_res[MAX_HASH_SIZE];
-    uint mac_len = MAX_HASH_SIZE;
-    u8 frame_err = 0;
+    byte mac[MAX_HASH_SIZE];
+    uint mac_len = mac_type_length(pass->alg);
+    uint frame_err = 0;
 
     if (pass->accfrom > now_ || pass->accto < now_)
       continue;
 
-    if (babel_mac_hash(pass, &phdr,
-                       pkt, hash_len,
-                       mac_res, &mac_len))
-      continue;
+    babel_mac_fill(pass, &phdr, pkt, pkt_len, mac);
 
-    WALK_TLVS((void *)trailer, end, tlv, frame_err, saddr, ifa->ifname)
+    struct babel_tlv *tlv0;
+    WALK_TLVS((void *)trailer, end, tlv0, frame_err, saddr, ifa->ifname)
     {
-      struct babel_tlv_mac *mac = (void *)tlv;
+      struct babel_tlv_mac *tlv = (void *)tlv0;
 
       if (tlv->type != BABEL_TLV_MAC)
 	continue;
 
-      if (tlv->length == mac_len && !memcmp(mac->mac, mac_res, mac_len))
-        return 0;
+      if ((TLV_OPT_LENGTH(tlv) == mac_len) && !memcmp(tlv->mac, mac, mac_len))
+        return 1;
 
       DBG("MAC mismatch key id %d pos %d len %d/%d\n",
-	  pass->id, (byte *)tlv - (byte *)pkt, mac_len, tlv->length);
+	  pass->id, (int) ((byte *)tlv - (byte *)pkt), mac_len, tlv->length);
     }
     WALK_TLVS_END;
 
-    if (frame_err) {
-      DBG("MAC trailer TLV framing error\n");
-      return 1;
-    }
+    if (frame_err)
+      return 0;
   }
 
-  LOG_PKT_AUTH("No MAC key matching packet from %I found on %s",
+  LOG_PKT_AUTH("Authentication failed for %I on %s - no matching key",
                saddr, ifa->ifname);
-  return 1;
+  return 0;
 }
 
 /**
@@ -1859,7 +1861,7 @@ babel_auth_check(struct babel_iface *ifa,
                  struct babel_pkt_header *pkt,
                  byte *trailer, uint trailer_len)
 {
-  u8 frame_err UNUSED = 0;
+  uint frame_err UNUSED = 0;
   struct babel_proto *p = ifa->proto;
   struct babel_tlv *tlv;
 
@@ -1868,25 +1870,25 @@ babel_auth_check(struct babel_iface *ifa,
     .proto        = p,
     .ifa          = ifa,
     .saddr        = saddr,
-    .is_unicast     = !(ipa_classify(daddr) & IADDR_MULTICAST),
+    .is_unicast   = !(ipa_classify(daddr) & IADDR_MULTICAST),
     .auth = {
       .sender = saddr,
     },
   };
 
   if (ifa->cf->auth_type == BABEL_AUTH_NONE)
-    return 0;
+    return 1;
 
   TRACE(D_PACKETS, "Checking packet authentication signature");
 
-  if (babel_auth_check_mac(ifa, (byte *)pkt,
+  if (!babel_auth_check_mac(ifa, (byte *)pkt,
                            trailer, trailer_len,
                            saddr, sport,
                            daddr, dport))
     goto fail;
 
   /* MAC verified; parse packet to check packet counter and challenge */
-  WALK_TLVS(FIRST_TLV(pkt), trailer, tlv, frame_err, saddr, ifa->iface->name)
+  WALK_TLVS(FIRST_TLV(pkt), trailer, tlv, frame_err, saddr, ifa->ifname)
   {
     union babel_msg msg;
     enum parse_result res;
@@ -1894,26 +1896,26 @@ babel_auth_check(struct babel_iface *ifa,
     res = babel_read_tlv(tlv, &msg, &state);
     if (res == PARSE_ERROR)
     {
-      LOG_PKT_AUTH("Bad TLV from %I via %s type %d pos %d - parse error",
-                   saddr, ifa->iface->name, tlv->type, (byte *)tlv - (byte *)pkt);
+      LOG_PKT("Bad TLV from %I via %s type %d pos %d - parse error",
+	      saddr, ifa->ifname, tlv->type, (int) ((byte *)tlv - (byte *)pkt));
       goto fail;
     }
   }
   WALK_TLVS_END;
 
-  if (babel_auth_check_pc(ifa, &state.auth))
+  if (!babel_auth_check_pc(ifa, &state.auth))
     goto fail;
 
   TRACE(D_PACKETS, "Packet from %I via %s authenticated successfully",
         saddr, ifa->ifname);
-  return 0;
+  return 1;
 
 fail:
-  LOG_PKT_AUTH("Packet from %I via %s failed authentication%s",
+  TRACE(D_PACKETS, "Packet from %I via %s failed authentication%s",
                saddr, ifa->ifname,
                ifa->cf->auth_permissive ? " but accepted in permissive mode" : "");
 
-  return !ifa->cf->auth_permissive;
+  return ifa->cf->auth_permissive;
 }
 
 /**
@@ -1927,17 +1929,17 @@ fail:
  * counter TLV that must be included in every packet.
  */
 int
-babel_auth_add_tlvs(struct babel_iface *ifa, struct babel_tlv *tlv, int max_len)
+babel_auth_add_tlvs(struct babel_iface *ifa, struct babel_tlv *hdr, uint max_len)
 {
   struct babel_proto *p = ifa->proto;
-  struct babel_tlv_pc *msg;
-  int len;
+  struct babel_tlv_pc *tlv;
+  uint len;
 
   if (ifa->cf->auth_type == BABEL_AUTH_NONE)
     return 0;
 
-  msg = (void *)tlv;
-  len = sizeof(*msg) + BABEL_AUTH_INDEX_LEN;
+  tlv = (void *) hdr;
+  len = sizeof(struct babel_tlv_pc) + BABEL_AUTH_INDEX_LEN;
   max_len += ifa->auth_tx_overhead;
 
   if (len > max_len)
@@ -1947,10 +1949,9 @@ babel_auth_add_tlvs(struct babel_iface *ifa, struct babel_tlv *tlv, int max_len)
     return 0;
   }
 
-  msg->type = BABEL_TLV_PC;
-  msg->length = len - sizeof(struct babel_tlv);
-  put_u32(&msg->pc, ifa->auth_pc++);
-  memcpy(msg->index, ifa->auth_index, BABEL_AUTH_INDEX_LEN);
+  TLV_HDR(tlv, BABEL_TLV_PC, len);
+  put_u32(&tlv->pc, ifa->auth_pc++);
+  memcpy(tlv->index, ifa->auth_index, BABEL_AUTH_INDEX_LEN);
 
   /* Reset index on overflow to 0 */
   if (!ifa->auth_pc)
@@ -1971,58 +1972,48 @@ int
 babel_auth_sign(struct babel_iface *ifa, ip_addr dest)
 {
   struct babel_proto *p = ifa->proto;
-  struct babel_mac_pseudohdr phdr;
-  struct babel_pkt_header *hdr;
-  struct password_item *pass;
-  int tot_len = 0, i = 0;
-  struct babel_tlv *tlv;
   sock *sk = ifa->sk;
-  byte *pos, *end;
-  btime now_;
-  int len;
 
   if (ifa->cf->auth_type == BABEL_AUTH_NONE)
     return 0;
 
-  hdr = (void *) sk->tbuf;
-  len = get_u16(&hdr->length) + sizeof(struct babel_pkt_header);
+  struct babel_pkt_header *hdr = (void *) sk->tbuf;
+  int len = get_u16(&hdr->length) + sizeof(struct babel_pkt_header);
 
-  pos = (byte *)hdr + len;
-  end = (byte *)hdr + ifa->tx_length + ifa->auth_tx_overhead;
-  tlv = (void *)pos;
-  now_ = current_real_time();
+  byte *pkt = (byte *) hdr;
+  byte *pos = pkt + len;
+  byte *end = pkt + ifa->tx_length + ifa->auth_tx_overhead;
+  btime now_ = current_real_time();
 
-  babel_mac_build_phdr(&phdr, sk->saddr, sk->fport, dest, sk->dport);
+  struct babel_mac_pseudoheader phdr;
+  babel_mac_build_phdr(&phdr, sk->saddr, sk->sport, dest, sk->dport);
 
+  struct password_item *pass;
   WALK_LIST(pass, *ifa->cf->passwords)
   {
-    struct babel_tlv_mac *msg = (void *)tlv;
-    uint buf_len = (uint) (end - (byte *)msg - sizeof(*msg));
+    struct babel_tlv_mac *tlv = (void *) pos;
+    uint tlv_len = sizeof(struct babel_tlv_mac) + mac_type_length(pass->alg);
 
     if (pass->genfrom > now_ || pass->gento < now_)
       continue;
 
-    if (babel_mac_hash(pass, &phdr,
-                       (byte *)hdr, len,
-                       msg->mac, &buf_len))
+    if (pos + tlv_len > end)
     {
-      LOG_WARN("Insufficient space for MAC signatures on iface %s dest %I",
-               ifa->ifname, dest);
+      LOG_WARN("Insufficient space for MAC signatures on iface %s dst %I (%d/%d)",
+               ifa->ifname, dest, tlv_len, (int) (end-pos));
       break;
     }
 
-    msg->type = BABEL_TLV_MAC;
-    msg->length = buf_len;
+    TLV_HDR(tlv, BABEL_TLV_MAC, tlv_len);
+    babel_mac_fill(pass, &phdr, pkt, len, tlv->mac);
 
-    tlv = NEXT_TLV(tlv);
-    tot_len += buf_len + sizeof(*msg);
-    i++;
+    pos += tlv_len;
   }
 
-  DBG("Added %d MAC signatures (%d bytes) on ifa %s for dest %I\n",
-      i, tot_len, ifa->ifname, dest);
+  DBG("Added MAC signatures (%d bytes) on ifa %s for dest %I\n",
+      tot_len, ifa->ifname, dest);
 
-  return tot_len;
+  return pos - (pkt + len);
 }
 
 /**
@@ -2041,7 +2032,7 @@ babel_auth_set_tx_overhead(struct babel_iface *ifa)
     return;
   }
 
-  ifa->auth_tx_overhead = (sizeof(struct babel_tlv_pc) +
+  ifa->auth_tx_overhead = (sizeof(struct babel_tlv_pc) + BABEL_AUTH_INDEX_LEN +
                            sizeof(struct babel_tlv_mac) * ifa->cf->mac_num_keys +
                            ifa->cf->mac_total_len);
   ifa->tx_length -= ifa->auth_tx_overhead;