[SSSD] How to map primaryGID?

Stephen Gallagher sgallagh at redhat.com
Mon Apr 23 13:09:15 UTC 2012


On Mon, 2012-04-23 at 14:59 +0200, Jakub Hrozek wrote:
> On Sun, Apr 22, 2012 at 03:43:16PM -0400, Stephen Gallagher wrote:
> > On Sun, 2012-04-22 at 15:10 -0400, Stephen Gallagher wrote:
> > > Ok, I just hit a snag and I'm not sure how best to proceed. All users on
> > > a POSIX system need to have a default GID value, which in most cases is
> > > mapped to a user-private group to help avoid accidental permission-leaks
> > > when that user creates files.
> > > 
> > > However, when mapping a user from Active Directory's objectSID, we don't
> > > have an obvious group to which we can map the primaryGID. I'm not sure
> > > how best to proceed here.
> > > 
> > > One option is to map users' primaryGID to the special group "Domain
> > > Users" to which all AD users belong, but that runs the risk of
> > > reintroducing the above-mentioned permission leaks. I don't really have
> > > any other ideas here, though. Recommendations welcome.
> > 
> > 
> > I've got another set of (less) preliminary patches available now. I've
> > broken the patches up a bit more, so each one is easier to review on its
> > own. I dropped the useless SAFEALIGN macros as Simo suggested. I also
> > implemented a compatibility mode that will behave the same way that
> > autorid does (with one default domain and all other domains assigned a
> > slot as first-come, first-served).
> > 
> > These patches are more or less functionally complete now (excepting the
> > primaryGID question above), but they are still lacking in documentation
> > and options added to the SSSDConfig API. That will follow in the final
> > version.
> > 
> > Patch 0001: Add objectSID config option
> 
> Ack (I know these are preliminary patches, though)
> 
> > 
> > Patch 0002: Helper function to convert AD's binary blob to a 'struct
> > dom_sid'. This has moved since the previous patch set to ldap_common.c
> > so it can be used by both user and group calls.
> 
> Ack, but what does this mean?
> 
> +}/* [noprint,gensize,nopull,public,nopush,nosize] */;

I took that straight from the Samba sources. I don't know what it means
either :) I'll remove it.

> 
> It seems to be some meta data after definition of struct dom_sid.
> 
> > 
> > Patch 0003: Helper function to convert a 'struct dom_sid' to the string
> > representation. This has moved since the previous patch set to
> > ldap_common.c so it can be used by both user and group calls. It still
> > retains the 64-bit limitation discussed in the earlier email. So far
> > I've seen no great benefit to extending it to the full 96-bits (doesn't
> > seem to be used).
> > 
> 
> Ack
> 
> > Patch 0004: Add a boolean option to enable ID mapping
> > 
> 
> Ack
> 
> > Patch 0005: Add some new sysdb routines to manage saving ID mappings in
> > the cache so that they will persist between restarts of SSSD.
> > 
> 
> Ack
> 
> > Patch 0006: Add helper routines to the LDAP provider to handle adding
> > domains and initializing the maps at startup. This function implements
> > the murmurhash3-based autorid implementation.
> 
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -1085,6 +1085,8 @@ libsss_ldap_common_la_SOURCES = \
>      src/providers/ldap/sdap_child_helpers.c \
>      src/providers/ldap/sdap_fd_events.c \
>      src/providers/ldap/sdap_id_op.c \
> +    src/providers/ldap/sdap_idmap.c \
> +    src/providers/ldap/sdap_idmap.h \
>      src/providers/ldap/sdap.c
> ^^^ I think that only idmap.c belongs to _SOURCES and idmap.h belongs
> to noinst_HEADERS
> 

This is a place I've been meaning to talk about. Our use of
noinst_HEADERS is non-standard. It works, but it's not really the way
other autotools packages lay things out. I suppose I'll move this to
make it consistent, though.

> Should we also add a safeguard to avoid division by zero when the user
> sets the range size to 0?
> 

Good idea, I'll do that.

> 
> > 
> > Patch 0007: Add options to configure the ranges. As Simo requested, this
> > sets up 10,000 slices of up to 200,000 IDs by default.
> > 
> 
> Ack, but patch 0006 already uses some the options defined here, so the
> patch order seems to be reversed.
> 

I'm not going to worry about the ordering, as this set of patches is
highly inter-dependent. Mostly I broke these up to be easily-reviewable,
rather than necessarily be able to apply in order.

I have a few more patches coming that will be applying atop these rather
than amending them because it will be too difficult to rewrite history
to accommodate them.

> The options are also not defined in the configAPI.

Yeah, known issue. I'll fix that as I write manpages.

> 
> > Patch 0008: Initialize the sdap_idmap when ID-mapping is enabled.
> 
> Ack
> 
> > 
> > Patch 0009: Look up users with ID-mapping. This handles both getpwnam()
> > and initgroups, since it's done at the sdap_save_user() routine.
> > 
> 
> Ack, but I assume that the way GID is extracted and stored is going to
> change as the discussion on list suggested?

Yeah, this is already changing a bit in my latest patches in my tree. I
pulled this out into a common function and I'll update the internals of
that when Sumit gives me his changes.

> 
> > Patch 0010: As Jakub requested, this patch adds an autorid compatibility
> > mode which guarantees that all domains start at zero and increment up.
> 
> The option is not defined in the configAPI. Otherwise looks good.

Yup, will fix that as I write manpages.

> 
> > 
> > Patch 0011: This patch extends the previous patch, allowing the
> > configuration of a single default domain that will be guaranteed to be
> > slice 0. This option will need careful documentation when I get to it,
> > because it only has any meaning on the first startup from a clean cache.
> > Changing it without purging the cache will have no effect (for
> > consistency reasons).
> > 
> 
> The option is not defined in the configAPI. Otherwise looks good.

See above
> 
> > Patch 0012: Helper routine to extract the domain SID from a user or
> > group objectSID.
> > 
> 
> Ack
> 
> > Patch 0013: This routine will automatically allocate a slice for a
> > domain if it has not been seen before, assigning it a temporary name
> > that matches the domain SID.
> > 
> 
> Ack
> 
> > Patch 0014: Add support for getpwuid() requests. There's a catch here.
> > We can only look up UIDs for domains that we have a mapping for already.
> > This means that the ID has to either be in the default domain (if one is
> > configured), or at least one user from this domain must have been looked
> > up by name beforehand. In practice, this should not be a real
> > limitation, since a user login will always establish the domain mapping.
> > 
> 
> Ack
> 
> > Patch 0015: Similar to patch 0009, but for groups.
> > 
> 
> Ack
> 
> > Patch 0016: Similar to patch 0014, but for groups.
> 
> There are two breaks now in the BE_FILTER_IDNUM: handler.

Thanks, I'll fix that.

> 
> I also haven't tested the patches at all, only read them so far.

Sure, I recommend against testing them from these patches. I found a
couple additional issue late last night that I've fixed in my personal
tree. Specifically, I wasn't accounting for nested groups properly, and
I need to treat un-mappable SIDs (like the "Administrators" special
group) as non-POSIX groups so failure to map them doesn't break
processing.

I'll be sending one more set of functionality patches later today (or
whenever I can apply them atop Sumit's patches) and then I'll start
fixing the options/writing manpages.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20120423/89e4c819/attachment.sig>


More information about the sssd-devel mailing list