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