From c90cc2a73eabbda9887e67898389acbd759d7270 Mon Sep 17 00:00:00 2001 From: Ian Jackson Date: Wed, 20 Jun 2012 23:07:34 +0100 Subject: [PATCH] site: Deal with losing our MSG6 - retransmit MSG6 when we see MSG5 in RUN Fix the lost MSG6 bug in the case where we are the responder: If we are in RUN and we see a SENTMSG5 encrypted with what we now regard as the data transfer key, conclude that the peer must have lost the MSG6. Regenerate and transmit a new MSG6. This requires a generalised version of generate_msg6 which allows the new call site to specify which transform and session id to use, and which doesn't set st->retries. So break the meat of generate_msg6 out into a new function which I have chosen to call create_msg6. (The fact that functions called `generate_msg*' set st->retries is slightly unfortunate.) We also have to add a transform argument to process_msg5 so that we can try decrypting MSG5s with the data transfer key. It is still possible, even after this patch, to get the two ends out of sync: if, just after the responder has switched to the new key and entered RUN, the network fails entirely for the key setup timeout, the initiator will abandon the key setup and switch to WAIT, and continue to use the new key. However, the responder has already discarded the old key. To fix this is not trivial: it will require us to introduce a third key, with its own lifetime expiry. This will be in a later patch. Signed-off-by: Ian Jackson --- site.c | 42 +++++++++++++++++++++++++++++------------- 1 file changed, 29 insertions(+), 13 deletions(-) diff --git a/site.c b/site.c index c0f4387..9f3ee85 100644 --- a/site.c +++ b/site.c @@ -642,15 +642,15 @@ static bool_t generate_msg5(struct site *st) } static bool_t process_msg5(struct site *st, struct buffer_if *msg5, - const struct comm_addr *src) + const struct comm_addr *src, + struct transform_inst_if *transform) { struct msg0 m; cstring_t transform_err; if (!unpick_msg0(st,msg5,&m)) return False; - if (st->new_transform->reverse(st->new_transform->st, - msg5,&transform_err)) { + if (transform->reverse(transform->st,msg5,&transform_err)) { /* There's a problem */ slog(st,LOG_SEC,"process_msg5: transform: %s",transform_err); return False; @@ -666,7 +666,8 @@ static bool_t process_msg5(struct site *st, struct buffer_if *msg5, return True; } -static bool_t generate_msg6(struct site *st) +static void create_msg6(struct site *st, struct transform_inst_if *transform, + uint32_t session_id) { cstring_t transform_err; @@ -676,12 +677,15 @@ static bool_t generate_msg6(struct site *st) /* Give the netlink code an opportunity to put its own stuff in the message (configuration information, etc.) */ buf_prepend_uint32(&st->buffer,LABEL_MSG6); - st->new_transform->forwards(st->new_transform->st,&st->buffer, - &transform_err); + transform->forwards(transform->st,&st->buffer,&transform_err); buf_prepend_uint32(&st->buffer,LABEL_MSG6); buf_prepend_uint32(&st->buffer,st->index); - buf_prepend_uint32(&st->buffer,st->setup_session_id); + buf_prepend_uint32(&st->buffer,session_id); +} +static bool_t generate_msg6(struct site *st) +{ + create_msg6(st,st->new_transform,st->setup_session_id); st->retries=1; /* Peer will retransmit MSG5 if this packet gets lost */ return True; } @@ -1258,13 +1262,25 @@ static bool_t site_incoming(void *sst, struct buffer_if *buf, case we discard it. The peer will realise that we are using the new key when they see our data packets. Until then the peer's data packets to us get discarded. */ - if (st->state!=SITE_SENTMSG4) { - slog(st,LOG_UNEXPECTED,"unexpected MSG5"); - } else if (process_msg5(st,buf,source)) { - transport_setup_msgok(st,source); - enter_new_state(st,SITE_RUN); + if (st->state==SITE_SENTMSG4) { + if (process_msg5(st,buf,source,st->new_transform)) { + transport_setup_msgok(st,source); + enter_new_state(st,SITE_RUN); + } else { + slog(st,LOG_SEC,"invalid MSG5"); + } + } else if (st->state==SITE_RUN) { + if (process_msg5(st,buf,source,st->current_transform)) { + slog(st,LOG_DROP,"got MSG5, retransmitting MSG6"); + transport_setup_msgok(st,source); + create_msg6(st,st->current_transform,st->remote_session_id); + transport_xmit(st,&st->peers,&st->buffer,True); + BUF_FREE(&st->buffer); + } else { + slog(st,LOG_SEC,"invalid MSG5 (in state RUN)"); + } } else { - slog(st,LOG_SEC,"invalid MSG5"); + slog(st,LOG_UNEXPECTED,"unexpected MSG5"); } break; case LABEL_MSG6: -- 2.30.2