On Wed, Jun 20, 2018 at 02:21:18PM +0200, Máté Eckl wrote:
> On Wed, Jun 20, 2018 at 01:40:45PM +0200, Pablo Neira Ayuso wrote:
> > On Mon, Jun 18, 2018 at 11:57:10AM +0200, Máté Eckl wrote:
> > > Signed-off-by: Máté Eckl <ecklm94@xxxxxxxxx>
> > > ---
> > > src/evaluate.c | 20 ++++++++++++++++++--
> > > 1 file changed, 18 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/src/evaluate.c b/src/evaluate.c
> > > index d6aff61..0564b44 100644
> > > --- a/src/evaluate.c
> > > +++ b/src/evaluate.c
> > > @@ -2431,12 +2431,28 @@ static int evaluate_addr(struct eval_ctx *ctx, struct stmt *stmt,
> > > const struct datatype *dtype;
> > > unsigned int len;
> > >
> > > - if (pctx->family == NFPROTO_IPV4) {
> > > + switch (pctx->family) {
> > > + case NFPROTO_IPV4:
> > > dtype = &ipaddr_type;
> > > len = 4 * BITS_PER_BYTE;
> > > - } else {
> > > + break;
> > > + case NFPROTO_IPV6:
> > > dtype = &ip6addr_type;
> > > len = 16 * BITS_PER_BYTE;
> > > + break;
> > > + case NFPROTO_INET:
> > > + if (strchr((*expr)->identifier, ':')) {
> >
> > I'd suggest you specify this in this syntax:
> >
> > tproxy ip to 1.1.1.1
> >
> > for the bridge/netdev/inet families.
> >
> > From the kernel, this will also skip non-IP packets, so we don't need
> > to build an IP dependency for this statement.
>
> This patch solves a problem regardless of the tproxy functionality, as it was
> impossible to specify an address other than ipv6 in non-ip tables. Tproxy was
> only an example to demonstrate the error.
>
> If this patch is applied, there is no need for the 'ip' here (and I'd like to
> avoid it). Bridge and netdev tables are not supported to use tproxy in.
For ip/ip6, tproxy to 1.1.1.1 is fine.
But for bridge/netdev/inet, I think it makes explicit the dependency
with either ip and ip6. So this is visible from the ruleset that this
rule will only apply to either ip or ip6.
And we'll skip having to generate a dependency for this, which is
always more work, given that from the kernel we can just add:
if (skb->protocol != htons(ETH_P_IP))
... break verdict ...
which is actually needed for safety reasons.
I think your kernel patch is also lacking this, and a custom userspace
program may add a tproxy expression to deal with IPV6 traffic, which
may result in crashing the kernel.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
