Re: [Openvpn-devel] [PATCH] Repair --inetd

2020-07-24 Thread Arne Schwabe
Am 24.07.20 um 20:13 schrieb Gert Doering:
> commit 25a422cc60 deprecated --inetd, which is still something we want.
> 
> Unlike all "usual" deprecated option warnings, we cannot print this at
> option parsing time, because we need logging to be set up first - otherwise
> the deprecation warning is sent via the socket (on stdin/stdout)
> towards the connecting client, totally breaking this mode.
> 
> (Which is why we want to deprecate it: too special even for us)


Acked-By: Arne Schwabe 





signature.asc
Description: OpenPGP digital signature
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


[Openvpn-devel] [PATCH applied] Re: Improve Windows version detection with manifest

2020-07-24 Thread Gert Doering
Acked-by: Gert Doering 

Thanks for the v2.  Builds nicely with "./build-snapshot" on my 
ubuntu 18.04 builder now.  I have not tested the resulting installer.

The code change for the C code look good to me.

Thanks for the explanation about the manifest ("I've heard that 
before, but I won't ever become a windows programmer, it seems"),
and I assume that the .vcproj adaptions are related to that.

You are the expert on MSVC building, so if you say this is needed,
and it does not look harmful, I'm fine :-)

Your patch has been applied to the master branch.

commit a7d6977e6e14c512c89e51886b235af153cd4841
Author: Lev Stipakov
Date:   Fri Jul 24 22:56:34 2020 +0300

 Improve Windows version detection with manifest

 Signed-off-by: Lev Stipakov 
 Acked-by: Gert Doering 
 Message-Id: <20200724195634.493-1-lstipa...@gmail.com>
 URL: 
https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg20580.html
 Signed-off-by: Gert Doering 


--
kind regards,

Gert Doering



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH] Improve Windows version detection with manifest

2020-07-24 Thread Gert Doering
Hi,

On Fri, Jul 24, 2020 at 10:49:07PM +0300, Lev Stipakov wrote:
> > How do you build?  I use the "build-snapshot" script, and that does
> > a "make dist" somewhere in between.
> 
> ubuntu@ip-172-31-38-110:~/openvpn-build/windows-nsis$
> OPENVPN_VERSION=fix-remove-impersonation
> OPENVPN_URL=https://github.com/lstipakov/openvpn/archive/fix/remove-impersonation.zip
> ./build-complete --use-depcache

Ah, you point it at (effectively) a ready-made tarball.

I use "build-snapshot" which fetches a git repo + branch

Maybe build-complete does the same if "OPENVPN_URL" points to a
git repo... dunno.  I have

OPENVPN_URL="git://github.greenie.net/openvpn-236.git"
OPENVPN_VERSION="${OPENVPN_VERSION:-2.5_git}"; export OPENVPN_VERSION
OPENVPN_BRANCH="${OPENVPN_BRANCH:-master}"

gert

-- 
"If was one thing all people took for granted, was conviction that if you 
 feed honest figures into a computer, honest figures come out. Never doubted 
 it myself till I met a computer with a sense of humor."
 Robert A. Heinlein, The Moon is a Harsh Mistress

Gert Doering - Munich, Germany g...@greenie.muc.de


signature.asc
Description: PGP signature
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


[Openvpn-devel] [PATCH v2] Improve Windows version detection with manifest

2020-07-24 Thread Lev Stipakov
From: Lev Stipakov 

Add manifest file to detect Windows versions greater than Windows 8.

Below is example output on Windows 10.

Before:
Windows version 6.2 (Windows 8 or greater) 64bit

After:
Windows version 10.0 (Windows 10 or greater) 64bit

Signed-off-by: Lev Stipakov 
---

 v2: 
 - fix build from tarball (added manifest file to EXTRA_DIST)
 - fix build on old mingw like ubuntu 18.04 (added missig define)

 src/compat/compat-versionhelpers.h | 10 
 src/openvpn/Makefile.am|  3 ++-
 src/openvpn/openvpn.manifest   | 33 ++
 src/openvpn/openvpn.vcxproj| 15 
 src/openvpn/openvpn.vcxproj.filters|  5 
 src/openvpn/openvpn_win32_resources.rc |  2 ++
 src/openvpn/win32.c| 20 ++--
 src/openvpn/win32.h|  8 ---
 8 files changed, 90 insertions(+), 6 deletions(-)
 create mode 100644 src/openvpn/openvpn.manifest

diff --git a/src/compat/compat-versionhelpers.h 
b/src/compat/compat-versionhelpers.h
index 251fb047..9e25470e 100644
--- a/src/compat/compat-versionhelpers.h
+++ b/src/compat/compat-versionhelpers.h
@@ -18,6 +18,10 @@
 
 #define _WIN32_WINNT_WINBLUE0x0603
 
+#ifndef _WIN32_WINNT_WINTHRESHOLD
+#define _WIN32_WINNT_WINTHRESHOLD0x0A00 // Windows 10
+#endif
+
 VERSIONHELPERAPI
 IsWindowsVersionOrGreater(WORD major, WORD minor, WORD servpack)
 {
@@ -95,6 +99,12 @@ IsWindows8Point1OrGreater(void)
 return IsWindowsVersionOrGreater(HIBYTE(_WIN32_WINNT_WINBLUE), 
LOBYTE(_WIN32_WINNT_WINBLUE), 0);
 }
 
+VERSIONHELPERAPI
+IsWindows10OrGreater()
+{
+return IsWindowsVersionOrGreater(HIBYTE(_WIN32_WINNT_WINTHRESHOLD), 
LOBYTE(_WIN32_WINNT_WINTHRESHOLD), 0);
+}
+
 VERSIONHELPERAPI
 IsWindowsServer(void)
 {
diff --git a/src/openvpn/Makefile.am b/src/openvpn/Makefile.am
index 47ad762d..37b002c6 100644
--- a/src/openvpn/Makefile.am
+++ b/src/openvpn/Makefile.am
@@ -16,7 +16,8 @@ MAINTAINERCLEANFILES = \
 
 EXTRA_DIST = \
openvpn.vcxproj \
-   openvpn.vcxproj.filters
+   openvpn.vcxproj.filters \
+   openvpn.manifest
 
 AM_CPPFLAGS = \
-I$(top_srcdir)/include \
diff --git a/src/openvpn/openvpn.manifest b/src/openvpn/openvpn.manifest
new file mode 100644
index ..fa5b3d7f
--- /dev/null
+++ b/src/openvpn/openvpn.manifest
@@ -0,0 +1,33 @@
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
diff --git a/src/openvpn/openvpn.vcxproj b/src/openvpn/openvpn.vcxproj
index bd1a5844..5367979d 100644
--- a/src/openvpn/openvpn.vcxproj
+++ b/src/openvpn/openvpn.vcxproj
@@ -70,6 +70,18 @@
   
 <_ProjectFileVersion>10.0.30319.1
   
+  
+false
+  
+  
+false
+  
+  
+false
+  
+  
+false
+  
   
 
   
..\compat;$(TAP_WINDOWS_HOME)/include;$(OPENSSL_HOME)/include;$(LZO_HOME)/include;$(PKCS11H_HOME)/include;%(AdditionalIncludeDirectories)
@@ -311,6 +323,9 @@
   false
 
   
+  
+
+  
   
   
   
diff --git a/src/openvpn/openvpn.vcxproj.filters 
b/src/openvpn/openvpn.vcxproj.filters
index e0bc7d5e..cf5748c7 100644
--- a/src/openvpn/openvpn.vcxproj.filters
+++ b/src/openvpn/openvpn.vcxproj.filters
@@ -515,4 +515,9 @@
   Resource Files
 
   
+  
+
+  Resource Files
+
+  
 
\ No newline at end of file
diff --git a/src/openvpn/openvpn_win32_resources.rc 
b/src/openvpn/openvpn_win32_resources.rc
index e4f1ee95..1ea5f878 100644
--- a/src/openvpn/openvpn_win32_resources.rc
+++ b/src/openvpn/openvpn_win32_resources.rc
@@ -7,6 +7,8 @@
 
 #pragma code_page(65001) /* UTF8 */
 
+1 RT_MANIFEST "openvpn.manifest"
+
 LANGUAGE LANG_NEUTRAL, SUBLANG_NEUTRAL
 
 VS_VERSION_INFO VERSIONINFO
diff --git a/src/openvpn/win32.c b/src/openvpn/win32.c
index eb4c0307..7e913165 100644
--- a/src/openvpn/win32.c
+++ b/src/openvpn/win32.c
@@ -1417,10 +1417,18 @@ win32_version_info(void)
 {
 return WIN_7;
 }
-else
+
+if (!IsWindows8Point1OrGreater())
 {
 return WIN_8;
 }
+
+if (!IsWindows10OrGreater())
+{
+return WIN_8_1;
+}
+
+return WIN_10;
 }
 
 bool
@@ -1458,7 +1466,15 @@ win32_version_string(struct gc_arena *gc, bool add_name)
 break;
 
 case WIN_8:
-buf_printf(, "6.2%s", add_name ? " (Windows 8 or greater)" : 
"");
+buf_printf(, "6.2%s", add_name ? " (Windows 8)" : "");
+break;
+
+case WIN_8_1:
+buf_printf(, "6.3%s", add_name ? " (Windows 8.1)" : "");
+break;
+
+case WIN_10:
+buf_printf(, "10.0%s", add_name ? " (Windows 10 or greater)" : 
"");
 break;
 
 default:
diff --git a/src/openvpn/win32.h b/src/openvpn/win32.h
index 79504776..da85ed4d 100644
--- a/src/openvpn/win32.h
+++ 

Re: [Openvpn-devel] [PATCH] Improve Windows version detection with manifest

2020-07-24 Thread Lev Stipakov
> How do you build?  I use the "build-snapshot" script, and that does
> a "make dist" somewhere in between.

ubuntu@ip-172-31-38-110:~/openvpn-build/windows-nsis$
OPENVPN_VERSION=fix-remove-impersonation
OPENVPN_URL=https://github.com/lstipakov/openvpn/archive/fix/remove-impersonation.zip
./build-complete --use-depcache

-- 
-Lev


___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH] Improve Windows version detection with manifest

2020-07-24 Thread Gert Doering
Hi,

On Fri, Jul 24, 2020 at 10:34:17PM +0300, Lev Stipakov wrote:
> 2) When I tested mingw build, I pointed source url to git repo, not to
> tarball - that's why it worked.

How do you build?  I use the "build-snapshot" script, and that does
a "make dist" somewhere in between.

> That XML is required (no matter msvc or mingw) to tell Windows that
> your app is Win10-aware and allows you
> to use API which detects Windows >= 8.1

Thanks for explaining.

> I'll send V2 with above issues fixed.

Thanks :)

gert
-- 
"If was one thing all people took for granted, was conviction that if you 
 feed honest figures into a computer, honest figures come out. Never doubted 
 it myself till I met a computer with a sense of humor."
 Robert A. Heinlein, The Moon is a Harsh Mistress

Gert Doering - Munich, Germany g...@greenie.muc.de


signature.asc
Description: PGP signature
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH] Improve Windows version detection with manifest

2020-07-24 Thread Lev Stipakov
Hm.. I tested mingw build (windows-nsis) on Ubuntu 19.04 and
everything worked fine.

That's because:

1) Ubuntu 19.04 probably has recent enough mingw with _WIN32_WINNT_WINTHRESHOLD

2) When I tested mingw build, I pointed source url to git repo, not to
tarball - that's why it worked.

That XML is required (no matter msvc or mingw) to tell Windows that
your app is Win10-aware and allows you
to use API which detects Windows >= 8.1

I'll send V2 with above issues fixed.

pe 24. heinäk. 2020 klo 21.40 Gert Doering (g...@greenie.muc.de) kirjoitti:
>
> Hi,
>
> On Fri, Jul 24, 2020 at 05:14:45PM +0300, Lev Stipakov wrote:
> > From: Lev Stipakov 
> >
> > Add manifest file to detect Windows versions greater than Windows 8.
>
> NAK, this breaks Ubuntu/MinGW builds.  Actually, I think it breaks all
> windows builds, if you build from tarballs (which the "generic" build does -
> first, make dist, then, build from there).
>
> You introduce an include to "openvpn.manifest", but this is not bundled,
> so
>
> i686-w64-mingw32-windres -DHAVE_CONFIG_H -I. -I../.. -I../../include  
> -I../../include -I../../src/compat -DWIN32_LEAN_AND_MEAN 
> -DNTDDI_VERSION=NTDDI_VISTA -D_WIN32_WINNT=_WIN32_WINNT_VISTA -i 
> "openvpn_win32_resources.rc" -o "openvpn_win32_resources.o"
> i686-w64-mingw32-windres: can't open file `openvpn.manifest': No such file or 
> directory
> Makefile:933: recipe for target 'openvpn_win32_resources.o' failed
> make[4]: *** [openvpn_win32_resources.o] Error 1
> make[4]: *** Waiting for unfinished jobs
>
> And this is what breaks MinGW builds:
>
> ../../src/compat/compat-versionhelpers.h: In function 'IsWindows10OrGreater':
> ../../src/compat/compat-versionhelpers.h:101:45: error: 
> '_WIN32_WINNT_WINTHRESHOLD' undeclared (first use in this function); did you 
> mean '_WIN32_WINNT_WINBLUE'?
>  return IsWindowsVersionOrGreater(HIBYTE(_WIN32_WINNT_WINTHRESHOLD), 
> LOBYTE(_WIN32_WINNT_WINTHRESHOLD), 0);
>  ^
> ../../src/compat/compat-versionhelpers.h:101:45: note: each undeclared 
> identifier is reported only once for each function it appears in
>
>
> possibly the _WIN32_WINNT_WINTHRESHOLD is missing on MinGW, at least the
> version bundled with Ubuntu 18.04 (I've skimmed the headers, and it's
> missing).
>
>
> The rest of the code changes look good, and I have no idea what all the
> XML files do - but those are relevant for MSVC, and you are the expert
> on that ("if you say this is beneficial, I'll merge").
>
> gert
>
> --
> "If was one thing all people took for granted, was conviction that if you
>  feed honest figures into a computer, honest figures come out. Never doubted
>  it myself till I met a computer with a sense of humor."
>  Robert A. Heinlein, The Moon is a Harsh Mistress
>
> Gert Doering - Munich, Germany g...@greenie.muc.de



-- 
-Lev


___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH] Improve Windows version detection with manifest

2020-07-24 Thread Gert Doering
Hi,

On Fri, Jul 24, 2020 at 05:14:45PM +0300, Lev Stipakov wrote:
> From: Lev Stipakov 
> 
> Add manifest file to detect Windows versions greater than Windows 8.

NAK, this breaks Ubuntu/MinGW builds.  Actually, I think it breaks all 
windows builds, if you build from tarballs (which the "generic" build does -
first, make dist, then, build from there).

You introduce an include to "openvpn.manifest", but this is not bundled,
so

i686-w64-mingw32-windres -DHAVE_CONFIG_H -I. -I../.. -I../../include  
-I../../include -I../../src/compat -DWIN32_LEAN_AND_MEAN 
-DNTDDI_VERSION=NTDDI_VISTA -D_WIN32_WINNT=_WIN32_WINNT_VISTA -i 
"openvpn_win32_resources.rc" -o "openvpn_win32_resources.o"
i686-w64-mingw32-windres: can't open file `openvpn.manifest': No such file or 
directory
Makefile:933: recipe for target 'openvpn_win32_resources.o' failed
make[4]: *** [openvpn_win32_resources.o] Error 1
make[4]: *** Waiting for unfinished jobs

And this is what breaks MinGW builds:

../../src/compat/compat-versionhelpers.h: In function 'IsWindows10OrGreater':
../../src/compat/compat-versionhelpers.h:101:45: error: 
'_WIN32_WINNT_WINTHRESHOLD' undeclared (first use in this function); did you 
mean '_WIN32_WINNT_WINBLUE'?
 return IsWindowsVersionOrGreater(HIBYTE(_WIN32_WINNT_WINTHRESHOLD), 
LOBYTE(_WIN32_WINNT_WINTHRESHOLD), 0);
 ^
../../src/compat/compat-versionhelpers.h:101:45: note: each undeclared 
identifier is reported only once for each function it appears in


possibly the _WIN32_WINNT_WINTHRESHOLD is missing on MinGW, at least the
version bundled with Ubuntu 18.04 (I've skimmed the headers, and it's
missing).


The rest of the code changes look good, and I have no idea what all the
XML files do - but those are relevant for MSVC, and you are the expert
on that ("if you say this is beneficial, I'll merge").

gert

-- 
"If was one thing all people took for granted, was conviction that if you 
 feed honest figures into a computer, honest figures come out. Never doubted 
 it myself till I met a computer with a sense of humor."
 Robert A. Heinlein, The Moon is a Harsh Mistress

Gert Doering - Munich, Germany g...@greenie.muc.de


signature.asc
Description: PGP signature
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH] Remove unused no-op function

2020-07-24 Thread Gert Doering
Hi,

On Fri, Jul 24, 2020 at 02:26:31PM +0300, Lev Stipakov wrote:
> From: Lev Stipakov 
> 
> Body of check_subnet_conflict() was commented out
> (#if 0) back in 2011, so it is safe now to completely
> elimitate this function, including all calls to it.

I'm a bit sceptical about that one.  Can we make it *work* instead?

It's a good idea, and I've seen support calls with "hey, openvpn is
not working, why?" due to RFC address conflict - so having an indication
in the log might be helpful.

I agree that with "#if 0" this is just dead code.

Any indication in the commit why it was commented out?  Not working
right?

gert

-- 
"If was one thing all people took for granted, was conviction that if you 
 feed honest figures into a computer, honest figures come out. Never doubted 
 it myself till I met a computer with a sense of humor."
 Robert A. Heinlein, The Moon is a Harsh Mistress

Gert Doering - Munich, Germany g...@greenie.muc.de


signature.asc
Description: PGP signature
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


[Openvpn-devel] [PATCH] Repair --inetd

2020-07-24 Thread Gert Doering
commit 25a422cc60 deprecated --inetd, which is still something we want.

Unlike all "usual" deprecated option warnings, we cannot print this at
option parsing time, because we need logging to be set up first - otherwise
the deprecation warning is sent via the socket (on stdin/stdout)
towards the connecting client, totally breaking this mode.

(Which is why we want to deprecate it: too special even for us)

Signed-off-by: Gert Doering 
---
 src/openvpn/options.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 1a915e27..a8a9bb97 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -2050,6 +2050,11 @@ options_postprocess_verify_ce(const struct options 
*options, const struct connec
 msg(M_USAGE, "--inetd nowait only makes sense in --dev tap mode");
 }
 
+if (options->inetd)
+{
+msg(M_WARN, "DEPRECATED OPTION: --inetd mode is deprecated "
+"and will be removed in OpenVPN 2.6");
+}
 
 if (options->lladdr && dev != DEV_TYPE_TAP)
 {
@@ -5802,8 +5807,6 @@ add_option(struct options *options,
 }
 else if (streq(p[0], "inetd") && !p[3])
 {
-msg(M_WARN, "DEPRECATED OPTION: --inetd mode is deprecated "
-"and will be removed in OpenVPN 2.6");
 VERIFY_PERMISSION(OPT_P_GENERAL);
 if (!options->inetd)
 {
-- 
2.26.2



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


[Openvpn-devel] [PATCH v2 10/10] Add a note that ncp-ciphers is replaced by data-ciphers

2020-07-24 Thread Arne Schwabe
This patch adds a message that informs the user that the ncp-cipher
is renamed to data-ciphers. This should address the following concerns:

 - Users being confused by old options.
 - Nudge users to use the modern variant of an option

The man page already documents ncp-ciphers as an old name for
data-ciphers, so looking it up in the man page will also work.

Note that I did not add "deprecated old option" to this message
since I still think that eventually removing the option will only
break configs and we gain almost nothing from that.

Also still accepting the option even though we do not recommend usage of
it also follows the robustness principle of:
"be strict in what you send and tolerant in what you receive"

Signed-off-by: Arne Schwabe 
---
 src/openvpn/options.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 5beaba0f..2cff9473 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -7939,7 +7939,11 @@ add_option(struct options *options,
 && p[1] && !p[2])
 {
 VERIFY_PERMISSION(OPT_P_GENERAL|OPT_P_INSTANCE);
+if (streq(p[0], "ncp-ciphers"))
+{
+msg(M_INFO, "Note: Treating option '--ncp-ciphers' as "
+" '--data-ciphers' (renamed in OpenVPN 2.5).");
+}
 options->ncp_ciphers = p[1];
 }
 else if (streq(p[0], "ncp-disable") && !p[1])
-- 
2.26.2



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH 10/10] Add a note that ncp-ciphers is replaced by data-ciphers

2020-07-24 Thread Arne Schwabe
Am 24.07.20 um 16:04 schrieb Arne Schwabe:
> This patch adds a message that informs the user that the ncp-cipher
> is renamed to data-ciphers. This should address the following concerns:
> 
>  - Users being confused by old options.
>  - Nudge users to use the modern variant of an option
> 
> The man page already documents ncp-ciphers as an old name for
> data-ciphers, so looking it up in the man page will also work.
> 
> Note that I did not add "deprecated old option" to this message
> since I still think that eventually removing the option will only
> break configs and we gain almost nothing from that.
> 
> Also still accepting the option even though we do not recommend usage of
> it also follows the robustness principle of:
> "be strict in what you send and tolerant in what you receive"
> 
> Signed-off-by: Arne Schwabe 
> ---
>  src/openvpn/options.c | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/src/openvpn/options.c b/src/openvpn/options.c
> index 5beaba0f..01f0ca0f 100644
> --- a/src/openvpn/options.c
> +++ b/src/openvpn/options.c
> @@ -7939,6 +7939,11 @@ add_option(struct options *options,
>  && p[1] && !p[2])
>  {
>  VERIFY_PERMISSION(OPT_P_GENERAL|OPT_P_INSTANCE);
> +if (streq(p[0], "ncp-ciphers"))
> +{
> +msg(M_INFO, "Note: Rewriting option '--ncp-ciphers' to "
> +" '--data-ciphers'");
> +}
>  options->ncp_ciphers = p[1];
>  }
>  else if (streq(p[0], "ncp-disable") && !p[1])
> 

Sorry, send out an old version. V2 incoming.

Arne



signature.asc
Description: OpenPGP digital signature
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


[Openvpn-devel] [PATCH] Improve Windows version detection with manifest

2020-07-24 Thread Lev Stipakov
From: Lev Stipakov 

Add manifest file to detect Windows versions greater than Windows 8.

Below is an example output on Windows 10.

Before:
Windows version 6.2 (Windows 8 or greater) 64bit

After:
Windows version 10.0 (Windows 10 or greater) 64bit

Signed-off-by: Lev Stipakov 
---
 src/compat/compat-versionhelpers.h |  6 +
 src/openvpn/openvpn.manifest   | 33 ++
 src/openvpn/openvpn.vcxproj| 15 
 src/openvpn/openvpn.vcxproj.filters|  5 
 src/openvpn/openvpn_win32_resources.rc |  2 ++
 src/openvpn/win32.c| 20 ++--
 src/openvpn/win32.h|  8 ---
 7 files changed, 84 insertions(+), 5 deletions(-)
 create mode 100644 src/openvpn/openvpn.manifest

diff --git a/src/compat/compat-versionhelpers.h 
b/src/compat/compat-versionhelpers.h
index 251fb047..c8511d0f 100644
--- a/src/compat/compat-versionhelpers.h
+++ b/src/compat/compat-versionhelpers.h
@@ -95,6 +95,12 @@ IsWindows8Point1OrGreater(void)
 return IsWindowsVersionOrGreater(HIBYTE(_WIN32_WINNT_WINBLUE), 
LOBYTE(_WIN32_WINNT_WINBLUE), 0);
 }
 
+VERSIONHELPERAPI
+IsWindows10OrGreater()
+{
+return IsWindowsVersionOrGreater(HIBYTE(_WIN32_WINNT_WINTHRESHOLD), 
LOBYTE(_WIN32_WINNT_WINTHRESHOLD), 0);
+}
+
 VERSIONHELPERAPI
 IsWindowsServer(void)
 {
diff --git a/src/openvpn/openvpn.manifest b/src/openvpn/openvpn.manifest
new file mode 100644
index ..fa5b3d7f
--- /dev/null
+++ b/src/openvpn/openvpn.manifest
@@ -0,0 +1,33 @@
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
diff --git a/src/openvpn/openvpn.vcxproj b/src/openvpn/openvpn.vcxproj
index bd1a5844..5367979d 100644
--- a/src/openvpn/openvpn.vcxproj
+++ b/src/openvpn/openvpn.vcxproj
@@ -70,6 +70,18 @@
   
 <_ProjectFileVersion>10.0.30319.1
   
+  
+false
+  
+  
+false
+  
+  
+false
+  
+  
+false
+  
   
 
   
..\compat;$(TAP_WINDOWS_HOME)/include;$(OPENSSL_HOME)/include;$(LZO_HOME)/include;$(PKCS11H_HOME)/include;%(AdditionalIncludeDirectories)
@@ -311,6 +323,9 @@
   false
 
   
+  
+
+  
   
   
   
diff --git a/src/openvpn/openvpn.vcxproj.filters 
b/src/openvpn/openvpn.vcxproj.filters
index e0bc7d5e..cf5748c7 100644
--- a/src/openvpn/openvpn.vcxproj.filters
+++ b/src/openvpn/openvpn.vcxproj.filters
@@ -515,4 +515,9 @@
   Resource Files
 
   
+  
+
+  Resource Files
+
+  
 
\ No newline at end of file
diff --git a/src/openvpn/openvpn_win32_resources.rc 
b/src/openvpn/openvpn_win32_resources.rc
index e4f1ee95..1ea5f878 100644
--- a/src/openvpn/openvpn_win32_resources.rc
+++ b/src/openvpn/openvpn_win32_resources.rc
@@ -7,6 +7,8 @@
 
 #pragma code_page(65001) /* UTF8 */
 
+1 RT_MANIFEST "openvpn.manifest"
+
 LANGUAGE LANG_NEUTRAL, SUBLANG_NEUTRAL
 
 VS_VERSION_INFO VERSIONINFO
diff --git a/src/openvpn/win32.c b/src/openvpn/win32.c
index eb4c0307..7e913165 100644
--- a/src/openvpn/win32.c
+++ b/src/openvpn/win32.c
@@ -1417,10 +1417,18 @@ win32_version_info(void)
 {
 return WIN_7;
 }
-else
+
+if (!IsWindows8Point1OrGreater())
 {
 return WIN_8;
 }
+
+if (!IsWindows10OrGreater())
+{
+return WIN_8_1;
+}
+
+return WIN_10;
 }
 
 bool
@@ -1458,7 +1466,15 @@ win32_version_string(struct gc_arena *gc, bool add_name)
 break;
 
 case WIN_8:
-buf_printf(, "6.2%s", add_name ? " (Windows 8 or greater)" : 
"");
+buf_printf(, "6.2%s", add_name ? " (Windows 8)" : "");
+break;
+
+case WIN_8_1:
+buf_printf(, "6.3%s", add_name ? " (Windows 8.1)" : "");
+break;
+
+case WIN_10:
+buf_printf(, "10.0%s", add_name ? " (Windows 10 or greater)" : 
"");
 break;
 
 default:
diff --git a/src/openvpn/win32.h b/src/openvpn/win32.h
index 79504776..da85ed4d 100644
--- a/src/openvpn/win32.h
+++ b/src/openvpn/win32.h
@@ -298,10 +298,12 @@ bool win_wfp_block_dns(const NET_IFINDEX index, const 
HANDLE msg_channel);
 
 bool win_wfp_uninit(const NET_IFINDEX index, const HANDLE msg_channel);
 
-#define WIN_XP 0
+#define WIN_XP0
 #define WIN_VISTA 1
-#define WIN_7 2
-#define WIN_8 3
+#define WIN_7 2
+#define WIN_8 3
+#define WIN_8_1   4
+#define WIN_105
 
 int win32_version_info(void);
 
-- 
2.17.1



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


[Openvpn-devel] [PATCH 10/10] Add a note that ncp-ciphers is replaced by data-ciphers

2020-07-24 Thread Arne Schwabe
This patch adds a message that informs the user that the ncp-cipher
is renamed to data-ciphers. This should address the following concerns:

 - Users being confused by old options.
 - Nudge users to use the modern variant of an option

The man page already documents ncp-ciphers as an old name for
data-ciphers, so looking it up in the man page will also work.

Note that I did not add "deprecated old option" to this message
since I still think that eventually removing the option will only
break configs and we gain almost nothing from that.

Also still accepting the option even though we do not recommend usage of
it also follows the robustness principle of:
"be strict in what you send and tolerant in what you receive"

Signed-off-by: Arne Schwabe 
---
 src/openvpn/options.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 5beaba0f..01f0ca0f 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -7939,6 +7939,11 @@ add_option(struct options *options,
 && p[1] && !p[2])
 {
 VERIFY_PERMISSION(OPT_P_GENERAL|OPT_P_INSTANCE);
+if (streq(p[0], "ncp-ciphers"))
+{
+msg(M_INFO, "Note: Rewriting option '--ncp-ciphers' to "
+" '--data-ciphers'");
+}
 options->ncp_ciphers = p[1];
 }
 else if (streq(p[0], "ncp-disable") && !p[1])
-- 
2.26.2



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


[Openvpn-devel] [PATCH] Remove unused no-op function

2020-07-24 Thread Lev Stipakov
From: Lev Stipakov 

Body of check_subnet_conflict() was commented out
(#if 0) back in 2011, so it is safe now to completely
elimitate this function, including all calls to it.

As a bonus, remove unused local variable in do_set_mtu_service().

Signed-off-by: Lev Stipakov 
---
 src/openvpn/route.c |  1 -
 src/openvpn/tun.c   | 48 -
 src/openvpn/tun.h   |  4 
 3 files changed, 53 deletions(-)

diff --git a/src/openvpn/route.c b/src/openvpn/route.c
index b57da5dd..966f6297 100644
--- a/src/openvpn/route.c
+++ b/src/openvpn/route.c
@@ -1215,7 +1215,6 @@ add_routes(struct route_list *rl, struct route_ipv6_list 
*rl6,
 
 for (r = rl->routes; r; r = r->next)
 {
-check_subnet_conflict(r->network, r->netmask, "route");
 if (flags & ROUTE_DELETE_FIRST)
 {
 delete_route(r, tt, flags, >rgi, es, ctx);
diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c
index 82d96927..8a132b4d 100644
--- a/src/openvpn/tun.c
+++ b/src/openvpn/tun.c
@@ -221,7 +221,6 @@ out:
 static bool
 do_set_mtu_service(const struct tuntap *tt, const short family, const int mtu)
 {
-DWORD len;
 bool ret = false;
 ack_message_t ack;
 struct gc_arena gc = gc_new();
@@ -466,44 +465,6 @@ check_addr_clash(const char *name,
 gc_free();
 }
 
-/*
- * Issue a warning if ip/netmask (on the virtual IP network) conflicts with
- * the settings on the local LAN.  This is designed to flag issues where
- * (for example) the OpenVPN server LAN is running on 192.168.1.x, but then
- * an OpenVPN client tries to connect from a public location that is also 
running
- * off of a router set to 192.168.1.x.
- */
-void
-check_subnet_conflict(const in_addr_t ip,
-  const in_addr_t netmask,
-  const char *prefix)
-{
-#if 0 /* too many false positives */
-struct gc_arena gc = gc_new();
-in_addr_t lan_gw = 0;
-in_addr_t lan_netmask = 0;
-
-if (get_default_gateway(_gw, _netmask) && lan_netmask)
-{
-const in_addr_t lan_network = lan_gw & lan_netmask;
-const in_addr_t network = ip & netmask;
-
-/* do the two subnets defined by network/netmask and 
lan_network/lan_netmask intersect? */
-if ((network & lan_netmask) == lan_network
-|| (lan_network & netmask) == network)
-{
-msg(M_WARN, "WARNING: potential %s subnet conflict between local 
LAN [%s/%s] and remote VPN [%s/%s]",
-prefix,
-print_in_addr_t(lan_network, 0, ),
-print_in_addr_t(lan_netmask, 0, ),
-print_in_addr_t(network, 0, ),
-print_in_addr_t(netmask, 0, ));
-}
-}
-gc_free();
-#endif /* if 0 */
-}
-
 void
 warn_on_use_of_common_subnets(openvpn_net_ctx_t *ctx)
 {
@@ -763,15 +724,6 @@ init_tun(const char *dev,/* --dev option */
  tt->remote_netmask);
 }
 }
-
-if (tt->type == DEV_TYPE_TAP || (tt->type == DEV_TYPE_TUN && 
tt->topology == TOP_SUBNET))
-{
-check_subnet_conflict(tt->local, tt->remote_netmask, "TUN/TAP 
adapter");
-}
-else if (tt->type == DEV_TYPE_TUN)
-{
-check_subnet_conflict(tt->local, IPV4_NETMASK_HOST, "TUN/TAP 
adapter");
-}
 }
 
 #ifdef _WIN32
diff --git a/src/openvpn/tun.h b/src/openvpn/tun.h
index 99826cf7..e73be206 100644
--- a/src/openvpn/tun.h
+++ b/src/openvpn/tun.h
@@ -309,10 +309,6 @@ const char *ifconfig_options_string(const struct tuntap 
*tt, bool remote, bool d
 
 bool is_tun_p2p(const struct tuntap *tt);
 
-void check_subnet_conflict(const in_addr_t ip,
-   const in_addr_t netmask,
-   const char *prefix);
-
 void warn_on_use_of_common_subnets(openvpn_net_ctx_t *ctx);
 
 /*
-- 
2.17.1



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH 8/9] Rename ncp-ciphers to data-ciphers

2020-07-24 Thread David Sommerseth
On 24/07/2020 12:45, Arne Schwabe wrote:
> First of all I did not want to reply to this since we had a lengthy
> discussion on IRC.
> 
>> Lets take a few steps back try to see a broader picture.
>>
>> * --ncp-ciphers was introduced in OpenVPN 2.4 as a brand new option.
>>
>> * Steffan has suggested to add --data-ciphers alias into the next v2.4
>>   release; to which I agree(!).
> 
> Yes, this sounds like a simple idea but also has the danger of creating
> more confusion.
> 
> I feel it better to have ncp-cipher and data-ciphers coexist. Then you
> also have the understand that ncp-cipher is an older/less capable
> version of data-ciphers.
> 
> OpenVPN 2.4 has a number of oddities in NCP support:
> 
> - the client always announces AES-GCM support even if disabled in
> ncp-ciphers
> - the server will always push the first cipher of its ncp-ciphers list
> to the client.

These oddities, they sounds like bugs, or?  If I try to put on my
Never-used-NCP-before-hat, this would not be what I would expect.  Why haven't
we resolved this within the current 2.4 scope without changing options?



> From a user perspective data-ciphers is a true superset of ncp-ciphers.
> All existing ncp-ciphers setup will continue to work on 2.5 but the
> data-ciphers option from 2.5 is likely to break on 2.5 if you don't take
> into account the 2.4 oddities.
> 
>> * I propose we add a warning whenever --ncp-ciphers used, recommending the
>>   user to switch to --data-ciphers as soon as possible.
> 
> As compromise maybe something like
> 
> NOTE: Rewriting old-option x y z to new new-option x y z

As mentioned on IRC, yes.  I am willing to accept

msg(M_INFO, "Rewriting deprecated option X to the replacement Y" ...



[...]
>> * 12 months *after* the v2.6 release is out, we can remove --ncp-ciphers.  
>> But
>>   since we will not do option changes mid-release easily, it might be natural
>>   to remove this in OpenVPN 2.7 at earliest.
>>
>> This means --ncp-ciphers will be supported in 2.4 for the life time of that
>> release.  And v2.4 is supported for *at least* 18 months after v2.5 is
>> released.  It also guarantees --ncp-ciphers will first be removed when we
>> release OpenVPN 2.7.
>>
>> What would be a problem with such a schedule?  Because I don't understand the
>> real objection you have to remove options which ends up duplicating other 
>> options.
> 
> 
> My general problem is that I don't see a real advantage in removing
> ncp-cipher but see a lot of downsides removing it.

I hear about you seeing downsides ... but I have not seen any argument here
convincing me otherwise.  This overall process should take care of supporting
all reasonable OpenVPN 2.x versions.



>> At the same time ... it is also important for me that we try to see this at
>> from an even bigger perspective than just --ncp-ciphers/--data-ciphers or 
>> even
>> --udp-mtu alone.  Now I'm shifting the discussion to a more generic product
>> life cycle perspective.
>>
>> If we skimp through this and just allow whatever option duplications we head
>> into, just because we're too concerned about various breakages with old
>> configurations or setups - it will not be the last time we might end up in
>> such discussions UNLESS we can find a proper and reasonable process for
>> deprecation (which we in fact already defined for feature deprecation 10 
>> years
>> ago! [1]).
>>
>> If we cannot remove options during the whole life time of OpenVPN, we cannot
>> touch compression options or possibly even deprecate any compression at all -
>> because we need to support both compression and decompression for the whole
>> lifetime of OpenVPN.  This also extends into how to ensure proper OpenVPN 3
>> compatibility as well.
> 
> The amount of time we spend on testing/developing etc on compression and
> the complexity it introduces is vastly different from ncp-cipher

That is true.  But at the same time, if we do not have a proper deprecation
process where we draw the line in which OpenVPN versions we are willing to
support (from a functional perspective), we cannot touch compression.

Which is why I'm trying to raise the points of a bigger picture.  We need to
find a reasonable solution to which OpenVPN versions we are willing to
support, and when those versions enters the unsupported side we should be free
to cleanup code and options related to the behavior and functionality only
these now not supported releases provided.



>> And if we cannot remove any options during the eternal life time of OpenVPN, 
>> I
>> will see no other alternative to being even more critical and rejective to 
>> add
>> new options.  We already have pretty close to 300 options in OpenVPN.  That 
>> is
>> an insane amount - where a typical common OpenVPN setup might need up to 20 
>> of
>> these options, the rest are for all kinds of special cases.  And we have
>> several competing solutions which are far simpler in many aspects which new
>> users might just as well prefer.
>>
>> Even though I 

[Openvpn-devel] [PATCH] wintun: remove SYSTEM elevation hack

2020-07-24 Thread Lev Stipakov
From: Lev Stipakov 

As discussed a while ago on the mailing list and
community meetings, having SYSTEM elevation hack
inside openvpn code considered harmful.

Since interactive service is the recommended way
of using openvpn on Windows, limiting wintun usage to
interactive service should not be an issue.

Remove elevation hack code and provide an error message
telling user to use interactive service or do SYSTEM
elevation himself via psexec.

Move implementation of register_ring_buffers() to header
amd delete ring_buffer.c.

Signed-off-by: Lev Stipakov 
---
 src/openvpn/Makefile.am |  2 +-
 src/openvpn/openvpn.vcxproj |  1 -
 src/openvpn/openvpn.vcxproj.filters |  3 -
 src/openvpn/ring_buffer.c   | 56 
 src/openvpn/ring_buffer.h   | 31 +--
 src/openvpn/tun.c   | 33 +--
 src/openvpn/win32.c | 95 -
 src/openvpnserv/Makefile.am |  2 +-
 src/openvpnserv/openvpnserv.vcxproj |  1 -
 src/openvpnserv/openvpnserv.vcxproj.filters |  3 -
 10 files changed, 32 insertions(+), 195 deletions(-)
 delete mode 100644 src/openvpn/ring_buffer.c

diff --git a/src/openvpn/Makefile.am b/src/openvpn/Makefile.am
index f0e0ad23..47ad762d 100644
--- a/src/openvpn/Makefile.am
+++ b/src/openvpn/Makefile.am
@@ -141,6 +141,6 @@ openvpn_LDADD = \
$(OPTIONAL_DL_LIBS) \
$(OPTIONAL_INOTIFY_LIBS)
 if WIN32
-openvpn_SOURCES += openvpn_win32_resources.rc block_dns.c block_dns.h 
ring_buffer.c ring_buffer.h
+openvpn_SOURCES += openvpn_win32_resources.rc block_dns.c block_dns.h 
ring_buffer.h
 openvpn_LDADD += -lgdi32 -lws2_32 -lwininet -lcrypt32 -liphlpapi -lwinmm 
-lfwpuclnt -lrpcrt4 -lncrypt -lsetupapi
 endif
diff --git a/src/openvpn/openvpn.vcxproj b/src/openvpn/openvpn.vcxproj
index c34733ea..bd1a5844 100644
--- a/src/openvpn/openvpn.vcxproj
+++ b/src/openvpn/openvpn.vcxproj
@@ -189,7 +189,6 @@
 
 
 
-
 
 
 
diff --git a/src/openvpn/openvpn.vcxproj.filters 
b/src/openvpn/openvpn.vcxproj.filters
index 80eb52b3..e0bc7d5e 100644
--- a/src/openvpn/openvpn.vcxproj.filters
+++ b/src/openvpn/openvpn.vcxproj.filters
@@ -240,9 +240,6 @@
 
   Source Files
 
-
-  Source Files
-
 
   Source Files
 
diff --git a/src/openvpn/ring_buffer.c b/src/openvpn/ring_buffer.c
deleted file mode 100644
index 8c81dc46..
--- a/src/openvpn/ring_buffer.c
+++ /dev/null
@@ -1,56 +0,0 @@
-/*
- *  OpenVPN -- An application to securely tunnel IP networks
- * over a single UDP port, with support for SSL/TLS-based
- * session authentication and key exchange,
- * packet encryption, packet authentication, and
- * packet compression.
- *
- *  Copyright (C) 2002-2019 OpenVPN Inc 
- *2019 Lev Stipakov 
- *
- *  This program is free software; you can redistribute it and/or modify
- *  it under the terms of the GNU General Public License version 2
- *  as published by the Free Software Foundation.
- *
- *  This program is distributed in the hope that it will be useful,
- *  but WITHOUT ANY WARRANTY; without even the implied warranty of
- *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- *  GNU General Public License for more details.
- *
- *  You should have received a copy of the GNU General Public License along
- *  with this program; if not, write to the Free Software Foundation, Inc.,
- *  51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
- */
-
-#include "ring_buffer.h"
-
-#ifdef _WIN32
-
-bool
-register_ring_buffers(HANDLE device,
-  struct tun_ring *send_ring,
-  struct tun_ring *receive_ring,
-  HANDLE send_tail_moved,
-  HANDLE receive_tail_moved)
-{
-struct tun_register_rings rr;
-BOOL res;
-DWORD bytes_returned;
-
-ZeroMemory(, sizeof(rr));
-
-rr.send.ring = send_ring;
-rr.send.ring_size = sizeof(struct tun_ring);
-rr.send.tail_moved = send_tail_moved;
-
-rr.receive.ring = receive_ring;
-rr.receive.ring_size = sizeof(struct tun_ring);
-rr.receive.tail_moved = receive_tail_moved;
-
-res = DeviceIoControl(device, TUN_IOCTL_REGISTER_RINGS, , sizeof(rr),
-  NULL, 0, _returned, NULL);
-
-return res != FALSE;
-}
-
-#endif /* ifdef _WIN32 */
\ No newline at end of file
diff --git a/src/openvpn/ring_buffer.h b/src/openvpn/ring_buffer.h
index af46f106..4293f633 100644
--- a/src/openvpn/ring_buffer.h
+++ b/src/openvpn/ring_buffer.h
@@ -94,11 +94,32 @@ struct TUN_PACKET
  *that data has been written to receive ring
  * @returntrue if registration is successful, false 
otherwise - use GetLastError()
  */
-bool register_ring_buffers(HANDLE device,
-   struct tun_ring *send_ring,
- 

Re: [Openvpn-devel] [PATCH 8/9] Rename ncp-ciphers to data-ciphers

2020-07-24 Thread Arne Schwabe
First of all I did not want to reply to this since we had a lengthy
discussion on IRC.

> Lets take a few steps back try to see a broader picture.
> 
> * --ncp-ciphers was introduced in OpenVPN 2.4 as a brand new option.
> 
> * Steffan has suggested to add --data-ciphers alias into the next v2.4
>   release; to which I agree(!).

Yes, this sounds like a simple idea but also has the danger of creating
more confusion.

I feel it better to have ncp-cipher and data-ciphers coexist. Then you
also have the understand that ncp-cipher is an older/less capable
version of data-ciphers.

OpenVPN 2.4 has a number of oddities in NCP support:

- the client always announces AES-GCM support even if disabled in
ncp-ciphers
- the server will always push the first cipher of its ncp-ciphers list
to the client.


From a user perspective data-ciphers is a true superset of ncp-ciphers.
All existing ncp-ciphers setup will continue to work on 2.5 but the
data-ciphers option from 2.5 is likely to break on 2.5 if you don't take
into account the 2.4 oddities.

> * I propose we add a warning whenever --ncp-ciphers used, recommending the
>   user to switch to --data-ciphers as soon as possible.

As compromise maybe something like

NOTE: Rewriting old-option x y z to new new-option x y z


> 
> * We keep both --ncp-ciphers *and* --data-ciphers in v2.5 and v2.6.
> 
> * When v2.5 is released v2.4 goes into "old stable support" - where both
>   alternatives will work.  v2.4 is guaranteed to be supported by the community
>   for at least 6 months with full support [0] after v2.5 is released.
> 
> * When 2.6 is released, v2.4 goes into "git tree only" support but will have a
> 
>   12 month "old stable support" guarantee [0].
> 
> * 12 months *after* the v2.6 release is out, we can remove --ncp-ciphers.  But
>   since we will not do option changes mid-release easily, it might be natural
>   to remove this in OpenVPN 2.7 at earliest.
> 
> This means --ncp-ciphers will be supported in 2.4 for the life time of that
> release.  And v2.4 is supported for *at least* 18 months after v2.5 is
> released.  It also guarantees --ncp-ciphers will first be removed when we
> release OpenVPN 2.7.
> 
> What would be a problem with such a schedule?  Because I don't understand the
> real objection you have to remove options which ends up duplicating other 
> options.


My general problem is that I don't see a real advantage in removing
ncp-cipher but see a lot of downsides removing it.

> 
> At the same time ... it is also important for me that we try to see this at
> from an even bigger perspective than just --ncp-ciphers/--data-ciphers or even
> --udp-mtu alone.  Now I'm shifting the discussion to a more generic product
> life cycle perspective.
> 
> If we skimp through this and just allow whatever option duplications we head
> into, just because we're too concerned about various breakages with old
> configurations or setups - it will not be the last time we might end up in
> such discussions UNLESS we can find a proper and reasonable process for
> deprecation (which we in fact already defined for feature deprecation 10 years
> ago! [1]).
> 
> If we cannot remove options during the whole life time of OpenVPN, we cannot
> touch compression options or possibly even deprecate any compression at all -
> because we need to support both compression and decompression for the whole
> lifetime of OpenVPN.  This also extends into how to ensure proper OpenVPN 3
> compatibility as well.

The amount of time we spend on testing/developing etc on compression and
the complexity it introduces is vastly different from ncp-cipher

> And if we cannot remove any options during the eternal life time of OpenVPN, I
> will see no other alternative to being even more critical and rejective to add
> new options.  We already have pretty close to 300 options in OpenVPN.  That is
> an insane amount - where a typical common OpenVPN setup might need up to 20 of
> these options, the rest are for all kinds of special cases.  And we have
> several competing solutions which are far simpler in many aspects which new
> users might just as well prefer.
> 
> Even though I highlight the number of options we have, I do NOT say that we
> should swipe them all out and reduce the amount to 50 or so; for some users
> they have a _real_ value and provides really useful features.  But I want us
> to save the options which do have a REAL value, not because unsupported
> OpenVPN versions might break or "10 bytes extra source code" is too heavy to
> carry around for an alias option.  I'm arguing for keeping options covering
> _REAL_ USE CASE for users.  And no, breaking unsupported OpenVPN releases is
> NOT a real use case from my point of view.

This is the point where we disagree. I still think that we should weigh
the pros and cons if we still want have compatibility. And also to
consider if there are alternatives/workaround for the setups/users
affected.

> But back to the deprecated options  ... If we 

Re: [Openvpn-devel] Regarding deprecation of --route-nopull

2020-07-24 Thread Arne Schwabe

>> Also route-pull works in both OpenVPN 2.x and 3.x
>> clients while pull-filter is currently 2.x only.
> 
> Actually pull-filter cannot be compared with route-nopull as the
> former is customizable. The real question is whether there is any
> compelling reason to use it other than lack of alternatives in 2.3 and
> older. I didn't know 3.x does not support pull-filter. Why? It's easy
> to code (at least I know that for sure) so that can't be the reason.

The compelling reason for route-nopull is that it will catch all route
related options even if OpenVPN adds a new one in a future version. It
is a simple way of saying I don't want my routes to be modified.


To emulate pull-filter with pull-filter you need to block this list:

redirect-private
redirect-gateway
block-ipv6
client-nat
route
route-ipv6
route-gateway
route-metric
ip-win32
dhcp-option
dhcp-renew
register-dns
tap-sleep
block-outside-dns

but not block

route-gateway
route-dealy

And without looking at the source code you will be hard pressed to come
up with the same list.

Arne



signature.asc
Description: OpenPGP digital signature
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH 8/9] Rename ncp-ciphers to data-ciphers

2020-07-24 Thread David Sommerseth
On 24/07/2020 10:14, Steffan Karger wrote:
> Hi,
> 
> On 17-07-2020 15:47, Arne Schwabe wrote:
>> The change in name signals that data-ciphers is the preferred way to
>> configure data channel (and not --cipher). The data prefix is chosen
>> to avoid ambiguity and make it distinct from tls-cipher for the TLS
>> ciphers.
>>
>> Signed-off-by: Arne Schwabe 
>> ---
>>  Changes.rst| 13 ++---
>>  doc/man-sections/protocol-options.rst  | 11 +++
>>  doc/man-sections/server-options.rst|  4 ++--
>>  sample/sample-config-files/client.conf |  2 +-
>>  src/openvpn/multi.c|  4 ++--
>>  src/openvpn/options.c  |  5 +++--
>>  src/openvpn/ssl_ncp.c  |  4 ++--
>>  7 files changed, 27 insertions(+), 16 deletions(-)
>>
>> diff --git a/Changes.rst b/Changes.rst
>> index 6e283270..2158c8e7 100644
>> --- a/Changes.rst
>> +++ b/Changes.rst
>> @@ -14,12 +14,19 @@ ChaCha20-Poly1305 cipher support
>>  channel.
>>  
>>  Improved Data channel cipher negotiation
>> +The option ``ncp-ciphers`` has been renamed to ``data-ciphers``.
>> +The old name is still accepted. The change in name signals that
>> +``data-ciphers`` is the preferred way to configure data channel
>> +ciphers and the data prefix is chosen to avoid the ambiguity that
>> +exists with ``--cipher`` for the data cipher and ``tls-cipher``
>> +for the TLS ciphers.
>> +
>>  OpenVPN clients will now signal all supported ciphers from the
>> -``ncp-ciphers`` option to the server via ``IV_CIPHERS``. OpenVPN
>> -servers will select the first common cipher from the ``ncp-ciphers``
>> +``data-ciphers`` option to the server via ``IV_CIPHERS``. OpenVPN
>> +servers will select the first common cipher from the ``data-ciphers``
>>  list instead of blindly pushing the first cipher of the list. This
>>  allows to use a configuration like
>> -``ncp-ciphers ChaCha20-Poly1305:AES-256-GCM`` on the server that
>> +``data-ciphers ChaCha20-Poly1305:AES-256-GCM`` on the server that
>>  prefers ChaCha20-Poly1305 but uses it only if the client supports it.
>>  
>>  Asynchronous (deferred) authentication support for auth-pam plugin.
>> diff --git a/doc/man-sections/protocol-options.rst 
>> b/doc/man-sections/protocol-options.rst
>> index 923d2da0..051f1d32 100644
>> --- a/doc/man-sections/protocol-options.rst
>> +++ b/doc/man-sections/protocol-options.rst
>> @@ -62,7 +62,7 @@ configured in a compatible way between both the local and 
>> remote side.
>>The default is :code:`BF-CBC`, an abbreviation for Blowfish in Cipher
>>Block Chaining mode. When cipher negotiation (NCP) is allowed,
>>OpenVPN 2.4 and newer on both client and server side will automatically
>> -  upgrade to :code:`AES-256-GCM`.  See ``--ncp-ciphers`` and
>> +  upgrade to :code:`AES-256-GCM`.  See ``--data-ciphers`` and
>>``--ncp-disable`` for more details on NCP.
>>  
>>Using :code:`BF-CBC` is no longer recommended, because of its 64-bit
>> @@ -169,7 +169,7 @@ configured in a compatible way between both the local 
>> and remote side.
>>non-standard key lengths, and a larger key may offer no real guarantee
>>of greater security, or may even reduce security.
>>  
>> ---ncp-ciphers cipher-list
>> +--data-ciphers cipher-list
>>Restrict the allowed ciphers to be negotiated to the ciphers in
>>``cipher-list``. ``cipher-list`` is a colon-separated list of ciphers,
>>and defaults to :code:`AES-256-GCM:AES-128-GCM`.
>> @@ -189,9 +189,9 @@ configured in a compatible way between both the local 
>> and remote side.
>>Additionally, to allow for more smooth transition, if NCP is enabled,
>>OpenVPN will inherit the cipher of the peer if that cipher is different
>>from the local ``--cipher`` setting, but the peer cipher is one of the
>> -  ciphers specified in ``--ncp-ciphers``. E.g. a non-NCP client (<=v2.3,
>> +  ciphers specified in ``--data-ciphers``. E.g. a non-NCP client (<=v2.3,
>>or with --ncp-disabled set) connecting to a NCP server (v2.4+) with
>> -  ``--cipher BF-CBC`` and ``--ncp-ciphers AES-256-GCM:AES-256-CBC`` set can
>> +  ``--cipher BF-CBC`` and ``--data-ciphers AES-256-GCM:AES-256-CBC`` set can
>>either specify ``--cipher BF-CBC`` or ``--cipher AES-256-CBC`` and both
>>will work.
>>  
>> @@ -201,6 +201,9 @@ configured in a compatible way between both the local 
>> and remote side.
>>This list is restricted to be 127 chars long after conversion to OpenVPN
>>ciphers.
>>  
>> +  This option was called ``ncp-ciphers`` in OpenVPN 2.4 but has been renamed
>> +  to ``data-ciphers`` in OpenVPN 2.5 to more accurately reflect its meaning.
>> +
>>  --ncp-disable
>>Disable "Negotiable Crypto Parameters". This completely disables cipher
>>negotiation.
>> diff --git a/doc/man-sections/server-options.rst 
>> b/doc/man-sections/server-options.rst
>> index c24aec0b..74ad5e18 100644
>> --- 

Re: [Openvpn-devel] Regarding deprecation of --route-nopull

2020-07-24 Thread David Sommerseth
On 24/07/2020 02:35, Selva Nair wrote:
>> Also route-pull works in both OpenVPN 2.x and 3.x
>> clients while pull-filter is currently 2.x only.
> 
> Actually pull-filter cannot be compared with route-nopull as the
> former is customizable. The real question is whether there is any
> compelling reason to use it other than lack of alternatives in 2.3 and
> older. I didn't know 3.x does not support pull-filter. Why? It's easy
> to code (at least I know that for sure) so that can't be the reason.

This is on our todo-list.  We want pull-filter in OpenVPN 3.  The filter
itself is simple to implement, just hasn't surfaced on the more critical
issues we've needed to tackle.

-- 
kind regards,

David Sommerseth
OpenVPN Inc




signature.asc
Description: OpenPGP digital signature
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH 8/9] Rename ncp-ciphers to data-ciphers

2020-07-24 Thread Steffan Karger
Hi,

On 17-07-2020 15:47, Arne Schwabe wrote:
> The change in name signals that data-ciphers is the preferred way to
> configure data channel (and not --cipher). The data prefix is chosen
> to avoid ambiguity and make it distinct from tls-cipher for the TLS
> ciphers.
> 
> Signed-off-by: Arne Schwabe 
> ---
>  Changes.rst| 13 ++---
>  doc/man-sections/protocol-options.rst  | 11 +++
>  doc/man-sections/server-options.rst|  4 ++--
>  sample/sample-config-files/client.conf |  2 +-
>  src/openvpn/multi.c|  4 ++--
>  src/openvpn/options.c  |  5 +++--
>  src/openvpn/ssl_ncp.c  |  4 ++--
>  7 files changed, 27 insertions(+), 16 deletions(-)
> 
> diff --git a/Changes.rst b/Changes.rst
> index 6e283270..2158c8e7 100644
> --- a/Changes.rst
> +++ b/Changes.rst
> @@ -14,12 +14,19 @@ ChaCha20-Poly1305 cipher support
>  channel.
>  
>  Improved Data channel cipher negotiation
> +The option ``ncp-ciphers`` has been renamed to ``data-ciphers``.
> +The old name is still accepted. The change in name signals that
> +``data-ciphers`` is the preferred way to configure data channel
> +ciphers and the data prefix is chosen to avoid the ambiguity that
> +exists with ``--cipher`` for the data cipher and ``tls-cipher``
> +for the TLS ciphers.
> +
>  OpenVPN clients will now signal all supported ciphers from the
> -``ncp-ciphers`` option to the server via ``IV_CIPHERS``. OpenVPN
> -servers will select the first common cipher from the ``ncp-ciphers``
> +``data-ciphers`` option to the server via ``IV_CIPHERS``. OpenVPN
> +servers will select the first common cipher from the ``data-ciphers``
>  list instead of blindly pushing the first cipher of the list. This
>  allows to use a configuration like
> -``ncp-ciphers ChaCha20-Poly1305:AES-256-GCM`` on the server that
> +``data-ciphers ChaCha20-Poly1305:AES-256-GCM`` on the server that
>  prefers ChaCha20-Poly1305 but uses it only if the client supports it.
>  
>  Asynchronous (deferred) authentication support for auth-pam plugin.
> diff --git a/doc/man-sections/protocol-options.rst 
> b/doc/man-sections/protocol-options.rst
> index 923d2da0..051f1d32 100644
> --- a/doc/man-sections/protocol-options.rst
> +++ b/doc/man-sections/protocol-options.rst
> @@ -62,7 +62,7 @@ configured in a compatible way between both the local and 
> remote side.
>The default is :code:`BF-CBC`, an abbreviation for Blowfish in Cipher
>Block Chaining mode. When cipher negotiation (NCP) is allowed,
>OpenVPN 2.4 and newer on both client and server side will automatically
> -  upgrade to :code:`AES-256-GCM`.  See ``--ncp-ciphers`` and
> +  upgrade to :code:`AES-256-GCM`.  See ``--data-ciphers`` and
>``--ncp-disable`` for more details on NCP.
>  
>Using :code:`BF-CBC` is no longer recommended, because of its 64-bit
> @@ -169,7 +169,7 @@ configured in a compatible way between both the local and 
> remote side.
>non-standard key lengths, and a larger key may offer no real guarantee
>of greater security, or may even reduce security.
>  
> ---ncp-ciphers cipher-list
> +--data-ciphers cipher-list
>Restrict the allowed ciphers to be negotiated to the ciphers in
>``cipher-list``. ``cipher-list`` is a colon-separated list of ciphers,
>and defaults to :code:`AES-256-GCM:AES-128-GCM`.
> @@ -189,9 +189,9 @@ configured in a compatible way between both the local and 
> remote side.
>Additionally, to allow for more smooth transition, if NCP is enabled,
>OpenVPN will inherit the cipher of the peer if that cipher is different
>from the local ``--cipher`` setting, but the peer cipher is one of the
> -  ciphers specified in ``--ncp-ciphers``. E.g. a non-NCP client (<=v2.3,
> +  ciphers specified in ``--data-ciphers``. E.g. a non-NCP client (<=v2.3,
>or with --ncp-disabled set) connecting to a NCP server (v2.4+) with
> -  ``--cipher BF-CBC`` and ``--ncp-ciphers AES-256-GCM:AES-256-CBC`` set can
> +  ``--cipher BF-CBC`` and ``--data-ciphers AES-256-GCM:AES-256-CBC`` set can
>either specify ``--cipher BF-CBC`` or ``--cipher AES-256-CBC`` and both
>will work.
>  
> @@ -201,6 +201,9 @@ configured in a compatible way between both the local and 
> remote side.
>This list is restricted to be 127 chars long after conversion to OpenVPN
>ciphers.
>  
> +  This option was called ``ncp-ciphers`` in OpenVPN 2.4 but has been renamed
> +  to ``data-ciphers`` in OpenVPN 2.5 to more accurately reflect its meaning.
> +
>  --ncp-disable
>Disable "Negotiable Crypto Parameters". This completely disables cipher
>negotiation.
> diff --git a/doc/man-sections/server-options.rst 
> b/doc/man-sections/server-options.rst
> index c24aec0b..74ad5e18 100644
> --- a/doc/man-sections/server-options.rst
> +++ b/doc/man-sections/server-options.rst
> @@ -473,8 +473,8 @@ fast hardware. SSL/TLS authentication must be used in 
> 

Re: [Openvpn-devel] [PATCH 8/9] Rename ncp-ciphers to data-ciphers

2020-07-24 Thread Steffan Karger
Hi,

On 23-07-2020 18:09, David Sommerseth wrote:
>> This was a deliberate decision. We really want to people to move towards
>> ncp and putting another hurdle with having an option that works better
>> on but gives a warning and a option that does not work on 2.4 does not
>> help here. If we decide that really aliases are a no-go in OpenVPN then
>> I would rather drop data-ciphers and stay with ncp-ciphers forever for
>> this reason.
> 
> Lets take a few steps back try to see a broader picture.
> 
> [..snip..]
> 
> We really need a proper and sane processes to allow the development of OpenVPN
> to have a chance to move on and leave things behind when appropriate, to be
> able to evolve and grow with the future - without being strangled by what
> existed in the far past (meaning: no longer community supported releases).
> Otherwise I do fear for the future of OpenVPN 2.x.
> 
> By having a clear strategy and adhering to a process of feature/option
> management in OpenVPN, we give clearly defined time-window for stability and
> functionality for our users.  This predictability is, in my experience, much
> more important to users than if a specifically named option is supported or 
> not.

Yes, you've made these points clear earlier on IRC. I (and with me Arne
and Gert) just don't agree with you on some of the details, resulting in
a different verdict on this patch.

None of us has trouble with deprecating options. We appreciate the work
you've put into the DeprecatedOptions page, and all of us have sent
and/or acked patched to remove dangerous or obsolete options.

This difference is in how we weigh the pros and cons per option. So I'll
leave the broader picture for now, and summarize why I'm going to ACK
Arne's patch exactly because is *doesn't* print a warning when the old
name is used.

Option name aliases add negligible code complexity and are trivial to
maintain. (Just look at --udp-mtu.) Keeping them in allows users to
write configs that work well and do not produce any warnings on both
older and newer versions. (Printing warnings for harmless things reduces
the value of the other warnings we print.)

Let's focus our time and effort on reducing actual complexity.

-Steffan


___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel