[PATCH v3 0/2] Add UID/GID info to NFQUEUE

[PATCH v3 0/2] Add UID/GID info to NFQUEUE

[valentina.giusti]

From: Valentina Giusti <valentina.giusti@bmw-carit.de>

Hi,
I have tested this patchset against the nf-next tree and run make C=2 to
make sure it doesn't add sparse errors or warnings.
Hopefully I did it right this time ;)

--
BR,
Val

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

[valentina.giusti]

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 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           | 34 ++++++++++++++++++++++++++
 2 files changed, 38 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..f11f974 100644
--- a/net/netfilter/nfnetlink_queue_core.c
+++ b/net/netfilter/nfnetlink_queue_core.c
@@ -297,6 +297,32 @@ 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_state == TCP_TIME_WAIT)
+		return -1;
+
+	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_be32(skb, NFQA_UID,
+		    htonl(from_kuid_munged(&init_user_ns, cred->fsuid))))
+			goto nla_put_failure;
+		if (nla_put_be32(skb, NFQA_GID,
+		    htonl(from_kgid_munged(&init_user_ns, 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 +398,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 +514,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;
 
-- 
1.8.5.1

[PATCH v3 2/2] libnetfilter_queue: add support for UID/GID socket info

[valentina.giusti]

From: Valentina Giusti <Valentina.Giusti@bmw-carit.de>

With this patch libnetfilter_queue is able to parse UID/GID
socket information.

Signed-off-by: Valentina Giusti <Valentina.Giusti@bmw-carit.de>
---
 include/libnetfilter_queue/libnetfilter_queue.h    |  4 ++++
 include/libnetfilter_queue/linux_nfnetlink_queue.h |  5 +++-
 include/linux/netfilter/nfnetlink_queue.h          |  6 ++++-
 src/libnetfilter_queue.c                           | 28 ++++++++++++++++++++++
 src/nlmsg.c                                        |  2 ++
 5 files changed, 43 insertions(+), 2 deletions(-)

diff --git a/include/libnetfilter_queue/libnetfilter_queue.h b/include/libnetfilter_queue/libnetfilter_queue.h
index b9f16e2..b4e2679 100644
--- a/include/libnetfilter_queue/libnetfilter_queue.h
+++ b/include/libnetfilter_queue/libnetfilter_queue.h
@@ -103,6 +103,8 @@ extern u_int32_t nfq_get_indev(struct nfq_data *nfad);
 extern u_int32_t nfq_get_physindev(struct nfq_data *nfad);
 extern u_int32_t nfq_get_outdev(struct nfq_data *nfad);
 extern u_int32_t nfq_get_physoutdev(struct nfq_data *nfad);
+extern u_int32_t nfq_get_uid(struct nfq_data *nfad);
+extern u_int32_t nfq_get_gid(struct nfq_data *nfad);
 
 extern int nfq_get_indev_name(struct nlif_handle *nlif_handle,
 			      struct nfq_data *nfad, char *name);
@@ -125,6 +127,8 @@ enum {
 	NFQ_XML_PHYSDEV	= (1 << 3),
 	NFQ_XML_PAYLOAD	= (1 << 4),
 	NFQ_XML_TIME	= (1 << 5),
+	NFQ_XML_UID	= (1 << 6),
+	NFQ_XML_GID	= (1 << 7),
 	NFQ_XML_ALL	= ~0U,
 };
 
diff --git a/include/libnetfilter_queue/linux_nfnetlink_queue.h b/include/libnetfilter_queue/linux_nfnetlink_queue.h
index 81a485b..884ab0e 100644
--- a/include/libnetfilter_queue/linux_nfnetlink_queue.h
+++ b/include/libnetfilter_queue/linux_nfnetlink_queue.h
@@ -50,6 +50,8 @@ enum nfqnl_attr_type {
 	NFQA_CAP_LEN,                   /* __u32 length of captured packet */
 	NFQA_SKB_INFO,                  /* __u32 skb meta information */
 
+	NFQA_UID,			/* __u32 sk uid */
+	NFQA_GID,			/* __u32 sk gid */
 	__NFQA_MAX
 };
 #define NFQA_MAX (__NFQA_MAX - 1)
@@ -101,7 +103,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/include/linux/netfilter/nfnetlink_queue.h b/include/linux/netfilter/nfnetlink_queue.h
index a2308ae..22f5d45 100644
--- a/include/linux/netfilter/nfnetlink_queue.h
+++ b/include/linux/netfilter/nfnetlink_queue.h
@@ -46,6 +46,9 @@ enum nfqnl_attr_type {
 	NFQA_CT_INFO,			/* enum ip_conntrack_info */
 	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
 };
@@ -98,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/src/libnetfilter_queue.c b/src/libnetfilter_queue.c
index fa8efe7..bf1266b 100644
--- a/src/libnetfilter_queue.c
+++ b/src/libnetfilter_queue.c
@@ -668,6 +668,9 @@ EXPORT_SYMBOL(nfq_set_mode);
  *  if this bit is set, the layer 3/4 checksums of the packet appear incorrect,
  *  but are not (because they will be corrected later by the kernel).
  *
+ *  - NFQA_CFG_F_UID_GID: the kernel will dump UID and GID of the socket to
+ *  which each packet belongs.
+ *
  * Here's a little code snippet to show how to use this API:
  * \verbatim
 	uint32_t flags = NFQA_CFG_F_FAIL_OPEN;
@@ -1180,6 +1183,18 @@ struct nfqnl_msg_packet_hw *nfq_get_packet_hw(struct nfq_data *nfad)
 }
 EXPORT_SYMBOL(nfq_get_packet_hw);
 
+uint32_t nfq_get_uid(struct nfq_data *nfad)
+{
+	return ntohl(nfnl_get_data(nfad->data, NFQA_UID, u_int32_t));
+}
+EXPORT_SYMBOL(nfq_get_uid);
+
+uint32_t nfq_get_gid(struct nfq_data *nfad)
+{
+	return ntohl(nfnl_get_data(nfad->data, NFQA_GID, u_int32_t));
+}
+EXPORT_SYMBOL(nfq_get_gid);
+
 /**
  * nfq_get_payload - get payload 
  * \param nfad Netlink packet data handle passed to callback function
@@ -1250,6 +1265,7 @@ int nfq_snprintf_xml(char *buf, size_t rem, struct nfq_data *tb, int flags)
 	struct nfqnl_msg_packet_hdr *ph;
 	struct nfqnl_msg_packet_hw *hwph;
 	u_int32_t mark, ifi;
+	u_int32_t uid, gid;
 	int size, offset = 0, len = 0, ret;
 	unsigned char *data;
 
@@ -1365,6 +1381,18 @@ int nfq_snprintf_xml(char *buf, size_t rem, struct nfq_data *tb, int flags)
 		SNPRINTF_FAILURE(size, rem, offset, len);
 	}
 
+	uid = nfq_get_uid(tb);
+	if (uid && (flags & NFQ_XML_UID)) {
+		size = snprintf(buf + offset, rem, "<uid>%u</uid>", uid);
+		SNPRINTF_FAILURE(size, rem, offset, len);
+	}
+
+	gid = nfq_get_gid(tb);
+	if (gid && (flags & NFQ_XML_GID)) {
+		size = snprintf(buf + offset, rem, "<gid>%u</gid>", gid);
+		SNPRINTF_FAILURE(size, rem, offset, len);
+	}
+
 	ret = nfq_get_payload(tb, &data);
 	if (ret >= 0 && (flags & NFQ_XML_PAYLOAD)) {
 		int i;
diff --git a/src/nlmsg.c b/src/nlmsg.c
index e7a30e0..81e170e 100644
--- a/src/nlmsg.c
+++ b/src/nlmsg.c
@@ -134,6 +134,8 @@ static int nfq_pkt_parse_attr_cb(const struct nlattr *attr, void *data)
 	case NFQA_IFINDEX_PHYSOUTDEV:
 	case NFQA_CAP_LEN:
 	case NFQA_SKB_INFO:
+	case NFQA_UID:
+	case NFQA_GID:
 		if (mnl_attr_validate(attr, MNL_TYPE_U32) < 0)
 			return MNL_CB_ERROR;
 		break;
-- 
1.8.5.1

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

[Florian Westphal]

valentina.giusti@bmw-carit.de <valentina.giusti@bmw-carit.de> wrote:
> --- a/net/netfilter/nfnetlink_queue_core.c
> +++ b/net/netfilter/nfnetlink_queue_core.c
> @@ -297,6 +297,32 @@ 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_state == TCP_TIME_WAIT)
> +		return -1;

I think this should be 'return 0'?

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

[Valentina Giusti]

On 12/20/2013 03:09 PM, Florian Westphal wrote:
> valentina.giusti@bmw-carit.de <valentina.giusti@bmw-carit.de> wrote:
>> --- a/net/netfilter/nfnetlink_queue_core.c
>> +++ b/net/netfilter/nfnetlink_queue_core.c
>> @@ -297,6 +297,32 @@ 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_state == TCP_TIME_WAIT)
>> +		return -1;
> I think this should be 'return 0'?
I put return -1 because I think that if userspace has requested to 
receive UID and GID, then it should be dumped only packets that have 
that information available.
Are you suggesting that it should be otherwise?

Thanks,
- Val

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

[Florian Westphal]

Valentina Giusti <valentina.giusti@bmw-carit.de> wrote:
> >I think this should be 'return 0'?
> I put return -1 because I think that if userspace has requested to
> receive UID and GID, then it should be dumped only packets that have
> that information available.
> Are you suggesting that it should be otherwise?

Yes, doing that doesn't make sense to me.
And it is inconsitent:
Packets without socket information are queued normally
in your patch, but suddently if its a timewait socket
its an error?

Why would we want timewait packets to NOT be queued?
vs. for example forwarded packets?

Userspace can test for presence of the attributes, i.e.
no NFQA_UID attribute -> no socket present, or lack of uid
information.

If you have a counter-example?

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

[Valentina Giusti]

On 12/20/2013 04:03 PM, Florian Westphal wrote:
> Valentina Giusti <valentina.giusti@bmw-carit.de> wrote:
>>> I think this should be 'return 0'?
>> I put return -1 because I think that if userspace has requested to
>> receive UID and GID, then it should be dumped only packets that have
>> that information available.
>> Are you suggesting that it should be otherwise?
>
> Yes, doing that doesn't make sense to me.
> And it is inconsitent:
> Packets without socket information are queued normally
> in your patch, but suddently if its a timewait socket
> its an error?
>
> Why would we want timewait packets to NOT be queued?
> vs. for example forwarded packets?
>
> Userspace can test for presence of the attributes, i.e.
> no NFQA_UID attribute -> no socket present, or lack of uid
> information.
>
> If you have a counter-example?
>

Sorry no, I think you are right, thanks for the explanation. Should I 
resend the patch or is it a change small enough that can be done when 
applied? Unless there are still other comments, ofc.

Thanks,
- Val