These patches are built atop Sumit's recent patch "Allow different SID representations in libidmap". I added the manpage as a single patch near the end because it was just too much trouble to do it piecemeal throughout the set. This patch series went through numerous iterations, so some of the patches may be slightly out of order. Please review as a whole. The patches themselves are separated primarily the way they are to make reviewing easier. Some have notes below to guide the reviewer to changes that may have been revised later but were nontrivial to rewrite history for.
Patch 0001: Add objectSID config option This patch adds an option to specify the objectSID attribute on the LDAP server, for use when performing ID-mapping
Patch 0002: Add option to enable id-mapping
Patch 0003: Add sysdb routines for storing ID maps in the cache
Patch 0004: Add helper routines to the LDAP provider for ID mapping This contains the meat of the ID-mapping algorithm. See the manpage in Patch 0020 for full details.
Patch 0005: Add options for configuring range settings
Patch 0006: LDAP: Initialize ID mapping when configured
Patch 0007: Do ID-mapped lookups for users Note: this patch contains a bit of code that has been refactored by the end of the patchset (specifically it references binary_to_dom_sid() and dom_sid_to_string()). I didn't bother rewriting this particular bit of history because it's replaced entirely by Patch 0016 anyway (which uses the correct functions from Sumit's patch).
Patch 0008: Add an autorid compatibility mode Autorid assigns slices as first-come-first-served. This will force the LDAP ID-mapping to behave the same way.
Patch 0009: Add a feature to guarantee that a single "default" domain is always assigned to slice 0, regardless of hashing. This can be used to extend autorid compatibility mode.
Patch 0010: Helper routine to get the domain SID portion from a user or group objectSID
Patch 0011: Allow us to auto-provision a new domain when we see one for the first time. Note: this gets turned into its own function in Patch 0018 for reuse in groups and initgroups.
Patch 0012: Look up mapped users by UID
Patch 0013: Look up mapped groups by name
Patch 0014: Look up mapped groups by GID
Patch 0015: Map the user's primaryGID. ActiveDirectory stores only the RID of the primary group, so we extract the domain SID from the user SID and then convert the group ID from that.
Patch 0016: Common routine to convert an LDAP blob of the objectSID into a UNIX ID.
Patch 0017: Hack to ensure that uidNumber and gidNumber are not deleted when we save users and groups.
Patch 0018: Convert the auto-provisioning code into a common routine, rather than copying it everywhere.
Patch 0019: Handle cases where we have an unmappable ID (such as special-case SIDs) for a group membership. In these cases, we'll treat the groups as non-POSIX so we can safely continue.
Patch 0020: (Hopefully) comprehensive manpage on the ID-mapping feature. It's separated into its own include file so that it will be possible to import it into the manpage for the AD provider when that is available.
On Fri, Apr 27, 2012 at 01:47:51PM -0400, Stephen Gallagher wrote:
These patches are built atop Sumit's recent patch "Allow different SID representations in libidmap". I added the manpage as a single patch near the end because it was just too much trouble to do it piecemeal throughout the set. This patch series went through numerous iterations, so some of the patches may be slightly out of order. Please review as a whole. The patches themselves are separated primarily the way they are to make reviewing easier. Some have notes below to guide the reviewer to changes that may have been revised later but were nontrivial to rewrite history for.
Patch 0001: Add objectSID config option This patch adds an option to specify the objectSID attribute on the LDAP server, for use when performing ID-mapping
Ack
Patch 0002: Add option to enable id-mapping
Ack
Patch 0003: Add sysdb routines for storing ID maps in the cache
Nack, you forgot to free tmp_ctx in sysdb_idmap_store_mapping()
Patch 0004: Add helper routines to the LDAP provider for ID mapping This contains the meat of the ID-mapping algorithm. See the manpage in Patch 0020 for full details.
Mostly ack, but in sdap_idmap_add_domain I would prefer to check for rangesize > 0 before dividing with its value.
Patch 0005: Add options for configuring range settings
Ack
Patch 0006: LDAP: Initialize ID mapping when configured
Ack
Patch 0007: Do ID-mapped lookups for users Note: this patch contains a bit of code that has been refactored by the end of the patchset (specifically it references binary_to_dom_sid() and dom_sid_to_string()). I didn't bother rewriting this particular bit of history because it's replaced entirely by Patch 0016 anyway (which uses the correct functions from Sumit's patch).
The diff looked good, so ack, but I actually ended up reviewing the sdap_save_user code as a whole.
Patch 0008: Add an autorid compatibility mode Autorid assigns slices as first-come-first-served. This will force the LDAP ID-mapping to behave the same way.
Ack
Patch 0009: Add a feature to guarantee that a single "default" domain is always assigned to slice 0, regardless of hashing. This can be used to extend autorid compatibility mode.
Ack
Patch 0010: Helper routine to get the domain SID portion from a user or group objectSID
Ack
Patch 0011: Allow us to auto-provision a new domain when we see one for the first time. Note: this gets turned into its own function in Patch 0018 for reuse in groups and initgroups.
Looks good, full review done as part of patch 18
Patch 0012: Look up mapped users by UID
UID and GID values are 32bit unsigned values in general, right? Wouldn't it be better to use strtouint32() and not strtoint32(), then to avoid errors when a very large ID is requested? The strto(u)int32 functions also set errno to 0 themselves, no need to do it manually.
Patch 0013: Look up mapped groups by name
Ack for the changes that actually made it into sdap_save_group().
I noticed that even though the basic filters for group lookups have been changed to not include SDAP_AT_GROUP_GID, the enumeration filters still do include it. Does id-mapped enumeration work? (I haven't tested the patches yet, I'll do it while waiting for the next revision)
Patch 0014: Look up mapped groups by GID
Same comment about strtoint32 vs. strtouint32 as I had for patch #12 applies here, too. Otherwise looks good.
Patch 0015: Map the user's primaryGID. ActiveDirectory stores only the RID of the primary group, so we extract the domain SID from the user SID and then convert the group ID from that.
Ack
Patch 0016: Common routine to convert an LDAP blob of the objectSID into a UNIX ID.
Ack, but please fix the compound bracket indentation in sdap_add_incomplete_groups - it's line 155 when all patches are applied.
Patch 0017: Hack to ensure that uidNumber and gidNumber are not deleted when we save users and groups.
Ack
Patch 0018: Convert the auto-provisioning code into a common routine, rather than copying it everywhere.
Ack
Patch 0019: Handle cases where we have an unmappable ID (such as special-case SIDs) for a group membership. In these cases, we'll treat the groups as non-POSIX so we can safely continue.
Ack
Patch 0020: (Hopefully) comprehensive manpage on the ID-mapping feature. It's separated into its own include file so that it will be possible to import it into the manpage for the AD provider when that is available.
+ components that represent the Active Directory domain identity and + the relative identifier (ID) of the user or group object.
Shouldn't the sentence say "RID" in the brackets?
Otherwise Ack
On Fri, Apr 27, 2012 at 01:47:51PM -0400, Stephen Gallagher wrote:
These patches are built atop Sumit's recent patch "Allow different SID representations in libidmap". I added the manpage as a single patch near the end because it was just too much trouble to do it piecemeal throughout the set. This patch series went through numerous iterations, so some of the patches may be slightly out of order. Please review as a whole. The patches themselves are separated primarily the way they are to make reviewing easier. Some have notes below to guide the reviewer to changes that may have been revised later but were nontrivial to rewrite history for.
Patch 0001: Add objectSID config option This patch adds an option to specify the objectSID attribute on the LDAP server, for use when performing ID-mapping
Ack
Nack, please add ldap_group_objectsid to man page and API definition
Patch 0002: Add option to enable id-mapping
Ack
Nack, please add ldap_id_mapping to man page
Patch 0003: Add sysdb routines for storing ID maps in the cache
Nack, you forgot to free tmp_ctx in sysdb_idmap_store_mapping()
Line 254 in the patch: if (old_slice == -1) - you forgot to set the ret value
I'm not sure about name comparison on line 282 in the patch. Since it is related to windows, I suppose it should be case insensitive
Patch 0004: Add helper routines to the LDAP provider for ID mapping This contains the meat of the ID-mapping algorithm. See the manpage in Patch 0020 for full details.
Mostly ack, but in sdap_idmap_add_domain I would prefer to check for rangesize > 0 before dividing with its value.
To avoid problems, I think all three values (idmap_lower, idmap_upper and rangesize) should be checked for sanity.
The rest of the review wil follow, I just didn't want to add one more round to the review process.
Jan
Patch 0005: Add options for configuring range settings
Ack
Patch 0006: LDAP: Initialize ID mapping when configured
Ack
Patch 0007: Do ID-mapped lookups for users Note: this patch contains a bit of code that has been refactored by the end of the patchset (specifically it references binary_to_dom_sid() and dom_sid_to_string()). I didn't bother rewriting this particular bit of history because it's replaced entirely by Patch 0016 anyway (which uses the correct functions from Sumit's patch).
The diff looked good, so ack, but I actually ended up reviewing the sdap_save_user code as a whole.
Patch 0008: Add an autorid compatibility mode Autorid assigns slices as first-come-first-served. This will force the LDAP ID-mapping to behave the same way.
Ack
Patch 0009: Add a feature to guarantee that a single "default" domain is always assigned to slice 0, regardless of hashing. This can be used to extend autorid compatibility mode.
Ack
Patch 0010: Helper routine to get the domain SID portion from a user or group objectSID
Ack
Patch 0011: Allow us to auto-provision a new domain when we see one for the first time. Note: this gets turned into its own function in Patch 0018 for reuse in groups and initgroups.
Looks good, full review done as part of patch 18
Patch 0012: Look up mapped users by UID
UID and GID values are 32bit unsigned values in general, right? Wouldn't it be better to use strtouint32() and not strtoint32(), then to avoid errors when a very large ID is requested? The strto(u)int32 functions also set errno to 0 themselves, no need to do it manually.
Patch 0013: Look up mapped groups by name
Ack for the changes that actually made it into sdap_save_group().
I noticed that even though the basic filters for group lookups have been changed to not include SDAP_AT_GROUP_GID, the enumeration filters still do include it. Does id-mapped enumeration work? (I haven't tested the patches yet, I'll do it while waiting for the next revision)
Patch 0014: Look up mapped groups by GID
Same comment about strtoint32 vs. strtouint32 as I had for patch #12 applies here, too. Otherwise looks good.
Patch 0015: Map the user's primaryGID. ActiveDirectory stores only the RID of the primary group, so we extract the domain SID from the user SID and then convert the group ID from that.
Ack
Patch 0016: Common routine to convert an LDAP blob of the objectSID into a UNIX ID.
Ack, but please fix the compound bracket indentation in sdap_add_incomplete_groups - it's line 155 when all patches are applied.
Patch 0017: Hack to ensure that uidNumber and gidNumber are not deleted when we save users and groups.
Ack
Patch 0018: Convert the auto-provisioning code into a common routine, rather than copying it everywhere.
Ack
Patch 0019: Handle cases where we have an unmappable ID (such as special-case SIDs) for a group membership. In these cases, we'll treat the groups as non-POSIX so we can safely continue.
Ack
Patch 0020: (Hopefully) comprehensive manpage on the ID-mapping feature. It's separated into its own include file so that it will be possible to import it into the manpage for the AD provider when that is available.
- components that represent the Active Directory domain identity and
- the relative identifier (ID) of the user or group object.
Shouldn't the sentence say "RID" in the brackets?
Otherwise Ack
Nack, please add ldap_group_objectsid to man page and API definition
Patch 0002: Add option to enable id-mapping
Ack
Nack, please add ldap_id_mapping to man page
Never mind those man page comments, I just noticed they are in the last patch. However there is still the missing ldap_group_objectsid in config API
Jan
On Wed, 2012-05-02 at 16:21 +0200, Jan Zelený wrote:
On Fri, Apr 27, 2012 at 01:47:51PM -0400, Stephen Gallagher wrote:
These patches are built atop Sumit's recent patch "Allow different SID representations in libidmap". I added the manpage as a single patch near the end because it was just too much trouble to do it piecemeal throughout the set. This patch series went through numerous iterations, so some of the patches may be slightly out of order. Please review as a whole. The patches themselves are separated primarily the way they are to make reviewing easier. Some have notes below to guide the reviewer to changes that may have been revised later but were nontrivial to rewrite history for.
Patch 0001: Add objectSID config option This patch adds an option to specify the objectSID attribute on the LDAP server, for use when performing ID-mapping
Ack
Nack, please add ldap_group_objectsid to man page and API definition
Fixed. Thanks for catching that. I'd added the user version but not the group one.
Patch 0002: Add option to enable id-mapping
Ack
Nack, please add ldap_id_mapping to man page
As noted elsewhere, this is covered in patch 0020.
Patch 0003: Add sysdb routines for storing ID maps in the cache
Nack, you forgot to free tmp_ctx in sysdb_idmap_store_mapping()
Line 254 in the patch: if (old_slice == -1) - you forgot to set the ret value
Good catches. Fixed both.
I'm not sure about name comparison on line 282 in the patch. Since it is related to windows, I suppose it should be case insensitive
I think this is okay here in general. The only time this functionality should ever be used is if we're populating it with known data, which will therefore be the canonical capitalization. If it's different from what's there, we'll update it.
Patch 0004: Add helper routines to the LDAP provider for ID mapping This contains the meat of the ID-mapping algorithm. See the manpage in Patch 0020 for full details.
Mostly ack, but in sdap_idmap_add_domain I would prefer to check for rangesize > 0 before dividing with its value.
To avoid problems, I think all three values (idmap_lower, idmap_upper and rangesize) should be checked for sanity.
Added checks for rangesize <= 0, for upper being lower than the lower bound and for the difference being less than a rangesize.
The rest of the review wil follow, I just didn't want to add one more round to the review process.
Thanks
Patch 0005: Add options for configuring range settings
Ack
Patch 0006: LDAP: Initialize ID mapping when configured
Ack
Patch 0007: Do ID-mapped lookups for users Note: this patch contains a bit of code that has been refactored by the end of the patchset (specifically it references binary_to_dom_sid() and dom_sid_to_string()). I didn't bother rewriting this particular bit of history because it's replaced entirely by Patch 0016 anyway (which uses the correct functions from Sumit's patch).
The diff looked good, so ack, but I actually ended up reviewing the sdap_save_user code as a whole.
That's probably the right approach.
Patch 0008: Add an autorid compatibility mode Autorid assigns slices as first-come-first-served. This will force the LDAP ID-mapping to behave the same way.
Ack
Patch 0009: Add a feature to guarantee that a single "default" domain is always assigned to slice 0, regardless of hashing. This can be used to extend autorid compatibility mode.
Ack
Patch 0010: Helper routine to get the domain SID portion from a user or group objectSID
Ack
Patch 0011: Allow us to auto-provision a new domain when we see one for the first time. Note: this gets turned into its own function in Patch 0018 for reuse in groups and initgroups.
Looks good, full review done as part of patch 18
Patch 0012: Look up mapped users by UID
UID and GID values are 32bit unsigned values in general, right? Wouldn't it be better to use strtouint32() and not strtoint32(), then to avoid errors when a very large ID is requested? The strto(u)int32 functions also set errno to 0 themselves, no need to do it manually.
The errno thing isn't harmful, so I'm not going to bother with it. It actually turns out that id_t (as well as uid_t and gid_t) are actually *signed* values, not unsigned. This has actually caused us issues elsewhere before. See https://fedorahosted.org/sssd/ticket/1216 for details.
Patch 0013: Look up mapped groups by name
Ack for the changes that actually made it into sdap_save_group().
I noticed that even though the basic filters for group lookups have been changed to not include SDAP_AT_GROUP_GID, the enumeration filters still do include it. Does id-mapped enumeration work? (I haven't tested the patches yet, I'll do it while waiting for the next revision)
Eep. Thanks, I had completely forgotten about enumeration. I'm going to add that support as new patches atop these. Let's get these in first, please.
Patch 0014: Look up mapped groups by GID
Same comment about strtoint32 vs. strtouint32 as I had for patch #12 applies here, too. Otherwise looks good.
Patch 0015: Map the user's primaryGID. ActiveDirectory stores only the RID of the primary group, so we extract the domain SID from the user SID and then convert the group ID from that.
Ack
Patch 0016: Common routine to convert an LDAP blob of the objectSID into a UNIX ID.
Ack, but please fix the compound bracket indentation in sdap_add_incomplete_groups - it's line 155 when all patches are applied.
Patch 0017: Hack to ensure that uidNumber and gidNumber are not deleted when we save users and groups.
Ack
Patch 0018: Convert the auto-provisioning code into a common routine, rather than copying it everywhere.
Ack
Patch 0019: Handle cases where we have an unmappable ID (such as special-case SIDs) for a group membership. In these cases, we'll treat the groups as non-POSIX so we can safely continue.
Ack
Patch 0020: (Hopefully) comprehensive manpage on the ID-mapping feature. It's separated into its own include file so that it will be possible to import it into the manpage for the AD provider when that is available.
- components that represent the Active Directory domain identity and
- the relative identifier (ID) of the user or group object.
Shouldn't the sentence say "RID" in the brackets?
Otherwise Ack
Yes, thanks. Fixed.
New patches attached.
On Wed, 2012-05-02 at 14:26 -0400, Stephen Gallagher wrote:
On Wed, 2012-05-02 at 16:21 +0200, Jan Zelený wrote:
Patch 0013: Look up mapped groups by name
Ack for the changes that actually made it into sdap_save_group().
I noticed that even though the basic filters for group lookups have been changed to not include SDAP_AT_GROUP_GID, the enumeration filters still do include it. Does id-mapped enumeration work? (I haven't tested the patches yet, I'll do it while waiting for the next revision)
Eep. Thanks, I had completely forgotten about enumeration. I'm going to add that support as new patches atop these. Let's get these in first, please.
Ok, that turned out to be a lot less trouble than I'd feared it would be. I only needed to correct the filters and the sdap_async_* code took care of the rest. One more patch (0021) attached that adds enumeration support for users and groups. I'm not reattaching the first 20, they're unchanged.
On Wed, May 02, 2012 at 07:57:42PM -0400, Stephen Gallagher wrote:
On Wed, 2012-05-02 at 14:26 -0400, Stephen Gallagher wrote:
On Wed, 2012-05-02 at 16:21 +0200, Jan Zelený wrote:
Patch 0013: Look up mapped groups by name
Ack for the changes that actually made it into sdap_save_group().
I noticed that even though the basic filters for group lookups have been changed to not include SDAP_AT_GROUP_GID, the enumeration filters still do include it. Does id-mapped enumeration work? (I haven't tested the patches yet, I'll do it while waiting for the next revision)
Eep. Thanks, I had completely forgotten about enumeration. I'm going to add that support as new patches atop these. Let's get these in first, please.
Ok, that turned out to be a lot less trouble than I'd feared it would be. I only needed to correct the filters and the sdap_async_* code took care of the rest. One more patch (0021) attached that adds enumeration support for users and groups. I'm not reattaching the first 20, they're unchanged.
Ack
The mail is getting too big, I'm going to trim the acked patches.
On Wed, May 02, 2012 at 02:26:38PM -0400, Stephen Gallagher wrote:
On Wed, 2012-05-02 at 16:21 +0200, Jan Zelený wrote:
On Fri, Apr 27, 2012 at 01:47:51PM -0400, Stephen Gallagher wrote:
These patches are built atop Sumit's recent patch "Allow different SID representations in libidmap". I added the manpage as a single patch near the end because it was just too much trouble to do it piecemeal throughout the set. This patch series went through numerous iterations, so some of the patches may be slightly out of order. Please review as a whole. The patches themselves are separated primarily the way they are to make reviewing easier. Some have notes below to guide the reviewer to changes that may have been revised later but were nontrivial to rewrite history for.
Patch 0001: Add objectSID config option This patch adds an option to specify the objectSID attribute on the LDAP server, for use when performing ID-mapping
Ack
Nack, please add ldap_group_objectsid to man page and API definition
ldap_group_objectsid seems to be still missing from src/config/SSSDConfig.py
Fixed. Thanks for catching that. I'd added the user version but not the group one.
Patch 0002: Add option to enable id-mapping
Ack
Nack, please add ldap_id_mapping to man page
As noted elsewhere, this is covered in patch 0020.
Yes, Ack
Patch 0003: Add sysdb routines for storing ID maps in the cache
Nack, you forgot to free tmp_ctx in sysdb_idmap_store_mapping()
Line 254 in the patch: if (old_slice == -1) - you forgot to set the ret value
Good catches. Fixed both.
I'm not sure about name comparison on line 282 in the patch. Since it is related to windows, I suppose it should be case insensitive
I think this is okay here in general. The only time this functionality should ever be used is if we're populating it with known data, which will therefore be the canonical capitalization. If it's different from what's there, we'll update it.
Ack
Patch 0004: Add helper routines to the LDAP provider for ID mapping This contains the meat of the ID-mapping algorithm. See the manpage in Patch 0020 for full details.
Mostly ack, but in sdap_idmap_add_domain I would prefer to check for rangesize > 0 before dividing with its value.
To avoid problems, I think all three values (idmap_lower, idmap_upper and rangesize) should be checked for sanity.
Added checks for rangesize <= 0, for upper being lower than the lower bound and for the difference being less than a rangesize.
Ack
The rest of the review wil follow, I just didn't want to add one more round to the review process.
Thanks
0005: No change, ack remains 0006: No change, ack remains 0007: No change, ack remains 0008: No change, ack remains 0009: No change, ack remains 0010: No change, ack remains 0011: No change, ack remains
Patch 0012: Look up mapped users by UID
UID and GID values are 32bit unsigned values in general, right? Wouldn't it be better to use strtouint32() and not strtoint32(), then to avoid errors when a very large ID is requested? The strto(u)int32 functions also set errno to 0 themselves, no need to do it manually.
The errno thing isn't harmful, so I'm not going to bother with it. It actually turns out that id_t (as well as uid_t and gid_t) are actually *signed* values, not unsigned. This has actually caused us issues elsewhere before. See https://fedorahosted.org/sssd/ticket/1216 for details.
Wasn't that bug just a matter of /proc not representing the data correctly? On my system, both uid_t and gid_t eventually resolve to unsigned int (through a __U32_TYPE constant).
Patch 0013: Look up mapped groups by name
Ack for the changes that actually made it into sdap_save_group().
I noticed that even though the basic filters for group lookups have been changed to not include SDAP_AT_GROUP_GID, the enumeration filters still do include it. Does id-mapped enumeration work? (I haven't tested the patches yet, I'll do it while waiting for the next revision)
I acked patch #21 in the other thread.
Ack to patch #13 now
Eep. Thanks, I had completely forgotten about enumeration. I'm going to add that support as new patches atop these. Let's get these in first, please.
Patch 0014: Look up mapped groups by GID
Same comment about strtoint32 vs. strtouint32 as I had for patch #12 applies here, too. Otherwise looks good.
0015: No change, ack remains
Patch 0016: Common routine to convert an LDAP blob of the objectSID into a UNIX ID.
Ack, but please fix the compound bracket indentation in sdap_add_incomplete_groups - it's line 155 when all patches are applied.
That was fixed, ack
0017: No change, ack remains 0018: No change, ack remains 0019: No change, ack remains
Patch 0020: (Hopefully) comprehensive manpage on the ID-mapping feature. It's separated into its own include file so that it will be possible to import it into the manpage for the AD provider when that is available.
- components that represent the Active Directory domain identity and
- the relative identifier (ID) of the user or group object.
Shouldn't the sentence say "RID" in the brackets?
Otherwise Ack
Yes, thanks. Fixed.
Ack
On Thu, 2012-05-03 at 16:51 +0200, Jakub Hrozek wrote:
The mail is getting too big, I'm going to trim the acked patches. ldap_group_objectsid seems to be still missing from src/config/SSSDConfig.py
Man, this just keeps biting me. Fixed.
Patch 0012: Look up mapped users by UID
UID and GID values are 32bit unsigned values in general, right? Wouldn't it be better to use strtouint32() and not strtoint32(), then to avoid errors when a very large ID is requested? The strto(u)int32 functions also set errno to 0 themselves, no need to do it manually.
The errno thing isn't harmful, so I'm not going to bother with it. It actually turns out that id_t (as well as uid_t and gid_t) are actually *signed* values, not unsigned. This has actually caused us issues elsewhere before. See https://fedorahosted.org/sssd/ticket/1216 for details.
Wasn't that bug just a matter of /proc not representing the data correctly? On my system, both uid_t and gid_t eventually resolve to unsigned int (through a __U32_TYPE constant).
Yeah, you're right. I switched them over to strtouint32 and removed the errno=0. Same for patch 0014.
All patches reattached, but only those three have changed.
On Thu, May 03, 2012 at 11:28:51AM -0400, Stephen Gallagher wrote:
On Thu, 2012-05-03 at 16:51 +0200, Jakub Hrozek wrote:
The mail is getting too big, I'm going to trim the acked patches. ldap_group_objectsid seems to be still missing from src/config/SSSDConfig.py
Man, this just keeps biting me. Fixed.
Patch 0012: Look up mapped users by UID
UID and GID values are 32bit unsigned values in general, right? Wouldn't it be better to use strtouint32() and not strtoint32(), then to avoid errors when a very large ID is requested? The strto(u)int32 functions also set errno to 0 themselves, no need to do it manually.
The errno thing isn't harmful, so I'm not going to bother with it. It actually turns out that id_t (as well as uid_t and gid_t) are actually *signed* values, not unsigned. This has actually caused us issues elsewhere before. See https://fedorahosted.org/sssd/ticket/1216 for details.
Wasn't that bug just a matter of /proc not representing the data correctly? On my system, both uid_t and gid_t eventually resolve to unsigned int (through a __U32_TYPE constant).
Yeah, you're right. I switched them over to strtouint32 and removed the errno=0. Same for patch 0014.
All patches reattached, but only those three have changed.
Only patches #1, #12 and #14 have changed, all the ID operations work as expected.
Ack to all patches.
sssd-devel@lists.fedorahosted.org
-
Jakub Hrozek -
Jan Zelený -
Stephen Gallagher