Fix client NCP OCC fallback when server and client cipher are identical

If we do not get a cipher pushed we call tls_poor_mans_ncp to determine
whether we can use the server's cipher. Inherited from OpenVPN
2.4's code we only did this check when the ciphers were different.
Since OpenVPN 2.5 does not assume that our cipher we report in OCC
(options->ciphername) is always a valid cipher we always need to perform
this check.

V2: Only call tls_item_in_cipher_list if remote_cipher is non-null to
    avoid calling strcmp with NULL.

Reported-By: Rafael Gava <gava100@gmail.com>
Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20200830131440.10933-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg20843.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>

diff --git a/src/openvpn/ssl_ncp.c b/src/openvpn/ssl_ncp.c
index c9ab85ce2793070ae46f2010e0df45533478a543..55496395d0146e8b73f874d90bc5458e75705b13 100644 (file)
--- a/src/openvpn/ssl_ncp.c
+++ b/src/openvpn/ssl_ncp.c
@@ -269,14 +269,11 @@ static bool
 tls_poor_mans_ncp(struct options *o, const char *remote_ciphername)
 {
     if (remote_ciphername
-        && 0 != strcmp(o->ciphername, remote_ciphername))
+        && tls_item_in_cipher_list(remote_ciphername, o->ncp_ciphers))
     {
-        if (tls_item_in_cipher_list(remote_ciphername, o->ncp_ciphers))
-        {
-            o->ciphername = string_alloc(remote_ciphername, &o->gc);
-            msg(D_TLS_DEBUG_LOW, "Using peer cipher '%s'", o->ciphername);
-            return true;
-        }
+        o->ciphername = string_alloc(remote_ciphername, &o->gc);
+        msg(D_TLS_DEBUG_LOW, "Using peer cipher '%s'", o->ciphername);
+        return true;
     }
     return false;
 }