Merge pull request #10857 from rouault/http_redirect_bearer

/vsicurl/: no longer forward Authorization header when doing redirection to other hosts.
OSGeo · Sep 30, 2024 · aef927b · aef927b
2 parents 092b1b3 + 58cc577
commit aef927b
Show file tree

Hide file tree

Showing 7 changed files with 323 additions and 28 deletions.
diff --git a/autotest/gcore/vsicurl.py b/autotest/gcore/vsicurl.py
@@ -233,11 +233,204 @@ def server():
     webserver.server_stop(process, port)
 
 
+###############################################################################
+# Test regular redirection
+
+
+@pytest.mark.parametrize(
+    "authorization_header_allowed", [None, "YES", "NO", "IF_SAME_HOST"]
+)
+def test_vsicurl_test_redirect(server, authorization_header_allowed):
+
+    gdal.VSICurlClearCache()
+
+    expected_headers = None
+    unexpected_headers = []
+    if authorization_header_allowed != "NO":
+        expected_headers = {"Authorization": "Bearer xxx"}
+    else:
+        unexpected_headers = ["Authorization"]
+
+    handler = webserver.SequentialHandler()
+    handler.add("GET", "/test_redirect/", 404)
+    handler.add(
+        "HEAD",
+        "/test_redirect/test.bin",
+        301,
+        {"Location": "http://localhost:%d/redirected/test.bin" % server.port},
+        expected_headers={"Authorization": "Bearer xxx"},
+    )
+
+    # Curl always forward Authorization if same server when handling itself
+    # the redirect, so this means that CPL_VSIL_CURL_AUTHORIZATION_HEADER_ALLOWED_IF_REDIRECT=NO
+    # is not honored for that particular request. To honour it, we would have
+    # to disable CURLOPT_FOLLOWLOCATION and implement it at hand
+    handler.add(
+        "HEAD",
+        "/redirected/test.bin",
+        200,
+        {"Content-Length": "3"},
+        expected_headers={"Authorization": "Bearer xxx"},
+    )
+
+    handler.add(
+        "GET",
+        "/redirected/test.bin",
+        200,
+        {"Content-Length": "3"},
+        b"xyz",
+        expected_headers=expected_headers,
+        unexpected_headers=unexpected_headers,
+    )
+
+    options = {"GDAL_HTTP_HEADERS": "Authorization: Bearer xxx"}
+    if authorization_header_allowed:
+        options[
+            "CPL_VSIL_CURL_AUTHORIZATION_HEADER_ALLOWED_IF_REDIRECT"
+        ] = authorization_header_allowed
+    with webserver.install_http_handler(handler), gdal.config_options(options):
+        f = gdal.VSIFOpenL(
+            "/vsicurl/http://localhost:%d/test_redirect/test.bin" % server.port,
+            "rb",
+        )
+        assert f is not None
+        try:
+            assert gdal.VSIFReadL(1, 3, f) == b"xyz"
+        finally:
+            gdal.VSIFCloseL(f)
+
+
+###############################################################################
+# Test regular redirection
+
+
+@pytest.mark.parametrize(
+    "authorization_header_allowed", [None, "YES", "NO", "IF_SAME_HOST"]
+)
+def test_vsicurl_test_redirect_different_server(server, authorization_header_allowed):
+
+    gdal.VSICurlClearCache()
+
+    expected_headers = None
+    unexpected_headers = []
+    if authorization_header_allowed == "YES":
+        expected_headers = {"Authorization": "Bearer xxx"}
+    else:
+        unexpected_headers = ["Authorization"]
+
+    handler = webserver.SequentialHandler()
+    handler.add("GET", "/test_redirect/", 404)
+    handler.add(
+        "HEAD",
+        "/test_redirect/test.bin",
+        301,
+        {"Location": "http://127.0.0.1:%d/redirected/test.bin" % server.port},
+        expected_headers={"Authorization": "Bearer xxx"},
+    )
+    handler.add(
+        "HEAD",
+        "/redirected/test.bin",
+        200,
+        {"Content-Length": "3"},
+        expected_headers=expected_headers,
+        unexpected_headers=unexpected_headers,
+    )
+    handler.add(
+        "GET",
+        "/redirected/test.bin",
+        200,
+        {"Content-Length": "3"},
+        b"xyz",
+        expected_headers=expected_headers,
+        unexpected_headers=unexpected_headers,
+    )
+
+    options = {"GDAL_HTTP_HEADERS": "Authorization: Bearer xxx"}
+    if authorization_header_allowed:
+        options[
+            "CPL_VSIL_CURL_AUTHORIZATION_HEADER_ALLOWED_IF_REDIRECT"
+        ] = authorization_header_allowed
+    with webserver.install_http_handler(handler), gdal.config_options(options):
+        f = gdal.VSIFOpenL(
+            "/vsicurl/http://localhost:%d/test_redirect/test.bin" % server.port,
+            "rb",
+        )
+        try:
+            assert gdal.VSIFReadL(1, 3, f) == b"xyz"
+        finally:
+            gdal.VSIFCloseL(f)
+
+
+###############################################################################
+# Test regular redirection
+
+
+@gdaltest.enable_exceptions()
+@pytest.mark.require_curl(7, 61, 0)
+@pytest.mark.parametrize(
+    "authorization_header_allowed", [None, "YES", "NO", "IF_SAME_HOST"]
+)
+def test_vsicurl_test_redirect_different_server_with_bearer(
+    server, authorization_header_allowed
+):
+
+    gdal.VSICurlClearCache()
+
+    expected_headers = None
+    unexpected_headers = []
+    if authorization_header_allowed == "YES":
+        expected_headers = {"Authorization": "Bearer xxx"}
+    else:
+        unexpected_headers = ["Authorization"]
+
+    handler = webserver.SequentialHandler()
+    handler.add("GET", "/test_redirect/", 404)
+    handler.add(
+        "HEAD",
+        "/test_redirect/test.bin",
+        301,
+        {"Location": "http://127.0.0.1:%d/redirected/test.bin" % server.port},
+        expected_headers={"Authorization": "Bearer xxx"},
+    )
+    handler.add(
+        "HEAD",
+        "/redirected/test.bin",
+        200,
+        {"Content-Length": "3"},
+        expected_headers=expected_headers,
+        unexpected_headers=unexpected_headers,
+    )
+    handler.add(
+        "GET",
+        "/redirected/test.bin",
+        200,
+        {"Content-Length": "3"},
+        b"xyz",
+        expected_headers=expected_headers,
+        unexpected_headers=unexpected_headers,
+    )
+
+    options = {"GDAL_HTTP_AUTH": "BEARER", "GDAL_HTTP_BEARER": "xxx"}
+    if authorization_header_allowed:
+        options[
+            "CPL_VSIL_CURL_AUTHORIZATION_HEADER_ALLOWED_IF_REDIRECT"
+        ] = authorization_header_allowed
+    with webserver.install_http_handler(handler), gdal.config_options(options):
+        f = gdal.VSIFOpenL(
+            "/vsicurl/http://localhost:%d/test_redirect/test.bin" % server.port,
+            "rb",
+        )
+        try:
+            assert gdal.VSIFReadL(1, 3, f) == b"xyz"
+        finally:
+            gdal.VSIFCloseL(f)
+
+
 ###############################################################################
 # Test redirection with Expires= type of signed URLs
 
 
-def test_vsicurl_test_redirect(server):
+def test_vsicurl_test_redirect_with_expires(server):
 
     gdal.VSICurlClearCache()
 

diff --git a/doc/source/spelling_wordlist.txt b/doc/source/spelling_wordlist.txt
@@ -2733,6 +2733,7 @@ RecordBatch
 RecordBatchAsNumpy
 recurse
 recursionLevel
+redirections
 Redistributable
 reentrancy
 reentrant

diff --git a/doc/source/user/configoptions.rst b/doc/source/user/configoptions.rst
@@ -626,6 +626,20 @@ Networking options
       Try to query quietly redirected URLs to Amazon S3 signed URLs during their
       validity period, so as to minimize round-trips.
 
+-  .. config:: CPL_VSIL_CURL_AUTHORIZATION_HEADER_ALLOWED_IF_REDIRECT
+      :choices: YES, NO, IF_SAME_HOST
+      :default: IF_SAME_HOST
+      :since: 3.10
+
+      Determines if the HTTP ``Authorization`` header must be forwarded when
+      redirections are followed:
+
+      - ``NO`` to always disable forwarding of Authorization header
+      - ``YES`` to always enable forwarding of Authorization header (was the
+        default value prior to GDAL 3.10)
+      - ``IF_SAME_HOST`` to enable forwarding of Authorization header only if
+        the redirection is to the same host.
+
 -  .. config:: CPL_VSIL_USE_TEMP_FILE_FOR_RANDOM_WRITE
       :choices: YES, NO
 

diff --git a/doc/source/user/virtual_file_systems.rst b/doc/source/user/virtual_file_systems.rst
@@ -413,6 +413,11 @@ As an alternative, starting with GDAL 3.6, the
 :config:`GDAL_HTTP_HEADERS` configuration option can also be
 used to specify headers. :config:`CPL_CURL_VERBOSE=YES` allows one to see them and more, when combined with ``--debug``.
 
+Starting with GDAL 3.10, the ``Authorization`` header is no longer automatically
+forwarded when redirections are followed.
+That behavior can be configured by setting the
+:config:`CPL_VSIL_CURL_AUTHORIZATION_HEADER_ALLOWED_IF_REDIRECT` configuration option.
+
 Starting with GDAL 2.3, the :config:`GDAL_HTTP_MAX_RETRY` (number of attempts) and :config:`GDAL_HTTP_RETRY_DELAY` (in seconds) configuration option can be set, so that request retries are done in case of HTTP errors 429, 502, 503 or 504.
 
 Starting with GDAL 3.6, the following configuration options control the TCP keep-alive functionality (cf https://daniel.haxx.se/blog/2020/02/10/curl-ootw-keepalive-time/ for a detailed explanation):

diff --git a/port/cpl_http.cpp b/port/cpl_http.cpp
@@ -2237,14 +2237,22 @@ void *CPLHTTPSetOptions(void *pcurl, const char *pszURL,
                                    CURLAUTH_ANYSAFE);
     else if (EQUAL(pszHttpAuth, "BEARER"))
     {
-        const char *pszBearer = CSLFetchNameValue(papszOptions, "HTTP_BEARER");
-        if (pszBearer == nullptr)
-            pszBearer = CPLGetConfigOption("GDAL_HTTP_BEARER", nullptr);
-        if (pszBearer != nullptr)
-            unchecked_curl_easy_setopt(http_handle, CURLOPT_XOAUTH2_BEARER,
-                                       pszBearer);
-        unchecked_curl_easy_setopt(http_handle, CURLOPT_HTTPAUTH,
-                                   CURLAUTH_BEARER);
+        const char *pszAuthorizationHeaderAllowed = CSLFetchNameValueDef(
+            papszOptions, "AUTHORIZATION_HEADER_ALLOWED", "YES");
+        const bool bAuthorizationHeaderAllowed =
+            CPLTestBool(pszAuthorizationHeaderAllowed);
+        if (bAuthorizationHeaderAllowed)
+        {
+            const char *pszBearer =
+                CSLFetchNameValue(papszOptions, "HTTP_BEARER");
+            if (pszBearer == nullptr)
+                pszBearer = CPLGetConfigOption("GDAL_HTTP_BEARER", nullptr);
+            if (pszBearer != nullptr)
+                unchecked_curl_easy_setopt(http_handle, CURLOPT_XOAUTH2_BEARER,
+                                           pszBearer);
+            unchecked_curl_easy_setopt(http_handle, CURLOPT_HTTPAUTH,
+                                       CURLAUTH_BEARER);
+        }
     }
     else if (EQUAL(pszHttpAuth, "NEGOTIATE"))
         unchecked_curl_easy_setopt(http_handle, CURLOPT_HTTPAUTH,
@@ -2365,6 +2373,15 @@ void *CPLHTTPSetOptions(void *pcurl, const char *pszURL,
                                1L);
 
     unchecked_curl_easy_setopt(http_handle, CURLOPT_FOLLOWLOCATION, 1);
+    const char *pszUnrestrictedAuth = CPLGetConfigOption(
+        "CPL_VSIL_CURL_AUTHORIZATION_HEADER_ALLOWED_IF_REDIRECT",
+        "IF_SAME_HOST");
+    if (!EQUAL(pszUnrestrictedAuth, "IF_SAME_HOST") &&
+        CPLTestBool(pszUnrestrictedAuth))
+    {
+        unchecked_curl_easy_setopt(http_handle, CURLOPT_UNRESTRICTED_AUTH, 1);
+    }
+
     unchecked_curl_easy_setopt(http_handle, CURLOPT_MAXREDIRS, 10);
     unchecked_curl_easy_setopt(http_handle, CURLOPT_POSTREDIR,
                                CURL_REDIR_POST_ALL);
@@ -2664,6 +2681,11 @@ void *CPLHTTPSetOptions(void *pcurl, const char *pszURL,
         }
         if (!bHeadersDone)
         {
+            const char *pszAuthorizationHeaderAllowed = CSLFetchNameValueDef(
+                papszOptions, "AUTHORIZATION_HEADER_ALLOWED", "YES");
+            const bool bAuthorizationHeaderAllowed =
+                CPLTestBool(pszAuthorizationHeaderAllowed);
+
             // We accept both raw headers with \r\n as a separator, or as
             // a comma separated list of foo: bar values.
             const CPLStringList aosTokens(
@@ -2672,7 +2694,11 @@ void *CPLHTTPSetOptions(void *pcurl, const char *pszURL,
                     : CSLTokenizeString2(pszHeaders, ",", CSLT_HONOURSTRINGS));
             for (int i = 0; i < aosTokens.size(); ++i)
             {
-                headers = curl_slist_append(headers, aosTokens[i]);
+                if (bAuthorizationHeaderAllowed ||
+                    !STARTS_WITH_CI(aosTokens[i], "Authorization:"))
+                {
+                    headers = curl_slist_append(headers, aosTokens[i]);
+                }
             }
         }
     }