[Swan-dev] "already determined local IKE proposals" ??

[Andrew Cagney]

On Wed, 15 Aug 2018 at 15:28, Paul Wouters <paul at nohats.ca> wrote:
>
>
> I noticed in a log:
>
>         already determined local IKE proposals for XXXXX
>
> This happens when we received an IKE proposal we liked but where we did
> not like the first DH, so there is a mismatched KE, and we send INVALID_KE.
>
> When the remote comes back with (we presume) the same proposal but an
> updated KE payload, we call this code:
>
> void ikev2_need_ike_proposals(struct connection *c, const char *why) {
>          if (c->ike_proposals != NULL) {
>                  DBGF(DBG_CONTROL, "already determined local IKE proposals for %s (%s)",
>                       c->name, why);
>                  return;
>          }

It's safe.  This is the entire proposal set for this end (not the
selected proposal) so caching it is safe.  Short of re-defining the
connection, screwing with NSS, or perhaps mucking with some policy bit
(????), it shouldn't change.  Like you suggest, It can probably be
done when creating the connection.

It's a cop-out, not an optimization.  At the time I wasn't sure which
code path would trigger the need for this to be constructed from the
connection and wasn't clear on connection vs state life times
(actually the latter is still true).

This is annoyingly different to ESP/AH where the proposal set does
change between the AUTH exchange and a REKEY exchange.

> However, there is no guarantee that the new proposal indeed contains the
> transforms we saw before. So is this safe?
>
> I'm also confused about c->ike_proposals being set. It is on the
> connection, not on the state. What if this was a road warrior where
> we instantiate the connection?  What if we got two road warriors from
> different IPs and we ended up receiving their packets interleaved?
>
> I assume it might be stored on the connection because we likely didn't
> create a state for a packet that required us sending INVALID_KE.
>
> I think the code would be more secure if iwe remove this "optimalization".
>
> Thoughts?
>
> Paul
>
> _______________________________________________
> Swan-dev mailing list
> Swan-dev at lists.libreswan.org
> https://lists.libreswan.org/mailman/listinfo/swan-dev