netdev - Re: [PATCH netfilter] netfilter: xt_owner: use sk->sk

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 for Android: free password hash cracker in your pocket

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

Message-ID: <1nrqq669-2r5o-qq5o-207r-p6pnr614s769@vanv.qr>
Date:   Thu, 23 Dec 2021 11:35:08 +0100 (CET)
From:   Jan Engelhardt <jengelh@...i.de>
To:     Maciej Żenczykowski <zenczykowski@...il.com>
cc:     Maciej Żenczykowski <maze@...gle.com>,
        Pablo Neira Ayuso <pablo@...filter.org>,
        Florian Westphal <fw@...len.de>,
        Linux Network Development Mailing List 
        <netdev@...r.kernel.org>,
        Netfilter Development Mailing List 
        <netfilter-devel@...r.kernel.org>,
        Lorenzo Colitti <lorenzo@...gle.com>
Subject: Re: [PATCH netfilter] netfilter: xt_owner: use sk->sk_uid for owner
 lookup


On Thursday 2021-12-23 08:06, Maciej Żenczykowski wrote:

>diff --git a/net/netfilter/xt_owner.c b/net/netfilter/xt_owner.c
>index e85ce69924ae..3eebd9c7ea4b 100644
>--- a/net/netfilter/xt_owner.c
>+++ b/net/netfilter/xt_owner.c
>@@ -84,8 +84,8 @@ owner_mt(const struct sk_buff *skb, struct xt_action_param *par)
> 	if (info->match & XT_OWNER_UID) {
> 		kuid_t uid_min = make_kuid(net->user_ns, info->uid_min);
> 		kuid_t uid_max = make_kuid(net->user_ns, info->uid_max);
>-		if ((uid_gte(filp->f_cred->fsuid, uid_min) &&
>-		     uid_lte(filp->f_cred->fsuid, uid_max)) ^
>+		if ((uid_gte(sk->sk_uid, uid_min) &&
>+		     uid_lte(sk->sk_uid, uid_max)) ^

I have a "déjà rencontré" moment about these lines...

filp->f_cred->fsuid should be the EUID which performed the access (after
peeling away the setfsuid(2) logic...), and sk_uid has a value that the
original author of ipt_owner did not find useful. I think that was the
motivation. listen(80) then drop privileges by set(e)uid. sk_uid would be 0,
and thus not useful.