Hi Selva, thank you very much for your feedback!
On Tue, Jan 31, 2017 at 03:02:33PM -0500, Selva Nair wrote:
> Hi,
>
> On Tue, Jan 31, 2017 at 1:22 PM, Antonio Quartulli <a...@unstable.cc> wrote:
>
> > iff --git a/src/openvpn/proxy.c b/src/openvpn/proxy.c
> > index b0ed3279..27f34bed 100644
> > --- a/src/openvpn/proxy.c
> > +++ b/src/openvpn/proxy.c
> > @@ -256,7 +256,16 @@ username_password_as_base64(const struct
> > http_proxy_info *p,
> > 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)
> > + {
> > + static_proxy_user_pass.defined = false;
> > + }
> >
>
> While unsetting the define attribute appears to have no unintended side
> effects, purge_user_pass(&static_proxy_user_pass, true) would be cleaner.
>
yeah, I Was thinking about that too, but I was not sure. I will re-check, it
might really be cleaner.
>
> > +
> > + if (!static_proxy_user_pass.defined)
> > {
> > unsigned int flags = GET_USER_PASS_MANAGEMENT;
> > if (p->queried_creds)
> >
>
> That said, there is one issue with this approach. Looks like SIGUSR1
> restarts will now always prompt for proxy password, which is not proper.
Right! Thanks for pointing this out!
> If
> so, a more nuanced fix that preserves the current behaviour for a single
> proxy credentials is required. May be, do the "purge proxy pass" in init.c
> only if the connection entry has http-proxy specified?
>
> I think the intent of the original code/man page is to have proxy password
> work the same way as auth-user-pass with an implicit auth-nocache assumed.
> Then multiple credentials is not expected, but could still work if the
> proxy password is purged when an authentication error happens instead of
> when the remote changes.
>
I am thinking that maybe I could just restructure the way the user/pass are
stored. Right now there is one static variable and I am trying to hack around it
to make it work with multiple http proxies....but why not just storing one
user/pass object per proxy? (like we currently do with the other proxy data)
That would probably be much cleaner, but will require a more intrusive change.
> Finally, socks credentials may also be affected.
I am not sure, because they use a different object/code-path. But I will
re-check that too
Thanks a lot!
Cheers,
--
Antonio Quartulli
signature.asc
Description: Digital signature
------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
