[PATCH libnetfilter_queue 0/1] libnfnetlink dependency elimination

[PATCH libnetfilter_queue 0/1] libnfnetlink dependency elimination

[Duncan Roe]

Hi Pablo,

We need this patch for mnl cut-over of nfnl API.
It's just an added function, should be fine to apply straight away(?)

Cheers ... Duncan.

Duncan Roe (1):
  src: Add nfq_nlmsg_put2() - header flags include NLM_F_ACK

 .../libnetfilter_queue/libnetfilter_queue.h   |  1 +
 src/nlmsg.c                                   | 72 ++++++++++++++++---
 2 files changed, 65 insertions(+), 8 deletions(-)

-- 
2.35.8

[PATCH libnetfilter_queue 1/1] src: Add nfq_nlmsg_put2() - header flags include NLM_F_ACK

[Duncan Roe]

Enable mnl programs to check whether a config request was accepted.
(nfnl programs do this already).

Signed-off-by: Duncan Roe <duncan_roe@optusnet.com.au>
---
 .../libnetfilter_queue/libnetfilter_queue.h   |  1 +
 src/nlmsg.c                                   | 72 ++++++++++++++++---
 2 files changed, 65 insertions(+), 8 deletions(-)

diff --git a/include/libnetfilter_queue/libnetfilter_queue.h b/include/libnetfilter_queue/libnetfilter_queue.h
index 3d8e444..084a2ea 100644
--- a/include/libnetfilter_queue/libnetfilter_queue.h
+++ b/include/libnetfilter_queue/libnetfilter_queue.h
@@ -151,6 +151,7 @@ void nfq_nlmsg_verdict_put_pkt(struct nlmsghdr *nlh, const void *pkt, uint32_t p
 
 int nfq_nlmsg_parse(const struct nlmsghdr *nlh, struct nlattr **attr);
 struct nlmsghdr *nfq_nlmsg_put(char *buf, int type, uint32_t queue_num);
+struct nlmsghdr *nfq_nlmsg_put2(char *buf, int type, uint32_t queue_num);
 
 #ifdef __cplusplus
 } /* extern "C" */
diff --git a/src/nlmsg.c b/src/nlmsg.c
index 5400dd7..ba53df2 100644
--- a/src/nlmsg.c
+++ b/src/nlmsg.c
@@ -300,6 +300,21 @@ int nfq_nlmsg_parse(const struct nlmsghdr *nlh, struct nlattr **attr)
 			      nfq_pkt_parse_attr_cb, attr);
 }
 
+static struct nlmsghdr *__nfq_nlmsg_put(char *buf, int type, uint32_t queue_num,
+					uint16_t flags)
+{
+	struct nlmsghdr *nlh = mnl_nlmsg_put_header(buf);
+	nlh->nlmsg_type = (NFNL_SUBSYS_QUEUE << 8) | type;
+	nlh->nlmsg_flags = flags;
+
+	struct nfgenmsg *nfg = mnl_nlmsg_put_extra_header(nlh, sizeof(*nfg));
+	nfg->nfgen_family = AF_UNSPEC;
+	nfg->version = NFNETLINK_V0;
+	nfg->res_id = htons(queue_num);
+
+	return nlh;
+}
+
 /**
  * nfq_nlmsg_put - Convert memory buffer into a Netlink buffer
  * \param *buf Pointer to memory buffer
@@ -310,16 +325,57 @@ int nfq_nlmsg_parse(const struct nlmsghdr *nlh, struct nlattr **attr)
 EXPORT_SYMBOL
 struct nlmsghdr *nfq_nlmsg_put(char *buf, int type, uint32_t queue_num)
 {
-	struct nlmsghdr *nlh = mnl_nlmsg_put_header(buf);
-	nlh->nlmsg_type = (NFNL_SUBSYS_QUEUE << 8) | type;
-	nlh->nlmsg_flags = NLM_F_REQUEST;
+	return __nfq_nlmsg_put(buf, type, queue_num, NLM_F_REQUEST);
+}
 
-	struct nfgenmsg *nfg = mnl_nlmsg_put_extra_header(nlh, sizeof(*nfg));
-	nfg->nfgen_family = AF_UNSPEC;
-	nfg->version = NFNETLINK_V0;
-	nfg->res_id = htons(queue_num);
+/**
+ * nfq_nlmsg_put2 - Convert memory buffer into a Netlink buffer with NLM_F_ACK
+ * flag present
+ * \param *buf Pointer to memory buffer
+ * \param type Either NFQNL_MSG_CONFIG or NFQNL_MSG_VERDICT
+ * \param queue_num Queue number
+ * \returns Pointer to netlink message
+ *
+ * Use this function before performing an action that might fail, e.g.
+ * attempt to configure NFQA_CFG_F_SECCTX on a system not runnine SELinux.
+ * \n
+ * NLM_F_ACK instructs the kernel to send a message in response
+ * to a successful command.
+ * The kernel always sends a message in response to a failed command.
+ * \n
+ * This code snippet demonstrates reading these responses:
+ * \verbatim
+	nlh = nfq_nlmsg_put2(nltxbuf, NFQNL_MSG_CONFIG, queue_num);
+	mnl_attr_put_u32(nlh, NFQA_CFG_FLAGS, NFQA_CFG_F_SECCTX);
+	mnl_attr_put_u32(nlh, NFQA_CFG_MASK, NFQA_CFG_F_SECCTX);
 
-	return nlh;
+	if (mnl_socket_sendto(nl, nlh, nlh->nlmsg_len) < 0) {
+		perror("mnl_socket_send");
+		exit(EXIT_FAILURE);
+	}
+
+	ret = mnl_socket_recvfrom(nl, nlrxbuf, sizeof nlrxbuf);
+	if (ret == -1) {
+		perror("mnl_socket_recvfrom");
+		exit(EXIT_FAILURE);
+	}
+
+	ret = mnl_cb_run(nlrxbuf, ret, 0, portid, NULL, NULL);
+	if (ret == -1)
+		perror("configure NFQA_CFG_F_SECCTX");
+\endverbatim
+ *
+ * \note
+ * The program above can continue after the error because NFQA_CFG_F_SECCTX
+ * was the only item in the preceding **mnl_socket_sendto**.
+ * If there had been other items, they would not have been actioned and it would
+ * not now be safe to proceed.
+ */
+
+EXPORT_SYMBOL
+struct nlmsghdr *nfq_nlmsg_put2(char *buf, int type, uint32_t queue_num)
+{
+	return __nfq_nlmsg_put(buf, type, queue_num, NLM_F_REQUEST|NLM_F_ACK);
 }
 
 /**
-- 
2.35.8

Re: [PATCH libnetfilter_queue 1/1] src: Add nfq_nlmsg_put2() - header flags include NLM_F_ACK

[Pablo Neira Ayuso]

On Mon, Nov 13, 2023 at 09:12:35AM +1100, Duncan Roe wrote:
> Enable mnl programs to check whether a config request was accepted.
> (nfnl programs do this already).
> 
> Signed-off-by: Duncan Roe <duncan_roe@optusnet.com.au>
> ---
>  .../libnetfilter_queue/libnetfilter_queue.h   |  1 +
>  src/nlmsg.c                                   | 72 ++++++++++++++++---
>  2 files changed, 65 insertions(+), 8 deletions(-)
> 
> diff --git a/include/libnetfilter_queue/libnetfilter_queue.h b/include/libnetfilter_queue/libnetfilter_queue.h
> index 3d8e444..084a2ea 100644
> --- a/include/libnetfilter_queue/libnetfilter_queue.h
> +++ b/include/libnetfilter_queue/libnetfilter_queue.h
> @@ -151,6 +151,7 @@ void nfq_nlmsg_verdict_put_pkt(struct nlmsghdr *nlh, const void *pkt, uint32_t p
>  
>  int nfq_nlmsg_parse(const struct nlmsghdr *nlh, struct nlattr **attr);
>  struct nlmsghdr *nfq_nlmsg_put(char *buf, int type, uint32_t queue_num);
> +struct nlmsghdr *nfq_nlmsg_put2(char *buf, int type, uint32_t queue_num);

I like this, but I'd suggest instead:

  struct nlmsghdr *nfq_nlmsg_put2(char *buf, int type, uint32_t queue_num, uint16_flags);

I should have expose those netlink flags in first place.

There are more useful netlink flags, so just expose them all.

Please send a v2.

Thanks.

[PATCH libnetfilter_queue v2 1/1] src: Add nfq_nlmsg_put2() - user specifies header flags

[Duncan Roe]

Enable mnl programs to check whether a config request was accepted.
(nfnl programs do this already).

v2: take flags as an arg (Pablo request)
Signed-off-by: Duncan Roe <duncan_roe@optusnet.com.au>
---
 .../libnetfilter_queue/libnetfilter_queue.h   |  1 +
 src/nlmsg.c                                   | 57 ++++++++++++++++++-
 2 files changed, 57 insertions(+), 1 deletion(-)

diff --git a/include/libnetfilter_queue/libnetfilter_queue.h b/include/libnetfilter_queue/libnetfilter_queue.h
index 3d8e444..f254984 100644
--- a/include/libnetfilter_queue/libnetfilter_queue.h
+++ b/include/libnetfilter_queue/libnetfilter_queue.h
@@ -151,6 +151,7 @@ void nfq_nlmsg_verdict_put_pkt(struct nlmsghdr *nlh, const void *pkt, uint32_t p
 
 int nfq_nlmsg_parse(const struct nlmsghdr *nlh, struct nlattr **attr);
 struct nlmsghdr *nfq_nlmsg_put(char *buf, int type, uint32_t queue_num);
+struct nlmsghdr *nfq_nlmsg_put2(char *buf, int type, uint32_t queue_num, uint16_t flags);
 
 #ifdef __cplusplus
 } /* extern "C" */
diff --git a/src/nlmsg.c b/src/nlmsg.c
index 5400dd7..865e508 100644
--- a/src/nlmsg.c
+++ b/src/nlmsg.c
@@ -309,10 +309,65 @@ int nfq_nlmsg_parse(const struct nlmsghdr *nlh, struct nlattr **attr)
  */
 EXPORT_SYMBOL
 struct nlmsghdr *nfq_nlmsg_put(char *buf, int type, uint32_t queue_num)
+{
+	return nfq_nlmsg_put2(buf, type, queue_num, NLM_F_REQUEST);
+}
+
+/**
+ * nfq_nlmsg_put2 - Convert memory buffer into a Netlink buffer with
+ * user-specified flags
+ * \param *buf Pointer to memory buffer
+ * \param type Either NFQNL_MSG_CONFIG or NFQNL_MSG_VERDICT
+ * \param queue_num Queue number
+ * \param flags flags to put in message header,
+ *              commonly NLM_F_REQUEST|NLM_F_ACK.
+ *              NLM_F_REQUEST by itself is the same as calling nfq_nlmsg_put()
+ * \returns Pointer to netlink message
+ *
+ * Use NLM_F_REQUEST|NLM_F_ACK before performing an action that might fail, e.g.
+ * attempt to configure NFQA_CFG_F_SECCTX on a system not runnine SELinux.
+ * \n
+ * NLM_F_ACK instructs the kernel to send a message in response
+ * to a successful command.
+ * The kernel always sends a message in response to a failed command.
+ * \n
+ * This code snippet demonstrates reading these responses:
+ * \verbatim
+	nlh = nfq_nlmsg_put2(nltxbuf, NFQNL_MSG_CONFIG, queue_num,
+			     NLM_F_REQUEST|NLM_F_ACK);
+	mnl_attr_put_u32(nlh, NFQA_CFG_FLAGS, NFQA_CFG_F_SECCTX);
+	mnl_attr_put_u32(nlh, NFQA_CFG_MASK, NFQA_CFG_F_SECCTX);
+
+	if (mnl_socket_sendto(nl, nlh, nlh->nlmsg_len) < 0) {
+		perror("mnl_socket_send");
+		exit(EXIT_FAILURE);
+	}
+
+	ret = mnl_socket_recvfrom(nl, nlrxbuf, sizeof nlrxbuf);
+	if (ret == -1) {
+		perror("mnl_socket_recvfrom");
+		exit(EXIT_FAILURE);
+	}
+
+	ret = mnl_cb_run(nlrxbuf, ret, 0, portid, NULL, NULL);
+	if (ret == -1)
+		perror("configure NFQA_CFG_F_SECCTX");
+\endverbatim
+ *
+ * \note
+ * The program above can continue after the error because NFQA_CFG_F_SECCTX
+ * was the only item in the preceding **mnl_socket_sendto**.
+ * If there had been other items, they would not have been actioned and it would
+ * not now be safe to proceed.
+ */
+
+EXPORT_SYMBOL
+struct nlmsghdr *nfq_nlmsg_put2(char *buf, int type, uint32_t queue_num,
+				uint16_t flags)
 {
 	struct nlmsghdr *nlh = mnl_nlmsg_put_header(buf);
 	nlh->nlmsg_type = (NFNL_SUBSYS_QUEUE << 8) | type;
-	nlh->nlmsg_flags = NLM_F_REQUEST;
+	nlh->nlmsg_flags = flags;
 
 	struct nfgenmsg *nfg = mnl_nlmsg_put_extra_header(nlh, sizeof(*nfg));
 	nfg->nfgen_family = AF_UNSPEC;
-- 
2.35.8

Re: [PATCH libnetfilter_queue v2 1/1] src: Add nfq_nlmsg_put2() - user specifies header flags

[Pablo Neira Ayuso]

On Wed, Nov 15, 2023 at 09:09:50PM +1100, Duncan Roe wrote:
> +EXPORT_SYMBOL
> +struct nlmsghdr *nfq_nlmsg_put2(char *buf, int type, uint32_t queue_num,
> +				uint16_t flags)
>  {
>  	struct nlmsghdr *nlh = mnl_nlmsg_put_header(buf);
>  	nlh->nlmsg_type = (NFNL_SUBSYS_QUEUE << 8) | type;
> -	nlh->nlmsg_flags = NLM_F_REQUEST
> +	nlh->nlmsg_flags = flags;

Leave this as is.

NLM_F_REQUEST means this message goes to the kernel, this flag is a
must have.

Re: [PATCH libnetfilter_queue v2 1/1] src: Add nfq_nlmsg_put2() - user specifies header flags

[Duncan Roe]

On Wed, Nov 15, 2023 at 11:25:27AM +0100, Pablo Neira Ayuso wrote:
> On Wed, Nov 15, 2023 at 09:09:50PM +1100, Duncan Roe wrote:
> > +EXPORT_SYMBOL
> > +struct nlmsghdr *nfq_nlmsg_put2(char *buf, int type, uint32_t queue_num,
> > +				uint16_t flags)
> >  {
> >  	struct nlmsghdr *nlh = mnl_nlmsg_put_header(buf);
> >  	nlh->nlmsg_type = (NFNL_SUBSYS_QUEUE << 8) | type;
> > -	nlh->nlmsg_flags = NLM_F_REQUEST
> > +	nlh->nlmsg_flags = flags;
>
> Leave this as is.
>
> NLM_F_REQUEST means this message goes to the kernel, this flag is a
> must have.

How about

	nlh->nlmsg_flags = NLM_F_REQUEST | flags;

Or, you could apply v1.
I couldn't see a use case for other flags (NLM_F_DUMP and so on) otherwise I
would have made flags an arg in v1.

On Tue, Nov 14, 2023 at 04:26:26PM +0100, Pablo Neira Ayuso wrote:
[...]
> I like this, but I'd suggest instead:
>
>   struct nlmsghdr *nfq_nlmsg_put2(char *buf, int type, uint32_t queue_num, uint16_flags);
>
> I should have expose those netlink flags in first place.
>
> There are more useful netlink flags, so just expose them all.
>
LMK,

Cheers ... Duncan.

Re: [PATCH libnetfilter_queue v2 1/1] src: Add nfq_nlmsg_put2() - user specifies header flags

[Pablo Neira Ayuso]

On Wed, Nov 15, 2023 at 09:53:24PM +1100, Duncan Roe wrote:
> On Wed, Nov 15, 2023 at 11:25:27AM +0100, Pablo Neira Ayuso wrote:
> > On Wed, Nov 15, 2023 at 09:09:50PM +1100, Duncan Roe wrote:
> > > +EXPORT_SYMBOL
> > > +struct nlmsghdr *nfq_nlmsg_put2(char *buf, int type, uint32_t queue_num,
> > > +				uint16_t flags)
> > >  {
> > >  	struct nlmsghdr *nlh = mnl_nlmsg_put_header(buf);
> > >  	nlh->nlmsg_type = (NFNL_SUBSYS_QUEUE << 8) | type;
> > > -	nlh->nlmsg_flags = NLM_F_REQUEST
> > > +	nlh->nlmsg_flags = flags;
> >
> > Leave this as is.
> >
> > NLM_F_REQUEST means this message goes to the kernel, this flag is a
> > must have.
> 
> How about
> 
> 	nlh->nlmsg_flags = NLM_F_REQUEST | flags;

Yes, that is fine.

[PATCH libnetfilter_queue v3 1/1] src: Add nfq_nlmsg_put2() - user specifies header flags

[Duncan Roe]

Enable mnl programs to check whether a config request was accepted.
(nfnl programs do this already).

v3: force on NLM_F_REQUEST

v2: take flags as an arg (Pablo request)
Signed-off-by: Duncan Roe <duncan_roe@optusnet.com.au>
---
 .../libnetfilter_queue/libnetfilter_queue.h   |  1 +
 src/nlmsg.c                                   | 55 ++++++++++++++++++-
 2 files changed, 55 insertions(+), 1 deletion(-)

diff --git a/include/libnetfilter_queue/libnetfilter_queue.h b/include/libnetfilter_queue/libnetfilter_queue.h
index 3d8e444..f254984 100644
--- a/include/libnetfilter_queue/libnetfilter_queue.h
+++ b/include/libnetfilter_queue/libnetfilter_queue.h
@@ -151,6 +151,7 @@ void nfq_nlmsg_verdict_put_pkt(struct nlmsghdr *nlh, const void *pkt, uint32_t p
 
 int nfq_nlmsg_parse(const struct nlmsghdr *nlh, struct nlattr **attr);
 struct nlmsghdr *nfq_nlmsg_put(char *buf, int type, uint32_t queue_num);
+struct nlmsghdr *nfq_nlmsg_put2(char *buf, int type, uint32_t queue_num, uint16_t flags);
 
 #ifdef __cplusplus
 } /* extern "C" */
diff --git a/src/nlmsg.c b/src/nlmsg.c
index 5400dd7..999ccfe 100644
--- a/src/nlmsg.c
+++ b/src/nlmsg.c
@@ -309,10 +309,63 @@ int nfq_nlmsg_parse(const struct nlmsghdr *nlh, struct nlattr **attr)
  */
 EXPORT_SYMBOL
 struct nlmsghdr *nfq_nlmsg_put(char *buf, int type, uint32_t queue_num)
+{
+	return nfq_nlmsg_put2(buf, type, queue_num, 0);
+}
+
+/**
+ * nfq_nlmsg_put2 - Convert memory buffer into a Netlink buffer with
+ * user-specified flags
+ * \param *buf Pointer to memory buffer
+ * \param type Either NFQNL_MSG_CONFIG or NFQNL_MSG_VERDICT
+ * \param queue_num Queue number
+ * \param flags additional (to NLM_F_REQUEST) flags to put in message header,
+ *              commonly NLM_F_ACK
+ * \returns Pointer to netlink message
+ *
+ * Use NLM_F_ACK before performing an action that might fail, e.g.
+ * attempt to configure NFQA_CFG_F_SECCTX on a system not runnine SELinux.
+ * \n
+ * NLM_F_ACK instructs the kernel to send a message in response
+ * to a successful command.
+ * The kernel always sends a message in response to a failed command.
+ * \n
+ * This code snippet demonstrates reading these responses:
+ * \verbatim
+	nlh = nfq_nlmsg_put2(nltxbuf, NFQNL_MSG_CONFIG, queue_num, NLM_F_ACK);
+	mnl_attr_put_u32(nlh, NFQA_CFG_FLAGS, NFQA_CFG_F_SECCTX);
+	mnl_attr_put_u32(nlh, NFQA_CFG_MASK, NFQA_CFG_F_SECCTX);
+
+	if (mnl_socket_sendto(nl, nlh, nlh->nlmsg_len) < 0) {
+		perror("mnl_socket_send");
+		exit(EXIT_FAILURE);
+	}
+
+	ret = mnl_socket_recvfrom(nl, nlrxbuf, sizeof nlrxbuf);
+	if (ret == -1) {
+		perror("mnl_socket_recvfrom");
+		exit(EXIT_FAILURE);
+	}
+
+	ret = mnl_cb_run(nlrxbuf, ret, 0, portid, NULL, NULL);
+	if (ret == -1)
+		perror("configure NFQA_CFG_F_SECCTX");
+\endverbatim
+ *
+ * \note
+ * The program above can continue after the error because NFQA_CFG_F_SECCTX
+ * was the only item in the preceding **mnl_socket_sendto**.
+ * If there had been other items, they would not have been actioned and it would
+ * not now be safe to proceed.
+ */
+
+EXPORT_SYMBOL
+struct nlmsghdr *nfq_nlmsg_put2(char *buf, int type, uint32_t queue_num,
+				uint16_t flags)
 {
 	struct nlmsghdr *nlh = mnl_nlmsg_put_header(buf);
 	nlh->nlmsg_type = (NFNL_SUBSYS_QUEUE << 8) | type;
-	nlh->nlmsg_flags = NLM_F_REQUEST;
+	nlh->nlmsg_flags = NLM_F_REQUEST | flags;
 
 	struct nfgenmsg *nfg = mnl_nlmsg_put_extra_header(nlh, sizeof(*nfg));
 	nfg->nfgen_family = AF_UNSPEC;
-- 
2.35.8

Re: [PATCH libnetfilter_queue v3 1/1] src: Add nfq_nlmsg_put2() - user specifies header flags

[Pablo Neira Ayuso]

On Wed, Nov 15, 2023 at 10:30:11PM +1100, Duncan Roe wrote:
> Enable mnl programs to check whether a config request was accepted.
> (nfnl programs do this already).
> 
> v3: force on NLM_F_REQUEST
> 
> v2: take flags as an arg (Pablo request)
> Signed-off-by: Duncan Roe <duncan_roe@optusnet.com.au>
> ---
>  .../libnetfilter_queue/libnetfilter_queue.h   |  1 +
>  src/nlmsg.c                                   | 55 ++++++++++++++++++-
>  2 files changed, 55 insertions(+), 1 deletion(-)
> 
> diff --git a/include/libnetfilter_queue/libnetfilter_queue.h b/include/libnetfilter_queue/libnetfilter_queue.h
> index 3d8e444..f254984 100644
> --- a/include/libnetfilter_queue/libnetfilter_queue.h
> +++ b/include/libnetfilter_queue/libnetfilter_queue.h
> @@ -151,6 +151,7 @@ void nfq_nlmsg_verdict_put_pkt(struct nlmsghdr *nlh, const void *pkt, uint32_t p
>  
>  int nfq_nlmsg_parse(const struct nlmsghdr *nlh, struct nlattr **attr);
>  struct nlmsghdr *nfq_nlmsg_put(char *buf, int type, uint32_t queue_num);
> +struct nlmsghdr *nfq_nlmsg_put2(char *buf, int type, uint32_t queue_num, uint16_t flags);
>  
>  #ifdef __cplusplus
>  } /* extern "C" */
> diff --git a/src/nlmsg.c b/src/nlmsg.c
> index 5400dd7..999ccfe 100644
> --- a/src/nlmsg.c
> +++ b/src/nlmsg.c
> @@ -309,10 +309,63 @@ int nfq_nlmsg_parse(const struct nlmsghdr *nlh, struct nlattr **attr)
>   */
>  EXPORT_SYMBOL
>  struct nlmsghdr *nfq_nlmsg_put(char *buf, int type, uint32_t queue_num)
> +{
> +	return nfq_nlmsg_put2(buf, type, queue_num, 0);
> +}
> +
> +/**
> + * nfq_nlmsg_put2 - Convert memory buffer into a Netlink buffer with
> + * user-specified flags

This is setting up a netlink header in the memory buffer.

> + * \param *buf Pointer to memory buffer
> + * \param type Either NFQNL_MSG_CONFIG or NFQNL_MSG_VERDICT
> + * \param queue_num Queue number
> + * \param flags additional (to NLM_F_REQUEST) flags to put in message header,
> + *              commonly NLM_F_ACK

remove NLM_F_REQUEST here.

> + * \returns Pointer to netlink message

               Pointer to netlink header

> + *
> + * Use NLM_F_ACK before performing an action that might fail, e.g.

Failures are always reported.

if you set NLM_F_ACK, then you always get an acknowledgment from the
kernel, either 0 to report success or negative to report failure.

if you do not set NLM_F_ACK, then only failures are reported by the
kernel.

> + * attempt to configure NFQA_CFG_F_SECCTX on a system not runnine SELinux.
> + * \n
> + * NLM_F_ACK instructs the kernel to send a message in response
> + * to a successful command.

As I said above, this is not accurate.

> + * The kernel always sends a message in response to a failed command.
> + * \n
> + * This code snippet demonstrates reading these responses:
> + * \verbatim
> +	nlh = nfq_nlmsg_put2(nltxbuf, NFQNL_MSG_CONFIG, queue_num, NLM_F_ACK);
> +	mnl_attr_put_u32(nlh, NFQA_CFG_FLAGS, NFQA_CFG_F_SECCTX);
> +	mnl_attr_put_u32(nlh, NFQA_CFG_MASK, NFQA_CFG_F_SECCTX);
> +
> +	if (mnl_socket_sendto(nl, nlh, nlh->nlmsg_len) < 0) {
> +		perror("mnl_socket_send");
> +		exit(EXIT_FAILURE);
> +	}
> +
> +	ret = mnl_socket_recvfrom(nl, nlrxbuf, sizeof nlrxbuf);
> +	if (ret == -1) {
> +		perror("mnl_socket_recvfrom");
> +		exit(EXIT_FAILURE);
> +	}
> +
> +	ret = mnl_cb_run(nlrxbuf, ret, 0, portid, NULL, NULL);
> +	if (ret == -1)
> +		perror("configure NFQA_CFG_F_SECCTX");
> +\endverbatim
> + *
> + * \note
> + * The program above can continue after the error because NFQA_CFG_F_SECCTX
> + * was the only item in the preceding **mnl_socket_sendto**.
> + * If there had been other items, they would not have been actioned and it would
> + * not now be safe to proceed.
> + */
> +
> +EXPORT_SYMBOL
> +struct nlmsghdr *nfq_nlmsg_put2(char *buf, int type, uint32_t queue_num,
> +				uint16_t flags)
>  {
>  	struct nlmsghdr *nlh = mnl_nlmsg_put_header(buf);
>  	nlh->nlmsg_type = (NFNL_SUBSYS_QUEUE << 8) | type;
> -	nlh->nlmsg_flags = NLM_F_REQUEST;
> +	nlh->nlmsg_flags = NLM_F_REQUEST | flags;
>  
>  	struct nfgenmsg *nfg = mnl_nlmsg_put_extra_header(nlh, sizeof(*nfg));
>  	nfg->nfgen_family = AF_UNSPEC;
> -- 
> 2.35.8
> 

Re: [PATCH libnetfilter_queue v3 1/1] src: Add nfq_nlmsg_put2() - user specifies header flags

[Duncan Roe]

On Wed, Nov 15, 2023 at 12:41:03PM +0100, Pablo Neira Ayuso wrote:
> On Wed, Nov 15, 2023 at 10:30:11PM +1100, Duncan Roe wrote:
> > Enable mnl programs to check whether a config request was accepted.
> > (nfnl programs do this already).
> >
> > v3: force on NLM_F_REQUEST
> >
> > v2: take flags as an arg (Pablo request)
> > Signed-off-by: Duncan Roe <duncan_roe@optusnet.com.au>
> > ---
> >  .../libnetfilter_queue/libnetfilter_queue.h   |  1 +
> >  src/nlmsg.c                                   | 55 ++++++++++++++++++-
> >  2 files changed, 55 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/libnetfilter_queue/libnetfilter_queue.h b/include/libnetfilter_queue/libnetfilter_queue.h
> > index 3d8e444..f254984 100644
> > --- a/include/libnetfilter_queue/libnetfilter_queue.h
> > +++ b/include/libnetfilter_queue/libnetfilter_queue.h
> > @@ -151,6 +151,7 @@ void nfq_nlmsg_verdict_put_pkt(struct nlmsghdr *nlh, const void *pkt, uint32_t p
> >
> >  int nfq_nlmsg_parse(const struct nlmsghdr *nlh, struct nlattr **attr);
> >  struct nlmsghdr *nfq_nlmsg_put(char *buf, int type, uint32_t queue_num);
> > +struct nlmsghdr *nfq_nlmsg_put2(char *buf, int type, uint32_t queue_num, uint16_t flags);
> >
> >  #ifdef __cplusplus
> >  } /* extern "C" */
> > diff --git a/src/nlmsg.c b/src/nlmsg.c
> > index 5400dd7..999ccfe 100644
> > --- a/src/nlmsg.c
> > +++ b/src/nlmsg.c
> > @@ -309,10 +309,63 @@ int nfq_nlmsg_parse(const struct nlmsghdr *nlh, struct nlattr **attr)
> >   */
> >  EXPORT_SYMBOL
> >  struct nlmsghdr *nfq_nlmsg_put(char *buf, int type, uint32_t queue_num)
> > +{
> > +	return nfq_nlmsg_put2(buf, type, queue_num, 0);
> > +}
> > +
> > +/**
> > + * nfq_nlmsg_put2 - Convert memory buffer into a Netlink buffer with
> > + * user-specified flags
>
> This is setting up a netlink header in the memory buffer.

Yes. I just copied the description from nfq_nlmsg_put and tacked on "with
user-specified flags". Do you want to see
> Set up a netlink header in a memory buffer with user-specified flags
perhaps better
> Set up a netlink header with user-specified flags in a memory buffer
instead? And would you like me to change the nfq_nlmsg_put description to match?
>
> > + * \param *buf Pointer to memory buffer
> > + * \param type Either NFQNL_MSG_CONFIG or NFQNL_MSG_VERDICT
> > + * \param queue_num Queue number
> > + * \param flags additional (to NLM_F_REQUEST) flags to put in message header,
> > + *              commonly NLM_F_ACK
>
> remove NLM_F_REQUEST here.

Ok
>
> > + * \returns Pointer to netlink message
>
>                Pointer to netlink header

Again, copied from nfq_nlmsg_put. Fix that as well?
>
> > + *
> > + * Use NLM_F_ACK before performing an action that might fail, e.g.
>
> Failures are always reported.
>
> if you set NLM_F_ACK, then you always get an acknowledgment from the
> kernel, either 0 to report success or negative to report failure.
>
> if you do not set NLM_F_ACK, then only failures are reported by the
> kernel.

Yes, I was trying to explain that. The point being, if you don't specify
NLM_F_ACK and the command succeeds then mnl_socket_recvfrom() will hang.
>
> > + * attempt to configure NFQA_CFG_F_SECCTX on a system not runnine SELinux.
> > + * \n
> > + * NLM_F_ACK instructs the kernel to send a message in response
> > + * to a successful command.
>
> As I said above, this is not accurate.

Sorry, it looks to me to be the same as what you said. Which bit is not
accurate, what should it say instead?
>
> > + * The kernel always sends a message in response to a failed command.
> > + * \n
> > + * This code snippet demonstrates reading these responses:
> > + * \verbatim
> > +	nlh = nfq_nlmsg_put2(nltxbuf, NFQNL_MSG_CONFIG, queue_num, NLM_F_ACK);
> > +	mnl_attr_put_u32(nlh, NFQA_CFG_FLAGS, NFQA_CFG_F_SECCTX);
> > +	mnl_attr_put_u32(nlh, NFQA_CFG_MASK, NFQA_CFG_F_SECCTX);
> > +
> > +	if (mnl_socket_sendto(nl, nlh, nlh->nlmsg_len) < 0) {
> > +		perror("mnl_socket_send");
> > +		exit(EXIT_FAILURE);
> > +	}
> > +
> > +	ret = mnl_socket_recvfrom(nl, nlrxbuf, sizeof nlrxbuf);
> > +	if (ret == -1) {
> > +		perror("mnl_socket_recvfrom");
> > +		exit(EXIT_FAILURE);
> > +	}
> > +
> > +	ret = mnl_cb_run(nlrxbuf, ret, 0, portid, NULL, NULL);
> > +	if (ret == -1)
> > +		perror("configure NFQA_CFG_F_SECCTX");
> > +\endverbatim
> > + *
> > + * \note
> > + * The program above can continue after the error because NFQA_CFG_F_SECCTX
> > + * was the only item in the preceding **mnl_socket_sendto**.
> > + * If there had been other items, they would not have been actioned and it would
> > + * not now be safe to proceed.
> > + */
> > +
> > +EXPORT_SYMBOL
> > +struct nlmsghdr *nfq_nlmsg_put2(char *buf, int type, uint32_t queue_num,
> > +				uint16_t flags)
> >  {
> >  	struct nlmsghdr *nlh = mnl_nlmsg_put_header(buf);
> >  	nlh->nlmsg_type = (NFNL_SUBSYS_QUEUE << 8) | type;
> > -	nlh->nlmsg_flags = NLM_F_REQUEST;
> > +	nlh->nlmsg_flags = NLM_F_REQUEST | flags;
> >
> >  	struct nfgenmsg *nfg = mnl_nlmsg_put_extra_header(nlh, sizeof(*nfg));
> >  	nfg->nfgen_family = AF_UNSPEC;
> > --
> > 2.35.8
> >

Re: [PATCH libnetfilter_queue v3 1/1] src: Add nfq_nlmsg_put2() - user specifies header flags

[Duncan Roe]

Hi Pablo,

Can we please sort out just what you want before I send nfq_nlmsg_put2 v4?

And, where applicable, would you like the same changes made to nfq_nlmsg_put?

On Wed, Nov 15, 2023 at 12:41:03PM +0100, Pablo Neira Ayuso wrote:
> [...]
> > + * nfq_nlmsg_put2 - Convert memory buffer into a Netlink buffer with
> > + * user-specified flags
>
> This is setting up a netlink header in the memory buffer.
propose
> > + * nfq_nlmsg_put2 - Set up a netlink header with user-specified flags
> > + *                  in a memory buffer
> [...]
> > + * \param flags additional (to NLM_F_REQUEST) flags to put in message header,
> > + *              commonly NLM_F_ACK
>
> remove NLM_F_REQUEST here.
propose
> > + * \param flags flags to put in message header, commonly NLM_F_ACK
> [...]
> > + * \returns Pointer to netlink message
>
>                Pointer to netlink header
propose
> > + * \returns Pointer to netlink header
> [...]
> > + * Use NLM_F_ACK before performing an action that might fail, e.g.
>
> Failures are always reported.
>
> if you set NLM_F_ACK, then you always get an acknowledgment from the
> kernel, either 0 to report success or negative to report failure.
>
> if you do not set NLM_F_ACK, then only failures are reported by the
> kernel.
>
> > + * attempt to configure NFQA_CFG_F_SECCTX on a system not runnine SELinux.
> > + * \n
> > + * NLM_F_ACK instructs the kernel to send a message in response
> > + * to a successful command.
>
> As I said above, this is not accurate.
> > + * The kernel always sends a message in response to a failed command.
I dispute that my description was inaccurate, but admit it could be clearer,
maybe if I change the order and elaborate a bit.
propose
> > + * The kernel always sends a message in response to a failed command.
> > + * NLM_F_ACK instructs the kernel to also send a message in response
> > + * to a successful command. This ensures a following read() will not block.
> [...]

Cheers ... Duncan.

Re: [PATCH libnetfilter_queue v3 1/1] src: Add nfq_nlmsg_put2() - user specifies header flags

[Pablo Neira Ayuso]

On Sat, Nov 18, 2023 at 03:11:56PM +1100, Duncan Roe wrote:
> Hi Pablo,
> 
> Can we please sort out just what you want before I send nfq_nlmsg_put2 v4?
> 
> And, where applicable, would you like the same changes made to nfq_nlmsg_put?

Just send a v4 with the changes I request for this patch, then once
applied, you can follow up to update nfq_nlmsg_put() in a separated
patch to amend that description too.

So, please, only one patch series at a time.

> On Wed, Nov 15, 2023 at 12:41:03PM +0100, Pablo Neira Ayuso wrote:
[...]
> > > + * attempt to configure NFQA_CFG_F_SECCTX on a system not runnine SELinux.
> > > + * \n
> > > + * NLM_F_ACK instructs the kernel to send a message in response
> > > + * to a successful command.
> >
> > As I said above, this is not accurate.
> > > + * The kernel always sends a message in response to a failed command.
>
> I dispute that my description was inaccurate, but admit it could be clearer,
> maybe if I change the order and elaborate a bit.
> propose
>
> > > + * The kernel always sends a message in response to a failed command.
> > > + * NLM_F_ACK instructs the kernel to also send a message in response
> > > + * to a successful command.

LGTM, however:

> > > + * This ensures a following read() will not block.

Remove this sentence, because the blocking behaviour you observe is
because !NLM_F_ACK and no failure means no message is sent, and if
your application is there to recv(), it will wait forever because
kernel will send nothing.

Re: [PATCH libnetfilter_queue v3 1/1] src: Add nfq_nlmsg_put2() - user specifies header flags

[Duncan Roe]

Hi Pablo,

On Sat, Nov 18, 2023 at 09:25:25PM +0100, Pablo Neira Ayuso wrote:
> On Sat, Nov 18, 2023 at 03:11:56PM +1100, Duncan Roe wrote:
> > Hi Pablo,
> >
> > Can we please sort out just what you want before I send nfq_nlmsg_put2 v4?
> >
> > And, where applicable, would you like the same changes made to nfq_nlmsg_put?
>
> Just send a v4 with the changes I request for this patch, then once
> applied, you can follow up to update nfq_nlmsg_put() in a separated
> patch to amend that description too.
>
> So, please, only one patch series at a time.
>
> > On Wed, Nov 15, 2023 at 12:41:03PM +0100, Pablo Neira Ayuso wrote:
> [...]
> > > > + * attempt to configure NFQA_CFG_F_SECCTX on a system not runnine SELinux.
> > > > + * \n
> > > > + * NLM_F_ACK instructs the kernel to send a message in response
> > > > + * to a successful command.
> > >
> > > As I said above, this is not accurate.
> > > > + * The kernel always sends a message in response to a failed command.
> >
> > I dispute that my description was inaccurate, but admit it could be clearer,
> > maybe if I change the order and elaborate a bit.
> > propose
> >
> > > > + * The kernel always sends a message in response to a failed command.
> > > > + * NLM_F_ACK instructs the kernel to also send a message in response
> > > > + * to a successful command.
>
> LGTM, however:
>
> > > > + * This ensures a following read() will not block.
>
> Remove this sentence, because the blocking behaviour you observe is
> because !NLM_F_ACK and no failure means no message is sent, and if
> your application is there to recv(), it will wait forever because
> kernel will send nothing.

I did post v4 but forgot --in-reply-to in git format-patch.
You'll find the updated patch furtheron in your mbox.

Cheers ... Duncan.

Re: [PATCH libnetfilter_queue v4 1/1] src: Add nfq_nlmsg_put2() - user specifies header flags

[Duncan Roe]

Hey Pablo,

Are you too busy to reply to my emails? There will be a lot more. Have you
thought of passing management of this libmnl-conversion project to another core
team member?

On Tue, Nov 21, 2023 at 08:25:54AM +1100, Duncan Roe wrote:
> Hi Pablo,
>
> On Sat, Nov 18, 2023 at 09:25:25PM +0100, Pablo Neira Ayuso wrote:
> > On Sat, Nov 18, 2023 at 03:11:56PM +1100, Duncan Roe wrote:
> > > Hi Pablo,
> > >
> > > Can we please sort out just what you want before I send nfq_nlmsg_put2 v4?
> > >
> > > And, where applicable, would you like the same changes made to nfq_nlmsg_put?
> >
> > Just send a v4 with the changes I request for this patch, then once
> > applied, you can follow up to update nfq_nlmsg_put() in a separated
> > patch to amend that description too.
> >
> > So, please, only one patch series at a time.
> >
> > > On Wed, Nov 15, 2023 at 12:41:03PM +0100, Pablo Neira Ayuso wrote:
> > [...]
> > > > > + * attempt to configure NFQA_CFG_F_SECCTX on a system not runnine SELinux.
> > > > > + * \n
> > > > > + * NLM_F_ACK instructs the kernel to send a message in response
> > > > > + * to a successful command.
> > > >
> > > > As I said above, this is not accurate.
> > > > > + * The kernel always sends a message in response to a failed command.
> > >
> > > I dispute that my description was inaccurate, but admit it could be clearer,
> > > maybe if I change the order and elaborate a bit.
> > > propose
> > >
> > > > > + * The kernel always sends a message in response to a failed command.
> > > > > + * NLM_F_ACK instructs the kernel to also send a message in response
> > > > > + * to a successful command.
> >
> > LGTM, however:
> >
> > > > > + * This ensures a following read() will not block.
> >
> > Remove this sentence, because the blocking behaviour you observe is
> > because !NLM_F_ACK and no failure means no message is sent, and if
> > your application is there to recv(), it will wait forever because
> > kernel will send nothing.

"it will wait forever" i.e. it will block.

I could send a v5 with this:
> + * Use NLM_F_ACK to ensure a kernel response for your application to read.

[...]

Cheers ... Duncan.

Re: [PATCH libnetfilter_queue v4 1/1] src: Add nfq_nlmsg_put2() - user specifies header flags

[Pablo Neira Ayuso]

On Fri, Nov 24, 2023 at 09:23:48AM +1100, Duncan Roe wrote:
> Hey Pablo,
> 
> Are you too busy to reply to my emails?

Yes, I am very busy.

> There will be a lot more. Have you thought of passing management of
> this libmnl-conversion project to another core team member?

Absolutely not.

Please, be patient.