[Openvpn-devel] Add warning if a p2p NCP client connects to a p2mp server

Message ID 20231009105336.34267-1-frank@lichtenheld.com
State Accepted
Headers show
Series [Openvpn-devel] Add warning if a p2p NCP client connects to a p2mp server | expand

Commit Message

Frank Lichtenheld Oct. 9, 2023, 10:53 a.m. UTC
From: Arne Schwabe <arne@rfc2549.org>

Change-Id: I85ae4e1167e1395b4f59d5d0ecf6c38befcaa8a7
Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
---

This change was reviewed on Gerrit and approved by at least one
developer. I request to merge it to master.

Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/323
This mail reflects revision 3 of this Change.
Acked-by according to Gerrit (reflected above):
Frank Lichtenheld <frank@lichtenheld.com>

Comments

Gert Doering Oct. 18, 2023, 10:37 a.m. UTC | #1
Lightly tested on the regular server testbed - "normal" connections
work just fine, connections from a client without --client fail (as
expected), and the server logs

  2023-10-18 11:56:42 Note: peer reports running in P2P mode (no --pull/--clientoption). It will not negotiate ciphers with this server. Expect this connection to fail.

"old" P2P NCP mode still works just fine, with no spurious messages.

Your patch has been applied to the master and release/2.6 branch
(mini-feature with minimal impact, aid troubleshooting).

commit 2574ae5e6961ed5b39531a7f98e537f72f87bcfb (master)
commit 3985da96215b8107111d2c0a1cd810e86b210cd1 (release/2.6)
Author: Arne Schwabe
Date:   Mon Oct 9 12:53:36 2023 +0200

     Add warning if a p2p NCP client connects to a p2mp server

     Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
     Message-Id: <20231009105336.34267-1-frank@lichtenheld.com>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg27191.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering

Patch

diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index 0d4e6f9..8b490ed 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -1811,6 +1811,15 @@ 
         return false;
     }
 
+    /* Print a warning if we detect the client being in P2P mode and will
+     * not accept our pushed ciphers */
+    if (proto & IV_PROTO_NCP_P2P)
+    {
+        msg(M_WARN, "Note: peer reports running in P2P mode (no --pull/--client"
+            "option). It will not negotiate ciphers with this server. "
+            "Expect this connection to fail.");
+    }
+
     if (proto & IV_PROTO_REQUEST_PUSH)
     {
         c->c2.push_request_received = true;
diff --git a/src/openvpn/ssl_ncp.c b/src/openvpn/ssl_ncp.c
index dafaef1..0ca6d42 100644
--- a/src/openvpn/ssl_ncp.c
+++ b/src/openvpn/ssl_ncp.c
@@ -24,7 +24,7 @@ 
  */
 
 /**
- * @file Control Channel SSL/Data dynamic negotion Module
+ * @file Control Channel SSL/Data dynamic negotiation Module
  * This file is split from ssl.c to be able to unit test it.
  */
 
@@ -258,8 +258,8 @@ 
 
     const char *peer_ncp_list = tls_peer_ncp_list(peer_info, &gc_tmp);
 
-    /* non-NCP client without OCC?  "assume nothing" */
-    /* For client doing the newer version of NCP (that send IV_CIPHER)
+    /* non-NCP clients without OCC?  "assume nothing" */
+    /* For client doing the newer version of NCP (that send IV_CIPHERS)
      * we cannot assume that they will accept remote_cipher */
     if (remote_cipher == NULL
         || (peer_info && strstr(peer_info, "IV_CIPHERS=")))
diff --git a/src/openvpn/ssl_ncp.h b/src/openvpn/ssl_ncp.h
index d27ed24..de7a0e4 100644
--- a/src/openvpn/ssl_ncp.h
+++ b/src/openvpn/ssl_ncp.h
@@ -23,7 +23,7 @@ 
  */
 
 /**
- * @file Control Channel SSL/Data dynamic negotion Module
+ * @file Control Channel SSL/Data dynamic negotiation Module
  * This file is split from ssl.h to be able to unit test it.
  */