[Swan-dev] the coding twine holding end's .host_addr, .client, .port, .protocol and .has_client together

[Andrew Cagney]

On Tue, 2 Mar 2021 at 12:52, Paul Wouters <paul at nohats.ca> wrote:
>
> On Tue, 2 Mar 2021, D. Hugh Redelmeier wrote:
>
> > | - instantiate non-template connections (this idea keeps coming up on IRC)
>
> > Then connections were derived from other ones, with more details
> > filled in.  This was instantiation.  It could happen for a variety of
> > reasons and with a variety of details added.

Some follow-up:

Hugh noted:
>> The original idea was that connections were immutable -- all changes were in state objects.

I think that explains:
-       /* my stuff */
-
-       /* Phase 2 ID payload info about my user */
-       uint8_t st_myuserprotoid;             /* IDcx.protoid */
-       uint16_t st_myuserport;
-
-       /* peers stuff */
-
-       /* Phase 2 ID payload info about peer's user */
-       uint8_t st_peeruserprotoid;           /* IDcx.protoid */
-       uint16_t st_peeruserport;
which pre-date modern history (currently they simply track
st->c->spd.{this,that}.{protocol.port}).

I've been adding selector_subnet_*() functions where code, based on
test results, is really just manipulating the subnet part of a
selector (i.e., in old code ignoring the port).  I wish they would go
away.


> This is what is basically happening with labeled IPsec. You configure
> one security label. You install this policy, and when you get a %trap,
> it is not for this label but for a "more narrow" label. We should
> instantiate the connection with this updated label and bring it up.
>
> That way, each label gets its own IPsec SA, which is the goal of
> labeled IPsec. The other end can then re-mark these packets with
> the same label (that did not travel across the network otherwise)
>
> The odd thing is that if the first connection is auto=start and not
> auto=ondemand (aka auto=route) then you do not get an ACQUIRE and
> you can only install the configured label, which actually never gets
> an ACQUIRE itself. But it is harmless.
>
> The IKEv1 code did this, but without using CK_TEMPLATE and CK_INSTANCE.
> It kind of abused the CK_PERMANENT type connection. The core libreswan
> devs thought this was all a bug and it should setup just 1 connection.
> We only recently understood the full design explained above. But it was
> too late to rewrite this into proper instantiating connections and
> template. But, that is what we should be doing.
>
> The old IKEv1 code ended up with multiple connections of CK_PERMANENT,
> where there was 1 set of SPD policies and multiple sets of SPD state.
> Apparently, the kernel would match the states with the security label,
> despite the policy having the wider label only. This worked due to
> some kernel magic. The reqid for all of these states are the same,
> but the label is different, so the kernel manages to find the right
> one despite this. But I doubt the kernel model is meant to be used
> in this way either,
>
> I assume these connections once up, never went down again, so it did
> not show any issue when one of these "permananents" were going down.
> It would bring down the SPD policies required for the other remaining
> SPD states. We see this now in the IKEv2 code.
>
> I don't know exactly why this appears more stable for IKEv1 than for
> IKEv2. I'm trying to convince our consumers of this code that this entire
> kernel state is wrong, and we should have one a set of SPD policies per
> set of SPD states. If we properly instantiate, we would get that.
>
> But our code for instantiation seems to have a buildin assumption that
> it only instantiates for incoming connections (aka roadwarriors) or
> for OE group instances. The narrowing=yes code also causes a connection
> to turn into a CK_TEMPLATE, and there are some hacks to make that work
> properly. But the support for this is a hack - the code assumes there
> is still only ever 1 connection that is just narrowed from the
> configuration, while full proper narrowing support would allow multiple
> instances that cover part of the original CK_TEMPLATE IP/port ranges of
> the connection. One reason I think we avoided doing this, is that
> conceptually, this is odd for us. If you have a /24 to /24 connection
> that allows narrowing, and you bring up a /32 to /32 from it, is the
> connection up or down? And how can you bring another one up, since you
> wouldn't get new ACQUIRES if you replaced the %trap with the /32 to /32
> policy. And if you don't replace the policy but add a new policy, the
> initial larval state from the ACQUIRE doesn't go away, and you prevent
> packet flow from that /32 to /32 to work until the larval state expires
> (default 30s). i've tried to talk to the kernel people to explain the
> issue of installing a tunnel policy that is only partially covering the
> %trap policy, and asked them how we can make this atomic and working,
> but haven't managed to convince them yet.
>
> We have a similar issue with the updating multiple Traffic Selectors
> per single IPsec SA. If you have 10 subnets configured, but the 8 has
> 8 subnets configured, we _should_ install this as one CK_PERMANENT with
> 8 subnets. But in theory we should put a %hold shunt in for the missing
> 2 subnets. Worse, for those not supporting this feature, the RFC states
> you should fallback to installing multiple IPsec SA's with one subnet,
> so only during the negotiation will you find out you should not be a
> CK_PERMANENT, but a CK_INSTANCE.
>
> It would probably be worth it to do a conference call on this with some
> of the people involved to talk about the RFC, the existing implementation,
> the architecture and how we can move to support all of these.
>
> Yes.
> _______________________________________________
> Swan-dev mailing list
> Swan-dev at lists.libreswan.org
> https://lists.libreswan.org/mailman/listinfo/swan-dev