Rev 2436: Handle nonce changes. Fix a nasty bug breaking the auth parameters sharing. in http://bazaar.launchpad.net/~bzr/bzr/bzr.http.auth
Vincent Ladeuil
v.ladeuil+lp at free.fr
Sun Apr 22 17:32:07 BST 2007
At http://bazaar.launchpad.net/~bzr/bzr/bzr.http.auth
------------------------------------------------------------
revno: 2436
revision-id: v.ladeuil+lp at free.fr-20070422163204-7iksk91jy9091nex
parent: v.ladeuil+lp at free.fr-20070422110259-dx0e5do9dzzf0qjt
committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
branch nick: bzr.http.auth
timestamp: Sun 2007-04-22 18:32:04 +0200
message:
Handle nonce changes. Fix a nasty bug breaking the auth parameters sharing.
* bzrlib/tests/HTTPTestUtil.py:
(DigestAuthServer.digest_authorized): Check the nonce validity.
* bzrlib/transport/http/_urllib.py:
(HttpTransport_urllib._perform): Always update auth parameters
after a request processing, they could change at any moment.
* bzrlib/transport/http/_urllib2_wrappers.py:
(AbstractAuthHandler.update_auth): New method to track auth
modifications that should be lead to new authentications.
(AbstractAuthHandler.auth_required): Do not copy the auth
parameters but track the changes instead (copying breaks the auth
parameters sharing between cloned transports). Do not restrict the
auth 'identity' to be (user, realm, password) only.
(AbstractAuthHandler.auth_match): Document the auth parameter
update policy that MUST be respected to avoid retrying to
authenticate with wrong parameters, endlessly.
(AbstractAuthHandler.auth_successful): Not used anymore, but kept
in place if we ever want to handle the 'Authorization-Info' header.
modified:
NEWS NEWS-20050323055033-4e00b5db738777ff
bzrlib/tests/HTTPTestUtil.py HTTPTestUtil.py-20050914180604-247d3aafb7a43343
bzrlib/tests/test_http.py testhttp.py-20051018020158-b2eef6e867c514d9
bzrlib/transport/http/_urllib.py _urlgrabber.py-20060113083826-0bbf7d992fbf090c
bzrlib/transport/http/_urllib2_wrappers.py _urllib2_wrappers.py-20060913231729-ha9ugi48ktx481ao-1
-------------- next part --------------
=== modified file 'NEWS'
--- a/NEWS 2007-04-21 21:31:32 +0000
+++ b/NEWS 2007-04-22 16:32:04 +0000
@@ -24,7 +24,8 @@
* Tags are now included in logs, that use the long log formatter.
(Erik B??gfors, Alexander Belchenko)
- * digest authentication is now supported for proxy and http.
+ * digest authentication is now supported for proxy and
+ http. Tested against Apache 2.0.55 and Squid 2.6.5.
(Vincent Ladeuil).
INTERNALS:
=== modified file 'bzrlib/tests/HTTPTestUtil.py'
--- a/bzrlib/tests/HTTPTestUtil.py 2007-04-21 21:31:32 +0000
+++ b/bzrlib/tests/HTTPTestUtil.py 2007-04-22 16:32:04 +0000
@@ -424,12 +424,15 @@
class DigestAuthServer(AuthServer):
"""A digest authentication server"""
- auth_nonce = 'rRQ+Lp4uBAA=301b77beb156b6158b73dee026b8be23302292b4'
+ auth_nonce = 'now!'
def __init__(self, request_handler, auth_scheme):
AuthServer.__init__(self, request_handler, auth_scheme)
def digest_authorized(self, auth, command):
+ nonce = auth['nonce']
+ if nonce != self.auth_nonce:
+ return False
realm = auth['realm']
if realm != self.auth_realm:
return False
@@ -453,7 +456,6 @@
H = lambda x: md5.new(x).hexdigest()
KD = lambda secret, data: H("%s:%s" % (secret, data))
- nonce = auth['nonce']
nonce_count = int(auth['nc'], 16)
ncvalue = '%08x' % nonce_count
=== modified file 'bzrlib/tests/test_http.py'
--- a/bzrlib/tests/test_http.py 2007-04-22 11:02:59 +0000
+++ b/bzrlib/tests/test_http.py 2007-04-22 16:32:04 +0000
@@ -1204,7 +1204,7 @@
self.server.add_user('joe', 'foo')
t = self.get_user_transport('joe', 'bar')
self.assertRaises(errors.InvalidHttpResponse, t.get, 'a')
- # Two 'Authentication Required' error should occur (the
+ # Two 'Authentication Required' errors should occur (the
# initial 'who are you' and 'this is not you, who are you')
self.assertEqual(2, self.server.auth_required_errors)
@@ -1304,10 +1304,10 @@
# Only one 'Authentication Required' error should have
# occured so far
self.assertEqual(1, self.server.auth_required_errors)
- # So far, so good, let's have fun now
- self.server.auth_nonce = self.server.auth_nonce + 'tagada'
+ # The server invalidates the current nonce
+ self.server.auth_nonce = self.server.auth_nonce + '. No, now!'
self.assertEqual('contents of a\n', t.get('a').read())
- # Two 'Authentication Required' error should occur (the
+ # Two 'Authentication Required' errors should occur (the
# initial 'who are you' and a second 'who are you' with the new nonce)
self.assertEqual(2, self.server.auth_required_errors)
=== modified file 'bzrlib/transport/http/_urllib.py'
--- a/bzrlib/transport/http/_urllib.py 2007-04-21 20:39:06 +0000
+++ b/bzrlib/transport/http/_urllib.py 2007-04-22 16:32:04 +0000
@@ -90,15 +90,14 @@
if self._debuglevel > 0:
print 'perform: %s base: %s, url: %s' % (request.method, self.base,
request.get_full_url())
-
response = self._opener.open(request)
if self._connection is None:
# Acquire connection when the first request is able
# to connect to the server
self._connection = request.connection
- # And get auth parameters too
- self._auth = request.auth
- self._proxy_auth = request.proxy_auth
+ # Always get auth parameters, they may change
+ self._auth = request.auth
+ self._proxy_auth = request.proxy_auth
code = response.code
if request.follow_redirections is False \
=== modified file 'bzrlib/transport/http/_urllib2_wrappers.py'
--- a/bzrlib/transport/http/_urllib2_wrappers.py 2007-04-22 10:01:32 +0000
+++ b/bzrlib/transport/http/_urllib2_wrappers.py 2007-04-22 16:32:04 +0000
@@ -753,8 +753,32 @@
This can be used for http and proxy, as well as for basic and
digest authentications.
- This provides an unified interface for all auth handlers
+ This provides an unified interface for all authentication handlers
(urllib2 provides far too many with different policies).
+
+ The interaction between this handler and the urllib2
+ framework is not obvious, it works as follow:
+
+ opener.open(request) is called:
+
+ - that may trigger http_request which will add an authentication header
+ (self.build_header) if enough info is available.
+
+ - the request is sent to the server,
+
+ - if an authentication error is received self.auth_required is called,
+ we acquire the authentication info in the error headers and call
+ self.auth_match to check that we are able to try the
+ authentication and complete the authentication parameters,
+
+ - we call parent.open(request), that may trigger http_request
+ and will add a header (self.build_header), but here we have
+ all the required info (keep in mind that the request and
+ authentication used in the recursive calls are really (and must be)
+ the *same* objects).
+
+ - if the call returns a response, the authentication have been
+ successful and the request authentication parameters have been updated.
"""
# The following attributes should be defined by daughter
@@ -767,6 +791,13 @@
self.find_user_password = password_manager.find_user_password
self.add_password = password_manager.add_password
+ def update_auth(self, auth, key, value):
+ """Update a value in auth marking the auth as modified if needed"""
+ old_value = auth.get(key, None)
+ if old_value != value:
+ auth[key] = value
+ auth['modified'] = True
+
def auth_required(self, request, headers):
"""Retry the request if the auth scheme is ours.
@@ -780,29 +811,23 @@
# header. This must never happen in production code.
raise KeyError('%s not found' % self.auth_required_header)
- auth = self.get_auth(request).copy()
+ auth = self.get_auth(request)
if auth.get('user', None) is None:
# Without a known user, we can't authenticate
return None
+ auth['modified'] = False
if self.auth_match(server_header, auth):
# auth_match may have modified auth (by adding the
# password or changing the realm, for example)
- old = self.get_auth(request)
if request.get_header(self.auth_header, None) is not None \
- and old.get('user') == auth.get('user') \
- and old.get('realm') == auth.get('realm') \
- and old.get('password') == auth.get('password'):
+ and not auth['modified']:
# We already tried that, give up
return None
- # We will try to authenticate, save the auth so that
- # the build_auth_header that will be called during
- # parent.open use the right values
- self.set_auth(request, auth)
response = self.parent.open(request)
if response:
- self.auth_successful(request, response, auth)
+ self.auth_successful(request, response)
return response
# We are not qualified to handle the authentication.
# Note: the authentication error handling will try all
@@ -821,7 +846,14 @@
"""Check that we are able to handle that authentication scheme.
The request authentication parameters may need to be
- updated with info from the server.
+ updated with info from the server. Some of these
+ parameters, when combined, are considered to be the
+ authentication key, if one of them change the
+ authentication result may change. 'user' and 'password'
+ are exampls, but some auth schemes may have others
+ (digest's nonce is an example, digest's nonce_count is a
+ *counter-example*). Such parameters must be updated by
+ using the update_auth() method.
:param header: The authentication header sent by the server.
:param auth: The auth parameters already known. They may be
@@ -840,22 +872,18 @@
"""
raise NotImplementedError(self.build_auth_header)
- def auth_successful(self, request, response, auth):
+ def auth_successful(self, request, response):
"""The authentification was successful for the request.
- The params are stored in the request to allow reuse and
- avoid rountrips associated with authentication errors.
+ Additional infos may be available in the response.
:param request: The succesfully authenticated request.
:param response: The server response (may contain auth info).
- :param auth: The parameters used to succeed.
"""
- self.set_auth(request, auth)
+ pass
def get_password(self, user, authuri, realm=None):
- """ASk user for a password if none is already available.
-
- """
+ """Ask user for a password if none is already available."""
user_found, password = self.find_user_password(realm, authuri)
if user_found != user:
# FIXME: write a test for that case
@@ -908,12 +936,12 @@
return False
# Put useful info into auth
- auth['scheme'] = scheme
- auth['realm'] = realm
+ self.update_auth(auth, 'scheme', scheme)
+ self.update_auth(auth, 'realm', realm)
if auth.get('password',None) is None:
- auth['password'] = self.get_password(auth['user'],
- auth['authuri'],
- auth['realm'])
+ password = self.get_password(auth['user'], auth['authuri'],
+ auth['realm'])
+ self.update_auth(auth, 'password', password)
return match is not None
def auth_params_reusable(self, auth):
@@ -964,12 +992,6 @@
if qop != 'auth': # No auth-int so far
return False
- nonce = req_auth.get('nonce', None)
- old_nonce = auth.get('nonce', None)
- if nonce and old_nonce and nonce == old_nonce:
- # We already tried that
- return False
-
H, KD = get_digest_algorithm_impls(req_auth.get('algorithm', 'MD5'))
if H is None:
return False
@@ -981,12 +1003,16 @@
realm)
# Put useful info into auth
try:
- auth['scheme'] = scheme
+ self.update_auth(auth, 'scheme', scheme)
if req_auth.get('algorithm', None) is not None:
- auth['algorithm'] = req_auth.get('algorithm')
- auth['realm'] = req_auth['realm']
- auth['nonce'] = req_auth['nonce']
- auth['qop'] = qop
+ self.update_auth(auth, 'algorithm', req_auth.get('algorithm'))
+ self.update_auth(auth, 'realm', realm)
+ nonce = req_auth['nonce']
+ if auth.get('nonce', None) != nonce:
+ # A new nonce, never used
+ self.update_auth(auth, 'nonce_count', 0)
+ self.update_auth(auth, 'nonce', nonce)
+ self.update_auth(auth, 'qop', qop)
auth['opaque'] = req_auth.get('opaque', None)
except KeyError:
# Some required field is not there
@@ -1004,8 +1030,7 @@
nonce = auth['nonce']
qop = auth['qop']
- nonce_count = auth.get('nonce_count',0)
- nonce_count += 1
+ nonce_count = auth['nonce_count'] + 1
ncvalue = '%08x' % nonce_count
cnonce = get_new_cnonce(nonce, nonce_count)
More information about the bazaar-commits
mailing list