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 <[email protected]> wrote: > > On Fri, 28 Sep 2018 at 12:03, Paul Wouters <[email protected]> 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 _______________________________________________ Swan-dev mailing list [email protected] https://lists.libreswan.org/mailman/listinfo/swan-dev
