site: Send NAKs for undecryptable data packets (msg0)

Packets which are not understood are supposed in general to produce
NAKs, to let the peer know that we don't have the key they were using.

However, previously this would only happen if the incoming packet had
a local site index which was not in use.  But it can happen that a
particular index value is reused by a recently restarted secnet.
Previously, in this case, data packets would simply be thrown away
undecryptable.

With this change, undecryptable data packets always generate a NAK.
(But we don't generate a NAK if the packet was decryptable, but
suffered from sequence number skew - if we did, then network glitches
would trigger spurious key exchanges.)

This is particularly relevant for mobile sites, as it can happen that
the fixed site doesn't have an address for the mobile site - so the
association will remain stuck in a broken state until the mobile site
is also restarted.

There is still a potential problem when a site is restarted mid key
exchange.  The peer will refuse to start a new key exchange (because
of the retry timeout) and the restarted site may not know it's
necessary.  This will be dealt with in a later patch.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>

diff --git a/site.c b/site.c
index a6fa9da..8fa4c11 100644 (file)
--- a/site.c
+++ b/site.c
@@ -731,7 +731,8 @@ 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 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;
@@ -750,11 +751,8 @@ static bool_t decrypt_msg0(struct site *st, struct buffer_if *msg0)
                           "peer has used new key","auxiliary key",LOG_SEC);
        return True;
     }
-
-    if (problem==2) {
-       slog(st,LOG_DROP,"transform: %s (merely skew)",transform_err);
-       return False;
-    }
+    if (problem==2)
+       goto skew;
 
     buffer_copy(msg0, &st->scratch);
     problem = st->auxiliary_key.transform->reverse
@@ -778,11 +776,14 @@ static bool_t decrypt_msg0(struct site *st, struct buffer_if *msg0)
        }
        return True;
     }
+    if (problem==2)
+       goto skew;
 
     if (st->state==SITE_SENTMSG5) {
        buffer_copy(msg0, &st->scratch);
-       if (!st->new_transform->reverse(st->new_transform->st,
-                                       msg0,&newkey_err)) {
+       problem = st->new_transform->reverse(st->new_transform->st,
+                                            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)");
@@ -791,11 +792,18 @@ static bool_t decrypt_msg0(struct site *st, struct buffer_if *msg0)
            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;
 }
 
@@ -804,7 +812,7 @@ static bool_t process_msg0(struct site *st, struct buffer_if *msg0,
 {
     uint32_t type;
 
-    if (!decrypt_msg0(st,msg0))
+    if (!decrypt_msg0(st,msg0,src))
        return False;
 
     CHECK_AVAIL(msg0,4);