key_update: rework the rekeying logic

While RFC 8446 4.6.3 says that the sender of a KeyUpdate message
should only update its sending key, the previous implementation
updated both the sending and receiving keys, preventing that any
application data interleaved being decrypted.

This splits the key update logic into 2 phases: when sending a
KeyUpdate, only update the sending key, and when receiving a
KeyUpdate, only update the receiving key.  In both cases, KeyUpdate
messages are encrypted/decrypted with the old keys.

Signed-off-by: Daiki Ueno <ueno@gnu.org>

diff --git a/lib/gnutls_int.h b/lib/gnutls_int.h
index e0835200553e4bb9bc128250da57f35194d78df5..f3caea1170718a15303ad17f07dea4418c1ca31a 100644 (file)
--- a/lib/gnutls_int.h
+++ b/lib/gnutls_int.h
@@ -1672,7 +1672,7 @@ typedef struct {
 } internals_st;
 
 /* Maximum number of epochs we keep around. */
-#define MAX_EPOCH_INDEX 4
+#define MAX_EPOCH_INDEX 16
 
 #define reset_cand_groups(session)                                            \
        session->internals.cand_ec_group = session->internals.cand_dh_group = \

diff --git a/lib/tls13/key_update.c b/lib/tls13/key_update.c
index 41243651b59154aa9ae6708cc3eab47f87ae2228..beee1dc41a4822ab672477088451b696e719af4a 100644 (file)
--- a/lib/tls13/key_update.c
+++ b/lib/tls13/key_update.c
@@ -52,45 +52,47 @@ static inline int set_ktls_keys(gnutls_session_t session,
        return 0;
 }
 
-static int update_keys(gnutls_session_t session, hs_stage_t stage)
+static int update_sending_key(gnutls_session_t session, hs_stage_t stage)
 {
        int ret;
 
-       ret = _tls13_update_secret(session,
-                                  session->key.proto.tls13.temp_secret,
-                                  session->key.proto.tls13.temp_secret_size);
+       _gnutls_epoch_bump(session);
+       ret = _gnutls_epoch_dup(session, EPOCH_WRITE_CURRENT);
        if (ret < 0)
                return gnutls_assert_val(ret);
 
-       _gnutls_epoch_bump(session);
-       ret = _gnutls_epoch_dup(session, EPOCH_READ_CURRENT);
+       ret = _tls13_write_connection_state_init(session, stage);
        if (ret < 0)
                return gnutls_assert_val(ret);
 
-       /* If we send a key update during early start, only update our
-        * write keys */
-       if (session->internals.recv_state == RECV_STATE_EARLY_START) {
-               ret = _tls13_write_connection_state_init(session, stage);
+       if (IS_KTLS_ENABLED(session, GNUTLS_KTLS_SEND)) {
+               ret = set_ktls_keys(session, GNUTLS_KTLS_SEND);
                if (ret < 0)
                        return gnutls_assert_val(ret);
+       }
 
-               if (IS_KTLS_ENABLED(session, GNUTLS_KTLS_SEND))
-                       ret = set_ktls_keys(session, GNUTLS_KTLS_SEND);
-       } else {
-               ret = _tls13_connection_state_init(session, stage);
-               if (ret < 0)
-                       return gnutls_assert_val(ret);
+       return 0;
+}
 
-               if (IS_KTLS_ENABLED(session, GNUTLS_KTLS_SEND) &&
-                   stage == STAGE_UPD_OURS)
-                       ret = set_ktls_keys(session, GNUTLS_KTLS_SEND);
-               else if (IS_KTLS_ENABLED(session, GNUTLS_KTLS_RECV) &&
-                        stage == STAGE_UPD_PEERS)
-                       ret = set_ktls_keys(session, GNUTLS_KTLS_RECV);
-       }
+static int update_receiving_key(gnutls_session_t session, hs_stage_t stage)
+{
+       int ret;
+
+       _gnutls_epoch_bump(session);
+       ret = _gnutls_epoch_dup(session, EPOCH_READ_CURRENT);
+       if (ret < 0)
+               return gnutls_assert_val(ret);
+
+       ret = _tls13_read_connection_state_init(session, stage);
        if (ret < 0)
                return gnutls_assert_val(ret);
 
+       if (IS_KTLS_ENABLED(session, GNUTLS_KTLS_RECV)) {
+               ret = set_ktls_keys(session, GNUTLS_KTLS_RECV);
+               if (ret < 0)
+                       return gnutls_assert_val(ret);
+       }
+
        return 0;
 }
 
@@ -128,7 +130,13 @@ int _gnutls13_recv_key_update(gnutls_session_t session, gnutls_buffer_st *buf)
        switch (buf->data[0]) {
        case 0:
                /* peer updated its key, not requested our key update */
-               ret = update_keys(session, STAGE_UPD_PEERS);
+               ret = _tls13_update_secret(
+                       session, session->key.proto.tls13.temp_secret,
+                       session->key.proto.tls13.temp_secret_size);
+               if (ret < 0)
+                       return gnutls_assert_val(ret);
+
+               ret = update_receiving_key(session, STAGE_UPD_PEERS);
                if (ret < 0)
                        return gnutls_assert_val(ret);
 
@@ -141,7 +149,13 @@ int _gnutls13_recv_key_update(gnutls_session_t session, gnutls_buffer_st *buf)
                }
 
                /* peer updated its key, requested our key update */
-               ret = update_keys(session, STAGE_UPD_PEERS);
+               ret = _tls13_update_secret(
+                       session, session->key.proto.tls13.temp_secret,
+                       session->key.proto.tls13.temp_secret_size);
+               if (ret < 0)
+                       return gnutls_assert_val(ret);
+
+               ret = update_receiving_key(session, STAGE_UPD_PEERS);
                if (ret < 0)
                        return gnutls_assert_val(ret);
 
@@ -248,7 +262,13 @@ int gnutls_session_key_update(gnutls_session_t session, unsigned flags)
        _gnutls_epoch_gc(session);
 
        /* it was completely sent, update the keys */
-       ret = update_keys(session, STAGE_UPD_OURS);
+       ret = _tls13_update_secret(session,
+                                  session->key.proto.tls13.temp_secret,
+                                  session->key.proto.tls13.temp_secret_size);
+       if (ret < 0)
+               return gnutls_assert_val(ret);
+
+       ret = update_sending_key(session, STAGE_UPD_OURS);
        if (ret < 0)
                return gnutls_assert_val(ret);