Here's a change to consider (I'm not pushing it):
Along the lines of "2.21.3. Error Handling after IKE SA is
Authenticated", when the state machine totally fails and there is a
protected (but not necessarily authenticated) channel, send back a
protected INVALID_SYNTAX and discard the state.
I suspect this is also the better thing to do when
encrypted_payload_status.bad but I didn't tweak that.
(and it should really be recording the response and then zombifying
the state so retransmits get handled)
Andrew
On Wed, 15 May 2019 at 16:55, Andrew Cagney <[email protected]> wrote:
>
> https://testing.libreswan.org/v3.27-1219-g7142d2c37-master/newoe-27-replace-sa-authnull-authnull/OUTPUT/east.pluto.log.gz
>
> FYI, several things go wrong. Most notably, pluto's inability to
> handle an IKE_AUTH where the IKE SA succeeds but the CHILD SA fails.
>
> - IKE_SA_INIT is exchanged
>
> - first IKE_AUTH packet arrives:
> -- IKE SA successfully authenticates; it starts constructing the
> response; state is transitioned to established _but_ Message IDs are
> not updated (since things are still in flux it is too early)
> -- CHILD SA barfs and is deleted; the response is thrown away and the
> Message IDs aren't updated
>
> - duplicate IKE_AUTH packet arrives:
> -- the duplicate isn't recognized because the Message IDs don't line up
> -- state machine doesn't match as this is nonsensical
> -- then the crash
>
> I've stopped the crash with:
>
> "private-or-clear#192.1.3.0/24"[2] ...192.1.3.209 #3: dropping message
> with no matching microcode
diff --git a/programs/pluto/ikev2.c b/programs/pluto/ikev2.c
index c6f6560cd..ce9a1969c 100644
--- a/programs/pluto/ikev2.c
+++ b/programs/pluto/ikev2.c
@@ -2200,33 +2200,54 @@ void ikev2_process_state_packet(struct ike_sa *ike,
struct state *st,
*/
send_v2N_response_from_md(md,
v2N_INVALID_IKE_SPI,
NULL/*no data*/);
- } else {
+ } else if (ike != NULL &&
ike->sa.hidden_variables.st_skeyid_calculated) {
/*
- * Presumably things are pretty messed
- * up. While there might be a state
- * there probably isn't an established
- * IKE SA (so don't even consider
- * trying to send an encrypted
- * response), for instance:
+ * Hopefully:
+ *
+ * 2.21.3. Error Handling after IKE
+ * SA is Authenticated: [...] If a
+ * peer parsing a request notices that
+ * it is badly formatted (after it has
+ * passed the message authentication
+ * code checks and window checks) and
+ * it returns an INVALID_SYNTAX
+ * notification, then this error
+ * notification is considered fatal in
+ * both peers, meaning that the IKE SA
+ * is deleted without needing an
+ * explicit Delete payload.
+ *
+ * However, the more common case is
+ * that this end is really messed up
+ * messed up.
*
- * - instead of an IKE_AUTH request,
- * the initiator sends something
- * totally unexpected (such as an
- * informational) and things end up
- * here
-
- * - when an IKE_AUTH request's IKE SA
- * succeeeds but CHILD SA fails (and
- * pluto screws up the IKE SA by
- * updating its state but not its
- * Message ID and not responding), the
- * re-transmitted IKE_AUTH ends up
- * here
+ * For instance, when an IKE_AUTH
+ * request's IKE SA succeeeds but
+ * CHILD SA fails (and pluto screws up
+ * the IKE SA by updating its state
+ * but not its Message ID and not
+ * responding), the re-transmitted
+ * IKE_AUTH ends up here.
+ */
+ libreswan_log("responding to message no
matching microcode with encrypted INVALID_SYNTAX notification and discarding
state");
+ send_v2N_response_from_state(ike, md,
v2N_INVALID_SYNTAX,
+ NULL/*no data*/);
+ /* i.e, discard_state(&ike->sa); */
+ change_state(&ike->sa, STATE_IKESA_DEL);
+ delete_state(&ike->sa);
+ } else {
+ /*
+ * For instance, instead of an
+ * IKE_AUTH request, the initiator
+ * sends something totally unexpected
+ * (such as an informational) and
+ * things end up here.
*
- * Should it send a non-encrypted
- * v2N_INVALID_SYNTAX?
+ * The message hasn't been integrity
+ * checked (if it had above would have
+ * handled it) so just toss it.
*/
- rate_log("dropping message with no matching
microcode");
+ rate_log("discarding message with no matching
microcode");
}
}
return;
_______________________________________________
Swan-dev mailing list
[email protected]
https://lists.libreswan.org/mailman/listinfo/swan-dev
