Re: [PATCH 1/2 WIP nf-next] netfilter: implement Passive OS fingerprint module in nft_osf
Hi Fernando,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on nf-next/master]
url:
https://github.com/0day-ci/linux/commits/Fernando-Fernandez-Mancera/netfilter-implement-Passive-OS-fingerprint-module-in-nft_osf/20180712-203655
base: https://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf-next.git master
config: ia64-allmodconfig (attached as .config)
compiler: ia64-linux-gcc (GCC) 8.1.0
reproduce:
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=8.1.0 make.cross ARCH=ia64
Note: the
linux-review/Fernando-Fernandez-Mancera/netfilter-implement-Passive-OS-fingerprint-module-in-nft_osf/20180712-203655
HEAD da6169296e9d1fd467a0717ebcdd42305488927d builds fine.
It only hurts bisectibility.
All errors (new ones prefixed by >>):
In file included from include/linux/netfilter/nf_osf.h:1,
from net/netfilter/nft_osf.c:2:
>> include/uapi/linux/netfilter/nf_osf.h:66:24: error: 'MAX_IPOPTLEN'
>> undeclared here (not in a function); did you mean 'MAY_OPEN'?
struct nf_osf_opt opt[MAX_IPOPTLEN];
^~~~
MAY_OPEN
>> include/uapi/linux/netfilter/nf_osf.h:71:17: error: field 'ip' has
>> incomplete type
struct iphdr ip;
^~
>> include/uapi/linux/netfilter/nf_osf.h:72:18: error: field 'tcp' has
>> incomplete type
struct tcphdr tcp;
^~~
>> net/netfilter/nft_osf.c:20:47: error: 'NFTA_OSF_MAX' undeclared here (not in
>> a function); did you mean 'NFTA_OBJ_MAX'?
static const struct nla_policy nft_osf_policy[NFTA_OSF_MAX + 1] = {
^~~~
NFTA_OBJ_MAX
>> net/netfilter/nft_osf.c:21:3: error: 'NFTA_OSF_GENRE' undeclared here (not
>> in a function); did you mean 'NF_OSF_GENRE'?
[NFTA_OSF_GENRE] = { .type = NLA_STRING, .len = OSF_GENRE_SIZE },
^~
NF_OSF_GENRE
>> net/netfilter/nft_osf.c:21:3: error: array index in initializer not of
>> integer type
net/netfilter/nft_osf.c:21:3: note: (near initialization for
'nft_osf_policy')
>> net/netfilter/nft_osf.c:21:23: error: field name not in record or union
>> initializer
[NFTA_OSF_GENRE] = { .type = NLA_STRING, .len = OSF_GENRE_SIZE },
^
net/netfilter/nft_osf.c:21:23: note: (near initialization for
'nft_osf_policy')
net/netfilter/nft_osf.c:21:43: error: field name not in record or union
initializer
[NFTA_OSF_GENRE] = { .type = NLA_STRING, .len = OSF_GENRE_SIZE },
^
net/netfilter/nft_osf.c:21:43: note: (near initialization for
'nft_osf_policy')
>> net/netfilter/nft_osf.c:22:3: error: 'NFTA_OSF_FLAGS' undeclared here (not
>> in a function); did you mean 'NFTA_FIB_FLAGS'?
[NFTA_OSF_FLAGS] = { .type = NLA_U32 },
^~
NFTA_FIB_FLAGS
net/netfilter/nft_osf.c:22:3: error: array index in initializer not of
integer type
net/netfilter/nft_osf.c:22:3: note: (near initialization for
'nft_osf_policy')
net/netfilter/nft_osf.c:22:23: error: field name not in record or union
initializer
[NFTA_OSF_FLAGS] = { .type = NLA_U32 },
^
net/netfilter/nft_osf.c:22:23: note: (near initialization for
'nft_osf_policy')
>> net/netfilter/nft_osf.c:23:3: error: 'NFTA_OSF_LOGLEVEL' undeclared here
>> (not in a function); did you mean 'NFTA_LOG_LEVEL'?
[NFTA_OSF_LOGLEVEL] = { .type = NLA_U32 },
^
NFTA_LOG_LEVEL
net/netfilter/nft_osf.c:23:3: error: array index in initializer not of
integer type
net/netfilter/nft_osf.c:23:3: note: (near initialization for
'nft_osf_policy')
net/netfilter/nft_osf.c:23:26: error: field name not in record or union
initializer
[NFTA_OSF_LOGLEVEL] = { .type = NLA_U32 },
^
net/netfilter/nft_osf.c:23:26: note: (near initialization for
'nft_osf_policy')
>> net/netfilter/nft_osf.c:24:3: error: 'NFTA_OSF_TTL' undeclared here (not in
>> a function); did you mean 'NF_OSF_TTL'?
[NFTA_OSF_TTL] = { .type = NLA_U32 },
^~~~
NF_OSF_TTL
net/netfilter/nft_osf.c:24:3: error: array index in initializer not of
integer type
net/netfilter/nft_osf.c:24:3: note: (near initialization for
'nft_osf_policy')
net/netfilter/nft_osf.c:24:22: error: field name not in record or union
initializer
[NFTA_OSF_TTL] = { .type = NLA_U32 },
^
net/netfilter/nft_osf.c:24:22: note: (near initialization for
'nft_osf_policy')
net/netfilter/nft_osf.c:20:32: warning: 'nft_osf_policy' defined but not
used [-Wunused-variable]
static const struct nla_policy nft_osf_policy[NFTA_OSF_MAX + 1] = {
Re: [PATCH 1/2 WIP nf-next] netfilter: implement Passive OS fingerprint module in nft_osf
On 07/12/2018 12:53 PM, Florian Westphal wrote:
Fernando Fernandez Mancera wrote:
Add basic module functions into nft_osf.[ch] in order to start the
implementation of OSF module in nf_tables.
+struct nft_osf {
+ chargenre[OSF_GENRE_SIZE];
+ __u32 flags;
+ __u32 loglevel;
+ __u32 ttl;
+ __u32 len;
+};
48 bytes is quite a lot. Can this be compressed further?
e.g. len appears to be useless, and flags/loglevel/ttl
can probably be u8 or u16.
Agree, u8 should be enough for flags/loglevel/ttl.
+static const struct nla_policy nft_osf_policy[NFTA_OSF_MAX + 1] = {
+ [NFTA_OSF_GENRE]= { .type = NLA_STRING, .len = OSF_GENRE_SIZE },
This allows strlen() of OSF_GENRE_SIZE.
+ [NFTA_OSF_FLAGS]= { .type = NLA_U32 },
+ [NFTA_OSF_LOGLEVEL] = { .type = NLA_U32 },
+ [NFTA_OSF_TTL] = { .type = NLA_U32 },
+};
This looks ok.
+static int nft_osf_init(const struct nft_ctx *ctx,
+ const struct nft_expr *expr,
+ const struct nlattr * const tb[])
+{
+ struct nft_osf *priv = nft_expr_priv(expr);
+
+ if (tb[NFTA_OSF_GENRE] == NULL)
+ return -EINVAL;
+ nla_strlcpy(priv->genre, tb[NFTA_OSF_GENRE], OSF_GENRE_SIZE);
This then copies OSF_GENRE_SIZE - 1 (for \0).
So its either .len = OSF_GENRE_SIZE - 1,
or genre[OSF_GENRE_SIZE+1], or char *genre + nla_strdup().
+ priv->len = strlen(priv->genre);
I don't understand need for this.
Yes, I am thinking on getting "len" out.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2 WIP nf-next] netfilter: implement Passive OS fingerprint module in nft_osf
Fernando Fernandez Mancera wrote:
> Add basic module functions into nft_osf.[ch] in order to start the
> implementation of OSF module in nf_tables.
>
> +struct nft_osf {
> + chargenre[OSF_GENRE_SIZE];
> + __u32 flags;
> + __u32 loglevel;
> + __u32 ttl;
> + __u32 len;
> +};
48 bytes is quite a lot. Can this be compressed further?
e.g. len appears to be useless, and flags/loglevel/ttl
can probably be u8 or u16.
> +static const struct nla_policy nft_osf_policy[NFTA_OSF_MAX + 1] = {
> + [NFTA_OSF_GENRE]= { .type = NLA_STRING, .len = OSF_GENRE_SIZE },
This allows strlen() of OSF_GENRE_SIZE.
> + [NFTA_OSF_FLAGS]= { .type = NLA_U32 },
> + [NFTA_OSF_LOGLEVEL] = { .type = NLA_U32 },
> + [NFTA_OSF_TTL] = { .type = NLA_U32 },
> +};
This looks ok.
> +static int nft_osf_init(const struct nft_ctx *ctx,
> + const struct nft_expr *expr,
> + const struct nlattr * const tb[])
> +{
> + struct nft_osf *priv = nft_expr_priv(expr);
> +
> + if (tb[NFTA_OSF_GENRE] == NULL)
> + return -EINVAL;
> + nla_strlcpy(priv->genre, tb[NFTA_OSF_GENRE], OSF_GENRE_SIZE);
This then copies OSF_GENRE_SIZE - 1 (for \0).
So its either .len = OSF_GENRE_SIZE - 1,
or genre[OSF_GENRE_SIZE+1], or char *genre + nla_strdup().
> + priv->len = strlen(priv->genre);
I don't understand need for this.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2 WIP nf-next] netfilter: implement Passive OS fingerprint module in nft_osf
Add basic module functions into nft_osf.[ch] in order to start the
implementation of OSF module in nf_tables.
Signed-off-by: Fernando Fernandez Mancera
---
include/uapi/linux/netfilter/nf_osf.h | 7 ++
include/uapi/linux/netfilter/xt_osf.h | 6 +-
net/netfilter/Kconfig | 7 ++
net/netfilter/Makefile| 1 +
net/netfilter/nft_osf.c | 108 ++
5 files changed, 128 insertions(+), 1 deletion(-)
create mode 100644 net/netfilter/nft_osf.c
diff --git a/include/uapi/linux/netfilter/nf_osf.h
b/include/uapi/linux/netfilter/nf_osf.h
index 8f2f2f403183..79882b2f7f8e 100644
--- a/include/uapi/linux/netfilter/nf_osf.h
+++ b/include/uapi/linux/netfilter/nf_osf.h
@@ -16,9 +16,16 @@
#define NF_OSF_TTL_TRUE0 /* True ip and
fingerprint TTL comparison */
+/* Check if ip TTL is less than fingerprint one */
+#define NF_OSF_TTL_LESS1
+
/* Do not compare ip and fingerprint TTL at all */
#define NF_OSF_TTL_NOCHECK 2
+#define NF_OSF_FLAGMASK(NF_OSF_GENRE | \
+NF_OSF_TTL | \
+NF_OSF_LOG | \
+NF_OSF_INVERT)
/* Wildcard MSS (kind of).
* It is used to implement a state machine for the different wildcard values
* of the MSS and window sizes.
diff --git a/include/uapi/linux/netfilter/xt_osf.h
b/include/uapi/linux/netfilter/xt_osf.h
index 72956eceeb09..2f5d4e6d0434 100644
--- a/include/uapi/linux/netfilter/xt_osf.h
+++ b/include/uapi/linux/netfilter/xt_osf.h
@@ -37,8 +37,12 @@
#define XT_OSF_TTL_TRUENF_OSF_TTL_TRUE
#define XT_OSF_TTL_NOCHECK NF_OSF_TTL_NOCHECK
+#define XT_OSF_TTL_LESSNF_OSF_TTL_LESS
-#define XT_OSF_TTL_LESS1 /* Check if ip TTL is less than
fingerprint one */
+#define XT_OSF_FLAGMASK(XT_OSF_GENRE | \
+ XT_OSF_TTL | \
+ XT_OSF_LOG | \
+ XT_OSF_INVERT)
#define xt_osf_wc nf_osf_wc
#define xt_osf_opt nf_osf_opt
diff --git a/net/netfilter/Kconfig b/net/netfilter/Kconfig
index dbd7d1fad277..e630aac8a081 100644
--- a/net/netfilter/Kconfig
+++ b/net/netfilter/Kconfig
@@ -631,6 +631,13 @@ config NFT_SOCKET
This option allows matching for the presence or absence of a
corresponding socket and its attributes.
+config NFT_OSF
+ tristate "Netfilter nf_tables passive OS fingerprinting support"
+ depends on NETFILTER_ADVANCED && NETFILTER_NETLINK
+ select NF_OSF
+ help
+ This option allows matching packets from an specific OS.
+
if NF_TABLES_NETDEV
config NF_DUP_NETDEV
diff --git a/net/netfilter/Makefile b/net/netfilter/Makefile
index 9389e527..d15bd858ecef 100644
--- a/net/netfilter/Makefile
+++ b/net/netfilter/Makefile
@@ -104,6 +104,7 @@ obj-$(CONFIG_NFT_FIB_INET) += nft_fib_inet.o
obj-$(CONFIG_NFT_FIB_NETDEV) += nft_fib_netdev.o
obj-$(CONFIG_NF_OSF) += nf_osf.o
obj-$(CONFIG_NFT_SOCKET) += nft_socket.o
+obj-$(CONFIG_NFT_OSF) += nft_osf.o
# nf_tables netdev
obj-$(CONFIG_NFT_DUP_NETDEV) += nft_dup_netdev.o
diff --git a/net/netfilter/nft_osf.c b/net/netfilter/nft_osf.c
new file mode 100644
index ..30c503f2bf53
--- /dev/null
+++ b/net/netfilter/nft_osf.c
@@ -0,0 +1,108 @@
+#include
+#include
+
+#define OSF_GENRE_SIZE 32
+
+struct nft_osf {
+ chargenre[OSF_GENRE_SIZE];
+ __u32 flags;
+ __u32 loglevel;
+ __u32 ttl;
+ __u32 len;
+};
+
+/* placeholder function WIP */
+static inline bool match_packet(struct nft_osf *priv, struct sk_buff *skb)
+{
+ return 1;
+}
+
+static const struct nla_policy nft_osf_policy[NFTA_OSF_MAX + 1] = {
+ [NFTA_OSF_GENRE]= { .type = NLA_STRING, .len = OSF_GENRE_SIZE },
+ [NFTA_OSF_FLAGS]= { .type = NLA_U32 },
+ [NFTA_OSF_LOGLEVEL] = { .type = NLA_U32 },
+ [NFTA_OSF_TTL] = { .type = NLA_U32 },
+};
+
+static void nft_osf_eval(const struct nft_expr *expr, struct nft_regs *regs,
+const struct nft_pktinfo *pkt)
+{
+ struct nft_osf *priv = nft_expr_priv(expr);
+ struct sk_buff *skb = pkt->skb;
+
+ if (!match_packet(priv, skb))
+ regs->verdict.code = NFT_BREAK;
+}
+
+static int nft_osf_init(const struct nft_ctx *ctx,
+ const struct nft_expr *expr,
+ const struct nlattr * const tb[])
+{
+ struct nft_osf *priv = nft_expr_priv(expr);
+
+ if (tb[NFTA_OSF_GENRE] == NULL)
+ return -EINVAL;
+ nla_strlcpy(priv->genre, tb[NFTA_OSF_GENRE], OSF_GENRE_SIZE);
+ if (ntohl(nla_get_be32(tb[NFTA_OSF_FLAGS])) & ~NF_OSF_FLAGMASK)
+ return -EINVAL;
+ priv->flags = ntohl(nla_get_be32(tb[NFTA_OSF_FLAGS]));
+ if (n
