dpward / rpms / sssd

Forked from rpms/sssd 3 years ago
Clone
Blob Blame History Raw
From 663fb2625181ca8b80d26526a8298674ab6242a1 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek@redhat.com>
Date: Sun, 23 Nov 2014 19:43:04 +0100
Subject: [PATCH 116/117] PAM: Check for trusted domain before sending the
 request to BE

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

Moving the checks to one place has the advantage of not duplicating
security decisions. Previously, the checks were scattered all over the
responder code, making testing hard.

The disadvantage is that we actually check for the presence of the user,
which might trigger some back end lookups. But I think the benefits
overweight the disadvantage.

Also only check the requested domains from a trusted client. An untrusted
client should simply have no say in what domains he wants to talk to, it
should ignore the 'domains' option.

Reviewed-by: Sumit Bose <sbose@redhat.com>
---
 src/responder/pam/pamsrv_cmd.c | 67 ++++++++++++++++--------------------------
 1 file changed, 26 insertions(+), 41 deletions(-)

diff --git a/src/responder/pam/pamsrv_cmd.c b/src/responder/pam/pamsrv_cmd.c
index dd0e6e4cf54e6d98da807808ad6973dddc013413..b60ccba2d4ff669e7ed0252923a53755410851e3 100644
--- a/src/responder/pam/pamsrv_cmd.c
+++ b/src/responder/pam/pamsrv_cmd.c
@@ -898,17 +898,6 @@ static int pam_forwarder(struct cli_ctx *cctx, int pam_cmd)
             goto done;
         }
 
-        /* Untrusted users can access only public domains. */
-        if (!pctx->is_uid_trusted &&
-            !is_domain_public(pd->domain, pctx->public_domains,
-                              pctx->public_domains_count)) {
-            DEBUG(SSSDBG_MINOR_FAILURE,
-                  "Untrusted user %"PRIu32" cannot access unpublic domain %s.\n",
-                  cctx->client_euid, pd->domain);
-            ret = EPERM;
-            goto done;
-        }
-
         ncret = sss_ncache_check_user(pctx->ncache, pctx->neg_timeout,
                                       preq->domain, pd->user);
         if (ncret == EEXIST) {
@@ -916,34 +905,12 @@ static int pam_forwarder(struct cli_ctx *cctx, int pam_cmd)
             ret = ENOENT;
             goto done;
         }
-
-        /* skip this domain if not requested */
-        if (!is_domain_requested(pd, pd->domain)) {
-            ret = ENOENT;
-            goto done;
-        }
     } else {
         for (dom = preq->cctx->rctx->domains;
              dom;
              dom = get_next_domain(dom, false)) {
             if (dom->fqnames) continue;
 
-            /* Untrusted users can access only public domains. */
-            if (!pctx->is_uid_trusted &&
-                !is_domain_public(dom->name, pctx->public_domains,
-                                  pctx->public_domains_count)) {
-                DEBUG(SSSDBG_MINOR_FAILURE,
-                      "Untrusted user %"PRIu32" cannot access unpublic domain %s."
-                      " Trying next domain.\n",
-                      cctx->client_euid, dom->name);
-                continue;
-            }
-
-            /* skip this domain if not requested */
-            if (!is_domain_requested(pd, dom->name)) {
-                continue;
-            }
-
             ncret = sss_ncache_check_user(pctx->ncache, pctx->neg_timeout,
                                           dom, pd->user);
             if (ncret == ENOENT) {
@@ -959,7 +926,7 @@ static int pam_forwarder(struct cli_ctx *cctx, int pam_cmd)
                    "Trying next domain.\n", pd->user, dom->name);
         }
 
-        if (!dom || !is_domain_requested(pd, dom->name)) {
+        if (!dom) {
             ret = ENOENT;
             goto done;
         }
@@ -1055,14 +1022,9 @@ static int pam_check_user_search(struct pam_auth_req *preq)
 
     while (dom) {
        /* if it is a domainless search, skip domains that require fully
-        * qualified names instead, also untrusted users can access only
-        * public domains */
+        * qualified names instead */
         while (dom && !preq->pd->domain && !preq->pd->name_is_upn
-               && (dom->fqnames ||
-                   (!pctx->is_uid_trusted &&
-                    !is_domain_public(dom->name,
-                                      pctx->public_domains,
-                                      pctx->public_domains_count)))) {
+               && dom->fqnames) {
             dom = get_next_domain(dom, false);
         }
 
@@ -1334,11 +1296,34 @@ done:
 static void pam_dom_forwarder(struct pam_auth_req *preq)
 {
     int ret;
+    struct pam_ctx *pctx =
+            talloc_get_type(preq->cctx->rctx->pvt_ctx, struct pam_ctx);
 
     if (!preq->pd->domain) {
         preq->pd->domain = preq->domain->name;
     }
 
+    /* Untrusted users can access only public domains. */
+    if (!pctx->is_uid_trusted &&
+            !is_domain_public(preq->pd->domain, pctx->public_domains,
+                            pctx->public_domains_count)) {
+        DEBUG(SSSDBG_MINOR_FAILURE,
+                "Untrusted user %"PRIu32" cannot access non-public domain %s.\n",
+                preq->cctx->client_euid, preq->pd->domain);
+        preq->pd->pam_status = PAM_PERM_DENIED;
+        pam_reply(preq);
+        return;
+    }
+
+    /* skip this domain if not requested and the user is trusted
+     * as untrusted users can't request a domain */
+    if (pctx->is_uid_trusted &&
+            !is_domain_requested(preq->pd, preq->pd->domain)) {
+        preq->pd->pam_status = PAM_USER_UNKNOWN;
+        pam_reply(preq);
+        return;
+    }
+
     if (!NEED_CHECK_PROVIDER(preq->domain->provider)) {
         preq->callback = pam_reply;
         ret = LOCAL_pam_handler(preq);
-- 
1.9.3