Re: [Openvpn-devel] [PATCH OpenVPN3] Add 'pull' to ignored options

[Frank Lichtenheld]

On Fri, Mar 08, 2024 at 10:47:27AM +0100, Merten Fermont wrote:
> Hi Frank,
> 
> Arne has indeed applied a fix based on this patch.
> 
> However, it looks like the case from this issue is not completely solved:
> https://github.com/OpenVPN/openvpn3/issues/277
> If you have options "client" and "pull" but no "tls-client" in the config,
> the "pull" option will not be touched.

True, due to short-circuit logic. I will prepare a fix.

Regards,
-- 
  Frank Lichtenheld


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

Re: [Openvpn-devel] [PATCH OpenVPN3] Add 'pull' to ignored options

[Frank Lichtenheld]

On Tue, Aug 01, 2023 at 08:22:24PM +0200, David Sommerseth wrote:
> On 27/07/2023 14:21, Merten Fermont wrote:
> > Hi Arne,
> > 
> > I changed my patch to check the client and client+pull options.
> > Giving an error when neither options are declared.
> > 
> > This however may break current implementations that depend on 'client'
> > not being a required option?
> > 
> > Greetings,
> > Merten
> > 
> > Subject: [PATCH] Check for client options
> > 
> 
> Hi,
> 
> Thanks for your patch.  Changes looks reasonable to me as well, and I wanted
> to pull it in.  But it turns out that the patch has been mangled somehow.
> Even good old 'patch' refuses to apply anything.  This is not an unknown
> issue with gmail.com; which is why we generally recommend to use 'git
> send-mail' [1].
> 
> In this specific case, resending the patch as an attachment can also work.

Since I was confused about the state of this patch:
It has been superseded by a patch from Arne, see commit
https://github.com/OpenVPN/openvpn3/commit/53614a0cce7775ba0ae4a43887ee03aa2fa098cc

Also marked it in Patchwork correctly.

Regards,
-- 
  Frank Lichtenheld


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

Re: [Openvpn-devel] [PATCH OpenVPN3] Add 'pull' to ignored options

[David Sommerseth]

On 27/07/2023 14:21, Merten Fermont wrote:

Hi Arne,

I changed my patch to check the client and client+pull options.
Giving an error when neither options are declared.

This however may break current implementations that depend on 'client'
not being a required option?

Greetings,
Merten

Subject: [PATCH] Check for client options



Hi,

Thanks for your patch.  Changes looks reasonable to me as well, and I 
wanted to pull it in.  But it turns out that the patch has been mangled 
somehow.  Even good old 'patch' refuses to apply anything.  This is not 
an unknown issue with gmail.com; which is why we generally recommend to 
use 'git send-mail' [1].


In this specific case, resending the patch as an attachment can also work.


[1] 


--
kind regards,

David Sommerseth
OpenVPN Inc



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

Re: [Openvpn-devel] [PATCH OpenVPN3] Add 'pull' to ignored options

[Arne Schwabe]

Am 27.07.23 um 14:21 schrieb Merten Fermont:

Hi Arne,

I changed my patch to check the client and client+pull options.
Giving an error when neither options are declared.

This however may break current implementations that depend on 'client'
not being a required option?



Acked-By: Arne Schwabe 

Yes. It might. But then again there are people that run into strange 
problems because these configuration are not really working when 
switching to an OpenVPN 2.x client. We rather have consistency between 
clients here.


Arne



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

Re: [Openvpn-devel] [PATCH OpenVPN3] Add 'pull' to ignored options

[Merten Fermont]

Hi Arne,

I changed my patch to check the client and client+pull options.
Giving an error when neither options are declared.

This however may break current implementations that depend on 'client'
not being a required option?

Greetings,
Merten

Subject: [PATCH] Check for client options

Require 'client' or 'tls-client'+'pull' to be declared in the config.
To prevent other errors, 'client' option is added when 'tls-client'
and 'pull' are both declared.

Fixes error that --pull is an unknown option.
---
openvpn/client/cliopt.hpp   |  1 -
openvpn/client/cliopthelper.hpp | 11 ++-
openvpn/common/options.hpp  |  4 ++--
3 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/openvpn/client/cliopt.hpp b/openvpn/client/cliopt.hpp
index f7be44a8..8c52a5c0 100644
--- a/openvpn/client/cliopt.hpp
+++ b/openvpn/client/cliopt.hpp
@@ -801,7 +801,6 @@ class ClientOptions : public RC
"replay-persist", /* Makes little sense in TLS mode */
"script-security",
"sndbuf",
-"tls-client", /* Always enabled */
"tmp-dir",
"tun-ipv6",   /* ignored in v2 as well */
"txqueuelen", /* so platforms evaluate that in tun, some do
not, do not warn about that */
diff --git a/openvpn/client/cliopthelper.hpp b/openvpn/client/cliopthelper.hpp
index 95aa6664..ad3b4445 100644
--- a/openvpn/client/cliopthelper.hpp
+++ b/openvpn/client/cliopthelper.hpp
@@ -367,13 +367,22 @@ class ParseClientConfig
bool added = false;

// client
-if (!options.exists("client"))
+if (options.exists("client"))
+{
+options.touch("tls-client", true);
+options.touch("pull", true);
+}
+else if (options.exists("tls-client") && options.exists("pull"))
{
Option opt;
opt.push_back("client");
options.push_back(std::move(opt));
added = true;
}
+else
+{
+throw option_error("No 'client' or
'tls-client'+'pull' directive declared. Other roles are not
supported.");
+}

// dev
if (!options.exists("dev"))
diff --git a/openvpn/common/options.hpp b/openvpn/common/options.hpp
index d594c41a..a813647e 100644
--- a/openvpn/common/options.hpp
+++ b/openvpn/common/options.hpp
@@ -1460,11 +1460,11 @@ class OptionList : public std::vector,
public RCCopyabletouch();
+o->touch(lightly);
}

// Render object as a string.
--
2.41.0

On Thu, 27 Jul 2023 at 11:14, Arne Schwabe  wrote:
>
> Am 27.07.23 um 10:52 schrieb Merten Fermont:
> > Fixes error that --pull is an unknown option in client config.
> > ---
> > openvpn/client/cliopt.hpp | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/openvpn/client/cliopt.hpp b/openvpn/client/cliopt.hpp
> > index f7be44a8..431791f3 100644
> > --- a/openvpn/client/cliopt.hpp
> > +++ b/openvpn/client/cliopt.hpp
> > @@ -797,6 +797,7 @@ class ClientOptions : public RC
> >  "mute-replay-warnings",
> >  "nobind", /* only behaviour in v3 client anyway */
> >  "prng",
> > +"pull", /* option is implied by 'client' */
> >  "rcvbuf", /* present in many configs */
> >  "replay-persist", /* Makes little sense in TLS mode */
> >  "script-security",
>
>
> While that will work, it would be better to not ignore that option but
> handle it in a similar way to --client. In general we should actually
> throw an error if neither client or tls-client+pull are present as
> OpenVPN3 cannot operate without these in p2p mode.
>
> Arne
>


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

Re: [Openvpn-devel] [PATCH OpenVPN3] Add 'pull' to ignored options

[Arne Schwabe]

Am 27.07.23 um 10:52 schrieb Merten Fermont:

Fixes error that --pull is an unknown option in client config.
---
openvpn/client/cliopt.hpp | 1 +
1 file changed, 1 insertion(+)

diff --git a/openvpn/client/cliopt.hpp b/openvpn/client/cliopt.hpp
index f7be44a8..431791f3 100644
--- a/openvpn/client/cliopt.hpp
+++ b/openvpn/client/cliopt.hpp
@@ -797,6 +797,7 @@ class ClientOptions : public RC
 "mute-replay-warnings",
 "nobind", /* only behaviour in v3 client anyway */
 "prng",
+"pull", /* option is implied by 'client' */
 "rcvbuf", /* present in many configs */
 "replay-persist", /* Makes little sense in TLS mode */
 "script-security",



While that will work, it would be better to not ignore that option but 
handle it in a similar way to --client. In general we should actually 
throw an error if neither client or tls-client+pull are present as 
OpenVPN3 cannot operate without these in p2p mode.


Arne



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