From 07e4774c32915eeb1d480854a4a10ec91160b57d Mon Sep 17 00:00:00 2001 From: Ian Jackson Date: Wed, 20 Jun 2012 21:24:26 +0100 Subject: [PATCH] site, transform: Do not initiate rekey when packets too much out of order 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 --- secnet.h | 6 +++++- site.c | 5 +++++ transform.c | 2 +- 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/secnet.h b/secnet.h index 5f8e43b..b276798 100644 --- 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 --- 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; diff --git a/transform.c b/transform.c index f55aa44..f1da564 100644 --- a/transform.c +++ b/transform.c @@ -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; -- 2.30.2