Rev 5962: (vila) Don't leak credentials with -Dhttp (Vincent Ladeuil) in file:///home/pqm/archives/thelove/bzr/%2Btrunk/
Canonical.com Patch Queue Manager
pqm at pqm.ubuntu.com
Tue Jun 7 18:39:40 UTC 2011
At file:///home/pqm/archives/thelove/bzr/%2Btrunk/
------------------------------------------------------------
revno: 5962 [merge]
revision-id: pqm at pqm.ubuntu.com-20110607183938-ir1i1e5ofmn31krv
parent: pqm at pqm.ubuntu.com-20110607172150-b8tdisesgnjxk7ku
parent: v.ladeuil+lp at free.fr-20110607143402-2xw29n3bc1l0ti1k
committer: Canonical.com Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: +trunk
timestamp: Tue 2011-06-07 18:39:38 +0000
message:
(vila) Don't leak credentials with -Dhttp (Vincent Ladeuil)
modified:
bzrlib/tests/http_utils.py HTTPTestUtil.py-20050914180604-247d3aafb7a43343
bzrlib/tests/test_http.py testhttp.py-20051018020158-b2eef6e867c514d9
bzrlib/transport/http/_pycurl.py pycurlhttp.py-20060110060940-4e2a705911af77a6
bzrlib/transport/http/_urllib2_wrappers.py _urllib2_wrappers.py-20060913231729-ha9ugi48ktx481ao-1
doc/en/release-notes/bzr-2.4.txt bzr2.4.txt-20110114053217-k7ym9jfz243fddjm-1
=== modified file 'bzrlib/tests/http_utils.py'
--- a/bzrlib/tests/http_utils.py 2011-01-12 01:01:53 +0000
+++ b/bzrlib/tests/http_utils.py 2011-06-07 11:11:08 +0000
@@ -284,21 +284,30 @@
# - auth_header_recv: the header received containing auth
# - auth_error_code: the error code to indicate auth required
+ def _require_authentication(self):
+ # Note that we must update test_case_server *before*
+ # sending the error or the client may try to read it
+ # before we have sent the whole error back.
+ tcs = self.server.test_case_server
+ tcs.auth_required_errors += 1
+ self.send_response(tcs.auth_error_code)
+ self.send_header_auth_reqed()
+ # We do not send a body
+ self.send_header('Content-Length', '0')
+ self.end_headers()
+ return
+
def do_GET(self):
if self.authorized():
return http_server.TestingHTTPRequestHandler.do_GET(self)
else:
- # Note that we must update test_case_server *before*
- # sending the error or the client may try to read it
- # before we have sent the whole error back.
- tcs = self.server.test_case_server
- tcs.auth_required_errors += 1
- self.send_response(tcs.auth_error_code)
- self.send_header_auth_reqed()
- # We do not send a body
- self.send_header('Content-Length', '0')
- self.end_headers()
- return
+ return self._require_authentication()
+
+ def do_HEAD(self):
+ if self.authorized():
+ return http_server.TestingHTTPRequestHandler.do_HEAD(self)
+ else:
+ return self._require_authentication()
class BasicAuthRequestHandler(AuthRequestHandler):
=== modified file 'bzrlib/tests/test_http.py'
--- a/bzrlib/tests/test_http.py 2011-06-07 08:12:32 +0000
+++ b/bzrlib/tests/test_http.py 2011-06-07 12:25:45 +0000
@@ -34,10 +34,12 @@
bzrdir,
cethread,
config,
+ debug,
errors,
osutils,
remote as _mod_remote,
tests,
+ trace,
transport,
ui,
)
@@ -100,21 +102,23 @@
]
# Add some attributes common to all scenarios
for scenario_id, scenario_dict in scenarios:
- scenario_dict.update(_username_prompt_prefix='',
+ scenario_dict.update(_auth_header='Authorization',
+ _username_prompt_prefix='',
_password_prompt_prefix='')
return scenarios
def vary_by_http_proxy_auth_scheme():
scenarios = [
- ('basic', dict(_auth_server=http_utils.ProxyBasicAuthServer)),
- ('digest', dict(_auth_server=http_utils.ProxyDigestAuthServer)),
- ('basicdigest',
+ ('proxy-basic', dict(_auth_server=http_utils.ProxyBasicAuthServer)),
+ ('proxy-digest', dict(_auth_server=http_utils.ProxyDigestAuthServer)),
+ ('proxy-basicdigest',
dict(_auth_server=http_utils.ProxyBasicAndDigestAuthServer)),
]
# Add some attributes common to all scenarios
for scenario_id, scenario_dict in scenarios:
- scenario_dict.update(_username_prompt_prefix='Proxy ',
+ scenario_dict.update(_auth_header='Proxy-Authorization',
+ _username_prompt_prefix='Proxy ',
_password_prompt_prefix='Proxy ')
return scenarios
@@ -1728,6 +1732,32 @@
# Only one 'Authentication Required' error should occur
self.assertEqual(1, self.server.auth_required_errors)
+ def test_no_credential_leaks_in_log(self):
+ self.overrideAttr(debug, 'debug_flags', set(['http']))
+ user = 'joe'
+ password = 'very-sensitive-password'
+ self.server.add_user(user, password)
+ t = self.get_user_transport(user, password)
+ # Capture the debug calls to mutter
+ self.mutters = []
+ def mutter(*args):
+ lines = args[0] % args[1:]
+ # Some calls output multiple lines, just split them now since we
+ # care about a single one later.
+ self.mutters.extend(lines.splitlines())
+ self.overrideAttr(trace, 'mutter', mutter)
+ # Issue a request to the server to connect
+ self.assertEqual(True, t.has('a'))
+ # Only one 'Authentication Required' error should occur
+ self.assertEqual(1, self.server.auth_required_errors)
+ # Since the authentification succeeded, there should be a corresponding
+ # debug line
+ sent_auth_headers = [line for line in self.mutters
+ if line.startswith('> %s' % (self._auth_header,))]
+ self.assertLength(1, sent_auth_headers)
+ self.assertStartsWith(sent_auth_headers[0],
+ '> %s: <masked>' % (self._auth_header,))
+
class TestProxyAuth(TestAuth):
"""Test proxy authentication schemes.
@@ -2243,4 +2273,3 @@
# stdout should be empty, stderr will contains the prompts
self.assertEqual('', stdout.getvalue())
-
=== modified file 'bzrlib/transport/http/_pycurl.py'
--- a/bzrlib/transport/http/_pycurl.py 2011-05-27 07:39:41 +0000
+++ b/bzrlib/transport/http/_pycurl.py 2011-06-07 12:25:45 +0000
@@ -37,9 +37,9 @@
from bzrlib import (
debug,
errors,
+ trace,
)
import bzrlib
-from bzrlib.trace import mutter
from bzrlib.transport.http import (
ca_bundle,
HttpTransportBase,
@@ -50,7 +50,7 @@
try:
import pycurl
except ImportError, e:
- mutter("failed to import pycurl: %s", e)
+ trace.mutter("failed to import pycurl: %s", e)
raise errors.DependencyNotPresent('pycurl', e)
try:
@@ -65,7 +65,7 @@
# reported by Alexander Belchenko, 2006-04-26
pycurl.Curl()
except pycurl.error, e:
- mutter("failed to initialize pycurl: %s", e)
+ trace.mutter("failed to initialize pycurl: %s", e)
raise errors.DependencyNotPresent('pycurl', e)
@@ -282,8 +282,8 @@
# error code and the headers are known to be available, we just
# swallow the exception, leaving the upper levels handle the
# 400+ error.
- mutter('got pycurl error in POST: %s, %s, %s, url: %s ',
- e[0], e[1], e, abspath)
+ trace.mutter('got pycurl error in POST: %s, %s, %s, url: %s ',
+ e[0], e[1], e, abspath)
else:
# Re-raise otherwise
raise
@@ -332,15 +332,26 @@
self._report_activity(len(text), 'read')
if (kind == pycurl.INFOTYPE_HEADER_IN
and 'http' in debug.debug_flags):
- mutter('< %s' % text)
+ trace.mutter('< %s' % (text.rstrip(),))
elif kind in (pycurl.INFOTYPE_HEADER_OUT, pycurl.INFOTYPE_DATA_OUT,
pycurl.INFOTYPE_SSL_DATA_OUT):
self._report_activity(len(text), 'write')
if (kind == pycurl.INFOTYPE_HEADER_OUT
and 'http' in debug.debug_flags):
- mutter('> %s' % text)
+ lines = []
+ for line in text.rstrip().splitlines():
+ # People are often told to paste -Dhttp output to help
+ # debug. Don't compromise credentials.
+ try:
+ header, details = line.split(':', 1)
+ except ValueError:
+ header = None
+ if header in ('Authorization', 'Proxy-Authorization'):
+ line = '%s: <masked>' % (header,)
+ lines.append(line)
+ trace.mutter('> ' + '\n> '.join(lines))
elif kind == pycurl.INFOTYPE_TEXT and 'http' in debug.debug_flags:
- mutter('* %s' % text)
+ trace.mutter('* %s' % text.rstrip())
def _set_curl_options(self, curl):
"""Set options for all requests"""
@@ -377,8 +388,8 @@
curl.perform()
except pycurl.error, e:
url = curl.getinfo(pycurl.EFFECTIVE_URL)
- mutter('got pycurl error: %s, %s, %s, url: %s ',
- e[0], e[1], e, url)
+ trace.mutter('got pycurl error: %s, %s, %s, url: %s ',
+ e[0], e[1], e, url)
if e[0] in (CURLE_COULDNT_RESOLVE_HOST,
CURLE_COULDNT_RESOLVE_PROXY,
CURLE_COULDNT_CONNECT,
=== modified file 'bzrlib/transport/http/_urllib2_wrappers.py'
--- a/bzrlib/transport/http/_urllib2_wrappers.py 2011-05-27 07:39:41 +0000
+++ b/bzrlib/transport/http/_urllib2_wrappers.py 2011-06-07 12:25:45 +0000
@@ -648,7 +648,13 @@
headers)
if 'http' in debug.debug_flags:
trace.mutter('> %s %s' % (method, url))
- hdrs = ['%s: %s' % (k, v) for k,v in headers.items()]
+ hdrs = []
+ for k,v in headers.iteritems():
+ # People are often told to paste -Dhttp output to help
+ # debug. Don't compromise credentials.
+ if k in ('Authorization', 'Proxy-Authorization'):
+ v = '<masked>'
+ hdrs.append('%s: %s' % (k, v))
trace.mutter('> ' + '\n> '.join(hdrs) + '\n')
if self._debuglevel >= 1:
print 'Request sent: [%r] from (%s)' \
=== modified file 'doc/en/release-notes/bzr-2.4.txt'
--- a/doc/en/release-notes/bzr-2.4.txt 2011-06-07 17:21:50 +0000
+++ b/doc/en/release-notes/bzr-2.4.txt 2011-06-07 18:39:38 +0000
@@ -36,6 +36,9 @@
.. Fixes for situations where bzr would previously crash or give incorrect
or undesirable results.
+* Credentials in the log output produced by ``-Dhttp`` are masked so users
+ can more freely post them in bug reports. (Vincent Ladeuil, #723074)
+
* Fix a race condition for ``server_started`` hooks leading to a spurious
test failure. (Vincent Ladeuil, #789167)
More information about the bazaar-commits
mailing list