[PATCH 6/6] site: New PROD message

Ian Jackson ijackson at chiark.greenend.org.uk
Wed Jul 17 14:07:53 BST 2013


We introduce a new message PROD which requests that the peer initiate
a key exchange with us, if it doesn't already have a key.

This helps significantly reduce a possible "dead period" when one end
of a connection is restarted during key exchange.  The dead period is
now limited to the time taken for the interrupted key exchange to time
out.

Signed-off-by: Ian Jackson <ijackson at chiark.greenend.org.uk>
---
 NOTES   |   34 +++++++++++++++++++++++
 magic.h |    1 +
 site.c  |   92 ++++++++++++++++++++++++++++++++++++++++++++++++++++----------
 udp.c   |    1 +
 4 files changed, 113 insertions(+), 15 deletions(-)

diff --git a/NOTES b/NOTES
index 477104f..a4e6b21 100644
--- a/NOTES
+++ b/NOTES
@@ -276,3 +276,37 @@ vaguely recent version of secnet.  (In fact, there is no evidence in
 the git history of it ever being sent.)
 
 This message number is reserved.
+
+11) *,*,PROD,A,B
+
+Sent in response to a NAK from B to A.  Requests that B initiates a
+key exchange with A, if B is willing and lacks a transport key for A.
+(If B doesn't have A's address configured, implicitly supplies A's
+public address.)
+
+This is necessary because if one end of a link (B) is restarted while
+a key exchange is in progress, the following bad state can persist:
+the non-restarted end (A) thinks that the key is still valid and keeps
+sending packets, but B either doesn't realise that a key exchange with
+A is necessary or (if A is a mobile site) doesn't know A's public IP
+address.
+
+Normally in these circumstances B would send NAKs to A, causing A to
+initiate a key exchange.  However if A and B were already in the
+middle of a key exchange then A will not want to try another one until
+the first one has timed out ("setup-time" x "setup-retries") and then
+the key exchange retry timeout ("wait-time") has elapsed.
+
+However if B's setup has timed out, B would be willing to participate
+in a key exchange initiated by A, if A could be induced to do so.
+This is the purpose of the PROD packet.
+
+We send no more PRODs than we would want to send data packets, to
+avoid a traffic amplification attack.  We also send them only in state
+WAIT, as in other states we wouldn't respond favourably.  And we only
+honour them if we don't already have a key.
+
+With PROD, the period of broken communication due to a key exchange
+interrupted by a restart is limited to the key exchange total
+retransmission timeout, rather than also including the key exchange
+retry timeout.
diff --git a/magic.h b/magic.h
index c2503a0c..7d2f427 100644
--- a/magic.h
+++ b/magic.h
@@ -14,5 +14,6 @@
 #define LABEL_MSG7 0x07070707
 #define LABEL_MSG8 0x08080808
 #define LABEL_MSG9 0x09090909
+#define LABEL_PROD 0x0a0a0a0a
 
 #endif /* magic_h */
diff --git a/site.c b/site.c
index aecd64d..8de482b 100644
--- a/site.c
+++ b/site.c
@@ -204,7 +204,8 @@ static void transport_peers_copy(struct site *st, transport_peers *dst,
 static void transport_setup_msgok(struct site *st, const struct comm_addr *a);
 static void transport_data_msgok(struct site *st, const struct comm_addr *a);
 static bool_t transport_compute_setupinit_peers(struct site *st,
-        const struct comm_addr *configured_addr /* 0 if none or not found */);
+        const struct comm_addr *configured_addr /* 0 if none or not found */,
+        const struct comm_addr *prod_hint_addr /* 0 if none */);
 static void transport_record_peer(struct site *st, transport_peers *peers,
 				  const struct comm_addr *addr, const char *m);
 
@@ -263,6 +264,7 @@ struct site {
 /* runtime information */
     uint32_t state;
     uint64_t now; /* Most recently seen time */
+    bool_t allow_send_prod;
 
     /* The currently established session */
     struct data_key current;
@@ -327,7 +329,8 @@ static void delete_one_key(struct site *st, struct data_key *key,
 			   const char *reason /* may be 0 meaning don't log*/,
 			   const char *which /* ignored if !reasonn */,
 			   uint32_t loglevel /* ignored if !reasonn */);
-static bool_t initiate_key_setup(struct site *st, cstring_t reason);
+static bool_t initiate_key_setup(struct site *st, cstring_t reason,
+				 const struct comm_addr *prod_hint);
 static void enter_state_run(struct site *st);
 static bool_t enter_state_resolve(struct site *st);
 static bool_t enter_new_state(struct site *st,uint32_t next);
@@ -418,6 +421,10 @@ static bool_t unpick_msg(struct site *st, uint32_t type,
     m->loclen=buf_unprepend_uint16(msg);
     CHECK_AVAIL(msg,m->loclen);
     m->local=buf_unprepend(msg,m->loclen);
+    if (type==LABEL_PROD) {
+	CHECK_EMPTY(msg);
+	return True;
+    }
     CHECK_AVAIL(msg,NONCELEN);
     m->nR=buf_unprepend(msg,NONCELEN);
     if (type==LABEL_MSG1) {
@@ -794,7 +801,7 @@ static bool_t decrypt_msg0(struct site *st, struct buffer_if *msg0)
 
     slog(st,LOG_SEC,"transform: %s (aux: %s, new: %s)",
 	 transform_err,auxkey_err,newkey_err);
-    initiate_key_setup(st,"incoming message would not decrypt");
+    initiate_key_setup(st,"incoming message would not decrypt",0);
     return False;
 }
 
@@ -819,7 +826,7 @@ static bool_t process_msg0(struct site *st, struct buffer_if *msg0,
 	transport_data_msgok(st,src);
 	/* See whether we should start negotiating a new key */
 	if (st->now > st->renegotiate_key_time)
-	    initiate_key_setup(st,"incoming packet in renegotiation window");
+	    initiate_key_setup(st,"incoming packet in renegotiation window",0);
 	return True;
     default:
 	slog(st,LOG_SEC,"incoming encrypted message of type %08x "
@@ -900,7 +907,7 @@ static void site_resolve_callback(void *sst, struct in_addr *address)
 	slog(st,LOG_ERROR,"resolution of %s failed",st->address);
 	ca_use=0;
     }
-    if (transport_compute_setupinit_peers(st,ca_use)) {
+    if (transport_compute_setupinit_peers(st,ca_use,0)) {
 	enter_new_state(st,SITE_SENTMSG1);
     } else {
 	/* Can't figure out who to try to to talk to */
@@ -909,14 +916,15 @@ static void site_resolve_callback(void *sst, struct in_addr *address)
     }
 }
 
-static bool_t initiate_key_setup(struct site *st, cstring_t reason)
+static bool_t initiate_key_setup(struct site *st, cstring_t reason,
+				 const struct comm_addr *prod_hint)
 {
     if (st->state!=SITE_RUN) return False;
     slog(st,LOG_SETUP_INIT,"initiating key exchange (%s)",reason);
     if (st->address) {
 	slog(st,LOG_SETUP_INIT,"resolving peer address");
 	return enter_state_resolve(st);
-    } else if (transport_compute_setupinit_peers(st,0)) {
+    } else if (transport_compute_setupinit_peers(st,0,prod_hint)) {
 	return enter_new_state(st,SITE_SENTMSG1);
     }
     slog(st,LOG_SETUP_INIT,"key exchange failed: no address for peer");
@@ -1126,6 +1134,30 @@ static void enter_state_wait(struct site *st)
     /* XXX Erase keys etc. */
 }
 
+static void generate_prod(struct site *st, struct buffer_if *buf)
+{
+    buffer_init(buf,0);
+    buf_append_uint32(buf,0);
+    buf_append_uint32(buf,0);
+    buf_append_uint32(buf,LABEL_PROD);
+    buf_append_string(buf,st->localname);
+    buf_append_string(buf,st->remotename);
+}
+
+static void generate_send_prod(struct site *st,
+			       const struct comm_addr *source)
+{
+    if (!st->allow_send_prod) return; /* too soon */
+    if (!(st->state==SITE_RUN || st->state==SITE_RESOLVE ||
+	  st->state==SITE_WAIT)) return; /* we'd ignore peer's MSG1 */
+
+    slog(st,LOG_SETUP_INIT,"prodding peer for key exchange)");
+    st->allow_send_prod=0;
+    generate_prod(st,&st->scratch);
+    dump_packet(st,&st->scratch,source,False);
+    source->comm->sendmsg(source->comm->st, &st->scratch, source);
+}
+
 static inline void site_settimeout(uint64_t timeout, int *timeout_io)
 {
     if (timeout) {
@@ -1198,6 +1230,8 @@ static void site_outgoing(void *sst, struct buffer_if *buf)
 	return;
     }
 
+    st->allow_send_prod=1;
+
     /* In all other states we consider delivering the packet if we have
        a valid key and a valid address to send it to. */
     if (current_valid(st) && transport_peers_valid(&st->peers)) {
@@ -1217,7 +1251,7 @@ static void site_outgoing(void *sst, struct buffer_if *buf)
 
     slog(st,LOG_DROP,"discarding outgoing packet of size %d",buf->size);
     BUF_FREE(buf);
-    initiate_key_setup(st,"outgoing packet");
+    initiate_key_setup(st,"outgoing packet",0);
 }
 
 static bool_t named_for_us(struct site *st, struct buffer_if *buf,
@@ -1233,7 +1267,10 @@ static bool_t named_for_us(struct site *st, struct buffer_if *buf,
 }
 
 /* This function is called by the communication device to deliver
-   packets from our peers. */
+   packets from our peers.
+   It should return True if the packet is recognised as being for
+   this current site instance (and should therefore not be processed
+   by other sites), even if the packet was otherwise ignored. */
 static bool_t site_incoming(void *sst, struct buffer_if *buf,
 			    const struct comm_addr *source)
 {
@@ -1288,6 +1325,20 @@ static bool_t site_incoming(void *sst, struct buffer_if *buf,
 	BUF_FREE(buf);
 	return True;
     }
+    if (msgtype==LABEL_PROD && named_for_us(st,buf,0)) {
+	struct msg m;
+	dump_packet(st,buf,source,True);
+	if (!unpick_msg(st,LABEL_PROD,buf,&m)) {
+	    slog(st,LOG_ERROR,"failed to unpick incoming PROD");
+	} else if (st->state!=SITE_RUN) {
+	    slog(st,LOG_DROP,"ignoring PROD when not in state RUN");
+	} else if (current_valid(st)) {
+	    slog(st,LOG_DROP,"ignoring PROD when we think we have a key");
+	} else {
+	    initiate_key_setup(st,"peer sent PROD packet",source);
+	}
+	return True;
+    }
     if (dest==st->index) {
 	/* Explicitly addressed to us */
 	if (msgtype!=LABEL_MSG0) dump_packet(st,buf,source,True);
@@ -1296,7 +1347,9 @@ static bool_t site_incoming(void *sst, struct buffer_if *buf,
 	    /* If the source is our current peer then initiate a key setup,
 	       because our peer's forgotten the key */
 	    if (get_uint32(buf->start+4)==st->current.remote_session_id) {
-		initiate_key_setup(st,"received a NAK");
+		bool_t initiated;
+		initiated = initiate_key_setup(st,"received a NAK",0);
+		if (!initiated) generate_send_prod(st,source);
 	    } else {
 		slog(st,LOG_SEC,"bad incoming NAK");
 	    }
@@ -1531,6 +1584,8 @@ static list_t *site_apply(closure_t *self, struct cloc loc, dict_t *context,
     st->log_events=string_list_to_word(dict_lookup(dict,"log-events"),
 				       log_event_table,"site");
 
+    st->allow_send_prod=0;
+
     st->tunname=safe_malloc(strlen(st->localname)+strlen(st->remotename)+5,
 			    "site_apply");
     sprintf(st->tunname,"%s<->%s",st->localname,st->remotename);
@@ -1687,23 +1742,30 @@ static void transport_record_peer(struct site *st, transport_peers *peers,
 }
 
 static bool_t transport_compute_setupinit_peers(struct site *st,
-        const struct comm_addr *configured_addr /* 0 if none or not found */) {
+        const struct comm_addr *configured_addr /* 0 if none or not found */,
+        const struct comm_addr *prod_hint_addr /* 0 if none */) {
 
-    if (!configured_addr && !transport_peers_valid(&st->peers))
+    if (!configured_addr && !prod_hint_addr &&
+	!transport_peers_valid(&st->peers))
 	return False;
 
     slog(st,LOG_SETUP_INIT,
-	 (!configured_addr ? "using only %d old peer address(es)"
-	  : "using configured address, and/or perhaps %d old peer address(es)"),
+	 "using:%s%s %d old peer address(es)",
+	 configured_addr ? " configured address;" : "",
+	 prod_hint_addr ? " PROD hint address;" : "",
 	 st->peers);
 
     /* Non-mobile peers havve st->peers.npeers==0 or ==1, since they
      * have transport_peers_max==1.  The effect is that this code
      * always uses the configured address if supplied, or otherwise
-     * the existing data peer if one exists; this is as desired. */
+     * the address of the incoming PROD, or the existing data peer if
+     * one exists; this is as desired. */
 
     transport_peers_copy(st,&st->setup_peers,&st->peers);
 
+    if (prod_hint_addr)
+	transport_record_peer(st,&st->setup_peers,prod_hint_addr,"prod");
+
     if (configured_addr)
 	transport_record_peer(st,&st->setup_peers,configured_addr,"setupinit");
 
diff --git a/udp.c b/udp.c
index 77be5b1..5d1ef7c 100644
--- a/udp.c
+++ b/udp.c
@@ -19,6 +19,7 @@
 #include <netinet/in.h>
 #include <arpa/inet.h>
 #include "util.h"
+#include "magic.h"
 #include "unaligned.h"
 #include "ipaddr.h"
 #include "magic.h"
-- 
1.7.2.5




More information about the sgo-software-discuss mailing list