[Swan-dev] Set keyingtries to 1 for Opportunistic Encryption connections

Andrew Cagney andrew.cagney at gmail.com
Thu Mar 26 13:52:03 UTC 2020


?

On Tue, 17 Mar 2020 at 10:15, Andrew Cagney <andrew.cagney at gmail.com> wrote:

> Can I attribute these test failures to this discussion?
>
> --- MASTER/testing/pluto/ikev2-59-multiple-acquires-alias/east.console.txt
> +++ OUTPUT/testing/pluto/ikev2-59-multiple-acquires-alias/east.console.txt
> @@ -33,7 +33,7 @@
>  000 "north-eastnets/0x1":   ike_life: 3600s; ipsec_life: 28800s;
> replay_window: 32; rekey_margin: 540s; rekey_fuzz: 100%; keyingtries:
> 0;
>  000 "north-eastnets/0x1":   retransmit-interval: 9999ms;
> retransmit-timeout: 99s;
>  000 "north-eastnets/0x1":   initial-contact:no; cisco-unity:no;
> fake-strongswan:no; send-vendorid:no; send-no-esp-tfc:no;
> -000 "north-eastnets/0x1":   policy:
>
> RSASIG+ECDSA+ENCRYPT+TUNNEL+PFS+UP+IKEV2_ALLOW+IKE_FRAG_ALLOW+ESN_NO+RSASIG_v1_5;
> +000 "north-eastnets/0x1":   policy:
>
> RSASIG+ECDSA+ENCRYPT+TUNNEL+PFS+IKEV2_ALLOW+IKE_FRAG_ALLOW+ESN_NO+RSASIG_v1_5;
>  000 "north-eastnets/0x1":   v2-auth-hash-policy:
> SHA2_256+SHA2_384+SHA2_512;
>  000 "north-eastnets/0x1":   conn_prio: 24,24; interface: eth1;
> metric: 0; mtu: unset; sa_prio:auto; sa_tfc:none;
>  000 "north-eastnets/0x1":   nflog-group: unset; mark: unset;
> vti-iface:unset; vti-routing:no; vti-shared:no; nic-offload:auto;
>
> On Sun, 8 Mar 2020 at 22:43, D. Hugh Redelmeier <hugh at mimosa.com> wrote:
> >
> > | From: Paul Wouters <paul at nohats.ca>
> >
> > | On Thu, 5 Mar 2020, D. Hugh Redelmeier wrote:
> > |
> > | > | I meant "resolved by setting it to 1".
> > | >
> > | > I don't really understand the issues.
> > |
> > | A failure shunt is installed after the first keyingtries= fails.
> > | If a second keyingtries is started, it will want to install the
> > | negotiationshunt. What does it mean when these two shunts are
> > | different? It is unclear what we _should_ do.
> > |
> > | The actual bug seems to be that sometimes, the second shunt comes
> > | in without widening to the full IP address. I do not yet know how
> > | or when this exactly happens. When it does, pluto installs a second
> > | shunt because it is different from the first one (eg proto 0 vs proto
> 6)
> > | then later on when a tunnel is installed, only one shunt is removed.
> >
> > OK.  That is (I think) a separate discussion that we should have.
> >
> > The code I wrote implemented the same logic as your fix.  So my change
> > is neutral with regard to this issue.
> >
> > | > If the shunts fail, except in special cases, and those cases are
> > | > undocumented, we should
> > | >
> > | > - fix the shunts issue (hard, I assume), or
> > |
> > | We should either fix the shunt issue, or think of removing shunts
> > | completely. While shunts give us a little more fine-grained control
> > | on what to do during negotiation, some of that is countered by the
> > | XFRM larval state anyway, and additionally, there is a _lot_ of
> > | complexity with shunts. That makes it tempting to remove.
> >
> > I think that this is an important discussion that should be separated
> > from this particular bug.
> >
> > Among other things, shunts were to prevent unexpected leaks in the clear.
> > That seems pretty important.  They were also used to mirror kernel state
> > in Pluto (it was unreasonable to try to query the kernel whenever Pluto
> > needed the information).  Unfortunately, the kernel could change the
> > kernel's representation without notifying userland.
> >
> > It is possible that the design constraints have changed over the last
> > decades and a different design might be better.  But just slashing
> > without careful analysis seems like a bad idea.  That analysis must
> > not be based on a subset of the cases.
> >
> > Such a change might require sysadmins to change firewalls.  If so, it
> > should not just be slipped into a release.
> >
> > | > Here's an add-on to Paul's code [UNTESTED].
> > | >
> > | > Since it changes starterwhack, something I'm not an expert in, the
> code is
> > | > particularly suspect.
> > |
> > | I actually changed it in add_connection() so it is independent on
> > | whether the parameters came in via whack or via ipsec.conf.
> >
> > Right.  That's why I didn't suggest removing your change.
> >
> > My code intended to implement the same result, but earlier, with a
> > more useful diagnostic (I hope -- untested!).
> >
> > | > It implements that last policy, I hope.
> > | > If one were to delete one line, it would only change a defaulted
> > | > keyingtries.
> > |
> > | Which makes this harder to do because we currently merge in our
> defaults
> > | via libipsecconf/whack interactions. By the time add_connection() is
> > | called, what was an implicit default is no longer known.
> >
> > Where I made the change, it is known whether this 0 came from a
> > default or an explicit assignment.  The diagnostic is only generated
> > if an explicit assignment is being over-ridden.
> >
> > I fear that your code emits too many diagnostics (i.e. where the 0
> > comes from default) and that they are not visible.
> >
> > That's why I was suggesting my additional change.
> >
> > Putting a check in the whack code would be good too.  But I don't
> > imagine that it is as important.
> > _______________________________________________
> > Swan-dev mailing list
> > Swan-dev at lists.libreswan.org
> > https://lists.libreswan.org/mailman/listinfo/swan-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libreswan.org/pipermail/swan-dev/attachments/20200326/bcc84386/attachment.html>


More information about the Swan-dev mailing list