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

[Fernando Fernandez Mancera <ffmancera@riseup.net>]

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
>