Hi Alexander, On Thu, Jun 21, 2012 at 06:26:02PM +0300, Alexander Bokovoy wrote: > Hi! > > Attached is the patch to support external group membership for trusted > domains. This is needed to get proper group membership with the work > Sumit and Jan are doing on both IPA and SSSD sides. > > We already have ipaExternalGroup class that includes ipaExternalMember > attribute (multivalued case-insensitive string). The group that has > ipaExternalGroup object class will have to be non-POSIX and > ipaExternalMember > attribute will contain security identifiers (SIDs) of members from > trusted domains. > > The patch takes care of three things: > 1. Extends 'ipa group-add' with --external option to add > ipaExternalGroup object class to a new group > 2. Modifies 'ipa group-add-member' to accept --external CSV argument > to specify SIDs > 3. Modifies 'ipa group-del-member' to allow removing external members.
thank you for the patch, it works as expected, but I have a few comments: - there is a trailing whitespace at the end of the "This means we can't check the correctness of a trusted domain SIDs" line - when using ipa group-add-member with --external there are still prompt for [member user] and [member group], can those be suppressed? - with ipa group-mod --posix it is possible to add the posxiGroup objectclass together with a GID to the extern group object. This should result in an error and also the other way round, adding --external to Posix groups. bye, Sumit > > When adding new external member we also perform SID correctness checks. > This is important part of the patch due to potential security > implications of allowing random SIDs. SIDs are universal identifiers and > can point to objects in own domain as well as any other. If so-called > builtin SIDs are used, they are resolved against local domain which will > allow granting permissions trusted domain user should have never had. > > Below is how we do perform validation of SIDs: > 1. Use Samba 4 bindings to parse SID and validate its format > 2. If SID is outside S-1-5- prefix (SID_NT_AUTHORITY), we reject it. > 3. If SID is from our own domain, we reject it. > 4. If SID is from any of our trusted domains, we accept it > 5. Otherwise we reject SID. > > Here is real code: > + def is_trusted_sid_valid(self, sid): > + if not self.domain: > + # our domain is not configured or self.is_configured() never run > + # reject SIDs as we can't check correctness of them > + return False > + # Parse sid string to see if it is really in a SID format > + try: > + test_sid = security.dom_sid(sid) > + except TypeError: > + return False > + (dom, sid_rid) = test_sid.split() > + sid_dom = str(dom) > + # Now we have domain prefix of the sid as sid_dom string and can > + # analyze it against known prefixes > + if sid_dom.find(security.SID_NT_AUTHORITY) != 0: > + # Ignore any potential SIDs that are not S-1-5-* > + return False > + if sid_dom.find(self.sid) == 0: > + # A SID from our own domain cannot be treated as trusted > domain's SID > + return False > + # At this point we have SID_NT_AUTHORITY family SID and really need > to > + # check it against prefixes of domain SIDs we trust to > + if not self._domains: > + self._domains = self.get_trusted_domains() > + if len(self._domains) == 0: > + # Our domain is configured but no trusted domains are configured > + # This means we can't check the correctness of a > trusted domain SIDs + return False > + # We have non-zero list of trusted domains and have to go through > them > + # one by one and check their sids as prefixes > + for (dn, domaininfo) in self._domains: > + if sid_dom.find(domaininfo[self.ATTR_TRUSTED_SID][0]) == 0: > + return True > + return False > > > > -- > / Alexander Bokovoy _______________________________________________ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel