On 11/27/2012 01:34 PM, Pavel Březina wrote:
On 11/26/2012 03:18 PM, Michal Židek wrote:
On 11/22/2012 07:16 PM, Pavel Březina wrote:
On 11/22/2012 06:47 PM, Jakub Hrozek wrote:
On Thu, Nov 22, 2012 at 10:38:28AM +0100, Michal Židek wrote:
On 11/22/2012 10:27 AM, Pavel Březina wrote:
On 11/21/2012 03:00 PM, Michal Židek wrote:
On 11/21/2012 11:04 AM, Jakub Hrozek wrote:
On Tue, Nov 20, 2012 at 03:20:06PM +0100, Pavel Březina wrote:
We should propagate the built-in sid error instead of misusing id.
Maybe
return IDMAP* directly and return errno value in new output
parameter.


I actually think that using a special ID value is OK. We've been
treating the UID and GID 0 as a special case before anyway for the
fake
users and groups. Also sdap_idmap_sid_to_unix() is supposed to
return
errno and not IDMAP* anyway, so even if we introduced a new IDMAP*
return code, we would have to translate it into an (errno, id)
tuple.

The NSS responder would skip groups with a zero GID anyway.


I let this as it was in the previous patch. The other things are
fixed.

New patch attached.

Thanks
Michal

Nack.

+static bool sss_idmap_sid_is_builtin(const char *sid)
+{
+    if (strncmp(sid, "S-1-5-32-", 9) == 0) {
+        return true;
+    }
+
+    return true;

should say false ^

It looks good otherwise.


New patch attached.

Thanks
Michal


I was thinking about it some more and I think that Pavel is right, a
special
errno code would make the caller's code more readable. Maybe ENOTSUP
would
be usable?

+1

Sorry I steered you in the wrong direction.


This patch does the same as previous, but uses the ENOTSUP to indicate
that SID was processed.

Thanks
Michal

Hi,
two minor things.

         ret = sdap_idmap_sid_to_unix(opts->idmap_ctx, sid_str, &gid);
-        if (ret != EOK) {
+        if (ret != EOK && ret != ENOTSUP) {
             DEBUG(SSSDBG_MINOR_FAILURE,
                   ("Could not convert SID string: [%s]\n",
                    strerror(ret)));
             goto fail;
+        } else if (ret == ENOTSUP) {
+            /* ENOTSUP is returned if built-in SID was provided
+             * => fail to store the group, but return EOK */
+            DEBUG(SSSDBG_TRACE_FUNC, ("Skipping built-in object.\n"));
+            ret = EOK;
+            goto fail;
         }

Can you switch occurrences of similar code to:
if (ret == ENOTSUP) {
...
} else if (ret != EOK) {
...
}

@@ -382,8 +388,10 @@ int sdap_save_user(TALLOC_CTX *memctx,
     return EOK;

 fail:
-    DEBUG(2, ("Failed to save user [%s]\n",
-              name ? name : "Unknown"));
+    if (ret) {
+        DEBUG(2, ("Failed to save user [%s]\n",
+                  name ? name : "Unknown"));
+    }
     talloc_free(tmpctx);
     return ret;
 }

You should never get to fail label with ret == EOK. Can you switch to
"done" pattern instead? I.e.

     talloc_steal(memctx, user_attrs);
     ret = EOK;

done:
     if (ret != EOK) {
         DEBUG(SSSDBG_OP_FAILURE, ("Failed to save user [%s]\n",
                                   name ? name : "Unknown"));
     }
     talloc_free(tmpctx);
     return ret;

Thanks,
Pavel.



You are right. New patch attached.
Thanks for the review Pavel.

Michal

>From 13be6415fdb62c3845c7c9c0dc11a6d555011910 Mon Sep 17 00:00:00 2001
From: Michal Zidek <mzi...@redhat.com>
Date: Wed, 14 Nov 2012 15:36:22 +0100
Subject: [PATCH] idmap: Silence DEBUG messages when dealing with built-in
 SIDs.

When converting built-in SID to unix GID/UID a confusing debug
message about the failed conversion was printed. This patch special
cases these built-in objects.

https://fedorahosted.org/sssd/ticket/1593
---
 src/lib/idmap/sss_idmap.c                     | 13 ++++
 src/lib/idmap/sss_idmap.h                     |  5 +-
 src/providers/ldap/sdap_async_groups.c        | 61 +++++++++--------
 src/providers/ldap/sdap_async_initgroups_ad.c |  6 +-
 src/providers/ldap/sdap_async_users.c         | 95 ++++++++++++++-------------
 src/providers/ldap/sdap_idmap.c               | 25 +++++--
 6 files changed, 125 insertions(+), 80 deletions(-)

diff --git a/src/lib/idmap/sss_idmap.c b/src/lib/idmap/sss_idmap.c
index c589bd4..d7254e3 100644
--- a/src/lib/idmap/sss_idmap.c
+++ b/src/lib/idmap/sss_idmap.c
@@ -280,6 +280,15 @@ fail:
     return IDMAP_OUT_OF_MEMORY;
 }
 
+static bool sss_idmap_sid_is_builtin(const char *sid)
+{
+    if (strncmp(sid, "S-1-5-32-", 9) == 0) {
+        return true;
+    }
+
+    return false;
+}
+
 enum idmap_error_code sss_idmap_sid_to_unix(struct sss_idmap_ctx *ctx,
                                             const char *sid,
                                             uint32_t *id)
@@ -293,6 +302,10 @@ enum idmap_error_code sss_idmap_sid_to_unix(struct sss_idmap_ctx *ctx,
 
     idmap_domain_info = ctx->idmap_domain_info;
 
+    if (sid && sss_idmap_sid_is_builtin(sid)) {
+        return IDMAP_BUILTIN_SID;
+    }
+
     while (idmap_domain_info != NULL) {
         dom_len = strlen(idmap_domain_info->sid);
         if (strlen(sid) > dom_len && sid[dom_len] == '-' &&
diff --git a/src/lib/idmap/sss_idmap.h b/src/lib/idmap/sss_idmap.h
index 6b7cbe5..22a4d54 100644
--- a/src/lib/idmap/sss_idmap.h
+++ b/src/lib/idmap/sss_idmap.h
@@ -68,7 +68,10 @@ enum idmap_error_code {
     IDMAP_SID_UNKNOWN,
 
     /** The provided UID or GID could not be mapped */
-    IDMAP_NO_RANGE
+    IDMAP_NO_RANGE,
+
+    /** The provided SID is a built-in one */
+    IDMAP_BUILTIN_SID
 };
 
 /**
diff --git a/src/providers/ldap/sdap_async_groups.c b/src/providers/ldap/sdap_async_groups.c
index b635dc2..c5d1fa1 100644
--- a/src/providers/ldap/sdap_async_groups.c
+++ b/src/providers/ldap/sdap_async_groups.c
@@ -334,13 +334,13 @@ static int sdap_save_group(TALLOC_CTX *memctx,
     tmpctx = talloc_new(NULL);
     if (!tmpctx) {
         ret = ENOMEM;
-        goto fail;
+        goto done;
     }
 
     group_attrs = sysdb_new_attrs(tmpctx);
     if (group_attrs == NULL) {
         ret = ENOMEM;
-        goto fail;
+        goto done;
     }
 
     ret = sysdb_attrs_primary_name(ctx, attrs,
@@ -348,7 +348,7 @@ static int sdap_save_group(TALLOC_CTX *memctx,
                                    &name);
     if (ret != EOK) {
         DEBUG(1, ("Failed to save the group - entry has no name attribute\n"));
-        goto fail;
+        goto done;
     }
     DEBUG(SSSDBG_TRACE_FUNC, ("Processing group %s\n", name));
 
@@ -366,7 +366,7 @@ static int sdap_save_group(TALLOC_CTX *memctx,
             DEBUG(SSSDBG_MINOR_FAILURE,
                   ("Could not identify objectSID: [%s]\n",
                    strerror(ret)));
-            goto fail;
+            goto done;
         }
 
         /* Add string representation to the cache for easier
@@ -377,16 +377,22 @@ static int sdap_save_group(TALLOC_CTX *memctx,
             DEBUG(SSSDBG_MINOR_FAILURE,
                   ("Could not add SID string: [%s]\n",
                    strerror(ret)));
-            goto fail;
+            goto done;
         }
 
         /* Convert the SID into a UNIX group ID */
         ret = sdap_idmap_sid_to_unix(opts->idmap_ctx, sid_str, &gid);
-        if (ret != EOK) {
+        if (ret == ENOTSUP) {
+            /* ENOTSUP is returned if built-in SID was provided
+             * => do not store the group, but return EOK */
+            DEBUG(SSSDBG_TRACE_FUNC, ("Skipping built-in object.\n"));
+            ret = EOK;
+            goto done;
+        } else if (ret != EOK) {
             DEBUG(SSSDBG_MINOR_FAILURE,
                   ("Could not convert SID string: [%s]\n",
                    strerror(ret)));
-            goto fail;
+            goto done;
         }
 
         /* Store the GID in the ldap_attrs so it doesn't get
@@ -397,7 +403,7 @@ static int sdap_save_group(TALLOC_CTX *memctx,
             DEBUG(SSSDBG_MINOR_FAILURE,
                   ("Could not store GID: [%s]\n",
                    strerror(ret)));
-            goto fail;
+            goto done;
         }
     } else {
         ret = sysdb_attrs_get_bool(attrs, SYSDB_POSIX, &posix_group);
@@ -407,7 +413,7 @@ static int sdap_save_group(TALLOC_CTX *memctx,
             DEBUG(SSSDBG_MINOR_FAILURE,
                   ("Error reading posix attribute: [%s]\n",
                    strerror(ret)));
-            goto fail;
+            goto done;
         }
 
         DEBUG(8, ("This is%s a posix group\n", (posix_group)?"":" not"));
@@ -416,7 +422,7 @@ static int sdap_save_group(TALLOC_CTX *memctx,
             DEBUG(SSSDBG_MINOR_FAILURE,
                   ("Error setting posix attribute: [%s]\n",
                    strerror(ret)));
-            goto fail;
+            goto done;
         }
 
         ret = sysdb_attrs_get_uint32_t(attrs,
@@ -426,7 +432,7 @@ static int sdap_save_group(TALLOC_CTX *memctx,
             DEBUG(1, ("no gid provided for [%s] in domain [%s].\n",
                       name, dom->name));
             ret = EINVAL;
-            goto fail;
+            goto done;
         }
     }
 
@@ -436,7 +442,7 @@ static int sdap_save_group(TALLOC_CTX *memctx,
             DEBUG(2, ("Group [%s] filtered out! (id out of range)\n",
                       name));
             ret = EINVAL;
-            goto fail;
+            goto done;
         }
         /* Group ID OK */
     }
@@ -447,7 +453,7 @@ static int sdap_save_group(TALLOC_CTX *memctx,
         DEBUG(SSSDBG_MINOR_FAILURE,
               ("Error setting original DN: [%s]\n",
                strerror(ret)));
-        goto fail;
+        goto done;
     }
 
     ret = sdap_attrs_add_string(attrs,
@@ -458,7 +464,7 @@ static int sdap_save_group(TALLOC_CTX *memctx,
         DEBUG(SSSDBG_MINOR_FAILURE,
               ("Error setting mod timestamp: [%s]\n",
                strerror(ret)));
-        goto fail;
+        goto done;
     }
 
     ret = sysdb_attrs_get_el(attrs,
@@ -467,7 +473,7 @@ static int sdap_save_group(TALLOC_CTX *memctx,
         DEBUG(SSSDBG_MINOR_FAILURE,
               ("Error looking up group USN: [%s]\n",
                strerror(ret)));
-        goto fail;
+        goto done;
     }
     if (el->num_values == 0) {
         DEBUG(7, ("Original USN value is not available for [%s].\n",
@@ -480,12 +486,12 @@ static int sdap_save_group(TALLOC_CTX *memctx,
             DEBUG(SSSDBG_MINOR_FAILURE,
                   ("Error setting group USN: [%s]\n",
                    strerror(ret)));
-            goto fail;
+            goto done;
         }
         usn_value = talloc_strdup(tmpctx, (const char*)el->values[0].data);
         if (!usn_value) {
             ret = ENOMEM;
-            goto fail;
+            goto done;
         }
     }
 
@@ -494,13 +500,13 @@ static int sdap_save_group(TALLOC_CTX *memctx,
                                      group_attrs);
     if (ret != EOK) {
         DEBUG(SSSDBG_OP_FAILURE, ("Failed to save ghost members\n"));
-        goto fail;
+        goto done;
     }
 
     ret = sdap_save_all_names(name, attrs, !dom->case_sensitive, group_attrs);
     if (ret != EOK) {
         DEBUG(1, ("Failed to save group names\n"));
-        goto fail;
+        goto done;
     }
 
     DEBUG(6, ("Storing info for group %s\n", name));
@@ -513,7 +519,7 @@ static int sdap_save_group(TALLOC_CTX *memctx,
         DEBUG(SSSDBG_MINOR_FAILURE,
               ("Could not store group with GID: [%s]\n",
                strerror(ret)));
-        goto fail;
+        goto done;
     }
 
     if (_usn_value) {
@@ -521,14 +527,15 @@ static int sdap_save_group(TALLOC_CTX *memctx,
     }
 
     talloc_steal(memctx, group_attrs);
-    talloc_free(tmpctx);
-    return EOK;
+    ret = EOK;
 
-fail:
-    DEBUG(SSSDBG_MINOR_FAILURE,
-          ("Failed to save group [%s]: [%s]\n",
-           name ? name : "Unknown",
-           strerror(ret)));
+done:
+    if (ret) {
+        DEBUG(SSSDBG_MINOR_FAILURE,
+              ("Failed to save group [%s]: [%s]\n",
+               name ? name : "Unknown",
+               strerror(ret)));
+    }
     talloc_free(tmpctx);
     return ret;
 }
diff --git a/src/providers/ldap/sdap_async_initgroups_ad.c b/src/providers/ldap/sdap_async_initgroups_ad.c
index 7da3f50..8c0e706 100644
--- a/src/providers/ldap/sdap_async_initgroups_ad.c
+++ b/src/providers/ldap/sdap_async_initgroups_ad.c
@@ -452,7 +452,11 @@ sdap_get_ad_tokengroups_initgroups_lookup_done(struct tevent_req *subreq)
                sid_str));
         ret = sdap_idmap_sid_to_unix(state->opts->idmap_ctx, sid_str,
                                      &gid);
-        if (ret != EOK) {
+        if (ret == ENOTSUP) {
+            DEBUG(SSSDBG_TRACE_FUNC, ("Skipping built-in object.\n"));
+            ret = EOK;
+            continue;
+        } else if (ret != EOK) {
             DEBUG(SSSDBG_MINOR_FAILURE,
                   ("Could not convert SID to GID: [%s]. Skipping\n",
                    strerror(ret)));
diff --git a/src/providers/ldap/sdap_async_users.c b/src/providers/ldap/sdap_async_users.c
index d706740..f640b97 100644
--- a/src/providers/ldap/sdap_async_users.c
+++ b/src/providers/ldap/sdap_async_users.c
@@ -67,13 +67,13 @@ int sdap_save_user(TALLOC_CTX *memctx,
     tmpctx = talloc_new(NULL);
     if (!tmpctx) {
         ret = ENOMEM;
-        goto fail;
+        goto done;
     }
 
     user_attrs = sysdb_new_attrs(tmpctx);
     if (user_attrs == NULL) {
         ret = ENOMEM;
-        goto fail;
+        goto done;
     }
 
     ret = sysdb_attrs_primary_name(ctx, attrs,
@@ -81,7 +81,7 @@ int sdap_save_user(TALLOC_CTX *memctx,
                                    &name);
     if (ret != EOK) {
         DEBUG(1, ("Failed to save the user - entry has no name attribute\n"));
-        goto fail;
+        goto done;
     }
 
     if (opts->schema_type == SDAP_SCHEMA_AD) {
@@ -90,22 +90,22 @@ int sdap_save_user(TALLOC_CTX *memctx,
         if (ret == EOK) {
             ret = sysdb_attrs_add_string(user_attrs, SYSDB_FULLNAME, fullname);
             if (ret != EOK) {
-                goto fail;
+                goto done;
             }
         } else if (ret != ENOENT) {
-            goto fail;
+            goto done;
         }
     }
 
     ret = sysdb_attrs_get_el(attrs,
                              opts->user_map[SDAP_AT_USER_PWD].sys_name, &el);
-    if (ret) goto fail;
+    if (ret) goto done;
     if (el->num_values == 0) pwd = NULL;
     else pwd = (const char *)el->values[0].data;
 
     ret = sysdb_attrs_get_el(attrs,
                              opts->user_map[SDAP_AT_USER_GECOS].sys_name, &el);
-    if (ret) goto fail;
+    if (ret) goto done;
     if (el->num_values == 0) gecos = NULL;
     else gecos = (const char *)el->values[0].data;
 
@@ -114,19 +114,19 @@ int sdap_save_user(TALLOC_CTX *memctx,
         ret = sysdb_attrs_get_el(
                 attrs,
                 opts->user_map[SDAP_AT_USER_FULLNAME].sys_name, &el);
-        if (ret) goto fail;
+        if (ret) goto done;
         if (el->num_values > 0) gecos = (const char *)el->values[0].data;
     }
 
     ret = sysdb_attrs_get_el(attrs,
                              opts->user_map[SDAP_AT_USER_HOME].sys_name, &el);
-    if (ret) goto fail;
+    if (ret) goto done;
     if (el->num_values == 0) homedir = NULL;
     else homedir = (const char *)el->values[0].data;
 
     ret = sysdb_attrs_get_el(attrs,
                              opts->user_map[SDAP_AT_USER_SHELL].sys_name, &el);
-    if (ret) goto fail;
+    if (ret) goto done;
     if (el->num_values == 0) shell = NULL;
     else shell = (const char *)el->values[0].data;
 
@@ -139,23 +139,29 @@ int sdap_save_user(TALLOC_CTX *memctx,
                 tmpctx, opts->idmap_ctx, attrs,
                 opts->user_map[SDAP_AT_USER_OBJECTSID].sys_name,
                 &sid_str);
-        if (ret != EOK) goto fail;
+        if (ret != EOK) goto done;
 
         /* Add string representation to the cache for easier
          * debugging
          */
         ret = sysdb_attrs_add_string(user_attrs, SYSDB_SID_STR, sid_str);
-        if (ret != EOK) goto fail;
+        if (ret != EOK) goto done;
 
         /* Convert the SID into a UNIX user ID */
         ret = sdap_idmap_sid_to_unix(opts->idmap_ctx, sid_str, &uid);
-        if (ret != EOK) goto fail;
+        if (ret == ENOTSUP) {
+            DEBUG(SSSDBG_TRACE_FUNC, ("Skipping built-in object.\n"));
+            ret = EOK;
+            goto done;
+        } else if (ret != EOK) {
+            goto done;
+        }
 
         /* Store the UID in the ldap_attrs so it doesn't get
          * treated as a missing attribute from LDAP and removed.
          */
         ret = sysdb_attrs_add_uint32(attrs, SYSDB_UIDNUM, uid);
-        if (ret != EOK) goto fail;
+        if (ret != EOK) goto done;
     } else {
         ret = sysdb_attrs_get_uint32_t(attrs,
                                        opts->user_map[SDAP_AT_USER_UID].sys_name,
@@ -164,7 +170,7 @@ int sdap_save_user(TALLOC_CTX *memctx,
             DEBUG(1, ("no uid provided for [%s] in domain [%s].\n",
                       name, dom->name));
             ret = EINVAL;
-            goto fail;
+            goto done;
         }
     }
     /* check that the uid is valid for this domain */
@@ -172,7 +178,7 @@ int sdap_save_user(TALLOC_CTX *memctx,
             DEBUG(2, ("User [%s] filtered out! (uid out of range)\n",
                       name));
         ret = EINVAL;
-        goto fail;
+        goto done;
     }
 
     if (use_id_mapping) {
@@ -185,7 +191,7 @@ int sdap_save_user(TALLOC_CTX *memctx,
                   ("no primary group ID provided for [%s] in domain [%s].\n",
                    name, dom->name));
             ret = EINVAL;
-            goto fail;
+            goto done;
         }
 
         /* The primary group ID is just the RID part of the objectSID
@@ -200,7 +206,7 @@ int sdap_save_user(TALLOC_CTX *memctx,
             if (ret != EOK) {
                 DEBUG(SSSDBG_MINOR_FAILURE,
                       ("Could not parse domain SID from [%s]\n", sid_str));
-                goto fail;
+                goto done;
             }
         }
 
@@ -210,18 +216,18 @@ int sdap_save_user(TALLOC_CTX *memctx,
                                         (unsigned long)primary_gid);
         if (!group_sid_str) {
             ret = ENOMEM;
-            goto fail;
+            goto done;
         }
 
         /* Convert the SID into a UNIX group ID */
         ret = sdap_idmap_sid_to_unix(opts->idmap_ctx, group_sid_str, &gid);
-        if (ret != EOK) goto fail;
+        if (ret != EOK) goto done;
 
         /* Store the GID in the ldap_attrs so it doesn't get
          * treated as a missing attribute from LDAP and removed.
          */
         ret = sysdb_attrs_add_uint32(attrs, SYSDB_GIDNUM, gid);
-        if (ret != EOK) goto fail;
+        if (ret != EOK) goto done;
     } else {
         ret = sysdb_attrs_get_uint32_t(attrs,
                                        opts->user_map[SDAP_AT_USER_GID].sys_name,
@@ -230,7 +236,7 @@ int sdap_save_user(TALLOC_CTX *memctx,
             DEBUG(1, ("no gid provided for [%s] in domain [%s].\n",
                       name, dom->name));
             ret = EINVAL;
-            goto fail;
+            goto done;
         }
     }
 
@@ -239,12 +245,12 @@ int sdap_save_user(TALLOC_CTX *memctx,
             DEBUG(2, ("User [%s] filtered out! (primary gid out of range)\n",
                       name));
         ret = EINVAL;
-        goto fail;
+        goto done;
     }
 
     ret = sysdb_attrs_get_el(attrs, SYSDB_ORIG_DN, &el);
     if (ret) {
-        goto fail;
+        goto done;
     }
     if (!el || el->num_values == 0) {
         DEBUG(SSSDBG_MINOR_FAILURE,
@@ -256,13 +262,13 @@ int sdap_save_user(TALLOC_CTX *memctx,
 
         ret = sysdb_attrs_add_string(user_attrs, SYSDB_ORIG_DN, orig_dn);
         if (ret) {
-            goto fail;
+            goto done;
         }
     }
 
     ret = sysdb_attrs_get_el(attrs, SYSDB_MEMBEROF, &el);
     if (ret) {
-        goto fail;
+        goto done;
     }
     if (el->num_values == 0) {
         DEBUG(7, ("Original memberOf is not available for [%s].\n",
@@ -274,7 +280,7 @@ int sdap_save_user(TALLOC_CTX *memctx,
             ret = sysdb_attrs_add_string(user_attrs, SYSDB_ORIG_MEMBEROF,
                     (const char *) el->values[i].data);
             if (ret) {
-                goto fail;
+                goto done;
             }
         }
     }
@@ -284,13 +290,13 @@ int sdap_save_user(TALLOC_CTX *memctx,
                             "original mod-Timestamp",
                             name, user_attrs);
     if (ret != EOK) {
-        goto fail;
+        goto done;
     }
 
     ret = sysdb_attrs_get_el(attrs,
                       opts->user_map[SDAP_AT_USER_USN].sys_name, &el);
     if (ret) {
-        goto fail;
+        goto done;
     }
     if (el->num_values == 0) {
         DEBUG(7, ("Original USN value is not available for [%s].\n",
@@ -300,19 +306,19 @@ int sdap_save_user(TALLOC_CTX *memctx,
                           opts->user_map[SDAP_AT_USER_USN].sys_name,
                           (const char*)el->values[0].data);
         if (ret) {
-            goto fail;
+            goto done;
         }
         usn_value = talloc_strdup(tmpctx, (const char*)el->values[0].data);
         if (!usn_value) {
             ret = ENOMEM;
-            goto fail;
+            goto done;
         }
     }
 
     ret = sysdb_attrs_get_el(attrs,
                              opts->user_map[SDAP_AT_USER_PRINC].sys_name, &el);
     if (ret) {
-        goto fail;
+        goto done;
     }
     if (el->num_values == 0) {
         DEBUG(7, ("User principal is not available for [%s].\n", name));
@@ -320,7 +326,7 @@ int sdap_save_user(TALLOC_CTX *memctx,
         upn = talloc_strdup(user_attrs, (const char*) el->values[0].data);
         if (!upn) {
             ret = ENOMEM;
-            goto fail;
+            goto done;
         }
         if (dp_opt_get_bool(opts->basic, SDAP_FORCE_UPPER_CASE_REALM)) {
             make_realm_upper_case(upn);
@@ -329,7 +335,7 @@ int sdap_save_user(TALLOC_CTX *memctx,
                   upn, name));
         ret = sysdb_attrs_add_string(user_attrs, SYSDB_UPN, upn);
         if (ret) {
-            goto fail;
+            goto done;
         }
     }
 
@@ -337,7 +343,7 @@ int sdap_save_user(TALLOC_CTX *memctx,
         ret = sdap_attrs_add_list(attrs, opts->user_map[i].sys_name,
                                   NULL, name, user_attrs);
         if (ret) {
-            goto fail;
+            goto done;
         }
     }
 
@@ -348,14 +354,14 @@ int sdap_save_user(TALLOC_CTX *memctx,
                                      (cache_timeout ?
                                       (time(NULL) + cache_timeout) : 0));
         if (ret) {
-            goto fail;
+            goto done;
         }
     }
 
     ret = sdap_save_all_names(name, attrs, !dom->case_sensitive, user_attrs);
     if (ret != EOK) {
         DEBUG(1, ("Failed to save user names\n"));
-        goto fail;
+        goto done;
     }
 
     /* Make sure that any attributes we requested from LDAP that we
@@ -364,26 +370,27 @@ int sdap_save_user(TALLOC_CTX *memctx,
     ret = list_missing_attrs(user_attrs, opts->user_map, SDAP_OPTS_USER,
                              attrs, &missing);
     if (ret != EOK) {
-        goto fail;
+        goto done;
     }
 
     DEBUG(6, ("Storing info for user %s\n", name));
 
     ret = sysdb_store_user(ctx, name, pwd, uid, gid, gecos, homedir, shell,
                            orig_dn, user_attrs, missing, cache_timeout, now);
-    if (ret) goto fail;
+    if (ret) goto done;
 
     if (_usn_value) {
         *_usn_value = talloc_steal(memctx, usn_value);
     }
 
     talloc_steal(memctx, user_attrs);
-    talloc_free(tmpctx);
-    return EOK;
+    ret = EOK;
 
-fail:
-    DEBUG(2, ("Failed to save user [%s]\n",
-              name ? name : "Unknown"));
+done:
+    if (ret) {
+        DEBUG(2, ("Failed to save user [%s]\n",
+                  name ? name : "Unknown"));
+    }
     talloc_free(tmpctx);
     return ret;
 }
diff --git a/src/providers/ldap/sdap_idmap.c b/src/providers/ldap/sdap_idmap.c
index 9ace11b..e51fcc6 100644
--- a/src/providers/ldap/sdap_idmap.c
+++ b/src/providers/ldap/sdap_idmap.c
@@ -380,13 +380,10 @@ 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) {
-        DEBUG(SSSDBG_MINOR_FAILURE,
-              ("Could not convert objectSID [%s] to a UNIX ID\n",
-               sid_str));
-        ret = EIO;
-        goto done;
-    } else if (err == IDMAP_NO_DOMAIN) {
+    switch (err) {
+    case IDMAP_SUCCESS:
+        break;
+    case IDMAP_NO_DOMAIN:
         /* This is the first time we've seen this domain
          * Create a new domain for it. We'll use the dom-sid
          * as the domain name for now, since we don't have
@@ -420,6 +417,20 @@ sdap_idmap_sid_to_unix(struct sdap_idmap_ctx *idmap_ctx,
             ret = EIO;
             goto done;
         }
+        break;
+    case IDMAP_BUILTIN_SID:
+        DEBUG(SSSDBG_TRACE_FUNC,
+              ("Object SID [%s] is a built-in one.\n", sid_str));
+        /* ENOTSUP indicates built-in SID */
+        ret = ENOTSUP;
+        goto done;
+        break;
+    default:
+        DEBUG(SSSDBG_MINOR_FAILURE,
+              ("Could not convert objectSID [%s] to a UNIX ID\n",
+               sid_str));
+        ret = EIO;
+        goto done;
     }
 
     ret = EOK;
-- 
1.7.11.2

_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Reply via email to