PR#3008: Allow kojiweb to proxy users obtained via different mechanisms - koji

		`@@ -54,6 +54,9 @@`

		`## Other options ##`
		`LoginCreatesUser = On`
		`+ # Clients with ProxyPrincipals can use different method for proxying user than GSSAPI. In such case`
		`+ # it need to be explicitely allowed via AllowProxyAuthType.`
		`+ # AllowProxyAuthType = Off`
		`KojiWebURL = http://kojiweb.example.com/koji`
		`# The domain name that will be appended to Koji usernames`
		`# when creating email notifications`

hub/kojixmlrpc.py

file modified

		`@@ -427,6 +427,7 @@`
		`['CheckClientIP', 'boolean', True],`

		`['LoginCreatesUser', 'boolean', True],`
		`+ ['AllowProxyAuthType', 'boolean', False],`
		`['KojiWebURL', 'string', 'http://localhost.localdomain/koji'],`
		`['EmailDomain', 'string', None],`
		`['NotifyOnSuccess', 'boolean', True],`

koji/__init__.py

file modified

+12 -4

		`@@ -2472,7 +2472,8 @@`
		`return self.gssapi_login(principal=principal, keytab=keytab,`
		`ccache=ccache, proxyuser=proxyuser)`

		`- def gssapi_login(self, principal=None, keytab=None, ccache=None, proxyuser=None):`
		`+ def gssapi_login(self, principal=None, keytab=None, ccache=None,`
		`+ proxyuser=None, proxyauthtype=None):`
		`if not reqgssapi:`
		`raise PythonImportError(`
		`"Please install python-requests-gssapi to use GSSAPI."`
		`@@ -2515,7 +2516,10 @@`
		`# Depending on the server configuration, we might not be able to`
		`# 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)`
		`+ kwargs = {'proxyuser': proxyuser}`
		`+ if proxyauthtype is not None:`
		`+ kwargs['proxyauthtype'] = proxyauthtype`
		`+ sinfo = self._callMethod('sslLogin', [], kwargs, retry=False)`
		`except Exception as e:`
		`e_str = ''.join(traceback.format_exception_only(type(e), e)).strip('\n')`
		`e_str = '(gssapi auth failed: %s)\n' % e_str`
		`@@ -2542,7 +2546,7 @@`
		`self.authtype = AUTHTYPE_GSSAPI`
		`return True`

		`- def ssl_login(self, cert=None, ca=None, serverca=None, proxyuser=None):`
		`+ def ssl_login(self, cert=None, ca=None, serverca=None, proxyuser=None, proxyauthtype=None):`
		`cert = cert or self.opts.get('cert')`
		`serverca = serverca or self.opts.get('serverca')`
		`if cert is None:`
		`@@ -2571,7 +2575,11 @@`
		`self.opts['serverca'] = serverca`
		`e_str = None`
		`try:`
		`- sinfo = self.callMethod('sslLogin', proxyuser)`
		`+ kwargs = {'proxyuser': proxyuser}`
		`+ if proxyauthtype is not None:`
		`+ kwargs['proxyauthtype'] = proxyauthtype`
		`+ sinfo = self._callMethod('sslLogin', [], kwargs)`
		`+`
		`except Exception as ex:`
		`e_str = ''.join(traceback.format_exception_only(type(ex), ex))`
		`e_str = 'ssl auth failed: %s' % e_str`

koji/auth.py

file modified

+23 -1

		`@@ -315,7 +315,19 @@`

		`return (local_ip, local_port, remote_ip, remote_port)`

		`- def sslLogin(self, proxyuser=None):`
		`+ def sslLogin(self, proxyuser=None, proxyauthtype=None):`
		`+`
		`+ """Login into brew via SSL. proxyuser name can be specified and if it is`
		`+ allowed in the configuration file then connection is allowed to login as`
		`+ that user. By default we assume that proxyuser is coming via same`
		`+ authentication mechanism but proxyauthtype can be set to koji.AUTHTYPE_*`
		`+ value for different handling. Typical case is proxying kerberos user via`
		`+ web ui which itself is authenticated via SSL certificate. (See kojiweb`
		`+ for usage).`
		`+`
		`+ proxyauthtype is working only if AllowProxyAuthType option is set to`
		`+ 'On' in the hub.conf`
		`+ """`
		`if self.logged_in:`
		`raise koji.AuthError("Already logged in")`

		`@@ -362,6 +374,16 @@`
		`else:`
		`raise koji.AuthError('%s is not authorized to login other users' % client_dn)`

		`+ # in this point we can continue with proxied user in same way as if it is not proxied`
		`+ if proxyauthtype is not None:`
		`+ if not context.opts['AllowProxyAuthType']:`
		`+ raise koji.AuthError("Proxy must use same auth mechanism as hub (behaviour "`
		`+ "can be overriden via AllowProxyAuthType hub option)")`
		`+ if proxyauthtype not in (koji.AUTHTYPE_GSSAPI, koji.AUTHTYPE_SSL):`
		`+ raise koji.AuthError(`
		`+ "Proxied authtype %s is not valid for sslLogin" % proxyauthtype)`
		`+ authtype = proxyauthtype`
		`+`
		`if authtype == koji.AUTHTYPE_GSSAPI and '@' in username:`
		`user_id = self.getUserIdFromKerberos(username)`
		`else:`

tests/test_lib/test_gssapi.py

file modified

+6 -7

		`@@ -26,8 +26,8 @@`
		`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.session._callMethod.assert_called_once_with(`
		`+ 'sslLogin', [], {'proxyuser': None}, retry=False)`
		`self.assertEqual(old_environ, dict(**os.environ))`

		`@mock.patch('koji.reqgssapi.HTTPKerberosAuth')`
		`@@ -46,9 +46,8 @@`
		`for accepted_version in accepted_versions:`
		`koji.reqgssapi.__version__ = accepted_version`
		`rv = self.session.gssapi_login(principal, keytab, ccache)`
		`- self.session._callMethod.assert_called_once_with('sslLogin',`
		`- [None],`
		`- retry=False)`
		`+ self.session._callMethod.assert_called_once_with(`
		`+ 'sslLogin', [], {'proxyuser': None}, retry=False)`
		`self.assertEqual(old_environ, dict(**os.environ))`
		`self.assertTrue(rv)`
		`self.session._callMethod.reset_mock()`
		`@@ -84,8 +83,8 @@`
		`self.session._callMethod.side_effect = Exception('login failed')`
		`with self.assertRaises(koji.GSSAPIAuthError):`
		`self.session.gssapi_login()`
		`- self.session._callMethod.assert_called_once_with('sslLogin', [None],`
		`- retry=False)`
		`+ self.session._callMethod.assert_called_once_with(`
		`+ 'sslLogin', [], {'proxyuser': None}, retry=False)`
		`self.assertEqual(old_environ, dict(**os.environ))`

		`def test_gssapi_login_http(self):`

www/conf/web.conf

file modified

		`@@ -21,6 +21,12 @@`
		`# it already. Note, that it will override that bundle.`
		`# KojiHubCA = /etc/kojiweb/kojihubca.crt`

		`+ # How the users authenticate to kojiweb, if different from the`
		`+ # way Kojiweb authenticates to the hub. This can be used`
		`+ # to have users authenticate to kojiweb via kerberos while`
		`+ # still using an SSL certificate to authenticate to the hub.`
		`+ # WebAuthType = kerberos`
		`+`
		`LoginTimeout = 72`

		`# This must be CHANGED to random value and uncommented before deployment`

www/kojiweb/index.py

file modified

+25 -15

		`@@ -151,17 +151,18 @@`
		`wprinc = options['WebPrincipal']`
		`keytab = options['WebKeytab']`
		`ccache = options['WebCCache']`
		`+ authtype = options['WebAuthType']`
		`return session.gssapi_login(principal=wprinc, keytab=keytab,`
		`- ccache=ccache, proxyuser=principal)`
		`+ ccache=ccache, proxyuser=principal, proxyauthtype=authtype)`


		`def _sslLogin(environ, session, username):`
		`options = environ['koji.options']`
		`client_cert = options['WebCert']`
		`server_ca = options['KojiHubCA']`
		`-`
		`+ authtype = options['WebAuthType']`
		`return session.ssl_login(client_cert, None, server_ca,`
		`- proxyuser=username)`
		`+ proxyuser=username, proxyauthtype=authtype)`


		`def _assertLogin(environ):`
		`@@ -272,8 +273,9 @@`
		`session = _getServer(environ)`
		`options = environ['koji.options']`

		`- # try SSL first, fall back to Kerberos`
		`- if options['WebCert']:`
		`+ if options['WebAuthType'] == koji.AUTHTYPE_SSL:`
		`+ ## Clients authenticate to KojiWeb by SSL, so extract`
		`+ ## the username via the (verified) client certificate`
		`if environ['wsgi.url_scheme'] != 'https':`
		`dest = 'login'`
		`if page:`
		`@@ -288,23 +290,31 @@`
		`username = environ.get('SSL_CLIENT_S_DN_CN')`
		`if not username:`
		`raise koji.AuthError('unable to get user information from client certificate')`
		`-`
		`- if not _sslLogin(environ, session, username):`
		`- raise koji.AuthError('could not login %s using SSL certificates' % username)`
		`-`
		`- authlogger.info('Successful SSL authentication by %s', username)`
		`-`
		`- elif options['WebPrincipal']:`
		`+ elif options['WebAuthType'] == koji.AUTHTYPE_GSSAPI:`
		`+ ## Clients authenticate to KojiWeb by Kerberos, so extract`
		`+ ## the username via the REMOTE_USER which will be the`
		`+ ## Kerberos principal`
		`principal = environ.get('REMOTE_USER')`
		`if not principal:`
		`raise koji.AuthError(`
		`'configuration error: mod_auth_gssapi should have performed authentication before '`
		`'presenting this page')`

		`- if not _gssapiLogin(environ, session, principal):`
		`- raise koji.AuthError('could not login using principal: %s' % principal)`
		`-`
		`username = principal`
		`+ else:`
		`+ raise koji.AuthError(`
		`+ 'configuration error: set WebAuthType or on of WebPrincipal/WebCert options')`
		`+`
		`+ ## This now is how we proxy the user to the hub`
		`+ if options['WebCert']:`
		`+ if not _sslLogin(environ, session, username):`
		`+ raise koji.AuthError('could not login %s using SSL certificates' % username)`
		`+`
		`+ authlogger.info('Successful SSL authentication by %s', username)`
		`+ elif options['WebPrincipal']:`
		`+ if not _gssapiLogin(environ, session, username):`
		`+ raise koji.AuthError('could not login using principal: %s' % username)`
		`+`
		`authlogger.info('Successful Kerberos authentication by %s', username)`
		`else:`
		`raise koji.AuthError(`

www/kojiweb/wsgi_publisher.py

file modified

+16

		`@@ -78,6 +78,8 @@`
		`['KrbCanonHost', 'boolean', False],`
		`['KrbServerRealm', 'string', None],`

		`+ ['WebAuthType', 'string', None],`
		`+`
		`['WebCert', 'string', None],`
		`['KojiHubCA', 'string', '/etc/kojiweb/kojihubca.crt'],`

		`@@ -148,6 +150,20 @@`
		`else:`
		`opts[name] = default`
		`opts['Secret'] = koji.util.HiddenValue(opts['Secret'])`
		`+`
		`+ if opts['WebAuthType'] not in (None, 'gssapi', 'ssl'):`
		`+ raise koji.ConfigurationError(f"Invalid value {opts['WebAuthType']} for "`
		`+ "WebAuthType (ssl/gssapi)")`
		`+ if opts['WebAuthType'] == 'gssapi':`
		`+ opts['WebAuthType'] = koji.AUTHTYPE_GSSAPI`
		`+ elif opts['WebAuthType'] == 'ssl':`
		`+ opts['WebAuthType'] = koji.AUTHTYPE_SSL`
		`+ # if there is no explicit request, use same authtype as web has`
		`+ elif opts['WebPrincipal']:`
		`+ opts['WebAuthType'] = koji.AUTHTYPE_GSSAPI`
		`+ elif opts['WebCert']:`
		`+ opts['WebAuthType'] = koji.AUTHTYPE_SSL`
		`+`
		`self.options = opts`
		`return opts`

tkopecek commented 2 years ago

Extension of #2550
Added proxyauthtype option to sslLogin

mikem commented 2 years ago

proxyauthtype should only be considered during proxy auth (so, only after we've checked that proxy auth is allowed). This patch seems to unconditionally apply the parameter.

mikem commented 2 years ago

The lack of an else: case in the WebAuthType if block in login() bothers me somewhat. I don't think we're going to see an error from it, but WebAuthType can technically still be None here.

With the current code, the None case will error in the next block because we can only get the None when both WebCert and WebPrincipal are unset. Still, it feels a little fragile to me, since it relies on an inference about load_config's behavior.

mikem commented 2 years ago

In #2550, I was concerned about the case where web auth is based on a user cookie. However, I don't believe this will cause any issues because if we have a cookie then the user must already exist (in fact, _getServer will error if it does not before we even get to _assertLogin).

So, even though in the cookie case we're going to pass the plain username (no domain) with proxyauthtype=koji.AUTHTYPE_GSSAPI, it's all going to work out.

rebased onto 2143cf2f7ead88900d33b9f1a10db14887be393a

2 years ago

tkopecek commented 2 years ago

Added option to hub.conf + exception in else branch.

5 new commits added

further fixes (will be squashed before merge)
proxyauthtype for web users
Reverse check order between WebCert and WebPrincipal in case both are set
Allow kojiweb to proxy users obtained via different mechanisms
proxy login method

2 years ago

mikem commented 2 years ago

The proxyauthtype parameter is passed to ClientSession methods gssapi_login and ssl_login, but neither of these accept it. They should pass it though as they do proxyuser.

Adding the ProxyAuthType boolean config is reasonable, but it doesn't address the issue that the code is still accepting and honoring proxyauthtype even when proxyuser is not set (and therefore its associated access checks are not performed. I think that the if proxyauthtype block should be nested under the if proxyuser block.

Also, might be better to name the option AllowProxyAuthType for clarity.

rebased onto 24e4620d2eaddb7bb4dfc1972f1b19d03e7937df

2 years ago

8 new commits added