netfilter: ctnetlink: use netlink policy range checks

Replace manual range and mask validations with netlink policy
annotations in ctnetlink code paths, so that the netlink core rejects
invalid values early and can generate extack errors.

- CTA_PROTOINFO_TCP_STATE: reject values > TCP_CONNTRACK_SYN_SENT2 at
  policy level, removing the manual >= TCP_CONNTRACK_MAX check.
- CTA_PROTOINFO_TCP_WSCALE_ORIGINAL/REPLY: reject values > TCP_MAX_WSCALE
  (14). The normal TCP option parsing path already clamps to this value,
  but the ctnetlink path accepted 0-255, causing undefined behavior when
  used as a u32 shift count.
- CTA_FILTER_ORIG_FLAGS/REPLY_FLAGS: use NLA_POLICY_MASK with
  CTA_FILTER_F_ALL, removing the manual mask checks.
- CTA_EXPECT_FLAGS: use NLA_POLICY_MASK with NF_CT_EXPECT_MASK, adding
  a new mask define grouping all valid expect flags.

Extracted from a broader nf-next patch by Florian Westphal, scoped to
ctnetlink for the fixes tree.

Fixes: c8e2078cfe41 ("[NETFILTER]: ctnetlink: add support for internal tcp connection tracking flags handling")
Signed-off-by: David Carlier <devnexen@gmail.com>
Co-developed-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

diff --git a/include/uapi/linux/netfilter/nf_conntrack_common.h b/include/uapi/linux/netfilter/nf_conntrack_common.h
index 26071021e986f6e3f6f71cf12fba8bd11a2390bd..56b6b60a814f5ef56472ebaa3917011ebfa823b4 100644 (file)
--- a/include/uapi/linux/netfilter/nf_conntrack_common.h
+++ b/include/uapi/linux/netfilter/nf_conntrack_common.h
@@ -159,5 +159,9 @@ enum ip_conntrack_expect_events {
 #define NF_CT_EXPECT_INACTIVE          0x2
 #define NF_CT_EXPECT_USERSPACE         0x4
 
+#ifdef __KERNEL__
+#define NF_CT_EXPECT_MASK      (NF_CT_EXPECT_PERMANENT | NF_CT_EXPECT_INACTIVE | \
+                                NF_CT_EXPECT_USERSPACE)
+#endif
 
 #endif /* _UAPI_NF_CONNTRACK_COMMON_H */

diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c
index 6e6aeb0ab0a1bd1779eed6ee7cf7db4b397d6da3..3f408f3713bb333374a722af6209df8dfa38d66d 100644 (file)
--- a/net/netfilter/nf_conntrack_netlink.c
+++ b/net/netfilter/nf_conntrack_netlink.c
@@ -910,8 +910,8 @@ struct ctnetlink_filter {
 };
 
 static const struct nla_policy cta_filter_nla_policy[CTA_FILTER_MAX + 1] = {
-       [CTA_FILTER_ORIG_FLAGS]         = { .type = NLA_U32 },
-       [CTA_FILTER_REPLY_FLAGS]        = { .type = NLA_U32 },
+       [CTA_FILTER_ORIG_FLAGS]         = NLA_POLICY_MASK(NLA_U32, CTA_FILTER_F_ALL),
+       [CTA_FILTER_REPLY_FLAGS]        = NLA_POLICY_MASK(NLA_U32, CTA_FILTER_F_ALL),
 };
 
 static int ctnetlink_parse_filter(const struct nlattr *attr,
@@ -925,17 +925,11 @@ static int ctnetlink_parse_filter(const struct nlattr *attr,
        if (ret)
                return ret;
 
-       if (tb[CTA_FILTER_ORIG_FLAGS]) {
+       if (tb[CTA_FILTER_ORIG_FLAGS])
                filter->orig_flags = nla_get_u32(tb[CTA_FILTER_ORIG_FLAGS]);
-               if (filter->orig_flags & ~CTA_FILTER_F_ALL)
-                       return -EOPNOTSUPP;
-       }
 
-       if (tb[CTA_FILTER_REPLY_FLAGS]) {
+       if (tb[CTA_FILTER_REPLY_FLAGS])
                filter->reply_flags = nla_get_u32(tb[CTA_FILTER_REPLY_FLAGS]);
-               if (filter->reply_flags & ~CTA_FILTER_F_ALL)
-                       return -EOPNOTSUPP;
-       }
 
        return 0;
 }
@@ -2634,7 +2628,7 @@ static const struct nla_policy exp_nla_policy[CTA_EXPECT_MAX+1] = {
        [CTA_EXPECT_HELP_NAME]  = { .type = NLA_NUL_STRING,
                                    .len = NF_CT_HELPER_NAME_LEN - 1 },
        [CTA_EXPECT_ZONE]       = { .type = NLA_U16 },
-       [CTA_EXPECT_FLAGS]      = { .type = NLA_U32 },
+       [CTA_EXPECT_FLAGS]      = NLA_POLICY_MASK(NLA_BE32, NF_CT_EXPECT_MASK),
        [CTA_EXPECT_CLASS]      = { .type = NLA_U32 },
        [CTA_EXPECT_NAT]        = { .type = NLA_NESTED },
        [CTA_EXPECT_FN]         = { .type = NLA_NUL_STRING },

diff --git a/net/netfilter/nf_conntrack_proto_tcp.c b/net/netfilter/nf_conntrack_proto_tcp.c
index 0c1d086e96cb3f69a6849b1fca6666714c4e0a98..b67426c2189b2d8a02e7caa26e848b539379a8cd 100644 (file)
--- a/net/netfilter/nf_conntrack_proto_tcp.c
+++ b/net/netfilter/nf_conntrack_proto_tcp.c
@@ -1385,9 +1385,9 @@ nla_put_failure:
 }
 
 static const struct nla_policy tcp_nla_policy[CTA_PROTOINFO_TCP_MAX+1] = {
-       [CTA_PROTOINFO_TCP_STATE]           = { .type = NLA_U8 },
-       [CTA_PROTOINFO_TCP_WSCALE_ORIGINAL] = { .type = NLA_U8 },
-       [CTA_PROTOINFO_TCP_WSCALE_REPLY]    = { .type = NLA_U8 },
+       [CTA_PROTOINFO_TCP_STATE]           = NLA_POLICY_MAX(NLA_U8, TCP_CONNTRACK_SYN_SENT2),
+       [CTA_PROTOINFO_TCP_WSCALE_ORIGINAL] = NLA_POLICY_MAX(NLA_U8, TCP_MAX_WSCALE),
+       [CTA_PROTOINFO_TCP_WSCALE_REPLY]    = NLA_POLICY_MAX(NLA_U8, TCP_MAX_WSCALE),
        [CTA_PROTOINFO_TCP_FLAGS_ORIGINAL]  = { .len = sizeof(struct nf_ct_tcp_flags) },
        [CTA_PROTOINFO_TCP_FLAGS_REPLY]     = { .len = sizeof(struct nf_ct_tcp_flags) },
 };
@@ -1414,10 +1414,6 @@ static int nlattr_to_tcp(struct nlattr *cda[], struct nf_conn *ct)
        if (err < 0)
                return err;
 
-       if (tb[CTA_PROTOINFO_TCP_STATE] &&
-           nla_get_u8(tb[CTA_PROTOINFO_TCP_STATE]) >= TCP_CONNTRACK_MAX)
-               return -EINVAL;
-
        spin_lock_bh(&ct->lock);
        if (tb[CTA_PROTOINFO_TCP_STATE])
                ct->proto.tcp.state = nla_get_u8(tb[CTA_PROTOINFO_TCP_STATE]);