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@xxxxxxxxxxxxxxx>
> > ---
> > .../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
> >
