ulogd2 patch ping

ulogd2 patch ping

[Jeremy Sowden]

[-- Attachment #1: Type: text/plain, Size: 243 bytes --]

There is a ulogd2 patch of mine from the end of last that is still under
review in Patchwork:

  https://patchwork.ozlabs.org/project/netfilter-devel/patch/20221208222208.681865-1-jeremy@azazel.net/

It would be great to get a yea or nay.

J.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: ulogd2 patch ping

[Pablo Neira Ayuso]

Hi Jeremy,

On Tue, Jul 25, 2023 at 08:11:28PM +0100, Jeremy Sowden wrote:
> There is a ulogd2 patch of mine from the end of last that is still under
> review in Patchwork:
> 
>   https://patchwork.ozlabs.org/project/netfilter-devel/patch/20221208222208.681865-1-jeremy@azazel.net/
> 
> It would be great to get a yea or nay.

What plugins are still IPv4-only in ulogd2?

Maybe add _IPV4 | _IPV6 flags to plugins hence it is possible to
validate if user's stack is valid, otherwise bail out and provide a
reason via logging?

Regarding translation from network to host byte, I think it makes more
sense to keep IPv4 addres in network byte, so filter and output
plugings always expect them such way as you did in your patch?

Let me know, thanks!

Re: ulogd2 patch ping

[Florian Westphal]

Jeremy Sowden <jeremy@azazel.net> wrote:
> There is a ulogd2 patch of mine from the end of last that is still under
> review in Patchwork:
> 
>   https://patchwork.ozlabs.org/project/netfilter-devel/patch/20221208222208.681865-1-jeremy@azazel.net/
> 
> It would be great to get a yea or nay.

Sorry, I don't use ulogd2 at all so I have no clue if this is good
or not.

I'll apply it on Friday in case noone objects until then.

Re: ulogd2 patch ping

[Florian Westphal]

Florian Westphal <fw@strlen.de> wrote:
> I'll apply it on Friday in case noone objects until then.

Just for completeness: I did not apply it due to the unanswered
question from Pablo.

Also, don't we risk ambiguity here, or is there a guarantee that kernel
never emits mapped addresses?

Re: ulogd2 patch ping

[Jeremy Sowden]

[-- Attachment #1: Type: text/plain, Size: 964 bytes --]

On 2023-08-01, at 00:15:13 +0200, Florian Westphal wrote:
> Florian Westphal <fw@strlen.de> wrote:
> > I'll apply it on Friday in case noone objects until then.
> 
> Just for completeness: I did not apply it due to the unanswered
> question from Pablo.

Thanks, Florian. I had hoped to have reponded over the week-end, but I
was busier than I expected and didn't pick this up again till yesterday
evening.  Answers should be forthcoming soon.

> Also, don't we risk ambiguity here, or is there a guarantee that
> kernel never emits mapped addresses?

It would indeed mean that real ipv4-in-ipv6 addresses would be output in
ipv4 format.  It's been several months, so I can't quite remember my
thinking, but I probably inclined towards regarding the ambiguity as
benign.  I have spotted a problem with the existing patch, however, so
another version will definitely be needed, and I can look at eliminating
the ambiguity at the same time.

J.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: ulogd2 patch ping

[Jeremy Sowden]

[-- Attachment #1: Type: text/plain, Size: 2503 bytes --]

On 2023-07-26, at 09:28:11 +0200, Pablo Neira Ayuso wrote:
> On Tue, Jul 25, 2023 at 08:11:28PM +0100, Jeremy Sowden wrote:
> > There is a ulogd2 patch of mine from the end of last that is still
> > under review in Patchwork:
> > 
> >   https://patchwork.ozlabs.org/project/netfilter-devel/patch/20221208222208.681865-1-jeremy@azazel.net/
> > 
> > It would be great to get a yea or nay.
> 
> What plugins are still IPv4-only in ulogd2?

ipfix, gprint, oprint and the SQL plug-ins all assume that input keys of
type ULOGD_RET_IPADDR are ipv4.  There is a separate ipv6 adddress type
(ULOGD_RET_IP6ADDR), but it is not used anywhere.

The question is what to do with ipv6 addresses on output, in particular
for the DB plug-ins where trying to write 128 bit values into possibly
32 bit columns might cause errors.  In the current version of the patch,
the gprint and oprint plug-ins are updated to output ipv6 addresses
correctly, the DB plug-ins replace the value with null, and I didn't fix
the ipfix plug-in.

As it happens, the mysql and pgsql schemas in doc/ should be fine, but
the sqlite3 one won't be.  My current thinking is to add a boolean
config setting to the SQL plug-ins to indicate whether the DB schema
can accommodate ipv6 addresses.

> Maybe add _IPV4 | _IPV6 flags to plugins hence it is possible to
> validate if user's stack is valid, otherwise bail out and provide a
> reason via logging?

The problem is that it isn't always possible to tell.  Consider a stack
with an NFLOG source.  The address family will depend on the family of
the table containing the rule outputting to the log, which is not
visible to ulogd.

> Regarding translation from network to host byte, I think it makes more
> sense to keep IPv4 addres in network byte, so filter and output
> plugings always expect them such way as you did in your patch?

Having revisited this work, I think my blithe assumption that I could
fix the handling of ipv6 addresses and the original endianness problem
in one patch may have been overly optimistic. :) In the next version, I
will separate the changes to keep ipv4 addresses in NBO from the ipv6
fixes.

Florian's observation about the ambiguity between real mapped ipv4
addresses and the synthetic ones created by this patch has prompted me
to revisit my approach to the ipv6 fixes.  My current thinking is to use
the `len` field of `struct ulogd_key` to keep track of the size of the
address: 4 = ipv4, 16 = ipv6.

J.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]