[libvirt] [PATCH[ nwfilter: Discard class D and E IP addresses when sniffing

When sniffing the network traffic, discard class D and E IP addresses when sniffing traffic. This was a reason why filters were not correctly rebuilt on VMs on the local 192.* network when libvirt was restarted and those VMs did not use a DHCP request to get its IP address. Signed-off-by: Stefan Berger <stefanb@us.ibm.com> --- src/nwfilter/nwfilter_learnipaddr.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) Index: libvirt-acl/src/nwfilter/nwfilter_learnipaddr.c =================================================================== --- libvirt-acl.orig/src/nwfilter/nwfilter_learnipaddr.c +++ libvirt-acl/src/nwfilter/nwfilter_learnipaddr.c @@ -546,9 +546,12 @@ learnIPAddressThread(void *arg) struct iphdr *iphdr = (struct iphdr*)(packet + ethHdrSize); vmaddr = iphdr->saddr; - // skip eth. bcast and mcast addresses, + // skip eth. bcast and mcast addresses (224.0.0.0 - + // 239.255.255.255), class E (255.*) // and zero address in DHCP Requests - if ((ntohl(vmaddr) & 0xc0000000) || vmaddr == 0) { + if ( (ntohl(vmaddr) & 0xe0000000) == 0xe0000000 || + (ntohl(vmaddr) & 0xf0000000) == 0xf0000000 || + vmaddr == 0) { vmaddr = 0; continue; }

On 08/13/2010 12:38 PM, Stefan Berger wrote:
When sniffing the network traffic, discard class D and E IP addresses when sniffing traffic. This was a reason why filters were not correctly rebuilt on VMs on the local 192.* network when libvirt was restarted and those VMs did not use a DHCP request to get its IP address.
Signed-off-by: Stefan Berger <stefanb@us.ibm.com>
--- src/nwfilter/nwfilter_learnipaddr.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
Index: libvirt-acl/src/nwfilter/nwfilter_learnipaddr.c =================================================================== --- libvirt-acl.orig/src/nwfilter/nwfilter_learnipaddr.c +++ libvirt-acl/src/nwfilter/nwfilter_learnipaddr.c @@ -546,9 +546,12 @@ learnIPAddressThread(void *arg) struct iphdr *iphdr = (struct iphdr*)(packet + ethHdrSize); vmaddr = iphdr->saddr; - // skip eth. bcast and mcast addresses, + // skip eth. bcast and mcast addresses (224.0.0.0 - + // 239.255.255.255), class E (255.*) // and zero address in DHCP Requests - if ((ntohl(vmaddr) & 0xc0000000) || vmaddr == 0) { + if ( (ntohl(vmaddr) & 0xe0000000) == 0xe0000000 ||
This line's fine for 224-239.*, but...
+ (ntohl(vmaddr) & 0xf0000000) == 0xf0000000 ||
shouldn't this be (ntohl(vmaddr) & 0xff000000) == 0xff000000, so that you are not excluding 254.*? ACK with that fix. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

libvir-list-bounces@redhat.com wrote on 08/13/2010 03:11:25 PM:
On 08/13/2010 12:38 PM, Stefan Berger wrote:
When sniffing the network traffic, discard class D and E IP addresses when sniffing traffic. This was a reason why filters were not
rebuilt on VMs on the local 192.* network when libvirt was restarted and those VMs did not use a DHCP request to get its IP address.
Signed-off-by: Stefan Berger <stefanb@us.ibm.com>
--- src/nwfilter/nwfilter_learnipaddr.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
Index: libvirt-acl/src/nwfilter/nwfilter_learnipaddr.c =================================================================== --- libvirt-acl.orig/src/nwfilter/nwfilter_learnipaddr.c +++ libvirt-acl/src/nwfilter/nwfilter_learnipaddr.c @@ -546,9 +546,12 @@ learnIPAddressThread(void *arg) struct iphdr *iphdr = (struct iphdr*)(packet + ethHdrSize); vmaddr = iphdr->saddr; - // skip eth. bcast and mcast addresses, + // skip eth. bcast and mcast addresses (224.0.0.0
correctly -
+ // 239.255.255.255), class E (255.*) // and zero address in DHCP Requests - if ((ntohl(vmaddr) & 0xc0000000) || vmaddr == 0) { + if ( (ntohl(vmaddr) & 0xe0000000) == 0xe0000000 ||
This line's fine for 224-239.*, but...
+ (ntohl(vmaddr) & 0xf0000000) == 0xf0000000 ||
shouldn't this be (ntohl(vmaddr) & 0xff000000) == 0xff000000, so that you are not excluding 254.*?
Looking at Wikipedia for this http://en.wikipedia.org/wiki/Classful_network Class D addresses have highest bits with pattern 1110 0000 -> 0xe0 Class E addresses have highest bits with pattern 1111 0000 -> 0xf0 I think my masks are fine and the masking with 0xf0 00 00 00 should also include 254.* = 0xfe.* . Stefan

On 08/13/2010 01:45 PM, Stefan Berger wrote:
- // skip eth. bcast and mcast addresses, + // skip eth. bcast and mcast addresses (224.0.0.0
+ // 239.255.255.255), class E (255.*) // and zero address in DHCP Requests - if ((ntohl(vmaddr) & 0xc0000000) || vmaddr == 0)
http://en.wikipedia.org/wiki/Classful_network
Class D addresses have highest bits with pattern 1110 0000 -> 0xe0 Class E addresses have highest bits with pattern 1111 0000 -> 0xf0
I think my masks are fine and the masking with 0xf0 00 00 00 should also include 254.* = 0xfe.* .
In that case, the comments are wrong. Class E is more than 255.*, it is 240.0.0.0-255.255.255.255. And in that case, the bit operations can be simplified: if ((ntohl(vmaddr) & 0xc0000000) == 0xc0000000) || vmaddr == 0) In other words, the logic bug is that we were rejecting IP addresses that had 1 or 2, but not all three, of the top three bits set. The desired action is to reject IP packets if all three of the top bits are simultaneously set. Let's see a v2 that gets the comments right, and uses the simpler logic. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

Eric Blake <eblake@redhat.com> wrote on 08/13/2010 03:56:12 PM:
[image removed]
On 08/13/2010 01:45 PM, Stefan Berger wrote:
- // skip eth. bcast and mcast addresses, + // skip eth. bcast and mcast addresses
-
+ // 239.255.255.255), class E (255.*) // and zero address in DHCP Requests - if ((ntohl(vmaddr) & 0xc0000000) || vmaddr ==
(224.0.0.0 0)
http://en.wikipedia.org/wiki/Classful_network
Class D addresses have highest bits with pattern 1110 0000 -> 0xe0 Class E addresses have highest bits with pattern 1111 0000 -> 0xf0
I think my masks are fine and the masking with 0xf0 00 00 00 should
also
include 254.* = 0xfe.* .
In that case, the comments are wrong. Class E is more than 255.*, it is 240.0.0.0-255.255.255.255. And in that case, the bit operations can be simplified:
if ((ntohl(vmaddr) & 0xc0000000) == 0xc0000000) || vmaddr == 0)
In other words, the logic bug is that we were rejecting IP addresses that had 1 or 2, but not all three, of the top three bits set. The desired action is to reject IP packets if all three of the top bits are simultaneously set.
Right, the comment was not correct. I think the simplified if should be if ((ntohl(vmaddr) & 0xe0000000) == 0xe0000000) || vmaddr == 0) That then covers class D and E since this mask then covers 0xe0.00.00.00 - 0xff.ff.ff.ff = 225.0.0.0 - 255.255.255.255.
Let's see a v2 that gets the comments right, and uses the simpler logic.
Coming soon. Stefan
participants (3)
-
Eric Blake
-
Stefan Berger
-
Stefan Berger