From 6bfef6cc1b0ef8165481d0005535804bc3bb44ce Mon Sep 17 00:00:00 2001 From: Patrick Uiterwijk Date: Dec 06 2017 15:24:33 +0000 Subject: [PATCH 1/7] Implement support for keytab in gssapi codepaths This had been a TODO that until now wasn't critical since python2 would fall back to krbV. For python3, we only have gssapi, so implement keytabs and ccache for gssapi. Signed-off-by: Patrick Uiterwijk --- diff --git a/koji/__init__.py b/koji/__init__.py index b462dd1..aa97ce6 100644 --- a/koji/__init__.py +++ b/koji/__init__.py @@ -23,6 +23,7 @@ from __future__ import absolute_import +import copy import sys from six.moves import range from six.moves import zip @@ -2126,16 +2127,15 @@ class ClientSession(object): principal. The principal must be in the "ProxyPrincipals" list on the server side.""" - if principal is None and keytab is None and ccache is None: - try: - # Silently try GSSAPI first - if self.gssapi_login(proxyuser=proxyuser): - return True - except: - if krbV: - pass - else: - raise + try: + # Silently try GSSAPI first + if self.gssapi_login(principal, keytab, ccache, proxyuser=proxyuser): + return True + except: + if krbV: + pass + else: + raise if not krbV: raise PythonImportError( @@ -2224,7 +2224,7 @@ class ClientSession(object): # else return host - def gssapi_login(self, proxyuser=None): + def gssapi_login(self, principal=None, keytab=None, ccache=None, proxyuser=None): if not HTTPKerberosAuth: raise PythonImportError( "Please install python-requests-kerberos to use GSSAPI." @@ -2241,10 +2241,18 @@ class ClientSession(object): # 60 second timeout during login sinfo = None + old_env = copy.copy(os.environ) old_opts = self.opts self.opts = old_opts.copy() self.opts['timeout'] = 60 - self.opts['auth'] = HTTPKerberosAuth() + kwargs = {} + if keytab: + os.environ['KRB5_CLIENT_KTNAME'] = keytab + if ccache: + os.environ['KRB5CCNAME'] = ccache + if principal: + kwargs['principal'] = principal + self.opts['auth'] = HTTPKerberosAuth(**kwargs) try: try: # Depending on the server configuration, we might not be able to @@ -2256,6 +2264,7 @@ class ClientSession(object): self.baseurl = old_baseurl finally: self.opts = old_opts + os.environ = old_env if not sinfo: raise AuthError('unable to obtain a session') From b03d827c1cca4e98d0ee01fc99bd0c9467b3edb9 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Dec 06 2017 15:24:33 +0000 Subject: [PATCH 2/7] avoid bare exception and add some debug logging --- diff --git a/koji/__init__.py b/koji/__init__.py index aa97ce6..2a41455 100644 --- a/koji/__init__.py +++ b/koji/__init__.py @@ -2131,8 +2131,10 @@ class ClientSession(object): # Silently try GSSAPI first if self.gssapi_login(principal, keytab, ccache, proxyuser=proxyuser): return True - except: + except Exception, e: if krbV: + e_str = ''.join(traceback.format_exception_only(type(e), e)) + self.logger.debug('gssapi auth failed: %s', e_str) pass else: raise From ec278cc148a332777f93d64c205ab183e32194cd Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Dec 06 2017 15:24:33 +0000 Subject: [PATCH 3/7] be a little more paranoid about undoing opts/environ mangling --- diff --git a/koji/__init__.py b/koji/__init__.py index 2a41455..9d9be6f 100644 --- a/koji/__init__.py +++ b/koji/__init__.py @@ -2246,16 +2246,16 @@ class ClientSession(object): old_env = copy.copy(os.environ) old_opts = self.opts self.opts = old_opts.copy() - self.opts['timeout'] = 60 - kwargs = {} - if keytab: - os.environ['KRB5_CLIENT_KTNAME'] = keytab - if ccache: - os.environ['KRB5CCNAME'] = ccache - if principal: - kwargs['principal'] = principal - self.opts['auth'] = HTTPKerberosAuth(**kwargs) try: + self.opts['timeout'] = 60 + kwargs = {} + if keytab: + os.environ['KRB5_CLIENT_KTNAME'] = keytab + if ccache: + os.environ['KRB5CCNAME'] = ccache + if principal: + kwargs['principal'] = principal + self.opts['auth'] = HTTPKerberosAuth(**kwargs) try: # Depending on the server configuration, we might not be able to # connect without client certificate, which means that the conn From 8a0ed9538cd3a303c2682ebf736988679e3fd2bf Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Dec 06 2017 15:24:33 +0000 Subject: [PATCH 4/7] fix another bare exception --- diff --git a/koji/__init__.py b/koji/__init__.py index 9d9be6f..f59d3de 100644 --- a/koji/__init__.py +++ b/koji/__init__.py @@ -2261,7 +2261,9 @@ class ClientSession(object): # connect without client certificate, which means that the conn # will fail with a handshake failure, which is retried by default. sinfo = self._callMethod('sslLogin', [proxyuser], retry=False) - except: + except Exception as e: + e_str = ''.join(traceback.format_exception_only(type(e), e)) + self.logger.debug('gssapi auth failed: %s', e_str) # Auth with https didn't work. Restore for the next attempt. self.baseurl = old_baseurl finally: From b4b197ca5a74bb0b71fd824edeceb4fec9e4dea0 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Dec 06 2017 15:24:33 +0000 Subject: [PATCH 5/7] unit tests for gssapi auth --- diff --git a/tests/test_lib/test_gssapi.py b/tests/test_lib/test_gssapi.py new file mode 100644 index 0000000..4927f96 --- /dev/null +++ b/tests/test_lib/test_gssapi.py @@ -0,0 +1,67 @@ +from __future__ import absolute_import +import mock +import os +import unittest + +import koji + + +class TestGSSAPI(unittest.TestCase): + + def setUp(self): + self.session = koji.ClientSession('https://koji.example.com/kojihub', {}) + self.session._callMethod = mock.MagicMock(name='_callMethod') + + def tearDown(self): + mock.patch.stopall() + + maxDiff = None + + @mock.patch('koji.HTTPKerberosAuth', new=None) + def test_gssapi_disabled(self): + with self.assertRaises(ImportError): + self.session.gssapi_login() + + def test_gssapi_login(self): + old_environ = dict(**os.environ) + self.session.gssapi_login() + self.session._callMethod.assert_called_once_with('sslLogin', [None], + retry=False) + self.assertEqual(old_environ, dict(**os.environ)) + + def test_gssapi_login_keytab(self): + principal = 'user@EXAMPLE.COM' + keytab = '/path/to/keytab' + ccache = '/path/to/cache' + old_environ = dict(**os.environ) + self.session.gssapi_login(principal, keytab, ccache) + self.session._callMethod.assert_called_once_with('sslLogin', [None], + retry=False) + self.assertEqual(old_environ, dict(**os.environ)) + + def test_gssapi_login_error(self): + old_environ = dict(**os.environ) + self.session._callMethod.side_effect = Exception('login failed') + with self.assertRaises(koji.AuthError): + self.session.gssapi_login() + self.session._callMethod.assert_called_once_with('sslLogin', [None], + retry=False) + self.assertEqual(old_environ, dict(**os.environ)) + + def test_gssapi_login_http(self): + old_environ = dict(**os.environ) + url1 = 'http://koji.example.com/kojihub' + url2 = 'https://koji.example.com/kojihub' + + # successful gssapi auth should force https + self.session.baseurl = url1 + self.session.gssapi_login() + self.assertEqual(self.session.baseurl, url2) + + # failed gssapi auth should leave the url alone + self.session.baseurl = url1 + self.session._callMethod.side_effect = Exception('login failed') + with self.assertRaises(koji.AuthError): + self.session.gssapi_login() + self.assertEqual(self.session.baseurl, url1) + self.assertEqual(old_environ, dict(**os.environ)) From 78a0f23b82df493891fbc0a8d7a0bb1df85a77bb Mon Sep 17 00:00:00 2001 From: Patrick Uiterwijk Date: Dec 06 2017 16:43:03 +0000 Subject: [PATCH 6/7] Fix resetting the environment Signed-off-by: Patrick Uiterwijk --- diff --git a/koji/__init__.py b/koji/__init__.py index f59d3de..2d953e2 100644 --- a/koji/__init__.py +++ b/koji/__init__.py @@ -23,7 +23,6 @@ from __future__ import absolute_import -import copy import sys from six.moves import range from six.moves import zip @@ -2243,15 +2242,17 @@ class ClientSession(object): # 60 second timeout during login sinfo = None - old_env = copy.copy(os.environ) + old_env = {} old_opts = self.opts self.opts = old_opts.copy() try: self.opts['timeout'] = 60 kwargs = {} if keytab: + old_env['KRB5_CLIENT_KTNAME'] = os.environ.get('KRB5_CLIENT_KTNAME') os.environ['KRB5_CLIENT_KTNAME'] = keytab if ccache: + old_env['KRB5CCNAME'] = os.environ.get('KRB5CCNAME') os.environ['KRB5CCNAME'] = ccache if principal: kwargs['principal'] = principal @@ -2268,7 +2269,11 @@ class ClientSession(object): self.baseurl = old_baseurl finally: self.opts = old_opts - os.environ = old_env + for key in old_env: + if old_env[key] is None: + del os.environ[key] + else: + os.environ[key] = old_env[key] if not sinfo: raise AuthError('unable to obtain a session') From baee8cc3bae56561a36d57fad6d8480eb8970884 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Dec 07 2017 15:03:45 +0000 Subject: [PATCH 7/7] fix exception syntax --- diff --git a/koji/__init__.py b/koji/__init__.py index 2d953e2..0aed17f 100644 --- a/koji/__init__.py +++ b/koji/__init__.py @@ -2130,7 +2130,7 @@ class ClientSession(object): # Silently try GSSAPI first if self.gssapi_login(principal, keytab, ccache, proxyuser=proxyuser): return True - except Exception, e: + except Exception as e: if krbV: e_str = ''.join(traceback.format_exception_only(type(e), e)) self.logger.debug('gssapi auth failed: %s', e_str)