chiark / gitweb /
site, transform: Do not initiate rekey when packets too much out of order
authorIan Jackson <ijackson@chiark.greenend.org.uk>
Wed, 20 Jun 2012 20:24:26 +0000 (21:24 +0100)
committerIan Jackson <ijackson@chiark.greenend.org.uk>
Thu, 12 Jul 2012 19:02:21 +0000 (20:02 +0100)
If packets arrive sufficiently out of order, we may unnecessarily
initiate a rekey simply because of the skew.  Solve this as follows:
 * transform->reverse gives a distinct error code for `too much skew'.
 * site does not initiate a rekey when it sees that error code.

The rekey-on-decrypt-failure feature is in principle unnecessary.
However, due to another bug it is possible for a key setup to be
regarded as successful at only one of the two ends, and the ability to
immediately do another key setup to try to fix things is useful.

In principle this new behaviour might prevent a necessary rekey: if
the sender has sent around 2^31 packets none of which were received,
the receiver would see the new packets as too old.  But: firstly, this
is very unlikely (it would have involved transmitting several Gbytes
into the void, in a period less than the maximum key lifetime).
Secondly, we ought to expire keys before then anyway so the sender
should know that the key had `worn out' (although currently this is
not enforced).

The benefit of this change is that the old behaviour would be likely
to initiate unnecessary rekeys when on unreliable links (eg, mobile
internet).  Unnecessary rekeys are a bad idea in these circumstances
not just because they clog up the link but also because they make the
association more fragile.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
secnet.h
site.c
transform.c

index 5f8e43b..b276798 100644 (file)
--- a/secnet.h
+++ b/secnet.h
@@ -389,7 +389,11 @@ typedef struct transform_inst_if *transform_createinstance_fn(void *st);
 typedef bool_t transform_setkey_fn(void *st, uint8_t *key, int32_t keylen);
 typedef void transform_delkey_fn(void *st);
 typedef void transform_destroyinstance_fn(void *st);
-/* Returns 0 for 'all is well', any other value for a problem */
+/* Returns:
+ *   0: all is well
+ *   1: for any other problem
+ *   2: message decrypted but sequence number was out of range
+ */
 typedef uint32_t transform_apply_fn(void *st, struct buffer_if *buf,
                                    const char **errmsg);
 
diff --git a/site.c b/site.c
index 4d3a612..4dbebaf 100644 (file)
--- a/site.c
+++ b/site.c
@@ -721,6 +721,11 @@ static bool_t decrypt_msg0(struct site *st, struct buffer_if *msg0)
                                             msg0,&transform_err);
     if (!problem) return True;
 
+    if (problem==2) {
+       slog(st,LOG_DROP,"transform: %s (merely skew)",transform_err);
+       return False;
+    }
+
     slog(st,LOG_SEC,"transform: %s",transform_err);
     initiate_key_setup(st,"incoming message would not decrypt");
     return False;
index f55aa44..f1da564 100644 (file)
@@ -247,7 +247,7 @@ static uint32_t transform_reverse(void *sst, struct buffer_if *buf,
     } else {
        /* Too much skew */
        *errmsg="seqnum: too much skew";
-       return 1;
+       return 2;
     }
     
     return 0;