[PATCH 0/2] ipset: Two fixes for destination MAC address matches in ip,mac types

[PATCH 0/2] ipset: Two fixes for destination MAC address matches in ip,mac types

[Stefano Brivio]

Commit 8cc4ccf58379 ("ipset: Allow matching on destination MAC address for
mac and ipmac sets"), ipset.git commit 1543514c46a7, properly allows
destination matching for hash:mac set types, but missed to remove the
previous restriction for type hash:ip,mac and introduced an obvious mistake
in both bitmap:ip,mac and hash:ip,mac.

Drop the left-over check and correct the mistake, to fix the issue reported
by Chen Yi.

Stefano Brivio (2):
  ipset: Actually allow destination MAC address for hash:ip,mac sets too
  ipset: Copy the right MAC address in bitmap:ip,mac and hash:ip,mac
    sets

 kernel/net/netfilter/ipset/ip_set_bitmap_ipmac.c | 2 +-
 kernel/net/netfilter/ipset/ip_set_hash_ipmac.c   | 6 +-----
 2 files changed, 2 insertions(+), 6 deletions(-)

-- 
2.20.1

[PATCH 1/2] ipset: Actually allow destination MAC address for hash:ip,mac sets too

[Stefano Brivio]

In commit 8cc4ccf58379 ("ipset: Allow matching on destination MAC address
for mac and ipmac sets"), ipset.git commit 1543514c46a7, I removed the
KADT check that prevents matching on destination MAC addresses for
hash:mac sets, but forgot to remove the same check for hash:ip,mac set.

Drop this check: functionality is now commented in man pages and there's
no reason to restrict to source MAC address matching anymore.

Reported-by: Chen Yi <yiche@redhat.com>
Fixes: 8cc4ccf58379 ("ipset: Allow matching on destination MAC address for mac and ipmac sets")
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
---
 kernel/net/netfilter/ipset/ip_set_hash_ipmac.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/kernel/net/netfilter/ipset/ip_set_hash_ipmac.c b/kernel/net/netfilter/ipset/ip_set_hash_ipmac.c
index c830c68142ff..5b926bf80986 100644
--- a/kernel/net/netfilter/ipset/ip_set_hash_ipmac.c
+++ b/kernel/net/netfilter/ipset/ip_set_hash_ipmac.c
@@ -92,10 +92,6 @@ hash_ipmac4_kadt(struct ip_set *set, const struct sk_buff *skb,
 	struct hash_ipmac4_elem e = { .ip = 0, { .foo[0] = 0, .foo[1] = 0 } };
 	struct ip_set_ext ext = IP_SET_INIT_KEXT(skb, opt, set);
 
-	 /* MAC can be src only */
-	if (!(opt->flags & IPSET_DIM_TWO_SRC))
-		return 0;
-
 	if (skb_mac_header(skb) < skb->head ||
 	    (skb_mac_header(skb) + ETH_HLEN) > skb->data)
 		return -EINVAL;
-- 
2.20.1

[PATCH 2/2] ipset: Copy the right MAC address in bitmap:ip,mac and hash:ip,mac sets

[Stefano Brivio]

In commit 8cc4ccf58379 ("ipset: Allow matching on destination MAC address
for mac and ipmac sets"), ipset.git commit 1543514c46a7, I added to the
KADT functions for sets matching on MAC addreses the copy of source or
destination MAC address depending on the configured match.

This was done correctly for hash:mac, but for hash:ip,mac and
bitmap:ip,mac, copying and pasting the same code block presents an
obvious problem: in these two set types, the MAC address is the second
dimension, not the first one, and we are actually selecting the MAC
address depending on whether the first dimension (IP address) specifies
source or destination.

Fix this by checking for the IPSET_DIM_TWO_SRC flag in option flags.

This way, mixing source and destination matches for the two dimensions
of ip,mac set types works as expected. With this setup:

  ip netns add A
  ip link add veth1 type veth peer name veth2 netns A
  ip addr add 192.0.2.1/24 dev veth1
  ip -net A addr add 192.0.2.2/24 dev veth2
  ip link set veth1 up
  ip -net A link set veth2 up

  dst=$(ip netns exec A cat /sys/class/net/veth2/address)

  ip netns exec A ipset create test_bitmap bitmap:ip,mac range 192.0.0.0/16
  ip netns exec A ipset add test_bitmap 192.0.2.1,${dst}
  ip netns exec A iptables -A INPUT -m set ! --match-set test_bitmap src,dst -j DROP

  ip netns exec A ipset create test_hash hash:ip,mac
  ip netns exec A ipset add test_hash 192.0.2.1,${dst}
  ip netns exec A iptables -A INPUT -m set ! --match-set test_hash src,dst -j DROP

ipset correctly matches a test packet:

  # ping -c1 192.0.2.2 >/dev/null
  # echo $?
  0

Reported-by: Chen Yi <yiche@redhat.com>
Fixes: 8cc4ccf58379 ("ipset: Allow matching on destination MAC address for mac and ipmac sets")
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
---
 kernel/net/netfilter/ipset/ip_set_bitmap_ipmac.c | 2 +-
 kernel/net/netfilter/ipset/ip_set_hash_ipmac.c   | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/net/netfilter/ipset/ip_set_bitmap_ipmac.c b/kernel/net/netfilter/ipset/ip_set_bitmap_ipmac.c
index 9317b8fbc805..bf8da83a06e8 100644
--- a/kernel/net/netfilter/ipset/ip_set_bitmap_ipmac.c
+++ b/kernel/net/netfilter/ipset/ip_set_bitmap_ipmac.c
@@ -232,7 +232,7 @@ bitmap_ipmac_kadt(struct ip_set *set, const struct sk_buff *skb,
 
 	e.id = ip_to_id(map, ip);
 
-	if (opt->flags & IPSET_DIM_ONE_SRC)
+	if (opt->flags & IPSET_DIM_TWO_SRC)
 		ether_addr_copy(e.ether, eth_hdr(skb)->h_source);
 	else
 		ether_addr_copy(e.ether, eth_hdr(skb)->h_dest);
diff --git a/kernel/net/netfilter/ipset/ip_set_hash_ipmac.c b/kernel/net/netfilter/ipset/ip_set_hash_ipmac.c
index 5b926bf80986..25560ea742d6 100644
--- a/kernel/net/netfilter/ipset/ip_set_hash_ipmac.c
+++ b/kernel/net/netfilter/ipset/ip_set_hash_ipmac.c
@@ -96,7 +96,7 @@ hash_ipmac4_kadt(struct ip_set *set, const struct sk_buff *skb,
 	    (skb_mac_header(skb) + ETH_HLEN) > skb->data)
 		return -EINVAL;
 
-	if (opt->flags & IPSET_DIM_ONE_SRC)
+	if (opt->flags & IPSET_DIM_TWO_SRC)
 		ether_addr_copy(e.ether, eth_hdr(skb)->h_source);
 	else
 		ether_addr_copy(e.ether, eth_hdr(skb)->h_dest);
-- 
2.20.1

Re: [PATCH 0/2] ipset: Two fixes for destination MAC address matches in ip,mac types

[Jozsef Kadlecsik]

Hi Stefano,

On Mon, 24 Jun 2019, Stefano Brivio wrote:

> Commit 8cc4ccf58379 ("ipset: Allow matching on destination MAC address for
> mac and ipmac sets"), ipset.git commit 1543514c46a7, properly allows
> destination matching for hash:mac set types, but missed to remove the
> previous restriction for type hash:ip,mac and introduced an obvious mistake
> in both bitmap:ip,mac and hash:ip,mac.
> 
> Drop the left-over check and correct the mistake, to fix the issue reported
> by Chen Yi.
> 
> Stefano Brivio (2):
>   ipset: Actually allow destination MAC address for hash:ip,mac sets too
>   ipset: Copy the right MAC address in bitmap:ip,mac and hash:ip,mac
>     sets
> 
>  kernel/net/netfilter/ipset/ip_set_bitmap_ipmac.c | 2 +-
>  kernel/net/netfilter/ipset/ip_set_hash_ipmac.c   | 6 +-----
>  2 files changed, 2 insertions(+), 6 deletions(-)

Both patches are applied, thanks!

Best regards,
Jozsef
-
E-mail  : kadlec@blackhole.kfki.hu, kadlecsik.jozsef@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
          H-1525 Budapest 114, POB. 49, Hungary