Re: [PATCH 2/7] netfilter: xtables2: initial Netlink interface

[Pablo Neira Ayuso <pablo@netfilter.org>]

On Thu, Jan 19, 2012 at 05:26:16PM +0100, Jan Engelhardt wrote:
> This populates xt2_nfnetlink.c with support for NFXTM_IDENTIFY. Right
> now that just returns a freeform string but eventually shall dump all
> the match and target modules' info.
>
> Signed-off-by: Jan Engelhardt <jengelh@medozas.de>
> ---
>  include/linux/netfilter/Kbuild              |    1 +
>  include/linux/netfilter/nfnetlink.h         |    3 +-
>  include/linux/netfilter/nfnetlink_xtables.h |   20 ++++
>  net/netfilter/Kconfig                       |    8 ++
>  net/netfilter/Makefile                      |    1 +
>  net/netfilter/xt2_nfnetlink.c               |  153 +++++++++++++++++++++++++++

I prefer if you call this nfnetlink_xtables.c following the same
naming convention.

nf_conntrack_netlink may seems one exception but I have a patch here
to integrate ctnetlink with nf_queue (to allow delivering conntrack
events over netlink unicast using iptables) and, then, we'll have
nfnetlink_conntrack.c as well.

>  6 files changed, 185 insertions(+), 1 deletions(-)
>  create mode 100644 include/linux/netfilter/nfnetlink_xtables.h
>  create mode 100644 net/netfilter/xt2_nfnetlink.c
> 
> diff --git a/include/linux/netfilter/Kbuild b/include/linux/netfilter/Kbuild
> index a1b410c..6e568c9 100644
> --- a/include/linux/netfilter/Kbuild
> +++ b/include/linux/netfilter/Kbuild
> @@ -10,6 +10,7 @@ header-y += nfnetlink_compat.h
>  header-y += nfnetlink_conntrack.h
>  header-y += nfnetlink_log.h
>  header-y += nfnetlink_queue.h
> +header-y += nfnetlink_xtables.h
>  header-y += x_tables.h
>  header-y += xt_AUDIT.h
>  header-y += xt_CHECKSUM.h
> diff --git a/include/linux/netfilter/nfnetlink.h b/include/linux/netfilter/nfnetlink.h
> index 74d3386..93228b4 100644
> --- a/include/linux/netfilter/nfnetlink.h
> +++ b/include/linux/netfilter/nfnetlink.h
> @@ -48,7 +48,8 @@ struct nfgenmsg {
>  #define NFNL_SUBSYS_ULOG		4
>  #define NFNL_SUBSYS_OSF			5
>  #define NFNL_SUBSYS_IPSET		6
> -#define NFNL_SUBSYS_COUNT		7
> +#define NFNL_SUBSYS_XTABLES		8
> +#define NFNL_SUBSYS_COUNT		9
>  
>  #ifdef __KERNEL__
>  
> diff --git a/include/linux/netfilter/nfnetlink_xtables.h b/include/linux/netfilter/nfnetlink_xtables.h
> new file mode 100644
> index 0000000..4c53042
> --- /dev/null
> +++ b/include/linux/netfilter/nfnetlink_xtables.h
> @@ -0,0 +1,20 @@
> +#ifndef _LINUX_NFNETLINK_XTABLES_H
> +#define _LINUX_NFNETLINK_XTABLES_H 1
> +
> +enum nfxt_msg_type {
> +	NFXTM_IDENTIFY = 1,

I'd suggest:

NFXT_MSG_NEW,
NFXT_MSG_GET,
...

and so on.

> +};
> +
> +/**
> + * %NFXTA_NAME:			name of the object being operated on
> + */
> +enum nfxt_attr_type {
> +	NFXTA_UNSPEC = 0,
> +	NFXTA_NAME,
> +};
> +
> +enum nfxt_errno {
> +	NFXTE_SUCCESS = 0,
> +};
> +
> +#endif /* _LINUX_NFNETLINK_XTABLES_H */
> diff --git a/net/netfilter/Kconfig b/net/netfilter/Kconfig
> index 5b3d9ca..6237a30 100644
> --- a/net/netfilter/Kconfig
> +++ b/net/netfilter/Kconfig
> @@ -327,6 +327,14 @@ config NETFILTER_XTABLES2
>  	Xtables2 is a rework of the internal architecture of Xtables.
>  	It supersedes iptables, ip6tables, arptables and ebtables.
>  
> +if NETFILTER_XTABLES2
> +
> +config NETFILTER_XTNFNETLINK
> +	tristate "Configuration interface over nfnetlink"
> +	default m if NETFILTER_ADVANCED=n
> +
> +endif # NETFILTER_XTABLES2
> +
>  if NETFILTER_XTABLES || NETFILTER_XTABLES2
>  
>  comment "Xtables combined modules"
> diff --git a/net/netfilter/Makefile b/net/netfilter/Makefile
> index 8504ebd..634e6a8 100644
> --- a/net/netfilter/Makefile
> +++ b/net/netfilter/Makefile
> @@ -43,6 +43,7 @@ obj-$(CONFIG_NETFILTER_TPROXY) += nf_tproxy_core.o
>  # generic X tables 
>  obj-$(CONFIG_NETFILTER_XTABLES) += x_tables.o xt_tcpudp.o
>  obj-$(CONFIG_NETFILTER_XTABLES2) += xt2_core.o
> +obj-$(CONFIG_NETFILTER_XTNFNETLINK) += xt2_nfnetlink.o
>  
>  # combos
>  obj-$(CONFIG_NETFILTER_XT_MARK) += xt_mark.o
> diff --git a/net/netfilter/xt2_nfnetlink.c b/net/netfilter/xt2_nfnetlink.c
> new file mode 100644
> index 0000000..3dc241f
> --- /dev/null
> +++ b/net/netfilter/xt2_nfnetlink.c
> @@ -0,0 +1,153 @@
> +/*
> + *	Xtables2 nfnetlink interface
> + *	Copyright © Jan Engelhardt, 2010-2012
> + *
> + *	This program is free software: you can redistribute it and/or modify
> + *	it under the terms of the GNU General Public License as published by
> + *	the Free Software Foundation, either version 2 of the License, or
> + *	(at your option) any later version.
> + */
> +#include <linux/err.h>
> +#include <linux/errno.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/netlink.h>
> +#include <linux/skbuff.h>
> +#include <linux/netfilter.h>
> +#include <linux/netfilter/nfnetlink.h>
> +#include <linux/netfilter/nfnetlink_xtables.h>
> +#include <net/netlink.h>
> +#include <net/netfilter/x_tables2.h>
> +
> +MODULE_DESCRIPTION("Xtables2 nfnetlink interface");
> +MODULE_AUTHOR("Jan Engelhardt");
> +MODULE_LICENSE("GPL");
> +
> +/**
> + * Something to keep all the pointers that NL gives us in one place.
> + */
> +struct xtnetlink_pktref {
> +	union {
> +		struct sk_buff *skb;
> +		const struct sk_buff *c_skb;
> +	};
> +	union {
> +		struct nlmsghdr *msg;
> +		const struct nlmsghdr *c_msg;
> +	};
> +};
> +
> +/**
> + * @skb:	outgoing skb
> + * @old:	pointers to the original incoming skb/nl headers
> + * @flags:	extra flags to set in nlmsg
> + *
> + * Fill an skb (for outgoing direction) with NL and NFNL headers, using data
> + * from the original skb for initialization.
> + */
> +static struct nlmsghdr *
> +xtnetlink_fill(struct sk_buff *skb, const struct xtnetlink_pktref *old,
> +	       unsigned int flags)
> +{
> +	uint16_t nlmsg_type = NFNL_MSG_TYPE(old->msg->nlmsg_type);
> +	struct nlmsghdr *nlmsg;
> +	struct nfgenmsg *hdr;
> +
> +	nlmsg_type |= NFNL_SUBSYS_XTABLES << 8;
> +	nlmsg = nlmsg_put(skb, NETLINK_CB(old->skb).pid, old->msg->nlmsg_seq,
> +			  nlmsg_type, sizeof(*hdr), flags);
> +	if (nlmsg == NULL) {
> +		nlmsg_cancel(skb, nlmsg);
> +		return ERR_PTR(-ENOBUFS);

my experience is that it's better to use goto to handle errors.

> +	}
> +
> +	hdr               = nlmsg_data(nlmsg);
> +	hdr->nfgen_family = NFPROTO_UNSPEC;
> +	hdr->version      = NFNETLINK_V0;
> +	hdr->res_id       = 0;
> +	return nlmsg;
> +}
> +
> +/**
> + * Ran too often into NULL derefs. Now there is a dummy function for unused
> + * message type 0.
> + */
> +static int
> +xtnetlink_ignore(struct sock *k, struct sk_buff *s,
> +		 const struct nlmsghdr *n, const struct nlattr *const *a)
> +{
> +	return -ENXIO;
> +}
> +
> +/**
> + * Respond to a %NFXTM_IDENTIFY inquiry. Reports the Xtables2 version (for
> + * now), and in future the extension info.
> + */
> +static int
> +xtnetlink_identify2(struct sk_buff *skb, struct netlink_callback *nl_cb)

I'd add the infix _dump_ so you can notice this is the callback that
netlink_dump_start uses.

> +{
> +	struct xtnetlink_pktref ref = {.c_skb = skb, .c_msg = nl_cb->nlh};
> +	struct nlmsghdr *nlmsg = NULL;
> +
> +	switch (nl_cb->args[0]) {
> +	case 0:
> +		nlmsg = xtnetlink_fill(skb, &ref, NLM_F_MULTI);
> +		if (IS_ERR(nlmsg))
> +			return 0;
> +		NLA_PUT_STRING(skb, NFXTA_NAME, XTABLES2_VTAG);
> +		++nl_cb->args[0];
> +		break;
> +	}
> +	if (nlmsg != NULL)
> +		nlmsg_end(skb, nlmsg);
> +	return skb->len;
> + nla_put_failure:
> +	return 0;
> +}
> +
> +static int
> +xtnetlink_identify(struct sock *xtnl, struct sk_buff *iskb,
> +		   const struct nlmsghdr *imsg, const struct nlattr *const *ad)
> +{
> +	return netlink_dump_start(xtnl, iskb, imsg, xtnetlink_identify2,
> +				  NULL, 0);

you have to check for NLM_F_DUMP. Otherwise, you are requesting one
single table. Please, see ctnetlink or nfnetlink_acct for reference.

> +}
> +
> +static const struct nla_policy xtnetlink_policy[] = {
> +	[NFXTA_NAME] = {.type = NLA_NUL_STRING},
> +};
> +
> +/*
> + * Use the same policy for all messages. I do not want to see EINVAL anytime
> + * soon again just because I forgot sending an attribute from userspace.
> + * (If such occurs, it will be dealt with %NFXTE_ATTRSET_INCOMPLETE, tbd.)
> + */
> +#define pol \
> +	.policy = xtnetlink_policy, \
> +	.attr_count = ARRAY_SIZE(xtnetlink_policy)
> +static const struct nfnl_callback xtnetlink_callback[] = {
> +	[0] = {.call = xtnetlink_ignore},
> +	[NFXTM_IDENTIFY] = {.call = xtnetlink_identify, pol},
> +};
> +#undef pol
> +
> +static const struct nfnetlink_subsystem xtnetlink_subsys = {
> +	.name      = "xtables",
> +	.subsys_id = NFNL_SUBSYS_XTABLES,
> +	.cb        = xtnetlink_callback,
> +	.cb_count  = ARRAY_SIZE(xtnetlink_callback),
> +};
> +
> +static int __init xtnetlink_init(void)
> +{
> +	return nfnetlink_subsys_register(&xtnetlink_subsys);
> +}
> +
> +static void __exit xtnetlink_exit(void)
> +{
> +	nfnetlink_subsys_unregister(&xtnetlink_subsys);
> +}
> +
> +module_init(xtnetlink_init);
> +module_exit(xtnetlink_exit);
> +MODULE_ALIAS_NFNL_SUBSYS(NFNL_SUBSYS_XTABLES);
> -- 
> 1.7.7
> 
--
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