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

From: Fernando Fernandez Mancera <ffmancera@riseup.net>
To: Phil Sutter <phil@nwl.cc>, Pablo Neira Ayuso <pablo@netfilter.org>
Cc: netfilter-devel@vger.kernel.org
Subject: Re: [PATCH nft v2 1/6] osf: add version fingerprint support
Date: Thu, 14 Mar 2019 12:14:23 +0100	[thread overview]
Message-ID: <e61257f6-4281-d450-0f41-6891a8744e96@riseup.net> (raw)
In-Reply-To: <fa72537e-adf8-29fb-6b8b-ebe30cd9bfc6@riseup.net>

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.
> 