From 127b8d9cf23bf65aa42e6ee9ed8d7f8628bbac19 Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy <abokovoy@redhat.com>
Date: Mar 21 2020 05:32:52 +0000
Subject: Prevent adding IPA objects as external members of external groups


The purpose of external groups in FreeIPA is to be able to reference
objects only existing in trusted domains. These members get resolved
through SSSD interfaces but there is nothing that prevents SSSD from
resolving any IPA user or group if they have security identifiers
associated.

Enforce a check that a SID returned by SSSD does not belong to IPA
domain and raise a validation error if this is the case. This would
prevent adding IPA users or groups as external members of an external
group.

RN: Command 'ipa group-add-member' allowed to specify any user or group
RN: for '--external' option. A stricter check is added to verify that
RN: a group or user to be added as an external member does not come
RN: from IPA domain.

Fixes: https://pagure.io/freeipa/issue/8236
Signed-off-by: Alexander Bokovoy <abokovoy@redhat.com>
Reviewed-By: Florence Blanc-Renaud <frenaud@redhat.com>

---

diff --git a/ipaserver/dcerpc.py b/ipaserver/dcerpc.py
index 50e0ec8..27d0754 100644
--- a/ipaserver/dcerpc.py
+++ b/ipaserver/dcerpc.py
@@ -408,7 +408,12 @@ class DomainValidator:
         if object_name in result and \
            (pysss_nss_idmap.SID_KEY in result[object_name]):
             object_sid = result[object_name][pysss_nss_idmap.SID_KEY]
-            return object_sid
+            if self.is_trusted_sid_valid(object_sid):
+                return object_sid
+            else:
+                raise errors.ValidationError(name=_('trusted domain object'),
+                                             error=_('Object does not belong '
+                                                     'to a trusted domain'))
 
         # If fallback to AD DC LDAP is not allowed, bail out
         if not fallback_to_ldap:
diff --git a/ipatests/test_integration/test_sssd.py b/ipatests/test_integration/test_sssd.py
index 69a4771..872863a 100644
--- a/ipatests/test_integration/test_sssd.py
+++ b/ipatests/test_integration/test_sssd.py
@@ -19,6 +19,7 @@ from ipaplatform.tasks import tasks as platform_tasks
 from ipaplatform.osinfo import osinfo
 from ipaplatform.paths import paths
 from ipapython.dn import DN
+from ipalib import errors
 
 
 class TestSSSDWithAdTrust(IntegrationTest):
@@ -329,3 +330,26 @@ class TestSSSDWithAdTrust(IntegrationTest):
         finally:
             self.master.run_command(['ipa', 'user-del', user])
             self.master.run_command(['ipa', 'group-del', user, ext_group])
+
+    @pytest.mark.parametrize('user_origin', ['ipa', 'ad'])
+    def test_external_group_member_mismatch(self, user_origin):
+        """Prevent adding IPA objects as external group external members
+
+        External groups must only allow adding non-IPA objects as external
+        members in 'ipa group-add-member foo --external bar'.
+        """
+        master = self.master
+        tasks.clear_sssd_cache(master)
+        tasks.kinit_admin(master)
+        master.run_command(['ipa', 'group-add', '--external',
+                            'ext-ipatest'])
+        try:
+            master.run_command(['ipa', 'group-add-member',
+                                'ext-ipatest',
+                                '--external',
+                                self.users[user_origin]['name']])
+        except errors.ValidationError:
+            # Only 'ipa' origin should throw a validation error
+            assert user_origin == 'ipa'
+        finally:
+            master.run_command(['ipa', 'group-del', 'ext-ipatest'])