PR#198: Implement realm checks for Kerberos authentication - koji

koji

#198 Implement realm checks for Kerberos authentication

Closed 4 years ago by tkopecek. Opened 7 years ago by puiterwijk.

puiterwijk/koji check-krb-realm into master

Implement realm checks for Kerberos authentication

Patrick Uiterwijk • 7 years ago

cabb3ee

hub/hub.conf

file modified

		`@@ -21,6 +21,10 @@`
		`# ProxyPrincipals = koji/kojiweb@EXAMPLE.COM`
		`## format string for host principals (%s = hostname)`
		`# HostPrincipalFormat = compile/%s@EXAMPLE.COM`
		`+ ## Whether to allow anonymous tickets (principal WELLKNOWN/ANONYMOUS or realm WELLKNOWN:ANONYMOUS)`
		`+ # AllowAnonymousKrb = False`
		`+ ## Which realms to allow access (this is ignored if explicitly set on a user)`
		`+ # AllowedKrbRealms = EXAMPLE.COM\|MYDOMAIN.COM`

		`## end Kerberos auth configuration`

koji/auth.py

file modified

+13 -4

		`@@ -638,10 +638,19 @@`
		`"""Create a new user, based on the Kerberos principal. Their`
		`username will be everything before the "@" in the principal.`
		`Return the ID of the newly created user."""`
		`- atidx = krb_principal.find('@')`
		`- if atidx == -1:`
		`- raise koji.AuthError, 'invalid Kerberos principal: %s' % krb_principal`
		`- user_name = krb_principal[:atidx]`
		`+ principal = krb_principal.rsplit('@', 1)`
		`+ if len(principal) != 2 or len(principal[1]) < 1:`
		`+ # We didn't have a realm`
		`+ raise koji.AuthError, 'Unparseable principal'`
mikem commented 7 years ago koji.AuthError('Unparseable principal')
		`+ user_name, realm = principal`
		`+ if user_name == 'WELLKNOWN/ANONYMOUS' and not context.opts.get('AllowAnonymousKrb', False):`
		`+ raise koji.AuthError, 'Anonymous tickets not allowed'`
		`+ if realm == 'WELLKNOWN:ANONYMOUS' and not context.opts.get('AllowAnonymousKrb', False):`
		`+ raise koji.AuthError, 'Anonymous realm not allowed'`
		`+ allowed_realms = context.opts.get('AllowedKrbRealms', None)`
		`+ if allowed_realms is not None:`
		`+ if not realm in allowed_realms.split('\|'):`
		`+ raise koji.AuthError, 'Realm %s is not allowed' %s % realm`

		`# check if user already exists`
		`c = context.cnx.cursor()`

puiterwijk commented 7 years ago

With this patch, we allow disabling accepting anonymous tickets at Koji, and
for admins to specify which realms are allowed to contact the system in the
case of a trust relationship.
These checks are ignored if a user is explicitly created with the particular
principal, since the checks only happen in the createUserFromKerberos call.

Signed-off-by: Patrick Uiterwijk puiterwijk@redhat.com

rebased

7 years ago

rebased

7 years ago

rebased

7 years ago

ngompa commented 7 years ago

@puiterwijk Is this still needed with gssapi merged into koji?

mikem commented 7 years ago

Rebases to current HEAD just fine

mikem commented 7 years ago

Is there a use case for allowing user =WELLKNOWN/ANONYMOUS? It looks like this would allow anyone to use anon auth and create or log in as that singular user in Koji.

Do we need the allowed_realms enforcement? Seems like that would be something that happens at the httpd layer.

mikem commented on line 12 of koji/auth.py 7 years ago

koji.AuthError('Unparseable principal')

mikem commented 7 years ago

Please use raise Exception('msg') rather than raise Exception, 'msg'

puiterwijk commented 7 years ago

Is there a use case for allowing user =WELLKNOWN/ANONYMOUS? It looks like this would allow anyone to use anon auth and create or log in as that singular user in Koji.

This would be allowed indeed if the current realm allows anonymous connections.
I just didn't want to break anything if people do happen to use it for some crazy reason.

Do we need the allowed_realms enforcement? Seems like that would be something that happens at the httpd layer.

The httpd layer may also accept trusted realms, for example from partners, that you do not want to allow access to the koji installation.

puiterwijk commented 7 years ago

@puiterwijk Is this still needed with gssapi merged into koji?

Not for people that switch to gssapi.
However, not everyone might want to do so.

yulwang commented 4 years ago

https://pagure.io/koji/issue/1557

dgregor commented 4 years ago

This PR is over two years old and would only benefit users that haven't switched to gssapi. I haven't heard of anyone asking for this and suggest that we drop it. @mikem would you be ok with that?

ktdreyer commented 4 years ago

I think it's risky to keep the non-gssapi code around, because it is security sensitive and it will get no attention with Python 2 going EOL. I think we should drop all the non-gssapi Kerberos paths.