chiark / gitweb /
site: Randomise key setup retry time
[secnet] / site.c
diff --git a/site.c b/site.c
index 390853500f40552e8af120ef4312ec7b85d1ee63..3f5e66c63adf2a77af08192b2fee7105aa359e8e 100644 (file)
--- a/site.c
+++ b/site.c
@@ -321,15 +321,14 @@ struct 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 */
+    int32_t wait_timeout_mean; /* How long to wait if setup unsuccessful */
     int32_t mobile_peer_expiry; /* How long to remember 2ary addresses */
     int32_t key_lifetime; /* How long a key lasts once set up */
     int32_t key_renegotiate_time; /* If we see traffic (or a keepalive)
                                      after this time, initiate a new
                                      key exchange */
 
-    bool_t setup_priority; /* Do we have precedence if both sites emit
-                             message 1 simultaneously? */
+    bool_t our_name_later; /* our name > peer name */
     uint32_t log_events;
 
 /* runtime information */
@@ -471,14 +470,15 @@ static bool_t current_valid(struct site *st)
 }
 
 #define DEFINE_CALL_TRANSFORM(fwdrev)                                  \
-static int call_transform_##fwdrev(struct site *st,                    \
+static transform_apply_return                                           \
+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_apply_err;                                     \
     }                                                                  \
     return transform->fwdrev(transform->st,buf,errmsg);                        \
 }
@@ -535,6 +535,16 @@ struct msg {
     char *sig;
 };
 
+static int32_t wait_timeout(struct site *st) {
+    int32_t t = st->wait_timeout_mean;
+    int8_t factor;
+    if (t < INT_MAX/2) {
+       st->random->generate(st->random->st,sizeof(factor),&factor);
+       t += (t / 256) * factor;
+    }
+    return t;
+}
+
 static _Bool set_new_transform(struct site *st, char *pk)
 {
     _Bool ok;
@@ -557,7 +567,7 @@ static _Bool set_new_transform(struct site *st, char *pk)
     struct transform_if *generator=st->chosen_transform;
     struct transform_inst_if *generated=generator->create(generator->st);
     ok = generated->setkey(generated->st,st->sharedsecret,
-                          st->sharedsecretlen,st->setup_priority);
+                          st->sharedsecretlen,st->our_name_later);
 
     dispose_transform(&st->new_transform);
     if (!ok) return False;
@@ -1030,8 +1040,9 @@ static void create_msg6(struct site *st, struct transform_inst_if *transform,
     /* Give the netlink code an opportunity to put its own stuff in the
        message (configuration information, etc.) */
     buf_prepend_uint32(&st->buffer,LABEL_MSG6);
-    int problem = call_transform_forwards(st,transform,
-                                         &st->buffer,&transform_err);
+    transform_apply_return 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);
@@ -1071,12 +1082,13 @@ static bool_t process_msg6(struct site *st, struct buffer_if *msg6,
     return True;
 }
 
-static bool_t decrypt_msg0(struct site *st, struct buffer_if *msg0,
+static transform_apply_return
+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;
-    uint32_t problem;
+    transform_apply_return problem;
 
     if (!unpick_msg0(st,msg0,&m)) return False;
 
@@ -1089,15 +1101,15 @@ static bool_t decrypt_msg0(struct site *st, struct buffer_if *msg0,
        if (!st->auxiliary_is_new)
            delete_one_key(st,&st->auxiliary_key,
                           "peer has used new key","auxiliary key",LOG_SEC);
-       return True;
+       return 0;
     }
-    if (problem==2)
-       goto skew;
+    if (transform_apply_return_badseq(problem))
+       goto badseq;
 
     buffer_copy(msg0, &st->scratch);
     problem = call_transform_reverse(st,st->auxiliary_key.transform,
                                     msg0,&auxkey_err);
-    if (problem==0) {
+    if (!problem) {
        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
@@ -1114,10 +1126,10 @@ static bool_t decrypt_msg0(struct site *st, struct buffer_if *msg0,
            st->auxiliary_is_new=0;
            st->renegotiate_key_time=st->auxiliary_renegotiate_key_time;
        }
-       return True;
+       return 0;
     }
-    if (problem==2)
-       goto skew;
+    if (transform_apply_return_badseq(problem))
+       goto badseq;
 
     if (st->state==SITE_SENTMSG5) {
        buffer_copy(msg0, &st->scratch);
@@ -1130,20 +1142,21 @@ static bool_t decrypt_msg0(struct site *st, struct buffer_if *msg0,
            BUF_FREE(&st->buffer);
            st->timeout=0;
            activate_new_key(st);
-           return True; /* do process the data in this packet */
+           return 0; /* do process the data in this packet */
        }
-       if (problem==2)
-           goto skew;
+       if (transform_apply_return_badseq(problem))
+           goto badseq;
     }
 
     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",0);
     send_nak(src,m.dest,m.source,m.type,msg0,"message would not decrypt");
-    return False;
+    assert(problem);
+    return problem;
 
skew:
-    slog(st,LOG_DROP,"transform: %s (merely skew)",transform_err);
badseq:
+    slog(st,LOG_DROP,"transform: %s (bad seq.)",transform_err);
     assert(problem);
     return problem;
 }
@@ -1152,8 +1165,18 @@ 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))
+    transform_apply_return problem;
+
+    problem = decrypt_msg0(st,msg0,src);
+    if (problem==transform_apply_seqdupe) {
+       /* We recently received another copy of this packet, maybe due
+        * to polypath.  That's not a problem; indeed, for the
+        * purposes of transport address management it is a success.
+        * But we don't want to process the packet. */
+       transport_data_msgok(st,src);
+       return False;
+    }
+    if (problem)
        return False;
 
     CHECK_AVAIL(msg0,4);
@@ -1340,7 +1363,7 @@ static void decrement_resolving_count(struct site *st, int by)
        } else if (st->local_mobile) {
            /* Not very good.  We should queue (another) renegotiation
             * so that we can update the peer address. */
-           st->key_renegotiate_time=st->now+st->wait_timeout;
+           st->key_renegotiate_time=st->now+wait_timeout(st);
        } else {
            slog(st,LOG_SETUP_INIT,"resolution failed: "
                 " continuing to use source address of peer's packets");
@@ -1615,7 +1638,7 @@ static bool_t send_msg7(struct site *st, cstring_t reason)
 static void enter_state_wait(struct site *st)
 {
     slog(st,LOG_STATE,"entering state WAIT");
-    st->timeout=st->now+st->wait_timeout;
+    st->timeout=st->now+wait_timeout(st);
     st->state=SITE_WAIT;
     set_link_quality(st);
     BUF_FREE(&st->buffer); /* will have had an outgoing packet in it */
@@ -1756,6 +1779,35 @@ static bool_t named_for_us(struct site *st, const struct buffer_if *buf_in,
        && name_matches(&m->local,st->localname);
 }
 
+static bool_t we_have_priority(struct site *st, const struct msg *m) {
+    if ((st->local_capabilities & m->remote_capabilities)
+       && CAPAB_PRIORITY_MOBILE) {
+       if (st->local_mobile) return True;
+       if (st-> peer_mobile) return False;
+    }
+    return st->our_name_later;
+}
+
+static bool_t setup_late_msg_ok(struct site *st, 
+                               const struct buffer_if *buf_in,
+                               uint32_t msgtype,
+                               const struct comm_addr *source) {
+    /* For setup packets which seem from their type like they are
+     * late.  Maybe they came via a different path.  All we do is make
+     * a note of the sending address, iff they look like they are part
+     * of the current key setup attempt. */
+    struct msg m;
+    if (!named_for_us(st,buf_in,msgtype,&m))
+       /* named_for_us calls unpick_msg which gets the nonces */
+       return False;
+    if (!consttime_memeq(m.nR,st->remoteN,NONCELEN) ||
+       !consttime_memeq(m.nL,st->localN, NONCELEN))
+       /* spoof ?  from stale run ?  who knows */
+       return False;
+    transport_setup_msgok(st,source);
+    return True;
+}
+
 /* This function is called by the communication device to deliver
    packets from our peers.
    It should return True if the packet is recognised as being for
@@ -1797,7 +1849,7 @@ static bool_t site_incoming(void *sst, struct buffer_if *buf,
            /* 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) {
+           if (we_have_priority(st,&named_msg)) {
                BUF_FREE(buf);
                if (!st->msg1_crossed_logged++)
                    slog(st,LOG_SETUP_INIT,"crossed msg1s; we are higher "
@@ -1817,9 +1869,21 @@ static bool_t site_incoming(void *sst, struct buffer_if *buf,
                BUF_FREE(buf);
                return True;
            }
+       } else if (st->state==SITE_SENTMSG2 ||
+                  st->state==SITE_SENTMSG4) {
+           if (consttime_memeq(named_msg.nR,st->remoteN,NONCELEN)) {
+               /* We are ahead in the protocol, but that msg1 had the
+                * peer's nonce so presumably it is from this key
+                * exchange run, via a slower route */
+               transport_setup_msgok(st,source);
+           } else {
+               slog(st,LOG_UNEXPECTED,"competing incoming message 1");
+           }
+           BUF_FREE(buf);
+           return True;
        }
        /* The message 1 was received at an unexpected stage of the
-          key setup. XXX POLICY - what do we do? */
+          key setup.  Well, they lost the race. */
        slog(st,LOG_UNEXPECTED,"unexpected incoming message 1");
        BUF_FREE(buf);
        return True;
@@ -1864,6 +1928,10 @@ static bool_t site_incoming(void *sst, struct buffer_if *buf,
        case LABEL_MSG2:
            /* Setup packet: expected only in state SENTMSG1 */
            if (st->state!=SITE_SENTMSG1) {
+               if ((st->state==SITE_SENTMSG3 ||
+                    st->state==SITE_SENTMSG5) &&
+                   setup_late_msg_ok(st,buf,msgtype,source))
+                   break;
                slog(st,LOG_UNEXPECTED,"unexpected MSG2");
            } else if (process_msg2(st,buf,source)) {
                transport_setup_msgok(st,source);
@@ -1876,6 +1944,9 @@ static bool_t site_incoming(void *sst, struct buffer_if *buf,
        case LABEL_MSG3BIS:
            /* Setup packet: expected only in state SENTMSG2 */
            if (st->state!=SITE_SENTMSG2) {
+               if ((st->state==SITE_SENTMSG4) &&
+                   setup_late_msg_ok(st,buf,msgtype,source))
+                   break;
                slog(st,LOG_UNEXPECTED,"unexpected MSG3");
            } else if (process_msg3(st,buf,source,msgtype)) {
                transport_setup_msgok(st,source);
@@ -1887,6 +1958,9 @@ static bool_t site_incoming(void *sst, struct buffer_if *buf,
        case LABEL_MSG4:
            /* Setup packet: expected only in state SENTMSG3 */
            if (st->state!=SITE_SENTMSG3) {
+               if ((st->state==SITE_SENTMSG5) &&
+                   setup_late_msg_ok(st,buf,msgtype,source))
+                   break;
                slog(st,LOG_UNEXPECTED,"unexpected MSG4");
            } else if (process_msg4(st,buf,source)) {
                transport_setup_msgok(st,source);
@@ -2090,7 +2164,7 @@ static list_t *site_apply(closure_t *self, struct cloc loc, dict_t *context,
     st->key_lifetime=         CFG_NUMBER("key-lifetime",  KEY_LIFETIME);
     st->setup_retries=        CFG_NUMBER("setup-retries", SETUP_RETRIES);
     st->setup_retry_interval= CFG_NUMBER("setup-timeout", SETUP_RETRY_INTERVAL);
-    st->wait_timeout=         CFG_NUMBER("wait-time",     WAIT_TIME);
+    st->wait_timeout_mean=    CFG_NUMBER("wait-time",     WAIT_TIME);
     st->mtu_target= dict_read_number(dict,"mtu-target",False,"site",loc,0);
 
     st->mobile_peer_expiry= dict_read_number(
@@ -2130,7 +2204,7 @@ 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->setup_priority=(strcmp(st->localname,st->remotename)>0);
+    st->our_name_later=(strcmp(st->localname,st->remotename)>0);
 
     buffer_new(&st->buffer,SETUP_BUFFER_LEN);
 
@@ -2162,6 +2236,9 @@ static list_t *site_apply(closure_t *self, struct cloc loc, dict_t *context,
        st->local_capabilities |= capbit;
     }
 
+    if (st->local_mobile || st->peer_mobile)
+       st->local_capabilities |= CAPAB_PRIORITY_MOBILE;
+
     /* We need to register the remote networks with the netlink device */
     uint32_t netlink_mtu; /* local virtual interface mtu */
     st->netlink->reg(st->netlink->st, site_outgoing, st, &netlink_mtu);