* [PATCH 1/4] ipset: minor variable-naming corrections [not found] <51A54267.8050402-git-send-email-mr.dash.four@googlemail.com> @ 2013-05-29 0:13 ` Dash Four 2013-05-29 0:13 ` [PATCH 2/4] ipset: add "inner" flag implementation Dash Four ` (2 subsequent siblings) 3 siblings, 0 replies; 7+ messages in thread From: Dash Four @ 2013-05-29 0:13 UTC (permalink / raw) To: Jozsef Kadlecsik; +Cc: Pablo Neira Ayuso, Netfilter Core Team This patch corrects minor variable-naming inconsistencies in net/netfilter/ipset/ip_set_getport.c Signed-off-by: Dash Four <mr.dash.four@googlemail.com> --- kernel/net/netfilter/ipset/ip_set_getport.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/kernel/net/netfilter/ipset/ip_set_getport.c b/kernel/net/netfilter/ipset/ip_set_getport.c index 279a03c..6c019b3 100644 --- a/kernel/net/netfilter/ipset/ip_set_getport.c +++ b/kernel/net/netfilter/ipset/ip_set_getport.c @@ -133,7 +133,7 @@ bool ip_set_get_ip6_port(const struct sk_buff *skb, bool src, __be16 *port, u8 *proto) { - int protoff; + int protooff; u8 nexthdr; #if LINUX_VERSION_CODE >= KERNEL_VERSION(3, 3, 0) __be16 frag_off; @@ -141,15 +141,15 @@ ip_set_get_ip6_port(const struct sk_buff *skb, bool src, nexthdr = ipv6_hdr(skb)->nexthdr; #if LINUX_VERSION_CODE >= KERNEL_VERSION(3, 3, 0) - protoff = ipv6_skip_exthdr(skb, sizeof(struct ipv6hdr), &nexthdr, + protooff = ipv6_skip_exthdr(skb, sizeof(struct ipv6hdr), &nexthdr, &frag_off); #else - protoff = ipv6_skip_exthdr(skb, sizeof(struct ipv6hdr), &nexthdr); + protooff = ipv6_skip_exthdr(skb, sizeof(struct ipv6hdr), &nexthdr); #endif - if (protoff < 0) + if (protooff < 0) return false; - return get_port(skb, nexthdr, protoff, src, port, proto); + return get_port(skb, nexthdr, protooff, src, port, proto); } EXPORT_SYMBOL_GPL(ip_set_get_ip6_port); #endif ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/4] ipset: add "inner" flag implementation [not found] <51A54267.8050402-git-send-email-mr.dash.four@googlemail.com> 2013-05-29 0:13 ` [PATCH 1/4] ipset: minor variable-naming corrections Dash Four @ 2013-05-29 0:13 ` Dash Four 2013-06-01 10:54 ` Jozsef Kadlecsik 2013-05-29 0:13 ` [PATCH 3/4] iptables: add set match "inner" flag support Dash Four 2013-05-29 0:13 ` [PATCH 4/4] iptables (userspace): " Dash Four 3 siblings, 1 reply; 7+ messages in thread From: Dash Four @ 2013-05-29 0:13 UTC (permalink / raw) To: Jozsef Kadlecsik; +Cc: Pablo Neira Ayuso, Netfilter Core Team This patch implements "inner" flag option in the set iptables match, allowing matching based on the properties (source/destination IP address, protocol, port and so on) of the original (inner) connection in the event of the following ICMP[v4,v6] messages: ICMPv4 destination-unreachable (code 3); ICMPv4 source-quench (code 4); ICMPv4 time-exceeded (code 11); ICMPv6 destination-unreachable (code 1); ICMPv6 packet-too-big (code 2); ICMPv6 time-exceeded (code 3); Signed-off-by: Dash Four <mr.dash.four@googlemail.com> --- kernel/include/linux/netfilter/ipset/ip_set.h | 167 +++++++++++++++++++ .../include/linux/netfilter/ipset/ip_set_getport.h | 16 ++ kernel/include/uapi/linux/netfilter/ipset/ip_set.h | 2 + kernel/net/netfilter/ipset/ip_set_bitmap_ip.c | 4 +- kernel/net/netfilter/ipset/ip_set_bitmap_ipmac.c | 4 +- kernel/net/netfilter/ipset/ip_set_bitmap_port.c | 14 +- kernel/net/netfilter/ipset/ip_set_getport.c | 174 ++++++++++++++++++++ kernel/net/netfilter/ipset/ip_set_hash_ip.c | 13 +- kernel/net/netfilter/ipset/ip_set_hash_ipport.c | 37 ++++- kernel/net/netfilter/ipset/ip_set_hash_ipportip.c | 47 ++++-- kernel/net/netfilter/ipset/ip_set_hash_ipportnet.c | 49 ++++-- kernel/net/netfilter/ipset/ip_set_hash_net.c | 15 +- kernel/net/netfilter/ipset/ip_set_hash_netiface.c | 16 +- kernel/net/netfilter/ipset/ip_set_hash_netport.c | 41 +++-- 14 files changed, 542 insertions(+), 57 deletions(-) diff --git a/kernel/include/linux/netfilter/ipset/ip_set.h b/kernel/include/linux/netfilter/ipset/ip_set.h index 8499e25..ac29e16 100644 --- a/kernel/include/linux/netfilter/ipset/ip_set.h +++ b/kernel/include/linux/netfilter/ipset/ip_set.h @@ -17,6 +17,9 @@ #include <linux/netfilter/x_tables.h> #include <linux/stringify.h> #include <linux/vmalloc.h> +#include <net/ip.h> +#include <net/ipv6.h> +#include <net/icmp.h> #include <net/netlink.h> #include <uapi/linux/netfilter/ipset/ip_set.h> #include <linux/netfilter/ipset/ip_set_compat.h> @@ -373,6 +376,79 @@ ip4addrptr(const struct sk_buff *skb, bool src, __be32 *addr) *addr = src ? ip_hdr(skb)->saddr : ip_hdr(skb)->daddr; } +static inline __be32 +ip4inneraddr(const struct sk_buff *skb, bool src) +{ + struct iphdr _iph; + struct icmphdr _icmph; + u8 type; + const struct iphdr *ih; + const struct icmphdr *ich; + static const size_t len = 8 + sizeof(struct iphdr); + + ih = skb_header_pointer(skb, 0, sizeof(_iph), &_iph); + if (ih == NULL || ih->protocol != IPPROTO_ICMP || + ntohs(ih->frag_off) & IP_OFFSET) + goto err; + + ich = skb_header_pointer(skb, ih->ihl*4, + sizeof(_icmph), &_icmph); + if (ich == NULL || + (ich->type <= NR_ICMP_TYPES && skb->len - ih->ihl*4 < len)) + goto err; + + type = ich->type; + if (type == ICMP_DEST_UNREACH || + type == ICMP_SOURCE_QUENCH || + type == ICMP_TIME_EXCEEDED) { + ih = skb_header_pointer(skb, ih->ihl*4 + sizeof(_icmph), + sizeof(_iph), &_iph); + if (ih == NULL || ntohs(ih->frag_off) & IP_OFFSET) + goto err; + + return src ? ih->saddr : ih->daddr; + } +err: + return 0; +} + +static inline void +ip4inneraddrptr(const struct sk_buff *skb, bool src, __be32 *addr) +{ + struct iphdr _iph; + struct icmphdr _icmph; + u8 type; + const struct iphdr *ih; + const struct icmphdr *ich; + static const size_t len = 8 + sizeof(struct iphdr); + + ih = skb_header_pointer(skb, 0, sizeof(_iph), &_iph); + if (ih == NULL || ntohs(ih->frag_off) & IP_OFFSET || + ih->protocol != IPPROTO_ICMP) + goto err; + + ich = skb_header_pointer(skb, ih->ihl*4, + sizeof(_icmph), &_icmph); + if (ich == NULL || + (ich->type <= NR_ICMP_TYPES && skb->len - ih->ihl*4 < len)) + goto err; + + type = ich->type; + if (type == ICMP_DEST_UNREACH || + type == ICMP_SOURCE_QUENCH || + type == ICMP_TIME_EXCEEDED) { + ih = skb_header_pointer(skb, ih->ihl*4 + sizeof(_icmph), + sizeof(_iph), &_iph); + if (ih == NULL || ntohs(ih->frag_off) & IP_OFFSET) + goto err; + + *addr = src ? ih->saddr : ih->daddr; + return; + } +err: + *addr = 0; +} + static inline void ip6addrptr(const struct sk_buff *skb, bool src, struct in6_addr *addr) { @@ -380,6 +456,97 @@ ip6addrptr(const struct sk_buff *skb, bool src, struct in6_addr *addr) sizeof(*addr)); } +#define ip6_ext_hdr(hdr) ((hdr == IPPROTO_HOPOPTS) || \ + (hdr == IPPROTO_ROUTING) || \ + (hdr == IPPROTO_FRAGMENT) || \ + (hdr == IPPROTO_ESP) || \ + (hdr == IPPROTO_AH) || \ + (hdr == IPPROTO_NONE) || \ + (hdr == IPPROTO_DSTOPTS)) +static inline void +ip6inneraddrptr(const struct sk_buff *skb, bool src, struct in6_addr *addr) +{ + u8 type, currenthdr; + bool fragment = false; + unsigned int ptr, hdrlen = 0; + struct ipv6hdr _ip6h; + struct icmp6hdr _icmp6h; + const struct ipv6hdr *ih; + const struct icmp6hdr *ic; + + ih = skb_header_pointer(skb, 0, sizeof(_ip6h), &_ip6h); + if (ih == NULL) + goto err; + + ptr = sizeof(struct ipv6hdr); + currenthdr = ih->nexthdr; + while (currenthdr != NEXTHDR_NONE && ip6_ext_hdr(currenthdr)) { + struct ipv6_opt_hdr _hdr; + const struct ipv6_opt_hdr *hp; + + hp = skb_header_pointer(skb, ptr, sizeof(_hdr), &_hdr); + if (hp == NULL) + goto err; + + switch (currenthdr) { + case IPPROTO_FRAGMENT: { + struct frag_hdr _fhdr; + const struct frag_hdr *fh; + + fh = skb_header_pointer(skb, ptr, sizeof(_fhdr), + &_fhdr); + if (fh == NULL) + goto err; + if (ntohs(fh->frag_off) & 0xFFF8) + fragment = true; + + hdrlen = 8; + break; + } + case IPPROTO_DSTOPTS: + case IPPROTO_ROUTING: + case IPPROTO_HOPOPTS: + if (fragment) + goto err; + + hdrlen = ipv6_optlen(hp); + break; + case IPPROTO_AH: + hdrlen = (hp->hdrlen+2)<<2; + break; + case IPPROTO_ESP: + default: + goto err; + } /* switch IPPROTO */ + + currenthdr = hp->nexthdr; + ptr += hdrlen; + } + if (fragment || currenthdr != IPPROTO_ICMPV6) + goto err; + + ic = skb_header_pointer(skb, ptr, sizeof(_icmp6h), &_icmp6h); + if (ic == NULL) + goto err; + + type = ic->icmp6_type; + if (type == ICMPV6_DEST_UNREACH || + type == ICMPV6_PKT_TOOBIG || + type == ICMPV6_TIME_EXCEED) { + ih = skb_header_pointer(skb, ptr + sizeof(_icmp6h), + sizeof(_ip6h), &_ip6h); + if (ih == NULL) + goto err; + + memcpy(addr, src ? &ih->saddr : &ih->daddr, + sizeof(*addr)); + return; + } + +err: + memset(addr, 0, sizeof(*addr)); +} + /* Calculate the bytes required to store the inclusive range of a-b */ static inline int bitmap_bytes(u32 a, u32 b) diff --git a/kernel/include/linux/netfilter/ipset/ip_set_getport.h b/kernel/include/linux/netfilter/ipset/ip_set_getport.h index 90d0930..549d013 100644 --- a/kernel/include/linux/netfilter/ipset/ip_set_getport.h +++ b/kernel/include/linux/netfilter/ipset/ip_set_getport.h @@ -4,20 +4,36 @@ extern bool ip_set_get_ip4_port(const struct sk_buff *skb, bool src, __be16 *port, u8 *proto); +extern bool ip_set_get_inner_ip4_port(const struct sk_buff *skb, bool src, + __be16 *port, u8 *proto); + #if defined(CONFIG_IP6_NF_IPTABLES) || defined(CONFIG_IP6_NF_IPTABLES_MODULE) extern bool ip_set_get_ip6_port(const struct sk_buff *skb, bool src, __be16 *port, u8 *proto); + +extern bool ip_set_get_inner_ip6_port(const struct sk_buff *skb, bool src, + __be16 *port, u8 *proto); #else static inline bool ip_set_get_ip6_port(const struct sk_buff *skb, bool src, __be16 *port, u8 *proto) { return false; } + +static inline bool ip_set_get_inner_ip6_port(const struct sk_buff *skb, + bool src, + __be16 *port, u8 *proto) +{ + return false; +} #endif extern bool ip_set_get_ip_port(const struct sk_buff *skb, u8 pf, bool src, __be16 *port); +extern bool ip_set_get_inner_ip_port(const struct sk_buff *skb, u8 pf, + bool src, __be16 *port); + static inline bool ip_set_proto_with_ports(u8 proto) { switch (proto) { diff --git a/kernel/include/uapi/linux/netfilter/ipset/ip_set.h b/kernel/include/uapi/linux/netfilter/ipset/ip_set.h index 8024cdf..e9e6586 100644 --- a/kernel/include/uapi/linux/netfilter/ipset/ip_set.h +++ b/kernel/include/uapi/linux/netfilter/ipset/ip_set.h @@ -161,6 +161,8 @@ enum ipset_cmd_flags { (1 << IPSET_FLAG_BIT_SKIP_SUBCOUNTER_UPDATE), IPSET_FLAG_BIT_MATCH_COUNTERS = 5, IPSET_FLAG_MATCH_COUNTERS = (1 << IPSET_FLAG_BIT_MATCH_COUNTERS), + IPSET_FLAG_BIT_INNER = 6, + IPSET_FLAG_INNER = (1 << IPSET_FLAG_BIT_INNER), IPSET_FLAG_BIT_RETURN_NOMATCH = 7, IPSET_FLAG_RETURN_NOMATCH = (1 << IPSET_FLAG_BIT_RETURN_NOMATCH), IPSET_FLAG_CMD_MAX = 15, diff --git a/kernel/net/netfilter/ipset/ip_set_bitmap_ip.c b/kernel/net/netfilter/ipset/ip_set_bitmap_ip.c index ce99d26..7552d6d 100644 --- a/kernel/net/netfilter/ipset/ip_set_bitmap_ip.c +++ b/kernel/net/netfilter/ipset/ip_set_bitmap_ip.c @@ -116,7 +116,9 @@ bitmap_ip_kadt(struct ip_set *set, const struct sk_buff *skb, struct ip_set_ext ext = IP_SET_INIT_KEXT(skb, opt, map); u32 ip; - ip = ntohl(ip4addr(skb, opt->flags & IPSET_DIM_ONE_SRC)); + ip = opt->cmdflags & IPSET_FLAG_INNER ? + ntohl(ip4inneraddr(skb, opt->flags & IPSET_DIM_ONE_SRC)) : + ntohl(ip4addr(skb, opt->flags & IPSET_DIM_ONE_SRC)); if (ip < map->first_ip || ip > map->last_ip) return -IPSET_ERR_BITMAP_RANGE; diff --git a/kernel/net/netfilter/ipset/ip_set_bitmap_ipmac.c b/kernel/net/netfilter/ipset/ip_set_bitmap_ipmac.c index 6d5bad9..df5f3b4 100644 --- a/kernel/net/netfilter/ipset/ip_set_bitmap_ipmac.c +++ b/kernel/net/netfilter/ipset/ip_set_bitmap_ipmac.c @@ -223,7 +223,9 @@ bitmap_ipmac_kadt(struct ip_set *set, const struct sk_buff *skb, if (!(opt->flags & IPSET_DIM_TWO_SRC)) return 0; - ip = ntohl(ip4addr(skb, opt->flags & IPSET_DIM_ONE_SRC)); + ip = opt->cmdflags & IPSET_FLAG_INNER ? + ntohl(ip4inneraddr(skb, opt->flags & IPSET_DIM_ONE_SRC)) : + ntohl(ip4addr(skb, opt->flags & IPSET_DIM_ONE_SRC)); if (ip < map->first_ip || ip > map->last_ip) return -IPSET_ERR_BITMAP_RANGE; diff --git a/kernel/net/netfilter/ipset/ip_set_bitmap_port.c b/kernel/net/netfilter/ipset/ip_set_bitmap_port.c index b220489..d29395b 100644 --- a/kernel/net/netfilter/ipset/ip_set_bitmap_port.c +++ b/kernel/net/netfilter/ipset/ip_set_bitmap_port.c @@ -110,9 +110,17 @@ bitmap_port_kadt(struct ip_set *set, const struct sk_buff *skb, __be16 __port; u16 port = 0; - if (!ip_set_get_ip_port(skb, opt->family, - opt->flags & IPSET_DIM_ONE_SRC, &__port)) - return -EINVAL; + if (opt->cmdflags & IPSET_FLAG_INNER) { + if (!ip_set_get_inner_ip_port(skb, opt->family, + opt->flags & IPSET_DIM_ONE_SRC, + &__port)) + return -EINVAL; + } else { + if (!ip_set_get_ip_port(skb, opt->family, + opt->flags & IPSET_DIM_ONE_SRC, + &__port)) + return -EINVAL; + } port = ntohs(__port); diff --git a/kernel/net/netfilter/ipset/ip_set_getport.c b/kernel/net/netfilter/ipset/ip_set_getport.c index 6c019b3..f6ba3e8 100644 --- a/kernel/net/netfilter/ipset/ip_set_getport.c +++ b/kernel/net/netfilter/ipset/ip_set_getport.c @@ -19,6 +19,7 @@ #include <linux/netfilter_ipv6/ip6_tables.h> #include <net/ip.h> #include <net/ipv6.h> +#include <net/icmp.h> #include <linux/netfilter/ipset/ip_set_getport.h> @@ -128,6 +129,47 @@ ip_set_get_ip4_port(const struct sk_buff *skb, bool src, } EXPORT_SYMBOL_GPL(ip_set_get_ip4_port); +bool +ip_set_get_inner_ip4_port(const struct sk_buff *skb, bool src, + __be16 *port, u8 *proto) +{ + unsigned int protooff; + u8 type; + struct iphdr _iph; + struct icmphdr _icmph; + const struct iphdr *ih; + const struct icmphdr *ich; + static const size_t len = 8 + sizeof(struct iphdr); + + ih = skb_header_pointer(skb, 0, sizeof(_iph), &_iph); + if (ih == NULL || ih->protocol != IPPROTO_ICMP || + ntohs(ih->frag_off) & IP_OFFSET) + goto err; + + protooff = ih->ihl*4; + ich = skb_header_pointer(skb, protooff, sizeof(_icmph), &_icmph); + if (ich == NULL || + (ich->type <= NR_ICMP_TYPES && skb->len - protooff < len)) + goto err; + + type = ich->type; + if (type == ICMP_DEST_UNREACH || + type == ICMP_SOURCE_QUENCH || + type == ICMP_TIME_EXCEEDED) { + protooff += sizeof(_icmph); + ih = skb_header_pointer(skb, protooff, sizeof(_iph), &_iph); + if (ih == NULL || ntohs(ih->frag_off) & IP_OFFSET) + goto err; + + protooff += ih->ihl*4; + return get_port(skb, ih->protocol, protooff, src, + port, proto); + } +err: + return false; +} +EXPORT_SYMBOL_GPL(ip_set_get_inner_ip4_port); + #if defined(CONFIG_IP6_NF_IPTABLES) || defined(CONFIG_IP6_NF_IPTABLES_MODULE) bool ip_set_get_ip6_port(const struct sk_buff *skb, bool src, @@ -152,6 +194,109 @@ ip_set_get_ip6_port(const struct sk_buff *skb, bool src, return get_port(skb, nexthdr, protooff, src, port, proto); } EXPORT_SYMBOL_GPL(ip_set_get_ip6_port); + +#define ip6_ext_hdr(hdr) ((hdr == IPPROTO_HOPOPTS) || \ + (hdr == IPPROTO_ROUTING) || \ + (hdr == IPPROTO_FRAGMENT) || \ + (hdr == IPPROTO_ESP) || \ + (hdr == IPPROTO_AH) || \ + (hdr == IPPROTO_NONE) || \ + (hdr == IPPROTO_DSTOPTS)) + +bool process_ip6_hdr(const struct sk_buff *skb, u8 *currenthdr, + const unsigned int offset, unsigned int *ptr) +{ + unsigned int hdrlen = 0; + bool fragment = false; + struct ipv6hdr _ip6h; + const struct ipv6hdr *ih; + + ih = skb_header_pointer(skb, offset, sizeof(_ip6h), &_ip6h); + if (ih == NULL) + goto err; + + *ptr = offset + sizeof(struct ipv6hdr); + *currenthdr = ih->nexthdr; + while (*currenthdr != NEXTHDR_NONE && ip6_ext_hdr(*currenthdr)) { + struct ipv6_opt_hdr _hdr; + const struct ipv6_opt_hdr *hp; + + hp = skb_header_pointer(skb, *ptr, sizeof(_hdr), &_hdr); + if (hp == NULL) + goto err; + + switch (*currenthdr) { + case IPPROTO_FRAGMENT: { + struct frag_hdr _fhdr; + const struct frag_hdr *fh; + + fh = skb_header_pointer(skb, *ptr, sizeof(_fhdr), + &_fhdr); + if (fh == NULL) + goto err; + if (ntohs(fh->frag_off) & 0xFFF8) + fragment = true; + + hdrlen = 8; + break; + } + case IPPROTO_DSTOPTS: + case IPPROTO_ROUTING: + case IPPROTO_HOPOPTS: + if (fragment) + goto err; + + hdrlen = ipv6_optlen(hp); + break; + case IPPROTO_AH: + hdrlen = (hp->hdrlen+2)<<2; + break; + case IPPROTO_ESP: + default: + goto err; + } /* switch IPPROTO */ + + *currenthdr = hp->nexthdr; + *ptr += hdrlen; + } + return !fragment; + +err: + return false; +} + +bool +ip_set_get_inner_ip6_port(const struct sk_buff *skb, bool src, + __be16 *port, u8 *proto) +{ + u8 type, currenthdr = 0; + unsigned int ptr = 0; + struct icmp6hdr _icmp6h; + const struct icmp6hdr *ic; + + if (!process_ip6_hdr(skb, ¤thdr, 0, &ptr) || + currenthdr != IPPROTO_ICMPV6) + goto err; + + ic = skb_header_pointer(skb, ptr, sizeof(_icmp6h), &_icmp6h); + if (ic == NULL) + goto err; + + type = ic->icmp6_type; + if (type == ICMPV6_DEST_UNREACH || + type == ICMPV6_PKT_TOOBIG || + type == ICMPV6_TIME_EXCEED) { + if (!process_ip6_hdr(skb, ¤thdr, + ptr + sizeof(_icmp6h), &ptr) || ptr < 0) + goto err; + + return get_port(skb, currenthdr, ptr, src, port, proto); + } + +err: + return false; +} +EXPORT_SYMBOL_GPL(ip_set_get_inner_ip6_port); #endif bool @@ -181,3 +326,32 @@ ip_set_get_ip_port(const struct sk_buff *skb, u8 pf, bool src, __be16 *port) } } EXPORT_SYMBOL_GPL(ip_set_get_ip_port); + +bool +ip_set_get_inner_ip_port(const struct sk_buff *skb, u8 pf, bool src, + __be16 *port) +{ + bool ret; + u8 proto; + + switch (pf) { + case NFPROTO_IPV4: + ret = ip_set_get_inner_ip4_port(skb, src, port, &proto); + break; + case NFPROTO_IPV6: + ret = ip_set_get_inner_ip6_port(skb, src, port, &proto); + break; + default: + return false; + } + if (!ret) + return ret; + switch (proto) { + case IPPROTO_TCP: + case IPPROTO_UDP: + return true; + default: + return false; + } +} +EXPORT_SYMBOL_GPL(ip_set_get_inner_ip_port); diff --git a/kernel/net/netfilter/ipset/ip_set_hash_ip.c b/kernel/net/netfilter/ipset/ip_set_hash_ip.c index 260c9a8..e9b19a8 100644 --- a/kernel/net/netfilter/ipset/ip_set_hash_ip.c +++ b/kernel/net/netfilter/ipset/ip_set_hash_ip.c @@ -102,7 +102,11 @@ hash_ip4_kadt(struct ip_set *set, const struct sk_buff *skb, struct ip_set_ext ext = IP_SET_INIT_KEXT(skb, opt, h); __be32 ip; - ip4addrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, &ip); + if (opt->cmdflags & IPSET_FLAG_INNER) { + ip4inneraddrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, &ip); + } else { + ip4addrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, &ip); + } ip &= ip_set_netmask(h->netmask); if (ip == 0) return -EINVAL; @@ -255,7 +259,12 @@ hash_ip6_kadt(struct ip_set *set, const struct sk_buff *skb, struct hash_ip6_elem e = {}; struct ip_set_ext ext = IP_SET_INIT_KEXT(skb, opt, h); - ip6addrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, &e.ip.in6); + if (opt->cmdflags & IPSET_FLAG_INNER) { + ip6inneraddrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, + &e.ip.in6); + } else { + ip6addrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, &e.ip.in6); + } hash_ip6_netmask(&e.ip, h->netmask); if (ipv6_addr_any(&e.ip.in6)) return -EINVAL; diff --git a/kernel/net/netfilter/ipset/ip_set_hash_ipport.c b/kernel/net/netfilter/ipset/ip_set_hash_ipport.c index 64caad3..f3528ff 100644 --- a/kernel/net/netfilter/ipset/ip_set_hash_ipport.c +++ b/kernel/net/netfilter/ipset/ip_set_hash_ipport.c @@ -121,11 +121,22 @@ hash_ipport4_kadt(struct ip_set *set, const struct sk_buff *skb, struct hash_ipport4_elem e = { }; struct ip_set_ext ext = IP_SET_INIT_KEXT(skb, opt, h); - if (!ip_set_get_ip4_port(skb, opt->flags & IPSET_DIM_TWO_SRC, - &e.port, &e.proto)) - return -EINVAL; + if (opt->cmdflags & IPSET_FLAG_INNER) { + if (!ip_set_get_inner_ip4_port(skb, + opt->flags & IPSET_DIM_TWO_SRC, + &e.port, &e.proto)) + return -EINVAL; + + ip4inneraddrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, + &e.ip); + } else { + if (!ip_set_get_ip4_port(skb, opt->flags & IPSET_DIM_TWO_SRC, + &e.port, &e.proto)) + return -EINVAL; + + ip4addrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, &e.ip); + } - ip4addrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, &e.ip); return adtfn(set, &e, &ext, &opt->ext, opt->cmdflags); } @@ -311,11 +322,21 @@ hash_ipport6_kadt(struct ip_set *set, const struct sk_buff *skb, struct hash_ipport6_elem e = { }; struct ip_set_ext ext = IP_SET_INIT_KEXT(skb, opt, h); - if (!ip_set_get_ip6_port(skb, opt->flags & IPSET_DIM_TWO_SRC, - &e.port, &e.proto)) - return -EINVAL; + if (opt->cmdflags & IPSET_FLAG_INNER) { + if (!ip_set_get_inner_ip6_port(skb, + opt->flags & IPSET_DIM_TWO_SRC, + &e.port, &e.proto)) + return -EINVAL; + + ip6inneraddrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, + &e.ip.in6); + } else { + if (!ip_set_get_ip6_port(skb, opt->flags & IPSET_DIM_TWO_SRC, + &e.port, &e.proto)) + return -EINVAL; - ip6addrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, &e.ip.in6); + ip6addrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, &e.ip.in6); + } return adtfn(set, &e, &ext, &opt->ext, opt->cmdflags); } diff --git a/kernel/net/netfilter/ipset/ip_set_hash_ipportip.c b/kernel/net/netfilter/ipset/ip_set_hash_ipportip.c index 2873bbc..8b3bdee 100644 --- a/kernel/net/netfilter/ipset/ip_set_hash_ipportip.c +++ b/kernel/net/netfilter/ipset/ip_set_hash_ipportip.c @@ -125,12 +125,23 @@ hash_ipportip4_kadt(struct ip_set *set, const struct sk_buff *skb, struct hash_ipportip4_elem e = { }; struct ip_set_ext ext = IP_SET_INIT_KEXT(skb, opt, h); - if (!ip_set_get_ip4_port(skb, opt->flags & IPSET_DIM_TWO_SRC, - &e.port, &e.proto)) - return -EINVAL; - - ip4addrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, &e.ip); - ip4addrptr(skb, opt->flags & IPSET_DIM_THREE_SRC, &e.ip2); + if (opt->cmdflags & IPSET_FLAG_INNER) { + if (!ip_set_get_inner_ip4_port(skb, + opt->flags & IPSET_DIM_TWO_SRC, + &e.port, &e.proto)) + return -EINVAL; + + ip4inneraddrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, &e.ip); + ip4inneraddrptr(skb, opt->flags & IPSET_DIM_THREE_SRC, + &e.ip2); + } else { + if (!ip_set_get_ip4_port(skb, opt->flags & IPSET_DIM_TWO_SRC, + &e.port, &e.proto)) + return -EINVAL; + + ip4addrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, &e.ip); + ip4addrptr(skb, opt->flags & IPSET_DIM_THREE_SRC, &e.ip2); + } return adtfn(set, &e, &ext, &opt->ext, opt->cmdflags); } @@ -324,12 +335,24 @@ hash_ipportip6_kadt(struct ip_set *set, const struct sk_buff *skb, struct hash_ipportip6_elem e = { }; struct ip_set_ext ext = IP_SET_INIT_KEXT(skb, opt, h); - if (!ip_set_get_ip6_port(skb, opt->flags & IPSET_DIM_TWO_SRC, - &e.port, &e.proto)) - return -EINVAL; - - ip6addrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, &e.ip.in6); - ip6addrptr(skb, opt->flags & IPSET_DIM_THREE_SRC, &e.ip2.in6); + if (opt->cmdflags & IPSET_FLAG_INNER) { + if (!ip_set_get_inner_ip6_port(skb, + opt->flags & IPSET_DIM_TWO_SRC, + &e.port, &e.proto)) + return -EINVAL; + + ip6inneraddrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, + &e.ip.in6); + ip6inneraddrptr(skb, opt->flags & IPSET_DIM_THREE_SRC, + &e.ip2.in6); + } else { + if (!ip_set_get_ip6_port(skb, opt->flags & IPSET_DIM_TWO_SRC, + &e.port, &e.proto)) + return -EINVAL; + + ip6addrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, &e.ip.in6); + ip6addrptr(skb, opt->flags & IPSET_DIM_THREE_SRC, &e.ip2.in6); + } return adtfn(set, &e, &ext, &opt->ext, opt->cmdflags); } diff --git a/kernel/net/netfilter/ipset/ip_set_hash_ipportnet.c b/kernel/net/netfilter/ipset/ip_set_hash_ipportnet.c index db0e761..154c836 100644 --- a/kernel/net/netfilter/ipset/ip_set_hash_ipportnet.c +++ b/kernel/net/netfilter/ipset/ip_set_hash_ipportnet.c @@ -177,12 +177,24 @@ hash_ipportnet4_kadt(struct ip_set *set, const struct sk_buff *skb, if (adt == IPSET_TEST) e.cidr = HOST_MASK - 1; - if (!ip_set_get_ip4_port(skb, opt->flags & IPSET_DIM_TWO_SRC, - &e.port, &e.proto)) - return -EINVAL; - - ip4addrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, &e.ip); - ip4addrptr(skb, opt->flags & IPSET_DIM_THREE_SRC, &e.ip2); + if (opt->cmdflags & IPSET_FLAG_INNER) { + if (!ip_set_get_inner_ip4_port(skb, + opt->flags & IPSET_DIM_TWO_SRC, + &e.port, &e.proto)) + return -EINVAL; + + ip4inneraddrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, + &e.ip); + ip4inneraddrptr(skb, opt->flags & IPSET_DIM_THREE_SRC, + &e.ip2); + } else { + if (!ip_set_get_ip4_port(skb, opt->flags & IPSET_DIM_TWO_SRC, + &e.port, &e.proto)) + return -EINVAL; + + ip4addrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, &e.ip); + ip4addrptr(skb, opt->flags & IPSET_DIM_THREE_SRC, &e.ip2); + } e.ip2 &= ip_set_netmask(e.cidr + 1); return adtfn(set, &e, &ext, &opt->ext, opt->cmdflags); @@ -461,12 +473,25 @@ hash_ipportnet6_kadt(struct ip_set *set, const struct sk_buff *skb, if (adt == IPSET_TEST) e.cidr = HOST_MASK - 1; - if (!ip_set_get_ip6_port(skb, opt->flags & IPSET_DIM_TWO_SRC, - &e.port, &e.proto)) - return -EINVAL; - - ip6addrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, &e.ip.in6); - ip6addrptr(skb, opt->flags & IPSET_DIM_THREE_SRC, &e.ip2.in6); + if (opt->cmdflags & IPSET_FLAG_INNER) { + if (!ip_set_get_inner_ip6_port(skb, + opt->flags & IPSET_DIM_TWO_SRC, + &e.port, &e.proto)) + return -EINVAL; + + ip6inneraddrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, + &e.ip.in6); + ip6inneraddrptr(skb, opt->flags & IPSET_DIM_THREE_SRC, + &e.ip2.in6); + } else { + if (!ip_set_get_ip6_port(skb, opt->flags & IPSET_DIM_TWO_SRC, + &e.port, &e.proto)) + return -EINVAL; + + ip6addrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, &e.ip.in6); + ip6addrptr(skb, opt->flags & IPSET_DIM_THREE_SRC, + &e.ip2.in6); + } ip6_netmask(&e.ip2, e.cidr + 1); return adtfn(set, &e, &ext, &opt->ext, opt->cmdflags); diff --git a/kernel/net/netfilter/ipset/ip_set_hash_net.c b/kernel/net/netfilter/ipset/ip_set_hash_net.c index 846ec80..431706a 100644 --- a/kernel/net/netfilter/ipset/ip_set_hash_net.c +++ b/kernel/net/netfilter/ipset/ip_set_hash_net.c @@ -151,8 +151,13 @@ hash_net4_kadt(struct ip_set *set, const struct sk_buff *skb, return -EINVAL; if (adt == IPSET_TEST) e.cidr = HOST_MASK; + if (opt->cmdflags & IPSET_FLAG_INNER) { + ip4inneraddrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, + &e.ip); + } else { + ip4addrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, &e.ip); + } - ip4addrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, &e.ip); e.ip &= ip_set_netmask(e.cidr); return adtfn(set, &e, &ext, &opt->ext, opt->cmdflags); @@ -346,8 +351,12 @@ hash_net6_kadt(struct ip_set *set, const struct sk_buff *skb, return -EINVAL; if (adt == IPSET_TEST) e.cidr = HOST_MASK; - - ip6addrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, &e.ip.in6); + if (opt->cmdflags & IPSET_FLAG_INNER) { + ip6inneraddrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, + &e.ip.in6); + } else { + ip6addrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, &e.ip.in6); + } ip6_netmask(&e.ip, e.cidr); return adtfn(set, &e, &ext, &opt->ext, opt->cmdflags); diff --git a/kernel/net/netfilter/ipset/ip_set_hash_netiface.c b/kernel/net/netfilter/ipset/ip_set_hash_netiface.c index 8f0e496..51415fb 100644 --- a/kernel/net/netfilter/ipset/ip_set_hash_netiface.c +++ b/kernel/net/netfilter/ipset/ip_set_hash_netiface.c @@ -275,8 +275,12 @@ hash_netiface4_kadt(struct ip_set *set, const struct sk_buff *skb, return -EINVAL; if (adt == IPSET_TEST) e.cidr = HOST_MASK; - - ip4addrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, &e.ip); + if (opt->cmdflags & IPSET_FLAG_INNER) { + ip4inneraddrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, + &e.ip); + } else { + ip4addrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, &e.ip); + } e.ip &= ip_set_netmask(e.cidr); #define IFACE(dir) (par->dir ? par->dir->name : NULL) @@ -544,8 +548,12 @@ hash_netiface6_kadt(struct ip_set *set, const struct sk_buff *skb, return -EINVAL; if (adt == IPSET_TEST) e.cidr = HOST_MASK; - - ip6addrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, &e.ip.in6); + if (opt->cmdflags & IPSET_FLAG_INNER) { + ip6inneraddrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, + &e.ip.in6); + } else { + ip6addrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, &e.ip.in6); + } ip6_netmask(&e.ip, e.cidr); if (opt->cmdflags & IPSET_FLAG_PHYSDEV) { diff --git a/kernel/net/netfilter/ipset/ip_set_hash_netport.c b/kernel/net/netfilter/ipset/ip_set_hash_netport.c index 021d716..376a28e 100644 --- a/kernel/net/netfilter/ipset/ip_set_hash_netport.c +++ b/kernel/net/netfilter/ipset/ip_set_hash_netport.c @@ -169,11 +169,21 @@ hash_netport4_kadt(struct ip_set *set, const struct sk_buff *skb, if (adt == IPSET_TEST) e.cidr = HOST_MASK - 1; - if (!ip_set_get_ip4_port(skb, opt->flags & IPSET_DIM_TWO_SRC, - &e.port, &e.proto)) - return -EINVAL; - - ip4addrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, &e.ip); + if (opt->cmdflags & IPSET_FLAG_INNER) { + if (!ip_set_get_inner_ip4_port(skb, + opt->flags & IPSET_DIM_TWO_SRC, + &e.port, &e.proto)) + return -EINVAL; + + ip4inneraddrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, + &e.ip); + } else { + if (!ip_set_get_ip4_port(skb, opt->flags & IPSET_DIM_TWO_SRC, + &e.port, &e.proto)) + return -EINVAL; + + ip4addrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, &e.ip); + } e.ip &= ip_set_netmask(e.cidr + 1); return adtfn(set, &e, &ext, &opt->ext, opt->cmdflags); @@ -413,12 +423,21 @@ hash_netport6_kadt(struct ip_set *set, const struct sk_buff *skb, if (adt == IPSET_TEST) e.cidr = HOST_MASK - 1; - - if (!ip_set_get_ip6_port(skb, opt->flags & IPSET_DIM_TWO_SRC, - &e.port, &e.proto)) - return -EINVAL; - - ip6addrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, &e.ip.in6); + if (opt->cmdflags & IPSET_FLAG_INNER) { + if (!ip_set_get_inner_ip6_port(skb, + opt->flags & IPSET_DIM_TWO_SRC, + &e.port, &e.proto)) + return -EINVAL; + + ip6inneraddrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, + &e.ip.in6); + } else { + if (!ip_set_get_ip6_port(skb, opt->flags & IPSET_DIM_TWO_SRC, + &e.port, &e.proto)) + return -EINVAL; + + ip6addrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, &e.ip.in6); + } ip6_netmask(&e.ip, e.cidr + 1); return adtfn(set, &e, &ext, &opt->ext, opt->cmdflags); ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/4] ipset: add "inner" flag implementation 2013-05-29 0:13 ` [PATCH 2/4] ipset: add "inner" flag implementation Dash Four @ 2013-06-01 10:54 ` Jozsef Kadlecsik 2013-06-07 1:13 ` Dash Four 0 siblings, 1 reply; 7+ messages in thread From: Jozsef Kadlecsik @ 2013-06-01 10:54 UTC (permalink / raw) To: Dash Four; +Cc: Pablo Neira Ayuso, Netfilter Core Team Hi, On Wed, 29 May 2013, Dash Four wrote: > This patch implements "inner" flag option in the set iptables match, > allowing matching based on the properties (source/destination IP address, > protocol, port and so on) of the original (inner) connection in the event > of the following ICMP[v4,v6] messages: > > ICMPv4 destination-unreachable (code 3); > ICMPv4 source-quench (code 4); > ICMPv4 time-exceeded (code 11); > ICMPv6 destination-unreachable (code 1); > ICMPv6 packet-too-big (code 2); > ICMPv6 time-exceeded (code 3); There are a few problems with your patch: some things are duplicated, already existing function is reimplemented. There is no error path when the inner IPv4/6 header is broken: the returned zero IP address is valid for example in the case of the hash:net,iface type. Also, I'd prefer a single new function (or two inline functions if their size permits), which'd return the success code and in pointers the inner IP header and the offset to the header (for proto/ports). Read my comments below. > Signed-off-by: Dash Four <mr.dash.four@googlemail.com> > --- > kernel/include/linux/netfilter/ipset/ip_set.h | 167 +++++++++++++++++++ > .../include/linux/netfilter/ipset/ip_set_getport.h | 16 ++ > kernel/include/uapi/linux/netfilter/ipset/ip_set.h | 2 + > kernel/net/netfilter/ipset/ip_set_bitmap_ip.c | 4 +- > kernel/net/netfilter/ipset/ip_set_bitmap_ipmac.c | 4 +- > kernel/net/netfilter/ipset/ip_set_bitmap_port.c | 14 +- > kernel/net/netfilter/ipset/ip_set_getport.c | 174 > ++++++++++++++++++++ > kernel/net/netfilter/ipset/ip_set_hash_ip.c | 13 +- > kernel/net/netfilter/ipset/ip_set_hash_ipport.c | 37 ++++- > kernel/net/netfilter/ipset/ip_set_hash_ipportip.c | 47 ++++-- > kernel/net/netfilter/ipset/ip_set_hash_ipportnet.c | 49 ++++-- > kernel/net/netfilter/ipset/ip_set_hash_net.c | 15 +- > kernel/net/netfilter/ipset/ip_set_hash_netiface.c | 16 +- > kernel/net/netfilter/ipset/ip_set_hash_netport.c | 41 +++-- > 14 files changed, 542 insertions(+), 57 deletions(-) > > diff --git a/kernel/include/linux/netfilter/ipset/ip_set.h > b/kernel/include/linux/netfilter/ipset/ip_set.h > index 8499e25..ac29e16 100644 > --- a/kernel/include/linux/netfilter/ipset/ip_set.h > +++ b/kernel/include/linux/netfilter/ipset/ip_set.h > @@ -17,6 +17,9 @@ > #include <linux/netfilter/x_tables.h> > #include <linux/stringify.h> > #include <linux/vmalloc.h> > +#include <net/ip.h> > +#include <net/ipv6.h> > +#include <net/icmp.h> > #include <net/netlink.h> > #include <uapi/linux/netfilter/ipset/ip_set.h> > #include <linux/netfilter/ipset/ip_set_compat.h> > @@ -373,6 +376,79 @@ ip4addrptr(const struct sk_buff *skb, bool src, __be32 > *addr) > *addr = src ? ip_hdr(skb)->saddr : ip_hdr(skb)->daddr; > } > > +static inline __be32 > +ip4inneraddr(const struct sk_buff *skb, bool src) > +{ > + struct iphdr _iph; > + struct icmphdr _icmph; > + u8 type; > + const struct iphdr *ih; > + const struct icmphdr *ich; > + static const size_t len = 8 + sizeof(struct iphdr); > + > + ih = skb_header_pointer(skb, 0, sizeof(_iph), &_iph); > + if (ih == NULL || ih->protocol != IPPROTO_ICMP || > + ntohs(ih->frag_off) & IP_OFFSET) > + goto err; > + > + ich = skb_header_pointer(skb, ih->ihl*4, > + sizeof(_icmph), &_icmph); > + if (ich == NULL || > + (ich->type <= NR_ICMP_TYPES && skb->len - ih->ihl*4 < len)) > + goto err; > + > + type = ich->type; > + if (type == ICMP_DEST_UNREACH || > + type == ICMP_SOURCE_QUENCH || > + type == ICMP_TIME_EXCEEDED) { > + ih = skb_header_pointer(skb, ih->ihl*4 + sizeof(_icmph), > + sizeof(_iph), &_iph); > + if (ih == NULL || ntohs(ih->frag_off) & IP_OFFSET) > + goto err; > + > + return src ? ih->saddr : ih->daddr; > + } > +err: > + return 0; > +} ip4inneraddr is a duplication of ip4inneraddrptr: if this were the way, then ip4inneraddr should simply wrap and call ip4inneraddrptr. > +static inline void > +ip4inneraddrptr(const struct sk_buff *skb, bool src, __be32 *addr) > +{ > + struct iphdr _iph; > + struct icmphdr _icmph; > + u8 type; > + const struct iphdr *ih; > + const struct icmphdr *ich; > + static const size_t len = 8 + sizeof(struct iphdr); I don't like bare numbers as magic constants. > + ih = skb_header_pointer(skb, 0, sizeof(_iph), &_iph); > + if (ih == NULL || ntohs(ih->frag_off) & IP_OFFSET || > + ih->protocol != IPPROTO_ICMP) > + goto err; > + > + ich = skb_header_pointer(skb, ih->ihl*4, > + sizeof(_icmph), &_icmph); > + if (ich == NULL || > + (ich->type <= NR_ICMP_TYPES && skb->len - ih->ihl*4 < len)) > + goto err; > + > + type = ich->type; > + if (type == ICMP_DEST_UNREACH || > + type == ICMP_SOURCE_QUENCH || > + type == ICMP_TIME_EXCEEDED) { > + ih = skb_header_pointer(skb, ih->ihl*4 + sizeof(_icmph), > + sizeof(_iph), &_iph); > + if (ih == NULL || ntohs(ih->frag_off) & IP_OFFSET) > + goto err; > + > + *addr = src ? ih->saddr : ih->daddr; > + return; > + } > +err: > + *addr = 0; > +} > + > static inline void > ip6addrptr(const struct sk_buff *skb, bool src, struct in6_addr *addr) > { > @@ -380,6 +456,97 @@ ip6addrptr(const struct sk_buff *skb, bool src, struct > in6_addr *addr) > sizeof(*addr)); > } > > +#define ip6_ext_hdr(hdr) ((hdr == IPPROTO_HOPOPTS) || \ > + (hdr == IPPROTO_ROUTING) || \ > + (hdr == IPPROTO_FRAGMENT) || \ > + (hdr == IPPROTO_ESP) || \ > + (hdr == IPPROTO_AH) || \ > + (hdr == IPPROTO_NONE) || \ > + (hdr == IPPROTO_DSTOPTS)) > +static inline void > +ip6inneraddrptr(const struct sk_buff *skb, bool src, struct in6_addr *addr) > +{ > + u8 type, currenthdr; > + bool fragment = false; > + unsigned int ptr, hdrlen = 0; > + struct ipv6hdr _ip6h; > + struct icmp6hdr _icmp6h; > + const struct ipv6hdr *ih; > + const struct icmp6hdr *ic; > + > + ih = skb_header_pointer(skb, 0, sizeof(_ip6h), &_ip6h); > + if (ih == NULL) > + goto err; > + > + ptr = sizeof(struct ipv6hdr); > + currenthdr = ih->nexthdr; > + while (currenthdr != NEXTHDR_NONE && ip6_ext_hdr(currenthdr)) { > + struct ipv6_opt_hdr _hdr; > + const struct ipv6_opt_hdr *hp; > + > + hp = skb_header_pointer(skb, ptr, sizeof(_hdr), &_hdr); > + if (hp == NULL) > + goto err; > + > + switch (currenthdr) { > + case IPPROTO_FRAGMENT: { > + struct frag_hdr _fhdr; > + const struct frag_hdr *fh; > + > + fh = skb_header_pointer(skb, ptr, sizeof(_fhdr), > + &_fhdr); > + if (fh == NULL) > + goto err; > + if (ntohs(fh->frag_off) & 0xFFF8) > + fragment = true; > + > + hdrlen = 8; > + break; > + } > + case IPPROTO_DSTOPTS: > + case IPPROTO_ROUTING: > + case IPPROTO_HOPOPTS: > + if (fragment) > + goto err; > + > + hdrlen = ipv6_optlen(hp); > + break; > + case IPPROTO_AH: > + hdrlen = (hp->hdrlen+2)<<2; > + break; > + case IPPROTO_ESP: > + default: > + goto err; > + } /* switch IPPROTO */ > + > + currenthdr = hp->nexthdr; > + ptr += hdrlen; > + } The code segment above is the reimplementation of ipv6_skip_exthdr. > + if (fragment || currenthdr != IPPROTO_ICMPV6) > + goto err; > + > + ic = skb_header_pointer(skb, ptr, sizeof(_icmp6h), &_icmp6h); > + if (ic == NULL) > + goto err; > + > + type = ic->icmp6_type; > + if (type == ICMPV6_DEST_UNREACH || > + type == ICMPV6_PKT_TOOBIG || > + type == ICMPV6_TIME_EXCEED) { > + ih = skb_header_pointer(skb, ptr + sizeof(_icmp6h), > + sizeof(_ip6h), &_ip6h); > + if (ih == NULL) > + goto err; > + > + memcpy(addr, src ? &ih->saddr : &ih->daddr, > + sizeof(*addr)); > + return; > + } > + > +err: > + memset(addr, 0, sizeof(*addr)); > +} Zero valued IP addresses are valid, so it's not a proper error handling in the ip[46]inneraddrptr functions. These all can be replaced by a function which skips the external header and points to the inner one, something like: int ipset_inner_header(const struct sk_buff *skb, u8 pf, u8 *offset, void *inner) { /* IPv4 */ ... /* IPv6 */ nexthdr = ipv6_hdr(skb)->nexthdr; *offset = ipv6_skip_exthdr(skb, sizeof(struct ipv6hdr), &nexthdr, &frag_off); /* Validate offset and frag_off */ /* Check ICMPv6, update offset */ /* Copy inner IPv6 header to *inner by skb_header_pointer */ return success/error; } Please note, I updated the ipset git tree with a patch which adds the a missing verification of frag_off to ip_set_get_ip6_port(). > /* Calculate the bytes required to store the inclusive range of a-b */ > static inline int > bitmap_bytes(u32 a, u32 b) > diff --git a/kernel/include/linux/netfilter/ipset/ip_set_getport.h > b/kernel/include/linux/netfilter/ipset/ip_set_getport.h > index 90d0930..549d013 100644 > --- a/kernel/include/linux/netfilter/ipset/ip_set_getport.h > +++ b/kernel/include/linux/netfilter/ipset/ip_set_getport.h > @@ -4,20 +4,36 @@ > extern bool ip_set_get_ip4_port(const struct sk_buff *skb, bool src, > __be16 *port, u8 *proto); > > +extern bool ip_set_get_inner_ip4_port(const struct sk_buff *skb, bool src, > + __be16 *port, u8 *proto); > + > #if defined(CONFIG_IP6_NF_IPTABLES) || defined(CONFIG_IP6_NF_IPTABLES_MODULE) > extern bool ip_set_get_ip6_port(const struct sk_buff *skb, bool src, > __be16 *port, u8 *proto); > + > +extern bool ip_set_get_inner_ip6_port(const struct sk_buff *skb, bool src, > + __be16 *port, u8 *proto); > #else > static inline bool ip_set_get_ip6_port(const struct sk_buff *skb, bool src, > __be16 *port, u8 *proto) > { > return false; > } > + > +static inline bool ip_set_get_inner_ip6_port(const struct sk_buff *skb, > + bool src, > + __be16 *port, u8 *proto) > +{ > + return false; > +} > #endif > > extern bool ip_set_get_ip_port(const struct sk_buff *skb, u8 pf, bool src, > __be16 *port); > > +extern bool ip_set_get_inner_ip_port(const struct sk_buff *skb, u8 pf, > + bool src, __be16 *port); > + > static inline bool ip_set_proto_with_ports(u8 proto) > { > switch (proto) { > diff --git a/kernel/include/uapi/linux/netfilter/ipset/ip_set.h > b/kernel/include/uapi/linux/netfilter/ipset/ip_set.h > index 8024cdf..e9e6586 100644 > --- a/kernel/include/uapi/linux/netfilter/ipset/ip_set.h > +++ b/kernel/include/uapi/linux/netfilter/ipset/ip_set.h > @@ -161,6 +161,8 @@ enum ipset_cmd_flags { > (1 << IPSET_FLAG_BIT_SKIP_SUBCOUNTER_UPDATE), > IPSET_FLAG_BIT_MATCH_COUNTERS = 5, > IPSET_FLAG_MATCH_COUNTERS = (1 << IPSET_FLAG_BIT_MATCH_COUNTERS), > + IPSET_FLAG_BIT_INNER = 6, > + IPSET_FLAG_INNER = (1 << IPSET_FLAG_BIT_INNER), > IPSET_FLAG_BIT_RETURN_NOMATCH = 7, > IPSET_FLAG_RETURN_NOMATCH = (1 << IPSET_FLAG_BIT_RETURN_NOMATCH), > IPSET_FLAG_CMD_MAX = 15, > diff --git a/kernel/net/netfilter/ipset/ip_set_bitmap_ip.c > b/kernel/net/netfilter/ipset/ip_set_bitmap_ip.c > index ce99d26..7552d6d 100644 > --- a/kernel/net/netfilter/ipset/ip_set_bitmap_ip.c > +++ b/kernel/net/netfilter/ipset/ip_set_bitmap_ip.c > @@ -116,7 +116,9 @@ bitmap_ip_kadt(struct ip_set *set, const struct sk_buff > *skb, > struct ip_set_ext ext = IP_SET_INIT_KEXT(skb, opt, map); > u32 ip; > > - ip = ntohl(ip4addr(skb, opt->flags & IPSET_DIM_ONE_SRC)); > + ip = opt->cmdflags & IPSET_FLAG_INNER ? > + ntohl(ip4inneraddr(skb, opt->flags & IPSET_DIM_ONE_SRC)) : > + ntohl(ip4addr(skb, opt->flags & IPSET_DIM_ONE_SRC)); > if (ip < map->first_ip || ip > map->last_ip) > return -IPSET_ERR_BITMAP_RANGE; With the ipset_inner_header function, all these looks like this: we need new variables for the IP header, pointer to it and offset. If the IPSET_FLAG_INNER is true, ipset_inner_header is called and the returned error code handled or the pointer to IP header is updated. Something like this: struct ip_set_ext ext = IP_SET_INIT_KEXT(skb, opt, map); u32 ip; struct iphdr _iphdr, *iphdr = ip_hdr(skb); u8 offset = 0; if (opt->cmdflags & IPSET_FLAG_INNER) { if (ipset_inner_header(skb, NFPROTO_IPV4, &offset, &_iphdr)) return -EINVAL; iphdr = &_iphdr; } .... Current ip[46]addr[ptr] functions of course need to be modified to use the IP header structures instead of the skbuff one. > diff --git a/kernel/net/netfilter/ipset/ip_set_bitmap_ipmac.c > b/kernel/net/netfilter/ipset/ip_set_bitmap_ipmac.c > index 6d5bad9..df5f3b4 100644 > --- a/kernel/net/netfilter/ipset/ip_set_bitmap_ipmac.c > +++ b/kernel/net/netfilter/ipset/ip_set_bitmap_ipmac.c > @@ -223,7 +223,9 @@ bitmap_ipmac_kadt(struct ip_set *set, const struct sk_buff > *skb, > if (!(opt->flags & IPSET_DIM_TWO_SRC)) > return 0; > > - ip = ntohl(ip4addr(skb, opt->flags & IPSET_DIM_ONE_SRC)); > + ip = opt->cmdflags & IPSET_FLAG_INNER ? > + ntohl(ip4inneraddr(skb, opt->flags & IPSET_DIM_ONE_SRC)) : > + ntohl(ip4addr(skb, opt->flags & IPSET_DIM_ONE_SRC)); > if (ip < map->first_ip || ip > map->last_ip) > return -IPSET_ERR_BITMAP_RANGE; > > diff --git a/kernel/net/netfilter/ipset/ip_set_bitmap_port.c > b/kernel/net/netfilter/ipset/ip_set_bitmap_port.c > index b220489..d29395b 100644 > --- a/kernel/net/netfilter/ipset/ip_set_bitmap_port.c > +++ b/kernel/net/netfilter/ipset/ip_set_bitmap_port.c > @@ -110,9 +110,17 @@ bitmap_port_kadt(struct ip_set *set, const struct sk_buff > *skb, > __be16 __port; > u16 port = 0; > > - if (!ip_set_get_ip_port(skb, opt->family, > - opt->flags & IPSET_DIM_ONE_SRC, &__port)) > - return -EINVAL; > + if (opt->cmdflags & IPSET_FLAG_INNER) { > + if (!ip_set_get_inner_ip_port(skb, opt->family, > + opt->flags & IPSET_DIM_ONE_SRC, > + &__port)) > + return -EINVAL; > + } else { > + if (!ip_set_get_ip_port(skb, opt->family, > + opt->flags & IPSET_DIM_ONE_SRC, > + &__port)) > + return -EINVAL; > + } > > port = ntohs(__port); The ip_set_get*port functions need the new offset argument too. > diff --git a/kernel/net/netfilter/ipset/ip_set_getport.c > b/kernel/net/netfilter/ipset/ip_set_getport.c > index 6c019b3..f6ba3e8 100644 > --- a/kernel/net/netfilter/ipset/ip_set_getport.c > +++ b/kernel/net/netfilter/ipset/ip_set_getport.c > @@ -19,6 +19,7 @@ > #include <linux/netfilter_ipv6/ip6_tables.h> > #include <net/ip.h> > #include <net/ipv6.h> > +#include <net/icmp.h> > > #include <linux/netfilter/ipset/ip_set_getport.h> > > @@ -128,6 +129,47 @@ ip_set_get_ip4_port(const struct sk_buff *skb, bool src, > } > EXPORT_SYMBOL_GPL(ip_set_get_ip4_port); > > +bool > +ip_set_get_inner_ip4_port(const struct sk_buff *skb, bool src, > + __be16 *port, u8 *proto) > +{ > + unsigned int protooff; > + u8 type; > + struct iphdr _iph; > + struct icmphdr _icmph; > + const struct iphdr *ih; > + const struct icmphdr *ich; > + static const size_t len = 8 + sizeof(struct iphdr); > + > + ih = skb_header_pointer(skb, 0, sizeof(_iph), &_iph); > + if (ih == NULL || ih->protocol != IPPROTO_ICMP || > + ntohs(ih->frag_off) & IP_OFFSET) > + goto err; > + > + protooff = ih->ihl*4; > + ich = skb_header_pointer(skb, protooff, sizeof(_icmph), &_icmph); > + if (ich == NULL || > + (ich->type <= NR_ICMP_TYPES && skb->len - protooff < len)) > + goto err; > + > + type = ich->type; > + if (type == ICMP_DEST_UNREACH || > + type == ICMP_SOURCE_QUENCH || > + type == ICMP_TIME_EXCEEDED) { > + protooff += sizeof(_icmph); > + ih = skb_header_pointer(skb, protooff, sizeof(_iph), &_iph); > + if (ih == NULL || ntohs(ih->frag_off) & IP_OFFSET) > + goto err; > + > + protooff += ih->ihl*4; > + return get_port(skb, ih->protocol, protooff, src, > + port, proto); > + } > +err: > + return false; > +} > +EXPORT_SYMBOL_GPL(ip_set_get_inner_ip4_port); > + > #if defined(CONFIG_IP6_NF_IPTABLES) || defined(CONFIG_IP6_NF_IPTABLES_MODULE) > bool > ip_set_get_ip6_port(const struct sk_buff *skb, bool src, > @@ -152,6 +194,109 @@ ip_set_get_ip6_port(const struct sk_buff *skb, bool src, > return get_port(skb, nexthdr, protooff, src, port, proto); > } > EXPORT_SYMBOL_GPL(ip_set_get_ip6_port); > + > +#define ip6_ext_hdr(hdr) ((hdr == IPPROTO_HOPOPTS) || \ > + (hdr == IPPROTO_ROUTING) || \ > + (hdr == IPPROTO_FRAGMENT) || \ > + (hdr == IPPROTO_ESP) || \ > + (hdr == IPPROTO_AH) || \ > + (hdr == IPPROTO_NONE) || \ > + (hdr == IPPROTO_DSTOPTS)) > + > +bool process_ip6_hdr(const struct sk_buff *skb, u8 *currenthdr, > + const unsigned int offset, unsigned int *ptr) > +{ > + unsigned int hdrlen = 0; > + bool fragment = false; > + struct ipv6hdr _ip6h; > + const struct ipv6hdr *ih; > + > + ih = skb_header_pointer(skb, offset, sizeof(_ip6h), &_ip6h); > + if (ih == NULL) > + goto err; > + > + *ptr = offset + sizeof(struct ipv6hdr); > + *currenthdr = ih->nexthdr; > + while (*currenthdr != NEXTHDR_NONE && ip6_ext_hdr(*currenthdr)) { > + struct ipv6_opt_hdr _hdr; > + const struct ipv6_opt_hdr *hp; > + > + hp = skb_header_pointer(skb, *ptr, sizeof(_hdr), &_hdr); > + if (hp == NULL) > + goto err; > + > + switch (*currenthdr) { > + case IPPROTO_FRAGMENT: { > + struct frag_hdr _fhdr; > + const struct frag_hdr *fh; > + > + fh = skb_header_pointer(skb, *ptr, sizeof(_fhdr), > + &_fhdr); > + if (fh == NULL) > + goto err; > + if (ntohs(fh->frag_off) & 0xFFF8) > + fragment = true; > + > + hdrlen = 8; > + break; > + } > + case IPPROTO_DSTOPTS: > + case IPPROTO_ROUTING: > + case IPPROTO_HOPOPTS: > + if (fragment) > + goto err; > + > + hdrlen = ipv6_optlen(hp); > + break; > + case IPPROTO_AH: > + hdrlen = (hp->hdrlen+2)<<2; > + break; > + case IPPROTO_ESP: > + default: > + goto err; > + } /* switch IPPROTO */ > + > + *currenthdr = hp->nexthdr; > + *ptr += hdrlen; > + } > + return !fragment; > + > +err: > + return false; > +} > + > +bool > +ip_set_get_inner_ip6_port(const struct sk_buff *skb, bool src, > + __be16 *port, u8 *proto) > +{ > + u8 type, currenthdr = 0; > + unsigned int ptr = 0; > + struct icmp6hdr _icmp6h; > + const struct icmp6hdr *ic; > + > + if (!process_ip6_hdr(skb, ¤thdr, 0, &ptr) || > + currenthdr != IPPROTO_ICMPV6) > + goto err; > + > + ic = skb_header_pointer(skb, ptr, sizeof(_icmp6h), &_icmp6h); > + if (ic == NULL) > + goto err; > + > + type = ic->icmp6_type; > + if (type == ICMPV6_DEST_UNREACH || > + type == ICMPV6_PKT_TOOBIG || > + type == ICMPV6_TIME_EXCEED) { > + if (!process_ip6_hdr(skb, ¤thdr, > + ptr + sizeof(_icmp6h), &ptr) || ptr < 0) > + goto err; > + > + return get_port(skb, currenthdr, ptr, src, port, proto); > + } > + > +err: > + return false; > +} > +EXPORT_SYMBOL_GPL(ip_set_get_inner_ip6_port); > #endif > > bool > @@ -181,3 +326,32 @@ ip_set_get_ip_port(const struct sk_buff *skb, u8 pf, bool > src, __be16 *port) > } > } > EXPORT_SYMBOL_GPL(ip_set_get_ip_port); > + > +bool > +ip_set_get_inner_ip_port(const struct sk_buff *skb, u8 pf, bool src, > + __be16 *port) > +{ > + bool ret; > + u8 proto; > + > + switch (pf) { > + case NFPROTO_IPV4: > + ret = ip_set_get_inner_ip4_port(skb, src, port, &proto); > + break; > + case NFPROTO_IPV6: > + ret = ip_set_get_inner_ip6_port(skb, src, port, &proto); > + break; > + default: > + return false; > + } > + if (!ret) > + return ret; > + switch (proto) { > + case IPPROTO_TCP: > + case IPPROTO_UDP: > + return true; > + default: > + return false; > + } > +} > +EXPORT_SYMBOL_GPL(ip_set_get_inner_ip_port); With the offset argument, all of the above can eliminated. (Here you again duplicated and reimplemented code.) > diff --git a/kernel/net/netfilter/ipset/ip_set_hash_ip.c > b/kernel/net/netfilter/ipset/ip_set_hash_ip.c > index 260c9a8..e9b19a8 100644 > --- a/kernel/net/netfilter/ipset/ip_set_hash_ip.c > +++ b/kernel/net/netfilter/ipset/ip_set_hash_ip.c > @@ -102,7 +102,11 @@ hash_ip4_kadt(struct ip_set *set, const struct sk_buff > *skb, > struct ip_set_ext ext = IP_SET_INIT_KEXT(skb, opt, h); > __be32 ip; > > - ip4addrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, &ip); > + if (opt->cmdflags & IPSET_FLAG_INNER) { > + ip4inneraddrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, &ip); > + } else { > + ip4addrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, &ip); > + } > ip &= ip_set_netmask(h->netmask); > if (ip == 0) > return -EINVAL; > @@ -255,7 +259,12 @@ hash_ip6_kadt(struct ip_set *set, const struct sk_buff > *skb, > struct hash_ip6_elem e = {}; > struct ip_set_ext ext = IP_SET_INIT_KEXT(skb, opt, h); > > - ip6addrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, &e.ip.in6); > + if (opt->cmdflags & IPSET_FLAG_INNER) { > + ip6inneraddrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, > + &e.ip.in6); > + } else { > + ip6addrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, &e.ip.in6); > + } > hash_ip6_netmask(&e.ip, h->netmask); > if (ipv6_addr_any(&e.ip.in6)) > return -EINVAL; > diff --git a/kernel/net/netfilter/ipset/ip_set_hash_ipport.c > b/kernel/net/netfilter/ipset/ip_set_hash_ipport.c > index 64caad3..f3528ff 100644 > --- a/kernel/net/netfilter/ipset/ip_set_hash_ipport.c > +++ b/kernel/net/netfilter/ipset/ip_set_hash_ipport.c > @@ -121,11 +121,22 @@ hash_ipport4_kadt(struct ip_set *set, const struct > sk_buff *skb, > struct hash_ipport4_elem e = { }; > struct ip_set_ext ext = IP_SET_INIT_KEXT(skb, opt, h); > > - if (!ip_set_get_ip4_port(skb, opt->flags & IPSET_DIM_TWO_SRC, > - &e.port, &e.proto)) > - return -EINVAL; > + if (opt->cmdflags & IPSET_FLAG_INNER) { > + if (!ip_set_get_inner_ip4_port(skb, > + opt->flags & IPSET_DIM_TWO_SRC, > + &e.port, &e.proto)) > + return -EINVAL; > + > + ip4inneraddrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, > + &e.ip); > + } else { > + if (!ip_set_get_ip4_port(skb, opt->flags & IPSET_DIM_TWO_SRC, > + &e.port, &e.proto)) > + return -EINVAL; > + > + ip4addrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, &e.ip); > + } > > - ip4addrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, &e.ip); > return adtfn(set, &e, &ext, &opt->ext, opt->cmdflags); > } > > @@ -311,11 +322,21 @@ hash_ipport6_kadt(struct ip_set *set, const struct > sk_buff *skb, > struct hash_ipport6_elem e = { }; > struct ip_set_ext ext = IP_SET_INIT_KEXT(skb, opt, h); > > - if (!ip_set_get_ip6_port(skb, opt->flags & IPSET_DIM_TWO_SRC, > - &e.port, &e.proto)) > - return -EINVAL; > + if (opt->cmdflags & IPSET_FLAG_INNER) { > + if (!ip_set_get_inner_ip6_port(skb, > + opt->flags & IPSET_DIM_TWO_SRC, > + &e.port, &e.proto)) > + return -EINVAL; > + > + ip6inneraddrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, > + &e.ip.in6); > + } else { > + if (!ip_set_get_ip6_port(skb, opt->flags & IPSET_DIM_TWO_SRC, > + &e.port, &e.proto)) > + return -EINVAL; > > - ip6addrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, &e.ip.in6); > + ip6addrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, &e.ip.in6); > + } > return adtfn(set, &e, &ext, &opt->ext, opt->cmdflags); > } > > diff --git a/kernel/net/netfilter/ipset/ip_set_hash_ipportip.c > b/kernel/net/netfilter/ipset/ip_set_hash_ipportip.c > index 2873bbc..8b3bdee 100644 > --- a/kernel/net/netfilter/ipset/ip_set_hash_ipportip.c > +++ b/kernel/net/netfilter/ipset/ip_set_hash_ipportip.c > @@ -125,12 +125,23 @@ hash_ipportip4_kadt(struct ip_set *set, const struct > sk_buff *skb, > struct hash_ipportip4_elem e = { }; > struct ip_set_ext ext = IP_SET_INIT_KEXT(skb, opt, h); > > - if (!ip_set_get_ip4_port(skb, opt->flags & IPSET_DIM_TWO_SRC, > - &e.port, &e.proto)) > - return -EINVAL; > - > - ip4addrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, &e.ip); > - ip4addrptr(skb, opt->flags & IPSET_DIM_THREE_SRC, &e.ip2); > + if (opt->cmdflags & IPSET_FLAG_INNER) { > + if (!ip_set_get_inner_ip4_port(skb, > + opt->flags & IPSET_DIM_TWO_SRC, > + &e.port, &e.proto)) > + return -EINVAL; > + > + ip4inneraddrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, &e.ip); > + ip4inneraddrptr(skb, opt->flags & IPSET_DIM_THREE_SRC, > + &e.ip2); > + } else { > + if (!ip_set_get_ip4_port(skb, opt->flags & IPSET_DIM_TWO_SRC, > + &e.port, &e.proto)) > + return -EINVAL; > + > + ip4addrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, &e.ip); > + ip4addrptr(skb, opt->flags & IPSET_DIM_THREE_SRC, &e.ip2); > + } > return adtfn(set, &e, &ext, &opt->ext, opt->cmdflags); > } > > @@ -324,12 +335,24 @@ hash_ipportip6_kadt(struct ip_set *set, const struct > sk_buff *skb, > struct hash_ipportip6_elem e = { }; > struct ip_set_ext ext = IP_SET_INIT_KEXT(skb, opt, h); > > - if (!ip_set_get_ip6_port(skb, opt->flags & IPSET_DIM_TWO_SRC, > - &e.port, &e.proto)) > - return -EINVAL; > - > - ip6addrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, &e.ip.in6); > - ip6addrptr(skb, opt->flags & IPSET_DIM_THREE_SRC, &e.ip2.in6); > + if (opt->cmdflags & IPSET_FLAG_INNER) { > + if (!ip_set_get_inner_ip6_port(skb, > + opt->flags & IPSET_DIM_TWO_SRC, > + &e.port, &e.proto)) > + return -EINVAL; > + > + ip6inneraddrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, > + &e.ip.in6); > + ip6inneraddrptr(skb, opt->flags & IPSET_DIM_THREE_SRC, > + &e.ip2.in6); > + } else { > + if (!ip_set_get_ip6_port(skb, opt->flags & IPSET_DIM_TWO_SRC, > + &e.port, &e.proto)) > + return -EINVAL; > + > + ip6addrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, &e.ip.in6); > + ip6addrptr(skb, opt->flags & IPSET_DIM_THREE_SRC, &e.ip2.in6); > + } > return adtfn(set, &e, &ext, &opt->ext, opt->cmdflags); > } > > diff --git a/kernel/net/netfilter/ipset/ip_set_hash_ipportnet.c > b/kernel/net/netfilter/ipset/ip_set_hash_ipportnet.c > index db0e761..154c836 100644 > --- a/kernel/net/netfilter/ipset/ip_set_hash_ipportnet.c > +++ b/kernel/net/netfilter/ipset/ip_set_hash_ipportnet.c > @@ -177,12 +177,24 @@ hash_ipportnet4_kadt(struct ip_set *set, const struct > sk_buff *skb, > if (adt == IPSET_TEST) > e.cidr = HOST_MASK - 1; > > - if (!ip_set_get_ip4_port(skb, opt->flags & IPSET_DIM_TWO_SRC, > - &e.port, &e.proto)) > - return -EINVAL; > - > - ip4addrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, &e.ip); > - ip4addrptr(skb, opt->flags & IPSET_DIM_THREE_SRC, &e.ip2); > + if (opt->cmdflags & IPSET_FLAG_INNER) { > + if (!ip_set_get_inner_ip4_port(skb, > + opt->flags & IPSET_DIM_TWO_SRC, > + &e.port, &e.proto)) > + return -EINVAL; > + > + ip4inneraddrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, > + &e.ip); > + ip4inneraddrptr(skb, opt->flags & IPSET_DIM_THREE_SRC, > + &e.ip2); > + } else { > + if (!ip_set_get_ip4_port(skb, opt->flags & IPSET_DIM_TWO_SRC, > + &e.port, &e.proto)) > + return -EINVAL; > + > + ip4addrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, &e.ip); > + ip4addrptr(skb, opt->flags & IPSET_DIM_THREE_SRC, &e.ip2); > + } > e.ip2 &= ip_set_netmask(e.cidr + 1); > > return adtfn(set, &e, &ext, &opt->ext, opt->cmdflags); > @@ -461,12 +473,25 @@ hash_ipportnet6_kadt(struct ip_set *set, const struct > sk_buff *skb, > if (adt == IPSET_TEST) > e.cidr = HOST_MASK - 1; > > - if (!ip_set_get_ip6_port(skb, opt->flags & IPSET_DIM_TWO_SRC, > - &e.port, &e.proto)) > - return -EINVAL; > - > - ip6addrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, &e.ip.in6); > - ip6addrptr(skb, opt->flags & IPSET_DIM_THREE_SRC, &e.ip2.in6); > + if (opt->cmdflags & IPSET_FLAG_INNER) { > + if (!ip_set_get_inner_ip6_port(skb, > + opt->flags & IPSET_DIM_TWO_SRC, > + &e.port, &e.proto)) > + return -EINVAL; > + > + ip6inneraddrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, > + &e.ip.in6); > + ip6inneraddrptr(skb, opt->flags & IPSET_DIM_THREE_SRC, > + &e.ip2.in6); > + } else { > + if (!ip_set_get_ip6_port(skb, opt->flags & IPSET_DIM_TWO_SRC, > + &e.port, &e.proto)) > + return -EINVAL; > + > + ip6addrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, &e.ip.in6); > + ip6addrptr(skb, opt->flags & IPSET_DIM_THREE_SRC, > + &e.ip2.in6); > + } > ip6_netmask(&e.ip2, e.cidr + 1); > > return adtfn(set, &e, &ext, &opt->ext, opt->cmdflags); > diff --git a/kernel/net/netfilter/ipset/ip_set_hash_net.c > b/kernel/net/netfilter/ipset/ip_set_hash_net.c > index 846ec80..431706a 100644 > --- a/kernel/net/netfilter/ipset/ip_set_hash_net.c > +++ b/kernel/net/netfilter/ipset/ip_set_hash_net.c > @@ -151,8 +151,13 @@ hash_net4_kadt(struct ip_set *set, const struct sk_buff > *skb, > return -EINVAL; > if (adt == IPSET_TEST) > e.cidr = HOST_MASK; > + if (opt->cmdflags & IPSET_FLAG_INNER) { > + ip4inneraddrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, > + &e.ip); > + } else { > + ip4addrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, &e.ip); > + } > > - ip4addrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, &e.ip); > e.ip &= ip_set_netmask(e.cidr); > > return adtfn(set, &e, &ext, &opt->ext, opt->cmdflags); > @@ -346,8 +351,12 @@ hash_net6_kadt(struct ip_set *set, const struct sk_buff > *skb, > return -EINVAL; > if (adt == IPSET_TEST) > e.cidr = HOST_MASK; > - > - ip6addrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, &e.ip.in6); > + if (opt->cmdflags & IPSET_FLAG_INNER) { > + ip6inneraddrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, > + &e.ip.in6); > + } else { > + ip6addrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, &e.ip.in6); > + } > ip6_netmask(&e.ip, e.cidr); > > return adtfn(set, &e, &ext, &opt->ext, opt->cmdflags); > diff --git a/kernel/net/netfilter/ipset/ip_set_hash_netiface.c > b/kernel/net/netfilter/ipset/ip_set_hash_netiface.c > index 8f0e496..51415fb 100644 > --- a/kernel/net/netfilter/ipset/ip_set_hash_netiface.c > +++ b/kernel/net/netfilter/ipset/ip_set_hash_netiface.c > @@ -275,8 +275,12 @@ hash_netiface4_kadt(struct ip_set *set, const struct > sk_buff *skb, > return -EINVAL; > if (adt == IPSET_TEST) > e.cidr = HOST_MASK; > - > - ip4addrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, &e.ip); > + if (opt->cmdflags & IPSET_FLAG_INNER) { > + ip4inneraddrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, > + &e.ip); > + } else { > + ip4addrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, &e.ip); > + } > e.ip &= ip_set_netmask(e.cidr); > > #define IFACE(dir) (par->dir ? par->dir->name : NULL) > @@ -544,8 +548,12 @@ hash_netiface6_kadt(struct ip_set *set, const struct > sk_buff *skb, > return -EINVAL; > if (adt == IPSET_TEST) > e.cidr = HOST_MASK; > - > - ip6addrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, &e.ip.in6); > + if (opt->cmdflags & IPSET_FLAG_INNER) { > + ip6inneraddrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, > + &e.ip.in6); > + } else { > + ip6addrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, &e.ip.in6); > + } > ip6_netmask(&e.ip, e.cidr); > > if (opt->cmdflags & IPSET_FLAG_PHYSDEV) { > diff --git a/kernel/net/netfilter/ipset/ip_set_hash_netport.c > b/kernel/net/netfilter/ipset/ip_set_hash_netport.c > index 021d716..376a28e 100644 > --- a/kernel/net/netfilter/ipset/ip_set_hash_netport.c > +++ b/kernel/net/netfilter/ipset/ip_set_hash_netport.c > @@ -169,11 +169,21 @@ hash_netport4_kadt(struct ip_set *set, const struct > sk_buff *skb, > if (adt == IPSET_TEST) > e.cidr = HOST_MASK - 1; > > - if (!ip_set_get_ip4_port(skb, opt->flags & IPSET_DIM_TWO_SRC, > - &e.port, &e.proto)) > - return -EINVAL; > - > - ip4addrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, &e.ip); > + if (opt->cmdflags & IPSET_FLAG_INNER) { > + if (!ip_set_get_inner_ip4_port(skb, > + opt->flags & IPSET_DIM_TWO_SRC, > + &e.port, &e.proto)) > + return -EINVAL; > + > + ip4inneraddrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, > + &e.ip); > + } else { > + if (!ip_set_get_ip4_port(skb, opt->flags & IPSET_DIM_TWO_SRC, > + &e.port, &e.proto)) > + return -EINVAL; > + > + ip4addrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, &e.ip); > + } > e.ip &= ip_set_netmask(e.cidr + 1); > > return adtfn(set, &e, &ext, &opt->ext, opt->cmdflags); > @@ -413,12 +423,21 @@ hash_netport6_kadt(struct ip_set *set, const struct > sk_buff *skb, > > if (adt == IPSET_TEST) > e.cidr = HOST_MASK - 1; > - > - if (!ip_set_get_ip6_port(skb, opt->flags & IPSET_DIM_TWO_SRC, > - &e.port, &e.proto)) > - return -EINVAL; > - > - ip6addrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, &e.ip.in6); > + if (opt->cmdflags & IPSET_FLAG_INNER) { > + if (!ip_set_get_inner_ip6_port(skb, > + opt->flags & IPSET_DIM_TWO_SRC, > + &e.port, &e.proto)) > + return -EINVAL; > + > + ip6inneraddrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, > + &e.ip.in6); > + } else { > + if (!ip_set_get_ip6_port(skb, opt->flags & IPSET_DIM_TWO_SRC, > + &e.port, &e.proto)) > + return -EINVAL; > + > + ip6addrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, &e.ip.in6); > + } > ip6_netmask(&e.ip, e.cidr + 1); > > return adtfn(set, &e, &ext, &opt->ext, opt->cmdflags); I'd prefer the patch to be split into two parts: the first which implements the new functions and changes in the core and the second which applies them to the set types. 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/4] ipset: add "inner" flag implementation 2013-06-01 10:54 ` Jozsef Kadlecsik @ 2013-06-07 1:13 ` Dash Four 2013-06-09 8:54 ` Jozsef Kadlecsik 0 siblings, 1 reply; 7+ messages in thread From: Dash Four @ 2013-06-07 1:13 UTC (permalink / raw) To: Jozsef Kadlecsik; +Cc: Pablo Neira Ayuso, Netfilter Core Team Jozsef Kadlecsik wrote: > There are a few problems with your patch: some things are duplicated, > already existing function is reimplemented. Agreed. That will be addressed in the next version of patches. > There is no error path when > the inner IPv4/6 header is broken: the returned zero IP address is valid > for example in the case of the hash:net,iface type. I was unaware of that. If that is indeed the case, then yes, error code handling must be implemented in a different way. > Also, I'd prefer a > single new function (or two inline functions if their size permits), > which'd return the success code and in pointers the inner IP header and > the offset to the header (for proto/ports). Read my comments below. > It has to be 2 functions - one dealing with ipv4 headers and another implementing ipv6 header handling. Further comments and questions follow below. >> +static inline __be32 >> +ip4inneraddr(const struct sk_buff *skb, bool src) >> +{ >> + struct iphdr _iph; >> + struct icmphdr _icmph; >> + u8 type; >> + const struct iphdr *ih; >> + const struct icmphdr *ich; >> + static const size_t len = 8 + sizeof(struct iphdr); >> + >> + ih = skb_header_pointer(skb, 0, sizeof(_iph), &_iph); >> + if (ih == NULL || ih->protocol != IPPROTO_ICMP || >> + ntohs(ih->frag_off) & IP_OFFSET) >> + goto err; >> + >> + ich = skb_header_pointer(skb, ih->ihl*4, >> + sizeof(_icmph), &_icmph); >> + if (ich == NULL || >> + (ich->type <= NR_ICMP_TYPES && skb->len - ih->ihl*4 < len)) >> + goto err; >> + >> + type = ich->type; >> + if (type == ICMP_DEST_UNREACH || >> + type == ICMP_SOURCE_QUENCH || >> + type == ICMP_TIME_EXCEEDED) { >> + ih = skb_header_pointer(skb, ih->ihl*4 + sizeof(_icmph), >> + sizeof(_iph), &_iph); >> + if (ih == NULL || ntohs(ih->frag_off) & IP_OFFSET) >> + goto err; >> + >> + return src ? ih->saddr : ih->daddr; >> + } >> +err: >> + return 0; >> +} >> > > ip4inneraddr is a duplication of ip4inneraddrptr: if this were the way, > then ip4inneraddr should simply wrap and call ip4inneraddrptr. > Indeed, I'll address this in the next version of patches. >> +static inline void >> +ip4inneraddrptr(const struct sk_buff *skb, bool src, __be32 *addr) >> +{ >> + struct iphdr _iph; >> + struct icmphdr _icmph; >> + u8 type; >> + const struct iphdr *ih; >> + const struct icmphdr *ich; >> + static const size_t len = 8 + sizeof(struct iphdr); >> > > I don't like bare numbers as magic constants. > That's the size of a "standard" header (the "magic" number 8 is also used in ipv6_skip_exthdr as well). I can use some more meaningful constant if you wish - let me know. >> +#define ip6_ext_hdr(hdr) ((hdr == IPPROTO_HOPOPTS) || \ >> + (hdr == IPPROTO_ROUTING) || \ >> + (hdr == IPPROTO_FRAGMENT) || \ >> + (hdr == IPPROTO_ESP) || \ >> + (hdr == IPPROTO_AH) || \ >> + (hdr == IPPROTO_NONE) || \ >> + (hdr == IPPROTO_DSTOPTS)) >> +static inline void >> +ip6inneraddrptr(const struct sk_buff *skb, bool src, struct in6_addr *addr) >> +{ >> + u8 type, currenthdr; >> + bool fragment = false; >> + unsigned int ptr, hdrlen = 0; >> + struct ipv6hdr _ip6h; >> + struct icmp6hdr _icmp6h; >> + const struct ipv6hdr *ih; >> + const struct icmp6hdr *ic; >> + >> + ih = skb_header_pointer(skb, 0, sizeof(_ip6h), &_ip6h); >> + if (ih == NULL) >> + goto err; >> + >> + ptr = sizeof(struct ipv6hdr); >> + currenthdr = ih->nexthdr; >> + while (currenthdr != NEXTHDR_NONE && ip6_ext_hdr(currenthdr)) { >> + struct ipv6_opt_hdr _hdr; >> + const struct ipv6_opt_hdr *hp; >> + >> + hp = skb_header_pointer(skb, ptr, sizeof(_hdr), &_hdr); >> + if (hp == NULL) >> + goto err; >> + >> + switch (currenthdr) { >> + case IPPROTO_FRAGMENT: { >> + struct frag_hdr _fhdr; >> + const struct frag_hdr *fh; >> + >> + fh = skb_header_pointer(skb, ptr, sizeof(_fhdr), >> + &_fhdr); >> + if (fh == NULL) >> + goto err; >> + if (ntohs(fh->frag_off) & 0xFFF8) >> + fragment = true; >> + >> + hdrlen = 8; >> + break; >> + } >> + case IPPROTO_DSTOPTS: >> + case IPPROTO_ROUTING: >> + case IPPROTO_HOPOPTS: >> + if (fragment) >> + goto err; >> + >> + hdrlen = ipv6_optlen(hp); >> + break; >> + case IPPROTO_AH: >> + hdrlen = (hp->hdrlen+2)<<2; >> + break; >> + case IPPROTO_ESP: >> + default: >> + goto err; >> + } /* switch IPPROTO */ >> + >> + currenthdr = hp->nexthdr; >> + ptr += hdrlen; >> + } >> > > The code segment above is the reimplementation of ipv6_skip_exthdr. > No, not really. Even though there are similarities between the two fucntions, there are 2 differences - one is that ipv6_skip_exthdr wasn't dealing properly (from ipset's point of view) with fragments, "frag_off" is not present in kernel versions < 3.3 and the other is that ipv6_skip_exthdr doesn't return an error when either an unknown or ESP header type is encountered ("case IPPROTO_ESP" and "default" above). That is the reason for implementing a separate function. I see that you have amended the ipset code to handle frag_off, but I am not sure that would work in the case of kernel versions < 3.3. Again, though, I have limited knowledge of IPv6 as we don't use it here, so if you think it is better for me to replace the above code and use ipv6_skip_exthdr instead, then I'll do it in the next version of patches. >> + if (fragment || currenthdr != IPPROTO_ICMPV6) >> + goto err; >> + >> + ic = skb_header_pointer(skb, ptr, sizeof(_icmp6h), &_icmp6h); >> + if (ic == NULL) >> + goto err; >> + >> + type = ic->icmp6_type; >> + if (type == ICMPV6_DEST_UNREACH || >> + type == ICMPV6_PKT_TOOBIG || >> + type == ICMPV6_TIME_EXCEED) { >> + ih = skb_header_pointer(skb, ptr + sizeof(_icmp6h), >> + sizeof(_ip6h), &_ip6h); >> + if (ih == NULL) >> + goto err; >> + >> + memcpy(addr, src ? &ih->saddr : &ih->daddr, >> + sizeof(*addr)); >> + return; >> + } >> + >> +err: >> + memset(addr, 0, sizeof(*addr)); >> +} >> > > Zero valued IP addresses are valid, so it's not a proper error handling in > the ip[46]inneraddrptr functions. > I was unaware of this and error-handling will be implemented differently in the next version of the patches. > These all can be replaced by a function which skips the external header > and points to the inner one, something like: > > int > ipset_inner_header(const struct sk_buff *skb, u8 pf, u8 *offset, > void *inner) > { > /* IPv4 */ > ... > /* IPv6 */ > nexthdr = ipv6_hdr(skb)->nexthdr; > *offset = ipv6_skip_exthdr(skb, sizeof(struct ipv6hdr), > &nexthdr, &frag_off); > /* Validate offset and frag_off */ > /* Check ICMPv6, update offset */ > /* Copy inner IPv6 header to *inner by skb_header_pointer */ > > return success/error; > } > IPv4 and IPv6 should be treated as separate cases, so I'll create two separate functions for that. One other thing - this type of operation is not going to be confined just to ipset (I plan to do the same thing when submit the changes to the AUDIT iptables target, which logs these attributes as well), so I am thinking of offloading these two functions and moving everything to ip[v6].h so that they can be used not just by ipset. What do you think? > Please note, I updated the ipset git tree with a patch which adds the > a missing verification of frag_off to ip_set_get_ip6_port(). > I did see that, though I am not sure this can work in kernel versions < 3.3. >> diff --git a/kernel/net/netfilter/ipset/ip_set_bitmap_ip.c >> b/kernel/net/netfilter/ipset/ip_set_bitmap_ip.c >> index ce99d26..7552d6d 100644 >> --- a/kernel/net/netfilter/ipset/ip_set_bitmap_ip.c >> +++ b/kernel/net/netfilter/ipset/ip_set_bitmap_ip.c >> @@ -116,7 +116,9 @@ bitmap_ip_kadt(struct ip_set *set, const struct sk_buff >> *skb, >> struct ip_set_ext ext = IP_SET_INIT_KEXT(skb, opt, map); >> u32 ip; >> >> - ip = ntohl(ip4addr(skb, opt->flags & IPSET_DIM_ONE_SRC)); >> + ip = opt->cmdflags & IPSET_FLAG_INNER ? >> + ntohl(ip4inneraddr(skb, opt->flags & IPSET_DIM_ONE_SRC)) : >> + ntohl(ip4addr(skb, opt->flags & IPSET_DIM_ONE_SRC)); >> if (ip < map->first_ip || ip > map->last_ip) >> return -IPSET_ERR_BITMAP_RANGE; >> > > With the ipset_inner_header function, all these looks like this: we need > new variables for the IP header, pointer to it and offset. If the > IPSET_FLAG_INNER is true, ipset_inner_header is called and the returned > error code handled or the pointer to IP header is updated. Something like > this: > > struct ip_set_ext ext = IP_SET_INIT_KEXT(skb, opt, map); > u32 ip; > struct iphdr _iphdr, *iphdr = ip_hdr(skb); > u8 offset = 0; > > if (opt->cmdflags & IPSET_FLAG_INNER) { > if (ipset_inner_header(skb, NFPROTO_IPV4, &offset, &_iphdr)) > return -EINVAL; > iphdr = &_iphdr; > } > .... > > Current ip[46]addr[ptr] functions of course need to be modified to use the > IP header structures instead of the skbuff one. > I am not sure there should be one function for this - IPv6 and IPv4 are two separate cases (what if I don't have IPv6 - the whole IPv6 code will never be executed/needed, so there isn't any reason to be there at all), so I think there should be two separate functions for these. >> diff --git a/kernel/net/netfilter/ipset/ip_set_bitmap_port.c >> b/kernel/net/netfilter/ipset/ip_set_bitmap_port.c >> index b220489..d29395b 100644 >> --- a/kernel/net/netfilter/ipset/ip_set_bitmap_port.c >> +++ b/kernel/net/netfilter/ipset/ip_set_bitmap_port.c >> @@ -110,9 +110,17 @@ bitmap_port_kadt(struct ip_set *set, const struct sk_buff >> *skb, >> __be16 __port; >> u16 port = 0; >> >> - if (!ip_set_get_ip_port(skb, opt->family, >> - opt->flags & IPSET_DIM_ONE_SRC, &__port)) >> - return -EINVAL; >> + if (opt->cmdflags & IPSET_FLAG_INNER) { >> + if (!ip_set_get_inner_ip_port(skb, opt->family, >> + opt->flags & IPSET_DIM_ONE_SRC, >> + &__port)) >> + return -EINVAL; >> + } else { >> + if (!ip_set_get_ip_port(skb, opt->family, >> + opt->flags & IPSET_DIM_ONE_SRC, >> + &__port)) >> + return -EINVAL; >> + } >> >> port = ntohs(__port); >> > > The ip_set_get*port functions need the new offset argument too. > Yep, that's another change which I need to implement. >> @@ -413,12 +423,21 @@ hash_netport6_kadt(struct ip_set *set, const struct >> sk_buff *skb, >> >> if (adt == IPSET_TEST) >> e.cidr = HOST_MASK - 1; >> - >> - if (!ip_set_get_ip6_port(skb, opt->flags & IPSET_DIM_TWO_SRC, >> - &e.port, &e.proto)) >> - return -EINVAL; >> - >> - ip6addrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, &e.ip.in6); >> + if (opt->cmdflags & IPSET_FLAG_INNER) { >> + if (!ip_set_get_inner_ip6_port(skb, >> + opt->flags & IPSET_DIM_TWO_SRC, >> + &e.port, &e.proto)) >> + return -EINVAL; >> + >> + ip6inneraddrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, >> + &e.ip.in6); >> + } else { >> + if (!ip_set_get_ip6_port(skb, opt->flags & IPSET_DIM_TWO_SRC, >> + &e.port, &e.proto)) >> + return -EINVAL; >> + >> + ip6addrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, &e.ip.in6); >> + } >> ip6_netmask(&e.ip, e.cidr + 1); >> >> return adtfn(set, &e, &ext, &opt->ext, opt->cmdflags); >> > > I'd prefer the patch to be split into two parts: the first which > implements the new functions and changes in the core and the second which > applies them to the set types. Thanks. > Noted. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/4] ipset: add "inner" flag implementation 2013-06-07 1:13 ` Dash Four @ 2013-06-09 8:54 ` Jozsef Kadlecsik 0 siblings, 0 replies; 7+ messages in thread From: Jozsef Kadlecsik @ 2013-06-09 8:54 UTC (permalink / raw) To: Dash Four; +Cc: Pablo Neira Ayuso, Netfilter Core Team On Fri, 7 Jun 2013, Dash Four wrote: > Jozsef Kadlecsik wrote: > > Also, I'd prefer a single new function (or two inline functions if their > > size permits), which'd return the success code and in pointers the inner IP > > header and > > the offset to the header (for proto/ports). Read my comments below. > > > It has to be 2 functions - one dealing with ipv4 headers and another > implementing ipv6 header handling. Further comments and questions follow > below. I meant, similarly to ip_set_get_ip_port, which wraps around the two functions. So one exported function suffices. > > > +static inline void > > > +ip4inneraddrptr(const struct sk_buff *skb, bool src, __be32 *addr) > > > +{ > > > + struct iphdr _iph; > > > + struct icmphdr _icmph; > > > + u8 type; > > > + const struct iphdr *ih; > > > + const struct icmphdr *ich; > > > + static const size_t len = 8 + sizeof(struct iphdr); > > > > > > > I don't like bare numbers as magic constants. > > > That's the size of a "standard" header (the "magic" number 8 is also > used in ipv6_skip_exthdr as well). I can use some more meaningful > constant if you wish - let me know. If it's the size of a structure, use a sizeof. (I don't like the number in ipv6_skip_exthdr either, but that's another thing.) > > > +#define ip6_ext_hdr(hdr) ((hdr == IPPROTO_HOPOPTS) || \ > > > + (hdr == IPPROTO_ROUTING) || \ > > > + (hdr == IPPROTO_FRAGMENT) || \ > > > + (hdr == IPPROTO_ESP) || \ > > > + (hdr == IPPROTO_AH) || \ > > > + (hdr == IPPROTO_NONE) || \ > > > + (hdr == IPPROTO_DSTOPTS)) > > > +static inline void > > > +ip6inneraddrptr(const struct sk_buff *skb, bool src, struct in6_addr > > > *addr) > > > +{ > > > + u8 type, currenthdr; > > > + bool fragment = false; > > > + unsigned int ptr, hdrlen = 0; > > > + struct ipv6hdr _ip6h; > > > + struct icmp6hdr _icmp6h; > > > + const struct ipv6hdr *ih; > > > + const struct icmp6hdr *ic; > > > + > > > + ih = skb_header_pointer(skb, 0, sizeof(_ip6h), &_ip6h); > > > + if (ih == NULL) > > > + goto err; > > > + > > > + ptr = sizeof(struct ipv6hdr); > > > + currenthdr = ih->nexthdr; > > > + while (currenthdr != NEXTHDR_NONE && ip6_ext_hdr(currenthdr)) { > > > + struct ipv6_opt_hdr _hdr; > > > + const struct ipv6_opt_hdr *hp; > > > + > > > + hp = skb_header_pointer(skb, ptr, sizeof(_hdr), &_hdr); > > > + if (hp == NULL) > > > + goto err; > > > + > > > + switch (currenthdr) { > > > + case IPPROTO_FRAGMENT: { > > > + struct frag_hdr _fhdr; > > > + const struct frag_hdr *fh; > > > + > > > + fh = skb_header_pointer(skb, ptr, sizeof(_fhdr), > > > + &_fhdr); > > > + if (fh == NULL) > > > + goto err; > > > + if (ntohs(fh->frag_off) & 0xFFF8) > > > + fragment = true; > > > + > > > + hdrlen = 8; > > > + break; > > > + } > > > + case IPPROTO_DSTOPTS: > > > + case IPPROTO_ROUTING: > > > + case IPPROTO_HOPOPTS: > > > + if (fragment) > > > + goto err; > > > + > > > + hdrlen = ipv6_optlen(hp); > > > + break; > > > + case IPPROTO_AH: > > > + hdrlen = (hp->hdrlen+2)<<2; > > > + break; > > > + case IPPROTO_ESP: > > > + default: > > > + goto err; > > > + } /* switch IPPROTO */ > > > + > > > + currenthdr = hp->nexthdr; > > > + ptr += hdrlen; > > > + } > > > > > > > The code segment above is the reimplementation of ipv6_skip_exthdr. > > > No, not really. Even though there are similarities between the two > fucntions, there are 2 differences - one is that ipv6_skip_exthdr wasn't > dealing properly (from ipset's point of view) with fragments, "frag_off" > is not present in kernel versions < 3.3 and the other is that > ipv6_skip_exthdr doesn't return an error when either an unknown or ESP > header type is encountered ("case IPPROTO_ESP" and "default" above). ipv6_skip_exthdr handles ESP as a normal protocol, stops processing there, so the called deals with it. Also, we must process (ignore) unknown extension headers, that's not an error condition. Kernel versions before 3.3 are supported as is. I don't see the point in fixing ipv6_skip_exthdr just for ipset when the same function is used "unpatched" in the kernel itself before 3.3. > > These all can be replaced by a function which skips the external header and > > points to the inner one, something like: > > > > int > > ipset_inner_header(const struct sk_buff *skb, u8 pf, u8 *offset, > > void *inner) > > { > > /* IPv4 */ > > ... > > /* IPv6 */ > > nexthdr = ipv6_hdr(skb)->nexthdr; > > *offset = ipv6_skip_exthdr(skb, sizeof(struct ipv6hdr), > > &nexthdr, &frag_off); > > /* Validate offset and frag_off */ > > /* Check ICMPv6, update offset */ > > /* Copy inner IPv6 header to *inner by skb_header_pointer */ > > > > return success/error; > > } > > > IPv4 and IPv6 should be treated as separate cases, so I'll create two > separate functions for that. That'd require two exported functions (check the allowed size for inline functions), and I'm unsure two exports will be accepted. > One other thing - this type of operation is not going to be confined > just to ipset (I plan to do the same thing when submit the changes to > the AUDIT iptables target, which logs these attributes as well), so I am > thinking of offloading these two functions and moving everything to > ip[v6].h so that they can be used not just by ipset. What do you think? You can put inline functions/macros into header files only and function size may block using inlined. > > Current ip[46]addr[ptr] functions of course need to be modified to use the > > IP header structures instead of the skbuff one. > > > I am not sure there should be one function for this - IPv6 and IPv4 are two > separate cases (what if I don't have IPv6 - the whole IPv6 code will never be > executed/needed, so there isn't any reason to be there at all), so I think > there should be two separate functions for these. That can be solved by empty function definition. I mean, in the ipset_inner_header example above, the IPv4/IPv6 cases are handled in independent functions and the one for IPv6 is actually empty when there's no IPv6 enabled at all. 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 3/4] iptables: add set match "inner" flag support [not found] <51A54267.8050402-git-send-email-mr.dash.four@googlemail.com> 2013-05-29 0:13 ` [PATCH 1/4] ipset: minor variable-naming corrections Dash Four 2013-05-29 0:13 ` [PATCH 2/4] ipset: add "inner" flag implementation Dash Four @ 2013-05-29 0:13 ` Dash Four 2013-05-29 0:13 ` [PATCH 4/4] iptables (userspace): " Dash Four 3 siblings, 0 replies; 7+ messages in thread From: Dash Four @ 2013-05-29 0:13 UTC (permalink / raw) To: Jozsef Kadlecsik; +Cc: Pablo Neira Ayuso, Netfilter Core Team This patch adds "inner" flag support for the set match. Signed-off-by: Dash Four <mr.dash.four@googlemail.com> --- kernel/net/netfilter/xt_set.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/kernel/net/netfilter/xt_set.c b/kernel/net/netfilter/xt_set.c index 00b9c58..430bcff 100644 --- a/kernel/net/netfilter/xt_set.c +++ b/kernel/net/netfilter/xt_set.c @@ -464,6 +464,27 @@ static struct xt_match set_matches[] __read_mostly = { .destroy = set_match_v3_destroy, .me = THIS_MODULE }, + /* --inner flag support */ + { + .name = "set", + .family = NFPROTO_IPV4, + .revision = 4, + .match = set_match_v3, + .matchsize = sizeof(struct xt_set_info_match_v3), + .checkentry = set_match_v3_checkentry, + .destroy = set_match_v3_destroy, + .me = THIS_MODULE + }, + { + .name = "set", + .family = NFPROTO_IPV6, + .revision = 4, + .match = set_match_v3, + .matchsize = sizeof(struct xt_set_info_match_v3), + .checkentry = set_match_v3_checkentry, + .destroy = set_match_v3_destroy, + .me = THIS_MODULE + }, }; static struct xt_target set_targets[] __read_mostly = { ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 4/4] iptables (userspace): add set match "inner" flag support [not found] <51A54267.8050402-git-send-email-mr.dash.four@googlemail.com> ` (2 preceding siblings ...) 2013-05-29 0:13 ` [PATCH 3/4] iptables: add set match "inner" flag support Dash Four @ 2013-05-29 0:13 ` Dash Four 3 siblings, 0 replies; 7+ messages in thread From: Dash Four @ 2013-05-29 0:13 UTC (permalink / raw) To: Jozsef Kadlecsik; +Cc: Pablo Neira Ayuso, Netfilter Core Team This patch adds "inner" flag support for the set match in iptables. Signed-off-by: Dash Four <mr.dash.four@googlemail.com> --- extensions/libxt_set.c | 199 ++++++++++++++++++++++++++++++++ extensions/libxt_set.man | 10 ++ include/linux/netfilter/ipset/ip_set.h | 2 + 3 files changed, 211 insertions(+) diff --git a/extensions/libxt_set.c b/extensions/libxt_set.c index 2cb9e78..3b20c94 100644 --- a/extensions/libxt_set.c +++ b/extensions/libxt_set.c @@ -497,6 +497,191 @@ set_save_v3(const void *ip, const struct xt_entry_match *match) set_print_v3_matchinfo(info, "--match-set", "--"); } +/* Revision 4 - add --inner flag */ +static void +set_help_v4(void) +{ + printf("set match options:\n" + " [!] --match-set name flags [--return-nomatch] [--inner]\n" + " [! --update-counters] [! --update-subcounters]\n" + " [[!] --packets-eq value | --packets-lt value | --packets-gt value\n" + " [[!] --bytes-eq value | --bytes-lt value | --bytes-gt value\n" + " 'name' is the set name from to match,\n" + " 'flags' are the comma separated list of\n" + " 'src' and 'dst' specifications.\n"); +} + +static const struct option set_opts_v4[] = { + {.name = "match-set", .has_arg = true, .val = '1'}, + {.name = "set", .has_arg = true, .val = '2'}, + {.name = "return-nomatch", .has_arg = false, .val = '3'}, + {.name = "update-counters", .has_arg = false, .val = '4'}, + {.name = "packets-eq", .has_arg = true, .val = '5'}, + {.name = "packets-lt", .has_arg = true, .val = '6'}, + {.name = "packets-gt", .has_arg = true, .val = '7'}, + {.name = "bytes-eq", .has_arg = true, .val = '8'}, + {.name = "bytes-lt", .has_arg = true, .val = '9'}, + {.name = "bytes-gt", .has_arg = true, .val = '0'}, + {.name = "update-subcounters", .has_arg = false, .val = 'a'}, + {.name = "inner", .has_arg = false, .val = 'b'}, + XT_GETOPT_TABLEEND, +}; + +static int +set_parse_v4(int c, char **argv, int invert, unsigned int *flags, + const void *entry, struct xt_entry_match **match) +{ + struct xt_set_info_match_v3 *info = + (struct xt_set_info_match_v3 *) (*match)->data; + + switch (c) { + case 'b': + if (invert) + xtables_error(PARAMETER_PROBLEM, + "--inner flag cannot be inverted\n"); + info->flags |= IPSET_FLAG_INNER; + break; + case 'a': + if (invert) + info->flags |= IPSET_FLAG_SKIP_SUBCOUNTER_UPDATE; + break; + case '0': + if (info->bytes.op != IPSET_COUNTER_NONE) + xtables_error(PARAMETER_PROBLEM, + "only one of the --bytes-[eq|lt|gt]" + " is allowed\n"); + if (invert) + xtables_error(PARAMETER_PROBLEM, + "--bytes-gt option cannot be inverted\n"); + info->bytes.op = IPSET_COUNTER_GT; + info->bytes.value = parse_counter(optarg); + break; + case '9': + if (info->bytes.op != IPSET_COUNTER_NONE) + xtables_error(PARAMETER_PROBLEM, + "only one of the --bytes-[eq|lt|gt]" + " is allowed\n"); + if (invert) + xtables_error(PARAMETER_PROBLEM, + "--bytes-lt option cannot be inverted\n"); + info->bytes.op = IPSET_COUNTER_LT; + info->bytes.value = parse_counter(optarg); + break; + case '8': + if (info->bytes.op != IPSET_COUNTER_NONE) + xtables_error(PARAMETER_PROBLEM, + "only one of the --bytes-[eq|lt|gt]" + " is allowed\n"); + info->bytes.op = invert ? IPSET_COUNTER_NE : IPSET_COUNTER_EQ; + info->bytes.value = parse_counter(optarg); + break; + case '7': + if (info->packets.op != IPSET_COUNTER_NONE) + xtables_error(PARAMETER_PROBLEM, + "only one of the --packets-[eq|lt|gt]" + " is allowed\n"); + if (invert) + xtables_error(PARAMETER_PROBLEM, + "--packets-gt option cannot be inverted\n"); + info->packets.op = IPSET_COUNTER_GT; + info->packets.value = parse_counter(optarg); + break; + case '6': + if (info->packets.op != IPSET_COUNTER_NONE) + xtables_error(PARAMETER_PROBLEM, + "only one of the --packets-[eq|lt|gt]" + " is allowed\n"); + if (invert) + xtables_error(PARAMETER_PROBLEM, + "--packets-lt option cannot be inverted\n"); + info->packets.op = IPSET_COUNTER_LT; + info->packets.value = parse_counter(optarg); + break; + case '5': + if (info->packets.op != IPSET_COUNTER_NONE) + xtables_error(PARAMETER_PROBLEM, + "only one of the --packets-[eq|lt|gt]" + " is allowed\n"); + info->packets.op = invert ? IPSET_COUNTER_NE : IPSET_COUNTER_EQ; + info->packets.value = parse_counter(optarg); + break; + case '4': + if (invert) + info->flags |= IPSET_FLAG_SKIP_COUNTER_UPDATE; + break; + case '3': + if (invert) + xtables_error(PARAMETER_PROBLEM, + "--return-nomatch flag cannot be inverted\n"); + info->flags |= IPSET_FLAG_RETURN_NOMATCH; + break; + case '2': + fprintf(stderr, + "--set option deprecated, please use --match-set\n"); + case '1': /* --match-set <set> <flag>[,<flag> */ + if (info->match_set.dim) + xtables_error(PARAMETER_PROBLEM, + "--match-set can be specified only once"); + if (invert) + info->match_set.flags |= IPSET_INV_MATCH; + + if (!argv[optind] + || argv[optind][0] == '-' + || argv[optind][0] == '!') + xtables_error(PARAMETER_PROBLEM, + "--match-set requires two args."); + + if (strlen(optarg) > IPSET_MAXNAMELEN - 1) + xtables_error(PARAMETER_PROBLEM, + "setname `%s' too long, max %d characters.", + optarg, IPSET_MAXNAMELEN - 1); + + get_set_byname(optarg, &info->match_set); + parse_dirs(argv[optind], &info->match_set); + DEBUGP("parse: set index %u\n", info->match_set.index); + optind++; + + *flags = 1; + break; + } + + return 1; +} + +static void +set_print_v4_matchinfo(const struct xt_set_info_match_v3 *info, + const char *opt, const char *sep) +{ + print_match(opt, &info->match_set); + if (info->flags & IPSET_FLAG_RETURN_NOMATCH) + printf(" %sreturn-nomatch", sep); + if (info->flags & IPSET_FLAG_INNER) + printf(" %sinner", sep); + if ((info->flags & IPSET_FLAG_SKIP_COUNTER_UPDATE)) + printf(" ! %supdate-counters", sep); + if ((info->flags & IPSET_FLAG_SKIP_SUBCOUNTER_UPDATE)) + printf(" ! %supdate-subcounters", sep); + set_printv3_counter(&info->packets, "packets", sep); + set_printv3_counter(&info->bytes, "bytes", sep); +} + +/* Prints out the matchinfo. */ +static void +set_print_v4(const void *ip, const struct xt_entry_match *match, int numeric) +{ + const struct xt_set_info_match_v3 *info = (const void *)match->data; + + set_print_v4_matchinfo(info, "match-set", ""); +} + +static void +set_save_v4(const void *ip, const struct xt_entry_match *match) +{ + const struct xt_set_info_match_v3 *info = (const void *)match->data; + + set_print_v4_matchinfo(info, "--match-set", "--"); +} + static struct xtables_match set_mt_reg[] = { { .name = "set", @@ -554,6 +739,20 @@ static struct xtables_match set_mt_reg[] = { .save = set_save_v3, .extra_opts = set_opts_v3, }, + { + .name = "set", + .revision = 4, + .version = XTABLES_VERSION, + .family = NFPROTO_UNSPEC, + .size = XT_ALIGN(sizeof(struct xt_set_info_match_v3)), + .userspacesize = XT_ALIGN(sizeof(struct xt_set_info_match_v3)), + .help = set_help_v4, + .parse = set_parse_v4, + .final_check = set_check_v0, + .print = set_print_v4, + .save = set_save_v4, + .extra_opts = set_opts_v4, + }, }; void _init(void) diff --git a/extensions/libxt_set.man b/extensions/libxt_set.man index 7012ef2..21e443c 100644 --- a/extensions/libxt_set.man +++ b/extensions/libxt_set.man @@ -21,6 +21,16 @@ supports the \fBnomatch\fP flag, then the matching is reversed: a match with an element flagged with \fBnomatch\fP returns \fBtrue\fP, while a match with a plain element returns \fBfalse\fP. .TP +\fB\-\-inner\fP +Please note that this option will only produce matches in the event of +the following ICMPv4 and ICMPv6 messages: \fBdestination-unreachable\fP +(ICMPv4 code 3 and ICMPv6 code 1), \fBsource-quench\fP (ICMPv4 code 4), +\fBtime-exceeded\fP (ICMPv4 code 11 and ICMPv6 code 3) and +\fBpacket-too-big\fP (ICMPv6 code 2). If the \fB\-\-inner\fP option is +specified, then element matching is based on the properties +(source/destination IP address, protocol, port and so on) of the original +(inner) connection, which produced the above ICMP[v6] type messages. +.TP \fB!\fP \fB\-\-update\-counters\fP If the \fB\-\-update\-counters\fP flag is negated, then the packet and byte counters of the matching element in the set won't be updated. Default diff --git a/include/linux/netfilter/ipset/ip_set.h b/include/linux/netfilter/ipset/ip_set.h index eb9123e..1b3d9cb 100644 --- a/include/linux/netfilter/ipset/ip_set.h +++ b/include/linux/netfilter/ipset/ip_set.h @@ -161,6 +161,8 @@ enum ipset_cmd_flags { (1 << IPSET_FLAG_BIT_SKIP_SUBCOUNTER_UPDATE), IPSET_FLAG_BIT_MATCH_COUNTERS = 5, IPSET_FLAG_MATCH_COUNTERS = (1 << IPSET_FLAG_BIT_MATCH_COUNTERS), + IPSET_FLAG_BIT_INNER = 6, + IPSET_FLAG_INNER = (1 << IPSET_FLAG_BIT_INNER), IPSET_FLAG_BIT_RETURN_NOMATCH = 7, IPSET_FLAG_RETURN_NOMATCH = (1 << IPSET_FLAG_BIT_RETURN_NOMATCH), IPSET_FLAG_CMD_MAX = 15, ^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2013-06-09 8:54 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <51A54267.8050402-git-send-email-mr.dash.four@googlemail.com>
2013-05-29 0:13 ` [PATCH 1/4] ipset: minor variable-naming corrections Dash Four
2013-05-29 0:13 ` [PATCH 2/4] ipset: add "inner" flag implementation Dash Four
2013-06-01 10:54 ` Jozsef Kadlecsik
2013-06-07 1:13 ` Dash Four
2013-06-09 8:54 ` Jozsef Kadlecsik
2013-05-29 0:13 ` [PATCH 3/4] iptables: add set match "inner" flag support Dash Four
2013-05-29 0:13 ` [PATCH 4/4] iptables (userspace): " Dash Four
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).