From a5d2857297cfcf87ed8973df96e89ebcef22850d Mon Sep 17 00:00:00 2001

From: Antonio Torres <antorres@redhat.com>

Date: Mon, 8 Mar 2021 18:15:50 +0100

Subject: [PATCH] Add checks to prevent adding auth indicators to internal IPA

 services

Authentication indicators should not be enforced against internal

IPA services, since not all users of those services are able to produce

Kerberos tickets with all the auth indicator options. This includes

host, ldap, HTTP and cifs in IPA server and cifs in IPA clients.

If a client that is being promoted to replica has an auth indicator

in its host principal then the promotion is aborted.

Fixes: https://pagure.io/freeipa/issue/8206

Signed-off-by: Antonio Torres <antorres@redhat.com>

---

 ipaserver/install/server/replicainstall.py | 13 ++++++++++++

 ipaserver/plugins/host.py                  |  5 ++++-

 ipaserver/plugins/service.py               | 24 ++++++++++++++++++++++

 3 files changed, 41 insertions(+), 1 deletion(-)

diff --git a/ipaserver/install/server/replicainstall.py b/ipaserver/install/server/replicainstall.py

index 73967a224..f1fb91036 100644

--- a/ipaserver/install/server/replicainstall.py

+++ b/ipaserver/install/server/replicainstall.py

@@ -770,6 +770,15 @@ def promotion_check_ipa_domain(master_ldap_conn, basedn):

         ))

+def promotion_check_host_principal_auth_ind(conn, hostdn):

+    entry = conn.get_entry(hostdn, ['krbprincipalauthind'])

+    if 'krbprincipalauthind' in entry:

+        raise RuntimeError(

+            "Client cannot be promoted to a replica if the host principal "

+            "has an authentication indicator set."

+        )

+

+

 @common_cleanup

 @preserve_enrollment_state

 def promote_check(installer):

@@ -956,6 +965,10 @@ def promote_check(installer):

                                      config.master_host_name, None)

         promotion_check_ipa_domain(conn, remote_api.env.basedn)

+        hostdn = DN(('fqdn', api.env.host),

+                    api.env.container_host,

+                    api.env.basedn)

+        promotion_check_host_principal_auth_ind(conn, hostdn)

         # Make sure that domain fulfills minimal domain level

         # requirement

diff --git a/ipaserver/plugins/host.py b/ipaserver/plugins/host.py

index eb1f8ef04..41fa933e2 100644

--- a/ipaserver/plugins/host.py

+++ b/ipaserver/plugins/host.py

@@ -38,7 +38,7 @@ from .baseldap import (LDAPQuery, LDAPObject, LDAPCreate,

                                      LDAPAddAttributeViaOption,

                                      LDAPRemoveAttributeViaOption)

 from .service import (

-    validate_realm, normalize_principal,

+    validate_realm, validate_auth_indicator, normalize_principal,

     set_certificate_attrs, ticket_flags_params, update_krbticketflags,

     set_kerberos_attrs, rename_ipaallowedtoperform_from_ldap,

     rename_ipaallowedtoperform_to_ldap, revoke_certs)

@@ -735,6 +735,8 @@ class host_add(LDAPCreate):

         update_krbticketflags(ldap, entry_attrs, attrs_list, options, False)

         if 'krbticketflags' in entry_attrs:

             entry_attrs['objectclass'].append('krbticketpolicyaux')

+        validate_auth_indicator(entry_attrs)

+

         return dn

     def post_callback(self, ldap, dn, entry_attrs, *keys, **options):

@@ -993,6 +995,7 @@ class host_mod(LDAPUpdate):

             if 'krbprincipalaux' not in (item.lower() for item in

                                          entry_attrs['objectclass']):

                 entry_attrs['objectclass'].append('krbprincipalaux')

+            validate_auth_indicator(entry_attrs)

         add_sshpubkey_to_attrs_pre(self.context, attrs_list)

diff --git a/ipaserver/plugins/service.py b/ipaserver/plugins/service.py

index 1c9347804..cfbbff3c6 100644

--- a/ipaserver/plugins/service.py

+++ b/ipaserver/plugins/service.py

@@ -201,6 +201,28 @@ def validate_realm(ugettext, principal):

         raise errors.RealmMismatch()

+def validate_auth_indicator(entry):

+    new_value = entry.get('krbprincipalauthind', None)

+    if not new_value:

+        return

+    # The following services are considered internal IPA services

+    # and shouldn't be allowed to have auth indicators.

+    # https://pagure.io/freeipa/issue/8206

+    pkey = api.Object['service'].get_primary_key_from_dn(entry.dn)

+    principal = kerberos.Principal(pkey)

+    server = api.Command.server_find(principal.hostname)['result']

+    if server:

+        prefixes = ("host", "cifs", "ldap", "HTTP")

+    else:

+        prefixes = ("cifs",)

+    if principal.service_name in prefixes:

+        raise errors.ValidationError(

+            name='krbprincipalauthind',

+            error=_('authentication indicators not allowed '

+                    'in service "%s"' % principal.service_name)

+        )

+

+

 def normalize_principal(value):

     """

     Ensure that the name in the principal is lower-case. The realm is

@@ -652,6 +674,7 @@ class service_add(LDAPCreate):

                     hostname)

         self.obj.validate_ipakrbauthzdata(entry_attrs)

+        validate_auth_indicator(entry_attrs)

         if not options.get('force', False):

             # We know the host exists if we've gotten this far but we

@@ -846,6 +869,7 @@ class service_mod(LDAPUpdate):

         assert isinstance(dn, DN)

         self.obj.validate_ipakrbauthzdata(entry_attrs)

+        validate_auth_indicator(entry_attrs)

         # verify certificates

         certs = entry_attrs.get('usercertificate') or []

-- 

2.31.1

From 28484c3dee225662e41acc691bfe6b1c1cee99c8 Mon Sep 17 00:00:00 2001

From: Antonio Torres <antorres@redhat.com>

Date: Mon, 8 Mar 2021 18:20:35 +0100

Subject: [PATCH] ipatests: ensure auth indicators can't be added to internal

 IPA services

Authentication indicators should not be added to internal IPA services,

since this can lead to a broken IPA setup. In case a client with

an auth indicator set in its host principal, promoting it to a replica

should fail.

Related: https://pagure.io/freeipa/issue/8206

Signed-off-by: Antonio Torres <antorres@redhat.com>

---

 .../test_replica_promotion.py                 | 38 +++++++++++++++++++

 ipatests/test_xmlrpc/test_host_plugin.py      | 10 +++++

 ipatests/test_xmlrpc/test_service_plugin.py   | 21 ++++++++++

 3 files changed, 69 insertions(+)

diff --git a/ipatests/test_integration/test_replica_promotion.py b/ipatests/test_integration/test_replica_promotion.py

index 0a137dbdc..b9c56f775 100644

--- a/ipatests/test_integration/test_replica_promotion.py

+++ b/ipatests/test_integration/test_replica_promotion.py

@@ -101,6 +101,44 @@ class TestReplicaPromotionLevel1(ReplicaPromotionBase):

         assert result.returncode == 1

         assert expected_err in result.stderr_text

+    @replicas_cleanup

+    def test_install_with_host_auth_ind_set(self):

+        """ A client shouldn't be able to be promoted if it has

+        any auth indicator set in the host principal.

+        https://pagure.io/freeipa/issue/8206

+        """

+

+        client = self.replicas[0]

+        # Configure firewall first

+        Firewall(client).enable_services(["freeipa-ldap",

+                                          "freeipa-ldaps"])

+

+        client.run_command(['ipa-client-install', '-U',

+                            '--domain', self.master.domain.name,

+                            '--realm', self.master.domain.realm,

+                            '-p', 'admin',

+                            '-w', self.master.config.admin_password,

+                            '--server', self.master.hostname,

+                            '--force-join'])

+

+        tasks.kinit_admin(client)

+

+        client.run_command(['ipa', 'host-mod', '--auth-ind=otp',

+                            client.hostname])

+

+        res = client.run_command(['ipa-replica-install', '-U', '-w',

+                                  self.master.config.dirman_password],

+                                 raiseonerr=False)

+

+        client.run_command(['ipa', 'host-mod', '--auth-ind=',

+                            client.hostname])

+

+        expected_err = ("Client cannot be promoted to a replica if the host "

+                        "principal has an authentication indicator set.")

+        assert res.returncode == 1

+        assert expected_err in res.stderr_text

+

+

     @replicas_cleanup

     def test_one_command_installation(self):

         """

diff --git a/ipatests/test_xmlrpc/test_host_plugin.py b/ipatests/test_xmlrpc/test_host_plugin.py

index c66bbc865..9cfde3565 100644

--- a/ipatests/test_xmlrpc/test_host_plugin.py

+++ b/ipatests/test_xmlrpc/test_host_plugin.py

@@ -605,6 +605,16 @@ class TestProtectedMaster(XMLRPC_test):

                 error=u'An IPA master host cannot be deleted or disabled')):

             command()

+    def test_try_add_auth_ind_master(self, this_host):

+        command = this_host.make_update_command({

+            u'krbprincipalauthind': u'radius'})

+        with raises_exact(errors.ValidationError(

+            name='krbprincipalauthind',

+            error=u'authentication indicators not allowed '

+                'in service "host"'

+        )):

+            command()

+

 @pytest.mark.tier1

 class TestValidation(XMLRPC_test):

diff --git a/ipatests/test_xmlrpc/test_service_plugin.py b/ipatests/test_xmlrpc/test_service_plugin.py

index 4c845938c..ed634a045 100644

--- a/ipatests/test_xmlrpc/test_service_plugin.py

+++ b/ipatests/test_xmlrpc/test_service_plugin.py

@@ -25,6 +25,7 @@ from ipalib import api, errors

 from ipatests.test_xmlrpc.xmlrpc_test import Declarative, fuzzy_uuid, fuzzy_hash

 from ipatests.test_xmlrpc.xmlrpc_test import fuzzy_digits, fuzzy_date, fuzzy_issuer

 from ipatests.test_xmlrpc.xmlrpc_test import fuzzy_hex, XMLRPC_test

+from ipatests.test_xmlrpc.xmlrpc_test import raises_exact

 from ipatests.test_xmlrpc import objectclasses

 from ipatests.test_xmlrpc.testcert import get_testcert, subject_base

 from ipatests.test_xmlrpc.test_user_plugin import get_user_result, get_group_dn

@@ -1552,6 +1553,15 @@ def indicators_host(request):

     return tracker.make_fixture(request)

+@pytest.fixture(scope='function')

+def this_host(request):

+    """Fixture for the current master"""

+    tracker = HostTracker(name=api.env.host.partition('.')[0],

+                          fqdn=api.env.host)

+    tracker.exists = True

+    return tracker

+

+

 @pytest.fixture(scope='function')

 def indicators_service(request):

     tracker = ServiceTracker(

@@ -1587,6 +1597,17 @@ class TestAuthenticationIndicators(XMLRPC_test):

             expected_updates={u'krbprincipalauthind': [u'radius']}

         )

+    def test_update_indicator_internal_service(self, this_host):

+        command = this_host.make_command('service_mod',

+                                         'ldap/' + this_host.fqdn,

+                                         **dict(krbprincipalauthind='otp'))

+        with raises_exact(errors.ValidationError(

+            name='krbprincipalauthind',

+            error=u'authentication indicators not allowed '

+                 'in service "ldap"'

+        )):

+            command()

+

 @pytest.fixture(scope='function')

 def managing_host(request):

-- 

2.31.1