chiark / gitweb /
site: dynamically create and destroy transform instances
[secnet.git] / site.c
diff --git a/site.c b/site.c
index fcc36d8..f9a005a 100644 (file)
--- a/site.c
+++ b/site.c
@@ -214,6 +214,12 @@ static void transport_xmit(struct site *st, transport_peers *peers,
  /***** END of transport peers declarations *****/
 
 
+struct data_key {
+    struct transform_inst_if *transform;
+    uint64_t key_timeout; /* End of life of current key */
+    uint32_t remote_session_id;
+};
+
 struct site {
     closure_t cl;
     struct site_if ops;
@@ -238,6 +244,7 @@ struct site {
     struct hash_if *hash;
 
     uint32_t index; /* Index of this site */
+    uint32_t local_capabilities;
     int32_t setup_retries; /* How many times to send setup packets */
     int32_t setup_retry_interval; /* Initial timeout for setup packets */
     int32_t wait_timeout; /* How long to wait if setup unsuccessful */
@@ -247,9 +254,6 @@ struct site {
                                      after this time, initiate a new
                                      key exchange */
 
-    uint8_t *setupsig; /* Expected signature of incoming MSG1 packets */
-    int32_t setupsiglen; /* Allows us to discard packets quickly if
-                           they are not for us */
     bool_t setup_priority; /* Do we have precedence if both sites emit
                              message 1 simultaneously? */
     uint32_t log_events;
@@ -259,11 +263,11 @@ struct site {
     uint64_t now; /* Most recently seen time */
 
     /* The currently established session */
-    uint32_t remote_session_id;
-    struct transform_inst_if *current_transform;
-    bool_t current_valid;
-    uint64_t current_key_timeout; /* End of life of current key */
+    struct data_key current;
+    struct data_key auxiliary_key;
+    bool_t auxiliary_is_new;
     uint64_t renegotiate_key_time; /* When we can negotiate a new key */
+    uint64_t auxiliary_renegotiate_key_time;
     transport_peers peers; /* Current address(es) of peer for data traffic */
 
     /* The current key setup protocol exchange.  We can only be
@@ -272,15 +276,18 @@ struct site {
        packet; we keep trying to continue the exchange, and have to
        timeout before we can listen for another setup packet); perhaps
        we should keep a list of 'bad' sources for setup packets. */
+    uint32_t remote_capabilities;
     uint32_t setup_session_id;
     transport_peers setup_peers;
     uint8_t localN[NONCELEN]; /* Nonces for key exchange */
     uint8_t remoteN[NONCELEN];
     struct buffer_if buffer; /* Current outgoing key exchange packet */
+    struct buffer_if scratch;
     int32_t retries; /* Number of retries remaining */
     uint64_t timeout; /* Timeout for current state */
     uint8_t *dhsecret;
     uint8_t *sharedsecret;
+    uint32_t sharedsecretlen;
     struct transform_inst_if *new_transform; /* For key setup/verify */
 };
 
@@ -315,12 +322,53 @@ static void slog(struct site *st, uint32_t event, cstring_t msg, ...)
 }
 
 static void set_link_quality(struct site *st);
-static void delete_key(struct site *st, cstring_t reason, uint32_t loglevel);
+static void delete_keys(struct site *st, cstring_t reason, uint32_t loglevel);
+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 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);
 static void enter_state_wait(struct site *st);
+static void activate_new_key(struct site *st);
+
+static bool_t is_transform_valid(struct transform_inst_if *transform)
+{
+    return transform && transform->valid(transform->st);
+}
+
+static bool_t current_valid(struct site *st)
+{
+    return is_transform_valid(st->current.transform);
+}
+
+#define DEFINE_CALL_TRANSFORM(fwdrev)                                  \
+static int call_transform_##fwdrev(struct site *st,                    \
+                                  struct transform_inst_if *transform, \
+                                  struct buffer_if *buf,               \
+                                  const char **errmsg)                 \
+{                                                                      \
+    if (!is_transform_valid(transform)) {                              \
+       *errmsg="transform not set up";                                 \
+       return 1;                                                       \
+    }                                                                  \
+    return transform->fwdrev(transform->st,buf,errmsg);                        \
+}
+
+DEFINE_CALL_TRANSFORM(forwards)
+DEFINE_CALL_TRANSFORM(reverse)
+
+static void dispose_transform(struct transform_inst_if **transform_var)
+{
+    struct transform_inst_if *transform=*transform_var;
+    if (transform) {
+       transform->delkey(transform->st);
+       transform->destroy(transform->st);
+    }
+    *transform_var = 0;
+}    
 
 #define CHECK_AVAIL(b,l) do { if ((b)->size<(l)) return False; } while(0)
 #define CHECK_EMPTY(b) do { if ((b)->size!=0) return False; } while(0)
@@ -329,14 +377,19 @@ static void enter_state_wait(struct site *st);
     type=buf_unprepend_uint32((b)); \
     if (type!=(t)) return False; } while(0)
 
+struct parsedname {
+    int32_t len;
+    uint8_t *name;
+    struct buffer_if extrainfo;
+};
+
 struct msg {
     uint8_t *hashstart;
     uint32_t dest;
     uint32_t source;
-    int32_t remlen;
-    uint8_t *remote;
-    int32_t loclen;
-    uint8_t *local;
+    struct parsedname remote;
+    struct parsedname local;
+    uint32_t remote_capabilities;
     uint8_t *nR;
     uint8_t *nL;
     int32_t pklen;
@@ -346,6 +399,44 @@ struct msg {
     char *sig;
 };
 
+static void set_new_transform(struct site *st)
+{
+    struct transform_if *generator=st->transform;
+    struct transform_inst_if *generated=generator->create(generator->st);
+    generated->setkey(generated->st,st->sharedsecret,
+                     st->sharedsecretlen,st->setup_priority);
+    dispose_transform(&st->new_transform);
+    st->new_transform=generated;
+}
+
+struct xinfoadd {
+    int32_t lenpos, afternul;
+};
+static void append_string_xinfo_start(struct buffer_if *buf,
+                                     struct xinfoadd *xia,
+                                     const char *str)
+    /* Helps construct one of the names with additional info as found
+     * in MSG1..4.  Call this function first, then append all the
+     * desired extra info (not including the nul byte) to the buffer,
+     * then call append_string_xinfo_done. */
+{
+    xia->lenpos = buf->size;
+    buf_append_string(buf,str);
+    buf_append_uint8(buf,0);
+    xia->afternul = buf->size;
+}
+static void append_string_xinfo_done(struct buffer_if *buf,
+                                    struct xinfoadd *xia)
+{
+    /* we just need to adjust the string length */
+    if (buf->size == xia->afternul) {
+       /* no extra info, strip the nul too */
+       buf_unappend_uint8(buf);
+    } else {
+       put_uint16(buf->start+xia->lenpos, buf->size-(xia->lenpos+2));
+    }
+}
+
 /* Build any of msg1 to msg4. msg5 and msg6 are built from the inside
    out using a transform of config data supplied by netlink */
 static bool_t generate_msg(struct site *st, uint32_t type, cstring_t what)
@@ -361,7 +452,14 @@ static bool_t generate_msg(struct site *st, uint32_t type, cstring_t what)
        (type==LABEL_MSG1?0:st->setup_session_id));
     buf_append_uint32(&st->buffer,st->index);
     buf_append_uint32(&st->buffer,type);
-    buf_append_string(&st->buffer,st->localname);
+
+    struct xinfoadd xia;
+    append_string_xinfo_start(&st->buffer,&xia,st->localname);
+    if ((st->local_capabilities & CAPAB_EARLY) || (type != LABEL_MSG1)) {
+       buf_append_uint32(&st->buffer,st->local_capabilities);
+    }
+    append_string_xinfo_done(&st->buffer,&xia);
+
     buf_append_string(&st->buffer,st->remotename);
     memcpy(buf_append(&st->buffer,NONCELEN),st->localN,NONCELEN);
     if (type==LABEL_MSG1) return True;
@@ -384,6 +482,22 @@ static bool_t generate_msg(struct site *st, uint32_t type, cstring_t what)
     return True;
 }
 
+static bool_t unpick_name(struct buffer_if *msg, struct parsedname *nm)
+{
+    CHECK_AVAIL(msg,2);
+    nm->len=buf_unprepend_uint16(msg);
+    CHECK_AVAIL(msg,nm->len);
+    nm->name=buf_unprepend(msg,nm->len);
+    uint8_t *nul=memchr(nm->name,0,nm->len);
+    if (!nul) {
+       buffer_readonly_view(&nm->extrainfo,0,0);
+    } else {
+       buffer_readonly_view(&nm->extrainfo, nul+1, msg->start-(nul+1));
+       nm->len=nul-nm->name;
+    }
+    return True;
+}
+
 static bool_t unpick_msg(struct site *st, uint32_t type,
                         struct buffer_if *msg, struct msg *m)
 {
@@ -393,14 +507,13 @@ static bool_t unpick_msg(struct site *st, uint32_t type,
     CHECK_AVAIL(msg,4);
     m->source=buf_unprepend_uint32(msg);
     CHECK_TYPE(msg,type);
-    CHECK_AVAIL(msg,2);
-    m->remlen=buf_unprepend_uint16(msg);
-    CHECK_AVAIL(msg,m->remlen);
-    m->remote=buf_unprepend(msg,m->remlen);
-    CHECK_AVAIL(msg,2);
-    m->loclen=buf_unprepend_uint16(msg);
-    CHECK_AVAIL(msg,m->loclen);
-    m->local=buf_unprepend(msg,m->loclen);
+    if (!unpick_name(msg,&m->remote)) return False;
+    m->remote_capabilities=0;
+    if (m->remote.extrainfo.size) {
+       CHECK_AVAIL(&m->remote.extrainfo,4);
+       m->remote_capabilities=buf_unprepend_uint32(&m->remote.extrainfo);
+    }
+    if (!unpick_name(msg,&m->local)) return False;
     CHECK_AVAIL(msg,NONCELEN);
     m->nR=buf_unprepend(msg,NONCELEN);
     if (type==LABEL_MSG1) {
@@ -426,6 +539,14 @@ static bool_t unpick_msg(struct site *st, uint32_t type,
     return True;
 }
 
+static bool_t name_matches(const struct parsedname *nm, const char *expected)
+{
+    int expected_len=strlen(expected);
+    return
+       nm->len == expected_len &&
+       !memcmp(nm->name, expected, expected_len);
+}    
+
 static bool_t check_msg(struct site *st, uint32_t type, struct msg *m,
                        cstring_t *error)
 {
@@ -433,11 +554,11 @@ static bool_t check_msg(struct site *st, uint32_t type, struct msg *m,
 
     /* Check that the site names and our nonce have been sent
        back correctly, and then store our peer's nonce. */ 
-    if (memcmp(m->remote,st->remotename,strlen(st->remotename)!=0)) {
+    if (!name_matches(&m->remote,st->remotename)) {
        *error="wrong remote site name";
        return False;
     }
-    if (memcmp(m->local,st->localname,strlen(st->localname)!=0)) {
+    if (!name_matches(&m->local,st->localname)) {
        *error="wrong local site name";
        return False;
     }
@@ -446,11 +567,17 @@ static bool_t check_msg(struct site *st, uint32_t type, struct msg *m,
        return False;
     }
     if (type==LABEL_MSG2) return True;
-    if (memcmp(m->nR,st->remoteN,NONCELEN)!=0) {
+    if (!consttime_memeq(m->nR,st->remoteN,NONCELEN)!=0) {
        *error="wrong remotely-generated nonce";
        return False;
     }
+    /* MSG3 has complicated rules about capabilities, which are
+     * handled in process_msg3. */
     if (type==LABEL_MSG3) return True;
+    if (m->remote_capabilities!=st->remote_capabilities) {
+       *error="remote capabilities changed";
+       return False;
+    }
     if (type==LABEL_MSG4) return True;
     *error="unknown message type";
     return False;
@@ -463,19 +590,16 @@ static bool_t generate_msg1(struct site *st)
 }
 
 static bool_t process_msg1(struct site *st, struct buffer_if *msg1,
-                          const struct comm_addr *src)
+                          const struct comm_addr *src, struct msg *m)
 {
-    struct msg m;
-
     /* We've already determined we're in an appropriate state to
        process an incoming MSG1, and that the MSG1 has correct values
        of A and B. */
 
-    if (!unpick_msg(st,LABEL_MSG1,msg1,&m)) return False;
-
     transport_record_peer(st,&st->setup_peers,src,"msg1");
-    st->setup_session_id=m.source;
-    memcpy(st->remoteN,m.nR,NONCELEN);
+    st->setup_session_id=m->source;
+    st->remote_capabilities=m->remote_capabilities;
+    memcpy(st->remoteN,m->nR,NONCELEN);
     return True;
 }
 
@@ -497,6 +621,7 @@ static bool_t process_msg2(struct site *st, struct buffer_if *msg2,
        return False;
     }
     st->setup_session_id=m.source;
+    st->remote_capabilities=m.remote_capabilities;
     memcpy(st->remoteN,m.nR,NONCELEN);
     return True;
 }
@@ -522,6 +647,15 @@ static bool_t process_msg3(struct site *st, struct buffer_if *msg3,
        slog(st,LOG_SEC,"msg3: %s",err);
        return False;
     }
+    uint32_t capab_adv_late = m.remote_capabilities
+       & ~st->remote_capabilities & CAPAB_EARLY;
+    if (capab_adv_late) {
+       slog(st,LOG_SEC,"msg3 impermissibly adds early capability flag(s)"
+            " %#"PRIx32" (was %#"PRIx32", now %#"PRIx32")",
+            capab_adv_late, st->remote_capabilities, m.remote_capabilities);
+       return False;
+    }
+    st->remote_capabilities|=m.remote_capabilities;
 
     /* Check signature and store g^x mod m */
     hash=safe_malloc(st->hash->len, "process_msg3");
@@ -544,11 +678,10 @@ static bool_t process_msg3(struct site *st, struct buffer_if *msg3,
 
     /* Generate the shared key */
     st->dh->makeshared(st->dh->st,st->dhsecret,st->dh->len,m.pk,
-                      st->sharedsecret,st->transform->keylen);
+                      st->sharedsecret,st->sharedsecretlen);
 
     /* Set up the transform */
-    st->new_transform->setkey(st->new_transform->st,st->sharedsecret,
-                             st->transform->keylen);
+    set_new_transform(st);
 
     return True;
 }
@@ -592,10 +725,9 @@ static bool_t process_msg4(struct site *st, struct buffer_if *msg4,
     m.pk[m.pklen]=0;
     /* Generate the shared key */
     st->dh->makeshared(st->dh->st,st->dhsecret,st->dh->len,m.pk,
-                      st->sharedsecret,st->transform->keylen);
+                      st->sharedsecret,st->sharedsecretlen);
     /* Set up the transform */
-    st->new_transform->setkey(st->new_transform->st,st->sharedsecret,
-                             st->transform->keylen);
+    set_new_transform(st);
 
     return True;
 }
@@ -628,10 +760,10 @@ static bool_t generate_msg5(struct site *st)
     buffer_init(&st->buffer,st->transform->max_start_pad+(4*4));
     /* Give the netlink code an opportunity to put its own stuff in the
        message (configuration information, etc.) */
-    st->netlink->output_config(st->netlink->st,&st->buffer);
     buf_prepend_uint32(&st->buffer,LABEL_MSG5);
-    st->new_transform->forwards(st->new_transform->st,&st->buffer,
-                               &transform_err);
+    if (call_transform_forwards(st,st->new_transform,
+                               &st->buffer,&transform_err))
+       return False;
     buf_prepend_uint32(&st->buffer,LABEL_MSG5);
     buf_prepend_uint32(&st->buffer,st->index);
     buf_prepend_uint32(&st->buffer,st->setup_session_id);
@@ -641,15 +773,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 (call_transform_reverse(st,transform,msg5,&transform_err)) {
        /* There's a problem */
        slog(st,LOG_SEC,"process_msg5: transform: %s",transform_err);
        return False;
@@ -660,15 +792,13 @@ static bool_t process_msg5(struct site *st, struct buffer_if *msg5,
        slog(st,LOG_SEC,"MSG5/PING packet contained wrong label");
        return False;
     }
-    if (!st->netlink->check_config(st->netlink->st,msg5)) {
-       slog(st,LOG_SEC,"MSG5/PING packet contained bad netlink config");
-       return False;
-    }
-    CHECK_EMPTY(msg5);
+    /* Older versions of secnet used to write some config data here
+     * which we ignore.  So we don't CHECK_EMPTY */
     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;
 
@@ -677,14 +807,20 @@ static bool_t generate_msg6(struct site *st)
     buffer_init(&st->buffer,st->transform->max_start_pad+(4*4));
     /* Give the netlink code an opportunity to put its own stuff in the
        message (configuration information, etc.) */
-    st->netlink->output_config(st->netlink->st,&st->buffer);
     buf_prepend_uint32(&st->buffer,LABEL_MSG6);
-    st->new_transform->forwards(st->new_transform->st,&st->buffer,
-                               &transform_err);
+    int problem = call_transform_forwards(st,transform,
+                                         &st->buffer,&transform_err);
+    assert(!problem);
     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)
+{
+    if (!is_transform_valid(st->new_transform))
+       return False;
+    create_msg6(st,st->new_transform,st->setup_session_id);
     st->retries=1; /* Peer will retransmit MSG5 if this packet gets lost */
     return True;
 }
@@ -697,8 +833,7 @@ static bool_t process_msg6(struct site *st, struct buffer_if *msg6,
 
     if (!unpick_msg0(st,msg6,&m)) return False;
 
-    if (st->new_transform->reverse(st->new_transform->st,
-                                  msg6,&transform_err)) {
+    if (call_transform_reverse(st,st->new_transform,msg6,&transform_err)) {
        /* There's a problem */
        slog(st,LOG_SEC,"process_msg6: transform: %s",transform_err);
        return False;
@@ -709,40 +844,101 @@ static bool_t process_msg6(struct site *st, struct buffer_if *msg6,
        slog(st,LOG_SEC,"MSG6/PONG packet contained invalid data");
        return False;
     }
-    if (!st->netlink->check_config(st->netlink->st,msg6)) {
-       slog(st,LOG_SEC,"MSG6/PONG packet contained bad netlink config");
-       return False;
-    }
-    CHECK_EMPTY(msg6);
+    /* Older versions of secnet used to write some config data here
+     * which we ignore.  So we don't CHECK_EMPTY */
     return True;
 }
 
-static bool_t process_msg0(struct site *st, struct buffer_if *msg0,
+static bool_t decrypt_msg0(struct site *st, struct buffer_if *msg0,
                           const struct comm_addr *src)
 {
+    cstring_t transform_err, auxkey_err, newkey_err="n/a";
     struct msg0 m;
-    cstring_t transform_err;
-    uint32_t type;
-
-    if (!st->current_valid) {
-       slog(st,LOG_DROP,"incoming message but no current key -> dropping");
-       return initiate_key_setup(st,"incoming message but no current key");
-    }
+    uint32_t problem;
 
     if (!unpick_msg0(st,msg0,&m)) return False;
 
-    if (st->current_transform->reverse(st->current_transform->st,
-                                      msg0,&transform_err)) {
-       /* There's a problem */
-       slog(st,LOG_SEC,"transform: %s",transform_err);
-       return initiate_key_setup(st,"incoming message would not decrypt");
+    /* Keep a copy so we can try decrypting it with multiple keys */
+    buffer_copy(&st->scratch, msg0);
+
+    problem = call_transform_reverse(st,st->current.transform,
+                                    msg0,&transform_err);
+    if (!problem) {
+       if (!st->auxiliary_is_new)
+           delete_one_key(st,&st->auxiliary_key,
+                          "peer has used new key","auxiliary key",LOG_SEC);
+       return True;
+    }
+    if (problem==2)
+       goto skew;
+
+    buffer_copy(msg0, &st->scratch);
+    problem = call_transform_reverse(st,st->auxiliary_key.transform,
+                                    msg0,&auxkey_err);
+    if (problem==0) {
+       slog(st,LOG_DROP,"processing packet which uses auxiliary key");
+       if (st->auxiliary_is_new) {
+           /* We previously timed out in state SENTMSG5 but it turns
+            * out that our peer did in fact get our MSG5 and is
+            * using the new key.  So we should switch to it too. */
+           /* This is a bit like activate_new_key. */
+           struct data_key t;
+           t=st->current;
+           st->current=st->auxiliary_key;
+           st->auxiliary_key=t;
+
+           delete_one_key(st,&st->auxiliary_key,"peer has used new key",
+                          "previous key",LOG_SEC);
+           st->auxiliary_is_new=0;
+           st->renegotiate_key_time=st->auxiliary_renegotiate_key_time;
+       }
+       return True;
+    }
+    if (problem==2)
+       goto skew;
+
+    if (st->state==SITE_SENTMSG5) {
+       buffer_copy(msg0, &st->scratch);
+       problem = call_transform_reverse(st,st->new_transform,
+                                        msg0,&newkey_err);
+       if (!problem) {
+           /* It looks like we didn't get the peer's MSG6 */
+           /* This is like a cut-down enter_new_state(SITE_RUN) */
+           slog(st,LOG_STATE,"will enter state RUN (MSG0 with new key)");
+           BUF_FREE(&st->buffer);
+           st->timeout=0;
+           activate_new_key(st);
+           return True; /* do process the data in this packet */
+       }
+       if (problem==2)
+           goto skew;
     }
+
+    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");
+    send_nak(src,m.dest,m.source,m.type,msg0,"message would not decrypt");
+    return False;
+
+ skew:
+    slog(st,LOG_DROP,"transform: %s (merely skew)",transform_err);
+    return False;
+}
+
+static bool_t process_msg0(struct site *st, struct buffer_if *msg0,
+                          const struct comm_addr *src)
+{
+    uint32_t type;
+
+    if (!decrypt_msg0(st,msg0,src))
+       return False;
+
     CHECK_AVAIL(msg0,4);
     type=buf_unprepend_uint32(msg0);
     switch(type) {
     case LABEL_MSG7:
        /* We must forget about the current session. */
-       delete_key(st,"request from peer",LOG_SEC);
+       delete_keys(st,"request from peer",LOG_SEC);
        return True;
     case LABEL_MSG9:
        /* Deliver to netlink layer */
@@ -763,9 +959,9 @@ static bool_t process_msg0(struct site *st, struct buffer_if *msg0,
 static void dump_packet(struct site *st, struct buffer_if *buf,
                        const struct comm_addr *addr, bool_t incoming)
 {
-    uint32_t dest=ntohl(*(uint32_t *)buf->start);
-    uint32_t source=ntohl(*(uint32_t *)(buf->start+4));
-    uint32_t msgtype=ntohl(*(uint32_t *)(buf->start+8));
+    uint32_t dest=get_uint32(buf->start);
+    uint32_t source=get_uint32(buf->start+4);
+    uint32_t msgtype=get_uint32(buf->start+8);
 
     if (st->log_events & LOG_DUMP)
        slilog(st->log,M_DEBUG,"%s: %s: %08x<-%08x: %08x:",
@@ -785,6 +981,24 @@ static bool_t send_msg(struct site *st)
        st->timeout=st->now+st->setup_retry_interval;
        st->retries--;
        return True;
+    } else if (st->state==SITE_SENTMSG5) {
+       slog(st,LOG_SETUP_TIMEOUT,"timed out sending MSG5, stashing new key");
+       /* We stash the key we have produced, in case it turns out that
+        * our peer did see our MSG5 after all and starts using it. */
+       /* This is a bit like some of activate_new_key */
+       struct transform_inst_if *t;
+       t=st->auxiliary_key.transform;
+       st->auxiliary_key.transform=st->new_transform;
+       st->new_transform=t;
+       dispose_transform(&st->new_transform);
+
+       st->auxiliary_is_new=1;
+       st->auxiliary_key.key_timeout=st->now+st->key_lifetime;
+       st->auxiliary_renegotiate_key_time=st->now+st->key_renegotiate_time;
+       st->auxiliary_key.remote_session_id=st->setup_session_id;
+
+       enter_state_wait(st);
+       return False;
     } else {
        slog(st,LOG_SETUP_TIMEOUT,"timed out sending key setup packet "
            "(in state %s)",state_name(st->state));
@@ -840,34 +1054,44 @@ static void activate_new_key(struct site *st)
 {
     struct transform_inst_if *t;
 
-    /* We have two transform instances, which we swap between active
-       and setup */
-    t=st->current_transform;
-    st->current_transform=st->new_transform;
+    /* We have three transform instances, which we swap between old,
+       active and setup */
+    t=st->auxiliary_key.transform;
+    st->auxiliary_key.transform=st->current.transform;
+    st->current.transform=st->new_transform;
     st->new_transform=t;
+    dispose_transform(&st->new_transform);
 
-    t->delkey(t->st);
     st->timeout=0;
-    st->current_valid=True;
-    st->current_key_timeout=st->now+st->key_lifetime;
+    st->auxiliary_is_new=0;
+    st->auxiliary_key.key_timeout=st->current.key_timeout;
+    st->current.key_timeout=st->now+st->key_lifetime;
     st->renegotiate_key_time=st->now+st->key_renegotiate_time;
     transport_peers_copy(st,&st->peers,&st->setup_peers);
-    st->remote_session_id=st->setup_session_id;
+    st->current.remote_session_id=st->setup_session_id;
 
     slog(st,LOG_ACTIVATE_KEY,"new key activated");
     enter_state_run(st);
 }
 
-static void delete_key(struct site *st, cstring_t reason, uint32_t loglevel)
+static void delete_one_key(struct site *st, struct data_key *key,
+                          cstring_t reason, cstring_t which, uint32_t loglevel)
 {
-    if (st->current_valid) {
+    if (!is_transform_valid(key->transform)) return;
+    if (reason) slog(st,loglevel,"%s deleted (%s)",which,reason);
+    dispose_transform(&key->transform);
+    key->key_timeout=0;
+}
+
+static void delete_keys(struct site *st, cstring_t reason, uint32_t loglevel)
+{
+    if (current_valid(st)) {
        slog(st,loglevel,"session closed (%s)",reason);
 
-       st->current_valid=False;
-       st->current_transform->delkey(st->current_transform->st);
-       st->current_key_timeout=0;
+       delete_one_key(st,&st->current,0,0,0);
        set_link_quality(st);
     }
+    delete_one_key(st,&st->auxiliary_key,0,0,0);
 }
 
 static void state_assert(struct site *st, bool_t ok)
@@ -879,14 +1103,14 @@ static void enter_state_stop(struct site *st)
 {
     st->state=SITE_STOP;
     st->timeout=0;
-    delete_key(st,"entering state STOP",LOG_TIMEOUT_KEY);
-    st->new_transform->delkey(st->new_transform->st);
+    delete_keys(st,"entering state STOP",LOG_TIMEOUT_KEY);
+    dispose_transform(&st->new_transform);
 }
 
 static void set_link_quality(struct site *st)
 {
     uint32_t quality;
-    if (st->current_valid)
+    if (current_valid(st))
        quality=LINK_QUALITY_UP;
     else if (st->state==SITE_WAIT || st->state==SITE_STOP)
        quality=LINK_QUALITY_DOWN;
@@ -910,9 +1134,9 @@ static void enter_state_run(struct site *st)
     transport_peers_clear(st,&st->setup_peers);
     memset(st->localN,0,NONCELEN);
     memset(st->remoteN,0,NONCELEN);
-    st->new_transform->delkey(st->new_transform->st);
+    dispose_transform(&st->new_transform);
     memset(st->dhsecret,0,st->dh->len);
-    memset(st->sharedsecret,0,st->transform->keylen);
+    memset(st->sharedsecret,0,st->sharedsecretlen);
     set_link_quality(st);
 }
 
@@ -998,19 +1222,21 @@ static bool_t send_msg7(struct site *st, cstring_t reason)
 {
     cstring_t transform_err;
 
-    if (st->current_valid && st->buffer.free
+    if (current_valid(st) && st->buffer.free
        && transport_peers_valid(&st->peers)) {
        BUF_ALLOC(&st->buffer,"site:MSG7");
        buffer_init(&st->buffer,st->transform->max_start_pad+(4*3));
        buf_append_uint32(&st->buffer,LABEL_MSG7);
        buf_append_string(&st->buffer,reason);
-       st->current_transform->forwards(st->current_transform->st,
-                                       &st->buffer, &transform_err);
+       if (call_transform_forwards(st, st->current.transform,
+                                   &st->buffer, &transform_err))
+           goto free_out;
        buf_prepend_uint32(&st->buffer,LABEL_MSG0);
        buf_prepend_uint32(&st->buffer,st->index);
-       buf_prepend_uint32(&st->buffer,st->remote_session_id);
+       buf_prepend_uint32(&st->buffer,st->current.remote_session_id);
        transport_xmit(st,&st->peers,&st->buffer,True);
        BUF_FREE(&st->buffer);
+    free_out:
        return True;
     }
     return False;
@@ -1049,14 +1275,23 @@ static int site_beforepoll(void *sst, struct pollfd *fds, int *nfds_io,
     st->now=*now;
 
     /* Work out when our next timeout is. The earlier of 'timeout' or
-       'current_key_timeout'. A stored value of '0' indicates no timeout
+       'current.key_timeout'. A stored value of '0' indicates no timeout
        active. */
     site_settimeout(st->timeout, timeout_io);
-    site_settimeout(st->current_key_timeout, timeout_io);
+    site_settimeout(st->current.key_timeout, timeout_io);
+    site_settimeout(st->auxiliary_key.key_timeout, timeout_io);
 
     return 0; /* success */
 }
 
+static void check_expiry(struct site *st, struct data_key *key,
+                        const char *which)
+{
+    if (key->key_timeout && *now>key->key_timeout) {
+       delete_one_key(st,key,"maximum life exceeded",which,LOG_TIMEOUT_KEY);
+    }
+}
+
 /* NB site_afterpoll will be called before site_beforepoll is ever called */
 static void site_afterpoll(void *sst, struct pollfd *fds, int nfds)
 {
@@ -1075,9 +1310,8 @@ static void site_afterpoll(void *sst, struct pollfd *fds, int nfds)
                 st->state);
        }
     }
-    if (st->current_key_timeout && *now>st->current_key_timeout) {
-       delete_key(st,"maximum key life exceeded",LOG_TIMEOUT_KEY);
-    }
+    check_expiry(st,&st->current,"current key");
+    check_expiry(st,&st->auxiliary_key,"auxiliary key");
 }
 
 /* This function is called by the netlink device to deliver packets
@@ -1095,17 +1329,19 @@ static void site_outgoing(void *sst, struct buffer_if *buf)
 
     /* In all other states we consider delivering the packet if we have
        a valid key and a valid address to send it to. */
-    if (st->current_valid && transport_peers_valid(&st->peers)) {
+    if (current_valid(st) && transport_peers_valid(&st->peers)) {
        /* Transform it and send it */
        if (buf->size>0) {
            buf_prepend_uint32(buf,LABEL_MSG9);
-           st->current_transform->forwards(st->current_transform->st,
-                                           buf, &transform_err);
+           if (call_transform_forwards(st, st->current.transform,
+                                       buf, &transform_err))
+               goto free_out;
            buf_prepend_uint32(buf,LABEL_MSG0);
            buf_prepend_uint32(buf,st->index);
-           buf_prepend_uint32(buf,st->remote_session_id);
+           buf_prepend_uint32(buf,st->current.remote_session_id);
            transport_xmit(st,&st->peers,buf,False);
        }
+    free_out:
        BUF_FREE(buf);
        return;
     }
@@ -1115,6 +1351,18 @@ static void site_outgoing(void *sst, struct buffer_if *buf)
     initiate_key_setup(st,"outgoing packet");
 }
 
+static bool_t named_for_us(struct site *st, const struct buffer_if *buf_in,
+                          uint32_t type, struct msg *m)
+    /* For packets which are identified by the local and remote names.
+     * If it has our name and our peer's name in it it's for us. */
+{
+    struct buffer_if buf[1];
+    buffer_readonly_clone(buf,buf_in);
+    return unpick_msg(st,type,buf,m)
+       && name_matches(&m->remote,st->remotename)
+       && name_matches(&m->local,st->localname);
+}
+
 /* This function is called by the communication device to deliver
    packets from our peers. */
 static bool_t site_incoming(void *sst, struct buffer_if *buf,
@@ -1124,66 +1372,63 @@ static bool_t site_incoming(void *sst, struct buffer_if *buf,
 
     if (buf->size < 12) return False;
 
-    uint32_t dest=ntohl(*(uint32_t *)buf->start);
-
-    if (dest==0) {
-       /* It could be for any site - it should have LABEL_MSG1 and
-          might have our name and our peer's name in it */
-       if (buf->size<(st->setupsiglen+8+NONCELEN)) return False;
-       if (memcmp(buf->start+8,st->setupsig,st->setupsiglen)==0) {
-           /* It's addressed to us. Decide what to do about it. */
-           dump_packet(st,buf,source,True);
-           if (st->state==SITE_RUN || st->state==SITE_RESOLVE ||
-               st->state==SITE_WAIT) {
-               /* We should definitely process it */
-               if (process_msg1(st,buf,source)) {
-                   slog(st,LOG_SETUP_INIT,"key setup initiated by peer");
+    uint32_t dest=get_uint32(buf->start);
+    uint32_t msgtype=get_uint32(buf->start+8);
+    struct msg named_msg;
+
+    if (msgtype==LABEL_MSG1) {
+       if (!named_for_us(st,buf,msgtype,&named_msg))
+           return False;
+       /* It's a MSG1 addressed to us. Decide what to do about it. */
+       dump_packet(st,buf,source,True);
+       if (st->state==SITE_RUN || st->state==SITE_RESOLVE ||
+           st->state==SITE_WAIT) {
+           /* We should definitely process it */
+           if (process_msg1(st,buf,source,&named_msg)) {
+               slog(st,LOG_SETUP_INIT,"key setup initiated by peer");
+               enter_new_state(st,SITE_SENTMSG2);
+           } else {
+               slog(st,LOG_ERROR,"failed to process incoming msg1");
+           }
+           BUF_FREE(buf);
+           return True;
+       } else if (st->state==SITE_SENTMSG1) {
+           /* We've just sent a message 1! They may have crossed on
+              the wire. If we have priority then we ignore the
+              incoming one, otherwise we process it as usual. */
+           if (st->setup_priority) {
+               BUF_FREE(buf);
+               slog(st,LOG_DUMP,"crossed msg1s; we are higher "
+                    "priority => ignore incoming msg1");
+               return True;
+           } else {
+               slog(st,LOG_DUMP,"crossed msg1s; we are lower "
+                    "priority => use incoming msg1");
+               if (process_msg1(st,buf,source,&named_msg)) {
+                   BUF_FREE(&st->buffer); /* Free our old message 1 */
                    enter_new_state(st,SITE_SENTMSG2);
                } else {
-                   slog(st,LOG_ERROR,"failed to process incoming msg1");
+                   slog(st,LOG_ERROR,"failed to process an incoming "
+                        "crossed msg1 (we have low priority)");
                }
                BUF_FREE(buf);
                return True;
-           } else if (st->state==SITE_SENTMSG1) {
-               /* We've just sent a message 1! They may have crossed on
-                  the wire. If we have priority then we ignore the
-                  incoming one, otherwise we process it as usual. */
-               if (st->setup_priority) {
-                   BUF_FREE(buf);
-                   slog(st,LOG_DUMP,"crossed msg1s; we are higher "
-                        "priority => ignore incoming msg1");
-                   return True;
-               } else {
-                   slog(st,LOG_DUMP,"crossed msg1s; we are lower "
-                        "priority => use incoming msg1");
-                   if (process_msg1(st,buf,source)) {
-                       BUF_FREE(&st->buffer); /* Free our old message 1 */
-                       enter_new_state(st,SITE_SENTMSG2);
-                   } else {
-                       slog(st,LOG_ERROR,"failed to process an incoming "
-                            "crossed msg1 (we have low priority)");
-                   }
-                   BUF_FREE(buf);
-                   return True;
-               }
            }
-           /* The message 1 was received at an unexpected stage of the
-              key setup. XXX POLICY - what do we do? */
-           slog(st,LOG_UNEXPECTED,"unexpected incoming message 1");
-           BUF_FREE(buf);
-           return True;
        }
-       return False; /* Not for us. */
+       /* The message 1 was received at an unexpected stage of the
+          key setup. XXX POLICY - what do we do? */
+       slog(st,LOG_UNEXPECTED,"unexpected incoming message 1");
+       BUF_FREE(buf);
+       return True;
     }
     if (dest==st->index) {
        /* Explicitly addressed to us */
-       uint32_t msgtype=ntohl(get_uint32(buf->start+8));
        if (msgtype!=LABEL_MSG0) dump_packet(st,buf,source,True);
        switch (msgtype) {
-       case 0: /* NAK */
+       case LABEL_NAK:
            /* 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->remote_session_id) {
+           if (get_uint32(buf->start+4)==st->current.remote_session_id) {
                initiate_key_setup(st,"received a NAK");
            } else {
                slog(st,LOG_SEC,"bad incoming NAK");
@@ -1234,16 +1479,29 @@ static bool_t site_incoming(void *sst, struct buffer_if *buf,
            /* Setup packet: expected only in state SENTMSG4 */
            /* (may turn up in state RUN if our return MSG6 was lost
               and the new key has already been activated. In that
-              case we should treat it as an ordinary PING packet. We
-              can't pass it to process_msg5() because the
-              new_transform will now be unkeyed. XXX) */
-           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);
+              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) {
+               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->current.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:
@@ -1339,6 +1597,7 @@ static list_t *site_apply(closure_t *self, struct cloc loc, dict_t *context,
 
     assert(index_sequence < 0xffffffffUL);
     st->index = ++index_sequence;
+    st->local_capabilities = 0;
     st->netlink=find_cl_if(dict,"link",CL_NETLINK,True,"site",loc);
 
     list_t *comms_cfg=dict_lookup(dict,"comm");
@@ -1413,30 +1672,27 @@ static list_t *site_apply(closure_t *self, struct cloc loc, dict_t *context,
     /* The information we expect to see in incoming messages of type 1 */
     /* fixme: lots of unchecked overflows here, but the results are only
        corrupted packets rather than undefined behaviour */
-    st->setupsiglen=strlen(st->remotename)+strlen(st->localname)+8;
-    st->setupsig=safe_malloc(st->setupsiglen,"site_apply");
-    put_uint32(st->setupsig+0,LABEL_MSG1);
-    put_uint16(st->setupsig+4,strlen(st->remotename));
-    memcpy(&st->setupsig[6],st->remotename,strlen(st->remotename));
-    put_uint16(st->setupsig+(6+strlen(st->remotename)),strlen(st->localname));
-    memcpy(&st->setupsig[8+strlen(st->remotename)],st->localname,
-          strlen(st->localname));
     st->setup_priority=(strcmp(st->localname,st->remotename)>0);
 
     buffer_new(&st->buffer,SETUP_BUFFER_LEN);
 
+    buffer_new(&st->scratch,0);
+    BUF_ALLOC(&st->scratch,"site:scratch");
+
     /* We are interested in poll(), but only for timeouts. We don't have
        any fds of our own. */
     register_for_poll(st, site_beforepoll, site_afterpoll, 0, "site");
     st->timeout=0;
 
-    st->current_valid=False;
-    st->current_key_timeout=0;
+    st->remote_capabilities=0;
+    st->current.key_timeout=0;
+    st->auxiliary_key.key_timeout=0;
     transport_peers_clear(st,&st->peers);
     transport_peers_clear(st,&st->setup_peers);
     /* XXX mlock these */
     st->dhsecret=safe_malloc(st->dh->len,"site:dhsecret");
-    st->sharedsecret=safe_malloc(st->transform->keylen,"site:sharedsecret");
+    st->sharedsecretlen=st->transform->keylen?:st->dh->ceil_len;
+    st->sharedsecret=safe_malloc(st->sharedsecretlen,"site:sharedsecret");
 
     /* We need to compute some properties of our comms */
 #define COMPUTE_WORST(pad)                     \
@@ -1458,8 +1714,10 @@ static list_t *site_apply(closure_t *self, struct cloc loc, dict_t *context,
     for (i=0; i<st->ncomms; i++)
        st->comms[i]->request_notify(st->comms[i]->st, st, site_incoming);
 
-    st->current_transform=st->transform->create(st->transform->st);
-    st->new_transform=st->transform->create(st->transform->st);
+    st->current.transform=0;
+    st->auxiliary_key.transform=0;
+    st->new_transform=0;
+    st->auxiliary_is_new=0;
 
     enter_state_stop(st);