msg63101 - (view) |
Author: Brendan W. McAdams (bwmcadams) |
Date: 2008-02-28 18:26 |
URLLIB2 seems to have issues attempting to digest authenticate against a
Windows-based IIS 6.x server. The IIS server requests MD5-sess support,
and URLLIB2 throws an exception that ... far from explains this:
Traceback (most recent call last):
File "/usr/lib64/python2.5/runpy.py", line 95, in run_module
filename, loader, alter_sys)
File "/usr/lib64/python2.5/runpy.py", line 52, in _run_module_code
mod_name, mod_fname, mod_loader)
File "/usr/lib64/python2.5/runpy.py", line 32, in _run_code
exec code in run_globals
File "/srv/pubPal/test/test_auth.py", line 16, in <module>
print opener.open(uri)
File "/usr/lib64/python2.5/urllib2.py", line 380, in open
response = meth(req, response)
File "/usr/lib64/python2.5/urllib2.py", line 491, in http_response
'http', request, response, code, msg, hdrs)
File "/usr/lib64/python2.5/urllib2.py", line 412, in error
result = self._call_chain(*args)
File "/usr/lib64/python2.5/urllib2.py", line 353, in _call_chain
result = func(*args)
File "/usr/lib64/python2.5/urllib2.py", line 992, in http_error_401
host, req, headers)
File "/usr/lib64/python2.5/urllib2.py", line 884, in http_error_auth_reqed
return self.retry_http_digest_auth(req, authreq)
File "/usr/lib64/python2.5/urllib2.py", line 889, in
retry_http_digest_auth
auth = self.get_authorization(req, chal)
File "/usr/lib64/python2.5/urllib2.py", line 920, in get_authorization
H, KD = self.get_algorithm_impls(algorithm)
File "/usr/lib64/python2.5/urllib2.py", line 972, in get_algorithm_impls
return H, KD
UnboundLocalError: local variable 'H' referenced before assignment
The offending code is urllib2.py line 972:
def get_algorithm_impls(self, algorithm):
# lambdas assume digest modules are imported at the top level
if algorithm == 'MD5':
H = lambda x: hashlib.md5(x).hexdigest()
elif algorithm == 'SHA':
H = lambda x: hashlib.sha1(x).hexdigest()
# XXX MD5-sess
KD = lambda s, d: H("%s:%s" % (s, d))
return H, KD
I'm not sure what's meant by the # XXX MD5-sess comment there... But
what ends up happening is that MD5-sess matches neither the if or the
elif, and when return attempts to return, H is undefined.
This could be easily patched, but support for MD5-sess would be great as
well.
In my estimation, at the least, urllib2 shouldn't crap out in such an
obscure way when encountering an unsupported algorithm.
I'd suggest changing line 970 (The # XXX MD5-sess line) to:
raise Exception, "Unsupported Digest Authentication Algorithm '%s'" %
(algorithm)
|
msg63102 - (view) |
Author: Brendan W. McAdams (bwmcadams) |
Date: 2008-02-28 18:27 |
Sorry, this isn't a crash. It doesn't crash the interpreter. I'll
assume "behavior' is the correct categorization.
|
msg63108 - (view) |
Author: A.M. Kuchling (akuchling) *  |
Date: 2008-02-28 21:59 |
Marking as easy, assuming you add the exception; implementing MD5-sess
is probably not easy.
|
msg98115 - (view) |
Author: Manuel Muradás (dieresys) |
Date: 2010-01-21 21:53 |
Here is a patch for supporting MD5-sess, following RFC2617 specification.
Some comments/warnings:
* I've only tested the patch against IIS 6.0. I don't know about other servers supporting MD5-sess.
* IIS 6.0 expects the User Agent to send the URI (in the Authorization header) without the query string.
* This patch doesn't add support for Digest sessions. For each request we make, we get a new [401|407] message with a new nonce (depending if we're talking about a proxy with digest authentication or a web server). Then we generate another authenticated request using that nonce. For Digest sessions to be fully supported, we should be adding an [WWW|Proxy]-Authenticate header in each following request we made to the server using the last nonce. This includes both MD5-sess and MD5 digest implementations.
* A1 is being recalculated for every request. Given the above, this is not a real problem.
I'll open a new ticket regarding Digest sessions.
|
msg109874 - (view) |
Author: Mark Lawrence (BreamoreBoy) * |
Date: 2010-07-10 15:04 |
I tried to test the patch but got:-
c:\Python26>python.exe lib\test\test_urllib2_localnet.py
Traceback (most recent call last):
File "lib\test\test_urllib2_localnet.py", line 325, in <module>
class DigestMD5sessClientAuthTests(BaseTestCase):
NameError: name 'BaseTestCase' is not defined
|
msg133535 - (view) |
Author: Kristján Valur Jónsson (kristjan.jonsson) *  |
Date: 2011-04-11 16:15 |
Is this moving anywhere? I was asked to apply this patch locally and am curious as to whether it is moving into the stdlib.
|
msg133540 - (view) |
Author: Santoso Wijaya (santoso.wijaya) * |
Date: 2011-04-11 18:37 |
Adding some minor fixes to the original patch to apply cleanly to current 2.7 tip.
Also, add a conditional branch in the case that the server returns none of the supported algorithm strings, to raise a ValueError (the previous patch adds support for 'MD5-sess' but the original non-descript stack trace would be seen again with some other unsupported algorithm string).
|
msg138847 - (view) |
Author: Chris Withers (cjw296) *  |
Date: 2011-06-23 01:27 |
Just got bitten by this as well, what still needs to happen with the patch?
|
msg138848 - (view) |
Author: R. David Murray (r.david.murray) *  |
Date: 2011-06-23 01:44 |
One question is whether this is a bug fix or a feature request.
Other than that, I'd like to see the test classes collapsed into a single test class, considering that each one only has a single test in it. Probably ProxyAuthTests should be refactored so that the stuff that is currently in setUp is a method that gets called with appropriate parameters instead, and all the new tests moved on to ProxyAuthTests.
Also, a version of the patch for 3.x would be most helpful, since it won't port cleanly due to the renamings.
|
msg138849 - (view) |
Author: Senthil Kumaran (orsenthil) *  |
Date: 2011-06-23 02:11 |
Yes, it is a feature. Sorry that I have not paid attention to this. The Windows (IIS) part led me to delay as I did not have any to test. Let me take this up and see through it in 3.3.
|
msg138855 - (view) |
Author: Chris Withers (cjw296) *  |
Date: 2011-06-23 07:11 |
Hmm, I'd argue it's a bug:
File "/usr/lib64/python2.5/urllib2.py", line 972, in get_algorithm_impls
return H, KD
UnboundLocalError: local variable 'H' referenced before assignment
...does not say anything like:
"The digest authentication scheme you have requested is not supported"
Now, as to whether it's a bug that 'MD5-sess' isn't supported is a tougher call. The XXX indicates the intention was certainly for it to be supported...
|
msg138865 - (view) |
Author: R. David Murray (r.david.murray) *  |
Date: 2011-06-23 12:53 |
Oh, the bad error message is definitely a bug. The question is whether we can also add md5-sess support while fixing it. Sounds like Senthil thinks no, in which case this issue needs to be split into two parts.
|
msg138866 - (view) |
Author: Chris Withers (cjw296) *  |
Date: 2011-06-23 13:34 |
...which is, of course, rather disappointing.
When *would* md5-sess land? 2.7? 3.3?!
|
msg138869 - (view) |
Author: R. David Murray (r.david.murray) *  |
Date: 2011-06-23 15:46 |
3.3.
IMO this is in the grey area between feature and bug fix. I think it is possible to argue that it can be treated as a bug fix, but I think we need opinions from other developers if we want to try to go that route.
The reason I think it can be argued that it can be treated as a bug fix is that (if I understand correctly) there is no difference in the application code. The only difference is whether or not one can successfully communicate with IIS 6. That may not be a sufficient argument, but it is an argument :)
|
msg138992 - (view) |
Author: Chris Withers (cjw296) *  |
Date: 2011-06-24 21:49 |
Who with and where does the argument need to be had?
|
msg138995 - (view) |
Author: R. David Murray (r.david.murray) *  |
Date: 2011-06-24 22:13 |
Hmm. How about python-committers? I would suggest python-dev, but that feels like invoking the dragon :)
|
msg192499 - (view) |
Author: Christian Heimes (christian.heimes) *  |
Date: 2013-07-06 23:11 |
Python 3.3 has been released a while ago and is in bug fix mode.
Personally I'm against this enhancement. It adds a digest auth scheme that is based on a broken and insecure hashing algorithm.
|
msg220429 - (view) |
Author: Mathieu Dupuy (deronnax) * |
Date: 2014-06-13 10:44 |
Could we at least do something cleaner that let the interpreter raise an UnboundLocalError ? Maybe something like "NotImplemented" ?
|
msg220433 - (view) |
Author: R. David Murray (r.david.murray) *  |
Date: 2014-06-13 12:50 |
Sure. Would you like to propose a patch? It does seem that NotImplementedError would be the most appropriate. It could give Christian's reason why it is not implemented.
|
msg227540 - (view) |
Author: Mathieu Dupuy (deronnax) * |
Date: 2014-09-25 14:57 |
here is the patch, for the trunk
|
msg227541 - (view) |
Author: Mathieu Dupuy (deronnax) * |
Date: 2014-09-25 14:57 |
and here for the 2.7 branch
|
msg227884 - (view) |
Author: Mathieu Dupuy (deronnax) * |
Date: 2014-09-30 12:06 |
But I think md5-sess should really be integrated. It's a standard mechanism described by a RFC (https://www.ietf.org/rfc/rfc2617.txt), and people need it, however insecure it may be (aren't other method (md5) insecure too ?).
|
msg260423 - (view) |
Author: Mathieu Dupuy (deronnax) * |
Date: 2016-02-18 03:50 |
up
|
msg260437 - (view) |
Author: Mathieu Dupuy (deronnax) * |
Date: 2016-02-18 08:01 |
python 2.7
|
msg260438 - (view) |
Author: Mathieu Dupuy (deronnax) * |
Date: 2016-02-18 08:02 |
python current
|
msg260448 - (view) |
Author: Martin Panter (martin.panter) *  |
Date: 2016-02-18 10:57 |
I do wonder if NotImplementedError is the right exception. According to the documentation it is derived from RuntimeError and is meant for abstract methods, i.e. it is a programmer error. Other cases in urllib.request raise ValueError (e.g. AbstractBasicAuthHandler), which is fairly normal for protocol errors like this.
Also it would be nice to add a test case for the bug fix.
|
msg260971 - (view) |
Author: Mathieu Dupuy (deronnax) * |
Date: 2016-02-28 10:27 |
I can see in the tests (test_urllib2_localnet.py) that Digest auth is tested only through "ProxyAuthTests". Is that sufficient ? Isn't it a bug ? If no, should I add the test in that class ?
|
msg260972 - (view) |
Author: Mathieu Dupuy (deronnax) * |
Date: 2016-02-28 11:50 |
first draft
|
msg261163 - (view) |
Author: Mathieu Dupuy (deronnax) * |
Date: 2016-03-03 12:42 |
I'm waiting for reviews.
|
msg261252 - (view) |
Author: Roundup Robot (python-dev)  |
Date: 2016-03-06 14:17 |
New changeset 22eab50cb585 by Berker Peksag in branch '3.5':
Issue #2202: Fix UnboundLocalError in AbstractDigestAuthHandler.get_algorithm_impls
https://hg.python.org/cpython/rev/22eab50cb585
New changeset 046c75c9cd33 by Berker Peksag in branch 'default':
Issue #2202: Fix UnboundLocalError in AbstractDigestAuthHandler.get_algorithm_impls
https://hg.python.org/cpython/rev/046c75c9cd33
|
msg261253 - (view) |
Author: Roundup Robot (python-dev)  |
Date: 2016-03-06 14:27 |
New changeset d135799e952b by Berker Peksag in branch '2.7':
Issue #2202: Fix UnboundLocalError in AbstractDigestAuthHandler.get_algorithm_impls
https://hg.python.org/cpython/rev/d135799e952b
|
msg261254 - (view) |
Author: Berker Peksag (berker.peksag) *  |
Date: 2016-03-06 14:31 |
Thanks, Mathieu! I went with a simpler approach to test the code.
|
msg261356 - (view) |
Author: Mathieu Dupuy (deronnax) * |
Date: 2016-03-08 12:48 |
Much better indeed. Thanks.
|
|
Date |
User |
Action |
Args |
2022-04-11 14:56:31 | admin | set | github: 46455 |
2016-03-08 12:48:52 | deronnax | set | messages:
+ msg261356 |
2016-03-06 14:31:42 | berker.peksag | set | status: open -> closed
type: enhancement -> behavior
keywords:
- needs review nosy:
+ berker.peksag messages:
+ msg261254 resolution: fixed stage: patch review -> resolved |
2016-03-06 14:27:18 | python-dev | set | messages:
+ msg261253 |
2016-03-06 14:17:43 | python-dev | set | nosy:
+ python-dev messages:
+ msg261252
|
2016-03-03 12:42:27 | deronnax | set | messages:
+ msg261163 |
2016-02-28 11:50:25 | deronnax | set | files:
+ digest_md5sess_unittest.diff
messages:
+ msg260972 |
2016-02-28 10:27:32 | deronnax | set | messages:
+ msg260971 |
2016-02-18 10:57:47 | martin.panter | set | versions:
+ Python 2.7, Python 3.5, Python 3.6 nosy:
+ martin.panter
messages:
+ msg260448
stage: needs patch -> patch review |
2016-02-18 08:02:25 | deronnax | set | files:
+ md5-sess_not_implem_cur_v2.diff
messages:
+ msg260438 |
2016-02-18 08:01:53 | deronnax | set | files:
+ md5-sess_not_implem_27_v2.diff
messages:
+ msg260437 |
2016-02-18 03:50:51 | deronnax | set | messages:
+ msg260423 |
2014-12-31 16:19:29 | akuchling | set | nosy:
- akuchling
|
2014-09-30 12:06:15 | deronnax | set | messages:
+ msg227884 |
2014-09-25 14:57:54 | deronnax | set | files:
+ md5-sess_not_implem_27.diff
messages:
+ msg227541 |
2014-09-25 14:57:34 | deronnax | set | files:
+ md5-sess_not_implem_cur.diff
messages:
+ msg227540 versions:
- Python 3.4 |
2014-06-13 12:50:37 | r.david.murray | set | messages:
+ msg220433 stage: patch review -> needs patch |
2014-06-13 10:44:27 | deronnax | set | nosy:
+ deronnax messages:
+ msg220429
|
2014-02-03 17:06:23 | BreamoreBoy | set | nosy:
- BreamoreBoy
|
2013-07-06 23:11:37 | christian.heimes | set | versions:
+ Python 3.4, - Python 2.6, Python 3.1, Python 2.7, Python 3.2 nosy:
+ christian.heimes
messages:
+ msg192499
type: behavior -> enhancement resolution: accepted -> (no value) |
2013-04-18 05:08:29 | orsenthil | unlink | issue4140 dependencies |
2011-06-24 22:13:58 | r.david.murray | set | messages:
+ msg138995 |
2011-06-24 21:49:26 | cjw296 | set | messages:
+ msg138992 |
2011-06-23 15:46:40 | r.david.murray | set | messages:
+ msg138869 |
2011-06-23 13:34:49 | cjw296 | set | messages:
+ msg138866 |
2011-06-23 12:53:21 | r.david.murray | set | messages:
+ msg138865 |
2011-06-23 07:11:16 | cjw296 | set | messages:
+ msg138855 |
2011-06-23 02:11:07 | orsenthil | set | messages:
+ msg138849 |
2011-06-23 01:44:11 | r.david.murray | set | nosy:
+ r.david.murray messages:
+ msg138848
|
2011-06-23 01:27:18 | cjw296 | set | keywords:
- easy nosy:
+ cjw296 messages:
+ msg138847
|
2011-04-11 18:37:31 | santoso.wijaya | set | files:
+ issue2202_py27.patch nosy:
+ santoso.wijaya messages:
+ msg133540
|
2011-04-11 16:15:37 | kristjan.jonsson | set | nosy:
+ kristjan.jonsson messages:
+ msg133535
|
2010-07-11 05:20:39 | orsenthil | link | issue4140 dependencies |
2010-07-11 05:06:01 | orsenthil | set | assignee: georg.brandl -> orsenthil resolution: accepted |
2010-07-10 15:04:26 | BreamoreBoy | set | versions:
+ Python 3.1, Python 2.7, Python 3.2 nosy:
+ BreamoreBoy
messages:
+ msg109874
stage: test needed -> patch review |
2010-01-21 21:55:55 | brian.curtin | set | keywords:
+ needs review |
2010-01-21 21:53:55 | dieresys | set | files:
+ urllib2-support_md5-sess.diff
nosy:
+ dieresys messages:
+ msg98115
keywords:
+ patch |
2009-02-13 02:08:37 | ajaksu2 | set | nosy:
+ jjlee |
2009-02-12 17:37:27 | ajaksu2 | set | nosy:
+ orsenthil stage: test needed versions:
- Python 2.5 |
2008-03-20 04:28:00 | jafo | set | priority: normal assignee: georg.brandl nosy:
+ georg.brandl versions:
+ Python 2.6 |
2008-02-28 21:59:34 | akuchling | set | keywords:
+ easy nosy:
+ akuchling messages:
+ msg63108 |
2008-02-28 18:27:48 | bwmcadams | set | type: crash -> behavior messages:
+ msg63102 |
2008-02-28 18:26:33 | bwmcadams | create | |