| Message ID | 1409375835-28041-1-git-send-email-ycnian@gmail.com |
|---|---|
| State | Not Applicable |
| Delegated to: | Pablo Neira |
| Headers | show |
On Sat, Aug 30, 2014 at 01:17:15PM +0800, Yanchuan Nian wrote: > The protocol expression that should be killed when payload parsing > isn't the first one but the last one. Look at the result of this command: That patch is competely wrong. Have you actually tested any other case? You're simply not killing any payload dependency anymore. The correct fix is to check for OP_NEQ and deciding not to kill it based on that. > > nft> add rule ip filter input ip protocol != tcp tcp sport 80 drop > nft> list table ip filter > table ip filter { > chain input { > type filter hook input priority 0; > ip protocol tcp tcp sport http drop > } > } > nft> > > With this patch, the result is: > nft> add rule ip filter input ip protocol != tcp tcp sport 80 drop > nft> list table ip filter > table ip filter { > chain input { > type filter hook input priority 0; > ip protocol != tcp tcp sport http drop > } > } > nft> > > Signed-off-by: Yanchuan Nian <ycnian@gmail.com> > --- > src/netlink_delinearize.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c > index 195d432..322c7cc 100644 > --- a/src/netlink_delinearize.c > +++ b/src/netlink_delinearize.c > @@ -671,12 +671,11 @@ static void payload_match_postprocess(struct rule_pp_ctx *ctx, > nstmt = expr_stmt_alloc(&stmt->location, nexpr); > list_add_tail(&nstmt->list, &stmt->list); > > - /* Remember the first payload protocol expression to > + /* Remember the last payload protocol expression to > * kill it later on if made redundant by a higher layer > * payload expression. > */ > - if (ctx->pbase == PROTO_BASE_INVALID && > - left->flags & EXPR_F_PROTOCOL) > + if (left->flags & EXPR_F_PROTOCOL) > payload_dependency_store(ctx, nstmt, > left->payload.base); > else > -- > 1.9.3 > -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, Aug 30, 2014 at 11:52:49AM +0100, Patrick McHardy wrote: > On Sat, Aug 30, 2014 at 01:17:15PM +0800, Yanchuan Nian wrote: > > The protocol expression that should be killed when payload parsing > > isn't the first one but the last one. Look at the result of this command: > > That patch is competely wrong. Have you actually tested any other case? > You're simply not killing any payload dependency anymore. > > The correct fix is to check for OP_NEQ and deciding not to kill it based > on that. > Hi Patrick, Thanks to your replay. Yes, this patch is wrong. It was careless of me forgetting to test it. I am sorry and I will try to fix it. Thank you again. > > > > nft> add rule ip filter input ip protocol != tcp tcp sport 80 drop > > nft> list table ip filter > > table ip filter { > > chain input { > > type filter hook input priority 0; > > ip protocol tcp tcp sport http drop > > } > > } > > nft> > > > > With this patch, the result is: > > nft> add rule ip filter input ip protocol != tcp tcp sport 80 drop > > nft> list table ip filter > > table ip filter { > > chain input { > > type filter hook input priority 0; > > ip protocol != tcp tcp sport http drop > > } > > } > > nft> > > > > Signed-off-by: Yanchuan Nian <ycnian@gmail.com> > > --- > > src/netlink_delinearize.c | 5 ++--- > > 1 file changed, 2 insertions(+), 3 deletions(-) > > > > diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c > > index 195d432..322c7cc 100644 > > --- a/src/netlink_delinearize.c > > +++ b/src/netlink_delinearize.c > > @@ -671,12 +671,11 @@ static void payload_match_postprocess(struct rule_pp_ctx *ctx, > > nstmt = expr_stmt_alloc(&stmt->location, nexpr); > > list_add_tail(&nstmt->list, &stmt->list); > > > > - /* Remember the first payload protocol expression to > > + /* Remember the last payload protocol expression to > > * kill it later on if made redundant by a higher layer > > * payload expression. > > */ > > - if (ctx->pbase == PROTO_BASE_INVALID && > > - left->flags & EXPR_F_PROTOCOL) > > + if (left->flags & EXPR_F_PROTOCOL) > > payload_dependency_store(ctx, nstmt, > > left->payload.base); > > else > > -- > > 1.9.3 > > > -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c index 195d432..322c7cc 100644 --- a/src/netlink_delinearize.c +++ b/src/netlink_delinearize.c @@ -671,12 +671,11 @@ static void payload_match_postprocess(struct rule_pp_ctx *ctx, nstmt = expr_stmt_alloc(&stmt->location, nexpr); list_add_tail(&nstmt->list, &stmt->list); - /* Remember the first payload protocol expression to + /* Remember the last payload protocol expression to * kill it later on if made redundant by a higher layer * payload expression. */ - if (ctx->pbase == PROTO_BASE_INVALID && - left->flags & EXPR_F_PROTOCOL) + if (left->flags & EXPR_F_PROTOCOL) payload_dependency_store(ctx, nstmt, left->payload.base); else
The protocol expression that should be killed when payload parsing isn't the first one but the last one. Look at the result of this command: nft> add rule ip filter input ip protocol != tcp tcp sport 80 drop nft> list table ip filter table ip filter { chain input { type filter hook input priority 0; ip protocol tcp tcp sport http drop } } nft> With this patch, the result is: nft> add rule ip filter input ip protocol != tcp tcp sport 80 drop nft> list table ip filter table ip filter { chain input { type filter hook input priority 0; ip protocol != tcp tcp sport http drop } } nft> Signed-off-by: Yanchuan Nian <ycnian@gmail.com> --- src/netlink_delinearize.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)