]> git.ipfire.org Git - thirdparty/nftables.git/commitdiff
src: context tracking for multiple transport protocols
authorPablo Neira Ayuso <pablo@netfilter.org>
Mon, 14 Sep 2020 18:51:20 +0000 (20:51 +0200)
committerPablo Neira Ayuso <pablo@netfilter.org>
Tue, 15 Sep 2020 17:03:36 +0000 (19:03 +0200)
This patch extends the protocol context infrastructure to track multiple
transport protocols when they are specified from sets.

This removes errors like:

 "transport protocol mapping is only valid after transport protocol match"

when invoking:

 # nft add rule x z meta l4proto { tcp, udp } dnat to 1.1.1.1:80

This patch also catches conflicts like:

 # nft add rule x z ip protocol { tcp, udp } tcp dport 20 dnat to 1.1.1.1:80
 Error: conflicting protocols specified: udp vs. tcp
 add rule x z ip protocol { tcp, udp } tcp dport 20 dnat to 1.1.1.1:80
                                       ^^^^^^^^^
and:

 # nft add rule x z meta l4proto { tcp, udp } tcp dport 20 dnat to 1.1.1.1:80
 Error: conflicting protocols specified: udp vs. tcp
 add rule x z meta l4proto { tcp, udp } tcp dport 20 dnat to 1.1.1.1:80
                                        ^^^^^^^^^
Note that:

- the singleton protocol context tracker is left in place until the
  existing users are updated to use this new multiprotocol tracker.
  Moving forward, it would be good to consolidate things around this new
  multiprotocol context tracker infrastructure.

- link and network layers are not updated to use this infrastructure
  yet. The code that deals with vlan conflicts relies on forcing
  protocol context updates to the singleton protocol base.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
include/expression.h
include/proto.h
src/ct.c
src/evaluate.c
src/expression.c
src/meta.c
src/payload.c
src/proto.c
tests/py/inet/dnat.t
tests/py/inet/dnat.t.payload

index 130912a89e04c0d48d13d4f799cf11705a867b5e..b039882cf1d18f94d2b8679e5b75eca64de9351a 100644 (file)
@@ -167,7 +167,9 @@ struct expr_ops {
        bool                    (*cmp)(const struct expr *e1,
                                       const struct expr *e2);
        void                    (*pctx_update)(struct proto_ctx *ctx,
-                                              const struct expr *expr);
+                                              const struct location *loc,
+                                              const struct expr *left,
+                                              const struct expr *right);
        int                     (*build_udata)(struct nftnl_udata_buf *udbuf,
                                               const struct expr *expr);
        struct expr *           (*parse_udata)(const struct nftnl_udata *ud);
index 1771ba8e8d8cc7390f9c398ded470705a9f57461..304b048e4e6007d1eb97ebbfddea14162cd4c413 100644 (file)
@@ -152,6 +152,8 @@ struct dev_proto_desc {
 extern int proto_dev_type(const struct proto_desc *desc, uint16_t *res);
 extern const struct proto_desc *proto_dev_desc(uint16_t type);
 
+#define PROTO_CTX_NUM_PROTOS   16
+
 /**
  * struct proto_ctx - protocol context
  *
@@ -172,6 +174,11 @@ struct proto_ctx {
                struct location                 location;
                const struct proto_desc         *desc;
                unsigned int                    offset;
+               struct {
+                       struct location         location;
+                       const struct proto_desc *desc;
+               } protos[PROTO_CTX_NUM_PROTOS];
+               unsigned int                    num_protos;
        } protocol[PROTO_BASE_MAX + 1];
 };
 
@@ -180,6 +187,10 @@ extern void proto_ctx_init(struct proto_ctx *ctx, unsigned int family,
 extern void proto_ctx_update(struct proto_ctx *ctx, enum proto_bases base,
                             const struct location *loc,
                             const struct proto_desc *desc);
+bool proto_ctx_is_ambiguous(struct proto_ctx *ctx, enum proto_bases bases);
+const struct proto_desc *proto_ctx_find_conflict(struct proto_ctx *ctx,
+                                                enum proto_bases base,
+                                                const struct proto_desc *desc);
 extern const struct proto_desc *proto_find_upper(const struct proto_desc *base,
                                                 unsigned int num);
 extern int proto_find_num(const struct proto_desc *base,
index 0842c838b913392379ec6fd6ed8667adfe319f39..2218ecc7a684d1e0193cf0e2566f40f361cfae9e 100644 (file)
--- a/src/ct.c
+++ b/src/ct.c
@@ -351,9 +351,11 @@ static void ct_expr_clone(struct expr *new, const struct expr *expr)
        new->ct = expr->ct;
 }
 
-static void ct_expr_pctx_update(struct proto_ctx *ctx, const struct expr *expr)
+static void ct_expr_pctx_update(struct proto_ctx *ctx,
+                               const struct location *loc,
+                               const struct expr *left,
+                               const struct expr *right)
 {
-       const struct expr *left = expr->left, *right = expr->right;
        const struct proto_desc *base = NULL, *desc;
        uint32_t nhproto;
 
@@ -366,7 +368,7 @@ static void ct_expr_pctx_update(struct proto_ctx *ctx, const struct expr *expr)
        if (!desc)
                return;
 
-       proto_ctx_update(ctx, left->ct.base + 1, &expr->location, desc);
+       proto_ctx_update(ctx, left->ct.base + 1, loc, desc);
 }
 
 #define NFTNL_UDATA_CT_KEY 0
index e3fe70624699bb4a18b8d7fa9b84d65a57bd3a3e..c8045e5ded729fa375b3d5f9cdf191f6f46eafed 100644 (file)
@@ -710,6 +710,17 @@ static int __expr_evaluate_payload(struct eval_ctx *ctx, struct expr *expr)
                return 0;
        }
 
+       if (payload->payload.base == desc->base &&
+           proto_ctx_is_ambiguous(&ctx->pctx, base)) {
+               desc = proto_ctx_find_conflict(&ctx->pctx, base, payload->payload.desc);
+               assert(desc);
+
+               return expr_error(ctx->msgs, payload,
+                                 "conflicting protocols specified: %s vs. %s",
+                                 desc->name,
+                                 payload->payload.desc->name);
+       }
+
        /* No conflict: Same payload protocol as context, adjust offset
         * if needed.
         */
@@ -1874,8 +1885,7 @@ static int expr_evaluate_relational(struct eval_ctx *ctx, struct expr **expr)
                 * Update protocol context for payload and meta iiftype
                 * equality expressions.
                 */
-               if (expr_is_singleton(right))
-                       relational_expr_pctx_update(&ctx->pctx, rel);
+               relational_expr_pctx_update(&ctx->pctx, rel);
 
                /* fall through */
        case OP_NEQ:
index fe529f98de7b153818d686955d3f8ac6d10c302d..87bd4d01bb722b32d6ce7f14d31a18ade847d00c 100644 (file)
@@ -708,16 +708,26 @@ struct expr *relational_expr_alloc(const struct location *loc, enum ops op,
 void relational_expr_pctx_update(struct proto_ctx *ctx,
                                 const struct expr *expr)
 {
-       const struct expr *left = expr->left;
+       const struct expr *left = expr->left, *right = expr->right;
        const struct expr_ops *ops;
+       const struct expr *i;
 
        assert(expr->etype == EXPR_RELATIONAL);
        assert(expr->op == OP_EQ || expr->op == OP_IMPLICIT);
 
        ops = expr_ops(left);
        if (ops->pctx_update &&
-           (left->flags & EXPR_F_PROTOCOL))
-               ops->pctx_update(ctx, expr);
+           (left->flags & EXPR_F_PROTOCOL)) {
+               if (expr_is_singleton(right))
+                       ops->pctx_update(ctx, &expr->location, left, right);
+               else if (right->etype == EXPR_SET) {
+                       list_for_each_entry(i, &right->expressions, list) {
+                               if (i->etype == EXPR_SET_ELEM &&
+                                   i->key->etype == EXPR_VALUE)
+                                       ops->pctx_update(ctx, &expr->location, left, i->key);
+                       }
+               }
+       }
 }
 
 static void range_expr_print(const struct expr *expr, struct output_ctx *octx)
index d92d0d323b9b9b5787c47cbdaea179d7508f9f95..73d58b1f53b5d631928b1021ab138e587b0b71fa 100644 (file)
@@ -753,10 +753,11 @@ static void meta_expr_clone(struct expr *new, const struct expr *expr)
  * Update LL protocol context based on IIFTYPE meta match in non-LL hooks.
  */
 static void meta_expr_pctx_update(struct proto_ctx *ctx,
-                                 const struct expr *expr)
+                                 const struct location *loc,
+                                 const struct expr *left,
+                                 const struct expr *right)
 {
        const struct hook_proto_desc *h = &hook_proto_desc[ctx->family];
-       const struct expr *left = expr->left, *right = expr->right;
        const struct proto_desc *desc;
        uint8_t protonum;
 
@@ -771,7 +772,7 @@ static void meta_expr_pctx_update(struct proto_ctx *ctx,
                if (desc == NULL)
                        desc = &proto_unknown;
 
-               proto_ctx_update(ctx, PROTO_BASE_LL_HDR, &expr->location, desc);
+               proto_ctx_update(ctx, PROTO_BASE_LL_HDR, loc, desc);
                break;
        case NFT_META_NFPROTO:
                protonum = mpz_get_uint8(right->value);
@@ -784,7 +785,7 @@ static void meta_expr_pctx_update(struct proto_ctx *ctx,
                                desc = h->desc;
                }
 
-               proto_ctx_update(ctx, PROTO_BASE_NETWORK_HDR, &expr->location, desc);
+               proto_ctx_update(ctx, PROTO_BASE_NETWORK_HDR, loc, desc);
                break;
        case NFT_META_L4PROTO:
                desc = proto_find_upper(&proto_inet_service,
@@ -792,7 +793,7 @@ static void meta_expr_pctx_update(struct proto_ctx *ctx,
                if (desc == NULL)
                        desc = &proto_unknown;
 
-               proto_ctx_update(ctx, PROTO_BASE_TRANSPORT_HDR, &expr->location, desc);
+               proto_ctx_update(ctx, PROTO_BASE_TRANSPORT_HDR, loc, desc);
                break;
        case NFT_META_PROTOCOL:
                if (h->base != PROTO_BASE_LL_HDR)
@@ -806,7 +807,7 @@ static void meta_expr_pctx_update(struct proto_ctx *ctx,
                if (desc == NULL)
                        desc = &proto_unknown;
 
-               proto_ctx_update(ctx, PROTO_BASE_NETWORK_HDR, &expr->location, desc);
+               proto_ctx_update(ctx, PROTO_BASE_NETWORK_HDR, loc, desc);
                break;
        default:
                break;
index 29242537237e220068f255755a56811d11eb785f..ca422d5bcd561d67a52791e1ed8f9300e179ede0 100644 (file)
@@ -80,9 +80,10 @@ static void payload_expr_clone(struct expr *new, const struct expr *expr)
  * Update protocol context for relational payload expressions.
  */
 static void payload_expr_pctx_update(struct proto_ctx *ctx,
-                                    const struct expr *expr)
+                                    const struct location *loc,
+                                    const struct expr *left,
+                                    const struct expr *right)
 {
-       const struct expr *left = expr->left, *right = expr->right;
        const struct proto_desc *base, *desc;
        unsigned int proto = 0;
 
@@ -102,7 +103,7 @@ static void payload_expr_pctx_update(struct proto_ctx *ctx,
                assert(base->length > 0);
                ctx->protocol[base->base].offset += base->length;
        }
-       proto_ctx_update(ctx, desc->base, &expr->location, desc);
+       proto_ctx_update(ctx, desc->base, loc, desc);
 }
 
 #define NFTNL_UDATA_SET_KEY_PAYLOAD_DESC 0
index 7d001501d7d2f8fc516efa5f0b3428886325b306..7de2bbf91ae4abc09197d6d0928ee4ea6acedb09 100644 (file)
@@ -193,12 +193,69 @@ void proto_ctx_update(struct proto_ctx *ctx, enum proto_bases base,
                      const struct location *loc,
                      const struct proto_desc *desc)
 {
+       bool found = false;
+       unsigned int i;
+
+       switch (base) {
+       case PROTO_BASE_LL_HDR:
+       case PROTO_BASE_NETWORK_HDR:
+               break;
+       case PROTO_BASE_TRANSPORT_HDR:
+               if (ctx->protocol[base].num_protos >= PROTO_CTX_NUM_PROTOS)
+                       break;
+
+               for (i = 0; i < ctx->protocol[base].num_protos; i++) {
+                       if (ctx->protocol[base].protos[i].desc == desc) {
+                               found = true;
+                               break;
+                       }
+               }
+               if (!found) {
+                       i = ctx->protocol[base].num_protos++;
+                       ctx->protocol[base].protos[i].desc = desc;
+                       ctx->protocol[base].protos[i].location = *loc;
+               }
+               break;
+       default:
+               BUG("unknown protocol base %d", base);
+       }
+
        ctx->protocol[base].location    = *loc;
        ctx->protocol[base].desc        = desc;
 
        proto_ctx_debug(ctx, base, ctx->debug_mask);
 }
 
+bool proto_ctx_is_ambiguous(struct proto_ctx *ctx, enum proto_bases base)
+{
+       return ctx->protocol[base].num_protos > 1;
+}
+
+const struct proto_desc *proto_ctx_find_conflict(struct proto_ctx *ctx,
+                                                enum proto_bases base,
+                                                const struct proto_desc *desc)
+{
+       unsigned int i;
+
+       switch (base) {
+       case PROTO_BASE_LL_HDR:
+       case PROTO_BASE_NETWORK_HDR:
+               if (desc != ctx->protocol[base].desc)
+                       return ctx->protocol[base].desc;
+               break;
+       case PROTO_BASE_TRANSPORT_HDR:
+               for (i = 0; i < ctx->protocol[base].num_protos; i++) {
+                       if (desc != ctx->protocol[base].protos[i].desc)
+                               return ctx->protocol[base].protos[i].desc;
+               }
+               break;
+       default:
+               BUG("unknown protocol base %d", base);
+       }
+
+       return NULL;
+}
+
 #define HDR_TEMPLATE(__name, __dtype, __type, __member)                        \
        PROTO_HDR_TEMPLATE(__name, __dtype,                             \
                           BYTEORDER_BIG_ENDIAN,                        \
index fcdf9436c6764e8357bbe5a2b200ef698d0590b8..a266100890e3e1a3387e5b52ca1396b0ba671146 100644 (file)
@@ -14,3 +14,8 @@ dnat ip6 to 1.2.3.4;fail
 dnat to 1.2.3.4;fail
 dnat ip6 to ct mark . ip daddr map { 0x00000014 . 1.1.1.1 : 1.2.3.4};fail
 ip6 daddr dead::beef dnat to 10.1.2.3;fail
+
+meta l4proto { tcp, udp } dnat ip to 1.1.1.1:80;ok
+ip protocol { tcp, udp } dnat ip to 1.1.1.1:80;ok
+meta l4proto { tcp, udp } tcp dport 20 dnat to 1.1.1.1:80;fail
+ip protocol { tcp, udp } tcp dport 20 dnat to 1.1.1.1:80;fail
index 75cf1b7779b7d5c306c4b2f2def146a3ef006589..a741b9cbdb8d745da1512b5ebfe41c8352df859f 100644 (file)
@@ -52,3 +52,28 @@ inet test-inet prerouting
   [ payload load 4b @ network header + 16 => reg 9 ]
   [ lookup reg 1 set __map%d dreg 1 ]
   [ nat dnat ip addr_min reg 1 addr_max reg 0 ]
+
+# meta l4proto { tcp, udp } dnat ip to 1.1.1.1:80
+__set%d test-inet 3
+__set%d test-inet 0
+        element 00000006  : 0 [end]     element 00000011  : 0 [end]
+inet
+  [ meta load l4proto => reg 1 ]
+  [ lookup reg 1 set __set%d ]
+  [ immediate reg 1 0x01010101 ]
+  [ immediate reg 2 0x00005000 ]
+  [ nat dnat ip addr_min reg 1 addr_max reg 0 proto_min reg 2 proto_max reg 0 flags 0x2 ]
+
+# ip protocol { tcp, udp } dnat ip to 1.1.1.1:80
+__set%d test-inet 3
+__set%d test-inet 0
+        element 00000006  : 0 [end]     element 00000011  : 0 [end]
+inet
+  [ meta load nfproto => reg 1 ]
+  [ cmp eq reg 1 0x00000002 ]
+  [ payload load 1b @ network header + 9 => reg 1 ]
+  [ lookup reg 1 set __set%d ]
+  [ immediate reg 1 0x01010101 ]
+  [ immediate reg 2 0x00005000 ]
+  [ nat dnat ip addr_min reg 1 addr_max reg 0 proto_min reg 2 proto_max reg 0 flags 0x2 ]
+