Modify MBS to use a separate Kerberos context per thread so both threads can use the thread keyring to store the Kerberos cache

This commit includes the backport of the changes to `krb_login` in
https://pagure.io/koji/pull-request/1187. This change is required
for our separate threads to use a separate Kerberos context per thread.

module_build_service/__init__.py

@@ -41,7 +41,6 @@ for a number of tasks:
 """
 
 import pkg_resources
-import os
 from flask import Flask, has_app_context, url_for
 from flask_sqlalchemy import SQLAlchemy
 from sqlalchemy.pool import StaticPool
@@ -70,9 +69,6 @@ app.wsgi_app = ReverseProxy(app.wsgi_app)
 
 conf = init_config(app)
 
-# We want to use a separate Kerberos cache per thread to avoid Kerberos cache corruption
-os.environ['KRB5CCNAME'] = 'KEYRING:thread:mbs'
-
 
 class MBSSQLAlchemy(SQLAlchemy):
     """

module_build_service/builder/KojiModuleBuilder.py

@@ -49,6 +49,7 @@ from module_build_service import log, conf, models
 import module_build_service.scm
 import module_build_service.utils
 from module_build_service.builder.utils import execute_cmd
+from module_build_service.builder.koji_backports import ClientSession as KojiClientSession
 from module_build_service.errors import ProgrammingError
 
 from module_build_service.builder.base import GenericBuilder
@@ -455,7 +456,7 @@ chmod 644 %buildroot/etc/rpm/macros.zz-modules
 
         address = koji_config.server
         log.info("Connecting to koji %r.", address)
-        koji_session = koji.ClientSession(address, opts=koji_config)
+        koji_session = KojiClientSession(address, opts=koji_config)
 
         if not login:
             return koji_session
@@ -463,13 +464,23 @@ chmod 644 %buildroot/etc/rpm/macros.zz-modules
         authtype = koji_config.authtype
         log.info("Authenticate session with %r.", authtype)
         if authtype == "kerberos":
+            try:
+                import krbV
+            except ImportError:
+                raise RuntimeError(
+                    "python-krbV must be installed to authenticate with Koji using Kerberos")
             keytab = getattr(config, "krb_keytab", None)
             principal = getattr(config, "krb_principal", None)
             if not keytab and principal:
                 raise ValueError(
                     "The Kerberos keytab and principal aren't set for Koji authentication")
             log.debug("  keytab: %r, principal: %r" % (keytab, principal))
-            koji_session.krb_login(principal=principal, keytab=keytab)
+            # We want to create a context per thread to avoid Kerberos cache corruption
+            ctx = krbV.Context()
+            # We want to use the thread keyring for the ccache to ensure we have one cache per
+            # thread to avoid Kerberos cache corruption
+            ccache = "KEYRING:thread:mbs"
+            koji_session.krb_login(principal=principal, keytab=keytab, ctx=ctx, ccache=ccache)
         elif authtype == "ssl":
             koji_session.ssl_login(
                 os.path.expanduser(koji_config.cert),

module_build_service/builder/koji_backports.py

@@ -0,0 +1,98 @@
+# flake8: noqa
+import base64
+import traceback
+
+import koji
+# Import krbV from here so we don't have to redo the whole try except that Koji does
+from koji import krbV, PythonImportError, AuthError, AUTHTYPE_KERB
+
+
+class ClientSession(koji.ClientSession):
+    """The koji.ClientSession class with patches from upstream."""
+
+    # This backport comes from https://pagure.io/koji/pull-request/1187
+    def krb_login(self, principal=None, keytab=None, ccache=None, proxyuser=None, ctx=None):
+        """Log in using Kerberos.  If principal is not None and keytab is
+        not None, then get credentials for the given principal from the given keytab.
+        If both are None, authenticate using existing local credentials (as obtained
+        from kinit).  ccache is the absolute path to use for the credential cache. If
+        not specified, the default ccache will be used.  If proxyuser is specified,
+        log in the given user instead of the user associated with the Kerberos
+        principal.  The principal must be in the "ProxyPrincipals" list on
+        the server side.  ctx is the Kerberos context to use, and should be unique
+        per thread.  If ctx is not specified, the default context is used."""
+
+        try:
+            # Silently try GSSAPI first
+            if self.gssapi_login(principal, keytab, ccache, proxyuser=proxyuser):
+                return True
+        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)
+                pass
+            else:
+                raise
+
+        if not krbV:
+            raise PythonImportError(
+                "Please install python-krbV to use kerberos."
+            )
+
+        if not ctx:
+            ctx = krbV.default_context()
+
+        if ccache != None:
+            ccache = krbV.CCache(name=ccache, context=ctx)
+        else:
+            ccache = ctx.default_ccache()
+
+        if principal != None:
+            if keytab != None:
+                cprinc = krbV.Principal(name=principal, context=ctx)
+                keytab = krbV.Keytab(name=keytab, context=ctx)
+                ccache.init(cprinc)
+                ccache.init_creds_keytab(principal=cprinc, keytab=keytab)
+            else:
+                raise AuthError('cannot specify a principal without a keytab')
+        else:
+            # We're trying to log ourself in.  Connect using existing credentials.
+            cprinc = ccache.principal()
+
+        self.logger.debug('Authenticating as: %s', cprinc.name)
+        sprinc = krbV.Principal(name=self._serverPrincipal(cprinc), context=ctx)
+
+        ac = krbV.AuthContext(context=ctx)
+        ac.flags = krbV.KRB5_AUTH_CONTEXT_DO_SEQUENCE | krbV.KRB5_AUTH_CONTEXT_DO_TIME
+        ac.rcache = ctx.default_rcache()
+
+        # create and encode the authentication request
+        (ac, req) = ctx.mk_req(server=sprinc, client=cprinc,
+                               auth_context=ac, ccache=ccache,
+                               options=krbV.AP_OPTS_MUTUAL_REQUIRED)
+        req_enc = base64.encodestring(req)
+
+        # ask the server to authenticate us
+        (rep_enc, sinfo_enc, addrinfo) = self.callMethod('krbLogin', req_enc, proxyuser)
+        # Set the addrinfo we received from the server
+        # (necessary before calling rd_priv())
+        # addrinfo is in (serveraddr, serverport, clientaddr, clientport)
+        # format, so swap the pairs because clientaddr is now the local addr
+        ac.addrs = tuple((addrinfo[2], addrinfo[3], addrinfo[0], addrinfo[1]))
+
+        # decode and read the reply from the server
+        rep = base64.decodestring(rep_enc)
+        ctx.rd_rep(rep, auth_context=ac)
+
+        # decode and decrypt the login info
+        sinfo_priv = base64.decodestring(sinfo_enc)
+        sinfo_str = ac.rd_priv(sinfo_priv)
+        sinfo = dict(zip(['session-id', 'session-key'], sinfo_str.split()))
+
+        if not sinfo:
+            self.logger.warn('No session info received')
+            return False
+        self.setSession(sinfo)
+
+        self.authtype = AUTHTYPE_KERB
+        return True

tests/test_builder/test_koji.py

@@ -324,7 +324,7 @@ class TestKojiBuilder:
         expected_calls = [mock.call(1, 'foo'), mock.call(2, 'foo'), mock.call(1, 'bar')]
         assert mock_session.untagBuild.mock_calls == expected_calls
 
-    @patch('koji.ClientSession')
+    @patch('module_build_service.builder.KojiModuleBuilder.KojiClientSession')
     def test_get_build_weights(self, ClientSession):
         session = ClientSession.return_value
         session.getLoggedInUser.return_value = {"id": 123}
@@ -347,7 +347,7 @@ class TestKojiBuilder:
         # getLoggedInUser requires to a logged-in session
         session.krb_login.assert_called_once()
 
-    @patch('koji.ClientSession')
+    @patch('module_build_service.builder.KojiModuleBuilder.KojiClientSession')
     def test_get_build_weights_no_task_id(self, ClientSession):
         session = ClientSession.return_value
         session.getLoggedInUser.return_value = {"id": 123}
@@ -368,7 +368,7 @@ class TestKojiBuilder:
         assert session.getTaskDescendents.mock_calls == expected_calls
         session.krb_login.assert_called_once()
 
-    @patch('koji.ClientSession')
+    @patch('module_build_service.builder.KojiModuleBuilder.KojiClientSession')
     def test_get_build_weights_no_build(self, ClientSession):
         session = ClientSession.return_value
         session.getLoggedInUser.return_value = {"id": 123}
@@ -389,7 +389,7 @@ class TestKojiBuilder:
         assert session.getTaskDescendents.mock_calls == expected_calls
         session.krb_login.assert_called_once()
 
-    @patch('koji.ClientSession')
+    @patch('module_build_service.builder.KojiModuleBuilder.KojiClientSession')
     def test_get_build_weights_listBuilds_failed(self, ClientSession):
         session = ClientSession.return_value
         session.getLoggedInUser.return_value = {"id": 123}
@@ -406,7 +406,7 @@ class TestKojiBuilder:
         assert session.listBuilds.mock_calls == expected_calls
         session.krb_login.assert_called_once()
 
-    @patch('koji.ClientSession')
+    @patch('module_build_service.builder.KojiModuleBuilder.KojiClientSession')
     def test_get_build_weights_getPackageID_failed(self, ClientSession):
         session = ClientSession.return_value
         session.getLoggedInUser.return_value = {"id": 123}
@@ -421,7 +421,7 @@ class TestKojiBuilder:
 
         session.krb_login.assert_called_once()
 
-    @patch('koji.ClientSession')
+    @patch('module_build_service.builder.KojiModuleBuilder.KojiClientSession')
     def test_get_build_weights_getLoggedInUser_failed(self, ClientSession):
         session = ClientSession.return_value
         session.getAverageBuildDuration.return_value = None
@@ -546,7 +546,7 @@ class TestKojiBuilder:
             expected_calls = []
         assert session.packageListBlock.mock_calls == expected_calls
 
-    @patch('koji.ClientSession')
+    @patch('module_build_service.builder.KojiModuleBuilder.KojiClientSession')
     def test_get_built_rpms_in_module_build(self, ClientSession):
         session = ClientSession.return_value
         session.listTaggedRPMS.return_value = ([
@@ -601,7 +601,7 @@ class TestKojiBuilder:
             []
         ),
     ))
-    @patch('koji.ClientSession')
+    @patch('module_build_service.builder.KojiModuleBuilder.KojiClientSession')
     def test_get_filtered_rpms_on_self_dep(self, ClientSession, br_filtered_rpms, expected):
         session = ClientSession.return_value
         session.listTaggedRPMS.return_value = (
@@ -679,14 +679,14 @@ class TestKojiBuilder:
         else:
             mock_koji_cg.koji_import.assert_not_called()
 
-    @patch('koji.ClientSession')
+    @patch('module_build_service.builder.KojiModuleBuilder.KojiClientSession')
     def test_get_anonymous_session(self, ClientSession):
         mbs_config = mock.Mock(koji_profile='koji', koji_config='conf/koji.conf')
         session = KojiModuleBuilder.get_session(mbs_config, login=False)
         assert ClientSession.return_value == session
         assert ClientSession.return_value.krb_login.assert_not_called
 
-    @patch('koji.ClientSession')
+    @patch('module_build_service.builder.KojiModuleBuilder.KojiClientSession')
     def test_ensure_builder_use_a_logged_in_koji_session(self, ClientSession):
         builder = KojiModuleBuilder('owner', MagicMock(), conf, 'module-tag', [])
         builder.koji_session.krb_login.assert_called_once()

tests/test_content_generator.py

@@ -85,7 +85,7 @@ class TestBuild:
         except OSError:
             pass
 
-    @patch("koji.ClientSession")
+    @patch("module_build_service.builder.KojiModuleBuilder.KojiClientSession")
     @patch("subprocess.Popen")
     @patch("subprocess.check_output", return_value='1.4')
     @patch("pkg_resources.get_distribution")
@@ -142,7 +142,7 @@ class TestBuild:
         # Ensure an anonymous Koji session works
         koji_session.krb_login.assert_not_called()
 
-    @patch("koji.ClientSession")
+    @patch("module_build_service.builder.KojiModuleBuilder.KojiClientSession")
     @patch("subprocess.Popen")
     @patch("subprocess.check_output", return_value='1.4')
     @patch("pkg_resources.get_distribution")
@@ -206,7 +206,7 @@ class TestBuild:
         with open(path.join(dir_path, "modulemd.i686.txt")) as mmd:
             assert len(mmd.read()) == 255
 
-    @patch("koji.ClientSession")
+    @patch("module_build_service.builder.KojiModuleBuilder.KojiClientSession")
     def test_tag_cg_build(self, ClientSession):
         """ Test that the CG build is tagged. """
         koji_session = ClientSession.return_value
@@ -221,7 +221,7 @@ class TestBuild:
         # tagBuild requires logging into a session in advance.
         koji_session.krb_login.assert_called_once()
 
-    @patch("koji.ClientSession")
+    @patch("module_build_service.builder.KojiModuleBuilder.KojiClientSession")
     def test_tag_cg_build_fallback_to_default_tag(self, ClientSession):
         """ Test that the CG build is tagged to default tag. """
         koji_session = ClientSession.return_value
@@ -238,7 +238,7 @@ class TestBuild:
         # tagBuild requires logging into a session in advance.
         koji_session.krb_login.assert_called_once()
 
-    @patch("koji.ClientSession")
+    @patch("module_build_service.builder.KojiModuleBuilder.KojiClientSession")
     def test_tag_cg_build_no_tag_set(self, ClientSession):
         """ Test that the CG build is not tagged when no tag set. """
         koji_session = ClientSession.return_value
@@ -252,7 +252,7 @@ class TestBuild:
         # tagBuild requires logging into a session in advance.
         koji_session.krb_login.assert_called_once()
 
-    @patch("koji.ClientSession")
+    @patch("module_build_service.builder.KojiModuleBuilder.KojiClientSession")
     def test_tag_cg_build_no_tag_available(self, ClientSession):
         """ Test that the CG build is not tagged when no tag available. """
         koji_session = ClientSession.return_value
@@ -332,7 +332,7 @@ class TestBuild:
             'type': 'file'
         }
 
-    @patch("koji.ClientSession")
+    @patch("module_build_service.builder.KojiModuleBuilder.KojiClientSession")
     def test_koji_rpms_in_tag(self, ClientSession):
         koji_session = ClientSession.return_value
         koji_session.getUser.return_value = GET_USER_RV
@@ -420,7 +420,7 @@ class TestBuild:
         # Listing tagged RPMs does not require to log into a session
         koji_session.krb_login.assert_not_called()
 
-    @patch("koji.ClientSession")
+    @patch("module_build_service.builder.KojiModuleBuilder.KojiClientSession")
     def test_koji_rpms_in_tag_empty_tag(self, ClientSession):
         koji_session = ClientSession.return_value
         koji_session.getUser.return_value = GET_USER_RV
@@ -432,7 +432,7 @@ class TestBuild:
         assert rpms == []
         koji_session.multiCall.assert_not_called()
 
-    @patch("koji.ClientSession")
+    @patch("module_build_service.builder.KojiModuleBuilder.KojiClientSession")
     def test_koji_rpms_in_tag_empty_headers(self, ClientSession):
         koji_session = ClientSession.return_value
         koji_session.getUser.return_value = GET_USER_RV

tests/test_scheduler/test_poller.py

@@ -98,7 +98,7 @@ class TestPoller:
 
         assert len(start_build_component.mock_calls) == expected_build_calls
 
-    @patch("koji.ClientSession")
+    @patch("module_build_service.builder.KojiModuleBuilder.KojiClientSession")
     def test_trigger_new_repo_when_failed(
             self, ClientSession, create_builder, global_consumer, dbg):
         """
@@ -131,7 +131,7 @@ class TestPoller:
         koji_session.newRepo.assert_called_once_with(
             "module-testmodule-master-20170219191323-c40c156c-build")
 
-    @patch('koji.ClientSession')
+    @patch('module_build_service.builder.KojiModuleBuilder.KojiClientSession')
     def test_trigger_new_repo_when_succeeded(
             self, ClientSession, create_builder, global_consumer, dbg):
         """
@@ -203,7 +203,7 @@ class TestPoller:
         for component in components:
             assert component.state is None
 
-    @patch('koji.ClientSession')
+    @patch('module_build_service.builder.KojiModuleBuilder.KojiClientSession')
     def test_old_build_targets_are_not_associated_with_any_module_builds(
             self, ClientSession, create_builder, global_consumer, dbg):
         consumer = mock.MagicMock()
@@ -223,7 +223,7 @@ class TestPoller:
 
         koji_session.deleteBuildTarget.assert_not_called()
 
-    @patch('koji.ClientSession')
+    @patch('module_build_service.builder.KojiModuleBuilder.KojiClientSession')
     def test_dont_delete_base_module_build_target(
             self, ClientSession, create_builder, global_consumer, dbg):
         module_build = models.ModuleBuild.query.filter_by(id=3).one()
@@ -248,7 +248,7 @@ class TestPoller:
 
             koji_session.deleteBuildTarget.assert_not_called()
 
-    @patch('koji.ClientSession')
+    @patch('module_build_service.builder.KojiModuleBuilder.KojiClientSession')
     def test_dont_delete_build_target_for_unfinished_module_builds(
             self, ClientSession, create_builder, global_consumer, dbg):
         module_build = models.ModuleBuild.query.filter_by(id=3).one()
@@ -275,7 +275,7 @@ class TestPoller:
 
             koji_session.deleteBuildTarget.assert_not_called()
 
-    @patch('koji.ClientSession')
+    @patch('module_build_service.builder.KojiModuleBuilder.KojiClientSession')
     def test_only_delete_build_target_with_allowed_koji_tag_prefix(
             self, ClientSession, create_builder, global_consumer, dbg):
         module_build_2 = models.ModuleBuild.query.filter_by(id=2).one()
@@ -320,7 +320,7 @@ class TestPoller:
             koji_session.deleteBuildTarget.assert_called_once_with(1)
             koji_session.krb_login.assert_called_once()
 
-    @patch('koji.ClientSession')
+    @patch('module_build_service.builder.KojiModuleBuilder.KojiClientSession')
     def test_cant_delete_build_target_if_not_reach_delete_time(
             self, ClientSession, create_builder, global_consumer, dbg):
         module_build_2 = models.ModuleBuild.query.filter_by(id=2).one()

tests/test_utils/test_ursine.py

@@ -121,7 +121,7 @@ class TestGetModulemdsFromUrsineContent:
     def teardown_method(self, test_method):
         clean_database()
 
-    @patch('koji.ClientSession')
+    @patch('module_build_service.builder.KojiModuleBuilder.KojiClientSession')
     def test_return_empty_if_no_ursine_build_tag_is_found(self, ClientSession):
         session = ClientSession.return_value
 
@@ -141,7 +141,7 @@ class TestGetModulemdsFromUrsineContent:
         assert [] == modulemds
 
     @patch.object(conf, 'koji_tag_prefixes', new=['module'])
-    @patch('koji.ClientSession')
+    @patch('module_build_service.builder.KojiModuleBuilder.KojiClientSession')
     def test_get_modulemds(self, ClientSession):
         session = ClientSession.return_value
 
@@ -246,7 +246,7 @@ class TestRecordStreamCollisionModules:
     @patch.object(conf, 'base_module_names', new=['platform', 'project-platform'])
     @patch('module_build_service.utils.ursine.get_modulemds_from_ursine_content')
     @patch('module_build_service.resolver.GenericResolver.create')
-    @patch('koji.ClientSession')
+    @patch('module_build_service.builder.KojiModuleBuilder.KojiClientSession')
     def test_add_collision_modules(self, ClientSession, resolver_create,
                                    get_modulemds_from_ursine_content):
         xmd = {

tests/test_views/test_views.py

@@ -417,7 +417,7 @@ class TestViews:
                 for key, part in zip(nsvc_keys, nsvc_parts):
                     assert item[key] == part
 
-    @patch("koji.ClientSession")
+    @patch("module_build_service.builder.KojiModuleBuilder.KojiClientSession")
     def test_query_builds_with_binary_rpm(self, ClientSession):
         """
         Test for querying MBS with the binary rpm filename. MBS should return all the modules,