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>

1 files changed, 26 insertions, 41 deletions

diff --git a/src/responder/pam/pamsrv_cmd.c b/src/responder/pam/pamsrv_cmd.c
index dd0e6e4cf..b60ccba2d 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);