[PATCH nft v2 1/6] osf: add version fingerprint support

[PATCH nft v2 1/6] osf: add version fingerprint support

[Fernando Fernandez Mancera]

Add support for version fingerprint in "osf" expression. Example:

table ip foo {
	chain bar {
		type filter hook input priority filter; policy accept;
		osf ttl skip name "Linux"
		osf ttl skip name version "Linux:4.20"
	}
}

Signed-off-by: Fernando Fernandez Mancera <ffmancera@riseup.net>
---
v1: initial patch
v2: flags type is now u32
---
 include/expression.h                |  1 +
 include/linux/netfilter/nf_tables.h |  6 ++++++
 include/osf.h                       |  3 ++-
 src/netlink_delinearize.c           |  4 +++-
 src/netlink_linearize.c             |  1 +
 src/osf.c                           | 13 ++++++++++---
 src/parser_bison.y                  |  8 ++++++--
 7 files changed, 29 insertions(+), 7 deletions(-)

diff --git a/include/expression.h b/include/expression.h
index 6d72f64..6416ac0 100644
--- a/include/expression.h
+++ b/include/expression.h
@@ -350,6 +350,7 @@ struct expr {
 		struct {
 			/* EXPR_OSF */
 			uint8_t			ttl;
+			uint32_t		flags;
 		} osf;
 	};
 };
diff --git a/include/linux/netfilter/nf_tables.h b/include/linux/netfilter/nf_tables.h
index 37036be..09a7b9e 100644
--- a/include/linux/netfilter/nf_tables.h
+++ b/include/linux/netfilter/nf_tables.h
@@ -944,15 +944,21 @@ enum nft_socket_keys {
  *
  * @NFTA_OSF_DREG: destination register (NLA_U32: nft_registers)
  * @NFTA_OSF_TTL: Value of the TTL osf option (NLA_U8)
+ * @NFTA_OSF_FLAGS: flags (NLA_U32)
  */
 enum nft_osf_attributes {
 	NFTA_OSF_UNSPEC,
 	NFTA_OSF_DREG,
 	NFTA_OSF_TTL,
+	NFTA_OSF_FLAGS,
 	__NFTA_OSF_MAX
 };
 #define NFT_OSF_MAX		(__NFTA_OSF_MAX - 1)
 
+enum nft_osf_flags {
+	NFT_OSF_F_VERSION	= 1 << 0,	/* check fingerprint version */
+};
+
 /**
  * enum nft_ct_keys - nf_tables ct expression keys
  *
diff --git a/include/osf.h b/include/osf.h
index 23ea34d..8f6f584 100644
--- a/include/osf.h
+++ b/include/osf.h
@@ -1,7 +1,8 @@
 #ifndef NFTABLES_OSF_H
 #define NFTABLES_OSF_H
 
-struct expr *osf_expr_alloc(const struct location *loc, const uint8_t ttl);
+struct expr *osf_expr_alloc(const struct location *loc, const uint8_t ttl,
+			    const uint32_t flags);
 
 extern int nfnl_osf_load_fingerprints(struct netlink_ctx *ctx, int del);
 
diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c
index d0eaf5b..9a2d63d 100644
--- a/src/netlink_delinearize.c
+++ b/src/netlink_delinearize.c
@@ -655,10 +655,12 @@ static void netlink_parse_osf(struct netlink_parse_ctx *ctx,
 {
 	enum nft_registers dreg;
 	struct expr *expr;
+	uint32_t flags;
 	uint8_t ttl;
 
 	ttl = nftnl_expr_get_u8(nle, NFTNL_EXPR_OSF_TTL);
-	expr = osf_expr_alloc(loc, ttl);
+	flags = nftnl_expr_get_u32(nle, NFTNL_EXPR_OSF_FLAGS);
+	expr = osf_expr_alloc(loc, ttl, flags);
 
 	dreg = netlink_parse_register(nle, NFTNL_EXPR_OSF_DREG);
 	netlink_set_register(ctx, dreg, expr);
diff --git a/src/netlink_linearize.c b/src/netlink_linearize.c
index 61149bf..8df82d5 100644
--- a/src/netlink_linearize.c
+++ b/src/netlink_linearize.c
@@ -228,6 +228,7 @@ static void netlink_gen_osf(struct netlink_linearize_ctx *ctx,
 	nle = alloc_nft_expr("osf");
 	netlink_put_register(nle, NFTNL_EXPR_OSF_DREG, dreg);
 	nftnl_expr_set_u8(nle, NFTNL_EXPR_OSF_TTL, expr->osf.ttl);
+	nftnl_expr_set_u32(nle, NFTNL_EXPR_OSF_FLAGS, expr->osf.flags);
 	nftnl_rule_add_expr(ctx->nlr, nle);
 }
 
diff --git a/src/osf.c b/src/osf.c
index 9252934..b57fcfe 100644
--- a/src/osf.c
+++ b/src/osf.c
@@ -19,17 +19,22 @@ static void osf_expr_print(const struct expr *expr, struct output_ctx *octx)
 {
 	const char *ttl_str = osf_ttl_int_to_str(expr->osf.ttl);
 
-	nft_print(octx, "osf %sname", ttl_str);
+	if (expr->osf.flags & NFT_OSF_F_VERSION)
+		nft_print(octx, "osf %sname version", ttl_str);
+	else
+		nft_print(octx, "osf %sname", ttl_str);
 }
 
 static void osf_expr_clone(struct expr *new, const struct expr *expr)
 {
 	new->osf.ttl = expr->osf.ttl;
+	new->osf.flags = expr->osf.flags;
 }
 
 static bool osf_expr_cmp(const struct expr *e1, const struct expr *e2)
 {
-	return e1->osf.ttl == e2->osf.ttl;
+	return (e1->osf.ttl == e2->osf.ttl) &&
+	       (e1->osf.flags == e2->osf.flags);
 }
 
 const struct expr_ops osf_expr_ops = {
@@ -41,7 +46,8 @@ const struct expr_ops osf_expr_ops = {
 	.json		= osf_expr_json,
 };
 
-struct expr *osf_expr_alloc(const struct location *loc, const uint8_t ttl)
+struct expr *osf_expr_alloc(const struct location *loc, const uint8_t ttl,
+			    const uint32_t flags)
 {
 	unsigned int len = NFT_OSF_MAXGENRELEN * BITS_PER_BYTE;
 	const struct datatype *type = &string_type;
@@ -50,6 +56,7 @@ struct expr *osf_expr_alloc(const struct location *loc, const uint8_t ttl)
 	expr = expr_alloc(loc, EXPR_OSF, type,
 			  BYTEORDER_HOST_ENDIAN, len);
 	expr->osf.ttl = ttl;
+	expr->osf.flags = flags;
 
 	return expr;
 }
diff --git a/src/parser_bison.y b/src/parser_bison.y
index b20be3a..161f1a5 100644
--- a/src/parser_bison.y
+++ b/src/parser_bison.y
@@ -3190,9 +3190,13 @@ fib_tuple		:  	fib_flag	DOT	fib_tuple
 			|	fib_flag
 			;
 
-osf_expr		:	OSF	osf_ttl		NAME
+osf_expr		:	OSF	osf_ttl		NAME	HDRVERSION
 			{
-				$$ = osf_expr_alloc(&@$, $2);
+				$$ = osf_expr_alloc(&@$, $2, NFT_OSF_F_VERSION);
+			}
+			|	OSF	osf_ttl		NAME
+			{
+				$$ = osf_expr_alloc(&@$, $2, 0);
 			}
 			;
 
-- 
2.20.1

[PATCH nft v2 2/6] json: osf: add version json support

[Fernando Fernandez Mancera]

Signed-off-by: Fernando Fernandez Mancera <ffmancera@riseup.net>
---
v1: initial patch
v2: flags type is now u32
---
 doc/libnftables-json.adoc |  7 +++++-
 src/json.c                | 13 +++++++++++
 src/parser_json.c         | 48 ++++++++++++++++++++++++++++++++++++++-
 3 files changed, 66 insertions(+), 2 deletions(-)

diff --git a/doc/libnftables-json.adoc b/doc/libnftables-json.adoc
index dbe5ac3..6981c69 100644
--- a/doc/libnftables-json.adoc
+++ b/doc/libnftables-json.adoc
@@ -1302,11 +1302,16 @@ Construct a reference to packet's socket.
 ____
 *{ "osf": {
 	"key":* 'OSF_KEY'*,
-	"ttl":* 'OSF_TTL'
+	"ttl":* 'OSF_TTL'*,
+	"flags":* 'OSF_FLAGS'
 *}}*
 
 'OSF_KEY' := *"name"*
 'OSF_TTL' := *"loose"* | *"skip"*
+
+'OSF_FLAGS' := 'OSF_FLAG' | *[* 'OSF_FLAG_LIST' *]*
+'OSF_FLAG_LIST' := 'OSF_FLAG' [*,* 'OSF_FLAG_LIST' ]
+'OSF_FLAG' := *"version"*
 ____
 
 Perform OS fingerprinting. This expression is typically used in the LHS of a *match*
diff --git a/src/json.c b/src/json.c
index 276a3c0..a46188d 100644
--- a/src/json.c
+++ b/src/json.c
@@ -865,6 +865,7 @@ json_t *socket_expr_json(const struct expr *expr, struct output_ctx *octx)
 json_t *osf_expr_json(const struct expr *expr, struct output_ctx *octx)
 {
 	json_t *root = json_pack("{s:s}", "key", "name");
+	const char *osf_flags[] = { "version" }
 
 	switch (expr->osf.ttl) {
 	case 1:
@@ -875,6 +876,18 @@ json_t *osf_expr_json(const struct expr *expr, struct output_ctx *octx)
 		break;
 	}
 
+	if (expr->osf.flags) {
+		json_t *tmp = json_array();
+		unsigned int i;
+
+		for (i = 0; i < array_size(osf_flags); i++) {
+			if (osf.flags & (1 << i)) {
+				json_array_append_new(tmp, json_string(osf_flags[i]));
+			}
+		}
+		json_object_set_new(root, "flags", tmp);
+	}
+
 	return json_pack("{s:o}", "osf", root);
 }
 
diff --git a/src/parser_json.c b/src/parser_json.c
index 7b190bc..ae197f0 100644
--- a/src/parser_json.c
+++ b/src/parser_json.c
@@ -377,10 +377,26 @@ static struct expr *json_parse_meta_expr(struct json_ctx *ctx,
 	return meta_expr_alloc(int_loc, key);
 }
 
+static int osf_flag_parse(const char *name, int *flagval)
+{
+	const char *osf_flags[] = { "version" };
+	unsigned int i;
+
+	for (i = 0; i < array_size(osf_flags); i++) {
+		if (!strcmp(name, osf_flags[i])) {
+			*flagval |= (1 << i);
+			return 0;
+		}
+	}
+	return 1;
+}
+
 static struct expr *json_parse_osf_expr(struct json_ctx *ctx,
 					const char *type, json_t *root)
 {
+	json_t *flags, *value;
 	const char *key, *ttl;
+	uint8_t flagval = 0;
 	uint8_t ttlval = 0;
 
 	if (json_unpack_err(ctx, root, "{s:s}", "key", &key))
@@ -397,8 +413,38 @@ static struct expr *json_parse_osf_expr(struct json_ctx *ctx,
 		}
 	}
 
+	if (!json_unpack(root, "{s:o}", "flags", &flags)) {
+		const char *flag;
+
+		if (json_is_string(flags)) {
+			flag = json_string_value(flags);
+
+			if (osf_flag_parse(flag, &flagval)) {
+				json_error(ctx, "Invalidad osf flag '%s'.", flag);
+				return NULL;
+			}
+
+		} else if (!json_is_array) {
+			json_error(ctx, "Unexpected object type in osf flags tuple.");
+			return NULL;
+		}
+
+		json_array_foreach(flags, index, value) {
+			if (!json_is_string(value)) {
+				json_error(ctx, "Unexpected object type in osf flags array at index %zd.", index);
+				return NULL;
+			}
+			flag = json_string_value(value);
+
+			if (osf_flag_parse(flag, &flagval)) {
+				json_error(ctx, "Invalid osf flag '%s'.", flag);
+				return NULL;
+			}
+		}
+	}
+
 	if (!strcmp(key, "name"))
-		return osf_expr_alloc(int_loc, ttlval);
+		return osf_expr_alloc(int_loc, ttlval, flagval);
 
 	json_error(ctx, "Invalid osf key value.");
 	return NULL;
-- 
2.20.1

[PATCH nft v2 3/6] tests: py: add osf tests with versions

[Fernando Fernandez Mancera]

Signed-off-by: Fernando Fernandez Mancera <ffmancera@riseup.net>
---
v1: initial patch
v2: flags type is now u32
---
 tests/py/inet/osf.t         |  4 +++
 tests/py/inet/osf.t.json    | 16 +++++++++
 tests/py/inet/osf.t.payload | 66 +++++++++++++++++++++++++++++++++++++
 3 files changed, 86 insertions(+)

diff --git a/tests/py/inet/osf.t b/tests/py/inet/osf.t
index fd5d9ed..c8cac9b 100644
--- a/tests/py/inet/osf.t
+++ b/tests/py/inet/osf.t
@@ -7,8 +7,12 @@
 osf name "Linux";ok
 osf ttl loose name "Linux";ok
 osf ttl skip name "Linux";ok
+osf ttl skip name version "Linux:3.0";ok
+osf ttl skip name version "morethan:sixteenbytes";fail
 osf ttl nottl name "Linux";fail
 osf name "morethansixteenbytes";fail
 osf name ;fail
 osf name { "Windows", "MacOs" };ok
+osf name version { "Windows:XP", "MacOs:Sierra" };ok
 ct mark set osf name map { "Windows" : 0x00000001, "MacOs" : 0x00000002 };ok
+ct mark set osf name version map { "Windows:XP" : 0x00000003, "MacOs:Sierra" : 0x00000004 };ok
diff --git a/tests/py/inet/osf.t.json b/tests/py/inet/osf.t.json
index 452f302..82a2638 100644
--- a/tests/py/inet/osf.t.json
+++ b/tests/py/inet/osf.t.json
@@ -45,6 +45,22 @@
     }
 ]
 
+# osf name version "Linux:3.0"
+[
+    {
+        "match": {
+            "left": {
+                "osf": {
+                    "key": "name",
+                    "flags": "version"
+                }
+            },
+            "op": "==",
+            "right": "Linux:3.0"
+        }
+    }
+]
+
 # osf name { "Windows", "MacOs" }
 [
     {
diff --git a/tests/py/inet/osf.t.payload b/tests/py/inet/osf.t.payload
index 9b8f0bc..8f708f2 100644
--- a/tests/py/inet/osf.t.payload
+++ b/tests/py/inet/osf.t.payload
@@ -43,6 +43,21 @@ inet osfinet osfchain
   [ osf dreg 1 ]
   [ cmp eq reg 1 0x756e694c 0x00000078 0x00000000 0x00000000 ]
 
+# osf ttl skip name version "Linux:3.0"
+ip osfip osfchain
+  [ osf dreg 1 ]
+  [ cmp eq reg 1 0x756e694c 0x2e333a78 0x00000030 0x00000000 ]
+
+# osf ttl skip name version "Linux:3.0"
+ip6 osfip6 osfchain
+  [ osf dreg 1 ]
+  [ cmp eq reg 1 0x756e694c 0x2e333a78 0x00000030 0x00000000 ]
+
+# osf ttl skip name version "Linux:3.0"
+inet osfinet osfchain
+  [ osf dreg 1 ]
+  [ cmp eq reg 1 0x756e694c 0x2e333a78 0x00000030 0x00000000 ]
+
 # osf name { "Windows", "MacOs" }
 __set%d osfip 3 size 2
 __set%d osfip 0
@@ -67,6 +82,30 @@ inet osfinet osfchain
   [ osf dreg 1 ]
   [ lookup reg 1 set __set%d ]
 
+# osf name version { "Windows:XP", "MacOs:Sierra" }
+__set%d osfip 3 size 2
+__set%d osfip 0
+	element 646e6957 3a73776f 00005058 00000000  : 0 [end]	element 4f63614d 69533a73 61727265 00000000  : 0 [end]
+ip osfip osfchain
+  [ osf dreg 1 ]
+  [ lookup reg 1 set __set%d ]
+
+# osf name version { "Windows:XP", "MacOs:Sierra" }
+__set%d osfip6 3 size 2
+__set%d osfip6 0
+	element 646e6957 3a73776f 00005058 00000000  : 0 [end]	element 4f63614d 69533a73 61727265 00000000  : 0 [end]
+ip6 osfip6 osfchain
+  [ osf dreg 1 ]
+  [ lookup reg 1 set __set%d ]
+
+# osf name version { "Windows:XP", "MacOs:Sierra" }
+__set%d osfinet 3 size 2
+__set%d osfinet 0
+	element 646e6957 3a73776f 00005058 00000000  : 0 [end]	element 4f63614d 69533a73 61727265 00000000  : 0 [end]
+inet osfinet osfchain
+  [ osf dreg 1 ]
+  [ lookup reg 1 set __set%d ]
+
 # ct mark set osf name map { "Windows" : 0x00000001, "MacOs" : 0x00000002 }
 __map%d osfip b size 2
 __map%d osfip 0
@@ -93,3 +132,30 @@ inet osfinet osfchain
   [ osf dreg 1 ]
   [ lookup reg 1 set __map%d dreg 1 ]
   [ ct set mark with reg 1 ]
+
+# ct mark set osf name version map { "Windows:XP" : 0x00000003, "MacOs:Sierra" : 0x00000004 }
+__map%d osfip b size 2
+__map%d osfip 0
+	element 646e6957 3a73776f 00005058 00000000  : 00000003 0 [end]	element 4f63614d 69533a73 61727265 00000000  : 00000004 0 [end]
+ip osfip osfchain
+  [ osf dreg 1 ]
+  [ lookup reg 1 set __map%d dreg 1 ]
+  [ ct set mark with reg 1 ]
+
+# ct mark set osf name version map { "Windows:XP" : 0x00000003, "MacOs:Sierra" : 0x00000004 }
+__map%d osfip6 b size 2
+__map%d osfip6 0
+	element 646e6957 3a73776f 00005058 00000000  : 00000003 0 [end]	element 4f63614d 69533a73 61727265 00000000  : 00000004 0 [end]
+ip6 osfip6 osfchain
+  [ osf dreg 1 ]
+  [ lookup reg 1 set __map%d dreg 1 ]
+  [ ct set mark with reg 1 ]
+
+# ct mark set osf name version map { "Windows:XP" : 0x00000003, "MacOs:Sierra" : 0x00000004 }
+__map%d osfinet b size 2
+__map%d osfinet 0
+	element 646e6957 3a73776f 00005058 00000000  : 00000003 0 [end]	element 4f63614d 69533a73 61727265 00000000  : 00000004 0 [end]
+inet osfinet osfchain
+  [ osf dreg 1 ]
+  [ lookup reg 1 set __map%d dreg 1 ]
+  [ ct set mark with reg 1 ]
-- 
2.20.1

[PATCH nft v2 4/6] doc: add osf version option to man page

[Fernando Fernandez Mancera]

Signed-off-by: Fernando Fernandez Mancera <ffmancera@riseup.net>
---
v1: initial patch
v2: flags type is now u32
---
 doc/primary-expression.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/doc/primary-expression.txt b/doc/primary-expression.txt
index d819b24..a62ed00 100644
--- a/doc/primary-expression.txt
+++ b/doc/primary-expression.txt
@@ -219,6 +219,8 @@ and others) from packets with the SYN bit set.
 |ttl|
 Do TTL checks on the packet to determine the operating system.|
 string
+|version|
+Do OS version checks on the packet.|
 |name|
 Name of the OS signature to match. All signatures can be found at pf.os file.
 Use "unknown" for OS signatures that the expression could not detect.|
-- 
2.20.1

[PATCH nft v2 5/6] files: osf: update pf.os with newer OS fingerprints

[Fernando Fernandez Mancera]

After notice that some fingerprints are outdated we have updated the most common
of them.

Signed-off-by: Fernando Fernandez Mancera <ffmancera@riseup.net>
---
v1: initial patch
v2: flags type is now u32
---
 files/osf/pf.os | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/files/osf/pf.os b/files/osf/pf.os
index e285851..7612d76 100644
--- a/files/osf/pf.os
+++ b/files/osf/pf.os
@@ -233,6 +233,10 @@ S4:64:1:60:M*,S,T,N,W7:		Linux:2.6::Linux 2.6 (newer, 3)
 T4:64:1:60:M*,S,T,N,W7:		Linux:2.6::Linux 2.6 (newer, 4)
 
 S10:64:1:60:M*,S,T,N,W4:	Linux:3.0::Linux 3.0
+S10:64:1:60:M*,S,T,N,W6:	Linux:3.1::Linux 3.1
+S10:64:1:60:M*,S,T,N,W7:	Linux:3.4-3.10::Linux 3.4 - 3.10
+S20:64:1:60:M*,S,T,N,W7:	Linux:3.11-4.19::Linux 3.11 - 4.19
+S44:64:1:60:M*,S,T,N,W7:	Linux:4.20::Linux 4.20
 
 S3:64:1:60:M*,S,T,N,W1:		Linux:2.5::Linux 2.5 (sometimes 2.4)
 S4:64:1:60:M*,S,T,N,W1:		Linux:2.5-2.6::Linux 2.5/2.6
@@ -284,6 +288,8 @@ S22:64:1:52:M*,N,N,S,N,W0:	Linux:2.2:ts:Linux 2.2 w/o timestamps
 65535:64:1:60:M*,N,W1,N,N,T:	FreeBSD:4.7-4.11::FreeBSD 4.7-5.2
 65535:64:1:60:M*,N,W1,N,N,T:	FreeBSD:5.0-5.2::FreeBSD 4.7-5.2
 
+65535:64:1:60:M*,N,W6,S,T:	FreeBSD:9.0-12.0::FreeBSD 9.0 - 12.0
+
 # XXX need quirks support
 # 65535:64:1:60:M*,N,W0,N,N,T:Z:FreeBSD:5.1-5.4::5.1-current (1)
 # 65535:64:1:60:M*,N,W1,N,N,T:Z:FreeBSD:5.1-5.4::5.1-current (2)
-- 
2.20.1

[PATCH nft v2 6/6] files: pf.os: merge the signatures spllited by version

[Fernando Fernandez Mancera]

In order to be able to identify the OS version we need to merge the signatures
split by version. eg.

65535:64:1:60:M*,N,W1,N,N,T:	FreeBSD:4.7-4.11::FreeBSD 4.7-5.2
65535:64:1:60:M*,N,W1,N,N,T:	FreeBSD:5.0-5.2::FreeBSD 4.7-5.2

65535:64:1:60:M*,N,W1,N,N,T:	FreeBSD:4.7-5.2::FreeBSD 4.7-5.2

Signed-off-by: Fernando Fernandez Mancera <ffmancera@riseup.net>
---
v1: initial patch
v2: flags type is now u32
---
 files/osf/pf.os | 40 ++++++++++++++--------------------------
 1 file changed, 14 insertions(+), 26 deletions(-)

diff --git a/files/osf/pf.os b/files/osf/pf.os
index 7612d76..35cbb47 100644
--- a/files/osf/pf.os
+++ b/files/osf/pf.os
@@ -201,12 +201,9 @@
 45046:64:0:44:M*:		AIX:4.3::AIX 4.3
 16384:64:0:44:M512:		AIX:4.3:2-3:AIX 4.3.2 and earlier
 
-16384:64:0:60:M512,N,W%2,N,N,T:		AIX:4.3:3:AIX 4.3.3-5.2
-16384:64:0:60:M512,N,W%2,N,N,T:		AIX:5.1-5.2::AIX 4.3.3-5.2
-32768:64:0:60:M512,N,W%2,N,N,T:		AIX:4.3:3:AIX 4.3.3-5.2
-32768:64:0:60:M512,N,W%2,N,N,T:		AIX:5.1-5.2::AIX 4.3.3-5.2
-65535:64:0:60:M512,N,W%2,N,N,T:		AIX:4.3:3:AIX 4.3.3-5.2
-65535:64:0:60:M512,N,W%2,N,N,T:		AIX:5.1-5.2::AIX 4.3.3-5.2
+16384:64:0:60:M512,N,W%2,N,N,T:		AIX:4.3-5.2:3:AIX 4.3.3-5.2
+32768:64:0:60:M512,N,W%2,N,N,T:		AIX:4.3-5.2:3:AIX 4.3.3-5.2
+65535:64:0:60:M512,N,W%2,N,N,T:		AIX:4.3-5-2:3:AIX 4.3.3-5.2
 65535:64:0:64:M*,N,W1,N,N,T,N,N,S:	AIX:5.3:ML1:AIX 5.3 ML1
 
 # ----------------- Linux -------------------
@@ -224,8 +221,7 @@ S4:64:1:60:M1360,S,T,N,W0:	Linux:google::Linux (Google crawlbot)
 
 S2:64:1:60:M*,S,T,N,W0:		Linux:2.4::Linux 2.4 (big boy)
 S3:64:1:60:M*,S,T,N,W0:		Linux:2.4:.18-21:Linux 2.4.18 and newer
-S4:64:1:60:M*,S,T,N,W0:		Linux:2.4::Linux 2.4/2.6 <= 2.6.7
-S4:64:1:60:M*,S,T,N,W0:		Linux:2.6:.1-7:Linux 2.4/2.6 <= 2.6.7
+S4:64:1:60:M*,S,T,N,W0:		Linux:2.4/2.6::Linux 2.4/2.6 <= 2.6.7
 
 S4:64:1:60:M*,S,T,N,W5:		Linux:2.6::Linux 2.6 (newer, 1)
 S4:64:1:60:M*,S,T,N,W6:		Linux:2.6::Linux 2.6 (newer, 2)
@@ -271,9 +267,7 @@ S22:64:1:52:M*,N,N,S,N,W0:	Linux:2.2:ts:Linux 2.2 w/o timestamps
 
 # ----------------- FreeBSD -----------------
 
-16384:64:1:44:M*:		FreeBSD:2.0-2.2::FreeBSD 2.0-4.2
-16384:64:1:44:M*:		FreeBSD:3.0-3.5::FreeBSD 2.0-4.2
-16384:64:1:44:M*:		FreeBSD:4.0-4.2::FreeBSD 2.0-4.2
+16384:64:1:44:M*:		FreeBSD:2.0-4.2::FreeBSD 2.0-4.2
 16384:64:1:60:M*,N,W0,N,N,T:	FreeBSD:4.4::FreeBSD 4.4
 
 1024:64:1:60:M*,N,W0,N,N,T:	FreeBSD:4.4::FreeBSD 4.4
@@ -281,12 +275,9 @@ S22:64:1:52:M*,N,N,S,N,W0:	Linux:2.2:ts:Linux 2.2 w/o timestamps
 57344:64:1:44:M*:		FreeBSD:4.6-4.8:noRFC1323:FreeBSD 4.6-4.8 (no RFC1323)
 57344:64:1:60:M*,N,W0,N,N,T:	FreeBSD:4.6-4.9::FreeBSD 4.6-4.9
 
-32768:64:1:60:M*,N,W0,N,N,T:	FreeBSD:4.8-4.11::FreeBSD 4.8-5.1 (or MacOS X)
-32768:64:1:60:M*,N,W0,N,N,T:	FreeBSD:5.0-5.1::FreeBSD 4.8-5.1 (or MacOS X)
-65535:64:1:60:M*,N,W0,N,N,T:	FreeBSD:4.8-4.11::FreeBSD 4.8-5.2 (or MacOS X)
-65535:64:1:60:M*,N,W0,N,N,T:	FreeBSD:5.0-5.2::FreeBSD 4.8-5.2 (or MacOS X)
-65535:64:1:60:M*,N,W1,N,N,T:	FreeBSD:4.7-4.11::FreeBSD 4.7-5.2
-65535:64:1:60:M*,N,W1,N,N,T:	FreeBSD:5.0-5.2::FreeBSD 4.7-5.2
+32768:64:1:60:M*,N,W0,N,N,T:	FreeBSD:4.8-5.1::FreeBSD 4.8-5.1 (or MacOS X)
+65535:64:1:60:M*,N,W0,N,N,T:	FreeBSD:4.8-5.2::FreeBSD 4.8-5.2 (or MacOS X)
+65535:64:1:60:M*,N,W1,N,N,T:	FreeBSD:4.7-5.2::FreeBSD 4.7-5.2
 
 65535:64:1:60:M*,N,W6,S,T:	FreeBSD:9.0-12.0::FreeBSD 9.0 - 12.0
 
@@ -378,8 +369,7 @@ S34:64:1:52:M*,N,W0,N,N,S:		Solaris:10:beta:Solaris 10 (beta)
 # S2:255:1:48:M*,W0,E:.:MacOS:8.6 classic
 
 # XXX some of these use EOL too
-16616:255:1:48:M*,W0:			MacOS:7.3-7.6:OTTCP:MacOS 7.3-8.6 (OTTCP)
-16616:255:1:48:M*,W0:			MacOS:8.0-8.6:OTTCP:MacOS 7.3-8.6 (OTTCP)
+16616:255:1:48:M*,W0:			MacOS:7.3-8.6:OTTCP:MacOS 7.3-8.6 (OTTCP)
 16616:255:1:48:M*,N,N,N:		MacOS:8.1-8.6:OTTCP:MacOS 8.1-8.6 (OTTCP)
 32768:255:1:48:M*,W0,N:			MacOS:9.0-9.2::MacOS 9.0-9.2
 65535:255:1:48:M*,N,N,N,N:		MacOS:9.1::MacOS 9.1 (OT 2.7.4)
@@ -515,8 +505,7 @@ S8:64:0:44:M512:		NeXTSTEP:3.3::NeXTSTEP 3.3
 
 # ------------------ OS/400 -----------------
 
-8192:64:1:60:M1440,N,W0,N,N,T:	OS/400:VR4::OS/400 VR4/R5
-8192:64:1:60:M1440,N,W0,N,N,T:	OS/400:VR5::OS/400 VR4/R5
+8192:64:1:60:M1440,N,W0,N,N,T:	OS/400:VR4-VR5::OS/400 VR4/R5
 4096:64:1:60:M1440,N,W0,N,N,T:	OS/400:V4R5:CF67032:OS/400 V4R5 + CF67032
 
 # XXX quirk
@@ -532,9 +521,9 @@ S16:64:0:44:M512:		QNX:::QNX demodisk
 
 # ------------------ Novell -----------------
 
-16384:128:1:44:M1460:		Novell:NetWare:5.0:Novel Netware 5.0
-6144:128:1:44:M1460:		Novell:IntranetWare:4.11:Novell IntranetWare 4.11
-6144:128:1:44:M1368:		Novell:BorderManager::Novell BorderManager ?
+16384:128:1:44:M1460:		Novell:NW:5.0:Novel Netware 5.0
+6144:128:1:44:M1460:		Novell:IW:4.11:Novell IntranetWare 4.11
+6144:128:1:44:M1368:		Novell:BM::Novell BorderManager ?
 
 6144:128:1:52:M*,W0,N,S,N,N:	Novell:Netware:6:Novell Netware 6 SP3
 
@@ -637,8 +626,7 @@ S1:255:1:60:M1460,S,T,N,W0:		LookSmart:ZyBorg::LookSmart ZyBorg
 # ----------- Embedded systems --------------
 
 S9:255:0:44:M536:			PalmOS:Tungsten:C:PalmOS Tungsten C
-S5:255:0:44:M536:			PalmOS:3::PalmOS 3/4
-S5:255:0:44:M536:			PalmOS:4::PalmOS 3/4
+S5:255:0:44:M536:			PalmOS:3-4::PalmOS 3/4
 S4:255:0:44:M536:			PalmOS:3:5:PalmOS 3.5
 2948:255:0:44:M536:			PalmOS:3:5:PalmOS 3.5.3 (Handera)
 S29:255:0:44:M536:			PalmOS:5::PalmOS 5.0
-- 
2.20.1

Re: [PATCH nft v2 1/6] osf: add version fingerprint support

[Phil Sutter]

Hi Fernando,

On Mon, Mar 11, 2019 at 04:14:12PM +0100, Fernando Fernandez Mancera wrote:
> Add support for version fingerprint in "osf" expression. Example:
> 
> table ip foo {
> 	chain bar {
> 		type filter hook input priority filter; policy accept;
> 		osf ttl skip name "Linux"
> 		osf ttl skip name version "Linux:4.20"
> 	}
> }

The syntax seems overly complicated to me, although I'm not really
familiar with OSF so may lack background knowledge. Any reason why you
didn't go with 'osf ttl skip name "Linux" version "4.20"' instead?

Also with regards to your patch to json_parser, I guess you should
introduce an enum for flag values, something like:

| enum osf_flags {
| 	OSF_FLAG_INVALID = 0x0,
| 	OSF_FLAG_VERSION = 0x1
| };
| 
| const char *osf_flag_names[] = {
| 	[OSF_VERSION] = "version"
| };

What do you think?

Cheers, Phil

Re: [PATCH nft v2 1/6] osf: add version fingerprint support

[Fernando Fernandez Mancera]

Hi Phil,

On 3/13/19 10:44 AM, Phil Sutter wrote:
> Hi Fernando,
> 
> On Mon, Mar 11, 2019 at 04:14:12PM +0100, Fernando Fernandez Mancera wrote:
>> Add support for version fingerprint in "osf" expression. Example:
>>
>> table ip foo {
>> 	chain bar {
>> 		type filter hook input priority filter; policy accept;
>> 		osf ttl skip name "Linux"
>> 		osf ttl skip name version "Linux:4.20"
>> 	}
>> }
> 
> The syntax seems overly complicated to me, although I'm not really
> familiar with OSF so may lack background knowledge. Any reason why you
> didn't go with 'osf ttl skip name "Linux" version "4.20"' instead?
> 

You are right, 'osf ttl skip name "Linux" version "4.20"' was my first
thought but in compilation time the parser applies shift-reduce to the
expression.. I decided 'osf ttl skip name version "Linux:4.20"' to avoid
a complex workaround in the parser.

The fingerprints database syntax is "genre:version:subtype:details" so
the nft 'osf' expression syntax is like the original one.

> Also with regards to your patch to json_parser, I guess you should
> introduce an enum for flag values, something like:
> 
> | enum osf_flags {
> | 	OSF_FLAG_INVALID = 0x0,
> | 	OSF_FLAG_VERSION = 0x1
> | };
> | 
> | const char *osf_flag_names[] = {
> | 	[OSF_VERSION] = "version"
> | };
> 
> What do you think?
> 

This patch already introduces an enum for flags values, you can find it
below. Do you think we need another one? Sorry if I am misunderstanding
you. Thanks!

diff --git a/include/linux/netfilter/nf_tables.h
b/include/linux/netfilter/nf_tables.h
index 37036be..09a7b9e 100644
--- a/include/linux/netfilter/nf_tables.h
+++ b/include/linux/netfilter/nf_tables.h
@@ -944,15 +944,21 @@ enum nft_socket_keys {
  *
  * @NFTA_OSF_DREG: destination register (NLA_U32: nft_registers)
  * @NFTA_OSF_TTL: Value of the TTL osf option (NLA_U8)
+ * @NFTA_OSF_FLAGS: flags (NLA_U32)
  */
 enum nft_osf_attributes {
 	NFTA_OSF_UNSPEC,
 	NFTA_OSF_DREG,
 	NFTA_OSF_TTL,
+	NFTA_OSF_FLAGS,
 	__NFTA_OSF_MAX
 };
 #define NFT_OSF_MAX		(__NFTA_OSF_MAX - 1)

+enum nft_osf_flags {
+	NFT_OSF_F_VERSION	= 1 << 0,	/* check fingerprint version */
+};
+
 /**
  * enum nft_ct_keys - nf_tables ct expression keys
  *

> Cheers, Phil
> 

Re: [PATCH nft v2 1/6] osf: add version fingerprint support

[Phil Sutter]

On Wed, Mar 13, 2019 at 11:14:04AM +0100, Fernando Fernandez Mancera wrote:
> Hi Phil,
> 
> On 3/13/19 10:44 AM, Phil Sutter wrote:
> > Hi Fernando,
> > 
> > On Mon, Mar 11, 2019 at 04:14:12PM +0100, Fernando Fernandez Mancera wrote:
> >> Add support for version fingerprint in "osf" expression. Example:
> >>
> >> table ip foo {
> >> 	chain bar {
> >> 		type filter hook input priority filter; policy accept;
> >> 		osf ttl skip name "Linux"
> >> 		osf ttl skip name version "Linux:4.20"
> >> 	}
> >> }
> > 
> > The syntax seems overly complicated to me, although I'm not really
> > familiar with OSF so may lack background knowledge. Any reason why you
> > didn't go with 'osf ttl skip name "Linux" version "4.20"' instead?
> > 
> 
> You are right, 'osf ttl skip name "Linux" version "4.20"' was my first
> thought but in compilation time the parser applies shift-reduce to the
> expression.. I decided 'osf ttl skip name version "Linux:4.20"' to avoid
> a complex workaround in the parser.

Shift/reduce warnings often require voodoo to fix, but it's not
impossible. :)

Regarding my suggestion, I see that this string is actually the
right-hand-side of a relational expression. To implement what I had in
mind you would have to turn osf expression into a statement.

> The fingerprints database syntax is "genre:version:subtype:details" so
> the nft 'osf' expression syntax is like the original one.

Can we deduce required flags from the given string on RHS? I.e. by
looking at the amount of semi-colons and the number of characters
between them? I'm assuming the syntax works like "genre::subtype" and
"genre:::details" to omit certain parts, is that correct?

> > Also with regards to your patch to json_parser, I guess you should
> > introduce an enum for flag values, something like:
> > 
> > | enum osf_flags {
> > | 	OSF_FLAG_INVALID = 0x0,
> > | 	OSF_FLAG_VERSION = 0x1
> > | };
> > | 
> > | const char *osf_flag_names[] = {
> > | 	[OSF_VERSION] = "version"
> > | };
> > 
> > What do you think?
> > 
> 
> This patch already introduces an enum for flags values, you can find it
> below. Do you think we need another one? Sorry if I am misunderstanding
> you. Thanks!

Oh, I missed that one. My concern is how the index of values in
osf_flags array defined in osf_expr_json() and osf_flag_parse() is
relevant although it doesn't seem so when only looking at the array
definition.

Cheers, Phil

Re: [PATCH nft v2 1/6] osf: add version fingerprint support

[Fernando Fernandez Mancera]

On 3/13/19 12:27 PM, Phil Sutter wrote:
> On Wed, Mar 13, 2019 at 11:14:04AM +0100, Fernando Fernandez Mancera wrote:
>> Hi Phil,
>>
>> On 3/13/19 10:44 AM, Phil Sutter wrote:
>>> Hi Fernando,
>>>
>>> On Mon, Mar 11, 2019 at 04:14:12PM +0100, Fernando Fernandez Mancera wrote:
>>>> Add support for version fingerprint in "osf" expression. Example:
>>>>
>>>> table ip foo {
>>>> 	chain bar {
>>>> 		type filter hook input priority filter; policy accept;
>>>> 		osf ttl skip name "Linux"
>>>> 		osf ttl skip name version "Linux:4.20"
>>>> 	}
>>>> }
>>>
>>> The syntax seems overly complicated to me, although I'm not really
>>> familiar with OSF so may lack background knowledge. Any reason why you
>>> didn't go with 'osf ttl skip name "Linux" version "4.20"' instead?
>>>
>>
>> You are right, 'osf ttl skip name "Linux" version "4.20"' was my first
>> thought but in compilation time the parser applies shift-reduce to the
>> expression.. I decided 'osf ttl skip name version "Linux:4.20"' to avoid
>> a complex workaround in the parser.
> 
> Shift/reduce warnings often require voodoo to fix, but it's not
> impossible. :)
> 
> Regarding my suggestion, I see that this string is actually the
> right-hand-side of a relational expression. To implement what I had in
> mind you would have to turn osf expression into a statement.
> 
>> The fingerprints database syntax is "genre:version:subtype:details" so
>> the nft 'osf' expression syntax is like the original one.
> 
> Can we deduce required flags from the given string on RHS? I.e. by
> looking at the amount of semi-colons and the number of characters
> between them? I'm assuming the syntax works like "genre::subtype" and
> "genre:::details" to omit certain parts, is that correct?
> 

Yes that is correct. We can do that if you think it is more suitable. Do
we all agree then?

>>> Also with regards to your patch to json_parser, I guess you should
>>> introduce an enum for flag values, something like:
>>>
>>> | enum osf_flags {
>>> | 	OSF_FLAG_INVALID = 0x0,
>>> | 	OSF_FLAG_VERSION = 0x1
>>> | };
>>> | 
>>> | const char *osf_flag_names[] = {
>>> | 	[OSF_VERSION] = "version"
>>> | };
>>>
>>> What do you think?
>>>
>>
>> This patch already introduces an enum for flags values, you can find it
>> below. Do you think we need another one? Sorry if I am misunderstanding
>> you. Thanks!
> 
> Oh, I missed that one. My concern is how the index of values in
> osf_flags array defined in osf_expr_json() and osf_flag_parse() is
> relevant although it doesn't seem so when only looking at the array
> definition.
> 

Yes, we can use it in osf_expr_json(). I am going to work in a v3 patch
series if it looks fine to you. Thanks!

> Cheers, Phil
> 

Re: [PATCH nft v2 1/6] osf: add version fingerprint support

[Phil Sutter]

Hi Fernando,

On Wed, Mar 13, 2019 at 03:15:51PM +0100, Fernando Fernandez Mancera wrote:
> On 3/13/19 12:27 PM, Phil Sutter wrote:
> > On Wed, Mar 13, 2019 at 11:14:04AM +0100, Fernando Fernandez Mancera wrote:
> >> Hi Phil,
> >>
> >> On 3/13/19 10:44 AM, Phil Sutter wrote:
> >>> Hi Fernando,
> >>>
> >>> On Mon, Mar 11, 2019 at 04:14:12PM +0100, Fernando Fernandez Mancera wrote:
> >>>> Add support for version fingerprint in "osf" expression. Example:
> >>>>
> >>>> table ip foo {
> >>>> 	chain bar {
> >>>> 		type filter hook input priority filter; policy accept;
> >>>> 		osf ttl skip name "Linux"
> >>>> 		osf ttl skip name version "Linux:4.20"
> >>>> 	}
> >>>> }
> >>>
> >>> The syntax seems overly complicated to me, although I'm not really
> >>> familiar with OSF so may lack background knowledge. Any reason why you
> >>> didn't go with 'osf ttl skip name "Linux" version "4.20"' instead?
> >>>
> >>
> >> You are right, 'osf ttl skip name "Linux" version "4.20"' was my first
> >> thought but in compilation time the parser applies shift-reduce to the
> >> expression.. I decided 'osf ttl skip name version "Linux:4.20"' to avoid
> >> a complex workaround in the parser.
> > 
> > Shift/reduce warnings often require voodoo to fix, but it's not
> > impossible. :)
> > 
> > Regarding my suggestion, I see that this string is actually the
> > right-hand-side of a relational expression. To implement what I had in
> > mind you would have to turn osf expression into a statement.
> > 
> >> The fingerprints database syntax is "genre:version:subtype:details" so
> >> the nft 'osf' expression syntax is like the original one.
> > 
> > Can we deduce required flags from the given string on RHS? I.e. by
> > looking at the amount of semi-colons and the number of characters
> > between them? I'm assuming the syntax works like "genre::subtype" and
> > "genre:::details" to omit certain parts, is that correct?
> > 
> 
> Yes that is correct. We can do that if you think it is more suitable. Do
> we all agree then?

I think reducing redundancy is always a good thing. Only having to
specify the string and extracting the required info from it would make
it easier for users I guess.

That whole string is sent to the kernel, right? So it wouldn't make
sense to split the fields it is made up from into separate properties in
JSON, correct?

Thanks, Phil

Re: [PATCH nft v2 1/6] osf: add version fingerprint support

[Fernando Fernandez Mancera]

Hi Phil,

On 3/13/19 4:06 PM, Phil Sutter wrote:
> Hi Fernando,
> 
> On Wed, Mar 13, 2019 at 03:15:51PM +0100, Fernando Fernandez Mancera wrote:
>> On 3/13/19 12:27 PM, Phil Sutter wrote:
>>> On Wed, Mar 13, 2019 at 11:14:04AM +0100, Fernando Fernandez Mancera wrote:
>>>> Hi Phil,
>>>>
>>>> On 3/13/19 10:44 AM, Phil Sutter wrote:
>>>>> Hi Fernando,
>>>>>
>>>>> On Mon, Mar 11, 2019 at 04:14:12PM +0100, Fernando Fernandez Mancera wrote:
>>>>>> Add support for version fingerprint in "osf" expression. Example:
>>>>>>
>>>>>> table ip foo {
>>>>>> 	chain bar {
>>>>>> 		type filter hook input priority filter; policy accept;
>>>>>> 		osf ttl skip name "Linux"
>>>>>> 		osf ttl skip name version "Linux:4.20"
>>>>>> 	}
>>>>>> }
>>>>>
>>>>> The syntax seems overly complicated to me, although I'm not really
>>>>> familiar with OSF so may lack background knowledge. Any reason why you
>>>>> didn't go with 'osf ttl skip name "Linux" version "4.20"' instead?
>>>>>
>>>>
>>>> You are right, 'osf ttl skip name "Linux" version "4.20"' was my first
>>>> thought but in compilation time the parser applies shift-reduce to the
>>>> expression.. I decided 'osf ttl skip name version "Linux:4.20"' to avoid
>>>> a complex workaround in the parser.
>>>
>>> Shift/reduce warnings often require voodoo to fix, but it's not
>>> impossible. :)
>>>
>>> Regarding my suggestion, I see that this string is actually the
>>> right-hand-side of a relational expression. To implement what I had in
>>> mind you would have to turn osf expression into a statement.
>>>
>>>> The fingerprints database syntax is "genre:version:subtype:details" so
>>>> the nft 'osf' expression syntax is like the original one.
>>>
>>> Can we deduce required flags from the given string on RHS? I.e. by
>>> looking at the amount of semi-colons and the number of characters
>>> between them? I'm assuming the syntax works like "genre::subtype" and
>>> "genre:::details" to omit certain parts, is that correct?
>>>
>>
>> Yes that is correct. We can do that if you think it is more suitable. Do
>> we all agree then?
> 
> I think reducing redundancy is always a good thing. Only having to
> specify the string and extracting the required info from it would make
> it easier for users I guess.
> 
> That whole string is sent to the kernel, right? So it wouldn't make
> sense to split the fields it is made up from into separate properties in
> JSON, correct?
> 
> Thanks, Phil
> 

Yes, that makes sense. In this case, we don't need flags support anymore
so it reduces the patch series. Should we continue with the
implementation of the flags support or just forget about it until needed
again?

Re: [PATCH nft v2 1/6] osf: add version fingerprint support

[Phil Sutter]

On Wed, Mar 13, 2019 at 04:22:27PM +0100, Fernando Fernandez Mancera wrote:
> On 3/13/19 4:06 PM, Phil Sutter wrote:
> > Hi Fernando,
> > 
> > On Wed, Mar 13, 2019 at 03:15:51PM +0100, Fernando Fernandez Mancera wrote:
> >> On 3/13/19 12:27 PM, Phil Sutter wrote:
> >>> On Wed, Mar 13, 2019 at 11:14:04AM +0100, Fernando Fernandez Mancera wrote:
> >>>> Hi Phil,
> >>>>
> >>>> On 3/13/19 10:44 AM, Phil Sutter wrote:
> >>>>> Hi Fernando,
> >>>>>
> >>>>> On Mon, Mar 11, 2019 at 04:14:12PM +0100, Fernando Fernandez Mancera wrote:
> >>>>>> Add support for version fingerprint in "osf" expression. Example:
> >>>>>>
> >>>>>> table ip foo {
> >>>>>> 	chain bar {
> >>>>>> 		type filter hook input priority filter; policy accept;
> >>>>>> 		osf ttl skip name "Linux"
> >>>>>> 		osf ttl skip name version "Linux:4.20"
> >>>>>> 	}
> >>>>>> }
> >>>>>
> >>>>> The syntax seems overly complicated to me, although I'm not really
> >>>>> familiar with OSF so may lack background knowledge. Any reason why you
> >>>>> didn't go with 'osf ttl skip name "Linux" version "4.20"' instead?
> >>>>>
> >>>>
> >>>> You are right, 'osf ttl skip name "Linux" version "4.20"' was my first
> >>>> thought but in compilation time the parser applies shift-reduce to the
> >>>> expression.. I decided 'osf ttl skip name version "Linux:4.20"' to avoid
> >>>> a complex workaround in the parser.
> >>>
> >>> Shift/reduce warnings often require voodoo to fix, but it's not
> >>> impossible. :)
> >>>
> >>> Regarding my suggestion, I see that this string is actually the
> >>> right-hand-side of a relational expression. To implement what I had in
> >>> mind you would have to turn osf expression into a statement.
> >>>
> >>>> The fingerprints database syntax is "genre:version:subtype:details" so
> >>>> the nft 'osf' expression syntax is like the original one.
> >>>
> >>> Can we deduce required flags from the given string on RHS? I.e. by
> >>> looking at the amount of semi-colons and the number of characters
> >>> between them? I'm assuming the syntax works like "genre::subtype" and
> >>> "genre:::details" to omit certain parts, is that correct?
> >>>
> >>
> >> Yes that is correct. We can do that if you think it is more suitable. Do
> >> we all agree then?
> > 
> > I think reducing redundancy is always a good thing. Only having to
> > specify the string and extracting the required info from it would make
> > it easier for users I guess.
> > 
> > That whole string is sent to the kernel, right? So it wouldn't make
> > sense to split the fields it is made up from into separate properties in
> > JSON, correct?
> > 
> > Thanks, Phil
> > 
> 
> Yes, that makes sense. In this case, we don't need flags support anymore
> so it reduces the patch series. Should we continue with the
> implementation of the flags support or just forget about it until needed
> again?

What other flags do you have in mind?

Cheers, Phil

Re: [PATCH nft v2 1/6] osf: add version fingerprint support

[Fernando Fernandez Mancera]

On 3/13/19 4:34 PM, Phil Sutter wrote:
> On Wed, Mar 13, 2019 at 04:22:27PM +0100, Fernando Fernandez Mancera wrote:
>> On 3/13/19 4:06 PM, Phil Sutter wrote:
>>> Hi Fernando,
>>>
>>> On Wed, Mar 13, 2019 at 03:15:51PM +0100, Fernando Fernandez Mancera wrote:
>>>> On 3/13/19 12:27 PM, Phil Sutter wrote:
>>>>> On Wed, Mar 13, 2019 at 11:14:04AM +0100, Fernando Fernandez Mancera wrote:
>>>>>> Hi Phil,
>>>>>>
>>>>>> On 3/13/19 10:44 AM, Phil Sutter wrote:
>>>>>>> Hi Fernando,
>>>>>>>
>>>>>>> On Mon, Mar 11, 2019 at 04:14:12PM +0100, Fernando Fernandez Mancera wrote:
>>>>>>>> Add support for version fingerprint in "osf" expression. Example:
>>>>>>>>
>>>>>>>> table ip foo {
>>>>>>>> 	chain bar {
>>>>>>>> 		type filter hook input priority filter; policy accept;
>>>>>>>> 		osf ttl skip name "Linux"
>>>>>>>> 		osf ttl skip name version "Linux:4.20"
>>>>>>>> 	}
>>>>>>>> }
>>>>>>>
>>>>>>> The syntax seems overly complicated to me, although I'm not really
>>>>>>> familiar with OSF so may lack background knowledge. Any reason why you
>>>>>>> didn't go with 'osf ttl skip name "Linux" version "4.20"' instead?
>>>>>>>
>>>>>>
>>>>>> You are right, 'osf ttl skip name "Linux" version "4.20"' was my first
>>>>>> thought but in compilation time the parser applies shift-reduce to the
>>>>>> expression.. I decided 'osf ttl skip name version "Linux:4.20"' to avoid
>>>>>> a complex workaround in the parser.
>>>>>
>>>>> Shift/reduce warnings often require voodoo to fix, but it's not
>>>>> impossible. :)
>>>>>
>>>>> Regarding my suggestion, I see that this string is actually the
>>>>> right-hand-side of a relational expression. To implement what I had in
>>>>> mind you would have to turn osf expression into a statement.
>>>>>
>>>>>> The fingerprints database syntax is "genre:version:subtype:details" so
>>>>>> the nft 'osf' expression syntax is like the original one.
>>>>>
>>>>> Can we deduce required flags from the given string on RHS? I.e. by
>>>>> looking at the amount of semi-colons and the number of characters
>>>>> between them? I'm assuming the syntax works like "genre::subtype" and
>>>>> "genre:::details" to omit certain parts, is that correct?
>>>>>
>>>>
>>>> Yes that is correct. We can do that if you think it is more suitable. Do
>>>> we all agree then?
>>>
>>> I think reducing redundancy is always a good thing. Only having to
>>> specify the string and extracting the required info from it would make
>>> it easier for users I guess.
>>>
>>> That whole string is sent to the kernel, right? So it wouldn't make
>>> sense to split the fields it is made up from into separate properties in
>>> JSON, correct?
>>>
>>> Thanks, Phil
>>>
>>
>> Yes, that makes sense. In this case, we don't need flags support anymore
>> so it reduces the patch series. Should we continue with the
>> implementation of the flags support or just forget about it until needed
>> again?
> 
> What other flags do you have in mind?
> 
> Cheers, Phil
> 

Maybe in the future we could need them for logging. But we can ignore it
right now.

Re: [PATCH nft v2 1/6] osf: add version fingerprint support

[Fernando Fernandez Mancera]

Hi,

I have been thinking more about this today. I don't know how access to
the right-hand-side string from the kernel if it is possible. Sorry if
the question is very dumb, but I may lack experience with the nft
registers and RHS data of an expression.

Thanks!

On 3/13/19 5:46 PM, Fernando Fernandez Mancera wrote:
> On 3/13/19 4:34 PM, Phil Sutter wrote:
>> On Wed, Mar 13, 2019 at 04:22:27PM +0100, Fernando Fernandez Mancera wrote:
>>> On 3/13/19 4:06 PM, Phil Sutter wrote:
>>>> Hi Fernando,
>>>>
>>>> On Wed, Mar 13, 2019 at 03:15:51PM +0100, Fernando Fernandez Mancera wrote:
>>>>> On 3/13/19 12:27 PM, Phil Sutter wrote:
>>>>>> On Wed, Mar 13, 2019 at 11:14:04AM +0100, Fernando Fernandez Mancera wrote:
>>>>>>> Hi Phil,
>>>>>>>
>>>>>>> On 3/13/19 10:44 AM, Phil Sutter wrote:
>>>>>>>> Hi Fernando,
>>>>>>>>
>>>>>>>> On Mon, Mar 11, 2019 at 04:14:12PM +0100, Fernando Fernandez Mancera wrote:
>>>>>>>>> Add support for version fingerprint in "osf" expression. Example:
>>>>>>>>>
>>>>>>>>> table ip foo {
>>>>>>>>> 	chain bar {
>>>>>>>>> 		type filter hook input priority filter; policy accept;
>>>>>>>>> 		osf ttl skip name "Linux"
>>>>>>>>> 		osf ttl skip name version "Linux:4.20"
>>>>>>>>> 	}
>>>>>>>>> }
>>>>>>>>
>>>>>>>> The syntax seems overly complicated to me, although I'm not really
>>>>>>>> familiar with OSF so may lack background knowledge. Any reason why you
>>>>>>>> didn't go with 'osf ttl skip name "Linux" version "4.20"' instead?
>>>>>>>>
>>>>>>>
>>>>>>> You are right, 'osf ttl skip name "Linux" version "4.20"' was my first
>>>>>>> thought but in compilation time the parser applies shift-reduce to the
>>>>>>> expression.. I decided 'osf ttl skip name version "Linux:4.20"' to avoid
>>>>>>> a complex workaround in the parser.
>>>>>>
>>>>>> Shift/reduce warnings often require voodoo to fix, but it's not
>>>>>> impossible. :)
>>>>>>
>>>>>> Regarding my suggestion, I see that this string is actually the
>>>>>> right-hand-side of a relational expression. To implement what I had in
>>>>>> mind you would have to turn osf expression into a statement.
>>>>>>
>>>>>>> The fingerprints database syntax is "genre:version:subtype:details" so
>>>>>>> the nft 'osf' expression syntax is like the original one.
>>>>>>
>>>>>> Can we deduce required flags from the given string on RHS? I.e. by
>>>>>> looking at the amount of semi-colons and the number of characters
>>>>>> between them? I'm assuming the syntax works like "genre::subtype" and
>>>>>> "genre:::details" to omit certain parts, is that correct?
>>>>>>
>>>>>
>>>>> Yes that is correct. We can do that if you think it is more suitable. Do
>>>>> we all agree then?
>>>>
>>>> I think reducing redundancy is always a good thing. Only having to
>>>> specify the string and extracting the required info from it would make
>>>> it easier for users I guess.
>>>>
>>>> That whole string is sent to the kernel, right? So it wouldn't make
>>>> sense to split the fields it is made up from into separate properties in
>>>> JSON, correct?
>>>>
>>>> Thanks, Phil
>>>>
>>>
>>> Yes, that makes sense. In this case, we don't need flags support anymore
>>> so it reduces the patch series. Should we continue with the
>>> implementation of the flags support or just forget about it until needed
>>> again?
>>
>> What other flags do you have in mind?
>>
>> Cheers, Phil
>>
> 
> Maybe in the future we could need them for logging. But we can ignore it
> right now.
> 

Re: [PATCH nft v2 1/6] osf: add version fingerprint support

[Pablo Neira Ayuso]

Hi,

On Thu, Mar 14, 2019 at 12:14:23PM +0100, Fernando Fernandez Mancera wrote:
> Hi,
> 
> I have been thinking more about this today. I don't know how access to
> the right-hand-side string from the kernel if it is possible. Sorry if
> the question is very dumb, but I may lack experience with the nft
> registers and RHS data of an expression.

I think you can hide flags from json, which is what Phil suggests, I
mean, just infer version flags from the syntax, ie. if
"genre::version" is used, then set of the version flag.

I think Phil is not suggesting kernel changes.

Re: [PATCH nft v2 1/6] osf: add version fingerprint support

[Phil Sutter]

Hi,

On Thu, Mar 14, 2019 at 02:58:40PM +0100, Pablo Neira Ayuso wrote:
> Hi,
> 
> On Thu, Mar 14, 2019 at 12:14:23PM +0100, Fernando Fernandez Mancera wrote:
> > Hi,
> > 
> > I have been thinking more about this today. I don't know how access to
> > the right-hand-side string from the kernel if it is possible. Sorry if
> > the question is very dumb, but I may lack experience with the nft
> > registers and RHS data of an expression.
> 
> I think you can hide flags from json, which is what Phil suggests, I
> mean, just infer version flags from the syntax, ie. if
> "genre::version" is used, then set of the version flag.
> 
> I think Phil is not suggesting kernel changes.

Actually I'm still in the process of understanding how all this works.
What I got so far is (correct me if I'm wrong): osf expr does the
fingerprinting and returns a string which relational expr compares to
right-hand side. This new version flag defines whether osf expr adds the
version to returned string or not.

Assuming the above is correct, my suggestion of making the flag option
implicit does not quite hold, at least not without painful
postprocessing of relational statement in userspace.

Right now this all seems to me like enabling multiple comparisons within
a single relational, i.e. one for genre and the other for version.
Nftables doesn't quite do such things. E.g. matching on two TCP header
fields requires two relationals, e.g. 'tcp dport 22 tcp sport > 1024'.
Internally then, these two statements may be combined into a single
payload match if suitable.

Applying the same logic to osf expression, we would implement 'osf name
foo osf version 3.141' and add 'osf_try_merge()' routine to
'rule_postprocess()' which tries to combine the two statements.
Obviously, this is quite a bit of extra work, not sure if feasible.

Cheers, Phil

Re: [PATCH nft v2 1/6] osf: add version fingerprint support

[Fernando Fernandez Mancera]

El 14 de marzo de 2019 18:34:54 CET, Phil Sutter <phil@nwl.cc> escribió:
>Hi,
>
>On Thu, Mar 14, 2019 at 02:58:40PM +0100, Pablo Neira Ayuso wrote:
>> Hi,
>> 
>> On Thu, Mar 14, 2019 at 12:14:23PM +0100, Fernando Fernandez Mancera
>wrote:
>> > Hi,
>> > 
>> > I have been thinking more about this today. I don't know how access
>to
>> > the right-hand-side string from the kernel if it is possible. Sorry
>if
>> > the question is very dumb, but I may lack experience with the nft
>> > registers and RHS data of an expression.
>> 
>> I think you can hide flags from json, which is what Phil suggests, I
>> mean, just infer version flags from the syntax, ie. if
>> "genre::version" is used, then set of the version flag.
>> 
>> I think Phil is not suggesting kernel changes.

That makes sense to me.

>
>Actually I'm still in the process of understanding how all this works.
>What I got so far is (correct me if I'm wrong): osf expr does the
>fingerprinting and returns a string which relational expr compares to
>right-hand side. This new version flag defines whether osf expr adds
>the
>version to returned string or not.
>

Yes that is correct.

>Assuming the above is correct, my suggestion of making the flag option
>implicit does not quite hold, at least not without painful
>postprocessing of relational statement in userspace.
>
>Right now this all seems to me like enabling multiple comparisons
>within
>a single relational, i.e. one for genre and the other for version.
>Nftables doesn't quite do such things. E.g. matching on two TCP header
>fields requires two relationals, e.g. 'tcp dport 22 tcp sport > 1024'.
>Internally then, these two statements may be combined into a single
>payload match if suitable.

I think in this case we can't do that. In my opinion it doesn't make sense to evaluate only the version without the OS genre. Do you agree? Thanks!

>
>Applying the same logic to osf expression, we would implement 'osf name
>foo osf version 3.141' and add 'osf_try_merge()' routine to
>'rule_postprocess()' which tries to combine the two statements.
>Obviously, this is quite a bit of extra work, not sure if feasible.
>
>Cheers, Phil

Re: [PATCH nft v2 1/6] osf: add version fingerprint support

[Phil Sutter]

Hi,

Batching messages here since I'm apparently too slow in replying. :)

On Thu, Mar 14, 2019 at 07:24:23PM +0100, Fernando Fernandez Mancera wrote:
> El 14 de marzo de 2019 18:34:54 CET, Phil Sutter <phil@nwl.cc> escribió:
> >On Thu, Mar 14, 2019 at 02:58:40PM +0100, Pablo Neira Ayuso wrote:
> >> On Thu, Mar 14, 2019 at 12:14:23PM +0100, Fernando Fernandez Mancera
> >wrote:
> >> > I have been thinking more about this today. I don't know how access
> >to
> >> > the right-hand-side string from the kernel if it is possible. Sorry
> >if
> >> > the question is very dumb, but I may lack experience with the nft
> >> > registers and RHS data of an expression.
> >> 
> >> I think you can hide flags from json, which is what Phil suggests, I
> >> mean, just infer version flags from the syntax, ie. if
> >> "genre::version" is used, then set of the version flag.
> >> 
> >> I think Phil is not suggesting kernel changes.
> 
> That makes sense to me.

In general, I think we should not deviate too much in both APIs. Also, a
bit more complicated syntax is less of a problem in JSON, while OTOH I
think spending a few extra cycles on keeping CLI syntax as simple as
possible is worth doing.

> >Assuming the above is correct, my suggestion of making the flag option
> >implicit does not quite hold, at least not without painful
> >postprocessing of relational statement in userspace.
> >
> >Right now this all seems to me like enabling multiple comparisons
> >within
> >a single relational, i.e. one for genre and the other for version.
> >Nftables doesn't quite do such things. E.g. matching on two TCP header
> >fields requires two relationals, e.g. 'tcp dport 22 tcp sport > 1024'.
> >Internally then, these two statements may be combined into a single
> >payload match if suitable.
> 
> I think in this case we can't do that. In my opinion it doesn't make sense to evaluate only the version without the OS genre. Do you agree? Thanks!

Well, I guess we could but the question is indeed if we want to. In
general, I tend to leave the decision what makes sense and what not to
the user. Although a bit sloppy, something like 'osf version "XP"' might
be a valuable shortcut for some.

On Thu, Mar 14, 2019 at 09:07:37PM +0100, Pablo Neira Ayuso wrote:
> On Thu, Mar 14, 2019 at 06:34:54PM +0100, Phil Sutter wrote:
[...]
> > Actually I'm still in the process of understanding how all this works.
> > What I got so far is (correct me if I'm wrong): osf expr does the
> > fingerprinting and returns a string which relational expr compares to
> > right-hand side. This new version flag defines whether osf expr adds the
> > version to returned string or not.
> > 
> > Assuming the above is correct, my suggestion of making the flag option
> > implicit does not quite hold, at least not without painful
> > postprocessing of relational statement in userspace.
> > 
> > Right now this all seems to me like enabling multiple comparisons within
> > a single relational, i.e. one for genre and the other for version.
> > Nftables doesn't quite do such things. E.g. matching on two TCP header
> > fields requires two relationals, e.g. 'tcp dport 22 tcp sport > 1024'.
> > Internally then, these two statements may be combined into a single
> > payload match if suitable.
> 
> The osf expression returns a string with the OS genre, and if the
> version flag is set on, it appends the version to this string, ie.
> genre + version.
> 
> This allows us to build maps, ie.
> 
>         meta mark set osf genre { "linux" : 0x10, "windows" : 0x20, "macos" : 0x40 }
> 
> But, with this new version, you could also do:
> 
>         meta mark set osf genre { "linux::4.0" : 0x11, "linux::3.0" : 0x12, ...}
> 
> and so on.
> 
> So I see this version thing as a extended matching.
> 
> The osf engine actually _already_ finds a precise matching, ie. genre
> + version, since the fingerprint is per genre + version. But you can
> just decide to match on the genre (eg. linux).

The problem we're facing IMO is that nft_cmp is limited to a simple
memcmp(). This demands LHS to know what RHS contains. I'm not implying
it would be a good idea, but imagine nft_cmp could handle wildcards, we
could use "linux:*" to match on genre only, "linux:4.0:*" to match on
genre and version and even "linux:4.*" to match genre and major version
number.

Actually we might be able to implement the above by setting 'len' field
correctly.

> > Applying the same logic to osf expression, we would implement 'osf name
> > foo osf version 3.141' and add 'osf_try_merge()' routine to
> > 'rule_postprocess()' which tries to combine the two statements.
> > Obviously, this is quite a bit of extra work, not sure if feasible.
> 
> I think the discussion here is the syntax, ie.
> 
>         osf genre "Linux::4.10"
> 
> vs.
> 
>         osf genre "Linux" version "4.10"
> 
> This only requires changes to the userspace nftables side, if you
> prefer this syntax, which is what I understand you would like to see,
> right?

Not quite. I like how osf is an expression, not a statement. This makes
things like 'osf name != "Linux"' possible. What I didn't like was how
the proposed extension requires users to input redundant info:

| osf name version "Linux:4.20"

RHS contains the version number, so LHS should not need to have
"version" explicitly stated.

On Thu, Mar 14, 2019 at 09:13:09PM +0100, Pablo Neira Ayuso wrote:
[...]
> I think we could even extend this later on to match things like:
> 
> # Popular cluster config scripts disable timestamps and
> # selective ACK:
> S4:64:1:48:M1460,N,W0:          Linux:2.4:cluster:Linux 2.4 in cluster
>                                 ^^^^^^^^^^^^^^^^^
> Then, do:
> 
>         os gente "Linux:2.4:cluster"
> 
> by adding a new flag to match the "Subtype" field (according to the
> file description in pf.os).

In an ideal world, we could match on any (combination of) fields in the
database. I am aware this is probably over-engineering. :)

What we could do though with little effort is to make use of the OS info
structure in database by making use of nft_cmp comparing only the first
'len' bytes of data in registers. My idea would be that:

* 'osf' expression always returns "full" data, i.e.: "OS:VER:SUB"
* nft_cmp compares that string to RHS up to RHS length

So let's assume DB lookup returns "Windows:2003:AS:", then:

osf name "Windows" -> match
osf name "Windows:" -> match
osf name "Windows:XP:" -> no match
osf name "Windows:2000:" -> no match
osf name "Windows:200" -> match

So we have optional version match and even a poor-man's wildcard
functionality. Specifying the trailing semi-colon implicitly causes an
exact match on the last field.

What do you think?

Cheers, Phil

Re: [PATCH nft v2 1/6] osf: add version fingerprint support

[Pablo Neira Ayuso]

On Fri, Mar 15, 2019 at 11:03:33AM +0100, Phil Sutter wrote:
[...]
> On Thu, Mar 14, 2019 at 09:07:37PM +0100, Pablo Neira Ayuso wrote:
> [...]
> > The osf expression returns a string with the OS genre, and if thev
> > version flag is set on, it appends the version to this string, ie.
> > genre + version.
> > 
> > This allows us to build maps, ie.
> > 
> >         meta mark set osf genre { "linux" : 0x10, "windows" : 0x20, "macos" : 0x40 }
> > 
> > But, with this new version, you could also do:
> > 
> >         meta mark set osf genre { "linux::4.0" : 0x11, "linux::3.0" : 0x12, ...}
> > 
> > and so on.
> > 
> > So I see this version thing as a extended matching.
> > 
> > The osf engine actually _already_ finds a precise matching, ie. genre
> > + version, since the fingerprint is per genre + version. But you can
> > just decide to match on the genre (eg. linux).
> 
> The problem we're facing IMO is that nft_cmp is limited to a simple
> memcmp(). This demands LHS to know what RHS contains. I'm not implying
> it would be a good idea, but imagine nft_cmp could handle wildcards, we
> could use "linux:*" to match on genre only, "linux:4.0:*" to match on
> genre and version and even "linux:4.*" to match genre and major version
> number.
>
> Actually we might be able to implement the above by setting 'len' field
> correctly.

The wildcard at the end of the string already works out of the box
via:

        iifname eth\*

The wildcard matching is generic, so it can be used from any string
datatype, including the osf expression.

> > > Applying the same logic to osf expression, we would implement 'osf name
> > > foo osf version 3.141' and add 'osf_try_merge()' routine to
> > > 'rule_postprocess()' which tries to combine the two statements.
> > > Obviously, this is quite a bit of extra work, not sure if feasible.
> > 
> > I think the discussion here is the syntax, ie.
> > 
> >         osf genre "Linux::4.10"
> > 
> > vs.
> > 
> >         osf genre "Linux" version "4.10"
> > 
> > This only requires changes to the userspace nftables side, if you
> > prefer this syntax, which is what I understand you would like to see,
> > right?
> 
> Not quite. I like how osf is an expression, not a statement. This makes
> things like 'osf name != "Linux"' possible. What I didn't like was how
> the proposed extension requires users to input redundant info:
> 
> | osf name version "Linux:4.20"
> 
> RHS contains the version number, so LHS should not need to have
> "version" explicitly stated.

I see, then part of your discussion is focused on this syntax:

        osf name version "Linux:4.20"

in order to remove the "version" keyword there and make it more
compact.

> On Thu, Mar 14, 2019 at 09:13:09PM +0100, Pablo Neira Ayuso wrote:
> [...]
> > I think we could even extend this later on to match things like:
> > 
> > # Popular cluster config scripts disable timestamps and
> > # selective ACK:
> > S4:64:1:48:M1460,N,W0:          Linux:2.4:cluster:Linux 2.4 in cluster
> >                                 ^^^^^^^^^^^^^^^^^
> > Then, do:
> > 
> >         os gente "Linux:2.4:cluster"
> > 
> > by adding a new flag to match the "Subtype" field (according to the
> > file description in pf.os).
> 
> In an ideal world, we could match on any (combination of) fields in the
> database. I am aware this is probably over-engineering. :)

We can probably achieve this with a more advance nft_cmp expression,
that allows us to do some sort of limited regex matching. But I agree
in that adding this only for the osf expression is probably too much.
I don't like regex, they will use it for layer-7, and users do not
understand the computational complexity of the regular expressions
(they can easily ruin performance by adding a few expression that need
to be search all over the packet payload).

Anyway, this is a different topic :-).

> What we could do though with little effort is to make use of the OS info
> structure in database by making use of nft_cmp comparing only the first
> 'len' bytes of data in registers. My idea would be that:
> 
> * 'osf' expression always returns "full" data, i.e.: "OS:VER:SUB"
> * nft_cmp compares that string to RHS up to RHS length
> 
> So let's assume DB lookup returns "Windows:2003:AS:", then:
> 
> osf name "Windows" -> match
> osf name "Windows:" -> match
> osf name "Windows:XP:" -> no match
> osf name "Windows:2000:" -> no match
> osf name "Windows:200" -> match
> 
> So we have optional version match and even a poor-man's wildcard
> functionality. Specifying the trailing semi-colon implicitly causes an
> exact match on the last field.
> 
> What do you think?

Hm, if we follow this path, this would need a bit more work, note
that:

* nft userspace currently compares 16 bytes for the string case,
  according to what I see via --debug=mnl.

* When the string is less than 16 bytes, it assumes it is a wildcard
  matching and the end of the string.

* Kernel would need to inconditionally build the OS:version string.

* We may need to ask users to break existing osf ruleset so they
  explicitly add the wildcard at the end, ie.

        osf name "Windows\*"
        osf name "Linux:4.\*"

And the kernel would have no notion of what userspace is willing to
match.

If the problem is the syntax, not the NFT_OSF_F_VERSION flags, we
could explore this syntax:

        osf genre "Linux"
        osf version "Linux:4.20"

then, in the future (if ever needed) add subtypes:

        osf subtype "Linux:2.4:cluster"

With flags in place, we would have a bit of knowledge of what the user
is doing vs. matching part of a string.

Note that this would still allow you to do wildcard matching, ie:

        osf version "Linux:4.\*"

Thanks!

Re: [PATCH nft v2 1/6] osf: add version fingerprint support

[Fernando Fernandez Mancera]

Hi,

On 3/15/19 6:13 PM, Pablo Neira Ayuso wrote:
> On Fri, Mar 15, 2019 at 11:03:33AM +0100, Phil Sutter wrote:
> [...]
>> On Thu, Mar 14, 2019 at 09:07:37PM +0100, Pablo Neira Ayuso wrote:
>> [...]
>>> The osf expression returns a string with the OS genre, and if thev
>>> version flag is set on, it appends the version to this string, ie.
>>> genre + version.
>>>
>>> This allows us to build maps, ie.
>>>
>>>         meta mark set osf genre { "linux" : 0x10, "windows" : 0x20, "macos" : 0x40 }
>>>
>>> But, with this new version, you could also do:
>>>
>>>         meta mark set osf genre { "linux::4.0" : 0x11, "linux::3.0" : 0x12, ...}
>>>
>>> and so on.
>>>
>>> So I see this version thing as a extended matching.
>>>
>>> The osf engine actually _already_ finds a precise matching, ie. genre
>>> + version, since the fingerprint is per genre + version. But you can
>>> just decide to match on the genre (eg. linux).
>>
>> The problem we're facing IMO is that nft_cmp is limited to a simple
>> memcmp(). This demands LHS to know what RHS contains. I'm not implying
>> it would be a good idea, but imagine nft_cmp could handle wildcards, we
>> could use "linux:*" to match on genre only, "linux:4.0:*" to match on
>> genre and version and even "linux:4.*" to match genre and major version
>> number.
>>
>> Actually we might be able to implement the above by setting 'len' field
>> correctly.
> 
> The wildcard at the end of the string already works out of the box
> via:
> 
>         iifname eth\*
> 
> The wildcard matching is generic, so it can be used from any string
> datatype, including the osf expression.
> 
>>>> Applying the same logic to osf expression, we would implement 'osf name
>>>> foo osf version 3.141' and add 'osf_try_merge()' routine to
>>>> 'rule_postprocess()' which tries to combine the two statements.
>>>> Obviously, this is quite a bit of extra work, not sure if feasible.
>>>
>>> I think the discussion here is the syntax, ie.
>>>
>>>         osf genre "Linux::4.10"
>>>
>>> vs.
>>>
>>>         osf genre "Linux" version "4.10"
>>>
>>> This only requires changes to the userspace nftables side, if you
>>> prefer this syntax, which is what I understand you would like to see,
>>> right?
>>
>> Not quite. I like how osf is an expression, not a statement. This makes
>> things like 'osf name != "Linux"' possible. What I didn't like was how
>> the proposed extension requires users to input redundant info:
>>
>> | osf name version "Linux:4.20"
>>
>> RHS contains the version number, so LHS should not need to have
>> "version" explicitly stated.
> 
> I see, then part of your discussion is focused on this syntax:
> 
>         osf name version "Linux:4.20"
> 
> in order to remove the "version" keyword there and make it more
> compact.
> 
>> On Thu, Mar 14, 2019 at 09:13:09PM +0100, Pablo Neira Ayuso wrote:
>> [...]
>>> I think we could even extend this later on to match things like:
>>>
>>> # Popular cluster config scripts disable timestamps and
>>> # selective ACK:
>>> S4:64:1:48:M1460,N,W0:          Linux:2.4:cluster:Linux 2.4 in cluster
>>>                                 ^^^^^^^^^^^^^^^^^
>>> Then, do:
>>>
>>>         os gente "Linux:2.4:cluster"
>>>
>>> by adding a new flag to match the "Subtype" field (according to the
>>> file description in pf.os).
>>
>> In an ideal world, we could match on any (combination of) fields in the
>> database. I am aware this is probably over-engineering. :)
> 
> We can probably achieve this with a more advance nft_cmp expression,
> that allows us to do some sort of limited regex matching. But I agree
> in that adding this only for the osf expression is probably too much.
> I don't like regex, they will use it for layer-7, and users do not
> understand the computational complexity of the regular expressions
> (they can easily ruin performance by adding a few expression that need
> to be search all over the packet payload).
> 
> Anyway, this is a different topic :-).

I agree with you about this. Using regex is too much in this case and
can ruin performance easily.

> 
>> What we could do though with little effort is to make use of the OS info
>> structure in database by making use of nft_cmp comparing only the first
>> 'len' bytes of data in registers. My idea would be that:
>>
>> * 'osf' expression always returns "full" data, i.e.: "OS:VER:SUB"
>> * nft_cmp compares that string to RHS up to RHS length
>>
>> So let's assume DB lookup returns "Windows:2003:AS:", then:
>>
>> osf name "Windows" -> match
>> osf name "Windows:" -> match
>> osf name "Windows:XP:" -> no match
>> osf name "Windows:2000:" -> no match
>> osf name "Windows:200" -> match
>>
>> So we have optional version match and even a poor-man's wildcard
>> functionality. Specifying the trailing semi-colon implicitly causes an
>> exact match on the last field.
>>
>> What do you think?
> 
> Hm, if we follow this path, this would need a bit more work, note
> that:
> 
> * nft userspace currently compares 16 bytes for the string case,
>   according to what I see via --debug=mnl.
> 
> * When the string is less than 16 bytes, it assumes it is a wildcard
>   matching and the end of the string.
> 
> * Kernel would need to inconditionally build the OS:version string.
> 
> * We may need to ask users to break existing osf ruleset so they
>   explicitly add the wildcard at the end, ie.
> 
>         osf name "Windows\*"
>         osf name "Linux:4.\*"
> 
> And the kernel would have no notion of what userspace is willing to
> match.

Following this path, IMO the users could have some problems with always
adding the wildcard at the end. I think that it is not natural to set
the wildcard at the end by default.

> 
> If the problem is the syntax, not the NFT_OSF_F_VERSION flags, we
> could explore this syntax:
> 
>         osf genre "Linux"
>         osf version "Linux:4.20"
> 
> then, in the future (if ever needed) add subtypes:
> 
>         osf subtype "Linux:2.4:cluster"
> 
> With flags in place, we would have a bit of knowledge of what the user
> is doing vs. matching part of a string.
> 
> Note that this would still allow you to do wildcard matching, ie:
> 
>         osf version "Linux:4.\*"
> 
> Thanks!
> 

I like more this solution, we can add an easily understandable
explanation about the usage of 'genre' and 'version' in the manual file.
Furthermore, this syntax is easy to extend if needed in the future as
Pablo said and it seems more natural to me.

We can hide the flags for the json support if needed by counting the
numbers of colons. Thanks! :-)

Re: [PATCH nft v2 1/6] osf: add version fingerprint support

[Pablo Neira Ayuso]

On Fri, Mar 15, 2019 at 09:21:56PM +0100, Fernando Fernandez Mancera wrote:
[...]
> We can hide the flags for the json support if needed by counting the
> numbers of colons.

No need to parse colons, we can map:

        osf genre
        osf version

to json, I think.

Re: [PATCH nft v2 1/6] osf: add version fingerprint support

[Fernando Fernandez Mancera]

On 3/16/19 10:05 AM, Pablo Neira Ayuso wrote:
> On Fri, Mar 15, 2019 at 09:21:56PM +0100, Fernando Fernandez Mancera wrote:
> [...]
>> We can hide the flags for the json support if needed by counting the
>> numbers of colons.
> 
> No need to parse colons, we can map:
> 
>         osf genre
>         osf version
> 
> to json, I think.
> 

Yes, you are right. Then, if everyone agree I can start to work on these
changes. Thanks! :-)

Re: [PATCH nft v2 1/6] osf: add version fingerprint support

[Phil Sutter]

Hi,

On Fri, Mar 15, 2019 at 06:13:28PM +0100, Pablo Neira Ayuso wrote:
> On Fri, Mar 15, 2019 at 11:03:33AM +0100, Phil Sutter wrote:
> [...]
> > On Thu, Mar 14, 2019 at 09:07:37PM +0100, Pablo Neira Ayuso wrote:
> > [...]
> > > The osf expression returns a string with the OS genre, and if thev
> > > version flag is set on, it appends the version to this string, ie.
> > > genre + version.
> > > 
> > > This allows us to build maps, ie.
> > > 
> > >         meta mark set osf genre { "linux" : 0x10, "windows" : 0x20, "macos" : 0x40 }
> > > 
> > > But, with this new version, you could also do:
> > > 
> > >         meta mark set osf genre { "linux::4.0" : 0x11, "linux::3.0" : 0x12, ...}
> > > 
> > > and so on.
> > > 
> > > So I see this version thing as a extended matching.
> > > 
> > > The osf engine actually _already_ finds a precise matching, ie. genre
> > > + version, since the fingerprint is per genre + version. But you can
> > > just decide to match on the genre (eg. linux).
> > 
> > The problem we're facing IMO is that nft_cmp is limited to a simple
> > memcmp(). This demands LHS to know what RHS contains. I'm not implying
> > it would be a good idea, but imagine nft_cmp could handle wildcards, we
> > could use "linux:*" to match on genre only, "linux:4.0:*" to match on
> > genre and version and even "linux:4.*" to match genre and major version
> > number.
> >
> > Actually we might be able to implement the above by setting 'len' field
> > correctly.
> 
> The wildcard at the end of the string already works out of the box
> via:
> 
>         iifname eth\*
> 
> The wildcard matching is generic, so it can be used from any string
> datatype, including the osf expression.

For osf expression, I guess we would want to add it implicitly in
userspace.

> > > > Applying the same logic to osf expression, we would implement 'osf name
> > > > foo osf version 3.141' and add 'osf_try_merge()' routine to
> > > > 'rule_postprocess()' which tries to combine the two statements.
> > > > Obviously, this is quite a bit of extra work, not sure if feasible.
> > > 
> > > I think the discussion here is the syntax, ie.
> > > 
> > >         osf genre "Linux::4.10"
> > > 
> > > vs.
> > > 
> > >         osf genre "Linux" version "4.10"
> > > 
> > > This only requires changes to the userspace nftables side, if you
> > > prefer this syntax, which is what I understand you would like to see,
> > > right?
> > 
> > Not quite. I like how osf is an expression, not a statement. This makes
> > things like 'osf name != "Linux"' possible. What I didn't like was how
> > the proposed extension requires users to input redundant info:
> > 
> > | osf name version "Linux:4.20"
> > 
> > RHS contains the version number, so LHS should not need to have
> > "version" explicitly stated.
> 
> I see, then part of your discussion is focused on this syntax:
> 
>         osf name version "Linux:4.20"
> 
> in order to remove the "version" keyword there and make it more
> compact.

I really don't think it's needed. The only reason for it I can see is
supporting a use-case where users pass "Linux:4.20" on RHS but actually
don't want to match on version, but they could just omit it in the first
place then.

> > On Thu, Mar 14, 2019 at 09:13:09PM +0100, Pablo Neira Ayuso wrote:
> > [...]
> > > I think we could even extend this later on to match things like:
> > > 
> > > # Popular cluster config scripts disable timestamps and
> > > # selective ACK:
> > > S4:64:1:48:M1460,N,W0:          Linux:2.4:cluster:Linux 2.4 in cluster
> > >                                 ^^^^^^^^^^^^^^^^^
> > > Then, do:
> > > 
> > >         os gente "Linux:2.4:cluster"
> > > 
> > > by adding a new flag to match the "Subtype" field (according to the
> > > file description in pf.os).
> > 
> > In an ideal world, we could match on any (combination of) fields in the
> > database. I am aware this is probably over-engineering. :)
> 
> We can probably achieve this with a more advance nft_cmp expression,
> that allows us to do some sort of limited regex matching. But I agree
> in that adding this only for the osf expression is probably too much.
> I don't like regex, they will use it for layer-7, and users do not
> understand the computational complexity of the regular expressions
> (they can easily ruin performance by adding a few expression that need
> to be search all over the packet payload).
> 
> Anyway, this is a different topic :-).

Yes, no point in making this widely used expression more complicated
just for a case nobody uses eventually. :)

> > What we could do though with little effort is to make use of the OS info
> > structure in database by making use of nft_cmp comparing only the first
> > 'len' bytes of data in registers. My idea would be that:
> > 
> > * 'osf' expression always returns "full" data, i.e.: "OS:VER:SUB"
> > * nft_cmp compares that string to RHS up to RHS length
> > 
> > So let's assume DB lookup returns "Windows:2003:AS:", then:
> > 
> > osf name "Windows" -> match
> > osf name "Windows:" -> match
> > osf name "Windows:XP:" -> no match
> > osf name "Windows:2000:" -> no match
> > osf name "Windows:200" -> match
> > 
> > So we have optional version match and even a poor-man's wildcard
> > functionality. Specifying the trailing semi-colon implicitly causes an
> > exact match on the last field.
> > 
> > What do you think?
> 
> Hm, if we follow this path, this would need a bit more work, note
> that:
> 
> * nft userspace currently compares 16 bytes for the string case,
>   according to what I see via --debug=mnl.

This is because osf_expr_alloc() sets expr->len to NFT_OSF_MAXGENRELEN *
BITS_PER_BYTE (with NFT_OSF_MAXGENRELEN being defined to 16 in
include/linux/netfilter/nf_tables.h).

> * When the string is less than 16 bytes, it assumes it is a wildcard
>   matching and the end of the string.

I guess that's because RHS length is then smaller than LHS?

> * Kernel would need to inconditionally build the OS:version string.

Yes, exactly. nft_osf.c would return "OS:version:subtype:opt" and
nft_cmp.c will do the right thing if nft_cmp_expr->len is set to RHS
length.

> * We may need to ask users to break existing osf ruleset so they
>   explicitly add the wildcard at the end, ie.
> 
>         osf name "Windows\*"
>         osf name "Linux:4.\*"

Not if cmp expr len is set correctly.

> And the kernel would have no notion of what userspace is willing to
> match.

Yes, it boils down to a string comparison. Do you see a downside in
that?

> If the problem is the syntax, not the NFT_OSF_F_VERSION flags, we
> could explore this syntax:
> 
>         osf genre "Linux"
>         osf version "Linux:4.20"
> 
> then, in the future (if ever needed) add subtypes:
> 
>         osf subtype "Linux:2.4:cluster"

IMHO this is still redundant: RHS contains <something>:<something>, user
wants to match on OS and version. RHS contains <sth>:<sth>:<sth>, user
wants to match on OS, version and subtype.

> With flags in place, we would have a bit of knowledge of what the user
> is doing vs. matching part of a string.

Yes, but I don't see where this would be beneficial. Maybe in optimizing
redundant expressions? Like

| osf name "Linux" osf version "Linux:4.20"

but it's not a big deal. With my approach described above, userspace
could compare both RHS strings:

* if memcmp(str1, str2, min(len1, len2) != 0 rule will never match
* else keep only the expression with longer RHS.

Cheers, Phil

Re: [PATCH nft v2 1/6] osf: add version fingerprint support

[Pablo Neira Ayuso]

Hi Phil,

On Mon, Mar 18, 2019 at 06:42:43PM +0100, Phil Sutter wrote:
> On Fri, Mar 15, 2019 at 06:13:28PM +0100, Pablo Neira Ayuso wrote:
[...]
> > * Kernel would need to inconditionally build the OS:version string.
> 
> Yes, exactly. nft_osf.c would return "OS:version:subtype:opt" and
> nft_cmp.c will do the right thing if nft_cmp_expr->len is set to RHS
> length.

Problem are sets. They need a fixed size for the string, or we would
need to adjust the size to the largest string in the set. In case of
dynamic sets, this boils down to the maximum string length since we
don't know what the length of future element would be.

> > * We may need to ask users to break existing osf ruleset so they
> >   explicitly add the wildcard at the end, ie.
> > 
> >         osf name "Windows\*"
> >         osf name "Linux:4.\*"
> 
> Not if cmp expr len is set correctly.

cmp expr len is set correctly using the wildcard.

I think we should aim to make this work fine with sets and maps. The
wildcard cannot be used from there, where exact matches are needed. We
should encourage users to use osf with sets and maps.

> > then, in the future (if ever needed) add subtypes:
> > 
> >         osf subtype "Linux:2.4:cluster"
> 
> IMHO this is still redundant: RHS contains <something>:<something>, user
> wants to match on OS and version. RHS contains <sth>:<sth>:<sth>, user
> wants to match on OS, version and subtype.

This is not possible with dynamic sets, there the set may be empty so
we do not know how many colons will be used :-)

Re: [PATCH nft v2 1/6] osf: add version fingerprint support

[Phil Sutter]

On Tue, Mar 19, 2019 at 12:06:24PM +0100, Pablo Neira Ayuso wrote:
> Hi Phil,
> 
> On Mon, Mar 18, 2019 at 06:42:43PM +0100, Phil Sutter wrote:
> > On Fri, Mar 15, 2019 at 06:13:28PM +0100, Pablo Neira Ayuso wrote:
> [...]
> > > * Kernel would need to inconditionally build the OS:version string.
> > 
> > Yes, exactly. nft_osf.c would return "OS:version:subtype:opt" and
> > nft_cmp.c will do the right thing if nft_cmp_expr->len is set to RHS
> > length.
> 
> Problem are sets. They need a fixed size for the string, or we would
> need to adjust the size to the largest string in the set. In case of
> dynamic sets, this boils down to the maximum string length since we
> don't know what the length of future element would be.

I knew there's a rub! :)

> > > * We may need to ask users to break existing osf ruleset so they
> > >   explicitly add the wildcard at the end, ie.
> > > 
> > >         osf name "Windows\*"
> > >         osf name "Linux:4.\*"
> > 
> > Not if cmp expr len is set correctly.
> 
> cmp expr len is set correctly using the wildcard.
> 
> I think we should aim to make this work fine with sets and maps. The
> wildcard cannot be used from there, where exact matches are needed. We
> should encourage users to use osf with sets and maps.
> 
> > > then, in the future (if ever needed) add subtypes:
> > > 
> > >         osf subtype "Linux:2.4:cluster"
> > 
> > IMHO this is still redundant: RHS contains <something>:<something>, user
> > wants to match on OS and version. RHS contains <sth>:<sth>:<sth>, user
> > wants to match on OS, version and subtype.
> 
> This is not possible with dynamic sets, there the set may be empty so
> we do not know how many colons will be used :-)

Speaking of sets, it would be awesome if we could support something
like:

| osf genre { "Linux:2.4", "Windows" }

i.e., having elements with varying "granularity". Do you think that's
feasible? If so, how could we support that? Maybe using a concatenation
like:

| osf name . osf version { "Linux" . "2.4", "Windows" . "*" }

Cheers, Phil

Re: [PATCH nft v2 1/6] osf: add version fingerprint support

[Pablo Neira Ayuso]

On Wed, Mar 20, 2019 at 02:46:54PM +0100, Phil Sutter wrote:
[...]
> Speaking of sets, it would be awesome if we could support something
> like:
> 
> | osf genre { "Linux:2.4", "Windows" }

We can support something like this:

        osf version { "Linux:2.4", "Windows\*" }

where the wildcard specifies any Windows version.

We need a new set type though, to deal with strings.

> i.e., having elements with varying "granularity". Do you think that's
> feasible? If so, how could we support that? Maybe using a concatenation
> like:
> 
> | osf name . osf version { "Linux" . "2.4", "Windows" . "*" }

Not sure we want this level of granularity. Matching the version
without no OS genre does not look useful when I look at how the osf
engine works. I'd prefer we go for the first approach you have
mentioned here above.

Re: [PATCH nft v2 1/6] osf: add version fingerprint support

[Phil Sutter]

On Thu, Mar 21, 2019 at 09:32:45AM +0100, Pablo Neira Ayuso wrote:
> On Wed, Mar 20, 2019 at 02:46:54PM +0100, Phil Sutter wrote:
> [...]
> > Speaking of sets, it would be awesome if we could support something
> > like:
> > 
> > | osf genre { "Linux:2.4", "Windows" }
> 
> We can support something like this:
> 
>         osf version { "Linux:2.4", "Windows\*" }
> 
> where the wildcard specifies any Windows version.
> 
> We need a new set type though, to deal with strings.

Given that RHS for osf expression is a string in any option we discussed
so far this is inevitable, right?

> > i.e., having elements with varying "granularity". Do you think that's
> > feasible? If so, how could we support that? Maybe using a concatenation
> > like:
> > 
> > | osf name . osf version { "Linux" . "2.4", "Windows" . "*" }
> 
> Not sure we want this level of granularity. Matching the version
> without no OS genre does not look useful when I look at how the osf
> engine works. I'd prefer we go for the first approach you have
> mentioned here above.

I agree. It looks nice on paper, but without further optimization in
background it leads to multiple fingerprint lookups for the same
package.

Cheers, Phil

Re: [PATCH nft v2 1/6] osf: add version fingerprint support

[Pablo Neira Ayuso]

On Thu, Mar 21, 2019 at 12:15:15PM +0100, Phil Sutter wrote:
> On Thu, Mar 21, 2019 at 09:32:45AM +0100, Pablo Neira Ayuso wrote:
> > On Wed, Mar 20, 2019 at 02:46:54PM +0100, Phil Sutter wrote:
> > [...]
> > > Speaking of sets, it would be awesome if we could support something
> > > like:
> > > 
> > > | osf genre { "Linux:2.4", "Windows" }
> > 
> > We can support something like this:
> > 
> >         osf version { "Linux:2.4", "Windows\*" }
> > 
> > where the wildcard specifies any Windows version.
> > 
> > We need a new set type though, to deal with strings.
> 
> Given that RHS for osf expression is a string in any option we discussed
> so far this is inevitable, right?
> 
> > > i.e., having elements with varying "granularity". Do you think that's
> > > feasible? If so, how could we support that? Maybe using a concatenation
> > > like:
> > > 
> > > | osf name . osf version { "Linux" . "2.4", "Windows" . "*" }
> > 
> > Not sure we want this level of granularity. Matching the version
> > without no OS genre does not look useful when I look at how the osf
> > engine works. I'd prefer we go for the first approach you have
> > mentioned here above.
> 
> I agree. It looks nice on paper, but without further optimization in
> background it leads to multiple fingerprint lookups for the same
> package.

Not if we have a set that implements Aho-Corasick.

Re: [PATCH nft v2 1/6] osf: add version fingerprint support

[Phil Sutter]

On Thu, Mar 21, 2019 at 12:18:18PM +0100, Pablo Neira Ayuso wrote:
> On Thu, Mar 21, 2019 at 12:15:15PM +0100, Phil Sutter wrote:
> > On Thu, Mar 21, 2019 at 09:32:45AM +0100, Pablo Neira Ayuso wrote:
> > > On Wed, Mar 20, 2019 at 02:46:54PM +0100, Phil Sutter wrote:
> > > [...]
> > > > Speaking of sets, it would be awesome if we could support something
> > > > like:
> > > > 
> > > > | osf genre { "Linux:2.4", "Windows" }
> > > 
> > > We can support something like this:
> > > 
> > >         osf version { "Linux:2.4", "Windows\*" }
> > > 
> > > where the wildcard specifies any Windows version.
> > > 
> > > We need a new set type though, to deal with strings.
> > 
> > Given that RHS for osf expression is a string in any option we discussed
> > so far this is inevitable, right?
> > 
> > > > i.e., having elements with varying "granularity". Do you think that's
> > > > feasible? If so, how could we support that? Maybe using a concatenation
> > > > like:
> > > > 
> > > > | osf name . osf version { "Linux" . "2.4", "Windows" . "*" }
> > > 
> > > Not sure we want this level of granularity. Matching the version
> > > without no OS genre does not look useful when I look at how the osf
> > > engine works. I'd prefer we go for the first approach you have
> > > mentioned here above.
> > 
> > I agree. It looks nice on paper, but without further optimization in
> > background it leads to multiple fingerprint lookups for the same
> > package.
> 
> Not if we have a set that implements Aho-Corasick.

I was talking about the "osf name . osf version" part. If not optimized,
each osf expression would perform a database lookup and return different
parts of the fingerprint.

Or did I miss your point?

Cheers, Phil

Re: [PATCH nft v2 1/6] osf: add version fingerprint support

[Pablo Neira Ayuso]

Hi Phil,

On Thu, Mar 21, 2019 at 03:06:56PM +0100, Phil Sutter wrote:
[...]
> I was talking about the "osf name . osf version" part. If not optimized,
> each osf expression would perform a database lookup and return different
> parts of the fingerprint.
> 
> Or did I miss your point?

Oops, sorry. I misunderstood.

I don't think we should split this in two separated selectors.
Matching on 'osf version' alone itself does not make sense to me.

I think it's useful to match on:

* genre
* genre:version
* genre:version:subtype

anything else for this, I don't have a use case.

We can achieve something similar to what you suggest in your 'osf name
. osf version' example via:

        osf version {  "Linux:2.4", "Windows\*" }

Thanks!

Re: [PATCH nft v2 1/6] osf: add version fingerprint support

[Phil Sutter]

On Thu, Mar 21, 2019 at 05:57:11PM +0100, Pablo Neira Ayuso wrote:
> Hi Phil,
> 
> On Thu, Mar 21, 2019 at 03:06:56PM +0100, Phil Sutter wrote:
> [...]
> > I was talking about the "osf name . osf version" part. If not optimized,
> > each osf expression would perform a database lookup and return different
> > parts of the fingerprint.
> > 
> > Or did I miss your point?
> 
> Oops, sorry. I misunderstood.

OK, no worries!

> I don't think we should split this in two separated selectors.
> Matching on 'osf version' alone itself does not make sense to me.

I like how one may use concatenations with it, but I agree it's not
really useful by itself.

> I think it's useful to match on:
> 
> * genre
> * genre:version
> * genre:version:subtype
> 
> anything else for this, I don't have a use case.

Fine with me!

> We can achieve something similar to what you suggest in your 'osf name
> . osf version' example via:
> 
>         osf version {  "Linux:2.4", "Windows\*" }

Supporting strings in sets would be nice for ifname matches, too. :)

Cheers, Phil

Re: [PATCH nft v2 1/6] osf: add version fingerprint support

[Pablo Neira Ayuso]

On Thu, Mar 14, 2019 at 06:34:54PM +0100, Phil Sutter wrote:
> Hi,
> 
> On Thu, Mar 14, 2019 at 02:58:40PM +0100, Pablo Neira Ayuso wrote:
> > Hi,
> > 
> > On Thu, Mar 14, 2019 at 12:14:23PM +0100, Fernando Fernandez Mancera wrote:
> > > Hi,
> > > 
> > > I have been thinking more about this today. I don't know how access to
> > > the right-hand-side string from the kernel if it is possible. Sorry if
> > > the question is very dumb, but I may lack experience with the nft
> > > registers and RHS data of an expression.
> > 
> > I think you can hide flags from json, which is what Phil suggests, I
> > mean, just infer version flags from the syntax, ie. if
> > "genre::version" is used, then set of the version flag.
> > 
> > I think Phil is not suggesting kernel changes.
> 
> Actually I'm still in the process of understanding how all this works.
> What I got so far is (correct me if I'm wrong): osf expr does the
> fingerprinting and returns a string which relational expr compares to
> right-hand side. This new version flag defines whether osf expr adds the
> version to returned string or not.
> 
> Assuming the above is correct, my suggestion of making the flag option
> implicit does not quite hold, at least not without painful
> postprocessing of relational statement in userspace.
> 
> Right now this all seems to me like enabling multiple comparisons within
> a single relational, i.e. one for genre and the other for version.
> Nftables doesn't quite do such things. E.g. matching on two TCP header
> fields requires two relationals, e.g. 'tcp dport 22 tcp sport > 1024'.
> Internally then, these two statements may be combined into a single
> payload match if suitable.

The osf expression returns a string with the OS genre, and if the
version flag is set on, it appends the version to this string, ie.
genre + version.

This allows us to build maps, ie.

        meta mark set osf genre { "linux" : 0x10, "windows" : 0x20, "macos" : 0x40 }

But, with this new version, you could also do:

        meta mark set osf genre { "linux::4.0" : 0x11, "linux::3.0" : 0x12, ...}

and so on.

So I see this version thing as a extended matching.

The osf engine actually _already_ finds a precise matching, ie. genre
+ version, since the fingerprint is per genre + version. But you can
just decide to match on the genre (eg. linux).

> Applying the same logic to osf expression, we would implement 'osf name
> foo osf version 3.141' and add 'osf_try_merge()' routine to
> 'rule_postprocess()' which tries to combine the two statements.
> Obviously, this is quite a bit of extra work, not sure if feasible.

I think the discussion here is the syntax, ie.

        osf genre "Linux::4.10"

vs.

        osf genre "Linux" version "4.10"

This only requires changes to the userspace nftables side, if you
prefer this syntax, which is what I understand you would like to see,
right?

The use of the colon, which comes from the pf.os file:

512:64:0:44:M*:                 Linux:2.0:3x:Linux 2.0.3x
                                ^^^^^^^^^

Then, this allows us to match for "Linux:2.0".

Thanks!

Re: [PATCH nft v2 1/6] osf: add version fingerprint supportg

[Pablo Neira Ayuso]

On Thu, Mar 14, 2019 at 09:07:37PM +0100, Pablo Neira Ayuso wrote:
> On Thu, Mar 14, 2019 at 06:34:54PM +0100, Phil Sutter wrote:
> > Hi,
> > 
> > On Thu, Mar 14, 2019 at 02:58:40PM +0100, Pablo Neira Ayuso wrote:
> > > Hi,
> > > 
> > > On Thu, Mar 14, 2019 at 12:14:23PM +0100, Fernando Fernandez Mancera wrote:
> > > > Hi,
> > > > 
> > > > I have been thinking more about this today. I don't know how access to
> > > > the right-hand-side string from the kernel if it is possible. Sorry if
> > > > the question is very dumb, but I may lack experience with the nft
> > > > registers and RHS data of an expression.
> > > 
> > > I think you can hide flags from json, which is what Phil suggests, I
> > > mean, just infer version flags from the syntax, ie. if
> > > "genre::version" is used, then set of the version flag.
> > > 
> > > I think Phil is not suggesting kernel changes.
> > 
> > Actually I'm still in the process of understanding how all this works.
> > What I got so far is (correct me if I'm wrong): osf expr does the
> > fingerprinting and returns a string which relational expr compares to
> > right-hand side. This new version flag defines whether osf expr adds the
> > version to returned string or not.
> > 
> > Assuming the above is correct, my suggestion of making the flag option
> > implicit does not quite hold, at least not without painful
> > postprocessing of relational statement in userspace.
> > 
> > Right now this all seems to me like enabling multiple comparisons within
> > a single relational, i.e. one for genre and the other for version.
> > Nftables doesn't quite do such things. E.g. matching on two TCP header
> > fields requires two relationals, e.g. 'tcp dport 22 tcp sport > 1024'.
> > Internally then, these two statements may be combined into a single
> > payload match if suitable.
> 
> The osf expression returns a string with the OS genre, and if the
> version flag is set on, it appends the version to this string, ie.
> genre + version.
> 
> This allows us to build maps, ie.
> 
>         meta mark set osf genre { "linux" : 0x10, "windows" : 0x20, "macos" : 0x40 }
> 
> But, with this new version, you could also do:
> 
>         meta mark set osf genre { "linux::4.0" : 0x11, "linux::3.0" : 0x12, ...}
> 
> and so on.
> 
> So I see this version thing as a extended matching.
> 
> The osf engine actually _already_ finds a precise matching, ie. genre
> + version, since the fingerprint is per genre + version. But you can
> just decide to match on the genre (eg. linux).
> 
> > Applying the same logic to osf expression, we would implement 'osf name
> > foo osf version 3.141' and add 'osf_try_merge()' routine to
> > 'rule_postprocess()' which tries to combine the two statements.
> > Obviously, this is quite a bit of extra work, not sure if feasible.
> 
> I think the discussion here is the syntax, ie.
> 
>         osf genre "Linux::4.10"
> 
> vs.
> 
>         osf genre "Linux" version "4.10"
> 
> This only requires changes to the userspace nftables side, if you
> prefer this syntax, which is what I understand you would like to see,
> right?
> 
> The use of the colon, which comes from the pf.os file:
> 
> 512:64:0:44:M*:                 Linux:2.0:3x:Linux 2.0.3x
>                                 ^^^^^^^^^
> 
> Then, this allows us to match for "Linux:2.0".

I think we could even extend this later on to match things like:

# Popular cluster config scripts disable timestamps and
# selective ACK:
S4:64:1:48:M1460,N,W0:          Linux:2.4:cluster:Linux 2.4 in cluster
                                ^^^^^^^^^^^^^^^^^
Then, do:

        os gente "Linux:2.4:cluster"

by adding a new flag to match the "Subtype" field (according to the
file description in pf.os).

Thanks!