components/quagga/patches/07-cve-2012-0249.patch
author Mike Sullivan <Mike.Sullivan@Oracle.COM>
Wed, 29 Aug 2012 11:05:56 -0700
changeset 957 255465c5756f
parent 897 f239fb8865f3
permissions -rw-r--r--
Close of build 04.
The following patches are pulled directly from the GIT repository 
for the quagga community. They fix the following CVEs:

CVE-2012-0249
CVE-2012-0250
CVE-2012-0255

All of the patched CVEs are included in Quagga 0.99.20.1. This patch
file can be removed if Quagga is upgraded to that version.



From 5861739f8c38bc36ea9955e5cb2be2bf2f482d70 Mon Sep 17 00:00:00 2001
From: Paul Jakma <[email protected]>
Date: Mon, 09 Jan 2012 20:59:26 +0000
Subject: bgpd: Open option parse errors don't NOTIFY, resulting in abort & DoS

* bgp_packet.c: (bgp_open_receive) Errors from bgp_open_option_parse are
  detected, and the code will stop processing the OPEN and return.  However
  it does so without calling bgp_notify_send to send a NOTIFY - which means
  the peer FSM doesn't get stopped, and bgp_read will be called again later.
  Because it returns, it doesn't go through the code near the end of the
  function that removes the current message from the peer input streaam.
  Thus the next call to bgp_read will try to parse a half-parsed stream as
  if it were a new BGP message, leading to an assert later in the code when
  it tries to read stuff that isn't there. Add the required call to
  bgp_notify_send before returning.
* bgp_open.c: (bgp_capability_as4) Be a bit stricter, check the length field
  corresponds to the only value it can be, which is the amount we're going to
  read off the stream. And make sure the capability flag gets set, so
  callers can know this capability was read, regardless.
  (peek_for_as4_capability) Let bgp_capability_as4 do the length check.
---
diff --git bgpd/bgp_open.c bgpd/bgp_open.c
index 82deb3d..b5b50bb 100644
--- bgpd/bgp_open.c
+++ bgpd/bgp_open.c
@@ -421,13 +421,20 @@ bgp_capability_restart (struct peer *peer, struct capability_header *caphdr)
 static as_t
 bgp_capability_as4 (struct peer *peer, struct capability_header *hdr)
 {
+  SET_FLAG (peer->cap, PEER_CAP_AS4_RCV);
+  
+  if (hdr->length != CAPABILITY_CODE_AS4_LEN)
+    {
+      zlog_err ("%s AS4 capability has incorrect data length %d",
+                peer->host, hdr->length);
+      return 0;
+    }
+  
   as_t as4 = stream_getl (BGP_INPUT(peer));
   
   if (BGP_DEBUG (as4, AS4))
     zlog_debug ("%s [AS4] about to set cap PEER_CAP_AS4_RCV, got as4 %u",
                 peer->host, as4);
-  SET_FLAG (peer->cap, PEER_CAP_AS4_RCV);
-  
   return as4;
 }
 
@@ -689,9 +696,6 @@ peek_for_as4_capability (struct peer *peer, u_char length)
 
 	      if (hdr.code == CAPABILITY_CODE_AS4)
 	        {
-	          if (hdr.length != CAPABILITY_CODE_AS4_LEN)
-	            goto end;
-                  
 	          if (BGP_DEBUG (as4, AS4))
 	            zlog_info ("[AS4] found AS4 capability, about to parse");
 	          as4 = bgp_capability_as4 (peer, &hdr);
diff --git bgpd/bgp_packet.c bgpd/bgp_packet.c
index f5a74d1..5d8087a 100644
--- bgpd/bgp_packet.c
+++ bgpd/bgp_packet.c
@@ -1459,9 +1459,13 @@ bgp_open_receive (struct peer *peer, bgp_size_t size)
   /* Open option part parse. */
   if (optlen != 0) 
     {
-      ret = bgp_open_option_parse (peer, optlen, &capability);
-      if (ret < 0)
-	return ret;
+      if ((ret = bgp_open_option_parse (peer, optlen, &capability)) < 0)
+        {
+          bgp_notify_send (peer,
+                 BGP_NOTIFY_OPEN_ERR,
+                 BGP_NOTIFY_OPEN_UNACEP_HOLDTIME);
+	  return ret;
+        }
     }
   else
     {
--
cgit v0.9.0.2
From 393b2d64dc0625ba8e01e9e1516efac06d13072e Mon Sep 17 00:00:00 2001
From: Denis Ovsienko <[email protected]>
Date: Sun, 15 Jan 2012 15:12:19 +0000
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
---
diff --git ospfd/ospf_dump.c ospfd/ospf_dump.c
index e65b2e3..8ace095 100644
--- ospfd/ospf_dump.c
+++ ospfd/ospf_dump.c
@@ -661,7 +661,7 @@ ospf_header_dump (struct ospf_header *ospfh)
   zlog_debug ("Header");
   zlog_debug ("  Version %d", ospfh->version);
   zlog_debug ("  Type %d (%s)", ospfh->type,
-	     ospf_packet_type_str[ospfh->type]);
+	     LOOKUP (ospf_packet_type_str, ospfh->type));
   zlog_debug ("  Packet Len %d", ntohs (ospfh->length));
   zlog_debug ("  Router ID %s", inet_ntoa (ospfh->router_id));
   zlog_debug ("  Area ID %s", inet_ntoa (ospfh->area_id));
@@ -1457,7 +1457,7 @@ DEFUN (show_debugging_ospf,
     if (IS_DEBUG_OSPF_PACKET (i, SEND) && IS_DEBUG_OSPF_PACKET (i, RECV))
       {
 	vty_out (vty, "  OSPF packet %s%s debugging is on%s",
-		 ospf_packet_type_str[i + 1],
+		 LOOKUP (ospf_packet_type_str, i + 1),
 		 IS_DEBUG_OSPF_PACKET (i, DETAIL) ? " detail" : "",
 		 VTY_NEWLINE);
       }
@@ -1465,12 +1465,12 @@ DEFUN (show_debugging_ospf,
       {
 	if (IS_DEBUG_OSPF_PACKET (i, SEND))
 	  vty_out (vty, "  OSPF packet %s send%s debugging is on%s",
-		   ospf_packet_type_str[i + 1],
+		   LOOKUP (ospf_packet_type_str, i + 1),
 		   IS_DEBUG_OSPF_PACKET (i, DETAIL) ? " detail" : "",
 		   VTY_NEWLINE);
 	if (IS_DEBUG_OSPF_PACKET (i, RECV))
 	  vty_out (vty, "  OSPF packet %s receive%s debugging is on%s",
-		   ospf_packet_type_str[i + 1],
+		   LOOKUP (ospf_packet_type_str, i + 1),
 		   IS_DEBUG_OSPF_PACKET (i, DETAIL) ? " detail" : "",
 		   VTY_NEWLINE);
       }
diff --git ospfd/ospf_dump.h ospfd/ospf_dump.h
index fb81371..455214f 100644
--- ospfd/ospf_dump.h
+++ ospfd/ospf_dump.h
@@ -121,7 +121,6 @@ extern unsigned long term_debug_ospf_zebra;
 extern unsigned long term_debug_ospf_nssa;
 
 /* Message Strings. */
-extern const char *ospf_packet_type_str[];
 extern char *ospf_lsa_type_str[];
 
 /* Prototypes. */
diff --git ospfd/ospf_packet.c ospfd/ospf_packet.c
index 0f338d3..03e6d2a 100644
--- ospfd/ospf_packet.c
+++ 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;
@@ -2571,7 +2572,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));
diff --git ospfd/ospf_packet.h ospfd/ospf_packet.h
index 9a47208..2115f11 100644
--- ospfd/ospf_packet.h
+++ ospfd/ospf_packet.h
@@ -163,4 +163,7 @@ extern int ospf_ls_ack_timer (struct thread *);
 extern int ospf_poll_timer (struct thread *);
 extern int ospf_hello_reply_timer (struct thread *);
 
+extern const struct message ospf_packet_type_str[];
+extern const size_t ospf_packet_type_str_max;
+
 #endif /* _ZEBRA_OSPF_PACKET_H */
--
cgit v0.9.0.2
From 099ed6744881e71957f2bfeebc4c0727714d2394 Mon Sep 17 00:00:00 2001
From: Denis Ovsienko <[email protected]>
Date: Fri, 20 Jan 2012 18:32:10 +0000
Subject: ospfd: fix ospf_packet_add_top() to use LOOKUP()

---
diff --git ospfd/ospf_packet.c ospfd/ospf_packet.c
index 03e6d2a..500f245 100644
--- ospfd/ospf_packet.c
+++ 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 v0.9.0.2
From 3092cd57fb44c8293995d013bd86937d1a91745f Mon Sep 17 00:00:00 2001
From: Denis Ovsienko <[email protected]>
Date: Mon, 30 Jan 2012 11:41:39 +0000
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
---
diff --git ospfd/ospf_packet.c ospfd/ospf_packet.c
index 500f245..f425da8 100644
--- ospfd/ospf_packet.c
+++ 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)
@@ -2309,6 +2321,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,
@@ -2404,10 +2457,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. */
@@ -2442,16 +2495,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);
diff --git ospfd/ospf_packet.h ospfd/ospf_packet.h
index 2115f11..3cbe889 100644
--- ospfd/ospf_packet.h
+++ ospfd/ospf_packet.h
@@ -46,6 +46,10 @@
 
 #define OSPF_HELLO_REPLY_DELAY          1
 
+/* Return values of functions involved in packet verification, see ospf6d. */
+#define MSG_OK    0
+#define MSG_NG    1
+
 struct ospf_packet
 {
   struct ospf_packet *next;
--
cgit v0.9.0.2
From 3779a3bf9d27b3cccda7e45223884257af362c28 Mon Sep 17 00:00:00 2001
From: Denis Ovsienko <[email protected]>
Date: Mon, 30 Jan 2012 12:07:18 +0000
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.
---
diff --git ospfd/ospf_packet.c ospfd/ospf_packet.c
index f425da8..a71cc99 100644
--- ospfd/ospf_packet.c
+++ ospfd/ospf_packet.c
@@ -2255,8 +2255,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;
@@ -2282,7 +2281,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;
@@ -2406,7 +2405,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 v0.9.0.2
From 7edfc01207f3eee8f26d5c22cfef7c7f030c52ce Mon Sep 17 00:00:00 2001
From: Denis Ovsienko <[email protected]>
Date: Mon, 30 Jan 2012 16:32:39 +0000
Subject: ospfd: review ospf_check_md5_digest()

Rewrite some pointer arithmetics without the additional variables and
move byte order conversion inside the function.
---
diff --git ospfd/ospf_packet.c ospfd/ospf_packet.c
index a71cc99..5704f9d 100644
--- ospfd/ospf_packet.c
+++ 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));
@@ -2426,7 +2416,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 v0.9.0.2
From fba10fc38f482c617fdfbb81b8d855df56eeda51 Mon Sep 17 00:00:00 2001
From: Denis Ovsienko <[email protected]>
Date: Fri, 17 Feb 2012 12:20:50 +0000
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
---
diff --git ospfd/ospf_lsa.h ospfd/ospf_lsa.h
index bf3b083..ca0653c 100644
--- ospfd/ospf_lsa.h
+++ ospfd/ospf_lsa.h
@@ -153,6 +153,7 @@ struct router_lsa_link
 };
 
 /* OSPF Router-LSAs structure. */
+#define OSPF_ROUTER_LSA_MIN_SIZE                  16U /* w/1 link descriptor */
 struct router_lsa
 {
   struct lsa_header header;
@@ -170,6 +171,7 @@ struct router_lsa
 };
 
 /* OSPF Network-LSAs structure. */
+#define OSPF_NETWORK_LSA_MIN_SIZE                  8U /* w/1 router-ID */
 struct network_lsa
 {
   struct lsa_header header;
@@ -178,6 +180,7 @@ struct network_lsa
 };
 
 /* OSPF Summary-LSAs structure. */
+#define OSPF_SUMMARY_LSA_MIN_SIZE                  8U /* w/1 TOS metric block */
 struct summary_lsa
 {
   struct lsa_header header;
@@ -187,6 +190,7 @@ struct summary_lsa
 };
 
 /* OSPF AS-external-LSAs structure. */
+#define OSPF_AS_EXTERNAL_LSA_MIN_SIZE             16U /* w/1 TOS forwarding block */
 struct as_external_lsa
 {
   struct lsa_header header;
diff --git ospfd/ospf_packet.c ospfd/ospf_packet.c
index 5704f9d..3b82820 100644
--- ospfd/ospf_packet.c
+++ 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)
@@ -2310,11 +2328,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)
@@ -2348,7 +2554,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. */
diff --git ospfd/ospf_packet.h ospfd/ospf_packet.h
index 3cbe889..337686a 100644
--- ospfd/ospf_packet.h
+++ ospfd/ospf_packet.h
@@ -121,6 +121,10 @@ struct ospf_db_desc
   u_int32_t dd_seqnum;
 };
 
+struct ospf_ls_update
+{
+  u_int32_t num_lsas;
+};
 
 /* Macros. */
 /* XXX Perhaps obsolete; function in ospf_packet.c */
--
cgit v0.9.0.2
From b03ae9f2d22acd8e3f97714a9c0df744676e344d Mon Sep 17 00:00:00 2001
From: Denis Ovsienko <[email protected]>
Date: Mon, 20 Feb 2012 19:08:10 +0000
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"
---
diff --git ospfd/ospf_packet.c ospfd/ospf_packet.c
index 3b82820..7b661a3 100644
--- ospfd/ospf_packet.c
+++ ospfd/ospf_packet.c
@@ -2559,7 +2559,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
@@ -2567,14 +2567,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:
@@ -2584,7 +2584,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 */
     );
@@ -2594,7 +2594,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 v0.9.0.2
From 1bdd96caefaa76883bece4d358a60dc890f1e375 Mon Sep 17 00:00:00 2001
From: Denis Ovsienko <[email protected]>
Date: Sun, 26 Feb 2012 13:00:57 +0000
Subject: ospfd: introduce ospf_auth_type_str[]

---
diff --git ospfd/ospf_dump.c ospfd/ospf_dump.c
index 8ace095..7e11e25 100644
--- ospfd/ospf_dump.c
+++ ospfd/ospf_dump.c
@@ -115,6 +115,16 @@ const struct message ospf_network_type_msg[] =
 };
 const int ospf_network_type_msg_max = OSPF_IFTYPE_MAX;
 
+/* AuType */
+const struct message ospf_auth_type_str[] =
+{
+  { OSPF_AUTH_NULL,          "Null"          },
+  { OSPF_AUTH_SIMPLE,        "Simple"        },
+  { OSPF_AUTH_CRYPTOGRAPHIC, "Cryptographic" },
+};
+const size_t ospf_auth_type_str_max = sizeof (ospf_auth_type_str) /
+  sizeof (ospf_auth_type_str[0]);
+
 /* Configuration debug option variables. */
 unsigned long conf_debug_ospf_packet[5] = {0, 0, 0, 0, 0};
 unsigned long conf_debug_ospf_event = 0;
@@ -657,6 +667,7 @@ static void
 ospf_header_dump (struct ospf_header *ospfh)
 {
   char buf[9];
+  u_int16_t auth_type = ntohs (ospfh->auth_type);
 
   zlog_debug ("Header");
   zlog_debug ("  Version %d", ospfh->version);
@@ -666,9 +677,9 @@ ospf_header_dump (struct ospf_header *ospfh)
   zlog_debug ("  Router ID %s", inet_ntoa (ospfh->router_id));
   zlog_debug ("  Area ID %s", inet_ntoa (ospfh->area_id));
   zlog_debug ("  Checksum 0x%x", ntohs (ospfh->checksum));
-  zlog_debug ("  AuType %d", ntohs (ospfh->auth_type));
+  zlog_debug ("  AuType %s", LOOKUP (ospf_auth_type_str, auth_type));
 
-  switch (ntohs (ospfh->auth_type))
+  switch (auth_type)
     {
     case OSPF_AUTH_NULL:
       break;
diff --git ospfd/ospf_dump.h ospfd/ospf_dump.h
index 455214f..a2d5e8b 100644
--- ospfd/ospf_dump.h
+++ ospfd/ospf_dump.h
@@ -122,6 +122,8 @@ extern unsigned long term_debug_ospf_nssa;
 
 /* Message Strings. */
 extern char *ospf_lsa_type_str[];
+extern const struct message ospf_auth_type_str[];
+extern const size_t ospf_auth_type_str_max;
 
 /* Prototypes. */
 extern const char *ospf_area_name_string (struct ospf_area *);
--
cgit v0.9.0.2
From e5fa148725fb2a3d1a8df12683f023ff9d65273f Mon Sep 17 00:00:00 2001
From: Denis Ovsienko <[email protected]>
Date: Sun, 26 Feb 2012 13:59:43 +0000
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
---
diff --git ospfd/ospf_packet.c ospfd/ospf_packet.c
index 7b661a3..05651d3 100644
--- ospfd/ospf_packet.c
+++ 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)
@@ -2262,44 +2265,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
@@ -2518,7 +2568,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;
 
@@ -2533,11 +2583,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
@@ -2645,42 +2708,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 v0.9.0.2
From a59c5401a2df169de2c780f13a4563548c04a2b7 Mon Sep 17 00:00:00 2001
From: Denis Ovsienko <[email protected]>
Date: Tue, 28 Feb 2012 11:15:29 +0000
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.
---
diff --git ospfd/ospf_packet.c ospfd/ospf_packet.c
index 05651d3..de14ccc 100644
--- ospfd/ospf_packet.c
+++ ospfd/ospf_packet.c
@@ -2582,6 +2582,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;
@@ -2677,21 +2683,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 v0.9.0.2