diff --git a/programs/pluto/ikev1.c b/programs/pluto/ikev1.c index e0615323ed..401618b6dd 100644 --- a/programs/pluto/ikev1.c +++ b/programs/pluto/ikev1.c @@ -1101,10 +1101,20 @@ void process_v1_packet(struct msg_digest *md) struct state *st = NULL; enum state_kind from_state = STATE_UNDEFINED; /* state we started in */ + /* + * For the initial responses, don't leak the responder's SPI. + * Hence the use of send_v1_notification_from_md(). + * + * AGGR mode is a mess in that the R0->R1 transition happens + * well before the transition succeeds. + */ #define SEND_NOTIFICATION(t) \ { \ pstats(ikev1_sent_notifies_e, t); \ - if (st != NULL) \ + if (st != NULL && \ + st->st_state->kind != STATE_AGGR_R0 && \ + st->st_state->kind != STATE_AGGR_R1 && \ + st->st_state->kind != STATE_MAIN_R0) \ send_v1_notification_from_state(st, from_state, t); \ else \ send_v1_notification_from_md(md, t); \ @@ -1168,17 +1178,26 @@ void process_v1_packet(struct msg_digest *md) from_state = (md->hdr.isa_xchg == ISAKMP_XCHG_IDPROT ? STATE_MAIN_R0 : STATE_AGGR_R0); } else { - /* not an initial message */ + /* + * Possibly not an initial message. Possibly + * from initiator. Possibly from responder. + * + * Possibly. Which is probably hopeless. + */ st = find_state_ikev1(&md->hdr.isa_ike_spis, md->hdr.isa_msgid); if (st == NULL) { /* - * perhaps this is a first message + * Perhaps this is a first message * from the responder and contains a * responder cookie that we've not yet * seen. + * + * Perhaps this is a random message + * with a bogus non-zero responder IKE + * SPI. */ st = find_state_ikev1_init(&md->hdr.isa_ike_initiator_spi, md->hdr.isa_msgid); @@ -1189,6 +1208,21 @@ void process_v1_packet(struct msg_digest *md) /* XXX Could send notification back */ return; } + if (st->st_state->kind == STATE_AGGR_R0) { + /* + * The only way for this to + * happen is for the attacker + * to guess the responder's + * IKE SPI that hasn't been + * sent over the wire? + * + * Well that or played 1/2^32 + * odds. + */ + llog_pexpect(md->md_logger, HERE, + "phase 1 message matching AGGR_R0 state"); + return; + } } from_state = st->st_state->kind; } @@ -2870,7 +2904,28 @@ void complete_v1_state_transition(struct state *st, struct msg_digest *md, stf_s delete_state(st); /* wipe out dangling pointer to st */ md->v1_st = NULL; + } else if (st->st_state->kind == STATE_AGGR_R0 || + st->st_state->kind == STATE_AGGR_R1 || + st->st_state->kind == STATE_MAIN_R0) { + /* + * + * Wipe out the incomplete larval state. + * + * ARGH! In <=v4.10, the aggr code flipped the + * larval state to R1 right at the start of + * the transition and not the end, so using + * state to figure things out is close to + * useless. + * + * Deleting the state means that pluto has no + * way to detect and ignore amplification + * attacks. + */ + delete_state(st); + /* wipe out dangling pointer to st */ + md->v1_st = NULL; } + break; } } diff --git a/programs/pluto/ikev1_aggr.c b/programs/pluto/ikev1_aggr.c index 2732951beb..87be80cb6c 100644 --- a/programs/pluto/ikev1_aggr.c +++ b/programs/pluto/ikev1_aggr.c @@ -169,7 +169,7 @@ stf_status aggr_inI1_outR1(struct state *null_st UNUSED, /* Set up state */ struct ike_sa *ike = new_v1_rstate(c, md); md->v1_st = &ike->sa; /* (caller will reset cur_state) */ - change_v1_state(&ike->sa, STATE_AGGR_R1); + change_v1_state(&ike->sa, STATE_AGGR_R0); /* * Warn when peer is expected to use especially dangerous @@ -197,7 +197,8 @@ stf_status aggr_inI1_outR1(struct state *null_st UNUSED, if (!v1_decode_certs(md)) { llog_sa(RC_LOG, ike, "X509: CERT payload bogus or revoked"); - return false; + /* XXX notification is in order! */ + return STF_FAIL_v1N + v1N_INVALID_ID_INFORMATION; } /*