diff mbox series

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

Message ID 10eb2954e1a1f458c3f7b920ca2b395e0dccddbd-HTML@gerrit.openvpn.net
State Superseded
Headers show
Series [Openvpn-devel,M] Change in openvpn[master]: Http-proxy: Fix bug preventing proxy credentials caching. | expand

Commit Message

ordex (Code Review) Feb. 15, 2024, 10:01 a.m. UTC 
Attention is currently required from: flichtenheld, plaisthos.

Hello plaisthos, flichtenheld,

I'd like you to do a code review.
Please visit

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

to review the following change.


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_http(), which prevented the caching from working correctly.

This issue has been solved by rewriting the get_user_pass_http().
This method now sets the appropriate flags based on whether
credentials are defined, have been queried before, or are inline.
It then calls the get_user_pass() to retrieve the credentials
and store them in a static variable, 'static_proxy_user_pass',
which will be used for subsequent requests. If credentials
were not previously defined, or caching is not allowed, creds
are queried again.

Fixes: Trac #1187
Change-Id: Ia3e06c0832c4ca0ab868c845279fb71c01a1a78a
Signed-off-by: Gianmarco De Gregori <gianmarco@mandelbit.com>
---
M src/openvpn/options.c
M src/openvpn/proxy.c
M src/openvpn/proxy.h
3 files changed, 31 insertions(+), 27 deletions(-)



  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/23/523/1
diff mbox series

Patch

diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 2c79a1e..3f5301f 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -1858,6 +1858,7 @@ 
     SHOW_BOOL(persist_local_ip);
     SHOW_BOOL(persist_remote_ip);
     SHOW_BOOL(persist_key);
+    SHOW_BOOL(ce.http_proxy_options->nocache);
 
 #if PASSTOS_CAPABILITY
     SHOW_BOOL(passtos);
@@ -8978,6 +8979,7 @@ 
     {
         VERIFY_PERMISSION(OPT_P_GENERAL);
         ssl_set_auth_nocache();
+        options->ce.http_proxy_options->nocache = true;
     }
     else if (streq(p[0], "auth-token") && p[1] && !p[2])
     {
diff --git a/src/openvpn/proxy.c b/src/openvpn/proxy.c
index eeb3989..a4af720 100644
--- a/src/openvpn/proxy.c
+++ b/src/openvpn/proxy.c
@@ -257,40 +257,41 @@ 
 }
 
 static void
-get_user_pass_http(struct http_proxy_info *p, const bool force)
+get_user_pass_http(struct http_proxy_info *p)
 {
-    /*
-     * 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 bool is_first_time = true;
+    unsigned int flags = GET_USER_PASS_MANAGEMENT;
+
+    if (p->queried_creds && !p->options.nocache)
     {
-        clear_user_pass_http();
+        flags |= GET_USER_PASS_PREVIOUS_CREDS_FAILED;
     }
 
-    if (!static_proxy_user_pass.defined)
+    if (p->options.inline_creds)
     {
-        unsigned int flags = GET_USER_PASS_MANAGEMENT;
-        const char *auth_file = p->options.auth_file;
-        if (p->options.auth_file_up)
-        {
-            auth_file = p->options.auth_file_up;
-        }
-        if (p->queried_creds)
-        {
-            flags |= GET_USER_PASS_PREVIOUS_CREDS_FAILED;
-        }
-        if (p->options.inline_creds)
-        {
-            flags |= GET_USER_PASS_INLINE_CREDS;
-        }
+        flags |= GET_USER_PASS_INLINE_CREDS;
+    }
+
+    if (!static_proxy_user_pass.defined || (is_first_time && !p->options.nocache) )
+    {
         get_user_pass(&static_proxy_user_pass,
-                      auth_file,
+                      p->options.auth_file,
                       UP_TYPE_PROXY,
                       flags);
-        p->queried_creds = true;
-        p->up = static_proxy_user_pass;
+        is_first_time = false;
     }
+
+    else
+    {
+        get_user_pass(&static_proxy_user_pass,
+                      p->options.auth_file,
+                      UP_TYPE_PROXY,
+                      flags);
+        static_proxy_user_pass.defined = !p->options.nocache;
+    }
+
+    p->queried_creds = true;
+    p->up = static_proxy_user_pass;
 }
 
 #if 0
@@ -542,7 +543,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);
     }
 
 #if !NTLM
@@ -655,7 +656,7 @@ 
         || p->auth_method == HTTP_AUTH_DIGEST
         || p->auth_method == HTTP_AUTH_NTLM2)
     {
-        get_user_pass_http(p, false);
+        get_user_pass_http(p);
     }
 
     /* 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..b8b4ca9 100644
--- a/src/openvpn/proxy.h
+++ b/src/openvpn/proxy.h
@@ -57,6 +57,7 @@ 
     const char *user_agent;
     struct http_custom_header custom_headers[MAX_CUSTOM_HTTP_HEADER];
     bool inline_creds; /* auth_file_up is inline credentials */
+    bool nocache;
 };
 
 struct http_proxy_options_simple {