Re: [PATCH] security: use secure_getenv() to prevent env-var privilege escalation

[Florian Westphal <fw@strlen.de>]

Alan Ross <alan@sleuthco.ai> wrote:
> >> Would you have the cycles to go through all of nf software to make this
> change?
> 
>   Yes, happy to take this on. I'll work through them in order: nftables,
> ipset, conntrack-tools, ulogd. iproute2 I can
>   look at as well, though that's a separate tree/maintainer so I'd send
> those separately.

Sure, iproute2 patches go to netdev@ , not to netfilter-devel.

>  >> I think it just needs a rework of the commit message
> 
>   Will do — v2 will lead with the setcap/container-runtime scenario as the
> motivation.

Thanks!

>  >> Any reason for the wrapper to not do getauxval(AT_SECURE)?
> 
>   No good reason. getauxval is available since glibc 2.16 (one release
> before secure_getenv in 2.17), and since this is
>   all Linux-only code there's no portability concern. Your version is
> cleaner — I'll use that for the fallback.

Alright.

>   In practice the #ifdef HAVE_SECURE_GETENV path will hit on anything
> remotely modern, but agreed the getauxval fallback
>    is simpler than a uid/euid comparison.

Agreed, it should not be hit in practice.

>  >> Another option is to alter ef7781eb1437a ("libxtables: exit if called
> by setuid executable") to enforce non-capability
>    binary
> 
>   That would work as a first step for xtables specifically — extend the
> existing getuid() != geteuid() check to also
>   bail on getauxval(AT_SECURE). The secure_getenv changes would then be
> belt-and-suspenders on top. Want me to include
>   both in v2, or would you prefer the enforcement-first approach across the
> nf suite?

I would prefer enforcement-first, but you are free to followup with
getenv_secure if you want.  I mean, the change isn't wrong, I am just
not sure there isn't anything else that we might be missing.

xtables has a plugin architecture, so we don't know what other
extensions that might get shipped by some distros do.

For nftables, thats less of a concern, BUT I don't know what some of the
libraries we link against do with untrusted input.  Or what will happen
when nft is invoked with stdin/stdout/stderr closed.

So I would prefer to enforce a no-setcap/setuid approach in any case
and followup with secure_getenv later.

> I'll start with the iptables v2 (reworked message + getauxval fallback)
> and then work through nftables and the others
> as follow-up series.

Thank you, much appreciated.