PR#3322: improved sensitive values handling (requests/urllib3) - koji

koji

#3322 improved sensitive values handling (requests/urllib3)

Merged 2 years ago by tkopecek. Opened 2 years ago by tkopecek.

tkopecek/koji issue3246 into master

improved sensitive values handling (requests/urllib3)

Tomas Kopecek • 2 years ago

606444f

koji/__init__.py

file modified

+26 -11

		`@@ -65,7 +65,7 @@`
		`import six.moves.urllib`
		`from requests.adapters import HTTPAdapter`
		`from requests.packages.urllib3.util.retry import Retry`
		`- from requests.packages.urllib3.exceptions import MaxRetryError`
		`+ from requests.packages.urllib3.exceptions import MaxRetryError, HostChangedError`
		`from six.moves import range, zip`

		`from koji.tasks import parse_task_params`
		`@@ -2709,23 +2709,38 @@`
		`]`
		`return handler, headers, request`

		`+ def _sanitize_url(self, url):`
		`+ """Replace sensitive values in hub url"""`
		`+ url = re.sub(r'session-id=\d+', 'session-id=XXXXX', url)`
		`+ url = re.sub(r'session-key=[^&]+', 'session-key=XXXXX', url)`
		`+ return url`
		`+`
		`+ def _sanitize_connection_error(self, exc, top=True):`
		`+ """Replace sensitive values in nested exceptions"""`
		`+ if isinstance(exc, requests.exceptions.ConnectionError) or not top:`
		`+ # mask sensitive values, ConnectionError could contain underlying`
		`+ # urllib3 exception which displays full URL with session-id, etc.`
		`+ new_args = []`
		`+ for mre in exc.args:`
		`+ if isinstance(mre, (MaxRetryError, HostChangedError)):`
		`+ mre = self._sanitize_connection_error(mre, top=False)`
		`+ elif isinstance(mre, str):`
		`+ mre = self._sanitize_url(mre)`
		`+ new_args.append(mre)`
		`+ exc.args = tuple(new_args)`
		`+ if isinstance(exc, requests.exceptions.ConnectionError) and exc.request:`
		`+ exc.request.url = self._sanitize_url(exc.request.url)`
		`+ return exc`
		`+`
		`def _sendCall(self, handler, headers, request):`
		`# handle expired connections`
		`for i in (0, 1):`
		`try:`
		`return self._sendOneCall(handler, headers, request)`
		`except Exception as e:`
		`- if isinstance(e, requests.exceptions.ConnectionError):`
		`- # mask sensitive values, ConnectionError could contain underlying`
		`- # urllib3 exception which displays full URL with session-id, etc.`
		`- if e.args and isinstance(e.args[0], MaxRetryError):`
		`- mre = e.args[0]`
		`- s = mre.args[0]`
		`- s = re.sub(r'session-id=\d+', 'session-id=XXXXX', s)`
		`- s = re.sub(r'session-key=[^&]+', 'session-key=XXXXX', s)`
		`- mre.args = (s, mre.args[1:])`
		`+ e = self._sanitize_connection_error(e)`
		`if i or not is_conn_error(e):`
		`- raise`
		`+ raise e`
		`self.logger.debug("Connection Error: %s", e)`
		`self.new_session()`

tests/test_lib/test_client_session.py

file modified

+16

		`@@ -2,7 +2,9 @@`
		`import mock`
		`import six`
		`import weakref`
		`+ import requests`
		`import unittest`
		`+ from requests.packages.urllib3.exceptions import MaxRetryError, HostChangedError`

		`import koji`

		`@@ -207,3 +209,17 @@`

		`# This should not raise an exception`
		`koji.MultiCallHack(weakref.ref(self.ksession))`
		`+`
		`+ def test_sanitize_connection_error(self):`
		`+ url = "https://example.com/kojihub?session-id=12345&session-key=key"`
		`+ sanitized = "https://example.com/kojihub?session-id=XXXXX&session-key=XXXXX"`
		`+ exceptions = [`
		`+ requests.exceptions.ConnectionError("url: %s" % url),`
		`+ requests.exceptions.ConnectionError(MaxRetryError(pool="pool", url=url)),`
		`+ requests.exceptions.ConnectionError(HostChangedError(pool="pool", url=url))`
		`+ ]`
		`+ for exc in exceptions:`
		`+ res = self.ksession._sanitize_connection_error(exc)`
		`+ res = str(res)`
		`+ self.assertNotIn(url, res)`
		`+ self.assertIn(sanitized, res)`