On 11/15/2012 09:11 PM, Michal Židek wrote:
https://fedorahosted.org/sssd/ticket/1593
With this patch we get:
[sssd[be[AD]]] [sdap_save_group] (0x1000): Mapping group
[Administrators] objectSID to unix ID
[sssd[be[AD]]] [sdap_idmap_sid_to_unix] (0x0400): Object SID
[S-1-5-32-544] is a built-in one.
[sssd[be[AD]]] [sdap_save_group] (0x0400): Skipping built-in object.
Instead of the old messages, that looked like this:
[sssd[be[AD]]] [sdap_save_group] (0x1000): Mapping group
[Administrators] objectSID to unix ID
[sssd[be[AD]]] [sdap_idmap_sid_to_unix] (0x0080): Could not parse domain
SID from [S-1-5-32-544]
[sssd[be[AD]]] [sdap_save_group] (0x0080): Could not convert SID string:
[Invalid argument]
[sssd[be[AD]]] [sdap_save_group] (0x0080): Failed to save group
[Administrators]: [Invalid argument]
[sssd[be[AD]]] [sdap_save_groups] (0x0040): Failed to store group 0.
Ignoring.
The patch is attached.
NOTE: To reproduce, set enumerate = true for the AD domain, delete the
cache files and start SSSD.
Thanks
Michal
Nack.
+static bool sss_idmap_sid_is_builtin(const char *sid)
+{
+ if (strncmp(sid, "S-1-5-32-", 9)) {
+ return false;
+ } else {
+ return true;
+ }
+}
Please, change ^^ to:
+static bool sss_idmap_sid_is_builtin(const char *sid)
+{
+ if (strncmp(sid, "S-1-5-32-", 9) == 0) {
+ return true;
+ }
+
+ return false;
+}
@@ -380,7 +380,8 @@ sdap_idmap_sid_to_unix(struct sdap_idmap_ctx
*idmap_ctx,
err = sss_idmap_sid_to_unix(idmap_ctx->map,
sid_str,
(uint32_t *)id);
- if (err != IDMAP_SUCCESS && err != IDMAP_NO_DOMAIN) {
+ if (err != IDMAP_SUCCESS && err != IDMAP_NO_DOMAIN &&
+ err != IDMAP_BUILTIN_SID) {
DEBUG(SSSDBG_MINOR_FAILURE,
("Could not convert objectSID [%s] to a UNIX ID\n",
sid_str));
@@ -420,6 +421,10 @@ sdap_idmap_sid_to_unix(struct sdap_idmap_ctx *idmap_ctx,
ret = EIO;
goto done;
}
+ } else if (err == IDMAP_BUILTIN_SID) {
Can you use switch statement please? Like:
switch (err) {
case IDMAP_NO_DOMAIN:
...
break;
case IDMAP_BUILTIN_SID:
...
break;
default:
error
}
+ DEBUG(SSSDBG_TRACE_FUNC,
+ ("Object SID [%s] is a built-in one.\n", sid_str));
+ *id = 0; /* O indicates, that this ID should be ignored */
We should propagate the built-in sid error instead of misusing id. Maybe
return IDMAP* directly and return errno value in new output parameter.
Thanks,
Pavel.