[SSSD] [PATCH] Canonicalize user aliases in the proxy provider

Jakub Hrozek jhrozek at redhat.com
Thu Mar 29 20:27:03 UTC 2012


https://fedorahosted.org/sssd/ticket/1249

The patch also reduces code duplication between several error handling
switch/case statements and fixes a bug in get_initgr where we would try
to cancel a transaction immediately after committing it.

Do we also want to canonicalize group names in a similar fashion? To be
honest, downloading the whole group twice just for the sake of
determining the correct group name seems a little too much to me..
-------------- next part --------------
From ac8921a1faa06f711e91bd2d3a4acd2dc8f1dae3 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Wed, 28 Mar 2012 01:04:52 +0200
Subject: [PATCH] Canonicalize user aliases in the proxy provider

https://fedorahosted.org/sssd/ticket/1249
---
 src/providers/proxy/proxy_id.c |  336 ++++++++++++++++++++++-----------------
 1 files changed, 189 insertions(+), 147 deletions(-)

diff --git a/src/providers/proxy/proxy_id.c b/src/providers/proxy/proxy_id.c
index bf0f50932978263cf19ba6984cac0e52a8c57242..1ddd0ebf761978847c85a71f3aebe7d7404338bd 100644
--- a/src/providers/proxy/proxy_id.c
+++ b/src/providers/proxy/proxy_id.c
@@ -27,11 +27,13 @@
 
 /* =Getpwnam-wrapper======================================================*/
 
-static int delete_user(TALLOC_CTX *mem_ctx, struct sysdb_ctx *sysdb,
-                       struct sss_domain_info *domain, const char *name);
-
 static int save_user(struct sysdb_ctx *sysdb, bool lowercase,
-                     struct passwd *pwd, uint64_t cache_timeout);
+                     struct passwd *pwd, const char *alias,
+                     uint64_t cache_timeout);
+
+static int
+handle_getpw_result(enum nss_status status, struct passwd *pwd,
+                    struct sss_domain_info *dom, bool *del_user);
 
 static int get_pw_name(TALLOC_CTX *mem_ctx,
                        struct proxy_id_ctx *ctx,
@@ -45,10 +47,12 @@ static int get_pw_name(TALLOC_CTX *mem_ctx,
     char *buffer;
     size_t buflen;
     int ret;
+    uid_t uid;
+    bool del_user;
 
-    DEBUG(7, ("Searching user by name (%s)\n", name));
+    DEBUG(SSSDBG_TRACE_FUNC, ("Searching user by name (%s)\n", name));
 
-    tmpctx = talloc_new(mem_ctx);
+    tmpctx = talloc_new(NULL);
     if (!tmpctx) {
         return ENOMEM;
     }
@@ -71,67 +75,120 @@ static int get_pw_name(TALLOC_CTX *mem_ctx,
     /* FIXME: should we move this call outside the transaction to keep the
      * transaction as short as possible ? */
     status = ctx->ops.getpwnam_r(name, pwd, buffer, buflen, &ret);
+    ret = handle_getpw_result(status, pwd, dom, &del_user);
+    if (ret) {
+        DEBUG(SSSDBG_OP_FAILURE,
+              ("getpwnam failed [%d]: %s\n", ret, strerror(ret)));
+        goto done;
+    }
+
+    if (del_user) {
+        DEBUG(SSSDBG_TRACE_FUNC,
+              ("User %s does not exist (or is invalid) on remote server,"
+               " deleting!\n", name));
+        ret = sysdb_delete_user(sysdb, name, 0);
+        goto done;
+    }
+
+    uid = pwd->pw_uid;
+    memset(buffer, 0, buflen);
+
+    /* Canonicalize the username in case it was actually an alias */
+    status = ctx->ops.getpwuid_r(uid, pwd, buffer, buflen, &ret);
+    ret = handle_getpw_result(status, pwd, dom, &del_user);
+    if (ret) {
+        DEBUG(SSSDBG_OP_FAILURE,
+              ("getpwuid failed [%d]: %s\n", ret, strerror(ret)));
+        goto done;
+    }
+
+    if (del_user) {
+        DEBUG(SSSDBG_TRACE_FUNC,
+              ("User %s does not exist (or is invalid) on remote server,"
+               " deleting!\n", name));
+        ret = sysdb_delete_user(sysdb, name, uid);
+        goto done;
+    }
+
+    /* Both lookups went fine, we can save the user now */
+    ret = save_user(sysdb, !dom->case_sensitive, pwd,
+                    name, dom->user_timeout);
+
+done:
+    talloc_zfree(tmpctx);
+    if (ret) {
+        DEBUG(SSSDBG_OP_FAILURE,
+              ("proxy -> getpwnam_r failed for '%s' <%d>\n",
+               name, status));
+    }
+    return ret;
+}
+
+static int
+handle_getpw_result(enum nss_status status, struct passwd *pwd,
+                    struct sss_domain_info *dom, bool *del_user)
+{
+    int ret = EOK;
+    TALLOC_CTX *tmpctx;
+
+    if (!del_user) {
+        return EINVAL;
+    }
+    *del_user = false;
+
+    tmpctx = talloc_new(NULL);
+    if (!tmpctx) {
+        return ENOMEM;
+    }
 
     switch (status) {
     case NSS_STATUS_NOTFOUND:
 
-        DEBUG(7, ("User %s not found.\n", name));
-        ret = delete_user(tmpctx, sysdb, dom, name);
-        if (ret) {
-            goto done;
-        }
+        DEBUG(SSSDBG_MINOR_FAILURE, ("User not found.\n"));
+        *del_user = true;
         break;
 
     case NSS_STATUS_SUCCESS:
 
-        DEBUG(7, ("User %s found: (%s, %d, %d)\n",
-                  name, pwd->pw_name, pwd->pw_uid, pwd->pw_gid));
+        DEBUG(SSSDBG_TRACE_FUNC, ("User found: (%s, %d, %d)\n",
+              pwd->pw_name, pwd->pw_uid, pwd->pw_gid));
 
         /* uid=0 or gid=0 are invalid values */
         /* also check that the id is in the valid range for this domain */
         if (OUT_OF_ID_RANGE(pwd->pw_uid, dom->id_min, dom->id_max) ||
             OUT_OF_ID_RANGE(pwd->pw_gid, dom->id_min, dom->id_max)) {
 
-            DEBUG(2, ("User [%s] filtered out! (id out of range)\n", name));
-            ret = delete_user(tmpctx, sysdb, dom, name);
-            if (ret) {
-                goto done;
-            }
+            DEBUG(SSSDBG_MINOR_FAILURE,
+                  ("User filtered out! (id out of range)\n"));
+            *del_user = true;
             break;
         }
-
-        ret = save_user(sysdb, !dom->case_sensitive, pwd, dom->user_timeout);
-        if (ret) {
-            goto done;
-        }
         break;
 
     case NSS_STATUS_UNAVAIL:
-        /* "remote" backend unavailable. Enter offline mode */
+        DEBUG(SSSDBG_MINOR_FAILURE,
+              ("Remote back end is not available. Entering offline mode\n"));
         ret = ENXIO;
-        goto done;
+        break;
 
     default:
         ret = EIO;
-        goto done;
+        break;
     }
 
-done:
     talloc_zfree(tmpctx);
-    if (ret) {
-        DEBUG(2, ("proxy -> getpwnam_r failed for '%s' <%d>\n",
-                  name, status));
-    }
     return ret;
 }
 
 static int save_user(struct sysdb_ctx *sysdb, bool lowercase,
-                     struct passwd *pwd, uint64_t cache_timeout)
+                     struct passwd *pwd, const char *alias,
+                     uint64_t cache_timeout)
 {
     const char *shell;
     char *lower;
     struct sysdb_attrs *attrs = NULL;
     errno_t ret;
+    const char *cased_alias;
 
     if (pwd->pw_shell && pwd->pw_shell[0] != '\0') {
         shell = pwd->pw_shell;
@@ -139,13 +196,15 @@ static int save_user(struct sysdb_ctx *sysdb, bool lowercase,
         shell = NULL;
     }
 
-    if (lowercase) {
+    if (!lowercase || alias) {
         attrs = sysdb_new_attrs(NULL);
         if (!attrs) {
             DEBUG(SSSDBG_CRIT_FAILURE, ("Allocation error ?!\n"));
             return ENOMEM;
         }
+    }
 
+    if (lowercase) {
         lower = sss_tc_utf8_str_tolower(attrs, pwd->pw_name);
         if (!lower) {
             DEBUG(SSSDBG_CRIT_FAILURE, ("Cannot convert name to lowercase\n"));
@@ -161,6 +220,21 @@ static int save_user(struct sysdb_ctx *sysdb, bool lowercase,
         }
     }
 
+    if (alias) {
+        cased_alias = sss_get_cased_name(attrs, alias, !lowercase);
+        if (!cased_alias) {
+            talloc_zfree(attrs);
+            return ENOMEM;
+        }
+
+        ret = sysdb_attrs_add_string(attrs, SYSDB_NAME_ALIAS, cased_alias);
+        if (ret) {
+            DEBUG(SSSDBG_OP_FAILURE, ("Could not add name alias\n"));
+            talloc_zfree(attrs);
+            return ret;
+        }
+    }
+
     ret = sysdb_store_user(sysdb,
                            pwd->pw_name,
                            pwd->pw_passwd,
@@ -182,22 +256,6 @@ static int save_user(struct sysdb_ctx *sysdb, bool lowercase,
     return EOK;
 }
 
-static int delete_user(TALLOC_CTX *mem_ctx, struct sysdb_ctx *sysdb,
-                       struct sss_domain_info *domain, const char *name)
-{
-    struct ldb_dn *dn;
-
-    DEBUG(7, ("User %s does not exist (or is invalid) on remote server,"
-              " deleting!\n", name));
-
-    dn = sysdb_user_dn(sysdb, mem_ctx, domain->name, name);
-    if (!dn) {
-        return ENOMEM;
-    }
-
-    return sysdb_delete_entry(sysdb, dn, true);
-}
-
 /* =Getpwuid-wrapper======================================================*/
 
 static int get_pw_uid(TALLOC_CTX *mem_ctx,
@@ -214,9 +272,9 @@ static int get_pw_uid(TALLOC_CTX *mem_ctx,
     bool del_user = false;
     int ret;
 
-    DEBUG(7, ("Searching user by uid (%d)\n", uid));
+    DEBUG(SSSDBG_TRACE_FUNC, ("Searching user by uid (%d)\n", uid));
 
-    tmpctx = talloc_new(mem_ctx);
+    tmpctx = talloc_new(NULL);
     if (!tmpctx) {
         return ENOMEM;
     }
@@ -224,75 +282,40 @@ static int get_pw_uid(TALLOC_CTX *mem_ctx,
     pwd = talloc_zero(tmpctx, struct passwd);
     if (!pwd) {
         ret = ENOMEM;
-        DEBUG(1, ("proxy -> getpwuid_r failed for '%d': [%d] %s\n",
-                  uid, ret, strerror(ret)));
-        return ret;
+        goto done;
     }
 
     buflen = DEFAULT_BUFSIZE;
     buffer = talloc_size(tmpctx, buflen);
     if (!buffer) {
         ret = ENOMEM;
-        DEBUG(1, ("proxy -> getpwuid_r failed for '%d': [%d] %s\n",
-                  uid, ret, strerror(ret)));
-        return ret;
+        goto done;
     }
 
     status = ctx->ops.getpwuid_r(uid, pwd, buffer, buflen, &ret);
-
-    switch (status) {
-    case NSS_STATUS_NOTFOUND:
-
-        DEBUG(7, ("User %d not found.\n", uid));
-        del_user = true;
-        break;
-
-    case NSS_STATUS_SUCCESS:
-
-        DEBUG(7, ("User %d found (%s, %d, %d)\n",
-                  uid, pwd->pw_name, pwd->pw_uid, pwd->pw_gid));
-
-        /* uid=0 or gid=0 are invalid values */
-        /* also check that the id is in the valid range for this domain */
-        if (OUT_OF_ID_RANGE(pwd->pw_uid, dom->id_min, dom->id_max) ||
-            OUT_OF_ID_RANGE(pwd->pw_gid, dom->id_min, dom->id_max)) {
-
-            DEBUG(2, ("User [%s] filtered out! (id out of range)\n",
-                      pwd->pw_name));
-            del_user = true;
-            break;
-        }
-
-        ret = save_user(sysdb, !dom->case_sensitive, pwd, dom->user_timeout);
-        if (ret) {
-            goto done;
-        }
-        break;
-
-    case NSS_STATUS_UNAVAIL:
-        /* "remote" backend unavailable. Enter offline mode */
-        ret = ENXIO;
-        goto done;
-
-    default:
-        ret = EIO;
+    ret = handle_getpw_result(status, pwd, dom, &del_user);
+    if (ret) {
+        DEBUG(SSSDBG_OP_FAILURE,
+              ("getpwuid failed [%d]: %s\n", ret, strerror(ret)));
         goto done;
     }
 
     if (del_user) {
-        DEBUG(7, ("User %d does not exist (or is invalid) on remote server,"
-                  " deleting!\n", uid));
-
+        DEBUG(SSSDBG_TRACE_FUNC,
+              ("User %d does not exist (or is invalid) on remote server,"
+               " deleting!\n", uid));
         ret = sysdb_delete_user(sysdb, NULL, uid);
-        if (ret) {
-            goto done;
-        }
+        goto done;
     }
 
+    ret = save_user(sysdb, !dom->case_sensitive, pwd,
+                    NULL, dom->user_timeout);
+
 done:
     talloc_zfree(tmpctx);
     if (ret) {
-        DEBUG(2, ("proxy -> getpwuid_r failed for '%d' <%d>\n", uid, status));
+        DEBUG(SSSDBG_CRIT_FAILURE,
+              ("proxy -> getpwuid_r failed for '%d' <%d>\n", uid, status));
     }
     return ret;
 }
@@ -394,7 +417,8 @@ again:
             goto again; /* skip */
         }
 
-        ret = save_user(sysdb, !dom->case_sensitive, pwd, dom->user_timeout);
+        ret = save_user(sysdb, !dom->case_sensitive, pwd,
+                        NULL, dom->user_timeout);
         if (ret) {
             /* Do not fail completely on errors.
              * Just report the failure to save and go on */
@@ -993,6 +1017,8 @@ static int get_initgr(TALLOC_CTX *mem_ctx,
     char *buffer;
     size_t buflen;
     int ret;
+    bool del_user;
+    uid_t uid;
 
     tmpctx = talloc_new(mem_ctx);
     if (!tmpctx) {
@@ -1002,74 +1028,90 @@ static int get_initgr(TALLOC_CTX *mem_ctx,
     pwd = talloc_zero(tmpctx, struct passwd);
     if (!pwd) {
         ret = ENOMEM;
-        goto done;
+        goto fail;
     }
 
     buflen = DEFAULT_BUFSIZE;
     buffer = talloc_size(tmpctx, buflen);
     if (!buffer) {
         ret = ENOMEM;
-        goto done;
+        goto fail;
     }
 
     ret = sysdb_transaction_start(sysdb);
     if (ret) {
-        goto done;
+        goto fail;
     }
     in_transaction = true;
 
     /* FIXME: should we move this call outside the transaction to keep the
      * transaction as short as possible ? */
     status = ctx->ops.getpwnam_r(name, pwd, buffer, buflen, &ret);
-
-    switch (status) {
-    case NSS_STATUS_NOTFOUND:
-
-        DEBUG(7, ("User %s not found.\n", name));
-        ret = delete_user(tmpctx, sysdb, dom, name);
+    ret = handle_getpw_result(status, pwd, dom, &del_user);
+    if (ret) {
+        DEBUG(SSSDBG_OP_FAILURE,
+              ("getpwnam failed [%d]: %s\n", ret, strerror(ret)));
+        goto fail;
+    }
+
+    if (del_user) {
+        DEBUG(SSSDBG_TRACE_FUNC,
+              ("User %s does not exist (or is invalid) on remote server,"
+               " deleting!\n", name));
+        ret = sysdb_delete_user(sysdb, name, 0);
         if (ret) {
-            goto done;
-        }
-        break;
-
-    case NSS_STATUS_SUCCESS:
-
-        /* uid=0 or gid=0 are invalid values */
-        /* also check that the id is in the valid range for this domain */
-        if (OUT_OF_ID_RANGE(pwd->pw_uid, dom->id_min, dom->id_max) ||
-            OUT_OF_ID_RANGE(pwd->pw_gid, dom->id_min, dom->id_max)) {
-
-            DEBUG(2, ("User [%s] filtered out! (id out of range)\n",
-                      name));
-            ret = delete_user(tmpctx, sysdb, dom, name);
-            break;
+            DEBUG(SSSDBG_OP_FAILURE, ("Could not delete user\n"));
+            goto fail;
         }
-
-        ret = save_user(sysdb, !dom->case_sensitive, pwd, dom->user_timeout);
+        goto done;
+    }
+
+    uid = pwd->pw_uid;
+    memset(buffer, 0, buflen);
+
+    /* Canonicalize the username in case it was actually an alias */
+    status = ctx->ops.getpwuid_r(uid, pwd, buffer, buflen, &ret);
+    ret = handle_getpw_result(status, pwd, dom, &del_user);
+    if (ret) {
+        DEBUG(SSSDBG_OP_FAILURE,
+              ("getpwuid failed [%d]: %s\n", ret, strerror(ret)));
+        goto fail;
+    }
+
+    if (del_user) {
+        DEBUG(SSSDBG_TRACE_FUNC,
+              ("User %s does not exist (or is invalid) on remote server,"
+               " deleting!\n", name));
+        ret = sysdb_delete_user(sysdb, name, uid);
         if (ret) {
-            goto done;
-        }
-
-        ret = get_initgr_groups_process(tmpctx, ctx, sysdb, dom, pwd);
-        if (ret == EOK) {
-            ret = sysdb_transaction_commit(sysdb);
-            in_transaction = true;
+            DEBUG(SSSDBG_OP_FAILURE, ("Could not delete user\n"));
+            goto fail;
         }
-        break;
-
-    case NSS_STATUS_UNAVAIL:
-        /* "remote" backend unavailable. Enter offline mode */
-        ret = ENXIO;
-        break;
-
-    default:
-        DEBUG(2, ("proxy -> getpwnam_r failed for '%s' <%d>\n",
-                  name, status));
-        ret = EIO;
-        break;
+        goto done;
+    }
+
+    ret = save_user(sysdb, !dom->case_sensitive, pwd,
+                    name, dom->user_timeout);
+    if (ret) {
+        DEBUG(SSSDBG_OP_FAILURE, ("Could not save user\n"));
+        goto fail;
+    }
+
+    ret = get_initgr_groups_process(tmpctx, ctx, sysdb, dom, pwd);
+    if (ret != EOK) {
+        DEBUG(SSSDBG_OP_FAILURE, ("Could not process initgroups\n"));
+        goto fail;
     }
 
 done:
+    ret = sysdb_transaction_commit(sysdb);
+    if (ret) {
+        DEBUG(SSSDBG_OP_FAILURE, ("Failed to commit transaction\n"));
+        goto fail;
+    }
+    in_transaction = false;
+
+fail:
     talloc_zfree(tmpctx);
     if (in_transaction) {
         sysdb_transaction_cancel(sysdb);
-- 
1.7.7.6



More information about the sssd-devel mailing list