From 8cb3d80332d24ab9b92791893329fe7a3c542ed8 Mon Sep 17 00:00:00 2001
From: Quentin Young <qlyoung@cumulusnetworks.com>
Date: Wed, 6 Feb 2019 16:49:19 +0000
Subject: [PATCH] vrrpd: specify version when parsing vrrp packet

Move a bit more validation into vrrp_packet.c

Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
---
 vrrpd/vrrp.c        |  4 ++--
 vrrpd/vrrp_packet.c | 16 +++++++++-------
 vrrpd/vrrp_packet.h | 21 +++++++++++++++------
 3 files changed, 26 insertions(+), 15 deletions(-)

diff --git a/vrrpd/vrrp.c b/vrrpd/vrrp.c
index a03c675010..265cce33fb 100644
--- a/vrrpd/vrrp.c
+++ b/vrrpd/vrrp.c
@@ -654,8 +654,8 @@ static int vrrp_read(struct thread *thread)
 		   r->vr->vrid, family2str(r->family));
 	zlog_hexdump(r->ibuf, nbytes);
 
-	pktsize = vrrp_pkt_parse_datagram(r->family, &m, nbytes, &src, &pkt,
-					  errbuf, sizeof(errbuf));
+	pktsize = vrrp_pkt_parse_datagram(r->family, r->vr->version, &m, nbytes,
+					  &src, &pkt, errbuf, sizeof(errbuf));
 
 	if (pktsize < 0) {
 		zlog_warn(VRRP_LOGPFX VRRP_LOGPFX_VRID
diff --git a/vrrpd/vrrp_packet.c b/vrrpd/vrrp_packet.c
index fb72d921eb..0a569f6369 100644
--- a/vrrpd/vrrp_packet.c
+++ b/vrrpd/vrrp_packet.c
@@ -171,9 +171,10 @@ size_t vrrp_pkt_adver_dump(char *buf, size_t buflen, struct vrrp_pkt *pkt)
 	return rs;
 }
 
-ssize_t vrrp_pkt_parse_datagram(int family, struct msghdr *m, size_t read,
-				struct ipaddr *src, struct vrrp_pkt **pkt,
-				char *errmsg, size_t errmsg_len)
+ssize_t vrrp_pkt_parse_datagram(int family, int version, struct msghdr *m,
+				size_t read, struct ipaddr *src,
+				struct vrrp_pkt **pkt, char *errmsg,
+				size_t errmsg_len)
 {
 	/* Source (MAC & IP), Dest (MAC & IP) TTL validation done by kernel */
 	size_t addrsz = (family == AF_INET) ? sizeof(struct in_addr)
@@ -260,19 +261,20 @@ ssize_t vrrp_pkt_parse_datagram(int family, struct msghdr *m, size_t read,
 			"VRRP packet is oversized (%lu > %lu)", pktsize,
 			VRRP_MAX_PKT_SIZE);
 
+	/* Version check */
+	uint8_t pktver = (*pkt)->hdr.vertype >> 4;
+	VRRP_PKT_VCHECK(pktver == version, "Bad version %u", pktver);
+
 	/* Checksum check */
 	uint16_t chksum = vrrp_pkt_checksum(*pkt, pktsize, src);
 	VRRP_PKT_VCHECK((*pkt)->hdr.chksum == chksum,
 			"Bad VRRP checksum %" PRIu16 "; should be %" PRIu16 "",
 			(*pkt)->hdr.chksum, chksum);
 
-	/* Version check */
-	uint8_t version = (*pkt)->hdr.vertype >> 4;
-	VRRP_PKT_VCHECK(version == 3 || version == 2, "Bad version %u",
-			version);
 	/* Type check */
 	VRRP_PKT_VCHECK(((*pkt)->hdr.vertype & 0x0F) == 1, "Bad type %u",
 			(*pkt)->hdr.vertype & 0x0f);
+
 	/* # addresses check */
 	size_t ves = VRRP_PKT_SIZE(family, (*pkt)->hdr.naddr);
 	VRRP_PKT_VCHECK(pktsize == ves, "Packet has incorrect # addresses");
diff --git a/vrrpd/vrrp_packet.h b/vrrpd/vrrp_packet.h
index af51287718..2061b63c61 100644
--- a/vrrpd/vrrp_packet.h
+++ b/vrrpd/vrrp_packet.h
@@ -153,17 +153,25 @@ size_t vrrp_pkt_adver_dump(char *buf, size_t buflen, struct vrrp_pkt *pkt);
 /*
  * Parses a VRRP packet, checking for illegal or invalid data.
  *
- * This function does not check that the local router is not the IPvX owner for
- * the addresses received; that should be done by the caller.
+ * This function parses both VRRPv2 and VRRPv3 packets. Which version is
+ * expected is determined by the version argument. For example, if version is 3
+ * and the received packet has version field 2 it will fail to parse.
+ *
+ * Note that this function only checks whether the packet itself is a valid
+ * VRRP packet. It is up to the caller to validate whether the VRID is correct,
+ * priority and timer values are correct, etc.
  *
  * family
  *    Address family of received packet
  *
+ * version
+ *   VRRP version to use for validation
+ *
  * m
  *    msghdr containing results of recvmsg() on VRRP router socket
  *
  * read
- *    return value of recvmsg() on VRRP router socket; must be non-negative
+ *    Return value of recvmsg() on VRRP router socket; must be non-negative
  *
  * src
  *    Pointer to struct ipaddr to store address of datagram sender
@@ -181,8 +189,9 @@ size_t vrrp_pkt_adver_dump(char *buf, size_t buflen, struct vrrp_pkt *pkt);
  * Returns:
  *    Size of VRRP packet, or -1 upon error
  */
-ssize_t vrrp_pkt_parse_datagram(int family, struct msghdr *m, size_t read,
-				struct ipaddr *src, struct vrrp_pkt **pkt,
-				char *errmsg, size_t errmsg_len);
+ssize_t vrrp_pkt_parse_datagram(int family, int version, struct msghdr *m,
+				size_t read, struct ipaddr *src,
+				struct vrrp_pkt **pkt, char *errmsg,
+				size_t errmsg_len);
 
 #endif /* __VRRP_PACKET_H__ */
-- 
2.39.5