Rev 5960: Mask credentials in the -Dhttp logging in file:///home/vila/src/bzr/bugs/723074-http-debug-mask-credentials/

Vincent Ladeuil v.ladeuil+lp at free.fr
Tue Jun 7 12:25:45 UTC 2011


At file:///home/vila/src/bzr/bugs/723074-http-debug-mask-credentials/

------------------------------------------------------------
revno: 5960
revision-id: v.ladeuil+lp at free.fr-20110607122545-fx8ao2x977ettbv2
parent: v.ladeuil+lp at free.fr-20110607111108-bt0ud8l7vrxlgib6
fixes bug(s): https://launchpad.net/bugs/723074
committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
branch nick: 723074-http-debug-mask-credentials
timestamp: Tue 2011-06-07 14:25:45 +0200
message:
  Mask credentials in the -Dhttp logging
-------------- next part --------------
=== 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-06 11:13:41 +0000
+++ b/doc/en/release-notes/bzr-2.4.txt	2011-06-07 12:25:45 +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`` masks credentials 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