Re: dhclient/bpf.c
On Sat, Jul 23, 2016 at 12:15:59AM +0200, Holger Mikolon wrote:
> I played with different bpf filters and came up with the below patch. It
> compares the interface's LLADDR with the the respective address in the
> bootp structure instead of the ether header. I don't know if this is the
> right way to fix my regression and at the same time retain the original
> intent of the change in version 1.41 (i.e. having a more "narrow" filter).
> So any feedback is welcome.
>
> By the way, the comment in bpf.c explaining the exact "tcpdump -d ..."
> parameters doesn't work as is. tcpdump requires the keyword "and" before
> "dst port 67".
>
> Regards
> Holger
Thanks for reporting this. We have reverted the original change for now
because we're wrapping things up for the 6.0 release.
When the times comes to revisit this, your diff will be considered.
Thanks,
Stefan
>
>
> Index: bpf.c
> =======
> RCS file: /cvs/src/sbin/dhclient/bpf.c,v
> retrieving revision 1.41
> diff -u -p -u -r1.41 bpf.c
> --- bpf.c 19 Jul 2016 17:23:20 - 1.41
> +++ bpf.c 22 Jul 2016 21:39:19 -
> @@ -117,7 +117,8 @@ if_register_send(void)
> *
> * Adapted from script shown by
> *
> - * tcpdump -d 'ether dst 00:00:00:00:00:00 ip proto \udp dst port 67'
> + * tcpdump -d 'ip and udp dst port 67 and \
> + * udp[36:2] = 0x and udp[38:4] = 0x'
> *
> * NOTE: tcpdump shows absolute jumps and relative jumps are required here!
> */
> @@ -130,29 +131,32 @@ struct bpf_insn dhcp_bpf_filter[] = {
>* NOTE: MAC value must be patched in!
>*/
>
> - BPF_STMT(BPF_LD + BPF_W + BPF_ABS, 2),
> - BPF_JUMP(BPF_JMP + BPF_JEQ + BPF_K, 0x, 0, 12), /* patch */
> - BPF_STMT(BPF_LD + BPF_H + BPF_ABS, 0),
> - BPF_JUMP(BPF_JMP + BPF_JEQ + BPF_K, 0x, 0, 10), /* patch */
> -
> /* Make sure this is an IP packet. */
> BPF_STMT(BPF_LD + BPF_H + BPF_ABS, 12),
> - BPF_JUMP(BPF_JMP + BPF_JEQ + BPF_K, ETHERTYPE_IP, 0, 8),
> + BPF_JUMP(BPF_JMP + BPF_JEQ + BPF_K, ETHERTYPE_IP, 0, 12),
>
> /* Make sure it's a UDP packet. */
> BPF_STMT(BPF_LD + BPF_B + BPF_ABS, 23),
> - BPF_JUMP(BPF_JMP + BPF_JEQ + BPF_K, IPPROTO_UDP, 0, 6),
> + BPF_JUMP(BPF_JMP + BPF_JEQ + BPF_K, IPPROTO_UDP, 0, 10),
>
> /* Make sure this isn't a fragment. */
> BPF_STMT(BPF_LD + BPF_H + BPF_ABS, 20),
> - BPF_JUMP(BPF_JMP + BPF_JSET + BPF_K, 0x1fff, 4, 0),
> + BPF_JUMP(BPF_JMP + BPF_JSET + BPF_K, 0x1fff, 8, 0),
>
> /* Get the IP header length. */
> BPF_STMT(BPF_LDX + BPF_B + BPF_MSH, 14),
>
> /* Make sure it's to the right port. */
> BPF_STMT(BPF_LD + BPF_H + BPF_IND, 16),
> - BPF_JUMP(BPF_JMP + BPF_JEQ + BPF_K, 67, 0, 1), /* patch */
> + BPF_JUMP(BPF_JMP + BPF_JEQ + BPF_K, 67, 0, 5), /* patch */
> +
> + /* check bootp.hw.addr 2 bytes */
> + BPF_STMT(BPF_LD + BPF_H + BPF_IND, 50),
> + BPF_JUMP(BPF_JMP + BPF_JEQ + BPF_K, 0x, 0, 3), /*
> patch */
> +
> + /* check bootp.hw.addr 4 bytes */
> + BPF_STMT(BPF_LD + BPF_W + BPF_IND, 52),
> + BPF_JUMP(BPF_JMP + BPF_JEQ + BPF_K, 0x, 0, 1), /*
> patch */
>
> /* If we passed all the tests, ask for the whole packet. */
> BPF_STMT(BPF_RET+BPF_K, (u_int)-1),
> @@ -257,12 +261,12 @@ if_register_receive(void)
>* insn number(s) used below!
>*/
> memcpy(, ((uint8_t *)>hw_address) + 2, sizeof(bits));
> - dhcp_bpf_filter[1].k = ntohl(bits);
> + dhcp_bpf_filter[12].k = ntohl(bits);
>
> memcpy(, ((uint8_t *)>hw_address), sizeof(bits16));
> - dhcp_bpf_filter[3].k = ntohs(bits16);
> + dhcp_bpf_filter[10].k = ntohs(bits16);
>
> - dhcp_bpf_filter[12].k = LOCAL_PORT;
> + dhcp_bpf_filter[8].k = LOCAL_PORT;
>
> if (ioctl(ifi->bfdesc, BIOCSETF, ) < 0)
> error("Can't install packet filter program: %s",
>
>
>
> > Date: Fri, 22 Jul 2016 00:56:59
> > From: Holger Mikolon <hol...@mikolon.com>
> > To: tech@openbsd.org
> > Subject: dhclient/bpf.c
> >
> > Hi,
> >
> > I'm following -current and see a regression in dhclient on my machine:
> > It seems to be related to version 1.41 of sbin/dhclient/bpf.c.
> > Since then dhclient doesn't recognize the recieved lease.
> >
> > tcpdump shows this:
> > 00:21:6a:56:2b:36 ff:ff:ff:ff:ff:ff 342: 192.168.1.7.68 >
> > 255.255.255.255.67: udp 300 [tos 0x10]
> > 00:15:0c:01:a7:47 ff:ff:ff:ff:ff:ff 590: 192.168.1.1.67 >
> > 255.255.255.255.68: udp 548
> >
> > However dhclient does not recognize the response.
> > Reverting back to version 1.40 fixes the issue for me.
> >
> > Holger
>
Re: dhclient/bpf.c
I played with different bpf filters and came up with the below patch. It
compares the interface's LLADDR with the the respective address in the
bootp structure instead of the ether header. I don't know if this is the
right way to fix my regression and at the same time retain the original
intent of the change in version 1.41 (i.e. having a more "narrow" filter).
So any feedback is welcome.
By the way, the comment in bpf.c explaining the exact "tcpdump -d ..."
parameters doesn't work as is. tcpdump requires the keyword "and" before
"dst port 67".
Regards
Holger
Index: bpf.c
===
RCS file: /cvs/src/sbin/dhclient/bpf.c,v
retrieving revision 1.41
diff -u -p -u -r1.41 bpf.c
--- bpf.c 19 Jul 2016 17:23:20 - 1.41
+++ bpf.c 22 Jul 2016 21:39:19 -
@@ -117,7 +117,8 @@ if_register_send(void)
*
* Adapted from script shown by
*
- * tcpdump -d 'ether dst 00:00:00:00:00:00 ip proto \udp dst port 67'
+ * tcpdump -d 'ip and udp dst port 67 and \
+ * udp[36:2] = 0x and udp[38:4] = 0x'
*
* NOTE: tcpdump shows absolute jumps and relative jumps are required here!
*/
@@ -130,29 +131,32 @@ struct bpf_insn dhcp_bpf_filter[] = {
* NOTE: MAC value must be patched in!
*/
- BPF_STMT(BPF_LD + BPF_W + BPF_ABS, 2),
- BPF_JUMP(BPF_JMP + BPF_JEQ + BPF_K, 0x, 0, 12), /* patch */
- BPF_STMT(BPF_LD + BPF_H + BPF_ABS, 0),
- BPF_JUMP(BPF_JMP + BPF_JEQ + BPF_K, 0x, 0, 10), /* patch */
-
/* Make sure this is an IP packet. */
BPF_STMT(BPF_LD + BPF_H + BPF_ABS, 12),
- BPF_JUMP(BPF_JMP + BPF_JEQ + BPF_K, ETHERTYPE_IP, 0, 8),
+ BPF_JUMP(BPF_JMP + BPF_JEQ + BPF_K, ETHERTYPE_IP, 0, 12),
/* Make sure it's a UDP packet. */
BPF_STMT(BPF_LD + BPF_B + BPF_ABS, 23),
- BPF_JUMP(BPF_JMP + BPF_JEQ + BPF_K, IPPROTO_UDP, 0, 6),
+ BPF_JUMP(BPF_JMP + BPF_JEQ + BPF_K, IPPROTO_UDP, 0, 10),
/* Make sure this isn't a fragment. */
BPF_STMT(BPF_LD + BPF_H + BPF_ABS, 20),
- BPF_JUMP(BPF_JMP + BPF_JSET + BPF_K, 0x1fff, 4, 0),
+ BPF_JUMP(BPF_JMP + BPF_JSET + BPF_K, 0x1fff, 8, 0),
/* Get the IP header length. */
BPF_STMT(BPF_LDX + BPF_B + BPF_MSH, 14),
/* Make sure it's to the right port. */
BPF_STMT(BPF_LD + BPF_H + BPF_IND, 16),
- BPF_JUMP(BPF_JMP + BPF_JEQ + BPF_K, 67, 0, 1), /* patch */
+ BPF_JUMP(BPF_JMP + BPF_JEQ + BPF_K, 67, 0, 5), /* patch */
+
+ /* check bootp.hw.addr 2 bytes */
+ BPF_STMT(BPF_LD + BPF_H + BPF_IND, 50),
+ BPF_JUMP(BPF_JMP + BPF_JEQ + BPF_K, 0x, 0, 3), /*
patch */
+
+ /* check bootp.hw.addr 4 bytes */
+ BPF_STMT(BPF_LD + BPF_W + BPF_IND, 52),
+ BPF_JUMP(BPF_JMP + BPF_JEQ + BPF_K, 0x, 0, 1), /*
patch */
/* If we passed all the tests, ask for the whole packet. */
BPF_STMT(BPF_RET+BPF_K, (u_int)-1),
@@ -257,12 +261,12 @@ if_register_receive(void)
* insn number(s) used below!
*/
memcpy(, ((uint8_t *)>hw_address) + 2, sizeof(bits));
- dhcp_bpf_filter[1].k = ntohl(bits);
+ dhcp_bpf_filter[12].k = ntohl(bits);
memcpy(, ((uint8_t *)>hw_address), sizeof(bits16));
- dhcp_bpf_filter[3].k = ntohs(bits16);
+ dhcp_bpf_filter[10].k = ntohs(bits16);
- dhcp_bpf_filter[12].k = LOCAL_PORT;
+ dhcp_bpf_filter[8].k = LOCAL_PORT;
if (ioctl(ifi->bfdesc, BIOCSETF, ) < 0)
error("Can't install packet filter program: %s",
> Date: Fri, 22 Jul 2016 00:56:59
> From: Holger Mikolon <hol...@mikolon.com>
> To: tech@openbsd.org
> Subject: dhclient/bpf.c
>
> Hi,
>
> I'm following -current and see a regression in dhclient on my machine:
> It seems to be related to version 1.41 of sbin/dhclient/bpf.c.
> Since then dhclient doesn't recognize the recieved lease.
>
> tcpdump shows this:
> 00:21:6a:56:2b:36 ff:ff:ff:ff:ff:ff 342: 192.168.1.7.68 > 255.255.255.255.67:
> udp 300 [tos 0x10]
> 00:15:0c:01:a7:47 ff:ff:ff:ff:ff:ff 590: 192.168.1.1.67 > 255.255.255.255.68:
> udp 548
>
> However dhclient does not recognize the response.
> Reverting back to version 1.40 fixes the issue for me.
>
> Holger
dhclient/bpf.c
Hi,
I'm following -current and see a regression in dhclient on my machine:
It seems to be related to version 1.41 of sbin/dhclient/bpf.c.
Since then dhclient doesn't recognize the recieved lease.
tcpdump shows this:
00:21:6a:56:2b:36 ff:ff:ff:ff:ff:ff 342: 192.168.1.7.68 > 255.255.255.255.67:
udp 300 [tos 0x10]
00:15:0c:01:a7:47 ff:ff:ff:ff:ff:ff 590: 192.168.1.1.67 > 255.255.255.255.68:
udp 548
However dhclient does not recognize the response.
Reverting back to version 1.40 fixes the issue for me.
Holger
--- bpf.c Fri Jul 22 00:51:11 2016
+++ bpf.c Fri Jul 22 00:51:26 2016
@@ -1,4 +1,4 @@
-/* $OpenBSD: bpf.c,v 1.41 2016/07/19 17:23:20 krw Exp $*/
+/* $OpenBSD: bpf.c,v 1.40 2016/05/08 08:20:50 natano Exp $ */
/* BPF socket interface code, originally contributed by Archie Cobbs. */
@@ -114,27 +114,8 @@
*
* XXX: Changes to the filter program may require changes to the
* constant offsets used in if_register_receive to patch the BPF program!
- *
- * Adapted from script shown by
- *
- * tcpdump -d 'ether dst 00:00:00:00:00:00 ip proto \udp dst port 67'
- *
- * NOTE: tcpdump shows absolute jumps and relative jumps are required here!
*/
struct bpf_insn dhcp_bpf_filter[] = {
- /*
-* Make sure this is directed to our MAC.
-* a) compare last 4 octets
-* b) compare first 2 octets
-*
-* NOTE: MAC value must be patched in!
-*/
-
- BPF_STMT(BPF_LD + BPF_W + BPF_ABS, 2),
- BPF_JUMP(BPF_JMP + BPF_JEQ + BPF_K, 0x, 0, 12), /* patch */
- BPF_STMT(BPF_LD + BPF_H + BPF_ABS, 0),
- BPF_JUMP(BPF_JMP + BPF_JEQ + BPF_K, 0x, 0, 10), /* patch */
-
/* Make sure this is an IP packet. */
BPF_STMT(BPF_LD + BPF_H + BPF_ABS, 12),
BPF_JUMP(BPF_JMP + BPF_JEQ + BPF_K, ETHERTYPE_IP, 0, 8),
@@ -209,8 +190,6 @@
struct bpf_version v;
struct bpf_program p;
int flag = 1, sz;
- uint32_t bits;
- uint16_t bits16;
/* Open a BPF device and hang it on this interface. */
ifi->bfdesc = if_register_bpf();
@@ -256,13 +235,7 @@
* XXX: changes to filter program may require changes to the
* insn number(s) used below!
*/
- memcpy(, ((uint8_t *)>hw_address) + 2, sizeof(bits));
- dhcp_bpf_filter[1].k = ntohl(bits);
-
- memcpy(, ((uint8_t *)>hw_address), sizeof(bits16));
- dhcp_bpf_filter[3].k = ntohs(bits16);
-
- dhcp_bpf_filter[12].k = LOCAL_PORT;
+ dhcp_bpf_filter[8].k = LOCAL_PORT;
if (ioctl(ifi->bfdesc, BIOCSETF, ) < 0)
error("Can't install packet filter program: %s",
