From: Gert Doering <gert@greenie.muc.de>
Date: Mon, 22 Aug 2016 20:24:47 +0000 (+0200)
Subject: Fix problems with NCP and --inetd.
X-Git-Tag: v2.4_alpha1~46
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=d90249f73353c175ed9e7dd0a450cd084a729e20;p=thirdparty%2Fopenvpn.git

Fix problems with NCP and --inetd.

NCP only works with --pull or --mode server, leading to breakage
in --inetd mode (because that has --tls-server, but not --mode server,
but clients can still ask for PUSH_REQUEST).

Fix by turning off o->ncp_enable unless (pull or mode server), and
double-fix by logging an appropriate message and refusing to change
ciphers if the server has already set up its keys.

v2: wrap long msg() text lines

Trac: 715
Signed-off-by: Gert Doering <gert@greenie.muc.de>
Acked-by: Steffan Karger <steffan@karger.me>
Message-Id: 1471897487-8354-1-git-send-email-gert@greenie.muc.de
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg00060.html
Signed-off-by: David Sommerseth <davids@openvpn.net>
---

diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index c100d4ca0..e052042f3 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -2614,6 +2614,15 @@ options_postprocess_mutate (struct options *o)
       if (streq (o->dh_file, "none"))
 	o->dh_file = NULL;
     }
+
+  /* cipher negotiation (NCP) currently assumes --pull or --mode server */
+  if ( o->ncp_enabled &&
+        ! (o->pull || o->mode == MODE_SERVER) )
+    {
+      msg( M_WARN, "disabling NCP mode (--ncp-disable) because not "
+                   "in P2MP client or server mode" );
+      o->ncp_enabled = false;
+    }
 #endif
 
 #if ENABLE_MANAGEMENT
diff --git a/src/openvpn/push.c b/src/openvpn/push.c
index 000c82f93..a1b999e2d 100644
--- a/src/openvpn/push.c
+++ b/src/openvpn/push.c
@@ -321,11 +321,24 @@ prepare_push_reply (struct options *o, struct tls_multi *tls_multi)
   /* Push cipher if client supports Negotiable Crypto Parameters */
   if (tls_peer_info_ncp_ver (peer_info) >= 2 && o->ncp_enabled)
     {
-      /* Push the first cipher from --ncp-ciphers to the client.
-       * TODO: actual negotiation, instead of server dictatorship. */
-      char *push_cipher = string_alloc(o->ncp_ciphers, &o->gc);
-      o->ciphername = strtok (push_cipher, ":");
-      push_option_fmt(o, M_USAGE, "cipher %s", o->ciphername);
+      /* if we have already created our key, we cannot change our own
+       * cipher, so disable NCP and warn = explain why
+       */
+      struct tls_session *session = &tls_multi->session[TM_ACTIVE];
+      if ( session->key[KS_PRIMARY].crypto_options.key_ctx_bi.initialized )
+	{
+	   msg( M_INFO, "PUSH: client wants to negotiate cipher (NCP), but "
+			"server has already generated data channel keys, "
+			"ignoring client request" );
+	}
+      else
+	{
+	  /* Push the first cipher from --ncp-ciphers to the client.
+	   * TODO: actual negotiation, instead of server dictatorship. */
+	  char *push_cipher = string_alloc(o->ncp_ciphers, &o->gc);
+	  o->ciphername = strtok (push_cipher, ":");
+	  push_option_fmt(o, M_USAGE, "cipher %s", o->ciphername);
+	}
     }
   return true;
 }