[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


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

2024-03-11 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 5:

(4 comments)

Commit Message:

http://gerrit.openvpn.net/c/openvpn/+/523/comment/0095e7d0_7032f979 :
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. 
This needs to be updated.


File src/openvpn/options.c:

http://gerrit.openvpn.net/c/openvpn/+/523/comment/10c5ad5e_1e39dee3 :
PS5, Line 3126: if (ce->http_proxy_options)
This needs to be moved one level up. E.g. when using "--proto tcp4-client" 
currently --auth-nocache is ignored, since proto is already PROTO_TCP_CLIENT.
There should be no issue with always setting this independently of ce->proto.


File src/openvpn/proxy.h:

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


File src/openvpn/proxy.c:

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



--
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: its_Giaan 
Gerrit-Comment-Date: Mon, 11 Mar 2024 14:45:37 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


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

2024-03-11 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 5: Code-Review-2

(1 comment)

Patchset:

PS5:
Tried to test this change but my tests failed to produce the desired behavior. 
Should figure out the problem before merging.



--
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: its_Giaan 
Gerrit-Comment-Date: Mon, 11 Mar 2024 12:29: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] [S] Change in openvpn[master]: Http-proxy: fix bug preventing proxy credentials caching

2024-02-27 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 (#4).


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 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
6 files changed, 38 insertions(+), 2 deletions(-)


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

diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index 52b4308..dc1ee8d 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 2c79a1e..0d22df9 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -3123,6 +3123,10 @@
 if (ce->proto == PROTO_TCP)
 {
 ce->proto = PROTO_TCP_CLIENT;
+if (ce->http_proxy_options)
+{
+ce->http_proxy_options->nocache = ssl_get_auth_nocache();
+}
 }
 }

diff --git a/src/openvpn/proxy.c b/src/openvpn/proxy.c
index eeb3989..f1e1b21 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,6 +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
+ */
+else if (!static_proxy_user_pass.nocache)
+{
 p->queried_creds = true;
 p->up = static_proxy_user_pass;
 }
@@ -542,7 +552,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 +666,10 @@
 || 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? */
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 first 
iteration of the main loop */
+bool nocache;
 };

 struct http_proxy_options_simple {
diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index 33c8670..d174dad 100644
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -335,6 +335,15 @@
 }

 /*
+ * Get the password caching
+ */
+bool
+ssl_get_auth_nocache()
+{
+return passbuf.nocache;
+}
+
+/*
  * Set an authentication token
  */
 void
diff --git a/src/openvpn/ssl.h b/src/openvpn/ssl.h
index 71b99db..dd6538c 100644
--- a/src/openvpn/ssl.h
+++ b/src/openvpn/ssl.h
@@ -397,6 +397,11 @@
 void ssl_set_auth_nocache(void);

 /*
+ * Getter method for retrieving the auth-nocache option.
+ */
+bool ssl_get_auth_nocache();
+
+/*
  * Purge any stored authentication information, both for key files and tunnel
  * authentication. If PCKS #11 is enabled, 

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

2024-02-27 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 3:

(1 comment)

File src/openvpn/proxy.c:

http://gerrit.openvpn.net/c/openvpn/+/523/comment/07dbbffd_a306e8c1 :
PS3, Line 555: get_user_pass_http(p, p->options.first_time);
> get_user_pass_http uses p->options.nocache but that is only set below. […]
@fr...@lichtenheld.com, not at all, actually the p->options are setted little 
bit before the get_user_pass_http() through p->options = *o, so setting it 
below is something useless that I left during the last clean-up.



--
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: 3
Gerrit-Owner: its_Giaan 
Gerrit-Reviewer: flichtenheld 
Gerrit-Reviewer: plaisthos 
Gerrit-CC: openvpn-devel 
Gerrit-Attention: plaisthos 
Gerrit-Attention: flichtenheld 
Gerrit-Comment-Date: Tue, 27 Feb 2024 13:54:25 +
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] [S] Change in openvpn[master]: Http-proxy: fix bug preventing proxy credentials caching

2024-02-27 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 3: -Code-Review

(1 comment)

File src/openvpn/proxy.c:

http://gerrit.openvpn.net/c/openvpn/+/523/comment/e27428fc_b097b77f :
PS3, Line 555: get_user_pass_http(p, p->options.first_time);
get_user_pass_http uses p->options.nocache but that is only set below. Is that 
intentional?



--
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: 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, 27 Feb 2024 13:24:26 +
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] [S] Change in openvpn[master]: Http-proxy: fix bug preventing proxy credentials caching

2024-02-26 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 (#3).


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 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
6 files changed, 39 insertions(+), 2 deletions(-)


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

diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index 52b4308..dc1ee8d 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 2c79a1e..0d22df9 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -3123,6 +3123,10 @@
 if (ce->proto == PROTO_TCP)
 {
 ce->proto = PROTO_TCP_CLIENT;
+if (ce->http_proxy_options)
+{
+ce->http_proxy_options->nocache = ssl_get_auth_nocache();
+}
 }
 }

diff --git a/src/openvpn/proxy.c b/src/openvpn/proxy.c
index eeb3989..ff50539 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,6 +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
+ */
+else if (!static_proxy_user_pass.nocache)
+{
 p->queried_creds = true;
 p->up = static_proxy_user_pass;
 }
@@ -542,7 +552,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
@@ -553,6 +563,7 @@
 #endif

 p->defined = true;
+p->options.nocache = o->nocache;
 return p;
 }

@@ -656,6 +667,10 @@
 || 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? */
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 first 
iteration of the main loop */
+bool nocache;
 };

 struct http_proxy_options_simple {
diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index 33c8670..d174dad 100644
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -335,6 +335,15 @@
 }

 /*
+ * Get the password caching
+ */
+bool
+ssl_get_auth_nocache()
+{
+return passbuf.nocache;
+}
+
+/*
  * Set an authentication token
  */
 void
diff --git a/src/openvpn/ssl.h b/src/openvpn/ssl.h
index 71b99db..dd6538c 100644
--- a/src/openvpn/ssl.h
+++ b/src/openvpn/ssl.h
@@ -397,6 +397,11 @@
 void ssl_set_auth_nocache(void);

 /*
+ * Getter method for retrieving the auth-nocache option.
+ */
+bool ssl_get_auth_nocache();
+
+/*
  * Purge any 

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

2024-02-26 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 2:

(4 comments)

File src/openvpn/options.c:

http://gerrit.openvpn.net/c/openvpn/+/523/comment/2fba2993_384c818e :
PS1, Line 1861: SHOW_BOOL(ce.http_proxy_options->nocache);
> This belongs into show_connection_entry like the other o->ce options
Acknowledged


File src/openvpn/proxy.c:

http://gerrit.openvpn.net/c/openvpn/+/523/comment/0fc15362_dde1eff3 :
PS1, Line 275: if (!static_proxy_user_pass.defined || (is_first_time && 
!p->options.nocache) )
> This condition feels wrong to me. […]
Acknowledged


http://gerrit.openvpn.net/c/openvpn/+/523/comment/eda88547_5919fdf8 :
PS1, Line 278:   p->options.auth_file,
> I think you messed up a rebase on top of 
> a634cc5eccd55f1d14197da7376bb819bdf72cb6 here. […]
Acknowledged


http://gerrit.openvpn.net/c/openvpn/+/523/comment/607aa2a9_08aea895 :
PS1, Line 546: get_user_pass_http(p);
> So previously this forced a reset of the credentials. This you removed. […]
Acknowledged



--
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: 2
Gerrit-Owner: its_Giaan 
Gerrit-Reviewer: flichtenheld 
Gerrit-Reviewer: plaisthos 
Gerrit-CC: openvpn-devel 
Gerrit-Attention: plaisthos 
Gerrit-Attention: flichtenheld 
Gerrit-Comment-Date: Mon, 26 Feb 2024 18:29:07 +
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] [S] Change in openvpn[master]: Http-proxy: fix bug preventing proxy credentials caching

2024-02-26 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 (#2).


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

Http-proxy: fix bug preventing proxy credentials caching

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

This issue has been solved by getting the c->first_time
parameter 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 at
the restart of the context instance credentials would be erase.

The nocache option 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 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
6 files changed, 39 insertions(+), 2 deletions(-)


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

diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index 52b4308..dc1ee8d 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 2c79a1e..0d22df9 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -3123,6 +3123,10 @@
 if (ce->proto == PROTO_TCP)
 {
 ce->proto = PROTO_TCP_CLIENT;
+if (ce->http_proxy_options)
+{
+ce->http_proxy_options->nocache = ssl_get_auth_nocache();
+}
 }
 }

diff --git a/src/openvpn/proxy.c b/src/openvpn/proxy.c
index eeb3989..ff50539 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,6 +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
+ */
+else if (!static_proxy_user_pass.nocache)
+{
 p->queried_creds = true;
 p->up = static_proxy_user_pass;
 }
@@ -542,7 +552,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
@@ -553,6 +563,7 @@
 #endif

 p->defined = true;
+p->options.nocache = o->nocache;
 return p;
 }

@@ -656,6 +667,10 @@
 || 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? */
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 first 
iteration of the main loop */
+bool nocache;
 };

 struct http_proxy_options_simple {
diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index 33c8670..d174dad 100644
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -335,6 +335,15 @@
 }

 /*
+ * Get the password caching
+ */
+bool
+ssl_get_auth_nocache()
+{
+return passbuf.nocache;
+}
+
+/*
  * Set an authentication token
  */
 void
diff --git a/src/openvpn/ssl.h b/src/openvpn/ssl.h
index 71b99db..dd6538c 100644
--- a/src/openvpn/ssl.h
+++ b/src/openvpn/ssl.h
@@ -397,6 +397,11 @@
 void ssl_set_auth_nocache(void);

 /*
+ * Getter method for retrieving the auth-nocache option.
+ */
+bool