[libnftnl PATCH v3] utils: fix arp family number

[Arturo Borrero Gonzalez]

NFPROTO_ARP = 3 in kernel space.

We need the same value here in userspace in order to correctly communicate
with the kernel.

The failure solved by this patch made that {XML|JSON}-parsed tables of ARP
family unable to be directly injected into kernel.

To prevent future errors, this patch changes raw and AF_* values by the mathing
NFPROTO_* couterpart as seen in linux/netfilter.h in both functions:
* nft_family2str()
* nft_str2family()

Signed-off-by: Arturo Borrero Gonzalez <***@gmail.com>
---
v2: rework+fix using the array-matching approach suggested by Pablo.
v3: constify both the pointer and the data, suggested by Jan.
Keep setting errno to EAFNOSUPPORT in nft_str2family().

src/utils.c | 43 ++++++++++++++++++++-----------------------
1 file changed, 20 insertions(+), 23 deletions(-)

diff --git a/src/utils.c b/src/utils.c
index d70fbf1..9013b68 100644
--- a/src/utils.c
+++ b/src/utils.c
@@ -20,36 +20,33 @@
#include <linux/netfilter.h>
#include <linux/netfilter/nf_tables.h>

+static const char *const nft_family_str[NFPROTO_NUMPROTO] = {
+ [NFPROTO_INET] = "inet",
+ [NFPROTO_IPV4] = "ip",
+ [NFPROTO_ARP] = "arp",
+ [NFPROTO_BRIDGE] = "bridge",
+ [NFPROTO_IPV6] = "ip6",
+};
+
const char *nft_family2str(uint32_t family)
{
- switch (family) {
- case AF_INET:
- return "ip";
- case AF_INET6:
- return "ip6";
- case 1:
- return "inet";
- case AF_BRIDGE:
- return "bridge";
- case 3: /* NFPROTO_ARP */
- return "arp";
- default:
+ if (nft_family_str[family] == NULL)
return "unknown";
- }
+
+ return nft_family_str[family];
}

int nft_str2family(const char *family)
{
- if (strcmp(family, "ip") == 0)
- return AF_INET;
- else if (strcmp(family, "ip6") == 0)
- return AF_INET6;
- else if (strcmp(family, "inet") == 0)
- return 1;
- else if (strcmp(family, "bridge") == 0)
- return AF_BRIDGE;
- else if (strcmp(family, "arp") == 0)
- return 0;
+ int i;
+
+ for (i = 0; i < NFPROTO_NUMPROTO; i++) {
+ if (nft_family_str[i] == NULL)
+ continue;
+
+ if (strcmp(nft_family_str[i], family) == 0)
+ return i;
+ }

errno = EAFNOSUPPORT;
return -1;

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Pablo Neira Ayuso]

Post by Arturo Borrero Gonzalez
NFPROTO_ARP = 3 in kernel space.
We need the same value here in userspace in order to correctly communicate
with the kernel.
The failure solved by this patch made that {XML|JSON}-parsed tables of ARP
family unable to be directly injected into kernel.
To prevent future errors, this patch changes raw and AF_* values by the mathing
* nft_family2str()
* nft_str2family()

Applied, thanks.

BTW, it would be good to see a similar refactor in nft_verdict2str().
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Arturo Borrero Gonzalez]

Post by Pablo Neira Ayuso
BTW, it would be good to see a similar refactor in nft_verdict2str().

I don't see a clean way to do it, given some verdicts are negative
numbers (enum nft_verdicts in nf_tables.h).
We may end accessing a negative index, out of bounds of the array.

--=20
Arturo Borrero Gonz=C3=A1lez
--
To unsubscribe from this list: send the line "unsubscribe netfilter-dev=
el" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Pablo Neira Ayuso]

Post by Arturo Borrero Gonzalez

Post by Pablo Neira Ayuso
BTW, it would be good to see a similar refactor in nft_verdict2str().

I don't see a clean way to do it, given some verdicts are negative
numbers (enum nft_verdicts in nf_tables.h).
We may end accessing a negative index, out of bounds of the array.

I see, you mean:

enum nft_verdicts {
NFT_CONTINUE = -1,
NFT_BREAK = -2,
NFT_JUMP = -3,
NFT_GOTO = -4,
NFT_RETURN = -5,
};

You can add some function to shift the values:

#define nft_verdict_index(base) (base + 5)

... nft_verdict_array[] = {
[nft_verdict_index(NFT_RETURN)] = "return",
...
};
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Pablo Neira Ayuso]

Post by Pablo Neira Ayuso

Post by Arturo Borrero Gonzalez

Post by Pablo Neira Ayuso
BTW, it would be good to see a similar refactor in nft_verdict2str().

I don't see a clean way to do it, given some verdicts are negative
numbers (enum nft_verdicts in nf_tables.h).
We may end accessing a negative index, out of bounds of the array.

enum nft_verdicts {
NFT_CONTINUE = -1,
NFT_BREAK = -2,
NFT_JUMP = -3,
NFT_GOTO = -4,
NFT_RETURN = -5,
};
#define nft_verdict_index(base) (base + 5)

BTW, instead of 5, add:

#define NFT_VERDICT_BASE NFT_RETURN

and use it.

Post by Pablo Neira Ayuso
... nft_verdict_array[] = {
[nft_verdict_index(NFT_RETURN)] = "return",
...
};

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Arturo Borrero Gonzalez]

Post by Pablo Neira Ayuso

On Tue, Oct 21, 2014 at 10:53:19AM +0200, Arturo Borrero Gonzalez wr=

Post by Arturo Borrero Gonzalez

BTW, it would be good to see a similar refactor in nft_verdict2s=

tr().

Post by Pablo Neira Ayuso

Post by Arturo Borrero Gonzalez
I don't see a clean way to do it, given some verdicts are negative
numbers (enum nft_verdicts in nf_tables.h).
We may end accessing a negative index, out of bounds of the array.

enum nft_verdicts {
NFT_CONTINUE =3D -1,
NFT_BREAK =3D -2,
NFT_JUMP =3D -3,
NFT_GOTO =3D -4,
NFT_RETURN =3D -5,
};
#define nft_verdict_index(base) (base + 5)

#define NFT_VERDICT_BASE NFT_RETURN
and use it.

... nft_verdict_array[] =3D {
[nft_verdict_index(NFT_RETURN)] =3D "return",
...
};

Ok, following your idea, I end with something like this:

/*
* NF_DROP =3D 0
* NF_ACCEPT =3D 1
* NFT_JUMP =3D -3
* NFT_GOTO =3D -4
* NFT_RETURN =3D -5
*/

#define NFT_VERDICT_BASE -NFT_RETURN
#define nft_verdict_index(base) (base + NFT_VERDICT_BASE)
#define NFT_VERDICT_ARRAY_LEN nft_verdict_index(NF_ACCEPT)

static const char *const nft_verdict_str[NFT_VERDICT_ARRAY_LEN + 1] =3D=
{
[nft_verdict_index(NFT_RETURN)] =3D "return", /* 0 =
*/
[nft_verdict_index(NFT_GOTO)] =3D "goto", /* 1 =
*/
[nft_verdict_index(NFT_JUMP)] =3D "jump", /* 2 =
*/
[nft_verdict_index(NF_DROP)] =3D "drop", /* 5 =
*/
[nft_verdict_index(NF_ACCEPT)] =3D "accept", /* 6 =
*/
};

const char *nft_verdict2str(int verdict)
{
if (nft_verdict_str[nft_verdict_index(verdict)] =3D=3D NULL)
return "unknown";

return nft_verdict_str[nft_verdict_index(verdict)];
}

int nft_str2verdict(const char *verdict, int *verdict_num)
{
int i;

for (i =3D 0; i < NFT_VERDICT_ARRAY_LEN; i++) {
if (nft_verdict_str[i] =3D=3D NULL)
continue;

if (strcmp(nft_verdict_str[i], verdict) =3D=3D 0) {
*verdict_num =3D nft_verdict_index(i);
return 0;
}
}

return -1;
}

I think the current code (the switch based one) is better, don't you?
That array seems very error prone.
--=20
Arturo Borrero Gonz=C3=A1lez
--
To unsubscribe from this list: send the line "unsubscribe netfilter-dev=
el" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html