| Message ID | 1387451007-14363-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@oss.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 parse UID and GID socket info > also for incoming TCP and UDP connections. Having this info available, it > is convenient to let NFQUEUE parse it in order to improve and refine the > traffic analysis in userspace. > diff --git a/net/netfilter/nfnetlink_queue_core.c b/net/netfilter/nfnetlink_queue_core.c > index 21258cf..2cdef07 100644 > --- a/net/netfilter/nfnetlink_queue_core.c > +++ b/net/netfilter/nfnetlink_queue_core.c > @@ -297,6 +297,29 @@ nfqnl_put_packet_info(struct sk_buff *nlskb, struct sk_buff *packet, > return flags ? nla_put_be32(nlskb, NFQA_SKB_INFO, htonl(flags)) : 0; > } > > +static int nfqnl_put_sk_uidgid(struct sk_buff *skb, struct sock *sk) > +{ > + const struct cred *cred; > + > + if (sk && sk->sk_state != TCP_TIME_WAIT) { [..] > + if ((queue->flags & NFQA_CFG_F_UID_GID) && entskb->sk) > + if (nfqnl_put_sk_uidgid(skb, entskb->sk)) > + goto nla_put_failure; Minor nit: sk is tested for non-null twice. Suggestion: if ((queue->flags & NFQA_CFG_F_UID_GID) && nfqnl_put_sk_uidgid(skb, entskb->sk)) goto nla_put_failure; I don't think its necessary to resubmit the patch though, thus: Reviewed-by: Florian Westphal <fw@strlen.de> -- 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, On Thu, Dec 19, 2013 at 12:03:26PM +0100, valentina.giusti@bmw-carit.de wrote: > From: Valentina Giusti <valentina.giusti@oss.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 parse UID and GID socket info > also for incoming TCP and UDP connections. Having this info available, it > is convenient to let NFQUEUE parse it in order to improve and refine the > traffic analysis in userspace. > > Signed-off-by: Valentina Giusti <valentina.giusti@bmw-carit.de> > --- > include/uapi/linux/netfilter/nfnetlink_queue.h | 5 ++++- > net/netfilter/nfnetlink_queue_core.c | 31 ++++++++++++++++++++++++++ > 2 files changed, 35 insertions(+), 1 deletion(-) > > diff --git a/include/uapi/linux/netfilter/nfnetlink_queue.h b/include/uapi/linux/netfilter/nfnetlink_queue.h > index 0132bad..8dd819e 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 > }; > @@ -99,7 +101,8 @@ enum nfqnl_attr_config { > #define NFQA_CFG_F_FAIL_OPEN (1 << 0) > #define NFQA_CFG_F_CONNTRACK (1 << 1) > #define NFQA_CFG_F_GSO (1 << 2) > -#define NFQA_CFG_F_MAX (1 << 3) > +#define NFQA_CFG_F_UID_GID (1 << 3) > +#define NFQA_CFG_F_MAX (1 << 4) > > /* flags for NFQA_SKB_INFO */ > /* packet appears to have wrong checksums, but they are ok */ > diff --git a/net/netfilter/nfnetlink_queue_core.c b/net/netfilter/nfnetlink_queue_core.c > index 21258cf..2cdef07 100644 > --- a/net/netfilter/nfnetlink_queue_core.c > +++ b/net/netfilter/nfnetlink_queue_core.c > @@ -297,6 +297,29 @@ nfqnl_put_packet_info(struct sk_buff *nlskb, struct sk_buff *packet, > return flags ? nla_put_be32(nlskb, NFQA_SKB_INFO, htonl(flags)) : 0; > } > > +static int nfqnl_put_sk_uidgid(struct sk_buff *skb, struct sock *sk) > +{ > + const struct cred *cred; > + > + if (sk && sk->sk_state != TCP_TIME_WAIT) { > + read_lock_bh(&sk->sk_callback_lock); > + if (sk->sk_socket && sk->sk_socket->file) { > + cred = sk->sk_socket->file->f_cred; > + if (nla_put_u32(skb, NFQA_UID, htonl(cred->fsuid))) I'm hitting this compilation error here: net/netfilter/nfnetlink_queue_core.c: In function ‘nfqnl_put_sk_uidgid’: net/netfilter/nfnetlink_queue_core.c:308:4: error: aggregate value used where an integer was expected It seems that in recent kernels you have to use: from_kuid_munged(&init_user_ns, cred->fsuid); to retrieve the integer that represents the uid. Please, > + goto nla_put_failure; > + if (nla_put_u32(skb, NFQA_GID, htonl(cred->fsgid))) ^^^ Minor nitpick: This has to be _be32 otherwise sparse will complain (spotted by Florian Westphal). Make sure you patch is also clean when running make C=2. Also remove the extra checking for socket that Florian mentioned and resubmit. Thanks. -- 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
On Fri, Dec 20, 2013 at 10:26:38AM +0100, Pablo Neira Ayuso wrote: > Hi, > > On Thu, Dec 19, 2013 at 12:03:26PM +0100, valentina.giusti@bmw-carit.de wrote: > > From: Valentina Giusti <valentina.giusti@oss.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 parse UID and GID socket info > > also for incoming TCP and UDP connections. Having this info available, it > > is convenient to let NFQUEUE parse it in order to improve and refine the > > traffic analysis in userspace. > > > > Signed-off-by: Valentina Giusti <valentina.giusti@bmw-carit.de> > > --- > > include/uapi/linux/netfilter/nfnetlink_queue.h | 5 ++++- > > net/netfilter/nfnetlink_queue_core.c | 31 ++++++++++++++++++++++++++ > > 2 files changed, 35 insertions(+), 1 deletion(-) > > > > diff --git a/include/uapi/linux/netfilter/nfnetlink_queue.h b/include/uapi/linux/netfilter/nfnetlink_queue.h > > index 0132bad..8dd819e 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 > > }; > > @@ -99,7 +101,8 @@ enum nfqnl_attr_config { > > #define NFQA_CFG_F_FAIL_OPEN (1 << 0) > > #define NFQA_CFG_F_CONNTRACK (1 << 1) > > #define NFQA_CFG_F_GSO (1 << 2) > > -#define NFQA_CFG_F_MAX (1 << 3) > > +#define NFQA_CFG_F_UID_GID (1 << 3) > > +#define NFQA_CFG_F_MAX (1 << 4) > > > > /* flags for NFQA_SKB_INFO */ > > /* packet appears to have wrong checksums, but they are ok */ > > diff --git a/net/netfilter/nfnetlink_queue_core.c b/net/netfilter/nfnetlink_queue_core.c > > index 21258cf..2cdef07 100644 > > --- a/net/netfilter/nfnetlink_queue_core.c > > +++ b/net/netfilter/nfnetlink_queue_core.c > > @@ -297,6 +297,29 @@ nfqnl_put_packet_info(struct sk_buff *nlskb, struct sk_buff *packet, > > return flags ? nla_put_be32(nlskb, NFQA_SKB_INFO, htonl(flags)) : 0; > > } > > > > +static int nfqnl_put_sk_uidgid(struct sk_buff *skb, struct sock *sk) > > +{ > > + const struct cred *cred; > > + > > + if (sk && sk->sk_state != TCP_TIME_WAIT) { > > + read_lock_bh(&sk->sk_callback_lock); > > + if (sk->sk_socket && sk->sk_socket->file) { > > + cred = sk->sk_socket->file->f_cred; > > + if (nla_put_u32(skb, NFQA_UID, htonl(cred->fsuid))) > > I'm hitting this compilation error here: > > net/netfilter/nfnetlink_queue_core.c: In function ‘nfqnl_put_sk_uidgid’: > net/netfilter/nfnetlink_queue_core.c:308:4: error: aggregate value used where an integer was expected > > It seems that in recent kernels you have to use: > > from_kuid_munged(&init_user_ns, cred->fsuid); > > to retrieve the integer that represents the uid. > > Please, sorry, I left this sentence incomplete, I was telling... Please, test your patches with current git trees :-). Thanks. -- 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, On 12/20/2013 10:29 AM, Pablo Neira Ayuso wrote: > On Fri, Dec 20, 2013 at 10:26:38AM +0100, Pablo Neira Ayuso wrote: >> Hi, >> >> On Thu, Dec 19, 2013 at 12:03:26PM +0100, valentina.giusti@bmw-carit.de wrote: >>> From: Valentina Giusti <valentina.giusti@oss.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 parse UID and GID socket info >>> also for incoming TCP and UDP connections. Having this info available, it >>> is convenient to let NFQUEUE parse it in order to improve and refine the >>> traffic analysis in userspace. >>> >>> Signed-off-by: Valentina Giusti <valentina.giusti@bmw-carit.de> >>> --- >>> include/uapi/linux/netfilter/nfnetlink_queue.h | 5 ++++- >>> net/netfilter/nfnetlink_queue_core.c | 31 ++++++++++++++++++++++++++ >>> 2 files changed, 35 insertions(+), 1 deletion(-) >>> >>> diff --git a/include/uapi/linux/netfilter/nfnetlink_queue.h b/include/uapi/linux/netfilter/nfnetlink_queue.h >>> index 0132bad..8dd819e 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 >>> }; >>> @@ -99,7 +101,8 @@ enum nfqnl_attr_config { >>> #define NFQA_CFG_F_FAIL_OPEN (1 << 0) >>> #define NFQA_CFG_F_CONNTRACK (1 << 1) >>> #define NFQA_CFG_F_GSO (1 << 2) >>> -#define NFQA_CFG_F_MAX (1 << 3) >>> +#define NFQA_CFG_F_UID_GID (1 << 3) >>> +#define NFQA_CFG_F_MAX (1 << 4) >>> >>> /* flags for NFQA_SKB_INFO */ >>> /* packet appears to have wrong checksums, but they are ok */ >>> diff --git a/net/netfilter/nfnetlink_queue_core.c b/net/netfilter/nfnetlink_queue_core.c >>> index 21258cf..2cdef07 100644 >>> --- a/net/netfilter/nfnetlink_queue_core.c >>> +++ b/net/netfilter/nfnetlink_queue_core.c >>> @@ -297,6 +297,29 @@ nfqnl_put_packet_info(struct sk_buff *nlskb, struct sk_buff *packet, >>> return flags ? nla_put_be32(nlskb, NFQA_SKB_INFO, htonl(flags)) : 0; >>> } >>> >>> +static int nfqnl_put_sk_uidgid(struct sk_buff *skb, struct sock *sk) >>> +{ >>> + const struct cred *cred; >>> + >>> + if (sk && sk->sk_state != TCP_TIME_WAIT) { >>> + read_lock_bh(&sk->sk_callback_lock); >>> + if (sk->sk_socket && sk->sk_socket->file) { >>> + cred = sk->sk_socket->file->f_cred; >>> + if (nla_put_u32(skb, NFQA_UID, htonl(cred->fsuid))) >> I'm hitting this compilation error here: >> >> net/netfilter/nfnetlink_queue_core.c: In function ‘nfqnl_put_sk_uidgid’: >> net/netfilter/nfnetlink_queue_core.c:308:4: error: aggregate value used where an integer was expected >> >> It seems that in recent kernels you have to use: >> >> from_kuid_munged(&init_user_ns, cred->fsuid); >> >> to retrieve the integer that represents the uid. >> >> Please, > sorry, I left this sentence incomplete, I was telling... Please, test > your patches with current git trees :-). Thanks. Thank you for reviewing the patches, I'll send a new version soon :) -- Cheers, Val > -- > 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 -- 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, On 12/20/2013 10:29 AM, Pablo Neira Ayuso wrote: > On Fri, Dec 20, 2013 at 10:26:38AM +0100, Pablo Neira Ayuso wrote: >> Hi, >> >> On Thu, Dec 19, 2013 at 12:03:26PM +0100, valentina.giusti@bmw-carit.de wrote: >>> From: Valentina Giusti <valentina.giusti@oss.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 parse UID and GID socket info >>> also for incoming TCP and UDP connections. Having this info available, it >>> is convenient to let NFQUEUE parse it in order to improve and refine the >>> traffic analysis in userspace. >>> >>> Signed-off-by: Valentina Giusti <valentina.giusti@bmw-carit.de> >>> --- >>> include/uapi/linux/netfilter/nfnetlink_queue.h | 5 ++++- >>> net/netfilter/nfnetlink_queue_core.c | 31 ++++++++++++++++++++++++++ >>> 2 files changed, 35 insertions(+), 1 deletion(-) >>> >>> diff --git a/include/uapi/linux/netfilter/nfnetlink_queue.h b/include/uapi/linux/netfilter/nfnetlink_queue.h >>> index 0132bad..8dd819e 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 >>> }; >>> @@ -99,7 +101,8 @@ enum nfqnl_attr_config { >>> #define NFQA_CFG_F_FAIL_OPEN (1 << 0) >>> #define NFQA_CFG_F_CONNTRACK (1 << 1) >>> #define NFQA_CFG_F_GSO (1 << 2) >>> -#define NFQA_CFG_F_MAX (1 << 3) >>> +#define NFQA_CFG_F_UID_GID (1 << 3) >>> +#define NFQA_CFG_F_MAX (1 << 4) >>> >>> /* flags for NFQA_SKB_INFO */ >>> /* packet appears to have wrong checksums, but they are ok */ >>> diff --git a/net/netfilter/nfnetlink_queue_core.c b/net/netfilter/nfnetlink_queue_core.c >>> index 21258cf..2cdef07 100644 >>> --- a/net/netfilter/nfnetlink_queue_core.c >>> +++ b/net/netfilter/nfnetlink_queue_core.c >>> @@ -297,6 +297,29 @@ nfqnl_put_packet_info(struct sk_buff *nlskb, struct sk_buff *packet, >>> return flags ? nla_put_be32(nlskb, NFQA_SKB_INFO, htonl(flags)) : 0; >>> } >>> >>> +static int nfqnl_put_sk_uidgid(struct sk_buff *skb, struct sock *sk) >>> +{ >>> + const struct cred *cred; >>> + >>> + if (sk && sk->sk_state != TCP_TIME_WAIT) { >>> + read_lock_bh(&sk->sk_callback_lock); >>> + if (sk->sk_socket && sk->sk_socket->file) { >>> + cred = sk->sk_socket->file->f_cred; >>> + if (nla_put_u32(skb, NFQA_UID, htonl(cred->fsuid))) >> I'm hitting this compilation error here: >> >> net/netfilter/nfnetlink_queue_core.c: In function ‘nfqnl_put_sk_uidgid’: >> net/netfilter/nfnetlink_queue_core.c:308:4: error: aggregate value used where an integer was expected >> >> It seems that in recent kernels you have to use: >> >> from_kuid_munged(&init_user_ns, cred->fsuid); >> >> to retrieve the integer that represents the uid. >> >> Please, > sorry, I left this sentence incomplete, I was telling... Please, test > your patches with current git trees :-). Thanks. Just to be on the safe side, I am testing my patches against the nf-next tree, branch master. Is that correct? Thanks, Val -- 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
Valentina Giusti <valentina.giusti@bmw-carit.de> wrote: > Just to be on the safe side, I am testing my patches against the > nf-next tree, branch master. Is that correct? Yes, thats correct. -- 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..8dd819e 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 }; @@ -99,7 +101,8 @@ enum nfqnl_attr_config { #define NFQA_CFG_F_FAIL_OPEN (1 << 0) #define NFQA_CFG_F_CONNTRACK (1 << 1) #define NFQA_CFG_F_GSO (1 << 2) -#define NFQA_CFG_F_MAX (1 << 3) +#define NFQA_CFG_F_UID_GID (1 << 3) +#define NFQA_CFG_F_MAX (1 << 4) /* flags for NFQA_SKB_INFO */ /* packet appears to have wrong checksums, but they are ok */ diff --git a/net/netfilter/nfnetlink_queue_core.c b/net/netfilter/nfnetlink_queue_core.c index 21258cf..2cdef07 100644 --- a/net/netfilter/nfnetlink_queue_core.c +++ b/net/netfilter/nfnetlink_queue_core.c @@ -297,6 +297,29 @@ nfqnl_put_packet_info(struct sk_buff *nlskb, struct sk_buff *packet, return flags ? nla_put_be32(nlskb, NFQA_SKB_INFO, htonl(flags)) : 0; } +static int nfqnl_put_sk_uidgid(struct sk_buff *skb, struct sock *sk) +{ + const struct cred *cred; + + if (sk && sk->sk_state != TCP_TIME_WAIT) { + read_lock_bh(&sk->sk_callback_lock); + if (sk->sk_socket && sk->sk_socket->file) { + cred = sk->sk_socket->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); + } + + return 0; + +nla_put_failure: + read_unlock_bh(&sk->sk_callback_lock); + return -1; +} + static struct sk_buff * nfqnl_build_packet_message(struct net *net, struct nfqnl_instance *queue, struct nf_queue_entry *entry, @@ -372,6 +395,10 @@ nfqnl_build_packet_message(struct net *net, struct nfqnl_instance *queue, if (queue->flags & NFQA_CFG_F_CONNTRACK) ct = nfqnl_ct_get(entskb, &size, &ctinfo); + if (queue->flags & NFQA_CFG_F_UID_GID) + size += (nla_total_size(sizeof(u_int32_t)) /* uid */ + + nla_total_size(sizeof(u_int32_t))); /* gid */ + skb = nfnetlink_alloc_skb(net, size, queue->peer_portid, GFP_ATOMIC); if (!skb) @@ -484,6 +511,10 @@ nfqnl_build_packet_message(struct net *net, struct nfqnl_instance *queue, goto nla_put_failure; } + if ((queue->flags & NFQA_CFG_F_UID_GID) && entskb->sk) + if (nfqnl_put_sk_uidgid(skb, entskb->sk)) + goto nla_put_failure; + if (ct && nfqnl_ct_put(skb, ct, ctinfo) < 0) goto nla_put_failure;