Re: [PATCH v4 2/3] netfilter: nf_conntrack: add direction support for zones

[Daniel Borkmann <daniel@iogearbox.net>]

On 08/13/2015 11:50 AM, Pablo Neira Ayuso wrote:
> On Wed, Aug 12, 2015 at 10:05:11PM +0200, Daniel Borkmann wrote:
> [...]
>> But you are basically saying to add the nested CTA_TUPLE_ZONE container here,
>> that is part of a nested CTA_TUPLE_ORIG and/or CTA_TUPLE_REPLY attribute ...
>>
>> enum ctattr_tuple {
>> 	CTA_TUPLE_UNSPEC,
>> 	CTA_TUPLE_IP,
>> 	CTA_TUPLE_PROTO,
>> 	CTA_TUPLE_ZONE,  <---
>> 	__CTA_TUPLE_MAX
>> };
>
> Right.
>
>> ... where CTA_TUPLE_ZONE would be a container for further attributes, say
>> CTA_TUPLE_ZONE_ID, which is then the actual NLA_U16 zone id, right?
>
> Question is if we really need a nested attribute or not here, we've
> been discussing this before but future requirements are not clear. I
> think it would be good to keep those in mind to enhance this the right
> way.
>
> So, going back to this, I think the idea is to add new commands to
> ctnetlink to create zone objects with specific settings at some point,
> so we get three new enum cntl_msg_types.
>
>          IPCTNL_MSG_CT_NEW_ZONE
>          IPCTNL_MSG_CT_GET_ZONE
>          IPCTNL_MSG_CT_DEL_ZONE
>
> These new messages allow us to create/delete/retrieve custom zones
> with specific settings, each zone can be represented by:
>
> enum ctattr_zone {
>          CTA_ZONE_UNSPEC,
>          CTA_ZONE_ID,
>          CTA_ZONE_CONFIG,
>          __CTA_ZONE_MAX
> };
>
> The CTA_ZONE_CONFIG is a nested attribute with specific configuration
> for this zone, eg. the maximum number of connections.
>
> The custom zone can be used from the CT target, so we not only set a
> zone ID to the conntrack but we also can attach configurations.
>
> There's a problem though: By the time -j CT --zone X is loaded, the
> zone ID may not exists, so we need a new --zone-template X to
> explicitly refer to zones that are created via ctnetlink.

Yes, right. So the above makes sense to me. You would create zones with a
specific configuration attached over a separate interface.

And then, you could use the CTA_ZONE or CTA_TUPLE_ZONE (both NLA_U16 attrs
that represent the zone id) to look up a specific zone to be used. As a result,
that would mean, it might be best to have CTA_TUPLE_ZONE as NLA_U16. Seems a
clean way forward to me.

>> So, we'd have a zone id spread in 3 possible places, and additional (future)
>> meta data spread around in 2 possible places, hmm ... Okay, lets say, we'd
>> add future attribute X and Y to zones. Now, if I want a zone only in ORIG
>> dir or only in REPLY dir, that works fine from ctnetlink perspective, even
>> with your idea that there could be two different non-default zones entirely.
>
> If we ever get connection limiting per zone as Thomas suggested during
> NFWS, I think we may well have two different non-default zones, what
> it comes to my mind is a simple scenario with two uplinks, each uplink
> becomes a zone with different connection limits.
>
>> But, lets say I just want to use a traditional zones config (as in: nowadays)
>> and have my tuple for /one/ particular zone id that is the same in /both/
>> directions. That would mean I have to duplicate my parameters X and Y across
>> CTA_TUPLE_ORIG and CTA_TUPLE_REPLY, right? Or, we'd add a third attribute
>> set (as in: CTA_ZONE_INFO) only for the single zone in both directions?
>
> CTA_ZONE can still be used to set a zone for both directions, we can
> get rid of that. The new CTA_TUPLE_ZONE allows you to set the zone at
> per-tuple level. We only have to be careful on how to interpret if the
> user sends us two of them that have overlapping semantics.

Right, we'd need to reject conflicting configurations, of course.

>> So far I find the current approach a bit cleaner to be honest (I can, of
>> course, still change the CTA_TUPLE_ZONE back into CTA_ZONE_INFO name) ...
>> but when the time comes where someone really should need two /non-default/
>> zones for a single tuple, don't we need a global setting as in this patch
>> here anyway (due to reasons above)? (I'm fine either way, I'm just asking on
>> how we want to handle this in an ideal/clean way.)
>>
>> ...
>>> I'd suggest the output shows the zone on the corresponding tuple, eg.
>>> in case it only applies to the original tuple:
>>>
>>> udp      17 29 src=192.168.2.195 dst=192.168.2.1 sport=40446 dport=53 zone=1 \
>>>                 src=192.168.2.1 dst=192.168.2.195 sport=53 dport=40446 [ASSURED] mark=0 use=1
>>>
>>> We have a more compact output IMO.
>>
>> Okay, that's fine by me. It would mean we'd see zone=1 twice in case a
>> direction was not specified (thus, both directions apply), but I think
>> that should be totally okay for the stand-alone interface (and in future
>> conntrack -L).
>
> I think we can leave it the same way it looks now when the zone
> applies to two directions, but looking at this output now this may
> look ambiguous when the zone is in the reply tuple, so I think we have
> to add different tags, ie.
>
> 1) When the zone is used from the original tuple:
>
> udp      17 29 src=192.168.2.195 dst=192.168.2.1 sport=40446 dport=53 zone-orig=1 \
>                 src=192.168.2.1 dst=192.168.2.195 sport=53 dport=40446 [ASSURED] mark=0 use=1
>
> 2) When used from the reply tuple:
>
> udp      17 29 src=192.168.2.195 dst=192.168.2.1 sport=40446 dport=53 \
>                 src=192.168.2.1 dst=192.168.2.195 sport=53 dport=40446 zone-reply=X [ASSURED] mark=0 use=1
>
> 3) When used in both (existing output):
>
> udp      17 29 src=192.168.2.195 dst=192.168.2.1 sport=40446 dport=53 \
>                 src=192.168.2.1 dst=192.168.2.195 sport=53 dport=40446 [ASSURED] mark=0 zone=1 use=1

Agreed, sounds good. Will change it into this representation.

Thanks Pablo!

Best,
Daniel