From 61ab0301606053192f45c188bc48afc837518770 Mon Sep 17 00:00:00 2001
From: Denis Ovsienko <infrastation@yandex.ru>
Date: Mon, 26 Sep 2011 13:17:52 +0400
Subject: ospfd: CVE-2011-3325 part 1 (OSPF header underrun)

This vulnerability (CERT-FI #514838) was reported by CROSS project.

When only 14 first bytes of a Hello packet is delivered, ospfd crashes.

* ospf_packet.c
  * ospf_read(): add size check
---
 ospfd/ospf_packet.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

(limited to 'ospfd/ospf_packet.c')

diff --git a/ospfd/ospf_packet.c b/ospfd/ospf_packet.c
index be137d91..57278788 100644
--- a/ospfd/ospf_packet.c
+++ b/ospfd/ospf_packet.c
@@ -2430,10 +2430,19 @@ ospf_read (struct thread *thread)
       return 0;
     }
 
-  /* Adjust size to message length. */
+  /* Advance from IP header to OSPF header (iph->ip_hl has been verified
+     by ospf_recv_packet() to be correct). */
   stream_forward_getp (ibuf, iph->ip_hl * 4);
-  
-  /* Get ospf packet header. */
+
+  /* Make sure the OSPF header is really there. */
+  if (stream_get_endp (ibuf) - stream_get_getp (ibuf) < OSPF_HEADER_SIZE)
+  {
+    zlog_debug ("ospf_read: ignored OSPF packet with undersized (%u bytes) header",
+                stream_get_endp (ibuf) - stream_get_getp (ibuf));
+    return -1;
+  }
+
+  /* Now it is safe to access all fields of OSPF packet header. */
   ospfh = (struct ospf_header *) STREAM_PNT (ibuf);
 
   /* associate packet with ospf interface */
-- 
cgit v1.2.1


From 717750433839762d23a5f8d88fe0b4d57c8d490a Mon Sep 17 00:00:00 2001
From: Denis Ovsienko <infrastation@yandex.ru>
Date: Mon, 26 Sep 2011 13:18:02 +0400
Subject: ospfd: CVE-2011-3325 part 2 (OSPF pkt type segv)

This vulnerability (CERT-FI #514838) was reported by CROSS project.

The error is reproducible only when ospfd debugging is enabled:
  * debug ospf packet all
  * debug ospf zebra
When incoming packet header type field is set to 0x0a, ospfd will crash.

* ospf_packet.c
  * ospf_verify_header(): add type field check
  * ospf_read(): perform input checks early
---
 ospfd/ospf_packet.c | 32 ++++++++++++++++++--------------
 1 file changed, 18 insertions(+), 14 deletions(-)

(limited to 'ospfd/ospf_packet.c')

diff --git a/ospfd/ospf_packet.c b/ospfd/ospf_packet.c
index 57278788..151ed328 100644
--- a/ospfd/ospf_packet.c
+++ b/ospfd/ospf_packet.c
@@ -2321,6 +2321,13 @@ ospf_verify_header (struct stream *ibuf, struct ospf_interface *oi,
       return -1;
     }
 
+  /* Valid OSPFv2 packet types are 1 through 5 inclusive. */
+  if (ospfh->type < 1 || ospfh->type > 5)
+  {
+    zlog_warn ("interface %s: invalid packet type %u", IF_NAME (oi), ospfh->type);
+    return -1;
+  }
+
   /* Check Area ID. */
   if (!ospf_check_area_id (oi, ospfh))
     {
@@ -2448,6 +2455,17 @@ ospf_read (struct thread *thread)
   /* associate packet with ospf interface */
   oi = ospf_if_lookup_recv_if (ospf, iph->ip_src, ifp);
 
+  /* Verify header fields before any further processing. */
+  ret = ospf_verify_header (ibuf, oi, iph, ospfh);
+  if (ret < 0)
+  {
+    if (IS_DEBUG_OSPF_PACKET (0, RECV))
+      zlog_debug ("ospf_read[%s]: Header check failed, "
+                  "dropping.",
+                  inet_ntoa (iph->ip_src));
+    return ret;
+  }
+
   /* If incoming interface is passive one, ignore it. */
   if (oi && OSPF_IF_PASSIVE_STATUS (oi) == OSPF_IF_PASSIVE)
     {
@@ -2557,20 +2575,6 @@ ospf_read (struct thread *thread)
 	zlog_debug ("-----------------------------------------------------");
   }
 
-  /* Some header verification. */
-  ret = ospf_verify_header (ibuf, oi, iph, ospfh);
-  if (ret < 0)
-    {
-      if (IS_DEBUG_OSPF_PACKET (ospfh->type - 1, RECV))
-        {
-          zlog_debug ("ospf_read[%s/%s]: Header check failed, "
-                     "dropping.",
-                     ospf_packet_type_str[ospfh->type],
-                     inet_ntoa (iph->ip_src));
-        }
-      return ret;
-    }
-
   stream_forward_getp (ibuf, OSPF_HEADER_SIZE);
 
   /* Adjust size to message length. */
-- 
cgit v1.2.1


From 3aad46bdaa0a2700e43252904b9481750c8cb45d Mon Sep 17 00:00:00 2001
From: YAMAMOTO Shigeru <shigeru@iij.ad.jp>
Date: Wed, 28 Sep 2011 21:00:14 +0400
Subject: ospfd: fix regression in recent commit

commit '717750433839762d23a5f8d88fe0b4d57c8d490a' causes SEGV error,
when 'oi = ospf_if_lookup_recv_if (ospf, iph->ip_src, ifp);' returns
NULL.

* ospf_packet.c
  * ospf_read(): change a place of calling 'ospf_verify_header()'
---
 ospfd/ospf_packet.c | 25 +++++++++++++++----------
 1 file changed, 15 insertions(+), 10 deletions(-)

(limited to 'ospfd/ospf_packet.c')

diff --git a/ospfd/ospf_packet.c b/ospfd/ospf_packet.c
index 151ed328..0f338d35 100644
--- a/ospfd/ospf_packet.c
+++ b/ospfd/ospf_packet.c
@@ -2455,16 +2455,10 @@ ospf_read (struct thread *thread)
   /* associate packet with ospf interface */
   oi = ospf_if_lookup_recv_if (ospf, iph->ip_src, ifp);
 
-  /* Verify header fields before any further processing. */
-  ret = ospf_verify_header (ibuf, oi, iph, ospfh);
-  if (ret < 0)
-  {
-    if (IS_DEBUG_OSPF_PACKET (0, RECV))
-      zlog_debug ("ospf_read[%s]: Header check failed, "
-                  "dropping.",
-                  inet_ntoa (iph->ip_src));
-    return ret;
-  }
+  /* ospf_verify_header() relies on a valid "oi" and thus can be called only
+     after the passive/backbone/other checks below are passed. These checks
+     in turn access the fields of unverified "ospfh" structure for their own
+     purposes and must remain very accurate in doing this. */
 
   /* If incoming interface is passive one, ignore it. */
   if (oi && OSPF_IF_PASSIVE_STATUS (oi) == OSPF_IF_PASSIVE)
@@ -2556,6 +2550,17 @@ ospf_read (struct thread *thread)
       return 0;
     }
 
+  /* Verify more OSPF header fields. */
+  ret = ospf_verify_header (ibuf, oi, iph, ospfh);
+  if (ret < 0)
+  {
+    if (IS_DEBUG_OSPF_PACKET (0, RECV))
+      zlog_debug ("ospf_read[%s]: Header check failed, "
+                  "dropping.",
+                  inet_ntoa (iph->ip_src));
+    return ret;
+  }
+
   /* Show debug receiving packet. */
   if (IS_DEBUG_OSPF_PACKET (ospfh->type - 1, RECV))
     {
-- 
cgit v1.2.1


From 223da1a912305f2cac6f72c3b480ce103a25d7d6 Mon Sep 17 00:00:00 2001
From: Jaroslav Fojtik <jafojtik@seznam.cz>
Date: Sun, 11 Dec 2011 18:22:16 +0400
Subject: ospfd: more info in LSA checksum warning (BZ#685)

---
 ospfd/ospf_packet.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

(limited to 'ospfd/ospf_packet.c')

diff --git a/ospfd/ospf_packet.c b/ospfd/ospf_packet.c
index 0f338d35..f8887c5f 100644
--- a/ospfd/ospf_packet.c
+++ b/ospfd/ospf_packet.c
@@ -1568,8 +1568,13 @@ ospf_ls_upd_list_lsa (struct ospf_neighbor *nbr, struct stream *s,
       sum = lsah->checksum;
       if (sum != ospf_lsa_checksum (lsah))
 	{
-	  zlog_warn ("Link State Update: LSA checksum error %x, %x.",
-		     sum, lsah->checksum);
+	  /* (bug #685) more details in a one-line message make it possible
+	   * to identify problem source on the one hand and to have a better
+	   * chance to compress repeated messages in syslog on the other */
+	  zlog_warn ("Link State Update: LSA checksum error %x/%x, ID=%s from: nbr %s, router ID %s, adv router %s",
+		     sum, lsah->checksum, inet_ntoa (lsah->id),
+		     inet_ntoa (nbr->src), inet_ntoa (nbr->router_id),
+		     inet_ntoa (lsah->adv_router));
 	  continue;
 	}
 
-- 
cgit v1.2.1


From de5ccb96023181765c9904ab40e645c397f536d6 Mon Sep 17 00:00:00 2001
From: Dmitrij Tejblum <tejblum@yandex-team.ru>
Date: Mon, 12 Dec 2011 20:30:10 +0400
Subject: ospfd: fix packet reception for FreeBSD 10.

* ospf_packet.c (ospf_recv_packet): FreeBSD, starting from version 10, will not
  subtract the IP header size from ip_len.

This is the patch from FreeBSD's ports/net/quagga/files/patch-ospfd__ospf_packet.c,
by Boris Kovalenko.
---
 ospfd/ospf_packet.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'ospfd/ospf_packet.c')

diff --git a/ospfd/ospf_packet.c b/ospfd/ospf_packet.c
index f8887c5f..07050824 100644
--- a/ospfd/ospf_packet.c
+++ b/ospfd/ospf_packet.c
@@ -2121,7 +2121,7 @@ ospf_recv_packet (int fd, struct interface **ifp, struct stream *ibuf)
   
   ip_len = iph->ip_len;
   
-#if !defined(GNU_LINUX) && (OpenBSD < 200311)
+#if !defined(GNU_LINUX) && (OpenBSD < 200311) && (__FreeBSD_version < 1000000)
   /*
    * Kernel network code touches incoming IP header parameters,
    * before protocol specific processing.
-- 
cgit v1.2.1


From f63f06da2e7be6b17c72dd6110aae179f42f3700 Mon Sep 17 00:00:00 2001
From: Paul Jakma <paul@quagga.net>
Date: Fri, 8 Apr 2011 12:44:43 +0100
Subject: general: remove inline qualifiers and move in-header functions to
 objects

* (general) Move functions in headers into files, to be compiled into
  shared object files. Remove inline qualifier from functions. Let the
  compiler do the work.
---
 ospfd/ospf_packet.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

(limited to 'ospfd/ospf_packet.c')

diff --git a/ospfd/ospf_packet.c b/ospfd/ospf_packet.c
index 07050824..b714c27d 100644
--- a/ospfd/ospf_packet.c
+++ b/ospfd/ospf_packet.c
@@ -266,7 +266,7 @@ ospf_packet_dup (struct ospf_packet *op)
 }
 
 /* XXX inline */
-static inline unsigned int
+static unsigned int
 ospf_packet_authspace (struct ospf_interface *oi)
 {
   int auth = 0;
@@ -2213,7 +2213,7 @@ ospf_associate_packet_vl (struct ospf *ospf, struct interface *ifp,
   return NULL;
 }
 
-static inline int
+static int
 ospf_check_area_id (struct ospf_interface *oi, struct ospf_header *ospfh)
 {
   /* Check match the Area ID of the receiving interface. */
-- 
cgit v1.2.1


From 272ca1e33016b1cbf9c72038d5e631b0adb04dc9 Mon Sep 17 00:00:00 2001
From: Denis Ovsienko <infrastation@yandex.ru>
Date: Sun, 15 Jan 2012 19:12:19 +0400
Subject: ospfd: use LOOKUP() for ospf_packet_type_str

* ospf_packet.h: add proper str/max extern declarations
* ospf_packet.c
  * ospf_packet_type_str: rewrite in "struct message", add max value
  * ospf_packet_add(): use LOOKUP()
  * ospf_write(): ditto
  * ospf_hello(): ditto
  * ospf_read(): ditto
* ospf_dump.h: the declaration does not belong here
* ospf_dump.c
  * ospf_header_dump(): use LOOKUP()
  * show_debugging_ospf(): ditto
---
 ospfd/ospf_packet.c | 25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)

(limited to 'ospfd/ospf_packet.c')

diff --git a/ospfd/ospf_packet.c b/ospfd/ospf_packet.c
index b714c27d..a8102da4 100644
--- a/ospfd/ospf_packet.c
+++ b/ospfd/ospf_packet.c
@@ -50,15 +50,16 @@
 #include "ospfd/ospf_dump.h"
 
 /* Packet Type String. */
-const char *ospf_packet_type_str[] =
-{
-  "unknown",
-  "Hello",
-  "Database Description",
-  "Link State Request",
-  "Link State Update",
-  "Link State Acknowledgment",
+const struct message ospf_packet_type_str[] =
+{
+  { OSPF_MSG_HELLO,   "Hello"                     },
+  { OSPF_MSG_DB_DESC, "Database Description"      },
+  { OSPF_MSG_LS_REQ,  "Link State Request"        },
+  { OSPF_MSG_LS_UPD,  "Link State Update"         },
+  { OSPF_MSG_LS_ACK,  "Link State Acknowledgment" },
 };
+const size_t ospf_packet_type_str_max = sizeof (ospf_packet_type_str) /
+  sizeof (ospf_packet_type_str[0]);
 
 /* OSPF authentication checking function */
 static int
@@ -201,7 +202,7 @@ ospf_packet_add (struct ospf_interface *oi, struct ospf_packet *op)
 	       "destination %s) called with NULL obuf, ignoring "
 	       "(please report this bug)!\n",
 	       IF_NAME(oi), oi->state, LOOKUP (ospf_ism_state_msg, oi->state),
-	       ospf_packet_type_str[stream_getc_from(op->s, 1)],
+	       LOOKUP (ospf_packet_type_str, stream_getc_from(op->s, 1)),
 	       inet_ntoa (op->dst));
       return;
     }
@@ -755,7 +756,7 @@ ospf_write (struct thread *thread)
 	}
 
       zlog_debug ("%s sent to [%s] via [%s].",
-		 ospf_packet_type_str[type], inet_ntoa (op->dst),
+		 LOOKUP (ospf_packet_type_str, type), inet_ntoa (op->dst),
 		 IF_NAME (oi));
 
       if (IS_DEBUG_OSPF_PACKET (type - 1, DETAIL))
@@ -801,7 +802,7 @@ ospf_hello (struct ip *iph, struct ospf_header *ospfh,
         {
           zlog_debug ("ospf_header[%s/%s]: selforiginated, "
                      "dropping.",
-                     ospf_packet_type_str[ospfh->type],
+                     LOOKUP (ospf_packet_type_str, ospfh->type),
                      inet_ntoa (iph->ip_src));
         }
       return;
@@ -2576,7 +2577,7 @@ ospf_read (struct thread *thread)
         }
 
       zlog_debug ("%s received from [%s] via [%s]",
-                 ospf_packet_type_str[ospfh->type],
+                 LOOKUP (ospf_packet_type_str, ospfh->type),
                  inet_ntoa (ospfh->router_id), IF_NAME (oi));
       zlog_debug (" src [%s],", inet_ntoa (iph->ip_src));
       zlog_debug (" dst [%s]", inet_ntoa (iph->ip_dst));
-- 
cgit v1.2.1


From 7e0e2cb14ca16ce9eaca3b0300c1ffa92a6a104b Mon Sep 17 00:00:00 2001
From: Denis Ovsienko <infrastation@yandex.ru>
Date: Fri, 20 Jan 2012 22:32:10 +0400
Subject: ospfd: fix ospf_packet_add_top() to use LOOKUP()

---
 ospfd/ospf_packet.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'ospfd/ospf_packet.c')

diff --git a/ospfd/ospf_packet.c b/ospfd/ospf_packet.c
index a8102da4..876320ee 100644
--- a/ospfd/ospf_packet.c
+++ b/ospfd/ospf_packet.c
@@ -223,7 +223,7 @@ ospf_packet_add_top (struct ospf_interface *oi, struct ospf_packet *op)
 	       "destination %s) called with NULL obuf, ignoring "
 	       "(please report this bug)!\n",
 	       IF_NAME(oi), oi->state, LOOKUP (ospf_ism_state_msg, oi->state),
-	       ospf_packet_type_str[stream_getc_from(op->s, 1)],
+	       LOOKUP (ospf_packet_type_str, stream_getc_from(op->s, 1)),
 	       inet_ntoa (op->dst));
       return;
     }
-- 
cgit v1.2.1


From 75c8eabbb5d3dc8aa21b61e8700ab939ce272f5c Mon Sep 17 00:00:00 2001
From: Denis Ovsienko <infrastation@yandex.ru>
Date: Mon, 30 Jan 2012 15:41:39 +0400
Subject: ospfd: introduce ospf_packet_minlen[] (BZ#705)

This commit ports some of the OSPFv3 packet reception checks
to OSPFv2.

* ospf_packet.c
  * ospf_packet_minlen[]: a direct equivalent of ospf6_packet_minlen[]
  * ospf_packet_examin(): new function designed after the first part
    of ospf6_packet_examin()
  * ospf_read(): verify received packet with ospf_packet_examin()
* ospf_packet.h: add convenience macros
---
 ospfd/ospf_packet.c | 65 +++++++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 56 insertions(+), 9 deletions(-)

(limited to 'ospfd/ospf_packet.c')

diff --git a/ospfd/ospf_packet.c b/ospfd/ospf_packet.c
index 876320ee..d52430a3 100644
--- a/ospfd/ospf_packet.c
+++ b/ospfd/ospf_packet.c
@@ -61,6 +61,18 @@ const struct message ospf_packet_type_str[] =
 const size_t ospf_packet_type_str_max = sizeof (ospf_packet_type_str) /
   sizeof (ospf_packet_type_str[0]);
 
+/* Minimum (besides OSPF_HEADER_SIZE) lengths for OSPF packets of
+   particular types, offset is the "type" field of a packet. */
+static const u_int16_t ospf_packet_minlen[] =
+{
+  0,
+  OSPF_HELLO_MIN_SIZE,
+  OSPF_DB_DESC_MIN_SIZE,
+  OSPF_LS_REQ_MIN_SIZE,
+  OSPF_LS_UPD_MIN_SIZE,
+  OSPF_LS_ACK_MIN_SIZE,
+};
+
 /* OSPF authentication checking function */
 static int
 ospf_auth_type (struct ospf_interface *oi)
@@ -2314,6 +2326,47 @@ ospf_check_sum (struct ospf_header *ospfh)
   return 1;
 }
 
+/* Verify a complete OSPF packet for proper sizing/alignment. */
+static unsigned
+ospf_packet_examin (struct ospf_header * oh, const unsigned bytesonwire)
+{
+  u_int16_t bytesdeclared;
+
+  /* Length, 1st approximation. */
+  if (bytesonwire < OSPF_HEADER_SIZE)
+  {
+    if (IS_DEBUG_OSPF_PACKET (0, RECV))
+      zlog_debug ("%s: undersized (%u B) packet", __func__, bytesonwire);
+    return MSG_NG;
+  }
+  /* Now it is safe to access header fields. Performing length check, allow
+   * for possible extra bytes of crypto auth/padding, which are not counted
+   * in the OSPF header "length" field. */
+  bytesdeclared = ntohs (oh->length);
+  if (bytesdeclared > bytesonwire)
+  {
+    if (IS_DEBUG_OSPF_PACKET (0, RECV))
+      zlog_debug ("%s: packet length error (%u real, %u declared)",
+                  __func__, bytesonwire, bytesdeclared);
+    return MSG_NG;
+  }
+  /* Length, 2nd approximation. The type-specific constraint is checked
+     against declared length, not amount of bytes on wire. */
+  if
+  (
+    oh->type >= OSPF_MSG_HELLO &&
+    oh->type <= OSPF_MSG_LS_ACK &&
+    bytesdeclared < OSPF_HEADER_SIZE + ospf_packet_minlen[oh->type]
+  )
+  {
+    if (IS_DEBUG_OSPF_PACKET (0, RECV))
+      zlog_debug ("%s: undersized (%u B) %s packet", __func__,
+                  bytesdeclared, LOOKUP (ospf_packet_type_str, oh->type));
+    return MSG_NG;
+  }
+  return MSG_OK;
+}
+
 /* OSPF Header verification. */
 static int
 ospf_verify_header (struct stream *ibuf, struct ospf_interface *oi,
@@ -2409,10 +2462,10 @@ ospf_read (struct thread *thread)
   /* prepare for next packet. */
   ospf->t_read = thread_add_read (master, ospf_read, ospf, ospf->fd);
 
-  /* read OSPF packet. */
   stream_reset(ospf->ibuf);
   if (!(ibuf = ospf_recv_packet (ospf->fd, &ifp, ospf->ibuf)))
     return -1;
+  /* This raw packet is known to be at least as big as its IP header. */
   
   /* Note that there should not be alignment problems with this assignment
      because this is at the beginning of the stream data buffer. */
@@ -2447,16 +2500,10 @@ ospf_read (struct thread *thread)
      by ospf_recv_packet() to be correct). */
   stream_forward_getp (ibuf, iph->ip_hl * 4);
 
-  /* Make sure the OSPF header is really there. */
-  if (stream_get_endp (ibuf) - stream_get_getp (ibuf) < OSPF_HEADER_SIZE)
-  {
-    zlog_debug ("ospf_read: ignored OSPF packet with undersized (%u bytes) header",
-                stream_get_endp (ibuf) - stream_get_getp (ibuf));
+  ospfh = (struct ospf_header *) STREAM_PNT (ibuf);
+  if (MSG_OK != ospf_packet_examin (ospfh, stream_get_endp (ibuf) - stream_get_getp (ibuf)))
     return -1;
-  }
-
   /* Now it is safe to access all fields of OSPF packet header. */
-  ospfh = (struct ospf_header *) STREAM_PNT (ibuf);
 
   /* associate packet with ospf interface */
   oi = ospf_if_lookup_recv_if (ospf, iph->ip_src, ifp);
-- 
cgit v1.2.1


From e52591481ed64e4cf9f26a76ad682ed7e6b451e7 Mon Sep 17 00:00:00 2001
From: Denis Ovsienko <infrastation@yandex.ru>
Date: Mon, 30 Jan 2012 16:07:18 +0400
Subject: ospfd: review ospf_check_auth()

1. The only purpose of "ibuf" argument was to get stream size, which
was always equal to OSPF_MAX_PACKET_SIZE + 1, exactly as initialized
in ospf_new().

2. Fix the packet size check condition, which was incorrect for very
large packets, at least in theory.
---
 ospfd/ospf_packet.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

(limited to 'ospfd/ospf_packet.c')

diff --git a/ospfd/ospf_packet.c b/ospfd/ospf_packet.c
index d52430a3..b18117b5 100644
--- a/ospfd/ospf_packet.c
+++ b/ospfd/ospf_packet.c
@@ -2260,8 +2260,7 @@ ospf_check_network_mask (struct ospf_interface *oi, struct in_addr ip_src)
 }
 
 static int
-ospf_check_auth (struct ospf_interface *oi, struct stream *ibuf,
-		 struct ospf_header *ospfh)
+ospf_check_auth (struct ospf_interface *oi, struct ospf_header *ospfh)
 {
   int ret = 0;
   struct crypt_key *ck;
@@ -2287,7 +2286,7 @@ ospf_check_auth (struct ospf_interface *oi, struct stream *ibuf,
       /* This is very basic, the digest processing is elsewhere */
       if (ospfh->u.crypt.auth_data_len == OSPF_AUTH_MD5_SIZE && 
           ospfh->u.crypt.key_id == ck->key_id &&
-          ntohs (ospfh->length) + OSPF_AUTH_SIMPLE_SIZE <= stream_get_size (ibuf))
+          ntohs (ospfh->length) + OSPF_AUTH_MD5_SIZE <= OSPF_MAX_PACKET_SIZE)
         ret = 1;
       else
         ret = 0;
@@ -2411,7 +2410,7 @@ ospf_verify_header (struct stream *ibuf, struct ospf_interface *oi,
       return -1;
     }
 
-  if (! ospf_check_auth (oi, ibuf, ospfh))
+  if (! ospf_check_auth (oi, ospfh))
     {
       zlog_warn ("interface %s: ospf_read authentication failed.",
 		 IF_NAME (oi));
-- 
cgit v1.2.1


From 2d8223c5472129eba89d630dc4f2688ebeae1dd8 Mon Sep 17 00:00:00 2001
From: Denis Ovsienko <infrastation@yandex.ru>
Date: Mon, 30 Jan 2012 20:32:39 +0400
Subject: ospfd: review ospf_check_md5_digest()

Rewrite some pointer arithmetics without the additional variables and
move byte order conversion inside the function.
---
 ospfd/ospf_packet.c | 20 +++++---------------
 1 file changed, 5 insertions(+), 15 deletions(-)

(limited to 'ospfd/ospf_packet.c')

diff --git a/ospfd/ospf_packet.c b/ospfd/ospf_packet.c
index b18117b5..15ec3733 100644
--- a/ospfd/ospf_packet.c
+++ b/ospfd/ospf_packet.c
@@ -304,24 +304,14 @@ ospf_packet_max (struct ospf_interface *oi)
 
 
 static int
-ospf_check_md5_digest (struct ospf_interface *oi, struct stream *s,
-                       u_int16_t length)
+ospf_check_md5_digest (struct ospf_interface *oi, struct ospf_header *ospfh)
 {
-  unsigned char *ibuf;
   MD5_CTX ctx;
   unsigned char digest[OSPF_AUTH_MD5_SIZE];
-  unsigned char *pdigest;
   struct crypt_key *ck;
-  struct ospf_header *ospfh;
   struct ospf_neighbor *nbr;
+  u_int16_t length = ntohs (ospfh->length);
   
-
-  ibuf = STREAM_PNT (s);
-  ospfh = (struct ospf_header *) ibuf;
-
-  /* Get pointer to the end of the packet. */
-  pdigest = ibuf + length;
-
   /* Get secret key. */
   ck = ospf_crypt_key_lookup (OSPF_IF_PARAM (oi, auth_crypt),
 			      ospfh->u.crypt.key_id);
@@ -347,12 +337,12 @@ ospf_check_md5_digest (struct ospf_interface *oi, struct stream *s,
   /* Generate a digest for the ospf packet - their digest + our digest. */
   memset(&ctx, 0, sizeof(ctx));
   MD5Init(&ctx);
-  MD5Update(&ctx, ibuf, length);
+  MD5Update(&ctx, ospfh, length);
   MD5Update(&ctx, ck->auth_key, OSPF_AUTH_MD5_SIZE);
   MD5Final(digest, &ctx);
 
   /* compare the two */
-  if (memcmp (pdigest, digest, OSPF_AUTH_MD5_SIZE))
+  if (memcmp ((caddr_t)ospfh + length, digest, OSPF_AUTH_MD5_SIZE))
     {
       zlog_warn ("interface %s: ospf_check_md5 checksum mismatch",
 		 IF_NAME (oi));
@@ -2431,7 +2421,7 @@ ospf_verify_header (struct stream *ibuf, struct ospf_interface *oi,
     {
       if (ospfh->checksum != 0)
 	return -1;
-      if (ospf_check_md5_digest (oi, ibuf, ntohs (ospfh->length)) == 0)
+      if (ospf_check_md5_digest (oi, ospfh) == 0)
 	{
 	  zlog_warn ("interface %s: ospf_read md5 authentication failed.",
 		     IF_NAME (oi));
-- 
cgit v1.2.1


From 4e31de792ec5e48a97360b5b86196b4fa02996a3 Mon Sep 17 00:00:00 2001
From: Denis Ovsienko <infrastation@yandex.ru>
Date: Fri, 17 Feb 2012 16:20:50 +0400
Subject: ospfd: introduce ospf_lsa_minlen[] (BZ#705)

This commit ports more packet checks to OSPFv2, in particular, LSA size
verification and Router-LSA link blocks verification.

* ospf_lsa.h: add LSA size macros
* ospf_packet.h: add struct ospf_ls_update
* ospf_packet.c
  * ospf_lsa_minlen[]: a direct equivalent of ospf6_lsa_minlen[]
  * ospf_router_lsa_links_examin(): new function, verifies trailing
    part of a Router-LSA
  * ospf_lsa_examin(): new function like ospf6_lsa_examin()
  * ospf_lsaseq_examin(): new function like ospf6_lsaseq_examin()
  * ospf_packet_examin(): add type-specific deeper level checks
---
 ospfd/ospf_packet.c | 260 +++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 259 insertions(+), 1 deletion(-)

(limited to 'ospfd/ospf_packet.c')

diff --git a/ospfd/ospf_packet.c b/ospfd/ospf_packet.c
index 15ec3733..68c25790 100644
--- a/ospfd/ospf_packet.c
+++ b/ospfd/ospf_packet.c
@@ -73,6 +73,24 @@ static const u_int16_t ospf_packet_minlen[] =
   OSPF_LS_ACK_MIN_SIZE,
 };
 
+/* Minimum (besides OSPF_LSA_HEADER_SIZE) lengths for LSAs of particular
+   types, offset is the "LSA type" field. */
+static const u_int16_t ospf_lsa_minlen[] =
+{
+  0,
+  OSPF_ROUTER_LSA_MIN_SIZE,
+  OSPF_NETWORK_LSA_MIN_SIZE,
+  OSPF_SUMMARY_LSA_MIN_SIZE,
+  OSPF_SUMMARY_LSA_MIN_SIZE,
+  OSPF_AS_EXTERNAL_LSA_MIN_SIZE,
+  0,
+  OSPF_AS_EXTERNAL_LSA_MIN_SIZE,
+  0,
+  0,
+  0,
+  0,
+};
+
 /* OSPF authentication checking function */
 static int
 ospf_auth_type (struct ospf_interface *oi)
@@ -2315,11 +2333,199 @@ ospf_check_sum (struct ospf_header *ospfh)
   return 1;
 }
 
+/* Verify, that given link/TOS records are properly sized/aligned and match
+   Router-LSA "# links" and "# TOS" fields as specified in RFC2328 A.4.2. */
+static unsigned
+ospf_router_lsa_links_examin
+(
+  struct router_lsa_link * link,
+  u_int16_t linkbytes,
+  const u_int16_t num_links
+)
+{
+  unsigned counted_links = 0, thislinklen;
+
+  while (linkbytes)
+  {
+    thislinklen = OSPF_ROUTER_LSA_LINK_SIZE + 4 * link->m[0].tos_count;
+    if (thislinklen > linkbytes)
+    {
+      if (IS_DEBUG_OSPF_PACKET (0, RECV))
+        zlog_debug ("%s: length error in link block #%u", __func__, counted_links);
+      return MSG_NG;
+    }
+    link = (struct router_lsa_link *)((caddr_t) link + thislinklen);
+    linkbytes -= thislinklen;
+    counted_links++;
+  }
+  if (counted_links != num_links)
+  {
+    if (IS_DEBUG_OSPF_PACKET (0, RECV))
+      zlog_debug ("%s: %u link blocks declared, %u present",
+                  __func__, num_links, counted_links);
+    return MSG_NG;
+  }
+  return MSG_OK;
+}
+
+/* Verify, that the given LSA is properly sized/aligned (including type-specific
+   minimum length constraint). */
+static unsigned
+ospf_lsa_examin (struct lsa_header * lsah, const u_int16_t lsalen, const u_char headeronly)
+{
+  unsigned ret;
+  struct router_lsa * rlsa;
+  if
+  (
+    lsah->type < OSPF_MAX_LSA &&
+    ospf_lsa_minlen[lsah->type] &&
+    lsalen < OSPF_LSA_HEADER_SIZE + ospf_lsa_minlen[lsah->type]
+  )
+  {
+    if (IS_DEBUG_OSPF_PACKET (0, RECV))
+      zlog_debug ("%s: undersized (%u B) %s",
+                  __func__, lsalen, LOOKUP (ospf_lsa_type_msg, lsah->type));
+    return MSG_NG;
+  }
+  switch (lsah->type)
+  {
+  case OSPF_ROUTER_LSA:
+    /* RFC2328 A.4.2, LSA header + 4 bytes followed by N>=1 (12+)-byte link blocks */
+    if (headeronly)
+    {
+      ret = (lsalen - OSPF_LSA_HEADER_SIZE - OSPF_ROUTER_LSA_MIN_SIZE) % 4 ? MSG_NG : MSG_OK;
+      break;
+    }
+    rlsa = (struct router_lsa *) lsah;
+    ret = ospf_router_lsa_links_examin
+    (
+      (struct router_lsa_link *) rlsa->link,
+      lsalen - OSPF_LSA_HEADER_SIZE - 4, /* skip: basic header, "flags", 0, "# links" */
+      ntohs (rlsa->links) /* 16 bits */
+    );
+    break;
+  case OSPF_AS_EXTERNAL_LSA:
+    /* RFC2328 A.4.5, LSA header + 4 bytes followed by N>=1 12-bytes long blocks */
+  case OSPF_AS_NSSA_LSA:
+    /* RFC3101 C, idem */
+    ret = (lsalen - OSPF_LSA_HEADER_SIZE - OSPF_AS_EXTERNAL_LSA_MIN_SIZE) % 12 ? MSG_NG : MSG_OK;
+    break;
+  /* Following LSA types are considered OK length-wise as soon as their minimum
+   * length constraint is met and length of the whole LSA is a multiple of 4
+   * (basic LSA header size is already a multiple of 4). */
+  case OSPF_NETWORK_LSA:
+    /* RFC2328 A.4.3, LSA header + 4 bytes followed by N>=1 router-IDs */
+  case OSPF_SUMMARY_LSA:
+  case OSPF_ASBR_SUMMARY_LSA:
+    /* RFC2328 A.4.4, LSA header + 4 bytes followed by N>=1 4-bytes TOS blocks */
+#ifdef HAVE_OPAQUE_LSA
+  case OSPF_OPAQUE_LINK_LSA:
+  case OSPF_OPAQUE_AREA_LSA:
+  case OSPF_OPAQUE_AS_LSA:
+    /* RFC5250 A.2, "some number of octets (of application-specific
+     * data) padded to 32-bit alignment." This is considered equivalent
+     * to 4-byte alignment of all other LSA types, see OSPF-ALIGNMENT.txt
+     * file for the detailed analysis of this passage. */
+#endif
+    ret = lsalen % 4 ? MSG_NG : MSG_OK;
+    break;
+  default:
+    if (IS_DEBUG_OSPF_PACKET (0, RECV))
+      zlog_debug ("%s: unsupported LSA type 0x%02x", __func__, lsah->type);
+    return MSG_NG;
+  }
+  if (ret != MSG_OK && IS_DEBUG_OSPF_PACKET (0, RECV))
+    zlog_debug ("%s: alignment error in %s",
+                __func__, LOOKUP (ospf_lsa_type_msg, lsah->type));
+  return ret;
+}
+
+/* Verify if the provided input buffer is a valid sequence of LSAs. This
+   includes verification of LSA blocks length/alignment and dispatching
+   of deeper-level checks. */
+static unsigned
+ospf_lsaseq_examin
+(
+  struct lsa_header *lsah, /* start of buffered data */
+  size_t length,
+  const u_char headeronly,
+  /* When declared_num_lsas is not 0, compare it to the real number of LSAs
+     and treat the difference as an error. */
+  const u_int32_t declared_num_lsas
+)
+{
+  u_int32_t counted_lsas = 0;
+
+  while (length)
+  {
+    u_int16_t lsalen;
+    if (length < OSPF_LSA_HEADER_SIZE)
+    {
+      if (IS_DEBUG_OSPF_PACKET (0, RECV))
+        zlog_debug ("%s: undersized (%zu B) trailing (#%u) LSA header",
+                    __func__, length, counted_lsas);
+      return MSG_NG;
+    }
+    /* save on ntohs() calls here and in the LSA validator */
+    lsalen = ntohs (lsah->length);
+    if (lsalen < OSPF_LSA_HEADER_SIZE)
+    {
+      if (IS_DEBUG_OSPF_PACKET (0, RECV))
+        zlog_debug ("%s: malformed LSA header #%u, declared length is %u B",
+                    __func__, counted_lsas, lsalen);
+      return MSG_NG;
+    }
+    if (headeronly)
+    {
+      /* less checks here and in ospf_lsa_examin() */
+      if (MSG_OK != ospf_lsa_examin (lsah, lsalen, 1))
+      {
+        if (IS_DEBUG_OSPF_PACKET (0, RECV))
+          zlog_debug ("%s: malformed header-only LSA #%u", __func__, counted_lsas);
+        return MSG_NG;
+      }
+      lsah = (struct lsa_header *) ((caddr_t) lsah + OSPF_LSA_HEADER_SIZE);
+      length -= OSPF_LSA_HEADER_SIZE;
+    }
+    else
+    {
+      /* make sure the input buffer is deep enough before further checks */
+      if (lsalen > length)
+      {
+        if (IS_DEBUG_OSPF_PACKET (0, RECV))
+          zlog_debug ("%s: anomaly in LSA #%u: declared length is %u B, buffered length is %zu B",
+                      __func__, counted_lsas, lsalen, length);
+        return MSG_NG;
+      }
+      if (MSG_OK != ospf_lsa_examin (lsah, lsalen, 0))
+      {
+        if (IS_DEBUG_OSPF_PACKET (0, RECV))
+          zlog_debug ("%s: malformed LSA #%u", __func__, counted_lsas);
+        return MSG_NG;
+      }
+      lsah = (struct lsa_header *) ((caddr_t) lsah + lsalen);
+      length -= lsalen;
+    }
+    counted_lsas++;
+  }
+
+  if (declared_num_lsas && counted_lsas != declared_num_lsas)
+  {
+    if (IS_DEBUG_OSPF_PACKET (0, RECV))
+      zlog_debug ("%s: #LSAs declared (%u) does not match actual (%u)",
+                  __func__, declared_num_lsas, counted_lsas);
+    return MSG_NG;
+  }
+  return MSG_OK;
+}
+
 /* Verify a complete OSPF packet for proper sizing/alignment. */
 static unsigned
 ospf_packet_examin (struct ospf_header * oh, const unsigned bytesonwire)
 {
   u_int16_t bytesdeclared;
+  unsigned ret;
+  struct ospf_ls_update * lsupd;
 
   /* Length, 1st approximation. */
   if (bytesonwire < OSPF_HEADER_SIZE)
@@ -2353,7 +2559,59 @@ ospf_packet_examin (struct ospf_header * oh, const unsigned bytesonwire)
                   bytesdeclared, LOOKUP (ospf_packet_type_str, oh->type));
     return MSG_NG;
   }
-  return MSG_OK;
+  switch (oh->type)
+  {
+  case OSPF_MSG_HELLO:
+    /* RFC2328 A.3.2, packet header + OSPF_HELLO_MIN_SIZE bytes followed
+       by N>=0 router-IDs. */
+    ret = (bytesonwire - OSPF_HEADER_SIZE - OSPF_HELLO_MIN_SIZE) % 4 ? MSG_NG : MSG_OK;
+    break;
+  case OSPF_MSG_DB_DESC:
+    /* RFC2328 A.3.3, packet header + OSPF_DB_DESC_MIN_SIZE bytes followed
+       by N>=0 header-only LSAs. */
+    ret = ospf_lsaseq_examin
+    (
+      (struct lsa_header *) ((caddr_t) oh + OSPF_HEADER_SIZE + OSPF_DB_DESC_MIN_SIZE),
+      bytesonwire - OSPF_HEADER_SIZE - OSPF_DB_DESC_MIN_SIZE,
+      1, /* header-only LSAs */
+      0
+    );
+    break;
+  case OSPF_MSG_LS_REQ:
+    /* RFC2328 A.3.4, packet header followed by N>=0 12-bytes request blocks. */
+    ret = (bytesonwire - OSPF_HEADER_SIZE - OSPF_LS_REQ_MIN_SIZE) %
+      OSPF_LSA_KEY_SIZE ? MSG_NG : MSG_OK;
+    break;
+  case OSPF_MSG_LS_UPD:
+    /* RFC2328 A.3.5, packet header + OSPF_LS_UPD_MIN_SIZE bytes followed
+       by N>=0 full LSAs (with N declared beforehand). */
+    lsupd = (struct ospf_ls_update *) ((caddr_t) oh + OSPF_HEADER_SIZE);
+    ret = ospf_lsaseq_examin
+    (
+      (struct lsa_header *) ((caddr_t) lsupd + OSPF_LS_UPD_MIN_SIZE),
+      bytesonwire - OSPF_HEADER_SIZE - OSPF_LS_UPD_MIN_SIZE,
+      0, /* full LSAs */
+      ntohl (lsupd->num_lsas) /* 32 bits */
+    );
+    break;
+  case OSPF_MSG_LS_ACK:
+    /* RFC2328 A.3.6, packet header followed by N>=0 header-only LSAs. */
+    ret = ospf_lsaseq_examin
+    (
+      (struct lsa_header *) ((caddr_t) oh + OSPF_HEADER_SIZE + OSPF_LS_ACK_MIN_SIZE),
+      bytesonwire - OSPF_HEADER_SIZE - OSPF_LS_ACK_MIN_SIZE,
+      1, /* header-only LSAs */
+      0
+    );
+    break;
+  default:
+    if (IS_DEBUG_OSPF_PACKET (0, RECV))
+      zlog_debug ("%s: invalid packet type 0x%02x", __func__, oh->type);
+    return MSG_NG;
+  }
+  if (ret != MSG_OK && IS_DEBUG_OSPF_PACKET (0, RECV))
+    zlog_debug ("%s: malformed %s packet", __func__, LOOKUP (ospf_packet_type_str, oh->type));
+  return ret;
 }
 
 /* OSPF Header verification. */
-- 
cgit v1.2.1


From b29adf9c3e69f298f748564a20abdf7274bbc549 Mon Sep 17 00:00:00 2001
From: Denis Ovsienko <infrastation@yandex.ru>
Date: Mon, 20 Feb 2012 23:08:10 +0400
Subject: ospfd: fix packet length check for auth/LLS cases

An OSPFv2 packet with trailing data blocks (authentication and/or
link-local signaling) failed the recently implemented packet length
check, because trailing data length isn't counted in the packet header
"length" field. This commit fixes respective check conditions.

* ospf_packet.c
  * ospf_packet_examin(): use "bytesdeclared" instead of "bytesonwire"
---
 ospfd/ospf_packet.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

(limited to 'ospfd/ospf_packet.c')

diff --git a/ospfd/ospf_packet.c b/ospfd/ospf_packet.c
index 68c25790..3d827296 100644
--- a/ospfd/ospf_packet.c
+++ b/ospfd/ospf_packet.c
@@ -2564,7 +2564,7 @@ ospf_packet_examin (struct ospf_header * oh, const unsigned bytesonwire)
   case OSPF_MSG_HELLO:
     /* RFC2328 A.3.2, packet header + OSPF_HELLO_MIN_SIZE bytes followed
        by N>=0 router-IDs. */
-    ret = (bytesonwire - OSPF_HEADER_SIZE - OSPF_HELLO_MIN_SIZE) % 4 ? MSG_NG : MSG_OK;
+    ret = (bytesdeclared - OSPF_HEADER_SIZE - OSPF_HELLO_MIN_SIZE) % 4 ? MSG_NG : MSG_OK;
     break;
   case OSPF_MSG_DB_DESC:
     /* RFC2328 A.3.3, packet header + OSPF_DB_DESC_MIN_SIZE bytes followed
@@ -2572,14 +2572,14 @@ ospf_packet_examin (struct ospf_header * oh, const unsigned bytesonwire)
     ret = ospf_lsaseq_examin
     (
       (struct lsa_header *) ((caddr_t) oh + OSPF_HEADER_SIZE + OSPF_DB_DESC_MIN_SIZE),
-      bytesonwire - OSPF_HEADER_SIZE - OSPF_DB_DESC_MIN_SIZE,
+      bytesdeclared - OSPF_HEADER_SIZE - OSPF_DB_DESC_MIN_SIZE,
       1, /* header-only LSAs */
       0
     );
     break;
   case OSPF_MSG_LS_REQ:
     /* RFC2328 A.3.4, packet header followed by N>=0 12-bytes request blocks. */
-    ret = (bytesonwire - OSPF_HEADER_SIZE - OSPF_LS_REQ_MIN_SIZE) %
+    ret = (bytesdeclared - OSPF_HEADER_SIZE - OSPF_LS_REQ_MIN_SIZE) %
       OSPF_LSA_KEY_SIZE ? MSG_NG : MSG_OK;
     break;
   case OSPF_MSG_LS_UPD:
@@ -2589,7 +2589,7 @@ ospf_packet_examin (struct ospf_header * oh, const unsigned bytesonwire)
     ret = ospf_lsaseq_examin
     (
       (struct lsa_header *) ((caddr_t) lsupd + OSPF_LS_UPD_MIN_SIZE),
-      bytesonwire - OSPF_HEADER_SIZE - OSPF_LS_UPD_MIN_SIZE,
+      bytesdeclared - OSPF_HEADER_SIZE - OSPF_LS_UPD_MIN_SIZE,
       0, /* full LSAs */
       ntohl (lsupd->num_lsas) /* 32 bits */
     );
@@ -2599,7 +2599,7 @@ ospf_packet_examin (struct ospf_header * oh, const unsigned bytesonwire)
     ret = ospf_lsaseq_examin
     (
       (struct lsa_header *) ((caddr_t) oh + OSPF_HEADER_SIZE + OSPF_LS_ACK_MIN_SIZE),
-      bytesonwire - OSPF_HEADER_SIZE - OSPF_LS_ACK_MIN_SIZE,
+      bytesdeclared - OSPF_HEADER_SIZE - OSPF_LS_ACK_MIN_SIZE,
       1, /* header-only LSAs */
       0
     );
-- 
cgit v1.2.1


From bd5651f0ec7aa94627a2a6868dd458f016750a35 Mon Sep 17 00:00:00 2001
From: Denis Ovsienko <infrastation@yandex.ru>
Date: Sun, 26 Feb 2012 17:59:43 +0400
Subject: ospfd: bring ospf_check_auth() into focus

The old ospf_check_auth() function did two different jobs depending on
AuType. For Null and Simple cases it actually authenticated the packet,
but for Cryptographic case it only checked declared packet size (not
taking the actual number of bytes on wire into account). The calling
function, ospf_verify_header(), had its own set of MD5/checksum checks
dispatched depending on AuType.

This commit makes the packet size check work against the real number of
bytes and moves it to ospf_packet_examine(). All MD5/checksum
verification is now performed in ospf_check_auth() function.

* ospf_packet.c
  * ospf_packet_examin(): check length with MD5 bytes in mind
  * ospf_verify_header(): remove all AuType-specific code
  * ospf_check_auth(): completely rewrite
---
 ospfd/ospf_packet.c | 170 ++++++++++++++++++++++++++++++----------------------
 1 file changed, 100 insertions(+), 70 deletions(-)

(limited to 'ospfd/ospf_packet.c')

diff --git a/ospfd/ospf_packet.c b/ospfd/ospf_packet.c
index 3d827296..4842c4e2 100644
--- a/ospfd/ospf_packet.c
+++ b/ospfd/ospf_packet.c
@@ -91,6 +91,9 @@ static const u_int16_t ospf_lsa_minlen[] =
   0,
 };
 
+/* for ospf_check_auth() */
+static int ospf_check_sum (struct ospf_header *);
+
 /* OSPF authentication checking function */
 static int
 ospf_auth_type (struct ospf_interface *oi)
@@ -2267,44 +2270,91 @@ ospf_check_network_mask (struct ospf_interface *oi, struct in_addr ip_src)
  return 0;
 }
 
+/* Return 1, if the packet is properly authenticated and checksummed,
+   0 otherwise. In particular, check that AuType header field is valid and
+   matches the locally configured AuType, and that D.5 requirements are met. */
 static int
 ospf_check_auth (struct ospf_interface *oi, struct ospf_header *ospfh)
 {
-  int ret = 0;
   struct crypt_key *ck;
+  u_int16_t iface_auth_type;
+  u_int16_t pkt_auth_type = ntohs (ospfh->auth_type);
 
-  switch (ntohs (ospfh->auth_type))
+  switch (pkt_auth_type)
+  {
+  case OSPF_AUTH_NULL: /* RFC2328 D.5.1 */
+    if (OSPF_AUTH_NULL != (iface_auth_type = ospf_auth_type (oi)))
     {
-    case OSPF_AUTH_NULL:
-      ret = 1;
-      break;
-    case OSPF_AUTH_SIMPLE:
-      if (!memcmp (OSPF_IF_PARAM (oi, auth_simple), ospfh->u.auth_data, OSPF_AUTH_SIMPLE_SIZE))
-	ret = 1;
-      else
-	ret = 0;
-      break;
-    case OSPF_AUTH_CRYPTOGRAPHIC:
-      if ((ck = listgetdata (listtail(OSPF_IF_PARAM (oi,auth_crypt)))) == NULL)
-	{
-	  ret = 0;
-	  break;
-	}
-      
-      /* This is very basic, the digest processing is elsewhere */
-      if (ospfh->u.crypt.auth_data_len == OSPF_AUTH_MD5_SIZE && 
-          ospfh->u.crypt.key_id == ck->key_id &&
-          ntohs (ospfh->length) + OSPF_AUTH_MD5_SIZE <= OSPF_MAX_PACKET_SIZE)
-        ret = 1;
-      else
-        ret = 0;
-      break;
-    default:
-      ret = 0;
-      break;
+      if (IS_DEBUG_OSPF_PACKET (ospfh->type - 1, RECV))
+        zlog_warn ("interface %s: auth-type mismatch, local %s, rcvd Null",
+                   IF_NAME (oi), LOOKUP (ospf_auth_type_str, iface_auth_type));
+      return 0;
     }
-
-  return ret;
+    if (! ospf_check_sum (ospfh))
+    {
+      if (IS_DEBUG_OSPF_PACKET (ospfh->type - 1, RECV))
+        zlog_warn ("interface %s: Null auth OK, but checksum error, Router-ID %s",
+                   IF_NAME (oi), inet_ntoa (ospfh->router_id));
+      return 0;
+    }
+    return 1;
+  case OSPF_AUTH_SIMPLE: /* RFC2328 D.5.2 */
+    if (OSPF_AUTH_SIMPLE != (iface_auth_type = ospf_auth_type (oi)))
+    {
+      if (IS_DEBUG_OSPF_PACKET (ospfh->type - 1, RECV))
+        zlog_warn ("interface %s: auth-type mismatch, local %s, rcvd Simple",
+                   IF_NAME (oi), LOOKUP (ospf_auth_type_str, iface_auth_type));
+      return 0;
+    }
+    if (memcmp (OSPF_IF_PARAM (oi, auth_simple), ospfh->u.auth_data, OSPF_AUTH_SIMPLE_SIZE))
+    {
+      if (IS_DEBUG_OSPF_PACKET (ospfh->type - 1, RECV))
+        zlog_warn ("interface %s: Simple auth failed", IF_NAME (oi));
+      return 0;
+    }
+    if (! ospf_check_sum (ospfh))
+    {
+      if (IS_DEBUG_OSPF_PACKET (ospfh->type - 1, RECV))
+        zlog_warn ("interface %s: Simple auth OK, checksum error, Router-ID %s",
+                   IF_NAME (oi), inet_ntoa (ospfh->router_id));
+      return 0;
+    }
+    return 1;
+  case OSPF_AUTH_CRYPTOGRAPHIC: /* RFC2328 D.5.3 */
+    if (OSPF_AUTH_CRYPTOGRAPHIC != (iface_auth_type = ospf_auth_type (oi)))
+    {
+      if (IS_DEBUG_OSPF_PACKET (ospfh->type - 1, RECV))
+        zlog_warn ("interface %s: auth-type mismatch, local %s, rcvd Cryptographic",
+                   IF_NAME (oi), LOOKUP (ospf_auth_type_str, iface_auth_type));
+      return 0;
+    }
+    if (ospfh->checksum)
+    {
+      if (IS_DEBUG_OSPF_PACKET (ospfh->type - 1, RECV))
+        zlog_warn ("interface %s: OSPF header checksum is not 0", IF_NAME (oi));
+      return 0;
+    }
+    /* only MD5 crypto method can pass ospf_packet_examin() */
+    if
+    (
+      NULL == (ck = listgetdata (listtail(OSPF_IF_PARAM (oi,auth_crypt)))) ||
+      ospfh->u.crypt.key_id != ck->key_id ||
+      /* Condition above uses the last key ID on the list, which is
+         different from what ospf_crypt_key_lookup() does. A bug? */
+      ! ospf_check_md5_digest (oi, ospfh)
+    )
+    {
+      if (IS_DEBUG_OSPF_PACKET (ospfh->type - 1, RECV))
+        zlog_warn ("interface %s: MD5 auth failed", IF_NAME (oi));
+      return 0;
+    }
+    return 1;
+  default:
+    if (IS_DEBUG_OSPF_PACKET (ospfh->type - 1, RECV))
+      zlog_warn ("interface %s: invalid packet auth-type (%02x)",
+                 IF_NAME (oi), pkt_auth_type);
+    return 0;
+  }
 }
 
 static int
@@ -2523,7 +2573,7 @@ ospf_lsaseq_examin
 static unsigned
 ospf_packet_examin (struct ospf_header * oh, const unsigned bytesonwire)
 {
-  u_int16_t bytesdeclared;
+  u_int16_t bytesdeclared, bytesauth;
   unsigned ret;
   struct ospf_ls_update * lsupd;
 
@@ -2538,11 +2588,24 @@ ospf_packet_examin (struct ospf_header * oh, const unsigned bytesonwire)
    * for possible extra bytes of crypto auth/padding, which are not counted
    * in the OSPF header "length" field. */
   bytesdeclared = ntohs (oh->length);
-  if (bytesdeclared > bytesonwire)
+  if (ntohs (oh->auth_type) != OSPF_AUTH_CRYPTOGRAPHIC)
+    bytesauth = 0;
+  else
+  {
+    if (oh->u.crypt.auth_data_len != OSPF_AUTH_MD5_SIZE)
+    {
+      if (IS_DEBUG_OSPF_PACKET (0, RECV))
+        zlog_debug ("%s: unsupported crypto auth length (%u B)",
+                    __func__, oh->u.crypt.auth_data_len);
+      return MSG_NG;
+    }
+    bytesauth = OSPF_AUTH_MD5_SIZE;
+  }
+  if (bytesdeclared + bytesauth > bytesonwire)
   {
     if (IS_DEBUG_OSPF_PACKET (0, RECV))
-      zlog_debug ("%s: packet length error (%u real, %u declared)",
-                  __func__, bytesonwire, bytesdeclared);
+      zlog_debug ("%s: packet length error (%u real, %u+%u declared)",
+                  __func__, bytesonwire, bytesdeclared, bytesauth);
     return MSG_NG;
   }
   /* Length, 2nd approximation. The type-specific constraint is checked
@@ -2650,42 +2713,9 @@ ospf_verify_header (struct stream *ibuf, struct ospf_interface *oi,
       return -1;
     }
 
-  /* Check authentication. */
-  if (ospf_auth_type (oi) != ntohs (ospfh->auth_type))
-    {
-      zlog_warn ("interface %s: auth-type mismatch, local %d, rcvd %d",
-		 IF_NAME (oi), ospf_auth_type (oi), ntohs (ospfh->auth_type));
-      return -1;
-    }
-
+  /* Check authentication. The function handles logging actions, where required. */
   if (! ospf_check_auth (oi, ospfh))
-    {
-      zlog_warn ("interface %s: ospf_read authentication failed.",
-		 IF_NAME (oi));
-      return -1;
-    }
-
-  /* if check sum is invalid, packet is discarded. */
-  if (ntohs (ospfh->auth_type) != OSPF_AUTH_CRYPTOGRAPHIC)
-    {
-      if (! ospf_check_sum (ospfh))
-	{
-	  zlog_warn ("interface %s: ospf_read packet checksum error %s",
-		     IF_NAME (oi), inet_ntoa (ospfh->router_id));
-	  return -1;
-	}
-    }
-  else
-    {
-      if (ospfh->checksum != 0)
-	return -1;
-      if (ospf_check_md5_digest (oi, ospfh) == 0)
-	{
-	  zlog_warn ("interface %s: ospf_read md5 authentication failed.",
-		     IF_NAME (oi));
-	  return -1;
-	}
-    }
+    return -1;
 
   return 0;
 }
-- 
cgit v1.2.1


From aee567450eaf32877d00f47c4cc5d05c5fb85a51 Mon Sep 17 00:00:00 2001
From: Denis Ovsienko <infrastation@yandex.ru>
Date: Tue, 28 Feb 2012 15:15:29 +0400
Subject: ospfd: reduce ospf_verify_header()

Protocol version checks fits ospf_packet_examin() better (like it is
implemented in ospf6d), and packet type check is already there.
---
 ospfd/ospf_packet.c | 21 ++++++---------------
 1 file changed, 6 insertions(+), 15 deletions(-)

(limited to 'ospfd/ospf_packet.c')

diff --git a/ospfd/ospf_packet.c b/ospfd/ospf_packet.c
index 4842c4e2..351fb210 100644
--- a/ospfd/ospf_packet.c
+++ b/ospfd/ospf_packet.c
@@ -2587,6 +2587,12 @@ ospf_packet_examin (struct ospf_header * oh, const unsigned bytesonwire)
   /* Now it is safe to access header fields. Performing length check, allow
    * for possible extra bytes of crypto auth/padding, which are not counted
    * in the OSPF header "length" field. */
+  if (oh->version != OSPF_VERSION)
+  {
+    if (IS_DEBUG_OSPF_PACKET (0, RECV))
+      zlog_debug ("%s: invalid (%u) protocol version", __func__, oh->version);
+    return MSG_NG;
+  }
   bytesdeclared = ntohs (oh->length);
   if (ntohs (oh->auth_type) != OSPF_AUTH_CRYPTOGRAPHIC)
     bytesauth = 0;
@@ -2682,21 +2688,6 @@ static int
 ospf_verify_header (struct stream *ibuf, struct ospf_interface *oi,
 		    struct ip *iph, struct ospf_header *ospfh)
 {
-  /* check version. */
-  if (ospfh->version != OSPF_VERSION)
-    {
-      zlog_warn ("interface %s: ospf_read version number mismatch.",
-		 IF_NAME (oi));
-      return -1;
-    }
-
-  /* Valid OSPFv2 packet types are 1 through 5 inclusive. */
-  if (ospfh->type < 1 || ospfh->type > 5)
-  {
-    zlog_warn ("interface %s: invalid packet type %u", IF_NAME (oi), ospfh->type);
-    return -1;
-  }
-
   /* Check Area ID. */
   if (!ospf_check_area_id (oi, ospfh))
     {
-- 
cgit v1.2.1