]> git.ipfire.org Git - thirdparty/openvpn.git/commitdiff
Allows renegotiation only to start if session is fully established
authorArne Schwabe <arne@rfc2549.org>
Fri, 9 Sep 2022 19:59:00 +0000 (21:59 +0200)
committerGert Doering <gert@greenie.muc.de>
Tue, 18 Oct 2022 08:25:20 +0000 (10:25 +0200)
This change makes the state machine more strict in terms of transaction
that are allowed. The benefit of this change are twofold:

 - only allow renegotiations after pushed option handling is done,
   to ensure that pushed options which might affect renegotiation
   have been processed on both sides
   This is a prerequisite for the upcoming secure renegotiation patch set
 - avoids corner cases of a peer (or an attacker) trying to renegotiate the
   session while the original session is not fully setup. Currently there
   there are no problems known with this but it is better to avoid the
   corner case in the first time.

Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Heiko Hund <heiko@ist.eigentlich.net>
Message-Id: <20220909195902.2011798-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25162.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
src/openvpn/ssl.c

index 66aa9da3da4b47fb6e5d2be29b65d7d04f2ce433..d9b21819181a8e5868b2f0ba6b53916e06585c61 100644 (file)
@@ -2904,7 +2904,7 @@ tls_process(struct tls_multi *multi,
     ASSERT(session_id_defined(&session->session_id));
 
     /* Should we trigger a soft reset? -- new key, keeps old key for a while */
-    if (ks->state >= S_ACTIVE
+    if (ks->state >= S_GENERATED_KEYS
         && ((session->opt->renegotiate_seconds
              && now >= ks->established + session->opt->renegotiate_seconds)
             || (session->opt->renegotiate_bytes > 0
@@ -3591,9 +3591,11 @@ tls_pre_decrypt(struct tls_multi *multi,
         }
 
         /*
-         * Remote is requesting a key renegotiation
+         * Remote is requesting a key renegotiation.  We only allow renegotiation
+         * when the previous session is fully established to avoid weird corner
+         * cases.
          */
-        if (op == P_CONTROL_SOFT_RESET_V1 && TLS_AUTHENTICATED(multi, ks))
+        if (op == P_CONTROL_SOFT_RESET_V1 && ks->state >= S_GENERATED_KEYS)
         {
             if (!read_control_auth(buf, &session->tls_wrap, from,
                                    session->opt))