linux-kernel - Re: [PATCH] netfilter: nf_tables: add SECMARK support

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]

Message-ID: <20180920094458.6hwaw3vgfsxvcgb4@breakpoint.cc>
Date:   Thu, 20 Sep 2018 11:44:58 +0200
From:   Florian Westphal <fw@...len.de>
To:     Christian Göttsche <cgzones@...glemail.com>
Cc:     fw@...len.de, casey@...aufler-ca.com, pablo@...filter.org,
        kadlec@...ckhole.kfki.hu, davem@...emloft.net,
        netfilter-devel@...r.kernel.org, coreteam@...filter.org,
        netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
        Paul Moore <paul@...l-moore.com>,
        Stephen Smalley <sds@...ho.nsa.gov>,
        Eric Paris <eparis@...isplace.org>, jmorris@...ei.org,
        serge@...lyn.com, selinux <selinux@...ho.nsa.gov>,
        linux-security-module@...r.kernel.org
Subject: Re: [PATCH] netfilter: nf_tables: add SECMARK support

Christian Göttsche <cgzones@...glemail.com> wrote:
> > Fixes should go into nf.git whereas feature goes to nf-next.git.
> 
> No, that should not be a unroll fix.
> Currently there are no objects registered by the main nf_tables
> module, so for nft_secmark_obj_type I had to introduce this new logic.

I see, ok.

> > > > +   if (err) {
> > > > +           if (err == -EINVAL)
> > > > +                   pr_notice_ratelimited("invalid security context \'%s\'\n", priv->ctx);
> > > > +           else
> > > > +                   pr_notice_ratelimited("unable to convert security context \'%s\': %d\n", priv->ctx, -err);
> > > > +           return;
> > > > +   }
> >
> > Please remove these printks(), they do not really help as user can't
> > take any action anyway.
> 
> Aren't they helpful?
> "invalid security context" can pop up if someone supplies an invalid
> SELinux context (nft add secmark inet filter sshtag
> \"this_is_invalid\") and uses it

Can't that be caught at ->init() time?
We can then reject this via plain -EINVAL.

No need for printk because caller knows which expression/object caused
the error.

> "unable to convert security context" can pop up if no LSM is enabled

Can that be done at ->init() time when we can still reject the (invalid)
rule?

> "unable to map security context" should never happen, but one never knows

Ok, but what is user supposed to do?
This just causes perpetual spew of warnings in the kernel ring buffer.

> "unable to obtain relabeling permission" can pop up if e.g. the
> SELinux permission "kernel_t ssh_server_packet:packet relabelto" is
> missing

Makes sense, but this will need to be a plain
return -EPERM, this function can only be used in process context.