| Message ID | 20171204044907.32261-1-a@unstable.cc |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [Openvpn-devel,v4] reload HTTP proxy credentials when moving to the next connection profile | expand |
Hi, On 04-12-17 05:49, Antonio Quartulli wrote: > The HTTP proxy credentials are stored in a static variable that is > possibly initialized before each connection attempt. > > However, the variable is never "released" therefore get_user_pass() > refuses to overwrite its content and leaves it as it is. > Consequently, if the user config contains multiple connection profiles > with different http-proxy, each having its own credentials, only the > first user/pass couple is loaded and the others are all ignored. > This leads to connection failures because the proper credentials are > not associated with the right proxy server. > > The root of the misbehaviour seems to be located in the fact that, > despite the argument force passed to get_user_pass_http() being true, > no action is taken to release the static object containing the > credentials. > > Fix the misbehaviour by releasing the http-proxy credential object > when the reload is "forced". > > Trac: #836 > Signed-off-by: Antonio Quartulli <a@unstable.cc> > --- > > v4: > - move clear_user_pass_http() above its invocation to prevent compile error > > v3: > - call clear_user_pass_http() directly to clear user/pass object > > v2: > - rebased on current master > > > src/openvpn/proxy.c | 22 ++++++++++++++++------ > 1 file changed, 16 insertions(+), 6 deletions(-) > > diff --git a/src/openvpn/proxy.c b/src/openvpn/proxy.c > index fdc73b4a..de0188a9 100644 > --- a/src/openvpn/proxy.c > +++ b/src/openvpn/proxy.c > @@ -252,10 +252,25 @@ username_password_as_base64(const struct http_proxy_info *p, > return (const char *)make_base64_string((const uint8_t *)BSTR(&out), gc); > } > > +static void > +clear_user_pass_http(void) > +{ > + purge_user_pass(&static_proxy_user_pass, true); > +} > + > static void > get_user_pass_http(struct http_proxy_info *p, const bool force) > { > - if (!static_proxy_user_pass.defined || force) > + /* > + * in case of forced (re)load, make sure the static storage is set as > + * undefined, otherwise get_user_pass() won't try to load any credential > + */ > + if (force) > + { > + clear_user_pass_http(); > + } > + > + if (!static_proxy_user_pass.defined) > { > unsigned int flags = GET_USER_PASS_MANAGEMENT; > if (p->queried_creds) > @@ -274,11 +289,6 @@ get_user_pass_http(struct http_proxy_info *p, const bool force) > p->up = static_proxy_user_pass; > } > } > -static void > -clear_user_pass_http(void) > -{ > - purge_user_pass(&static_proxy_user_pass, true); > -} > > #if 0 > /* function only used in #if 0 debug statement */ > Only "compile-tested" (no test proxies around), but change makes total sense, code is clear and I trust Antonio to have properly tested the behaviour. Acked-by: Steffan Karger <steffan@karger.me> -Steffan ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512
ACK from me as well. Code looks sane and have tested this against
a quickly patched tinyproxy [1] with basic auth support. The client
switched between three different connection blocks, all depending on
which tinyproxy instance I brought up or took down.
[1] <https://github.com/dsommers/tinyproxy
Your patch has been applied to the following branches
commit 86b58ceb29cf1cc3acf32e2ff370d9a4af68c051 (master)
commit fbc50963b9f8a4bac4362d472366730fed23aae9 (release/2.4)
Author: Antonio Quartulli
Date: Mon Dec 4 12:49:07 2017 +0800
reload HTTP proxy credentials when moving to the next connection profile
Trac: #836
Signed-off-by: Antonio Quartulli <a@unstable.cc>
Acked-by: Steffan Karger <steffan@karger.me>
Acked-by: David Sommerseth <davids@openvpn.net>
Tested-by: David Sommerseth <davids@openvpn.net>
Message-Id: <20171204044907.32261-1-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg16007.html
Signed-off-by: David Sommerseth <davids@openvpn.net>
- --
kind regards,
David Sommerseth
OpenVPN Inc.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (GNU/Linux)
iQIcBAEBCgAGBQJaKFx8AAoJEIbPlEyWcf3yxJIQAL6ABxeacMYBJuFamF8gEoL+
W0a+grWzL7wNQ7n05pc/2UuOgbpliyGerhBx85VCFPTdYN6JBuVUEEDj67CptVL9
N3zSCbzVxeX21oaLfyoeYVypTopOBHB1rxBhwdC8MBdLBNPhZQ2Qy1r7XWLDX1mH
4vY1SP/X25NJcU71dmDN8zCimEPns/ANTjMf0r8lyQAdvMuSZE5sCRCutwTRNh/S
REAcXHtgFRfmNsmDRRCJvTvm+XiPNNY15EH/NmmHFquZAJuj2GkpGeBYa59q9l3D
tHcPzwes2xc1Te4kbca1tXAx6XCt/6e7FwENCQHCrwC0/NRSdmOFg89BiIZVnodh
L9NbVte8f+jFuV8DuBDqHIQdodry0JKD4YnC+QQhTJChGWwexyimLuxKMNANVEmb
6KRJvaUKzOyrsh8ct6ZMG35AgXM6MKQqMwmP+VB/SJrh6uCOZkxxxQMY5/vXL+hp
Zlzxs9WS/p2MbO9igHcMGXPGBEuzsp4KFKyMBAjrlpCFCFJFJfPPThi2eaiPSkXt
StFkSCLu1CnKifbB7KiLL2YORy3QyITDVshUbaBVZRl7g1Jlr+4UzG48NR9RIWE0
59XGJARegLPtxevO/DyArPOU+Ls5HQfN8YNCIf/dj41GuuYx4AdiKD1KSkClQHRK
83xeFyq2p7wWcIilIXXY
=NQs1
-----END PGP SIGNATURE-----
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
diff --git a/src/openvpn/proxy.c b/src/openvpn/proxy.c index fdc73b4a..de0188a9 100644 --- a/src/openvpn/proxy.c +++ b/src/openvpn/proxy.c @@ -252,10 +252,25 @@ username_password_as_base64(const struct http_proxy_info *p, return (const char *)make_base64_string((const uint8_t *)BSTR(&out), gc); } +static void +clear_user_pass_http(void) +{ + purge_user_pass(&static_proxy_user_pass, true); +} + static void get_user_pass_http(struct http_proxy_info *p, const bool force) { - if (!static_proxy_user_pass.defined || force) + /* + * in case of forced (re)load, make sure the static storage is set as + * undefined, otherwise get_user_pass() won't try to load any credential + */ + if (force) + { + clear_user_pass_http(); + } + + if (!static_proxy_user_pass.defined) { unsigned int flags = GET_USER_PASS_MANAGEMENT; if (p->queried_creds) @@ -274,11 +289,6 @@ get_user_pass_http(struct http_proxy_info *p, const bool force) p->up = static_proxy_user_pass; } } -static void -clear_user_pass_http(void) -{ - purge_user_pass(&static_proxy_user_pass, true); -} #if 0 /* function only used in #if 0 debug statement */
The HTTP proxy credentials are stored in a static variable that is possibly initialized before each connection attempt. However, the variable is never "released" therefore get_user_pass() refuses to overwrite its content and leaves it as it is. Consequently, if the user config contains multiple connection profiles with different http-proxy, each having its own credentials, only the first user/pass couple is loaded and the others are all ignored. This leads to connection failures because the proper credentials are not associated with the right proxy server. The root of the misbehaviour seems to be located in the fact that, despite the argument force passed to get_user_pass_http() being true, no action is taken to release the static object containing the credentials. Fix the misbehaviour by releasing the http-proxy credential object when the reload is "forced". Trac: #836 Signed-off-by: Antonio Quartulli <a@unstable.cc> --- v4: - move clear_user_pass_http() above its invocation to prevent compile error v3: - call clear_user_pass_http() directly to clear user/pass object v2: - rebased on current master src/openvpn/proxy.c | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-)