| Message ID | 1387389844-5263-2-git-send-email-valentina.giusti@bmw-carit.de |
|---|---|
| State | Changes Requested |
| Headers | show |
valentina.giusti@bmw-carit.de <valentina.giusti@bmw-carit.de> wrote: > From: Valentina Giusti <valentina.giusti@bmw-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 netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Florian, thank you very much for your review. On 12/18/2013 10:24 PM, Florian Westphal wrote: > valentina.giusti@bmw-carit.de <valentina.giusti@bmw-carit.de> wrote: >> From: Valentina Giusti <valentina.giusti@bmw-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. 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/include/uapi/linux/netfilter/nfnetlink_queue.h b/include/uapi/linux/netfilter/nfnetlink_queue.h index 0132bad..46ebbc4 100644 --- a/include/uapi/linux/netfilter/nfnetlink_queue.h +++ b/include/uapi/linux/netfilter/nfnetlink_queue.h @@ -47,6 +47,8 @@ enum nfqnl_attr_type { NFQA_CAP_LEN, /* __u32 length of captured packet */ NFQA_SKB_INFO, /* __u32 skb meta information */ NFQA_EXP, /* nf_conntrack_netlink.h */ + NFQA_UID, /* __u32 sk uid */ + NFQA_GID, /* __u32 sk gid */ __NFQA_MAX }; 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, + nla_total_size(sizeof(u_int32_t)) /* mark */ + nla_total_size(sizeof(struct nfqnl_msg_packet_hw)) + nla_total_size(sizeof(u_int32_t)) /* skbinfo */ - + nla_total_size(sizeof(u_int32_t)); /* cap_len */ + + nla_total_size(sizeof(u_int32_t)) /* cap_len */ + + nla_total_size(sizeof(u_int32_t)) /* uid */ + + nla_total_size(sizeof(u_int32_t)); /* gid */ 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); + 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, + htonl(cred->fsuid))) + goto nla_put_failure; + if (nla_put_u32(skb, NFQA_GID, + htonl(cred->fsgid))) + goto nla_put_failure; + read_unlock_bh(&sk->sk_callback_lock); + } else + read_unlock_bh(&sk->sk_callback_lock); + } + } + if (ct && nfqnl_ct_put(skb, ct, ctinfo) < 0) goto nla_put_failure;