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 0fdcdd9aa3ba6310b61c63fefceb39e50bbaf9ad..2460a1a32078b891c41eec2963543c1ad5eb69e0 100644 (file)
--- 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 4842c3579ae0cd8e8a1c6ef674a17b7dd9818637..7522bf28f9599d5b2d75b41142c016b57912b056 100644 (file)
--- 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 6c31bc2cafc8fa8e6ec972f310d09e7bde922468..8a0a7e33f2061988fa3071d7e898a8c1dbd83482 100644 (file)
--- 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 46bc6c379e8bbfec85ad19227f66d71f35e757e5..67f4456bcd0dc26806927235aec4b178bc159302 100644 (file)
--- 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 b9757be0affce9fe0bdc9a38918492e2f9bfe5f5..1be4adddd6a01416e51529193524d18e9c795c51 100644 (file)
--- 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 aad2142b92197c55c346494b006cdabf7367121d..499ced6c2bae738a7aa7b82398a7b6cbf2ef8f23 100644 (file)
--- 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;