netdev - Re: [PATCH net-next,v2 2/2] net: sched: add em_ipt ematch for calling xtables matches

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 26 Jan 2018 21:57:00 +0200
From:   Eyal Birger <eyal.birger@...il.com>
To:     Pablo Neira Ayuso <pablo@...filter.org>
Cc:     David Miller <davem@...emloft.net>,
        Jamal Hadi Salim <jhs@...atatu.com>,
        Cong Wang <xiyou.wangcong@...il.com>,
        Linux Kernel Network Developers <netdev@...r.kernel.org>,
        shmulik@...anetworks.com, Eyal Birger <eyal@...anetworks.com>
Subject: Re: [PATCH net-next,v2 2/2] net: sched: add em_ipt ematch for calling
 xtables matches

On Fri, Jan 26, 2018 at 8:50 PM, Pablo Neira Ayuso <pablo@...filter.org> wrote:
> On Fri, Jan 26, 2018 at 06:48:53PM +0200, Eyal Birger wrote:
>> diff --git a/net/sched/em_ipt.c b/net/sched/em_ipt.c
>> new file mode 100644
>> index 0000000..2103b30
>> --- /dev/null
>> +++ b/net/sched/em_ipt.c
> [...]
>> +static int em_ipt_match(struct sk_buff *skb, struct tcf_ematch *em,
>> +                     struct tcf_pkt_info *info)
>> +{
>> +     const struct em_ipt_match *im = (const void *)em->data;
>> +     struct xt_action_param acpar = {};
>> +     struct net_device *indev = NULL;
>> +     struct nf_hook_state state;
>> +     int ret;
>> +
>> +     if (unlikely(!skb_at_tc_ingress(skb))) {
>> +             pr_notice_once("ipt match must not be used at egress\n");
>
> Isn't there a way to reject the use of this from ->change()? ie. from
> control plane configuration.

I wasn't able to find a simple way of doing so:

- AFAIU tc filters are detached from the qdiscs they operate on via
tcf_block instances
  that may be shared by different qdiscs. I was not able to be sure that filters
  attached to ingress qdiscs via tcf_blocks at configuration time
cannot be later be shared
  with non ingress qdiscs. Nor was I able to find another classifier
making the ingress/egress
  distinction at configuration time.

- ematches are not provided with 'ingress/egress' information at
'change()' invocation, though
  of course the infrastructure could be extended to provide this,
given the distinction is available.

Eyal.