chiark / gitweb /
site: Log about crossed MSG1 ignored only once
[secnet.git] / site.c
diff --git a/site.c b/site.c
index dcac0baaf3428f20c8a97b83eb4d169954271119..11fc28be2443973ab84c30490a7d79dd09efabf0 100644 (file)
--- a/site.c
+++ b/site.c
@@ -6,7 +6,7 @@
  *
  * secnet is free software; you can redistribute it and/or modify it
  * under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version d of the License, or
+ * the Free Software Foundation; either version 3 of the License, or
  * (at your option) any later version.
  * 
  * secnet is distributed in the hope that it will be useful, but
@@ -336,6 +336,7 @@ struct site {
     uint32_t state;
     uint64_t now; /* Most recently seen time */
     bool_t allow_send_prod;
+    bool_t msg1_crossed_logged;
     int resolving_count;
     int resolving_n_results_all;
     int resolving_n_results_stored;
@@ -534,8 +535,10 @@ struct msg {
     char *sig;
 };
 
-static void set_new_transform(struct site *st, char *pk)
+static _Bool set_new_transform(struct site *st, char *pk)
 {
+    _Bool ok;
+
     /* Make room for the shared key */
     st->sharedsecretlen=st->chosen_transform->keylen?:st->dh->ceil_len;
     assert(st->sharedsecretlen);
@@ -553,15 +556,18 @@ static void set_new_transform(struct site *st, char *pk)
     /* Set up the transform */
     struct transform_if *generator=st->chosen_transform;
     struct transform_inst_if *generated=generator->create(generator->st);
-    generated->setkey(generated->st,st->sharedsecret,
-                     st->sharedsecretlen,st->setup_priority);
+    ok = generated->setkey(generated->st,st->sharedsecret,
+                          st->sharedsecretlen,st->setup_priority);
+
     dispose_transform(&st->new_transform);
+    if (!ok) return False;
     st->new_transform=generated;
 
     slog(st,LOG_SETUP_INIT,"key exchange negotiated transform"
         " %d (capabilities ours=%#"PRIx32" theirs=%#"PRIx32")",
         st->chosen_transform->capab_transformnum,
         st->local_capabilities, st->remote_capabilities);
+    return True;
 }
 
 struct xinfoadd {
@@ -713,6 +719,13 @@ static bool_t unpick_msg(struct site *st, uint32_t type,
     CHECK_AVAIL(msg,m->siglen);
     m->sig=buf_unprepend(msg,m->siglen);
     CHECK_EMPTY(msg);
+
+    /* In `process_msg3_msg4' below, we assume that we can write a nul
+     * terminator following the signature.  Make sure there's enough space.
+     */
+    if (msg->start >= msg->base + msg->alloclen)
+       return False;
+
     return True;
 }
 
@@ -846,7 +859,7 @@ static bool_t process_msg3_msg4(struct site *st, struct msg *m)
     hst=st->hash->init();
     st->hash->update(hst,m->hashstart,m->hashlen);
     st->hash->final(hst,hash);
-    /* Terminate signature with a '0' - cheating, but should be ok */
+    /* Terminate signature with a '0' - already checked that this will fit */
     m->sig[m->siglen]=0;
     if (!st->pubkey->check(st->pubkey->st,hash,st->hash->len,m->sig)) {
        slog(st,LOG_SEC,"msg3/msg4 signature failed check!");
@@ -905,7 +918,7 @@ static bool_t process_msg3(struct site *st, struct buffer_if *msg3,
     st->random->generate(st->random->st,st->dh->len,st->dhsecret);
 
     /* Generate the shared key and set up the transform */
-    set_new_transform(st,m.pk);
+    if (!set_new_transform(st,m.pk)) return False;
 
     return True;
 }
@@ -936,7 +949,7 @@ static bool_t process_msg4(struct site *st, struct buffer_if *msg4,
     m.pk[m.pklen]=0;
 
     /* Generate the shared key and set up the transform */
-    set_new_transform(st,m.pk);
+    if (!set_new_transform(st,m.pk)) return False;
 
     return True;
 }
@@ -1131,7 +1144,8 @@ static bool_t decrypt_msg0(struct site *st, struct buffer_if *msg0,
 
  skew:
     slog(st,LOG_DROP,"transform: %s (merely skew)",transform_err);
-    return False;
+    assert(problem);
+    return problem;
 }
 
 static bool_t process_msg0(struct site *st, struct buffer_if *msg0,
@@ -1449,7 +1463,7 @@ static void enter_state_run(struct site *st)
     FILLZERO(st->remoteN);
     dispose_transform(&st->new_transform);
     memset(st->dhsecret,0,st->dh->len);
-    memset(st->sharedsecret,0,st->sharedsecretlen);
+    if (st->sharedsecret) memset(st->sharedsecret,0,st->sharedsecretlen);
     set_link_quality(st);
 
     if (st->keepalive && !current_valid(st))
@@ -1512,6 +1526,7 @@ static bool_t enter_new_state(struct site *st, uint32_t next)
     case SITE_SENTMSG1:
        state_assert(st,st->state==SITE_RUN || st->state==SITE_RESOLVE);
        gen=generate_msg1;
+       st->msg1_crossed_logged = False;
        break;
     case SITE_SENTMSG2:
        state_assert(st,st->state==SITE_RUN || st->state==SITE_RESOLVE ||
@@ -1784,8 +1799,9 @@ static bool_t site_incoming(void *sst, struct buffer_if *buf,
               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");
+               if (!st->msg1_crossed_logged++)
+                   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 "
@@ -2249,11 +2265,18 @@ static void transport_record_peers(struct site *st, transport_peers *peers,
      *
      * Caller must first call transport_peers_expire. */
 
-    if (naddrs==1 && peers->npeers>=1 &&
-       comm_addr_equal(&addrs[0], &peers->peers[0].addr)) {
-       /* optimisation, also avoids debug for trivial updates */
-       peers->peers[0].last = *tv_now;
-       return;
+    if (naddrs==1) {
+       /* avoids debug for uninteresting updates */
+       int i;
+       for (i=0; i<peers->npeers; i++) {
+           if (comm_addr_equal(&addrs[0], &peers->peers[i].addr)) {
+               memmove(peers->peers+1, peers->peers,
+                       sizeof(peers->peers[0]) * i);
+               peers->peers[0].addr = addrs[0];
+               peers->peers[0].last = *tv_now;
+               return;
+           }
+       }
     }
 
     int old_npeers=peers->npeers;