Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Should Authorization header be cleared in https -> http redirect? #4716

Closed
bmerry opened this issue Jun 27, 2018 · 4 comments
Closed

Should Authorization header be cleared in https -> http redirect? #4716

bmerry opened this issue Jun 27, 2018 · 4 comments
Labels

Comments

@bmerry
Copy link
Contributor

bmerry commented Jun 27, 2018

This may be considered intentional behaviour (in which case feel free to close this), but if a request is made to an https endpoint with authorization and it redirects to http on the same host, the Authorization header is not stripped and will be exposed on the wire.

Expected Result

rebuild_auth would strip the Authorization header if the scheme is changed from https to http.

Actual Result

The credentials that were intended to be sent over TLS were transmitted in plaintext with the redirected request.

Reproduction Steps

Run an HTTPS server on localhost:4443 that replies with a 302 redirect to http://localhost:8000, and a plain HTTP server (or netcat) on localhost:8000. Then run

import requests
requests.get('https://localhost:4443', auth=('hello', 'world'), verify=False)

The basic auth credentials are sent in plaintext to http://localhost:8000 (the verify=False is just because I had a self-signed cert).

Here's the code I used for the SSL server:

import BaseHTTPServer
import ssl

class Handler(BaseHTTPServer.BaseHTTPRequestHandler):
    def do_GET(self):
        self.send_response(302)
        self.send_header('Location', 'http://localhost:8000/')
        self.end_headers()
        self.wfile.write('')

httpd = BaseHTTPServer.HTTPServer(('localhost', 4443), Handler)
httpd.socket = ssl.wrap_socket (httpd.socket, server_side=True,
                                certfile='yourpemfile.pem')
httpd.serve_forever()

System Information

{
  "chardet": {
    "version": "3.0.4"
  }, 
  "cryptography": {
    "version": "2.2.2"
  }, 
  "idna": {
    "version": "2.7"
  }, 
  "implementation": {
    "name": "CPython", 
    "version": "2.7.12"
  }, 
  "platform": {
    "release": "4.15.0-23-generic", 
    "system": "Linux"
  }, 
  "pyOpenSSL": {
    "openssl_version": "1010008f", 
    "version": "18.0.0"
  }, 
  "requests": {
    "version": "2.19.1"
  }, 
  "system_ssl": {
    "version": "1000207f"
  }, 
  "urllib3": {
    "version": "1.23"
  }, 
  "using_pyopenssl": true
}
@bmerry
Copy link
Contributor Author

bmerry commented Jun 27, 2018

From what I can tell by experiment, Firefox and Chromium treat http and https versions of a site as separate authentication realms, and don't automatically reuse basic credentials on either a redirect or manual browsing between the two. Two http URLs with the same host (localhost) and different port numbers also seem to be treated as independent realms.

@bmerry
Copy link
Contributor Author

bmerry commented Jun 27, 2018

Found something about it in RFC 7235, section 2.2.

A protection space is defined by the canonical root URI (the scheme and authority components of the effective request URI; see Section 5.5 of [RFC7230]) of the server being accessed, in combination with the realm value if present.

which suggests that both the scheme and port number should be considered (and in theory one should also check the realm, but that doesn't really fit into request's model that basic credentials are supplied unconditionally rather than in response to WWW-Authentication).

bmerry added a commit to bmerry/requests that referenced this issue Jun 28, 2018
Previously the header was stripped only if the hostname changed, but in
an https -> http redirect that can leak the credentials on the wire
(psf#4716). Based on with RFC 7235 section 2.2, the header is now stripped
if the "canonical root URL" (scheme+authority) has changed.
bmerry added a commit to bmerry/requests that referenced this issue Jun 29, 2018
Previously the header was stripped only if the hostname changed, but in
an https -> http redirect that can leak the credentials on the wire
(psf#4716). Based on with RFC 7235 section 2.2, the header is now stripped
if the "canonical root URL" (scheme+authority) has changed, by checking
scheme, hostname and port.
bmerry added a commit to bmerry/requests that referenced this issue Sep 5, 2018
Previously the header was stripped only if the hostname changed, but in
an https -> http redirect that can leak the credentials on the wire
(psf#4716). Based on with RFC 7235 section 2.2, the header is now stripped
if the "canonical root URL" (scheme+authority) has changed, by checking
scheme, hostname and port.
bmerry added a commit to bmerry/requests that referenced this issue Sep 14, 2018
Previously the header was stripped only if the hostname changed, but in
an https -> http redirect that can leak the credentials on the wire
(psf#4716). Based on with RFC 7235 section 2.2, the header is now stripped
if the "canonical root URL" (scheme+authority) has changed, by checking
scheme, hostname and port.
@thinkt4nk
Copy link

is this going to get a release?

@nateprewitt
Copy link
Member

Yes @thinkt4nk, we’re currently coordinating a release with the urllib3 team.

ianklatzco added a commit to ianklatzco/discord-sms-bridge that referenced this issue Oct 29, 2018
mattdrees added a commit to CruGlobal/jenkins-jobs that referenced this issue Oct 29, 2018
This is primarily to resolve a vulnerability: CVE-2018-18074 [1]
(It's not clear to me that an https-to-http redirect is very feasible,
so this seems like a pretty difficult vulnerability to harness,
but it's probably worth fixing anyway.)

Since it's difficult [2] to upgrade just individual parts of the lockfile,
and since it's not all that important to only upgrade `requests`,
this commit upgrades everything.
In particular, pyyaml is upgraded to 3.13 and jenkins-job-builder
is upgraded from 3e7ad9692655450fe26371770ec87a17e2a0b23a to
1940ed63e06949d4224d64e12afae437d9d0c089.

[1]: psf/requests#4716,
     https://github.com/CruGlobal/jenkins-jobs/network/alert/Pipfile.lock/requests/open

[2]: pypa/pipenv#966
     (Which is way too long of a thread, but the gist is
     pipenv now has a --selective-upgrade option but it doesn't work right)
llimllib added a commit to llimllib/limbo that referenced this issue Oct 30, 2018
There's a CVE in requests 2.19: psf/requests#4716
llimllib added a commit to llimllib/limbo that referenced this issue Nov 6, 2018
There's a CVE in requests 2.19: psf/requests#4716

closes #163
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 7, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

4 participants