From: Roy Marples <roy@marples.name>
Date: Wed, 5 Feb 2020 13:29:45 +0000 (+0000)
Subject: BPF: Return the frame header with the data
X-Git-Tag: v8.1.7~19
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=984b9c34d4c1dab2271fa1cfe3c261fb5ff5caa3;p=thirdparty%2Fdhcpcd.git

BPF: Return the frame header with the data

For DHCP, we then just skip over the frame header.
For ARP, we extract the frame source and destination addresses
so we can log the source in the event of a conflict.

This is important as a user has found a router which sets the ARP
source and destination hardware addresses to all zeros but unicasts
the ARP straight to our hardware address.

https://serverfault.com/questions/297425/ip-address-conflict-with-mac-address-000000000000
https://discussions.flightaware.com/t/piaware-wont-reconnect-to-wifi-network-if-it-drops-off/59789
---

diff --git a/src/arp.c b/src/arp.c
index 0fdcdd9a..2460a1a3 100644
--- a/src/arp.c
+++ b/src/arp.c
@@ -115,7 +115,8 @@ static void
 arp_report_conflicted(const struct arp_state *astate,
     const struct arp_msg *amsg)
 {
-	char buf[HWADDR_LEN * 3];
+	char abuf[HWADDR_LEN * 3];
+	char fbuf[HWADDR_LEN * 3];
 
 	if (amsg == NULL) {
 		logerrx("%s: DAD detected %s",
@@ -123,9 +124,16 @@ arp_report_conflicted(const struct arp_state *astate,
 		return;
 	}
 
-	logerrx("%s: hardware address %s claims %s",
-	    astate->iface->name,
-	    hwaddr_ntoa(amsg->sha, astate->iface->hwlen, buf, sizeof(buf)),
+	hwaddr_ntoa(amsg->sha, astate->iface->hwlen, abuf, sizeof(abuf));
+	if (bpf_frame_header_len(astate->iface) == 0) {
+		logerrx("%s: %s claims %s",
+		    astate->iface->name, abuf, inet_ntoa(astate->addr));
+		return;
+	}
+
+	logerrx("%s: %s(%s) claims %s",
+	    astate->iface->name, abuf,
+	    hwaddr_ntoa(amsg->fsha, astate->iface->hwlen, fbuf, sizeof(fbuf)),
 	    inet_ntoa(astate->addr));
 }
 
@@ -209,6 +217,7 @@ arp_validate(const struct interface *ifp, struct arphdr *arp)
 static void
 arp_packet(struct interface *ifp, uint8_t *data, size_t len)
 {
+	size_t fl = bpf_frame_header_len(ifp), falen;
 	const struct interface *ifn;
 	struct arphdr ar;
 	struct arp_msg arm;
@@ -216,6 +225,19 @@ arp_packet(struct interface *ifp, uint8_t *data, size_t len)
 	struct arp_state *astate, *astaten;
 	uint8_t *hw_s, *hw_t;
 
+	/* Copy the frame header source and destination out */
+	memset(&arm, 0, sizeof(arm));
+	hw_s = bpf_frame_header_src(ifp, data, &falen);
+	if (hw_s != NULL && falen <= sizeof(arm.fsha))
+		memcpy(arm.fsha, hw_s, falen);
+	hw_t = bpf_frame_header_dst(ifp, data, &falen);
+	if (hw_t != NULL && falen <= sizeof(arm.ftha))
+		memcpy(arm.ftha, hw_t, falen);
+
+	/* Skip past the frame header */
+	data += fl;
+	len -= fl;
+
 	/* We must have a full ARP header */
 	if (len < sizeof(ar))
 		return;
diff --git a/src/arp.h b/src/arp.h
index 4842c357..7522bf28 100644
--- a/src/arp.h
+++ b/src/arp.h
@@ -54,10 +54,13 @@
 
 struct arp_msg {
 	uint16_t op;
-	unsigned char sha[HWADDR_LEN];
+	uint8_t sha[HWADDR_LEN];
 	struct in_addr sip;
-	unsigned char tha[HWADDR_LEN];
+	uint8_t tha[HWADDR_LEN];
 	struct in_addr tip;
+	/* Frame header and sender to diagnose failures */
+	uint8_t fsha[HWADDR_LEN];
+	uint8_t ftha[HWADDR_LEN];
 };
 
 struct arp_state {
diff --git a/src/bpf.c b/src/bpf.c
index 6c31bc2c..8a0a7e33 100644
--- a/src/bpf.c
+++ b/src/bpf.c
@@ -93,6 +93,38 @@ bpf_frame_header_len(const struct interface *ifp)
 	}
 }
 
+void *
+bpf_frame_header_src(const struct interface *ifp, void *fh, size_t *len)
+{
+	uint8_t *f = fh;
+
+	switch (ifp->family) {
+	case ARPHRD_ETHER:
+		*len = sizeof(((struct ether_header *)0)->ether_shost);
+		return f + offsetof(struct ether_header, ether_shost);
+	default:
+		*len = 0;
+		errno =	ENOTSUP;
+		return NULL;
+	}
+}
+
+void *
+bpf_frame_header_dst(const struct interface *ifp, void *fh, size_t *len)
+{
+	uint8_t *f = fh;
+
+	switch (ifp->family) {
+	case ARPHRD_ETHER:
+		*len = sizeof(((struct ether_header *)0)->ether_dhost);
+		return f + offsetof(struct ether_header, ether_dhost);
+	default:
+		*len = 0;
+		errno =	ENOTSUP;
+		return NULL;
+	}
+}
+
 static const uint8_t etherbcastaddr[] =
     { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff };
 
@@ -215,7 +247,6 @@ ssize_t
 bpf_read(struct interface *ifp, int fd, void *data, size_t len,
     unsigned int *flags)
 {
-	ssize_t fl = (ssize_t)bpf_frame_header_len(ifp);
 	ssize_t bytes;
 	struct ipv4_state *state = IPV4_STATE(ifp);
 
@@ -250,10 +281,10 @@ bpf_read(struct interface *ifp, int fd, void *data, size_t len,
 			*flags |= BPF_BCAST;
 		else
 			*flags &= ~BPF_BCAST;
-		payload += fl;
-		bytes = (ssize_t)packet.bh_caplen - fl;
-		if ((size_t)bytes > len)
+		if (packet.bh_caplen > len)
 			bytes = (ssize_t)len;
+		else
+			bytes = (ssize_t)packet.bh_caplen;
 		memcpy(data, payload, (size_t)bytes);
 next:
 		state->buffer_pos += BPF_WORDALIGN(packet.bh_hdrlen +
diff --git a/src/bpf.h b/src/bpf.h
index 46bc6c37..67f4456b 100644
--- a/src/bpf.h
+++ b/src/bpf.h
@@ -57,6 +57,8 @@
 
 extern const char *bpf_name;
 size_t bpf_frame_header_len(const struct interface *);
+void *bpf_frame_header_src(const struct interface *, void *, size_t *);
+void *bpf_frame_header_dst(const struct interface *, void *, size_t *);
 int bpf_frame_bcast(const struct interface *, const char *frame);
 int bpf_open(struct interface *, int (*)(struct interface *, int));
 int bpf_close(struct interface *, int);
diff --git a/src/dhcp.c b/src/dhcp.c
index b9757be0..1be4addd 100644
--- a/src/dhcp.c
+++ b/src/dhcp.c
@@ -3435,6 +3435,7 @@ dhcp_readbpf(void *arg)
 	uint8_t buf[MTU_MAX];
 	ssize_t bytes;
 	struct dhcp_state *state = D_STATE(ifp);
+	ssize_t fl = (ssize_t)bpf_frame_header_len(ifp);
 
 	/* Some RAW mechanisms are generic file descriptors, not sockets.
 	 * This means we have no kernel call to just get one packet,
@@ -3451,7 +3452,12 @@ dhcp_readbpf(void *arg)
 			}
 			break;
 		}
-		dhcp_packet(ifp, buf, (size_t)bytes);
+		if (bytes < fl) {
+			logerrx("%s: %s: short frame header",
+			    __func__, ifp->name);
+			break;
+		}
+		dhcp_packet(ifp, buf + fl, (size_t)(bytes - fl));
 		/* Check we still have a state after processing. */
 		if ((state = D_STATE(ifp)) == NULL)
 			break;
diff --git a/src/if-linux.c b/src/if-linux.c
index aad2142b..499ced6c 100644
--- a/src/if-linux.c
+++ b/src/if-linux.c
@@ -1569,16 +1569,13 @@ bpf_read(struct interface *ifp, int s, void *data, size_t len,
 	*flags |= BPF_EOF; /* We only ever read one packet. */
 	*flags &= ~BPF_PARTIALCSUM;
 	if (bytes) {
-		ssize_t fl = (ssize_t)bpf_frame_header_len(ifp);
-
 		if (bpf_frame_bcast(ifp, state->buffer) == 0)
 			*flags |= BPF_BCAST;
 		else
 			*flags &= ~BPF_BCAST;
-		bytes -= fl;
 		if ((size_t)bytes > len)
 			bytes = (ssize_t)len;
-		memcpy(data, state->buffer + fl, (size_t)bytes);
+		memcpy(data, state->buffer, (size_t)bytes);
 #ifdef PACKET_AUXDATA
 		for (cmsg = CMSG_FIRSTHDR(&msg);
 		     cmsg;