netdev - Re: [PATCH 1/2] netfilter_queue: enable UID/GID socket info retrieval

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: <20131218212430.GD15863@breakpoint.cc>
Date:	Wed, 18 Dec 2013 22:24:30 +0100
From:	Florian Westphal <fw@...len.de>
To:	valentina.giusti@...-carit.de
Cc:	netfilter-devel@...r.kernel.org, netdev@...r.kernel.org,
	eric.dumazet@...il.com, tgraf@...hat.com, jpa@...gle.com,
	pablo@...filter.org, davem@...emloft.net,
	daniel.wagner@...-carit.de
Subject: Re: [PATCH 1/2] netfilter_queue: enable UID/GID socket info retrieval

valentina.giusti@...-carit.de <valentina.giusti@...-carit.de> wrote:
> From: Valentina Giusti <valentina.giusti@...-carit.de>
> 
> Thanks to commits 41063e9 (ipv4: Early TCP socket demux) and 421b388 (udp: 
> ipv4: Add udp early demux) it is now possible to get UID and GID socket info
> also for incoming TCP and UDP connections. Having this info available, it
> is convenient to let NFQUEUE retrieve it in order to improve and refine the
> traffic analysis in userspace.

No objections from me, except a few comments below.

> diff --git a/net/netfilter/nfnetlink_queue_core.c b/net/netfilter/nfnetlink_queue_core.c
> index 21258cf..7257ddb 100644
> --- a/net/netfilter/nfnetlink_queue_core.c
> +++ b/net/netfilter/nfnetlink_queue_core.c
> @@ -328,7 +328,9 @@ nfqnl_build_packet_message(struct net *net, struct nfqnl_instance *queue,
[..]
>  	if (entskb->tstamp.tv64)
>  		size += nla_total_size(sizeof(struct nfqnl_msg_packet_timestamp));
> @@ -484,6 +486,25 @@ nfqnl_build_packet_message(struct net *net, struct nfqnl_instance *queue,
>  			goto nla_put_failure;
>  	}
>  
> +	if (entskb->sk) {
> +		struct sock *sk = entskb->sk;
> +		if (sk && sk->sk_state != TCP_TIME_WAIT) {
> +			read_lock_bh(&sk->sk_callback_lock);

nfqnl_build_packet_message is already a huge function, I think it
might be useful to add a small helper function for this,
e.g.  'nfqnl_skinfo_put(skb, entskb->sk)' or something like that.

Also, it might be a good idea to only dump the sk info
on userspace request (not sure how expensive the readlock is
compared to the nfqueue cost...), see NFQA_CFG_F_CONNTRACK flag for
instance.

> +			if (sk->sk_socket && sk->sk_socket->file) {
> +				struct file *file = sk->sk_socket->file;
> +				const struct cred *cred = file->f_cred;
> +				if (nla_put_u32(skb, NFQA_UID,

nla_put_be32

> +				    htonl(cred->fsuid)))
> +					goto nla_put_failure;

careful - the sk_callback_lock was taken.

Cheers,
Florian
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html