[Swan-dev] problem from IRC: confusing message and action of lost final packet

[Andrew Cagney]

FYI, I'm considering a second tweak:  While not necessary, it would
prevent some unnecessary decryption.

Instead of only saving the incoming packet when the current state has
the reply flag set; add an .st_drop_duplicates flag so that the
duplicate code has something concrete to check.

That would hopefully be conservative enough to not be screwed by xauth
exchanges reversing the initiator / responder polarity with
back-to-back packets.
-
On Fri, 28 Sep 2018 at 17:53, Andrew Cagney <andrew.cagney at gmail.com> wrote:
>
> I pushed a change that should stop IKEv1 responding when the decrypted
> packet has problems (but hasn't been verified).
>
> It seems to fix the test case.
>
> On Fri, 28 Sep 2018 at 12:45, Andrew Cagney <andrew.cagney at gmail.com> wrote:
> >
> > On Fri, 28 Sep 2018 at 12:03, Paul Wouters <paul at nohats.ca> wrote:
> > >
> > > On Fri, 28 Sep 2018, Andrew Cagney wrote:
> > >
> > > > with the above two applied, here's what's going wrong (other than it's
> > > > IKEv1 and we're stuffed)?
> > > >
> > > > - since the IKEv1 initiator is in STATE_MAIN_I4 the IKE SA has been
> > > > established - any message from an earlier part of the exchange should
> > > > be detected and dropped
> > >
> > > Except R3 I guess, if receiving that we need to retransmit our last
> > > packet?
> >
> > As in an R3 response packet causing the initiator to transition from
> > STATE_MAIN_I3 -> STATE_MAIN_R4?
> >
> > Once the initiator is in state STATE_MAIN_R4 I believe they should be
> > dropped on the floor.
> >
> > I guess there's the argument that when a main exchange is immediately
> > followed by a quick exchange then an R3 response should trigger
> > re-transmit of the CHILD SA's first quick request (rather than letting
> > timeouts deal with it).  But was that ever the case?
> >
> > > > In IKEv2, that's easy as the Message ID is a counter.
> > > > What about IKEv1?  During these exchange the message ID seems to always be zero.
> > >
> > >     The message ID in the ISAKMP header identifies a Quick Mode in
> > >     progress for a particular ISAKMP SA which itself is identified by the
> > >     cookies in the ISAKMP header.
> > >
> > > So I assume that in Main/Aggr Mode it is indeed 0 ?
> >
> > Here's the relevant packet's header:
> >
> > | **parse ISAKMP Message:
> > |    initiator cookie:
> > |   d9 80 c6 55  02 90 0c 4e
> > |    responder cookie:
> > |   2c ab c5 c6  0f b4 78 5c
> > |    next payload type: ISAKMP_NEXT_ID (0x5)
> > |    ISAKMP version: ISAKMP Version 1.0 (rfc2407) (0x10)
> > |    exchange type: ISAKMP_XCHG_IDPROT (0x2)
> > |    flags: ISAKMP_FLAG_v1_ENCRYPTION (0x1)
> > |    message ID:  00 00 00 00
> > |    length: 332 (0x14c)
> >
> > so yes for at least main mode.
> >
> > > > - since the IKEv1 IKE SA is established (almost) all packets should be
> > > > encrypted and have integrity, yet this packet fails that so why on
> > > > earth is libreswan sending out a notification
> > >
> > > It seems it failed to detect it as a retransmit, and started processing
> > > the packet as if it was a fresh never before received packet?
> >
> > Right, up to a point.
> >
> > Like I noted, regardless it should have detected:
> >
> > - that the packet was old - the last-received buffer is only one deep
> > - that the packet was corrupt - and not responded
> >
> > Its pretty easy to MITM an oh-so-slightly corrupt version of this
> > packet and get past the last received check (see --impair
> > replay-encrypted / corrupt-encrypted).
> >
> > > > In IKEv2 this is easy, find the SK payload and decrypt/verify as a single step.
> > > > What about IKEv1?  As best I can tell the process is to decrypt the
> > > > packet and then parse the resulting white noise looking for a HASH
> > > > et.al. payload to use as verification - until all that is done nothing
> > > > can be trusted and everything should have been dropped.
> > > >
> > > > So by pushing 'ikev1 retransmits: only save the received packet when
> > > > responding' I exposed the above two failings.  Reverting it wouldn't
> > > > be sufficient.  It would likely need some special state magic to
> > > > detect if/when that last outgoing packet should be re-transmitted; and
> > > > would still leave libreswan exposed to the above.
> > >
> > > If we are established and the message ID is 0, then we could retransmit
> > > the last main/aggressive packet? Once we receive a Quick Mode packet
> > > response (with non-zero message id) then we never need to retransmit a
> > > msg id 0 packet anymore.
> >
> > No.
> >
> > Since the initiator is in STATE_MAIN_I4 it knows that the responder
> > received its last outgoing packet - there's nothing left to send for
> > this exchange.  In fact, sending out the last packet would trigger a
> > re-transmit storm.
> >
> > However, per above, perhaps it could send out the QUICK packet
> > (assuming it is ready), but that feels too complicated.
> >
> > (please don't make me read the aggressive bit of the RFC :-)
> >
> > Andrew