#1419 checking kerberos prinicipal instead of username in GSSAPI authentication
Merged 2 years ago by tkopecek. Opened 2 years ago by julian8628.
julian8628/koji issue/1400  into  master

file modified
+10
@@ -55,3 +55,13 @@ 

  # In this case, you will need to enable these options globally (in ssl.conf):

  # SSLVerifyClient require

  # SSLVerifyDepth  10

+ 

+ # uncomment this to enable authentication via GSSAPI

+ # <Location /kojihub/ssllogin>

+ #         AuthType GSSAPI

+ #         GssapiSSLonly Off

+ #         GssapiLocalName Off

+ #         AuthName "GSSAPI Single Sign On Login"

+ #         GssapiCredStore keytab:/etc/koji.keytab

+ #         Require valid-user

+ # </Location>

file modified
+2
@@ -25,6 +25,8 @@ 

  ## Allowed Kerberos Realms separated by ','.

  ## Default value "*" indicates any Realm is allowed

  # AllowedKrbRealms = *

+ ## TODO: this option should be removed in future release

+ # DisableGSSAPIProxyDNFallback = False

  

  ## end Kerberos auth configuration

  

file modified
+2
@@ -422,6 +422,8 @@ 

          ['ProxyPrincipals', 'string', ''],

          ['HostPrincipalFormat', 'string', None],

          ['AllowedKrbRealms', 'string', '*'],

+         # TODO: this option should be removed in future release

+         ['DisableGSSAPIProxyDNFallback', 'boolean', False],

  

          ['DNUsernameComponent', 'string', 'CN'],

          ['ProxyDNs', 'string', ''],

file modified
+27 -4
@@ -398,7 +398,9 @@ 

          if self.logged_in:

              raise koji.AuthError("Already logged in")

  

+         # we use REMOTE_USER to identify user

          if context.environ.get('REMOTE_USER'):

+             # it is kerberos principal rather than user's name.

              username = context.environ.get('REMOTE_USER')

              client_dn = username

              authtype = koji.AUTHTYPE_GSSAPI
@@ -414,17 +416,38 @@ 

              authtype = koji.AUTHTYPE_SSL

  

          if proxyuser:

-             proxy_dns = [dn.strip() for dn in context.opts.get('ProxyDNs', '').split('|')]

+             if authtype == koji.AUTHTYPE_GSSAPI:

+                 delimiter = ','

+                 proxy_opt = 'ProxyPrincipals'

+             else:

+                 delimiter = '|'

+                 proxy_opt = 'ProxyDNs'

+             proxy_dns = [dn.strip() for dn in context.opts.get(proxy_opt, '').split(delimiter)]

+ 

+             # backwards compatible for GSSAPI.

+             # in old way, proxy user whitelist is ProxyDNs.

+             # TODO: this should be removed in future release

+             if authtype == koji.AUTHTYPE_GSSAPI and not context.opts.get(

+                     'DisableGSSAPIProxyDNFallback', False):

+                 proxy_dns += [dn.strip() for dn in

+                               context.opts.get('ProxyDNs', '').split('|')]

+ 

              if client_dn in proxy_dns:

-                 # the SSL-authenticated user authorized to login other users

+                 # the user authorized to login other users

                  username = proxyuser

              else:

                  raise koji.AuthError('%s is not authorized to login other users' % client_dn)

  

-         user_id = self.getUserId(username)

+         if authtype == koji.AUTHTYPE_GSSAPI and '@' in username:

+             user_id = self.getUserIdFromKerberos(username)

+         else:

+             user_id = self.getUserId(username)

          if not user_id:

              if context.opts.get('LoginCreatesUser'):

-                 user_id = self.createUser(username)

+                 if authtype == koji.AUTHTYPE_GSSAPI and '@' in username:

+                     user_id = self.createUserFromKerberos(username)

+                 else:

+                     user_id = self.createUser(username)

              else:

                  raise koji.AuthError('Unknown user: %s' % username)

  

fixes: #1400

  • when using GSSAPI auth, the value of REMOTE_USER should be full principal with realm
  • for GSSAPI, whitelist of proxy users' principal is configured by ProxyPrincipals rather than ProxyDNs

This requires configs below in /etc/krb5.conf on hub

[realms]
 EXAMPLE.COM = {
  ...
  auth_to_local = RULE:[1:$1@$0](.*@IPA\.EXAMPLE\.COM)s/@.*/@EXAMPLE.COM/
  auth_to_local = RULE:[1:$1@$0](.*@EXAMPLE\.COM)
  auth_to_local = RULE:[2:$1/$2@$0](.*@IPA\.EXAMPLE\.COM)s/@.*/@EXAMPLE.COM/
  auth_to_local = RULE:[2:$1/$2@$0](.*@EXAMPLE\.COM)
  ...
 }

1 new commit added

  • adding some notes in documents
2 years ago

added some content in documents

@mikem my only concern would be existing setups using GSSAPI on upgrade.
(Note that these changes would not be a problem for Fedora, and actually be quite nice, since right now we have a patch to use the getUserIdFromKerberos method.)

Since this changes the way users are looked up and created, I am concerned that existing setups might need to adapt settings suddenly to remain working (proxyDNs), or need to fix the kerberos principal values of existing users (especially if the krb5.conf changes are also applied).
I think that we will definitely need clear information about this in release notes.

Additionally, I would say that the change from ProxyDNs -> ProxyPrincipals might even be backwards incompatible, which would prevent it from being applied in non-rawhide and any EPEL releases, even if I do think it makes sense to make the change to bring it in line with krb5 auth.

Perhaps it would be an idea to add some code that:

if authtype == koji.AUTHTYPE_GSSAPI and '@' in username and not context.opts.get('DisableGSSAPIProxyDNFallback', False):
    proxy_dns += [dn.strip() for dn in context.opts.get('ProxyDNs', '').split('|')]

1 new commit added

  • backwards compatibility for ProxyDNs change
2 years ago

Update for ProxyDNs compatibility.
I didn't apply condition '@' in username, because I can't presume if REMOTE_USER is a principal or a username. it depends on httpd and krb5 configs.

With that, I see no concerns, and would be really excited to see this land!

@julian8628++

I would prefer to see this implemented without requiring GssapiLocalName On and without requiring any auth_to_local krb5.conf modifications. Principals for kojira, koji-builder, koji-gc, etc. don't need to map to local users.

On Fedora's instances where the Koji hub is probably a whole machine, it's no biggie to mess with the system krb5.conf, but for smaller users where the hub will reside on an Apache server that does many other things, keeping Koji containted to Koji is a good thing.

After my hub's upgrade to F30 (Python3) and after digging through /usr/lib/python3.7/site-packages/koji/auth.py, the following temporarily allows things to work. While it's hopefully just a temporary hack, something like this (even as you describe above), would allow Koji to continue to use service-style principals without mapping to local users.

In fact, it comes to mind that the users table probably no longer needs to distinguish between name and krb_principal since in F30 with mod_auth_gssapi, all the authentication is delegated to Apache, rather than Koji internal. The user column could just contain the full Kerberos principal name and if folks wanted shorter usernames or hostnames on the Koji-web display, the display code could strip off things after / or @ instead of the auth code.

--- /usr/lib/python3.7/site-packages/koji/auth.py.orig  2019-03-06 08:37:08.000000000 -0600
+++ /usr/lib/python3.7/site-packages/koji/auth.py       2019-05-11 18:55:55.586628816 -0500
@@ -393,6 +393,14 @@

         if context.environ.get('REMOTE_USER'):
             username = context.environ.get('REMOTE_USER')
+
+            if username and '@' in username:
+                username = username.partition('@')[0]
+                if username.startswith('compile/'):
+                    username = username.partition('/')[-1]
+                else:
+                    username = username.partition('/')[0]
+
             client_dn = username
             authtype = koji.AUTHTYPE_GSSAPI
         else:
@@ -410,7 +418,7 @@
             proxy_dns = [dn.strip() for dn in context.opts.get('ProxyDNs', '').split('|')]
             if client_dn in proxy_dns:
                 # the SSL-authenticated user authorized to login other users
-                username = proxyuser
+                username = proxyuser.partition('@')[0]
             else:
                 raise koji.AuthError('%s is not authorized to login other users' % client_dn)

Note I've been doing something like this for a while; please review https://pagure.io/koji/issue/249.

# Use GSSAPI for hub authentication
# https://pagure.io/koji/issue/249#comment-71013
<Location /kojihub/ssllogin>
    AuthType GSSAPI
    AuthName "Koji Hub"
    GssapiAllowedMech krb5
#    GssapiLocalName On
    GssapiNegotiateOnce On
    GssapiSSLonly On

    <RequireAll>
        Require env INTERNAL_NET
        <RequireAny>
            # Service principles for Kojira, the builder(s), and maintenance script
            Require user compile/builder1.example.com@EXAMPLE.COM koji/example.com@EXAMPLE.COM koji-maint/example.com@EXAMPLE.COM
            # FreeIPA HBAC for other "users"
            Require pam-account http-ipa-packagers
        </RequireAny>
    </RequireAll>
</Location>

Of course, temporarily, I did need to update ProxyDNs, but that could probably be combined with ProxyPrincipals in the code too.

ProxyDNs = koji

Metadata Update from @tkopecek:
- Pull-request tagged with: testing-ready

2 years ago

@amessina
It looks still possible(barely happening) to meet the problem described in #1400 by your patch and configuration if only using username for authentication. Because it is assumed that the first part of the krb principal is exactly the username.
But I do agree that koji GSSAPI auth would better to support GssapiLocalName Off, in terms of compatibility.
Maybe adding a switch in hub.conf as well and when it's off, validating if krb_principal matches env var GSS_NAME by some pattern.

@tkopecek @mikem any advice on this?

with PR #1648, simply setting GssapiLocalName Off for mod_gssapi is supported as well.
Multi-realm for one single user doesn't require realm mapping on system level(/etc/krb5.conf)

rebased onto 4a3dc40

2 years ago

Commit 8a0c39f fixes this pull-request

Pull-Request has been merged by tkopecek

2 years ago