[SSSD] [PATCH] Patches related to restricting domain access from PAM services

Jakub Hrozek jhrozek at redhat.com
Thu Dec 11 10:41:41 UTC 2014


Hi,

after further testing we realized there are some more bugs in the PAM
responder related to restricting users.

[PATCH 1/4] PAM: Domain names are case-insensitive
This bug was mostly hitting subdomain users in my tests.

[PATCH 2/4] PAM: Missing argument to domains= should fail auth
Prevents admin from misconfiguring his PAM stack in an insecure way.

[PATCH 3/4] MAN: Misspelled username in pam_trusted_users is not fatal
The man page described behaviour that wasn't implemented (and shouldn't
be)

[PATCH 4/4] RESPONDER: Log failures to resolve user names in csv_string_to_uid_array
Makes it easier to discover misspelled user names in the user lists.

I'm sorry there are no unit test (yet), I'm working on implementing
them:
https://fedorapeople.org/cgit/jhrozek/public_git/sssd.git/log/?h=pam

But the progress is slow and we need to apply the patches to downstream.
-------------- next part --------------
>From 843770764688674da43b0a2bbd73de5ee60575dc Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Wed, 10 Dec 2014 11:35:18 +0100
Subject: [PATCH 1/4] PAM: Domain names are case-insensitive

The pam_public_domains option and matching the domain requested by a
trusted process was done in a case-sensitive manner which is different
from how we match domain names in SSSD normally.
---
 src/responder/pam/pamsrv_cmd.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/responder/pam/pamsrv_cmd.c b/src/responder/pam/pamsrv_cmd.c
index a924c4da570757a82d10c538644748f469446630..7cab7b2b4be99833d1446a25e1411898a9e5e67b 100644
--- a/src/responder/pam/pamsrv_cmd.c
+++ b/src/responder/pam/pamsrv_cmd.c
@@ -56,7 +56,7 @@ static bool is_domain_requested(struct pam_data *pd, const char *domain_name)
     }
 
     for (i = 0; pd->requested_domains[i]; i++) {
-        if (strcmp(domain_name, pd->requested_domains[i])) {
+        if (strcasecmp(domain_name, pd->requested_domains[i])) {
             continue;
         }
 
@@ -831,7 +831,7 @@ static bool is_domain_public(char *name,
     size_t i;
 
     for(i=0; i < public_dom_names_count; i++) {
-        if (strcmp(name, public_dom_names[i]) == 0) {
+        if (strcasecmp(name, public_dom_names[i]) == 0) {
             return true;
         }
     }
-- 
1.9.3

-------------- next part --------------
>From 17bea1dae977dee6116119cecde77f8517ebd8ba Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Wed, 10 Dec 2014 12:02:47 +0100
Subject: [PATCH 2/4] PAM: Missing argument to domains= should fail auth

When the administrator sets the domains= list, he usually wants to
restrict the set of domains. An empty list is an undefined configuration
and it's safer to fail then.
---
 src/sss_client/pam_sss.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/src/sss_client/pam_sss.c b/src/sss_client/pam_sss.c
index d64e826daeb80be8998ef3b410047e3a44051b07..fdf6c9e6da75c9f7eaa7c00d9a5792fbdd97eabc 100644
--- a/src/sss_client/pam_sss.c
+++ b/src/sss_client/pam_sss.c
@@ -1487,6 +1487,12 @@ static int pam_sss(enum sss_cli_command task, pam_handle_t *pamh,
 
     eval_argv(pamh, argc, argv, &flags, &retries, &quiet_mode, &domains);
 
+    /* Fail all authentication on misconfigured domains= parameter. The admin
+     * probably wanted to restrict authentication, so it's safer to fail */
+    if (domains && strcmp(domains, "") == 0) {
+        return PAM_SYSTEM_ERR;
+    }
+
     pi.requested_domains = domains;
 
     ret = get_pam_items(pamh, &pi);
-- 
1.9.3

-------------- next part --------------
>From 077971dc3d8cefc8df123ed1e1018f06d7679545 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Thu, 11 Dec 2014 11:19:58 +0100
Subject: [PATCH 3/4] MAN: Misspelled username in pam_trusted_users is not
 fatal

The man page claimed that failing to resolve an user name results in
failure to start SSSD, but it's not the case and shouldn't be, because
marking a user as trusted only elevates privileges, so it's safe to
ignore that failure.
---
 src/man/sssd.conf.5.xml | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/src/man/sssd.conf.5.xml b/src/man/sssd.conf.5.xml
index 3525d78caff28fd05cc18061d023fec3c50d1a47..2002ccc7caf7013ead5b97c463fba46b734090ae 100644
--- a/src/man/sssd.conf.5.xml
+++ b/src/man/sssd.conf.5.xml
@@ -907,11 +907,6 @@ fallback_homedir = /home/%u
                             the PAM responder even in case it is not in the
                             pam_trusted_users list.
                         </para>
-                        <para>
-                            Also please note that if there is a user name in
-                            pam_trusted_users list which fails to be resolved
-                            it will cause that SSSD will not be started.
-                        </para>
                     </listitem>
                 </varlistentry>
                 <varlistentry>
-- 
1.9.3

-------------- next part --------------
>From fbb9f3d657ac5bd4692bda266e7780265e38967f Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Thu, 11 Dec 2014 11:28:57 +0100
Subject: [PATCH 4/4] RESPONDER: Log failures to resolve user names in
 csv_string_to_uid_array

This patch makes it more discoverable for the admin to find typos in the
various user lists. Typically, the user lists are used to add access to
some feature and printing a syslog message would make sure the admin
sees the mistake.
---
 src/responder/common/responder_common.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/responder/common/responder_common.c b/src/responder/common/responder_common.c
index 6646fa2587a8299de40eaef35830351136b8149a..a5a44478759e0d515e8437c3bb9bcd78dbb1e4f5 100644
--- a/src/responder/common/responder_common.c
+++ b/src/responder/common/responder_common.c
@@ -215,6 +215,9 @@ errno_t csv_string_to_uid_array(TALLOC_CTX *mem_ctx, const char *csv_string,
                 DEBUG(SSSDBG_OP_FAILURE, "List item [%s] is neither a valid "
                                          "UID nor a user name which could be "
                                          "resolved by getpwnam().\n", list[c]);
+                sss_log(SSS_LOG_WARNING, "List item [%s] is neither a valid "
+                                         "UID nor a user name which could be "
+                                         "resolved by getpwnam().\n", list[c]);
                 goto done;
             }
         }
-- 
1.9.3



More information about the sssd-devel mailing list