classification
Title: urllib2 sends Basic auth across redirects
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 2.6
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: orsenthil Nosy List: TFKyle, ajaksu2, dfischer, jjlee, kiilerix, orsenthil
Priority: normal Keywords: easy, patch

Created on 2008-09-09 15:12 by TFKyle, last changed 2010-05-19 23:15 by kiilerix. This issue is now closed.

Files
File name Uploaded Description Edit
test.py TFKyle, 2008-09-09 15:12 test: server-side part 1 (auth+redirect)
untest.py TFKyle, 2008-09-09 15:16 test: server-side part 2 (print env of redirected url)
bug3819.py TFKyle, 2008-09-09 15:19 test: client
siteminder_3819.py dfischer, 2010-02-10 03:43 illustration of the problem using siteminder
urllib2-3819.diff dfischer, 2010-02-11 00:00 possible fix for removing auth headers
Messages (11)
msg72871 - (view) Author: Kyle McFarland (TFKyle) Date: 2008-09-09 15:12
when you request a url that requests Basic authentication info
HTTPBasicAuthHandler adds the Authorization header to the request as a
normal (not unredirected) header, then if the server returns a 301 or
302 redirect HTTPRedirectHandler will send a request to the redirected
address keeping the normal headers including the Authorization header
HTTPBasicAuthHandler added, I'll attach the code I used to test this.

GET from libwww-perl seems to do this but most browsers don't seem to 
by default and although I can't find much in the RFCs about how
redirecting is supposed to work wrt. auth headers (feel free to point
out sections if I'm blind) I think it breaks
ftp://ftp.isi.edu/in-notes/rfc2617.txt somewhat (section 1.1, 
"""
The protection space determines the domain over which credentials can
be automatically applied. If a prior request has been authorized, the
same credentials MAY be reused for all other requests within that
protection space for a period of time determined by the
authentication scheme, parameters, and/or user preference. Unless
otherwise defined by the authentication scheme, a single protection
space cannot extend outside the scope of its server.
""") since redirects can point to arbitrary urls off of the server.

as in bug #1480067 just adding the header as an unredirected header
would stop the header being sent across redirects if that's indeed the
proper behaviour.
msg72893 - (view) Author: Senthil Kumaran (orsenthil) * (Python committer) Date: 2008-09-09 18:29
1) The section you refer to is 1.2 of RFC2617, which specifies the details on
Access Authentication in General and not specific to url redirects. So, I don't
think we should take it as a referece.

2) Under the section - 3.3 Digest Operation, the Authentication cases under
redirection is provided like this. (search for keyword 'redirect')

"""
The client will retry the request, at which time the server might respond with a 301/302 redirection, pointing to the URI on the second server. The client will follow the redirection, and pass an Authorization header , including the <opaque> data...
"""

This basically states that Authorization header should be passed on the
redirects in Digest authentication case and (should we assume in Basic
Authentication case also?) If yes, then urllib2 is actually doing the same
thing.  Do you have a practical scenario where this has resulted in failure/
security loophole?
msg73424 - (view) Author: Senthil Kumaran (orsenthil) * (Python committer) Date: 2008-09-19 10:59
This is working as designed and Requestor has not supplied any further
information on why he thinks it a bug.
msg76739 - (view) Author: John J Lee (jjlee) Date: 2008-12-02 13:45
I agree this is a bug.

Senthil -- re "1)", the paragraph you refer to (quoted by the OP) is
relevant.  The fact that it doesn't specifically mention redirection is
not relevant.

Re "2)": I don't know how digest auth works, but the paragraph you quote
from appears to be there for explanation rather than for prescribing how
digest auth or HTTP work (i.e. it appears to be "non-normative").  This
bug doesn't say that redirected requests shouldn't contain the
Authorization header.  It says that the Authorization header for an old
request shouldn't be sent with a new request (though it may turn out the
new one is equal to the old one in some cases).
msg81848 - (view) Author: Daniel Diniz (ajaksu2) (Python triager) Date: 2009-02-13 01:54
I think the test is close enough to acceptable, will adapt it if nobody
does first :)
msg99152 - (view) Author: David Fischer (dfischer) Date: 2010-02-10 03:43
I believe this bug affects urllib2 when it talks to the corporate single-sign-on solution Siteminder. Siteminder usually is installed as a web server module. When a request is made to the server (origin server), Siteminder issues a 302 redirect to a central authentication server running SSL passing the original request URL of the origin server. The central server responds with a 401 basic authentication challenge. Urllib2 responds with the password from the HTTPPasswordMgr. The central server sets some cookies and responds with a 302 redirect to the origin server on the original URL. Urllib2 then sends the authentication and cookies to the origin server which is virtually always unprotected. Browsers do not send the authentication to the origin server -- only the cookies.
msg99153 - (view) Author: Senthil Kumaran (orsenthil) * (Python committer) Date: 2010-02-10 03:54
Ok, in order to fix this bug, urllib2 should only send the cookies and not send the auth info across the the redirects. Yup, let me take this up.
msg99184 - (view) Author: David Fischer (dfischer) Date: 2010-02-11 00:00
I attached a diff of a fix for this bug. This may not be the ideal fix, but hopefully it will give the developer who actually does resolve it a good start.
msg100044 - (view) Author: Senthil Kumaran (orsenthil) * (Python committer) Date: 2010-02-24 16:47
The Basic Auth Authorization headers are added to unredirected_headers
and thus we will be able to prevent this problem.  ( Digest Authentication was already so, it gives additional confidence that is the right way).

Fixed and committed in revision 78422.
msg100045 - (view) Author: Senthil Kumaran (orsenthil) * (Python committer) Date: 2010-02-24 16:58
merged into other branches r78423, r78426, r78428
msg106116 - (view) Author: Mads Kiilerich (kiilerix) * Date: 2010-05-19 23:15
FYI, this change caused a regression in Mercurial - see http://mercurial.selenic.com/bts/issue2179.
History
Date User Action Args
2010-05-19 23:15:10kiilerixsetnosy: + kiilerix
messages: + msg106116
2010-02-24 16:58:32orsenthilsetstatus: open -> closed

messages: + msg100045
stage: needs patch -> resolved
2010-02-24 16:47:43orsenthilsetresolution: accepted -> fixed
messages: + msg100044
2010-02-11 00:00:15dfischersetfiles: + urllib2-3819.diff
keywords: + patch
messages: + msg99184
2010-02-10 03:54:52orsenthilsetassignee: orsenthil
resolution: accepted
messages: + msg99153
2010-02-10 03:43:29dfischersetfiles: + siteminder_3819.py
nosy: + dfischer
messages: + msg99152

2009-04-22 17:23:35ajaksu2setkeywords: + easy
2009-02-13 01:54:04ajaksu2setnosy: + ajaksu2
stage: needs patch
type: behavior
messages: + msg81848
versions: + Python 2.6
2008-12-02 13:45:59jjleesetnosy: + jjlee
messages: + msg76739
2008-09-19 10:59:12orsenthilsetmessages: + msg73424
2008-09-09 18:29:05orsenthilsetnosy: + orsenthil
messages: + msg72893
2008-09-09 15:19:29TFKylesetfiles: + bug3819.py
2008-09-09 15:16:57TFKylesetfiles: + untest.py
2008-09-09 15:12:43TFKylecreate