diff --git a/programs/pluto/ikev1.c b/programs/pluto/ikev1.c index 401618b6dd..56659a9e8f 100644 --- a/programs/pluto/ikev1.c +++ b/programs/pluto/ikev1.c @@ -1794,7 +1794,6 @@ void process_packet_tail(struct msg_digest *md) const struct state_v1_microcode *smc = md->smc; enum state_kind from_state = smc->state; bool new_iv_set = md->new_iv_set; - bool self_delete = false; if (md->hdr.isa_flags & ISAKMP_FLAGS_v1_ENCRYPTION) { @@ -2269,38 +2268,40 @@ void process_packet_tail(struct msg_digest *md) } } + pexpect(st == md->v1_st); /* could be NULL */ + for (struct payload_digest *p = md->chain[ISAKMP_NEXT_D]; p != NULL; p = p->next) { - self_delete |= accept_delete(md, p); - if (DBGP(DBG_BASE)) { - DBG_dump("del:", p->pbs.cur, - pbs_left(&p->pbs)); + if (!accept_delete(&st, md, p)) { + ldbg(md->md_logger, "bailing with bad delete message"); + return; } - if (md->v1_st != st) { - pexpect(md->v1_st == NULL); - dbg("zapping ST as accept_delete() zapped MD.ST"); - st = md->v1_st; + if (st == NULL) { + ldbg(md->md_logger, "bailing due to self-inflicted delete"); + return; } } + pexpect(st == md->v1_st); /* could be NULL */ + for (struct payload_digest *p = md->chain[ISAKMP_NEXT_VID]; p != NULL; p = p->next) { handle_v1_vendorid(md, pbs_in_left_as_shunk(&p->pbs), (st != NULL ? st->st_logger : md->md_logger)); } - if (self_delete) { - accept_self_delete(md); - st = md->v1_st; - /* note: st ought to be NULL from here on */ - } + pexpect(st == md->v1_st); /* could be NULL */ - pexpect(st == md->v1_st); - statetime_t start = statetime_start(md->v1_st); /* - * XXX: danger - the .informational() processor deletes ST; - * and then tunnels this loss through MD.ST. + * XXX: Danger. + * + * ++ the .informational() processor deletes ST; and then + * tries to tunnel this loss back through MD.ST. + * + * ++ the .aggressive() processor replaces .V1_ST with the IKE + * SA? */ + statetime_t start = statetime_start(st); stf_status e = smc->processor(st, md); complete_v1_state_transition(md->v1_st, md, e); statetime_stop(&start, "%s()", __func__); diff --git a/programs/pluto/ikev1_main.c b/programs/pluto/ikev1_main.c index 5c60615312..fe27836efa 100644 --- a/programs/pluto/ikev1_main.c +++ b/programs/pluto/ikev1_main.c @@ -2026,19 +2026,25 @@ void send_v1_delete(struct state *st) * @param md Message Digest * @param p Payload digest * - * returns TRUE to indicate st needs to be deleted. - * We dare not do that ourselves because st is still in use. - * accept_self_delete must be called to do this - * at a more appropriate time. + * DANGER: this may stomp on *SDP and md->v1_st. + * + * Returns FALSE when the payload is crud. */ -bool accept_delete(struct msg_digest *md, - struct payload_digest *p) +bool accept_delete(struct state **stp, + struct msg_digest *md, + struct payload_digest *p) { - struct state *st = md->v1_st; + struct state *st = *stp; struct isakmp_delete *d = &(p->payload.delete); size_t sizespi; int i; - bool self_delete = false; + + /* Need state for things to be encrypted */ + if (st == NULL) { + llog(RC_LOG_SERIOUS, md->md_logger, + "ignoring Delete SA with no matching state"); + return false; + } /* We only listen to encrypted notifications */ if (!md->encrypted) { @@ -2073,7 +2079,7 @@ bool accept_delete(struct msg_digest *md, case PROTO_IPCOMP: /* nothing interesting to delete */ - return false; + return true; default: { @@ -2132,21 +2138,20 @@ bool accept_delete(struct msg_digest *md, * identities */ log_state(RC_LOG_SERIOUS, st, "ignoring Delete SA payload: ISAKMP SA used to convey Delete has different IDs from ISAKMP SA it deletes"); - } else if (dst == st) { - /* - * remember this for later: - * we need st to do any remaining deletes - */ - self_delete = true; } else { /* note: this code is cloned for handling self_delete */ - log_state(RC_LOG_SERIOUS, st, "received Delete SA payload: deleting ISAKMP State #%lu", + log_state(RC_LOG_SERIOUS, st, "received Delete SA payload: %sdeleting ISAKMP State #%lu", + (dst == st ? "self-" : ""), dst->st_serialno); if (nat_traversal_enabled && dst->st_connection->ikev1_natt != NATT_NONE) { nat_traversal_change_port_lookup(md, dst); v1_maybe_natify_initiator_endpoints(st, HERE); - } + } delete_state(dst); + if (dst == st) { + *stp = dst = st = md->v1_st = NULL; + return true; + } } } else { /* @@ -2206,12 +2211,15 @@ bool accept_delete(struct msg_digest *md, event_force(EVENT_SA_REPLACE, dst); } else { log_state(RC_LOG_SERIOUS, st, - "received Delete SA(0x%08" PRIx32 ") payload: deleting IPsec State #%lu", + "received Delete SA(0x%08" PRIx32 ") payload: %sdeleting IPsec State #%lu", ntohl(spi), + (st == dst ? "self-" : ""), dst->st_serialno); delete_state(dst); - if (md->v1_st == dst) - md->v1_st = NULL; + if (md->v1_st == dst) { + *stp = dst = md->v1_st = NULL; + return true; + } } /* @@ -2236,29 +2244,15 @@ bool accept_delete(struct msg_digest *md, * states tied to the * connection? */ + dbg("%s() self-inflicted delete of ISAKMP", __func__); delete_states_by_connection(&rc); - md->v1_st = NULL; + *stp = st = dst = md->v1_st = NULL; + return true; } } } } } - return self_delete; -} - -/* now it is safe to delete our sponsor */ -void accept_self_delete(struct msg_digest *md) -{ - struct state *st = md->v1_st; - - /* note: this code is cloned from handling ISAKMP non-self_delete */ - log_state(RC_LOG_SERIOUS, st, "received Delete SA payload: self-deleting ISAKMP State #%lu", - st->st_serialno); - if (nat_traversal_enabled && st->st_connection->ikev1_natt != NATT_NONE) { - nat_traversal_change_port_lookup(md, st); - v1_maybe_natify_initiator_endpoints(st, HERE); - } - delete_state(st); - md->v1_st = st = NULL; + return true; } diff --git a/programs/pluto/ipsec_doi.h b/programs/pluto/ipsec_doi.h index 41fe5dad71..dc45602eb7 100644 --- a/programs/pluto/ipsec_doi.h +++ b/programs/pluto/ipsec_doi.h @@ -31,9 +31,9 @@ extern void ipsecdoi_replace(struct state *st, unsigned long try); extern void init_phase2_iv(struct state *st, const msgid_t *msgid); -extern bool accept_delete(struct msg_digest *md, +extern bool accept_delete(struct state **st, + struct msg_digest *md, struct payload_digest *p); -extern void accept_self_delete(struct msg_digest *md); extern stf_status send_isakmp_notification(struct state *st, uint16_t type, const void *data,