[Openvpn-devel] [PATCH v8] Http-proxy: fix bug preventing proxy credentials caching

2024-03-12 Thread gianmarco 
From: Gianmarco De Gregori 

Caching proxy credentials was not working due to the
lack of handling already defined creds in get_user_pass(),
which prevented the caching from working properly.

Fix this issue by getting the value of c->first_time,
that indicates if we're at the first iteration
of the main loop and use it as second argument of the
get_user_pass_http(). Otherwise, on SIGUSR1 or SIGHUP
upon instance context restart credentials would be erased
every time.

The nocache member has been added to the struct
http_proxy_options and also a getter method to retrieve
that option from ssl has been added, by doing this
we're able to erase previous queried user credentials
to ensure correct operation.

Fixes: Trac #1187
Signed-off-by: Gianmarco De Gregori 
Acked-by: Frank Lichtenheld 
Change-Id: Ia3e06c0832c4ca0ab868c845279fb71c01a1a78a
---

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/+/523
This mail reflects revision 8 of this Change.

Acked-by according to Gerrit (reflected above):
Frank Lichtenheld 


diff --git a/doc/man-sections/generic-options.rst 
b/doc/man-sections/generic-options.rst
index f8a0f48..1fb17f7 100644
--- a/doc/man-sections/generic-options.rst
+++ b/doc/man-sections/generic-options.rst
@@ -19,9 +19,6 @@
   When using ``--auth-nocache`` in combination with a user/password file
   and ``--chroot`` or ``--daemon``, make sure to use an absolute path.
 
-  This directive does not affect the ``--http-proxy`` username/password.
-  It is always cached.
-
 --cd dir
   Change directory to ``dir`` prior to reading any files such as
   configuration files, key files, scripts, etc. ``dir`` should be an
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index 52b3931..fdee199 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -697,6 +697,8 @@
 
 if (c->options.ce.http_proxy_options)
 {
+c->options.ce.http_proxy_options->first_time = c->first_time;
+
 /* Possible HTTP proxy user/pass input */
 c->c1.http_proxy = http_proxy_new(c->options.ce.http_proxy_options);
 if (c->c1.http_proxy)
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 94a88f9..d276a1a 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -1650,6 +1650,7 @@
 SHOW_STR(auth_file);
 SHOW_STR(auth_file_up);
 SHOW_BOOL(inline_creds);
+SHOW_BOOL(nocache);
 SHOW_STR(http_version);
 SHOW_STR(user_agent);
 for  (i = 0; i < MAX_CUSTOM_HTTP_HEADER && o->custom_headers[i].name; i++)
@@ -3151,6 +3152,11 @@
 ce->flags |= CE_DISABLED;
 }
 
+if (ce->http_proxy_options)
+{
+ce->http_proxy_options->nocache = ssl_get_auth_nocache();
+}
+
 /* our socks code is not fully IPv6 enabled yet (TCP works, UDP not)
  * so fall back to IPv4-only (trac #1221)
  */
diff --git a/src/openvpn/proxy.c b/src/openvpn/proxy.c
index eeb3989..4570fbc 100644
--- a/src/openvpn/proxy.c
+++ b/src/openvpn/proxy.c
@@ -276,7 +276,7 @@
 {
 auth_file = p->options.auth_file_up;
 }
-if (p->queried_creds)
+if (p->queried_creds && !static_proxy_user_pass.nocache)
 {
 flags |= GET_USER_PASS_PREVIOUS_CREDS_FAILED;
 }
@@ -288,9 +288,14 @@
   auth_file,
   UP_TYPE_PROXY,
   flags);
-p->queried_creds = true;
-p->up = static_proxy_user_pass;
+static_proxy_user_pass.nocache = p->options.nocache;
 }
+
+/*
+ * Using cached credentials
+ */
+p->queried_creds = true;
+p->up = static_proxy_user_pass;
 }
 
 #if 0
@@ -542,7 +547,7 @@
  * we know whether we need any. */
 if (p->auth_method == HTTP_AUTH_BASIC || p->auth_method == HTTP_AUTH_NTLM2)
 {
-get_user_pass_http(p, true);
+get_user_pass_http(p, p->options.first_time);
 }
 
 #if !NTLM
@@ -656,6 +661,11 @@
 || p->auth_method == HTTP_AUTH_NTLM2)
 {
 get_user_pass_http(p, false);
+
+if (p->up.nocache)
+{
+clear_user_pass_http();
+}
 }
 
 /* are we being called again after getting the digest server nonce in the 
previous transaction? */
@@ -1031,13 +1041,6 @@
 }
 goto error;
 }
-
-/* clear state */
-if (p->options.auth_retry)
-{
-clear_user_pass_http();
-}
-store_proxy_authenticate(p, NULL);
 }
 
 /* check return code, success = 200 */
diff --git a/src/openvpn/proxy.h b/src/openvpn/proxy.h
index 4e78772..474cfc9 100644
--- a/src/openvpn/proxy.h
+++ b/src/openvpn/proxy.h
@@ -57,6 +57,8 @@
 const char *user_agent;
 struct http_custom_header custom_headers[MAX_CUSTOM_HTTP_HEADER];
 bool inline_creds; /* auth_file_up is inline credentials */
+bool first_time; /* indicates if we need to wipe user creds at the

[Openvpn-devel] [M] Change in openvpn[master]: Route: add support for user defined routing table

2024-03-12 Thread flichtenheld (Code Review) 
Attention is currently required from: its_Giaan, plaisthos.

flichtenheld has posted comments on this change. ( 
http://gerrit.openvpn.net/c/openvpn/+/524?usp=email )

Change subject: Route: add support for user defined routing table
..


Patch Set 3: Code-Review-1

(7 comments)

File doc/man-sections/vpn-network-options.rst:

http://gerrit.openvpn.net/c/openvpn/+/524/comment/29fafc4d_019065e1 :
PS3, Line 374:   a user defined routing table can be used instead.
"an user"


http://gerrit.openvpn.net/c/openvpn/+/524/comment/cfaf208a_43bb3e07 :
PS3, Line 407: Since this option must be an entirely local choice, 
won't be pushable.
Time seems weird to me, would change to "it isn't pushable"


http://gerrit.openvpn.net/c/openvpn/+/524/comment/c5dce9cb_c194c9e8 :
PS3, Line 408:default taken from ``--route-table`` if set, otherwise 
:code:`0`.
please change tab to space


http://gerrit.openvpn.net/c/openvpn/+/524/comment/186800fb_289c2ebc :
PS3, Line 461:  route-ipv6 ipv6addr/bits [gateway]
when listing all possible choices, you can leave off the "[" optional markers


http://gerrit.openvpn.net/c/openvpn/+/524/comment/dc32db5a_b03f8605 :
PS3, Line 464:
trailing whitespace


http://gerrit.openvpn.net/c/openvpn/+/524/comment/c7665500_392297ff :
PS3, Line 475:default taken from ``--route-table`` if set, otherwise 
:code:`0`.
same comments apply here as for --route (tab, won't->it isn't)


File src/openvpn/route.h:

http://gerrit.openvpn.net/c/openvpn/+/524/comment/f42523ea_da5a7db8 :
PS3, Line 98:
spurious whitespace?



--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/524?usp=email
To unsubscribe, or for help writing mail filters, visit 
http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I3e4ebef484d2a04a383a65ede5617ee98bf218a7
Gerrit-Change-Number: 524
Gerrit-PatchSet: 3
Gerrit-Owner: its_Giaan 
Gerrit-Reviewer: flichtenheld 
Gerrit-Reviewer: plaisthos 
Gerrit-CC: openvpn-devel 
Gerrit-Attention: plaisthos 
Gerrit-Attention: its_Giaan 
Gerrit-Comment-Date: Tue, 12 Mar 2024 17:11:24 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [XS] Change in openvpn[master]: Avoid SIGUR1 to SIGHUP when the configuration is read from stdin

2024-03-12 Thread flichtenheld (Code Review) 
Attention is currently required from: plaisthos.

flichtenheld has posted comments on this change. ( 
http://gerrit.openvpn.net/c/openvpn/+/533?usp=email )

Change subject: Avoid SIGUR1 to SIGHUP when the configuration is read from stdin
..


Patch Set 1: Code-Review-1

(1 comment)

Commit Message:

http://gerrit.openvpn.net/c/openvpn/+/533/comment/2cfbe3af_29c12353 :
PS1, Line 7: Avoid SIGUR1 to SIGHUP when the configuration is read from stdin
Typo -> "SIGUSR1"



--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/533?usp=email
To unsubscribe, or for help writing mail filters, visit 
http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Icfc179490d6821e22d14817941fb0bad667c713f
Gerrit-Change-Number: 533
Gerrit-PatchSet: 1
Gerrit-Owner: plaisthos 
Gerrit-Reviewer: flichtenheld 
Gerrit-CC: openvpn-devel 
Gerrit-Attention: plaisthos 
Gerrit-Comment-Date: Tue, 12 Mar 2024 16:38:50 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [M] Change in openvpn[master]: Http-proxy: fix bug preventing proxy credentials caching

2024-03-12 Thread flichtenheld (Code Review) 
Attention is currently required from: its_Giaan, plaisthos.

flichtenheld has posted comments on this change. ( 
http://gerrit.openvpn.net/c/openvpn/+/523?usp=email )

Change subject: Http-proxy: fix bug preventing proxy credentials caching
..


Patch Set 8: Code-Review+2

(1 comment)

Patchset:

PS8:
Tested with "stdin ntlm" and "auto" against the community NTLM proxy. With and 
without --auth-nocache.



--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/523?usp=email
To unsubscribe, or for help writing mail filters, visit 
http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Ia3e06c0832c4ca0ab868c845279fb71c01a1a78a
Gerrit-Change-Number: 523
Gerrit-PatchSet: 8
Gerrit-Owner: its_Giaan 
Gerrit-Reviewer: flichtenheld 
Gerrit-Reviewer: plaisthos 
Gerrit-CC: openvpn-devel 
Gerrit-Attention: plaisthos 
Gerrit-Attention: its_Giaan 
Gerrit-Comment-Date: Tue, 12 Mar 2024 16:36:48 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [M] Change in openvpn[master]: Http-proxy: fix bug preventing proxy credentials caching

2024-03-12 Thread its_Giaan (Code Review) 
Attention is currently required from: flichtenheld, plaisthos.

its_Giaan has posted comments on this change. ( 
http://gerrit.openvpn.net/c/openvpn/+/523?usp=email )

Change subject: Http-proxy: fix bug preventing proxy credentials caching
..


Patch Set 7:

(1 comment)

File src/openvpn/proxy.c:

http://gerrit.openvpn.net/c/openvpn/+/523/comment/07861da3_4c72dba0 :
PS7, Line 293: p->up = static_proxy_user_pass;
> These two lines are redundant now. Please remove them.
Done



--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/523?usp=email
To unsubscribe, or for help writing mail filters, visit 
http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Ia3e06c0832c4ca0ab868c845279fb71c01a1a78a
Gerrit-Change-Number: 523
Gerrit-PatchSet: 7
Gerrit-Owner: its_Giaan 
Gerrit-Reviewer: flichtenheld 
Gerrit-Reviewer: plaisthos 
Gerrit-CC: openvpn-devel 
Gerrit-Attention: plaisthos 
Gerrit-Attention: flichtenheld 
Gerrit-Comment-Date: Tue, 12 Mar 2024 13:43:13 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: flichtenheld 
Gerrit-MessageType: comment
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [M] Change in openvpn[master]: Http-proxy: fix bug preventing proxy credentials caching

2024-03-12 Thread its_Giaan (Code Review) 
Attention is currently required from: flichtenheld, plaisthos.

Hello flichtenheld, plaisthos,

I'd like you to reexamine a change. Please visit

http://gerrit.openvpn.net/c/openvpn/+/523?usp=email

to look at the new patch set (#8).

The following approvals got outdated and were removed:
Code-Review-1 by flichtenheld


Change subject: Http-proxy: fix bug preventing proxy credentials caching
..

Http-proxy: fix bug preventing proxy credentials caching

Caching proxy credentials was not working due to the
lack of handling already defined creds in get_user_pass(),
which prevented the caching from working properly.

Fix this issue by getting the value of c->first_time,
that indicates if we're at the first iteration
of the main loop and use it as second argument of the
get_user_pass_http(). Otherwise, on SIGUSR1 or SIGHUP
upon instance context restart credentials would be erased
every time.

The nocache member has been added to the struct
http_proxy_options and also a getter method to retrieve
that option from ssl has been added, by doing this
we're able to erase previous queried user credentials
to ensure correct operation.

Fixes: Trac #1187
Signed-off-by: Gianmarco De Gregori 
Change-Id: Ia3e06c0832c4ca0ab868c845279fb71c01a1a78a
---
M doc/man-sections/generic-options.rst
M src/openvpn/init.c
M src/openvpn/options.c
M src/openvpn/proxy.c
M src/openvpn/proxy.h
M src/openvpn/ssl.c
M src/openvpn/ssl.h
7 files changed, 38 insertions(+), 14 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/23/523/8

diff --git a/doc/man-sections/generic-options.rst 
b/doc/man-sections/generic-options.rst
index f8a0f48..1fb17f7 100644
--- a/doc/man-sections/generic-options.rst
+++ b/doc/man-sections/generic-options.rst
@@ -19,9 +19,6 @@
   When using ``--auth-nocache`` in combination with a user/password file
   and ``--chroot`` or ``--daemon``, make sure to use an absolute path.

-  This directive does not affect the ``--http-proxy`` username/password.
-  It is always cached.
-
 --cd dir
   Change directory to ``dir`` prior to reading any files such as
   configuration files, key files, scripts, etc. ``dir`` should be an
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index 52b3931..fdee199 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -697,6 +697,8 @@

 if (c->options.ce.http_proxy_options)
 {
+c->options.ce.http_proxy_options->first_time = c->first_time;
+
 /* Possible HTTP proxy user/pass input */
 c->c1.http_proxy = http_proxy_new(c->options.ce.http_proxy_options);
 if (c->c1.http_proxy)
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 94a88f9..d276a1a 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -1650,6 +1650,7 @@
 SHOW_STR(auth_file);
 SHOW_STR(auth_file_up);
 SHOW_BOOL(inline_creds);
+SHOW_BOOL(nocache);
 SHOW_STR(http_version);
 SHOW_STR(user_agent);
 for  (i = 0; i < MAX_CUSTOM_HTTP_HEADER && o->custom_headers[i].name; i++)
@@ -3151,6 +3152,11 @@
 ce->flags |= CE_DISABLED;
 }

+if (ce->http_proxy_options)
+{
+ce->http_proxy_options->nocache = ssl_get_auth_nocache();
+}
+
 /* our socks code is not fully IPv6 enabled yet (TCP works, UDP not)
  * so fall back to IPv4-only (trac #1221)
  */
diff --git a/src/openvpn/proxy.c b/src/openvpn/proxy.c
index eeb3989..4570fbc 100644
--- a/src/openvpn/proxy.c
+++ b/src/openvpn/proxy.c
@@ -276,7 +276,7 @@
 {
 auth_file = p->options.auth_file_up;
 }
-if (p->queried_creds)
+if (p->queried_creds && !static_proxy_user_pass.nocache)
 {
 flags |= GET_USER_PASS_PREVIOUS_CREDS_FAILED;
 }
@@ -288,9 +288,14 @@
   auth_file,
   UP_TYPE_PROXY,
   flags);
-p->queried_creds = true;
-p->up = static_proxy_user_pass;
+static_proxy_user_pass.nocache = p->options.nocache;
 }
+
+/*
+ * Using cached credentials
+ */
+p->queried_creds = true;
+p->up = static_proxy_user_pass;
 }

 #if 0
@@ -542,7 +547,7 @@
  * we know whether we need any. */
 if (p->auth_method == HTTP_AUTH_BASIC || p->auth_method == HTTP_AUTH_NTLM2)
 {
-get_user_pass_http(p, true);
+get_user_pass_http(p, p->options.first_time);
 }

 #if !NTLM
@@ -656,6 +661,11 @@
 || p->auth_method == HTTP_AUTH_NTLM2)
 {
 get_user_pass_http(p, false);
+
+if (p->up.nocache)
+{
+clear_user_pass_http();
+}
 }

 /* are we being called again after getting the digest server nonce in the 
previous transaction? */
@@ -1031,13 +1041,6 @@
 }
 goto error;
 }
-
-/* clear state */
-if (p->options.auth_retry)
-{
-clear_user_pass_http();
-}
-

[Openvpn-devel] [M] Change in openvpn[master]: Http-proxy: fix bug preventing proxy credentials caching

2024-03-12 Thread flichtenheld (Code Review) 
Attention is currently required from: its_Giaan, plaisthos.

flichtenheld has posted comments on this change. ( 
http://gerrit.openvpn.net/c/openvpn/+/523?usp=email )

Change subject: Http-proxy: fix bug preventing proxy credentials caching
..


Patch Set 7: Code-Review-1

(2 comments)

Patchset:

PS7:
almost there...


File src/openvpn/proxy.c:

http://gerrit.openvpn.net/c/openvpn/+/523/comment/0285cd49_4660ab6b :
PS7, Line 293: p->up = static_proxy_user_pass;
These two lines are redundant now. Please remove them.



--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/523?usp=email
To unsubscribe, or for help writing mail filters, visit 
http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Ia3e06c0832c4ca0ab868c845279fb71c01a1a78a
Gerrit-Change-Number: 523
Gerrit-PatchSet: 7
Gerrit-Owner: its_Giaan 
Gerrit-Reviewer: flichtenheld 
Gerrit-Reviewer: plaisthos 
Gerrit-CC: openvpn-devel 
Gerrit-Attention: plaisthos 
Gerrit-Attention: its_Giaan 
Gerrit-Comment-Date: Tue, 12 Mar 2024 13:36:07 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [M] Change in openvpn[master]: Http-proxy: fix bug preventing proxy credentials caching

2024-03-12 Thread its_Giaan (Code Review) 
Attention is currently required from: its_Giaan, plaisthos.

Hello flichtenheld, plaisthos,

I'd like you to reexamine a change. Please visit

http://gerrit.openvpn.net/c/openvpn/+/523?usp=email

to look at the new patch set (#7).


Change subject: Http-proxy: fix bug preventing proxy credentials caching
..

Http-proxy: fix bug preventing proxy credentials caching

Caching proxy credentials was not working due to the
lack of handling already defined creds in get_user_pass(),
which prevented the caching from working properly.

Fix this issue by getting the value of c->first_time,
that indicates if we're at the first iteration
of the main loop and use it as second argument of the
get_user_pass_http(). Otherwise, on SIGUSR1 or SIGHUP
upon instance context restart credentials would be erased
every time.

The nocache member has been added to the struct
http_proxy_options and also a getter method to retrieve
that option from ssl has been added, by doing this
we're able to erase previous queried user credentials
to ensure correct operation.

Fixes: Trac #1187
Signed-off-by: Gianmarco De Gregori 
Change-Id: Ia3e06c0832c4ca0ab868c845279fb71c01a1a78a
---
M doc/man-sections/generic-options.rst
M src/openvpn/init.c
M src/openvpn/options.c
M src/openvpn/proxy.c
M src/openvpn/proxy.h
M src/openvpn/ssl.c
M src/openvpn/ssl.h
7 files changed, 38 insertions(+), 12 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/23/523/7

diff --git a/doc/man-sections/generic-options.rst 
b/doc/man-sections/generic-options.rst
index f8a0f48..1fb17f7 100644
--- a/doc/man-sections/generic-options.rst
+++ b/doc/man-sections/generic-options.rst
@@ -19,9 +19,6 @@
   When using ``--auth-nocache`` in combination with a user/password file
   and ``--chroot`` or ``--daemon``, make sure to use an absolute path.

-  This directive does not affect the ``--http-proxy`` username/password.
-  It is always cached.
-
 --cd dir
   Change directory to ``dir`` prior to reading any files such as
   configuration files, key files, scripts, etc. ``dir`` should be an
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index 52b3931..fdee199 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -697,6 +697,8 @@

 if (c->options.ce.http_proxy_options)
 {
+c->options.ce.http_proxy_options->first_time = c->first_time;
+
 /* Possible HTTP proxy user/pass input */
 c->c1.http_proxy = http_proxy_new(c->options.ce.http_proxy_options);
 if (c->c1.http_proxy)
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 94a88f9..d276a1a 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -1650,6 +1650,7 @@
 SHOW_STR(auth_file);
 SHOW_STR(auth_file_up);
 SHOW_BOOL(inline_creds);
+SHOW_BOOL(nocache);
 SHOW_STR(http_version);
 SHOW_STR(user_agent);
 for  (i = 0; i < MAX_CUSTOM_HTTP_HEADER && o->custom_headers[i].name; i++)
@@ -3151,6 +3152,11 @@
 ce->flags |= CE_DISABLED;
 }

+if (ce->http_proxy_options)
+{
+ce->http_proxy_options->nocache = ssl_get_auth_nocache();
+}
+
 /* our socks code is not fully IPv6 enabled yet (TCP works, UDP not)
  * so fall back to IPv4-only (trac #1221)
  */
diff --git a/src/openvpn/proxy.c b/src/openvpn/proxy.c
index eeb3989..74fe115 100644
--- a/src/openvpn/proxy.c
+++ b/src/openvpn/proxy.c
@@ -276,7 +276,7 @@
 {
 auth_file = p->options.auth_file_up;
 }
-if (p->queried_creds)
+if (p->queried_creds && !static_proxy_user_pass.nocache)
 {
 flags |= GET_USER_PASS_PREVIOUS_CREDS_FAILED;
 }
@@ -288,9 +288,16 @@
   auth_file,
   UP_TYPE_PROXY,
   flags);
+static_proxy_user_pass.nocache = p->options.nocache;
 p->queried_creds = true;
 p->up = static_proxy_user_pass;
 }
+
+/*
+ * Using cached credentials
+ */
+p->queried_creds = true;
+p->up = static_proxy_user_pass;
 }

 #if 0
@@ -542,7 +549,7 @@
  * we know whether we need any. */
 if (p->auth_method == HTTP_AUTH_BASIC || p->auth_method == HTTP_AUTH_NTLM2)
 {
-get_user_pass_http(p, true);
+get_user_pass_http(p, p->options.first_time);
 }

 #if !NTLM
@@ -656,6 +663,11 @@
 || p->auth_method == HTTP_AUTH_NTLM2)
 {
 get_user_pass_http(p, false);
+
+if (p->up.nocache)
+{
+clear_user_pass_http();
+}
 }

 /* are we being called again after getting the digest server nonce in the 
previous transaction? */
@@ -1031,13 +1043,6 @@
 }
 goto error;
 }
-
-/* clear state */
-if (p->options.auth_retry)
-{
-clear_user_pass_http();
-}
-store_proxy_authenticate(p, NULL);
 }

 /* check return code, success = 200 */
diff --git

[Openvpn-devel] [M] Change in openvpn[master]: Http-proxy: fix bug preventing proxy credentials caching

2024-03-12 Thread its_Giaan (Code Review) 
Attention is currently required from: flichtenheld, plaisthos.

its_Giaan has posted comments on this change. ( 
http://gerrit.openvpn.net/c/openvpn/+/523?usp=email )

Change subject: Http-proxy: fix bug preventing proxy credentials caching
..


Patch Set 7:

(1 comment)

File src/openvpn/proxy.c:

http://gerrit.openvpn.net/c/openvpn/+/523/comment/00ff12bf_f5ff24b0 :
PS6, Line 304: else if (static_proxy_user_pass.nocache && !force)
> You're right about the force. […]
Done



--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/523?usp=email
To unsubscribe, or for help writing mail filters, visit 
http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Ia3e06c0832c4ca0ab868c845279fb71c01a1a78a
Gerrit-Change-Number: 523
Gerrit-PatchSet: 7
Gerrit-Owner: its_Giaan 
Gerrit-Reviewer: flichtenheld 
Gerrit-Reviewer: plaisthos 
Gerrit-CC: openvpn-devel 
Gerrit-Attention: plaisthos 
Gerrit-Attention: flichtenheld 
Gerrit-Comment-Date: Tue, 12 Mar 2024 13:08:53 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: its_Giaan 
Comment-In-Reply-To: flichtenheld 
Gerrit-MessageType: comment
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [M] Change in openvpn[master]: Http-proxy: fix bug preventing proxy credentials caching

2024-03-12 Thread flichtenheld (Code Review) 
Attention is currently required from: its_Giaan, plaisthos.

flichtenheld has posted comments on this change. ( 
http://gerrit.openvpn.net/c/openvpn/+/523?usp=email )

Change subject: Http-proxy: fix bug preventing proxy credentials caching
..


Patch Set 6:

(1 comment)

File src/openvpn/proxy.c:

http://gerrit.openvpn.net/c/openvpn/+/523/comment/a8be4b5f_3cc2fa9a :
PS6, Line 304: else if (static_proxy_user_pass.nocache && !force)
> I think we need it otherwise in case of nocache true creds would be wiped at 
> the first call of the g […]
You're right about the force. I didn't really think about the fact that when 
you don't use "auto" get_user_pass_http is going to be always called twice. But 
that also invalidates my other suggestion about the "else". The test for force 
is not enough since on restart force will be false for the first call. And it 
that case we should not clear it. I think we should go back to the previous 
iteration in this case. Move it out of the function again after the second 
get_user_pass_http. We really only want to delete it there after all.



--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/523?usp=email
To unsubscribe, or for help writing mail filters, visit 
http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Ia3e06c0832c4ca0ab868c845279fb71c01a1a78a
Gerrit-Change-Number: 523
Gerrit-PatchSet: 6
Gerrit-Owner: its_Giaan 
Gerrit-Reviewer: flichtenheld 
Gerrit-Reviewer: plaisthos 
Gerrit-CC: openvpn-devel 
Gerrit-Attention: plaisthos 
Gerrit-Attention: its_Giaan 
Gerrit-Comment-Date: Tue, 12 Mar 2024 12:38:35 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: its_Giaan 
Comment-In-Reply-To: flichtenheld 
Gerrit-MessageType: comment
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [M] Change in openvpn[master]: Http-proxy: fix bug preventing proxy credentials caching

2024-03-12 Thread its_Giaan (Code Review) 
Attention is currently required from: flichtenheld, plaisthos.

its_Giaan has posted comments on this change. ( 
http://gerrit.openvpn.net/c/openvpn/+/523?usp=email )

Change subject: Http-proxy: fix bug preventing proxy credentials caching
..


Patch Set 6:

(2 comments)

File src/openvpn/proxy.c:

http://gerrit.openvpn.net/c/openvpn/+/523/comment/42ea2ef6_c71a07f0 :
PS6, Line 299: else if (!static_proxy_user_pass.nocache)
> Do not actually need to check for nocache here. Since it will never be 
> defined when nocache is set. […]
Acknowledged


http://gerrit.openvpn.net/c/openvpn/+/523/comment/19fd5e77_9975a90f :
PS6, Line 304: else if (static_proxy_user_pass.nocache && !force)
> I think this condition is too complicated. […]
I think we need it otherwise in case of nocache true creds would be wiped at 
the first call of the get_user_pass() and then asked again at the second call 
(the one with force == false), by checking force we know that we're at the 
second call of the function so we can clear previous creds since we don't need 
them anymore.
Totally agree about the "else".



--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/523?usp=email
To unsubscribe, or for help writing mail filters, visit 
http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Ia3e06c0832c4ca0ab868c845279fb71c01a1a78a
Gerrit-Change-Number: 523
Gerrit-PatchSet: 6
Gerrit-Owner: its_Giaan 
Gerrit-Reviewer: flichtenheld 
Gerrit-Reviewer: plaisthos 
Gerrit-CC: openvpn-devel 
Gerrit-Attention: plaisthos 
Gerrit-Attention: flichtenheld 
Gerrit-Comment-Date: Tue, 12 Mar 2024 11:27:31 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: flichtenheld 
Gerrit-MessageType: comment
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [M] Change in openvpn[master]: Http-proxy: fix bug preventing proxy credentials caching

2024-03-12 Thread flichtenheld (Code Review) 
Attention is currently required from: its_Giaan, plaisthos.

flichtenheld has posted comments on this change. ( 
http://gerrit.openvpn.net/c/openvpn/+/523?usp=email )

Change subject: Http-proxy: fix bug preventing proxy credentials caching
..


Patch Set 6: Code-Review-2

(2 comments)

File src/openvpn/proxy.c:

http://gerrit.openvpn.net/c/openvpn/+/523/comment/5c0d5864_4b0d142a :
PS6, Line 299: else if (!static_proxy_user_pass.nocache)
Do not actually need to check for nocache here. Since it will never be defined 
when nocache is set. So the check is redundant. You can even remove the else 
completely then and just always use static_proxy_user_pass at this point in the 
function.


http://gerrit.openvpn.net/c/openvpn/+/523/comment/b8aa24f7_cae22dbf :
PS6, Line 304: else if (static_proxy_user_pass.nocache && !force)
I think this condition is too complicated. What does this have to do with 
force? Also why the "else"? Just always delete it when nocache is true... Or am 
I missing something?



--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/523?usp=email
To unsubscribe, or for help writing mail filters, visit 
http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Ia3e06c0832c4ca0ab868c845279fb71c01a1a78a
Gerrit-Change-Number: 523
Gerrit-PatchSet: 6
Gerrit-Owner: its_Giaan 
Gerrit-Reviewer: flichtenheld 
Gerrit-Reviewer: plaisthos 
Gerrit-CC: openvpn-devel 
Gerrit-Attention: plaisthos 
Gerrit-Attention: its_Giaan 
Gerrit-Comment-Date: Tue, 12 Mar 2024 11:04:43 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [M] Change in openvpn[master]: Http-proxy: fix bug preventing proxy credentials caching

2024-03-12 Thread its_Giaan (Code Review) 
Attention is currently required from: flichtenheld, plaisthos.

Hello flichtenheld, plaisthos,

I'd like you to reexamine a change. Please visit

http://gerrit.openvpn.net/c/openvpn/+/523?usp=email

to look at the new patch set (#6).


Change subject: Http-proxy: fix bug preventing proxy credentials caching
..

Http-proxy: fix bug preventing proxy credentials caching

Caching proxy credentials was not working due to the
lack of handling already defined creds in get_user_pass(),
which prevented the caching from working properly.

Fix this issue by getting the value of c->first_time,
that indicates if we're at the first iteration
of the main loop and use it as second argument of the
get_user_pass_http(). Otherwise, on SIGUSR1 or SIGHUP
upon instance context restart credentials would be erased
every time.

The nocache member has been added to the struct
http_proxy_options and also a getter method to retrieve
that option from ssl has been added, by doing this
we're able to erase previous queried user credentials
to ensure correct operation.

Fixes: Trac #1187
Signed-off-by: Gianmarco De Gregori 
Change-Id: Ia3e06c0832c4ca0ab868c845279fb71c01a1a78a
---
M doc/man-sections/generic-options.rst
M src/openvpn/init.c
M src/openvpn/options.c
M src/openvpn/proxy.c
M src/openvpn/proxy.h
M src/openvpn/ssl.c
M src/openvpn/ssl.h
7 files changed, 40 insertions(+), 12 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/23/523/6

diff --git a/doc/man-sections/generic-options.rst 
b/doc/man-sections/generic-options.rst
index f8a0f48..1fb17f7 100644
--- a/doc/man-sections/generic-options.rst
+++ b/doc/man-sections/generic-options.rst
@@ -19,9 +19,6 @@
   When using ``--auth-nocache`` in combination with a user/password file
   and ``--chroot`` or ``--daemon``, make sure to use an absolute path.

-  This directive does not affect the ``--http-proxy`` username/password.
-  It is always cached.
-
 --cd dir
   Change directory to ``dir`` prior to reading any files such as
   configuration files, key files, scripts, etc. ``dir`` should be an
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index 52b3931..fdee199 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -697,6 +697,8 @@

 if (c->options.ce.http_proxy_options)
 {
+c->options.ce.http_proxy_options->first_time = c->first_time;
+
 /* Possible HTTP proxy user/pass input */
 c->c1.http_proxy = http_proxy_new(c->options.ce.http_proxy_options);
 if (c->c1.http_proxy)
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 94a88f9..d276a1a 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -1650,6 +1650,7 @@
 SHOW_STR(auth_file);
 SHOW_STR(auth_file_up);
 SHOW_BOOL(inline_creds);
+SHOW_BOOL(nocache);
 SHOW_STR(http_version);
 SHOW_STR(user_agent);
 for  (i = 0; i < MAX_CUSTOM_HTTP_HEADER && o->custom_headers[i].name; i++)
@@ -3151,6 +3152,11 @@
 ce->flags |= CE_DISABLED;
 }

+if (ce->http_proxy_options)
+{
+ce->http_proxy_options->nocache = ssl_get_auth_nocache();
+}
+
 /* our socks code is not fully IPv6 enabled yet (TCP works, UDP not)
  * so fall back to IPv4-only (trac #1221)
  */
diff --git a/src/openvpn/proxy.c b/src/openvpn/proxy.c
index eeb3989..d44d2c1 100644
--- a/src/openvpn/proxy.c
+++ b/src/openvpn/proxy.c
@@ -276,7 +276,7 @@
 {
 auth_file = p->options.auth_file_up;
 }
-if (p->queried_creds)
+if (p->queried_creds && !static_proxy_user_pass.nocache)
 {
 flags |= GET_USER_PASS_PREVIOUS_CREDS_FAILED;
 }
@@ -288,9 +288,23 @@
   auth_file,
   UP_TYPE_PROXY,
   flags);
+static_proxy_user_pass.nocache = p->options.nocache;
 p->queried_creds = true;
 p->up = static_proxy_user_pass;
 }
+
+/*
+ * Using cached credentials
+ */
+else if (!static_proxy_user_pass.nocache)
+{
+p->queried_creds = true;
+p->up = static_proxy_user_pass;
+}
+else if (static_proxy_user_pass.nocache && !force)
+{
+clear_user_pass_http();
+}
 }

 #if 0
@@ -542,7 +556,7 @@
  * we know whether we need any. */
 if (p->auth_method == HTTP_AUTH_BASIC || p->auth_method == HTTP_AUTH_NTLM2)
 {
-get_user_pass_http(p, true);
+get_user_pass_http(p, p->options.first_time);
 }

 #if !NTLM
@@ -1031,13 +1045,6 @@
 }
 goto error;
 }
-
-/* clear state */
-if (p->options.auth_retry)
-{
-clear_user_pass_http();
-}
-store_proxy_authenticate(p, NULL);
 }

 /* check return code, success = 200 */
diff --git a/src/openvpn/proxy.h b/src/openvpn/proxy.h
index 4e78772..474cfc9 100644
--- a/src/openvpn/proxy.h
+++ b/src/openvpn/proxy.h
@@ -57,6 +57,8 @@

[Openvpn-devel] [S] Change in openvpn[master]: Http-proxy: fix bug preventing proxy credentials caching

2024-03-12 Thread its_Giaan (Code Review) 
Attention is currently required from: flichtenheld, plaisthos.

its_Giaan has posted comments on this change. ( 
http://gerrit.openvpn.net/c/openvpn/+/523?usp=email )

Change subject: Http-proxy: fix bug preventing proxy credentials caching
..


Patch Set 5:

(4 comments)

Commit Message:

http://gerrit.openvpn.net/c/openvpn/+/523/comment/87905ca4_736be2e0 :
PS5, Line 21: http_proxy_options and also a getter method to retrieve
> The man page for --auth-nocache still says it has no effect on --http-proxy. 
> […]
Done


File src/openvpn/options.c:

http://gerrit.openvpn.net/c/openvpn/+/523/comment/3b08b487_e3fc606d :
PS5, Line 3126: if (ce->http_proxy_options)
> This needs to be moved one level up. E.g. […]
Done


File src/openvpn/proxy.h:

http://gerrit.openvpn.net/c/openvpn/+/523/comment/0c4157a3_109dd1dc :
PS5, Line 61: bool nocache;
> Please update "show_http_proxy_options" in options.c to show value of nocache.
Done


File src/openvpn/proxy.c:

http://gerrit.openvpn.net/c/openvpn/+/523/comment/f7da0dee_13ae81ae :
PS5, Line 669: if (p->up.nocache)
> wouldn't it be better to move this clearing into get_user_pass_http?
Done



--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/523?usp=email
To unsubscribe, or for help writing mail filters, visit 
http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Ia3e06c0832c4ca0ab868c845279fb71c01a1a78a
Gerrit-Change-Number: 523
Gerrit-PatchSet: 5
Gerrit-Owner: its_Giaan 
Gerrit-Reviewer: flichtenheld 
Gerrit-Reviewer: plaisthos 
Gerrit-CC: openvpn-devel 
Gerrit-Attention: plaisthos 
Gerrit-Attention: flichtenheld 
Gerrit-Comment-Date: Tue, 12 Mar 2024 08:36:36 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: flichtenheld 
Gerrit-MessageType: comment
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel