[S] Change in openvpn[master]: ssl: reject a pushed epoch data format tag with a non-AEAD cipher

Attention is currently required from: ordex.

Hello plaisthos, 

I'd like you to reexamine a change. Please visit

    http://gerrit.openvpn.net/c/openvpn/+/1836?usp=email

to look at the new patch set (#3).

The change is no longer submittable: checks~ChecksSubmitRule is unsatisfied now.


Change subject: ssl: reject a pushed epoch data format tag with a non-AEAD cipher
......................................................................

ssl: reject a pushed epoch data format tag with a non-AEAD cipher

The epoch data key format is defined for AEAD ciphers only. Both places
that enable it locally verify this - multi.c when picking the cipher to
push and ssl_ncp.c for p2p NCP - but the pulling side imports the
"aead-epoch" protocol flag without validating it against the cipher that
was actually negotiated.

A peer pushing "protocol-flags aead-epoch" together with a non-AEAD
cipher therefore makes us reach the M_FATAL in init_key_contexts() and
terminate the process. This can be triggered whenever a non-AEAD cipher
is part of our own --data-ciphers, which is not unusual in
configurations kept compatible with old peers, e.g.

  data-ciphers AES-256-GCM:AES-256-CBC

Validate the combination in do_deferred_options(), next to the existing
data v2 check, so that the mismatch is reported as an OPTIONS ERROR and
the connection is restarted. Turn the now unreachable M_FATAL in
init_key_contexts() into a session error as well, so that no future code
path can promote this to a process exit.

Change-Id: Icc0721fd4a910110507d06b4e941e9e6dbb11e9f
Signed-off-by: Antonio Quartulli <[email protected]>
---
M src/openvpn/init.c
M src/openvpn/ssl.c
2 files changed, 18 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/36/1836/3

diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index 0236886..84de986 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -2711,6 +2711,18 @@
         return false;
     }
 
+    /* The epoch data format is defined for AEAD ciphers only. A peer may push
+     * the tag along with a non-AEAD cipher, so this has to be checked here
+     * rather than trusted */
+    if (epoch_data && !cipher_kt_mode_aead(c->options.ciphername))
+    {
+        msg(D_PUSH_ERRORS,
+            "OPTIONS ERROR: Epoch key data format tag requires an AEAD "
+            "cipher, but '%s' was negotiated.",
+            c->options.ciphername);
+        return false;
+    }
+
 
     if (found & OPT_P_PUSH_MTU)
     {
diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index 4a4feaf..23c8ec8 100644
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -1428,10 +1428,14 @@
     {
         if (!cipher_kt_mode_aead(key_type->cipher))
         {
-            msg(M_FATAL,
-                "AEAD cipher (currently %s) "
+            /* The pulled options are validated in do_deferred_options(), so
+             * reaching this point means a code path escaped that check. Fail
+             * the session instead of the whole process */
+            msg(D_TLS_ERRORS,
+                "TLS Error: AEAD cipher (currently %s) "
                 "required for epoch data format.",
                 cipher_kt_name(key_type->cipher));
+            return KEY_GEN_FAILED;
         }
         init_epoch_keys(ks, multi, key_type, server, key2);
     }

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1836?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email

Gerrit-MessageType: newpatchset
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Icc0721fd4a910110507d06b4e941e9e6dbb11e9f
Gerrit-Change-Number: 1836
Gerrit-PatchSet: 3
Gerrit-Owner: ordex <[email protected]>
Gerrit-Reviewer: plaisthos <[email protected]>
Gerrit-CC: openvpn-devel <[email protected]>
Gerrit-Attention: ordex <[email protected]>

_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel