From 024c310b537abc3ddbac3054de71fd759d422824 Mon Sep 17 00:00:00 2001
From: Ondrej Zajicek <santiago@crfreenet.org>
Date: Wed, 8 Apr 2009 20:15:01 +0200
Subject: [PATCH] Fixes broken cryptographic authentication in OSPF

Cryptographic authentication in OSPF is defective by
design - there might be several packets independently
sent to the network (for example HELLO, LSUPD and LSACK)
where they might be reordered and that causes crypt.
sequence number error.

That can be workarounded by not incresing sequence number
too often. Now we update it only when last packet was sent
before at least one second. This can constitute a risk of
replay attacks, but RFC supposes something similar (like time
in seconds used as CSN).
---
 nest/config.Y       |  8 +++++++-
 proto/ospf/config.Y | 18 ++++++++++++++++--
 proto/ospf/ospf.h   |  3 ++-
 proto/ospf/packet.c | 30 ++++++++++++++++++++++--------
 4 files changed, 47 insertions(+), 12 deletions(-)

diff --git a/nest/config.Y b/nest/config.Y
index 48940ffd..e8437770 100644
--- a/nest/config.Y
+++ b/nest/config.Y
@@ -22,7 +22,13 @@ static list *this_p_list;
 static struct password_item *this_p_item;
 static int password_id;
 
-static list *
+static inline void
+reset_passwords(void)
+{
+ this_p_list = NULL;
+}
+
+static inline list *
 get_passwords(void)
 {
   list *rv = this_p_list;
diff --git a/proto/ospf/config.Y b/proto/ospf/config.Y
index 00b6be7c..bfe2d9c8 100644
--- a/proto/ospf/config.Y
+++ b/proto/ospf/config.Y
@@ -19,6 +19,18 @@ static struct iface_patt *this_ipatt;
 static struct nbma_node *this_nbma;
 static struct area_net_config *this_pref;
 
+static void
+finish_iface_config(struct ospf_iface_patt *ip)
+{
+  ip->passwords = get_passwords();
+
+  if ((ip->autype == OSPF_AUTH_CRYPT) && (ip->helloint < 5))
+    log(L_WARN "Hello or poll interval less that 5 makes cryptographic authenication prone to replay attacks");
+
+  if ((ip->autype == OSPF_AUTH_NONE) && (ip->passwords != NULL))
+    log(L_WARN "Password option without authentication option does not make sense");
+}
+
 CF_DECLS
 
 CF_KEYWORDS(OSPF, AREA, OSPF_METRIC1, OSPF_METRIC2, OSPF_TAG)
@@ -83,7 +95,7 @@ ospf_area_item:
  ;
 
 ospf_vlink:
-   ospf_vlink_start '{' ospf_vlink_opts '}' { OSPF_PATT->passwords = get_passwords(); }
+   ospf_vlink_start '{' ospf_vlink_opts '}' { finish_iface_config(OSPF_PATT); }
  | ospf_vlink_start
  ;
 
@@ -121,6 +133,7 @@ ospf_vlink_start: VIRTUAL LINK idval
   OSPF_PATT->type = OSPF_IT_VLINK;
   init_list(&OSPF_PATT->nbma_list);
   OSPF_PATT->autype = OSPF_AUTH_NONE;
+  reset_passwords();
  }
 ;
 
@@ -223,6 +236,7 @@ ospf_iface_start:
   OSPF_PATT->stub = 0;
   init_list(&OSPF_PATT->nbma_list);
   OSPF_PATT->autype = OSPF_AUTH_NONE;
+  reset_passwords();
  }
 ;
 
@@ -237,7 +251,7 @@ ospf_iface_opt_list:
  ;
 
 ospf_iface:
-  ospf_iface_start iface_patt ospf_iface_opt_list { OSPF_PATT->passwords = get_passwords(); }
+  ospf_iface_start iface_patt ospf_iface_opt_list { finish_iface_config(OSPF_PATT); }
  ;
 
 ospf_iface_list:
diff --git a/proto/ospf/ospf.h b/proto/ospf/ospf.h
index 4e37c184..fb78af4e 100644
--- a/proto/ospf/ospf.h
+++ b/proto/ospf/ospf.h
@@ -161,7 +161,8 @@ struct ospf_iface
   u16 autype;
   u16 helloint;			/* number of seconds between hello sending */
   list *passwords;
-  u32 csn;                      /* Crypt seq num. that will be sent net */
+  u32 csn;                      /* Last used crypt seq number */
+  bird_clock_t csn_use;         /* Last time when packet with that CSN was sent */
   ip_addr drip;			/* Designated router */
   u32 drid;
   ip_addr bdrip;		/* Backup DR */
diff --git a/proto/ospf/packet.c b/proto/ospf/packet.c
index c6b233f8..23785fe8 100644
--- a/proto/ospf/packet.c
+++ b/proto/ospf/packet.c
@@ -75,13 +75,25 @@ ospf_pkt_finalize(struct ospf_iface *ifa, struct ospf_packet *pkt)
 
       pkt->checksum = 0;
 
+      /* Perhaps use random value to prevent replay attacks after
+	 reboot when system does not have independent RTC? */
       if (!ifa->csn)
-        ifa->csn = (u32) time(NULL);
+	{
+	  ifa->csn = (u32) now;
+	  ifa->csn_use = now;
+	}
+
+      /* We must have sufficient delay between sending a packet and increasing 
+	 CSN to prevent reordering of packets (in a network) with different CSNs */
+      if ((now - ifa->csn_use) > 1)
+	ifa->csn++;
+
+      ifa->csn_use = now;
 
       pkt->u.md5.keyid = passwd->id;
       pkt->u.md5.len = OSPF_AUTH_CRYPT_SIZE;
       pkt->u.md5.zero = 0;
-      pkt->u.md5.csn = htonl(ifa->csn++);
+      pkt->u.md5.csn = htonl(ifa->csn);
       tail = ((void *)pkt) + ntohs(pkt->length);
       MD5Init(&ctxt);
       MD5Update(&ctxt, (char *) pkt, ntohs(pkt->length));
@@ -184,12 +196,14 @@ ospf_pkt_checkauth(struct ospf_neighbor *n, struct ospf_iface *ifa, struct ospf_
 
       if (n)
       {
-        if(ntohs(pkt->u.md5.csn) < n->csn)
-        {
-          OSPF_TRACE(D_PACKETS, "OSPF_auth: lower sequence number");
-          return 0;
-        }
-        n->csn = ntohs(pkt->u.md5.csn);
+	u32 rcv_csn = ntohl(pkt->u.md5.csn);
+	if(rcv_csn < n->csn)
+	{
+	  OSPF_TRACE(D_PACKETS, "OSPF_auth: lower sequence number (rcv %d, old %d)", rcv_csn, n->csn);
+	  return 0;
+	}
+
+	n->csn = rcv_csn;
       }
 
       MD5Init(&ctxt);