Blob Blame History Raw
From e6968d1d220891230bcca5340bfd364183ceaa31 Mon Sep 17 00:00:00 2001
From: Daniel Stenberg <daniel@haxx.se>
Date: Fri, 19 Jan 2018 13:19:25 +0100
Subject: [PATCH] http: prevent custom Authorization headers in redirects

... unless CURLOPT_UNRESTRICTED_AUTH is set to allow them. This matches how
curl already handles Authorization headers created internally.

Note: this changes behavior slightly, for the sake of reducing mistakes.

Added test 317 and 318 to verify.

Reported-by: Craig de Stigter
Bug: https://curl.haxx.se/docs/adv_2018-b3bf.html

Upstream-commit: af32cd3859336ab963591ca0df9b1e33a7ee066b
Signed-off-by: Kamil Dudka <kdudka@redhat.com>
---
 docs/libcurl/opts/CURLOPT_HTTPHEADER.3 | 12 ++++-
 lib/http.c                             | 10 +++-
 lib/url.c                              |  2 +-
 lib/urldata.h                          |  2 +-
 tests/data/Makefile.inc                |  2 +-
 tests/data/test317                     | 94 +++++++++++++++++++++++++++++++++
 tests/data/test318                     | 95 ++++++++++++++++++++++++++++++++++
 7 files changed, 212 insertions(+), 5 deletions(-)
 create mode 100644 tests/data/test317
 create mode 100644 tests/data/test318

diff --git a/docs/libcurl/opts/CURLOPT_HTTPHEADER.3 b/docs/libcurl/opts/CURLOPT_HTTPHEADER.3
index 6aeec22..781e570 100644
--- a/docs/libcurl/opts/CURLOPT_HTTPHEADER.3
+++ b/docs/libcurl/opts/CURLOPT_HTTPHEADER.3
@@ -5,7 +5,7 @@
 .\" *                            | (__| |_| |  _ <| |___
 .\" *                             \___|\___/|_| \_\_____|
 .\" *
-.\" * Copyright (C) 1998 - 2015, Daniel Stenberg, <daniel@haxx.se>, et al.
+.\" * Copyright (C) 1998 - 2018, Daniel Stenberg, <daniel@haxx.se>, et al.
 .\" *
 .\" * This software is licensed as described in the file COPYING, which
 .\" * you should have received as part of this distribution. The terms
@@ -77,6 +77,16 @@ the headers. They may be private or otherwise sensitive to leak.
 
 Use \fICURLOPT_HEADEROPT(3)\fP to make the headers only get sent to where you
 intend them to get sent.
+
+Custom headers are sent in all requests done by the easy handles, which
+implies that if you tell libcurl to follow redirects
+(\fICURLOPT_FOLLOWLOCATION(3)\fP), the same set of custom headers will be sent
+in the subsequent request. Redirects can of course go to other hosts and thus
+those servers will get all the contents of your custom headers too.
+
+Starting in 7.58.0, libcurl will specifically prevent "Authorization:" headers
+from being sent to other hosts than the first used one, unless specifically
+permitted with the \fICURLOPT_UNRESTRICTED_AUTH(3)\fP option.
 .SH DEFAULT
 NULL
 .SH PROTOCOLS
diff --git a/lib/http.c b/lib/http.c
index b73e58c..c15208d 100644
--- a/lib/http.c
+++ b/lib/http.c
@@ -726,7 +726,7 @@ Curl_http_output_auth(struct connectdata *conn,
   if(!data->state.this_is_a_follow ||
      conn->bits.netrc ||
      !data->state.first_host ||
-     data->set.http_disable_hostname_check_before_authentication ||
+     data->set.allow_auth_to_other_hosts ||
      strcasecompare(data->state.first_host, conn->host.name)) {
     result = output_auth_headers(conn, authhost, request, path, FALSE);
   }
@@ -1637,6 +1637,14 @@ CURLcode Curl_add_custom_headers(struct connectdata *conn,
                   checkprefix("Transfer-Encoding:", headers->data))
             /* HTTP/2 doesn't support chunked requests */
             ;
+          else if(checkprefix("Authorization:", headers->data) &&
+                  /* be careful of sending this potentially sensitive header to
+                     other hosts */
+                  (data->state.this_is_a_follow &&
+                   data->state.first_host &&
+                   !data->set.allow_auth_to_other_hosts &&
+                   !strcasecompare(data->state.first_host, conn->host.name)))
+            ;
           else {
             CURLcode result = Curl_add_bufferf(req_buffer, "%s\r\n",
                                                headers->data);
diff --git a/lib/url.c b/lib/url.c
index 71d4d8b..ba53131 100644
--- a/lib/url.c
+++ b/lib/url.c
@@ -985,7 +985,7 @@ CURLcode Curl_setopt(struct Curl_easy *data, CURLoption option,
      * Send authentication (user+password) when following locations, even when
      * hostname changed.
      */
-    data->set.http_disable_hostname_check_before_authentication =
+    data->set.allow_auth_to_other_hosts =
       (0 != va_arg(param, long)) ? TRUE : FALSE;
     break;
 
diff --git a/lib/urldata.h b/lib/urldata.h
index b4f18e7..1dd62ae 100644
--- a/lib/urldata.h
+++ b/lib/urldata.h
@@ -1682,7 +1682,7 @@ struct UserDefined {
   bool http_keep_sending_on_error; /* for HTTP status codes >= 300 */
   bool http_follow_location; /* follow HTTP redirects */
   bool http_transfer_encoding; /* request compressed HTTP transfer-encoding */
-  bool http_disable_hostname_check_before_authentication;
+  bool allow_auth_to_other_hosts;
   bool include_header;   /* include received protocol headers in data output */
   bool http_set_referer; /* is a custom referer used */
   bool http_auto_referer; /* set "correct" referer when following location: */
diff --git a/tests/data/Makefile.inc b/tests/data/Makefile.inc
index 6e2f402..870d0da 100644
--- a/tests/data/Makefile.inc
+++ b/tests/data/Makefile.inc
@@ -55,7 +55,7 @@ test280 test281 test282 test283 test284 test285 test286 test287 test288 \
 test289 test290 test291 test292 test293 test294 test295 test296 test297 \
 test298 test299 test300 test301 test302 test303 test304 test305 test306 \
 test307 test308 test309 test310 test311 test312 test313                 \
-                                test320 test321 test322 test323 test324 \
+        test317 test318         test320 test321 test322 test323 test324 \
 test325 \
 test350 test351 test352 test353 test354 \
 \
diff --git a/tests/data/test317 b/tests/data/test317
new file mode 100644
index 0000000..c6d8697
--- /dev/null
+++ b/tests/data/test317
@@ -0,0 +1,94 @@
+<testcase>
+<info>
+<keywords>
+HTTP
+HTTP proxy
+HTTP Basic auth
+HTTP proxy Basic auth
+followlocation
+</keywords>
+</info>
+#
+# Server-side
+<reply>
+<data>
+HTTP/1.1 302 OK
+Date: Thu, 09 Nov 2010 14:49:00 GMT
+Server: test-server/fake swsclose
+Content-Type: text/html
+Funny-head: yesyes
+Location: http://goto.second.host.now/3170002
+Content-Length: 8
+Connection: close
+
+contents
+</data>
+<data2>
+HTTP/1.1 200 OK
+Date: Thu, 09 Nov 2010 14:49:00 GMT
+Server: test-server/fake swsclose
+Content-Type: text/html
+Funny-head: yesyes
+Content-Length: 9
+
+contents
+</data2>
+
+<datacheck>
+HTTP/1.1 302 OK
+Date: Thu, 09 Nov 2010 14:49:00 GMT
+Server: test-server/fake swsclose
+Content-Type: text/html
+Funny-head: yesyes
+Location: http://goto.second.host.now/3170002
+Content-Length: 8
+Connection: close
+
+HTTP/1.1 200 OK
+Date: Thu, 09 Nov 2010 14:49:00 GMT
+Server: test-server/fake swsclose
+Content-Type: text/html
+Funny-head: yesyes
+Content-Length: 9
+
+contents
+</datacheck>
+</reply>
+
+#
+# Client-side
+<client>
+<server>
+http
+</server>
+ <name>
+HTTP with custom Authorization: and redirect to new host
+ </name>
+ <command>
+http://first.host.it.is/we/want/that/page/317 -x %HOSTIP:%HTTPPORT -H "Authorization: s3cr3t" --proxy-user testing:this --location
+</command>
+</client>
+
+#
+# Verify data after the test has been "shot"
+<verify>
+<strip>
+^User-Agent:.*
+</strip>
+<protocol>
+GET http://first.host.it.is/we/want/that/page/317 HTTP/1.1
+Host: first.host.it.is
+Proxy-Authorization: Basic dGVzdGluZzp0aGlz
+Accept: */*
+Proxy-Connection: Keep-Alive
+Authorization: s3cr3t
+
+GET http://goto.second.host.now/3170002 HTTP/1.1
+Host: goto.second.host.now
+Proxy-Authorization: Basic dGVzdGluZzp0aGlz
+Accept: */*
+Proxy-Connection: Keep-Alive
+
+</protocol>
+</verify>
+</testcase>
diff --git a/tests/data/test318 b/tests/data/test318
new file mode 100644
index 0000000..838d1ba
--- /dev/null
+++ b/tests/data/test318
@@ -0,0 +1,95 @@
+<testcase>
+<info>
+<keywords>
+HTTP
+HTTP proxy
+HTTP Basic auth
+HTTP proxy Basic auth
+followlocation
+</keywords>
+</info>
+#
+# Server-side
+<reply>
+<data>
+HTTP/1.1 302 OK
+Date: Thu, 09 Nov 2010 14:49:00 GMT
+Server: test-server/fake swsclose
+Content-Type: text/html
+Funny-head: yesyes
+Location: http://goto.second.host.now/3180002
+Content-Length: 8
+Connection: close
+
+contents
+</data>
+<data2>
+HTTP/1.1 200 OK
+Date: Thu, 09 Nov 2010 14:49:00 GMT
+Server: test-server/fake swsclose
+Content-Type: text/html
+Funny-head: yesyes
+Content-Length: 9
+
+contents
+</data2>
+
+<datacheck>
+HTTP/1.1 302 OK
+Date: Thu, 09 Nov 2010 14:49:00 GMT
+Server: test-server/fake swsclose
+Content-Type: text/html
+Funny-head: yesyes
+Location: http://goto.second.host.now/3180002
+Content-Length: 8
+Connection: close
+
+HTTP/1.1 200 OK
+Date: Thu, 09 Nov 2010 14:49:00 GMT
+Server: test-server/fake swsclose
+Content-Type: text/html
+Funny-head: yesyes
+Content-Length: 9
+
+contents
+</datacheck>
+</reply>
+
+#
+# Client-side
+<client>
+<server>
+http
+</server>
+ <name>
+HTTP with custom Authorization: and redirect to new host
+ </name>
+ <command>
+http://first.host.it.is/we/want/that/page/318 -x %HOSTIP:%HTTPPORT -H "Authorization: s3cr3t" --proxy-user testing:this --location-trusted
+</command>
+</client>
+
+#
+# Verify data after the test has been "shot"
+<verify>
+<strip>
+^User-Agent:.*
+</strip>
+<protocol>
+GET http://first.host.it.is/we/want/that/page/318 HTTP/1.1
+Host: first.host.it.is
+Proxy-Authorization: Basic dGVzdGluZzp0aGlz
+Accept: */*
+Proxy-Connection: Keep-Alive
+Authorization: s3cr3t
+
+GET http://goto.second.host.now/3180002 HTTP/1.1
+Host: goto.second.host.now
+Proxy-Authorization: Basic dGVzdGluZzp0aGlz
+Accept: */*
+Proxy-Connection: Keep-Alive
+Authorization: s3cr3t
+
+</protocol>
+</verify>
+</testcase>
-- 
2.13.6