[PATCH 1/1] Correct OSPF checksum validation

[Chris Boot]

Prior to this patch, BIRD validates the OSPF LSA checksum by calculating
a new checksum and comparing it with the checksum in the header. Due to
the specifics of the Fletcher checksum used in OSPF, this is not
necessarily correct as the checkbytes in the header may be calculated
via a different means and end up with a different value that is
nonetheless still correct.

The documented means of validating the checksum as specified in RFC 905
B.4 is to calculate c0 and c1 from the unchanged contents of the packet,
which must result in a zero value to be considered valid.

Signed-off-by: Chris Boot <bootc at bootc.net>
---
 proto/ospf/lsalib.c | 28 +++++++++++++++++++++++-----
 proto/ospf/lsalib.h |  2 +-
 proto/ospf/lsupd.c  |  2 +-
 3 files changed, 25 insertions(+), 7 deletions(-)

diff --git a/proto/ospf/lsalib.c b/proto/ospf/lsalib.c
index 579d13e88770..ce6fb17813e1 100644
--- a/proto/ospf/lsalib.c
+++ b/proto/ospf/lsalib.c
@@ -205,7 +205,7 @@ lsasum_calculate(struct ospf_lsa_header *h, void *body)
   buf_dump("CALC", buf, length);
   */
 
-  (void) lsasum_check(h, body);
+  (void) lsasum_check(h, body, 1);
 
   //  log(L_WARN "Checksum result %4x", h->checksum);
 
@@ -214,11 +214,21 @@ lsasum_calculate(struct ospf_lsa_header *h, void *body)
 }
 
 /*
- * Note, that this function expects that LSA is in big endianity
- * It also returns value in big endian
+ * Calculates the Fletcher checksum of an OSPF LSA.
+ *
+ * If 'update' is non-zero, the checkbytes (X and Y in RFC905) are calculated
+ * and the checksum field in the header is updated. The return value is the
+ * checksum as placed in the header (in network byte order).
+ *
+ * If 'update' is zero, only C0 and C1 are calculated and the header is kept
+ * intact. The return value is a combination of C0 and C1; if the return value
+ * is exactly zero the checksum is considered valid, any non-zero value is
+ * invalid.
+ *
+ * Note that this function expects the input LSA to be in network byte order.
  */
 u16
-lsasum_check(struct ospf_lsa_header *h, void *body)
+lsasum_check(struct ospf_lsa_header *h, void *body, int update)
 {
   u8 *sp, *ep, *p, *q, *b;
   int c0 = 0, c1 = 0;
@@ -229,7 +239,7 @@ lsasum_check(struct ospf_lsa_header *h, void *body)
   sp = (char *) h;
   sp += 2; /* Skip Age field */
   length = ntohs(h->length) - 2;
-  h->checksum = 0;
+  if (update) h->checksum = 0;
 
   for (ep = sp + length; sp < ep; sp = q)
   {				/* Actually MODX is very large, do we need the for-cyclus? */
@@ -259,6 +269,14 @@ lsasum_check(struct ospf_lsa_header *h, void *body)
     c1 %= 255;
   }
 
+  if (!update) {
+    /*
+     * When testing the checksum, we don't need to calculate x and y. The
+     * checksum passes if c0 and c1 are both 0.
+     */
+    return (c0 << 8) | (c1 & 0xff);
+  }
+
   x = (int)((length - LSA_CHECKSUM_OFFSET) * c0 - c1) % 255;
   if (x <= 0)
     x += 255;
diff --git a/proto/ospf/lsalib.h b/proto/ospf/lsalib.h
index d9e1a610dbda..4ad770e84812 100644
--- a/proto/ospf/lsalib.h
+++ b/proto/ospf/lsalib.h
@@ -48,7 +48,7 @@ static inline u32 lsa_get_etype(struct ospf_lsa_header *h, struct ospf_proto *p)
 int lsa_flooding_allowed(u32 type, u32 domain, struct ospf_iface *ifa);
 
 void lsasum_calculate(struct ospf_lsa_header *header, void *body);
-u16 lsasum_check(struct ospf_lsa_header *h, void *body);
+u16 lsasum_check(struct ospf_lsa_header *h, void *body, int update);
 #define CMP_NEWER 1
 #define CMP_SAME 0
 #define CMP_OLDER -1
diff --git a/proto/ospf/lsupd.c b/proto/ospf/lsupd.c
index ea32923ad536..9e7dc64b359d 100644
--- a/proto/ospf/lsupd.c
+++ b/proto/ospf/lsupd.c
@@ -531,7 +531,7 @@ ospf_receive_lsupd(struct ospf_packet *pkt, struct ospf_iface *ifa,
 	lsa_type, lsa.id, lsa.rt, lsa.sn, lsa.age, lsa.checksum);
 
     /* RFC 2328 13. (1) - validate LSA checksum */
-    if (lsa_n->checksum != lsasum_check(lsa_n, NULL))
+    if (lsa_n->checksum == 0 || lsasum_check(lsa_n, NULL, 0) != 0)
       SKIP("invalid checksum");
 
     /* RFC 2328 13. (2) */
-- 
2.1.4