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

14.2.0's decoding auth_digest parameters breaks authentication if "uri" contains encoded slashes #1716

Closed
1 task done
teoric opened this issue Jun 12, 2018 · 12 comments
Closed
1 task done
Labels
bug regression Something that worked earlier got broken in new release reproducer: present This PR or issue contains code, which reproduce the problem described or clearly understandable STR

Comments

@teoric
Copy link
Contributor

teoric commented Jun 12, 2018

  • I'm submitting a ...
  • bug report
  • What is the current behavior?

Starting with 14.2, parameters are decoded in cherrypy/lib/auth_digest.py:HttpDigestAuthorization's constructor.

One of the parameters is uri. If uricontains e.g. a slash, I get an authentication loop.

  • If the current behavior is a bug, please provide the steps to reproduce and if possible a screenshots and logs of the problem. If you can, show us your code.
  • start up application
  • enter an URL, e.g. testhost:8080/test_page?from=%2F
  • enjoy the loop
  • What is the expected behavior?
  • serving the page
  • Please tell us about your environment:
  • Cheroot version: 6.3.1
  • CherryPy version: 14.2.0 or 15.0.0
  • Python version: 3.6 and 3.5
  • OS: Mac OSX and Linux
  • Browser: at_least([Chrome | Firefox])

(PR to follow.)

@teoric teoric changed the title 14.2.0's decoding auth_digest parameters breaks if "uri" contains e.g. slashes 14.2.0's decoding auth_digest parameters breaks authentication if "uri" contains encoded slashes Jun 14, 2018
@webknjaz
Copy link
Member

cc @jaraco

@webknjaz webknjaz added bug reproducer: missing This PR or issue leaks code, which reproduce the problem described or clearly understandable STR labels Jun 15, 2018
@jaraco
Copy link
Member

jaraco commented Jun 17, 2018

I don't yet understand this bug nor why the PR addresses it.

Why is URI special? How is it that the redirect loop is occurring? How is it that not unquoting the uri parameter prevents that?

@teoric
Copy link
Contributor Author

teoric commented Jun 17, 2018

I did not dig too deeply in the internals, but decoding the uri with unquote_to_bytes will undo hexadecimal escaping. As far as I can see, URI is the only parameter that has hex escapes (at least in my setup). Now assume that uri contains a slash in a parameter (/); by unquoting the slash is is now part of the uri. (from https://testhost:8080/test_page?from=%2F to https://testhost:8080/test_page?from=/) I suppose this confuses some part of the program (routing?), which may rely on having only literal slashes that separate URI components, and only escaped slashes in the URI components?

Another question is: Why is decoding for URI needed at this stage now? If I understand correctly, these lines should take care of parameters in non-ASCII; but will the URI ever be non-ASCII?? If not, leaving it alone is the better path anyway.

Sorry for the guessing, but I can tell you that this was the only way I found to get authentication working again after dda3364.

@jaraco
Copy link
Member

jaraco commented Jun 17, 2018

I believe you have identified a proper regression... a regression that might have been prevented if our code base had a proper test for this apparently basic expectation.

In order to create a proper test case to capture this expectation, can you put together a minimal reproducer of the issue you're encountering, basically a cherrypy script that sets up a server with the digest auth configured and some requests (scripts or instructions) that trigger the unwanted behavior?

@teoric
Copy link
Contributor Author

teoric commented Jun 17, 2018

The following fails with 14.2+, but works with 14.1.0 if one uses http://127.0.0.1:8080/index?from=%2F. I hope this meets your standards. Just in case this is not clear: For authentication, please use no as user and body as password.

#!/usr/bin/env python3.5
# -*- coding: utf-8 -*-
import cherrypy

get_ha1 = cherrypy.lib.auth_digest.get_ha1_dict_plain({"no": "body"})

auth_conf = {
        'tools.auth_digest.on': True,
        'tools.auth_digest.realm': "realm",
        'tools.auth_digest.get_ha1': get_ha1,
        'tools.auth_digest.key': "key",
}


class HelloWorld(object):
    @cherrypy.expose
    def index(self, **params):
        return "Hello world!"


if __name__ == '__main__':
    cherrypy.config.update(auth_conf)
    cherrypy.quickstart(HelloWorld())

@webknjaz
Copy link
Member

webknjaz commented Jun 17, 2018

So I've tried it out and it looks like it returns HTTP 401 for /?from=%2F, while it returns 200 for just /:

$ curl -Iv localhost:8080/?from=%2F
*   Trying ::1...
* TCP_NODELAY set
* connect to ::1 port 8080 failed: Connection refused
*   Trying 127.0.0.1...
* TCP_NODELAY set
* Connected to localhost (127.0.0.1) port 8080 (#0)
> HEAD /?from=%2F HTTP/1.1
> Host: localhost:8080
> User-Agent: curl/7.60.0
> Accept: */*
>
< HTTP/1.1 401 Unauthorized
HTTP/1.1 401 Unauthorized
< Content-Type: text/html;charset=utf-8
Content-Type: text/html;charset=utf-8
< Server: CherryPy/15.0.0
Server: CherryPy/15.0.0
< Date: Sun, 17 Jun 2018 21:18:30 GMT
Date: Sun, 17 Jun 2018 21:18:30 GMT
< Www-Authenticate: Digest realm="realm", nonce="1529270310:7db002813ceae6961519be2df6d96d35", algorithm="MD5", qop="auth", charset="UTF-8"
Www-Authenticate: Digest realm="realm", nonce="1529270310:7db002813ceae6961519be2df6d96d35", algorithm="MD5", qop="auth", charset="UTF-8"
< Content-Length: 1754
Content-Length: 1754

<
* Connection #0 to host localhost left intact

$ curl -v -u no:body --digest localhost:8080/?from=%2F
*   Trying ::1...
* TCP_NODELAY set
* connect to ::1 port 8080 failed: Connection refused
*   Trying 127.0.0.1...
* TCP_NODELAY set
* Connected to localhost (127.0.0.1) port 8080 (#0)
* Server auth using Digest with user 'no'
> GET /?from=%2F HTTP/1.1
> Host: localhost:8080
> User-Agent: curl/7.60.0
> Accept: */*
>
< HTTP/1.1 401 Unauthorized
< Content-Type: text/html;charset=utf-8
< Server: CherryPy/15.0.0
< Date: Sun, 17 Jun 2018 21:22:15 GMT
< Www-Authenticate: Digest realm="realm", nonce="1529270535:619be778ba5ea5db6665a431d0de4f5a", algorithm="MD5", qop="auth", charset="UTF
-8"
< Content-Length: 1754
<
* Ignoring the response-body
* Connection #0 to host localhost left intact
* Issue another request to this URL: 'http://localhost:8080/?from=%2F'
* Found bundle for host localhost: 0x55c643423940 [can pipeline]
* Re-using existing connection! (#0) with host localhost
* Connected to localhost (127.0.0.1) port 8080 (#0)
* Server auth using Digest with user 'no'
> GET /?from=%2F HTTP/1.1
> Host: localhost:8080
> Authorization: Digest username="no", realm="realm", nonce="1529270535:619be778ba5ea5db6665a431d0de4f5a", uri="/?from=%2F", cnonce="MjE
wNDkzYWFlNjQ2N2M5ODU2ZDFlMTY4N2M1Y2FhNjI=", nc=00000001, qop=auth, response="5990fcf841ae6e59d154c45a8266e4bb", algorithm="MD5"
> User-Agent: curl/7.60.0
> Accept: */*
>
< HTTP/1.1 401 Unauthorized
< Content-Type: text/html;charset=utf-8
< Server: CherryPy/15.0.0
< Date: Sun, 17 Jun 2018 21:22:15 GMT
* Authentication problem. Ignoring this.
< Www-Authenticate: Digest realm="realm", nonce="1529270535:619be778ba5ea5db6665a431d0de4f5a", algorithm="MD5", qop="auth", charset="UTF
-8"
< Content-Length: 1754
<
<!DOCTYPE html PUBLIC
"-//W3C//DTD XHTML 1.0 Transitional//EN"
"http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<html>
<head>
    <meta http-equiv="Content-Type" content="text/html; charset=utf-8"></meta>
    <title>401 Unauthorized</title>
    <style type="text/css">
    #powered_by {
        margin-top: 20px;
        border-top: 2px solid black;
        font-style: italic;
    }

    #traceback {
        color: red;
    }
    </style>
</head>
    <body>
        <h2>401 Unauthorized</h2>
        <p>You are not authorized to access that resource</p>
        <pre id="traceback">Traceback (most recent call last):
  File "~wk/.pyenv/versions/3.6.5/lib/python3.6/site-packages/cherrypy/_cprequest.py", line 631, in respond
    self._do_respond(path_info)
  File "~wk/.pyenv/versions/3.6.5/lib/python3.6/site-packages/cherrypy/_cprequest.py", line 687, in _do_respond
    self.hooks.run('before_handler')
  File "~wk/.pyenv/versions/3.6.5/lib/python3.6/site-packages/cherrypy/_cprequest.py", line 114, in run
    raise exc
  File "~wk/.pyenv/versions/3.6.5/lib/python3.6/site-packages/cherrypy/_cprequest.py", line 104, in run
    hook()
  File "~wk/.pyenv/versions/3.6.5/lib/python3.6/site-packages/cherrypy/_cprequest.py", line 63, in __call__
    return self.callback(**self.kwargs)
  File "~wk/.pyenv/versions/3.6.5/lib/python3.6/site-packages/cherrypy/lib/auth_digest.py", line 448, in digest_auth
    401, 'You are not authorized to access that resource')
cherrypy._cperror.HTTPError: (401, 'You are not authorized to access that resource')
</pre>
    <div id="powered_by">
      <span>
        Powered by <a href="http://www.cherrypy.org">CherryPy 15.0.0</a>
      </span>
    </div>
    </body>
</html>
* Connection #0 to host localhost left intact

Which effectively means that this "redirect loop", doesn't actually contain "redirect" part. It's browser re-asking credentials.

So the issue is somewhere in answer do question "why does it raise 401?"

@teoric
Copy link
Contributor Author

teoric commented Jun 17, 2018

Thank you very much for your work and for confirming that this is an authentication loop.

@webknjaz
Copy link
Member

So there's where it fails:

> ~wk/src/github/cherrypy/cherrypy/cherrypy/lib/auth_digest.py(444)digest_auth()
    443     import ipdb; ipdb.set_trace()
--> 444     if digest != auth.response:
    445         respond_401()

for /, digest equals auth.response, for /?from=%2F it doesn't. for /?from=%2 digest also matches.
so it looks really related to percent-decoding somehow.

@webknjaz
Copy link
Member

so for /?from=%2F the client sends an uri param in Authorization header, which is uri="/?from=%2F", see full header below:

Authorization: Digest username="no", realm="realm", nonce="1529271425:9b4412e925a92d62e5dba489e76bde69", uri="/?from=%2F", cnonce="YjNlNWUyNGE2OWFlMmY3ZGVlMjZiOGExYTQxZmM4ZDY=", nc=00000001, qop=auth, response="107dc93cd855fa3eb46109867024330d", algorithm="MD5"

But during inspection I've found out that the auth.uri contains urldecoded version of it:

ipdb> auth.response
'107dc93cd855fa3eb46109867024330d'
ipdb> auth.uri
'/?from=/'

Which likely affects a digest calculation

@webknjaz
Copy link
Member

@jaraco It's reproducible with /?%40 as well, which leads me to conclusion that @teoric was right about the place where it got broken and perhaps I just processed params in header in a wrong way.

I think we misunderstood @teoric's explanation thinking that it's about Request-Line parsing, while in fact it's solely related to processing Authorization header.

Fun fact: when trying it from Google Chrome it didn't automatically encode uri parts, so /?@ or /?x=/ would work well.

@webknjaz webknjaz added reproducer: present This PR or issue contains code, which reproduce the problem described or clearly understandable STR regression Something that worked earlier got broken in new release and removed reproducer: missing This PR or issue leaks code, which reproduce the problem described or clearly understandable STR labels Jun 17, 2018
@teoric
Copy link
Contributor Author

teoric commented Jun 18, 2018

Thank you very much for figuring out a solution and sorry if I was unclear; I do not know much about CherryPy internals.

@webknjaz
Copy link
Member

It's alright that you don't know internals. You helped us a lot :)

jaraco added a commit that referenced this issue Jun 18, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug regression Something that worked earlier got broken in new release reproducer: present This PR or issue contains code, which reproduce the problem described or clearly understandable STR
Projects
None yet
Development

No branches or pull requests

3 participants