improved sensitive values handling (requests/urllib3)

Related: https://pagure.io/koji/issue/3246
This commit is contained in:
Tomas Kopecek 2022-04-12 14:57:06 +02:00
parent ba5ecbb0c9
commit 1652dcc606
2 changed files with 42 additions and 11 deletions

View file

@ -65,7 +65,7 @@ import six.moves.http_client
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
@ -2713,23 +2713,38 @@ class ClientSession(object):
]
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()

View file

@ -2,7 +2,9 @@ from __future__ import absolute_import
import mock
import six
import weakref
import requests
import unittest
from requests.packages.urllib3.exceptions import MaxRetryError, HostChangedError
import koji
@ -207,3 +209,17 @@ class TestMultiCall(unittest.TestCase):
# 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)