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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Florian,

thank you very much for your review.

On 12/18/2013 10:24 PM, Florian Westphal wrote:
valentina.giusti@xxxxxxxxxxxx <valentina.giusti@xxxxxxxxxxxx> wrote:
From: Valentina Giusti <valentina.giusti@xxxxxxxxxxxx>

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.

I was actually wondering if there could be a better solution instead of always sending UID/GID to userspace, thanks for your suggestion.

+			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.

Sorry, I totally overlooked this one.
I will send soon a new patchset that addresses all your comments.

--
BR,
Val


Cheers,
Florian

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Netfitler Users]     [LARTC]     [Bugtraq]     [Yosemite Forum]

  Powered by Linux