[Openvpn-devel,1/3] Allows renegotiation only to start if session is fully established

Message ID 20220909195902.2011798-1-arne@rfc2549.org
State New
Headers show
Series [Openvpn-devel,1/3] Allows renegotiation only to start if session is fully established | expand

Commit Message

Arne Schwabe Sept. 9, 2022, 7:59 p.m. UTC
This change makes the state machine more strict in terms of transation
that are allowed. The benefit of this change are two:

 - allows any option that might be pushed to affect renegotiation consistently
   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>
---
 src/openvpn/ssl.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

Patch

diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index 002871288..36a236fe3 100644
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -3011,7 +3011,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
@@ -3733,9 +3733,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))