sss_client: make code thread-safe

Add mutexes around nss operations and serialize them.
This is necessary because nss operations may have global state.

For pam it is sufficient to protect socket operations instead.
As pam functions use only the provided pam handler.

Fixes: https://fedorahosted.org/sssd/ticket/640

5 files changed, 219 insertions, 58 deletions

diff --git a/src/sss_client/common.c b/src/sss_client/common.c
index 7477c313e..0efdbdf37 100644
--- a/src/sss_client/common.c
+++ b/src/sss_client/common.c
@@ -45,6 +45,10 @@
 #include "config.h"
 #include "sss_cli.h"
 
+#if HAVE_PTHREAD
+#include <pthread.h>
+#endif
+
 /* common functions */
 
 int sss_cli_sd = -1; /* the sss client socket descriptor */
@@ -686,50 +690,67 @@ int sss_pam_make_request(enum sss_cli_command cmd,
     char *envval;
     struct stat stat_buf;
 
+    sss_pam_lock();
+
     /* avoid looping in the pam daemon */
     envval = getenv("_SSS_LOOPS");
     if (envval && strcmp(envval, "NO") == 0) {
-        return PAM_SERVICE_ERR;
+        ret = PAM_SERVICE_ERR;
+        goto out;
     }
 
     /* only root shall use the privileged pipe */
     if (getuid() == 0 && getgid() == 0) {
         ret = stat(SSS_PAM_PRIV_SOCKET_NAME, &stat_buf);
-        if (ret != 0) return PAM_SERVICE_ERR;
+        if (ret != 0) {
+            ret = PAM_SERVICE_ERR;
+            goto out;
+        }
         if ( ! (stat_buf.st_uid == 0 &&
                 stat_buf.st_gid == 0 &&
                 S_ISSOCK(stat_buf.st_mode) &&
                 (stat_buf.st_mode & ~S_IFMT) == 0600 )) {
             *errnop = ESSS_BAD_PRIV_SOCKET;
-            return PAM_SERVICE_ERR;
+            ret = PAM_SERVICE_ERR;
+            goto out;
         }
 
         ret = sss_cli_check_socket(errnop, SSS_PAM_PRIV_SOCKET_NAME);
     } else {
         ret = stat(SSS_PAM_SOCKET_NAME, &stat_buf);
-        if (ret != 0) return PAM_SERVICE_ERR;
+        if (ret != 0) {
+            ret = PAM_SERVICE_ERR;
+            goto out;
+        }
         if ( ! (stat_buf.st_uid == 0 &&
                 stat_buf.st_gid == 0 &&
                 S_ISSOCK(stat_buf.st_mode) &&
                 (stat_buf.st_mode & ~S_IFMT) == 0666 )) {
             *errnop = ESSS_BAD_PUB_SOCKET;
-            return PAM_SERVICE_ERR;
+            ret = PAM_SERVICE_ERR;
+            goto out;
         }
 
         ret = sss_cli_check_socket(errnop, SSS_PAM_SOCKET_NAME);
     }
     if (ret != NSS_STATUS_SUCCESS) {
-        return PAM_SERVICE_ERR;
+        ret = PAM_SERVICE_ERR;
+        goto out;
     }
 
     ret = check_server_cred(sss_cli_sd);
     if (ret != 0) {
         sss_cli_close_socket();
         *errnop = ret;
-        return PAM_SERVICE_ERR;
+        ret = PAM_SERVICE_ERR;
+        goto out;
     }
 
-    return sss_nss_make_request_nochecks(cmd, rd, repbuf, replen, errnop);
+    ret = sss_nss_make_request_nochecks(cmd, rd, repbuf, replen, errnop);
+
+out:
+    sss_pam_unlock();
+    return ret;
 }
 
 
@@ -790,3 +811,34 @@ errno_t sss_strnlen(const char *str, size_t maxlen, size_t *len)
 
     return 0;
 }
+
+#if HAVE_PTHREAD
+static pthread_mutex_t sss_nss_mutex = PTHREAD_MUTEX_INITIALIZER;
+static pthread_mutex_t sss_pam_mutex = PTHREAD_MUTEX_INITIALIZER;
+
+void sss_nss_lock(void)
+{
+    pthread_mutex_lock(&sss_nss_mutex);
+}
+void sss_nss_unlock(void)
+{
+    pthread_mutex_unlock(&sss_nss_mutex);
+}
+void sss_pam_lock(void)
+{
+    pthread_mutex_lock(&sss_pam_mutex);
+}
+void sss_pam_unlock(void)
+{
+    pthread_mutex_unlock(&sss_pam_mutex);
+}
+
+#else
+
+/* sorry no mutexes available */
+void sss_nss_lock(void) { return; }
+void sss_nss_unlock(void) { return; }
+void sss_pam_lock(void) { return; }
+void sss_pam_unlock(void) { return; }
+#endif
+
diff --git a/src/sss_client/nss_group.c b/src/sss_client/nss_group.c
index 60b4611c2..9e308c929 100644
--- a/src/sss_client/nss_group.c
+++ b/src/sss_client/nss_group.c
@@ -194,17 +194,20 @@ enum nss_status _nss_sss_initgroups_dyn(const char *user, gid_t group,
     rd.len = strlen(user) +1;
     rd.data = user;
 
+    sss_nss_lock();
+
     nret = sss_nss_make_request(SSS_NSS_INITGR, &rd,
                                 &repbuf, &replen, errnop);
     if (nret != NSS_STATUS_SUCCESS) {
-        return nret;
+        goto out;
     }
 
     /* no results if not found */
     num_ret = ((uint32_t *)repbuf)[0];
     if (num_ret == 0) {
         free(repbuf);
-        return NSS_STATUS_NOTFOUND;
+        nret = NSS_STATUS_NOTFOUND;
+        goto out;
     }
     max_ret = num_ret;
 
@@ -223,7 +226,8 @@ enum nss_status _nss_sss_initgroups_dyn(const char *user, gid_t group,
         if (!newgroups) {
             *errnop = ENOMEM;
             free(repbuf);
-            return NSS_STATUS_TRYAGAIN;
+            nret = NSS_STATUS_TRYAGAIN;
+            goto out;
         }
         *groups = newgroups;
         *size = newsize;
@@ -236,7 +240,11 @@ enum nss_status _nss_sss_initgroups_dyn(const char *user, gid_t group,
     }
 
     free(repbuf);
-    return NSS_STATUS_SUCCESS;
+    nret = NSS_STATUS_SUCCESS;
+
+out:
+    sss_nss_unlock();
+    return nret;
 }
 
 
@@ -256,10 +264,12 @@ enum nss_status _nss_sss_getgrnam_r(const char *name, struct group *result,
     rd.len = strlen(name) + 1;
     rd.data = name;
 
+    sss_nss_lock();
+
     nret = sss_nss_make_request(SSS_NSS_GETGRNAM, &rd,
                                 &repbuf, &replen, errnop);
     if (nret != NSS_STATUS_SUCCESS) {
-        return nret;
+        goto out;
     }
 
     grrep.result = result;
@@ -269,14 +279,16 @@ enum nss_status _nss_sss_getgrnam_r(const char *name, struct group *result,
     /* no results if not found */
     if (((uint32_t *)repbuf)[0] == 0) {
         free(repbuf);
-        return NSS_STATUS_NOTFOUND;
+        nret = NSS_STATUS_NOTFOUND;
+        goto out;
     }
 
     /* only 1 result is accepted for this function */
     if (((uint32_t *)repbuf)[0] != 1) {
         *errnop = EBADMSG;
         free(repbuf);
-        return NSS_STATUS_TRYAGAIN;
+        nret = NSS_STATUS_TRYAGAIN;
+        goto out;
     }
 
     len = replen - 8;
@@ -284,10 +296,15 @@ enum nss_status _nss_sss_getgrnam_r(const char *name, struct group *result,
     free(repbuf);
     if (ret) {
         *errnop = ret;
-        return NSS_STATUS_TRYAGAIN;
+        nret = NSS_STATUS_TRYAGAIN;
+        goto out;
     }
 
-    return NSS_STATUS_SUCCESS;
+    nret = NSS_STATUS_SUCCESS;
+
+out:
+    sss_nss_unlock();
+    return nret;
 }
 
 enum nss_status _nss_sss_getgrgid_r(gid_t gid, struct group *result,
@@ -308,10 +325,12 @@ enum nss_status _nss_sss_getgrgid_r(gid_t gid, struct group *result,
     rd.len = sizeof(uint32_t);
     rd.data = &group_gid;
 
+    sss_nss_lock();
+
     nret = sss_nss_make_request(SSS_NSS_GETGRGID, &rd,
                                 &repbuf, &replen, errnop);
     if (nret != NSS_STATUS_SUCCESS) {
-        return nret;
+        goto out;
     }
 
     grrep.result = result;
@@ -321,14 +340,16 @@ enum nss_status _nss_sss_getgrgid_r(gid_t gid, struct group *result,
     /* no results if not found */
     if (((uint32_t *)repbuf)[0] == 0) {
         free(repbuf);
-        return NSS_STATUS_NOTFOUND;
+        nret = NSS_STATUS_NOTFOUND;
+        goto out;
     }
 
     /* only 1 result is accepted for this function */
     if (((uint32_t *)repbuf)[0] != 1) {
         *errnop = EBADMSG;
         free(repbuf);
-        return NSS_STATUS_TRYAGAIN;
+        nret = NSS_STATUS_TRYAGAIN;
+        goto out;
     }
 
     len = replen - 8;
@@ -336,10 +357,15 @@ enum nss_status _nss_sss_getgrgid_r(gid_t gid, struct group *result,
     free(repbuf);
     if (ret) {
         *errnop = ret;
-        return NSS_STATUS_TRYAGAIN;
+        nret = NSS_STATUS_TRYAGAIN;
+        goto out;
     }
 
-    return NSS_STATUS_SUCCESS;
+    nret = NSS_STATUS_SUCCESS;
+
+out:
+    sss_nss_unlock();
+    return nret;
 }
 
 enum nss_status _nss_sss_setgrent(void)
@@ -347,6 +373,8 @@ enum nss_status _nss_sss_setgrent(void)
     enum nss_status nret;
     int errnop;
 
+    sss_nss_lock();
+
     /* make sure we do not have leftovers, and release memory */
     sss_nss_getgrent_data_clean();
 
@@ -354,14 +382,15 @@ enum nss_status _nss_sss_setgrent(void)
                                 NULL, NULL, NULL, &errnop);
     if (nret != NSS_STATUS_SUCCESS) {
         errno = errnop;
-        return nret;
     }
 
-    return NSS_STATUS_SUCCESS;
+    sss_nss_unlock();
+    return nret;
 }
 
-enum nss_status _nss_sss_getgrent_r(struct group *result,
-                                    char *buffer, size_t buflen, int *errnop)
+static enum nss_status internal_getgrent_r(struct group *result,
+                                           char *buffer, size_t buflen,
+                                           int *errnop)
 {
     struct sss_cli_req_data rd;
     struct sss_nss_gr_rep grrep;
@@ -424,7 +453,19 @@ enum nss_status _nss_sss_getgrent_r(struct group *result,
     sss_nss_getgrent_data.ptr = 8; /* skip metadata fields */
 
     /* call again ourselves, this will return the first result */
-    return _nss_sss_getgrent_r(result, buffer, buflen, errnop);
+    return internal_getgrent_r(result, buffer, buflen, errnop);
+}
+
+enum nss_status _nss_sss_getgrent_r(struct group *result,
+                                    char *buffer, size_t buflen, int *errnop)
+{
+    enum nss_status nret;
+
+    sss_nss_lock();
+    nret = internal_getgrent_r(result, buffer, buflen, errnop);
+    sss_nss_unlock();
+
+    return nret;
 }
 
 enum nss_status _nss_sss_endgrent(void)
@@ -432,6 +473,8 @@ enum nss_status _nss_sss_endgrent(void)
     enum nss_status nret;
     int errnop;
 
+    sss_nss_lock();
+
     /* make sure we do not have leftovers, and release memory */
     sss_nss_getgrent_data_clean();
 
@@ -439,8 +482,8 @@ enum nss_status _nss_sss_endgrent(void)
                                 NULL, NULL, NULL, &errnop);
     if (nret != NSS_STATUS_SUCCESS) {
         errno = errnop;
-        return nret;
     }
 
-    return NSS_STATUS_SUCCESS;
+    sss_nss_unlock();
+    return nret;
 }
diff --git a/src/sss_client/nss_netgroup.c b/src/sss_client/nss_netgroup.c
index 158e37b4c..2d1acc5df 100644
--- a/src/sss_client/nss_netgroup.c
+++ b/src/sss_client/nss_netgroup.c
@@ -196,15 +196,21 @@ enum nss_status _nss_sss_setnetgrent(const char *netgroup,
 
     if (!netgroup) return NSS_STATUS_NOTFOUND;
 
+    sss_nss_lock();
+
     /* make sure we do not have leftovers, and release memory */
     CLEAR_NETGRENT_DATA(result);
 
     ret = sss_strnlen(netgroup, MAX_NETGR_NAME_LENGTH, &name_len);
-    if (ret != 0) return NSS_STATUS_NOTFOUND;
+    if (ret != 0) {
+        nret = NSS_STATUS_NOTFOUND;
+        goto out;
+    }
 
     name = malloc(sizeof(char)*name_len + 1);
     if (name == NULL) {
-        return NSS_STATUS_TRYAGAIN;
+        nret = NSS_STATUS_TRYAGAIN;
+        goto out;
     }
     strncpy(name, netgroup, name_len + 1);
 
@@ -216,22 +222,27 @@ enum nss_status _nss_sss_setnetgrent(const char *netgroup,
     free(name);
     if (nret != NSS_STATUS_SUCCESS) {
         errno = errnop;
-        return nret;
+        goto out;
     }
 
     /* no results if not found */
     if ((((uint32_t *)repbuf)[0] == 0) || (replen < NETGR_METADATA_COUNT)) {
         free(repbuf);
-        return NSS_STATUS_NOTFOUND;
+        nret = NSS_STATUS_NOTFOUND;
+        goto out;
     }
 
     free(repbuf);
-    return NSS_STATUS_SUCCESS;
+    nret = NSS_STATUS_SUCCESS;
+
+out:
+    sss_nss_unlock();
+    return nret;
 }
 
-enum nss_status _nss_sss_getnetgrent_r(struct __netgrent *result,
-                       char *buffer, size_t buflen,
-                       int *errnop)
+static enum nss_status internal_getnetgrent_r(struct __netgrent *result,
+                                              char *buffer, size_t buflen,
+                                              int *errnop)
 {
     struct sss_cli_req_data rd;
     struct sss_nss_netgr_rep netgrrep;
@@ -294,7 +305,20 @@ enum nss_status _nss_sss_getnetgrent_r(struct __netgrent *result,
     result->idx.position = NETGR_METADATA_COUNT;
 
     /* call again ourselves, this will return the first result */
-    return _nss_sss_getnetgrent_r(result, buffer, buflen, errnop);
+    return internal_getnetgrent_r(result, buffer, buflen, errnop);
+}
+
+enum nss_status _nss_sss_getnetgrent_r(struct __netgrent *result,
+                       char *buffer, size_t buflen,
+                       int *errnop)
+{
+    enum nss_status nret;
+
+    sss_nss_lock();
+    nret = internal_getnetgrent_r(result, buffer, buflen, errnop);
+    sss_nss_unlock();
+
+    return nret;
 }
 
 enum nss_status _nss_sss_endnetgrent(struct __netgrent *result)
@@ -302,6 +326,8 @@ enum nss_status _nss_sss_endnetgrent(struct __netgrent *result)
     enum nss_status nret;
     int errnop;
 
+    sss_nss_lock();
+
     /* make sure we do not have leftovers, and release memory */
     CLEAR_NETGRENT_DATA(result);
 
@@ -309,8 +335,8 @@ enum nss_status _nss_sss_endnetgrent(struct __netgrent *result)
                                 NULL, NULL, NULL, &errnop);
     if (nret != NSS_STATUS_SUCCESS) {
         errno = errnop;
-        return nret;
     }
 
-    return NSS_STATUS_SUCCESS;
+    sss_nss_unlock();
+    return nret;
 }
diff --git a/src/sss_client/nss_passwd.c b/src/sss_client/nss_passwd.c
index 95c7de619..31b9a7949 100644
--- a/src/sss_client/nss_passwd.c
+++ b/src/sss_client/nss_passwd.c
@@ -189,10 +189,12 @@ enum nss_status _nss_sss_getpwnam_r(const char *name, struct passwd *result,
     rd.len = strlen(name) + 1;
     rd.data = name;
 
+    sss_nss_lock();
+
     nret = sss_nss_make_request(SSS_NSS_GETPWNAM, &rd,
                                 &repbuf, &replen, errnop);
     if (nret != NSS_STATUS_SUCCESS) {
-        return nret;
+        goto out;
     }
 
     pwrep.result = result;
@@ -202,14 +204,16 @@ enum nss_status _nss_sss_getpwnam_r(const char *name, struct passwd *result,
     /* no results if not found */
     if (((uint32_t *)repbuf)[0] == 0) {
         free(repbuf);
-        return NSS_STATUS_NOTFOUND;
+        nret = NSS_STATUS_NOTFOUND;
+        goto out;
     }
 
     /* only 1 result is accepted for this function */
     if (((uint32_t *)repbuf)[0] != 1) {
         *errnop = EBADMSG;
         free(repbuf);
-        return NSS_STATUS_TRYAGAIN;
+        nret = NSS_STATUS_TRYAGAIN;
+        goto out;
     }
 
     len = replen - 8;
@@ -217,10 +221,15 @@ enum nss_status _nss_sss_getpwnam_r(const char *name, struct passwd *result,
     free(repbuf);
     if (ret) {
         *errnop = ret;
-        return NSS_STATUS_TRYAGAIN;
+        nret = NSS_STATUS_TRYAGAIN;
+        goto out;
     }
 
-    return NSS_STATUS_SUCCESS;
+    nret = NSS_STATUS_SUCCESS;
+
+out:
+    sss_nss_unlock();
+    return nret;
 }
 
 enum nss_status _nss_sss_getpwuid_r(uid_t uid, struct passwd *result,
@@ -241,10 +250,12 @@ enum nss_status _nss_sss_getpwuid_r(uid_t uid, struct passwd *result,
     rd.len = sizeof(uint32_t);
     rd.data = &user_uid;
 
+    sss_nss_lock();
+
     nret = sss_nss_make_request(SSS_NSS_GETPWUID, &rd,
                                 &repbuf, &replen, errnop);
     if (nret != NSS_STATUS_SUCCESS) {
-        return nret;
+        goto out;
     }
 
     pwrep.result = result;
@@ -254,14 +265,16 @@ enum nss_status _nss_sss_getpwuid_r(uid_t uid, struct passwd *result,
     /* no results if not found */
     if (((uint32_t *)repbuf)[0] == 0) {
         free(repbuf);
-        return NSS_STATUS_NOTFOUND;
+        nret = NSS_STATUS_NOTFOUND;
+        goto out;
     }
 
     /* only 1 result is accepted for this function */
     if (((uint32_t *)repbuf)[0] != 1) {
         *errnop = EBADMSG;
         free(repbuf);
-        return NSS_STATUS_TRYAGAIN;
+        nret = NSS_STATUS_TRYAGAIN;
+        goto out;
     }
 
     len = replen - 8;
@@ -269,10 +282,15 @@ enum nss_status _nss_sss_getpwuid_r(uid_t uid, struct passwd *result,
     free(repbuf);
     if (ret) {
         *errnop = ret;
-        return NSS_STATUS_TRYAGAIN;
+        nret = NSS_STATUS_TRYAGAIN;
+        goto out;
     }
 
-    return NSS_STATUS_SUCCESS;
+    nret = NSS_STATUS_SUCCESS;
+
+out:
+    sss_nss_unlock();
+    return nret;
 }
 
 enum nss_status _nss_sss_setpwent(void)
@@ -280,6 +298,8 @@ enum nss_status _nss_sss_setpwent(void)
     enum nss_status nret;
     int errnop;
 
+    sss_nss_lock();
+
     /* make sure we do not have leftovers, and release memory */
     sss_nss_getpwent_data_clean();
 
@@ -287,15 +307,15 @@ enum nss_status _nss_sss_setpwent(void)
                                 NULL, NULL, NULL, &errnop);
     if (nret != NSS_STATUS_SUCCESS) {
         errno = errnop;
-        return nret;
     }
 
-    return NSS_STATUS_SUCCESS;
+    sss_nss_unlock();
+    return nret;
 }
 
-enum nss_status _nss_sss_getpwent_r(struct passwd *result,
-                                    char *buffer, size_t buflen,
-                                    int *errnop)
+static enum nss_status internal_getpwent_r(struct passwd *result,
+                                           char *buffer, size_t buflen,
+                                           int *errnop)
 {
     struct sss_cli_req_data rd;
     struct sss_nss_pw_rep pwrep;
@@ -356,7 +376,20 @@ enum nss_status _nss_sss_getpwent_r(struct passwd *result,
     sss_nss_getpwent_data.ptr = 8; /* skip metadata fields */
 
     /* call again ourselves, this will return the first result */
-    return _nss_sss_getpwent_r(result, buffer, buflen, errnop);
+    return internal_getpwent_r(result, buffer, buflen, errnop);
+}
+
+enum nss_status _nss_sss_getpwent_r(struct passwd *result,
+                                    char *buffer, size_t buflen,
+                                    int *errnop)
+{
+    enum nss_status nret;
+
+    sss_nss_lock();
+    nret = internal_getpwent_r(result, buffer, buflen, errnop);
+    sss_nss_unlock();
+
+    return nret;
 }
 
 enum nss_status _nss_sss_endpwent(void)
@@ -364,6 +397,8 @@ enum nss_status _nss_sss_endpwent(void)
     enum nss_status nret;
     int errnop;
 
+    sss_nss_lock();
+
     /* make sure we do not have leftovers, and release memory */
     sss_nss_getpwent_data_clean();
 
@@ -371,8 +406,8 @@ enum nss_status _nss_sss_endpwent(void)
                                 NULL, NULL, NULL, &errnop);
     if (nret != NSS_STATUS_SUCCESS) {
         errno = errnop;
-        return nret;
     }
 
-    return NSS_STATUS_SUCCESS;
+    sss_nss_unlock();
+    return nret;
 }
diff --git a/src/sss_client/sss_cli.h b/src/sss_client/sss_cli.h
index f8ccb4f5a..b0521067b 100644
--- a/src/sss_client/sss_cli.h
+++ b/src/sss_client/sss_cli.h
@@ -480,3 +480,8 @@ safealign_memcpy(void *dest, const void *src, size_t n, size_t *counter)
  * _len will return the result
  */
 errno_t sss_strnlen(const char *str, size_t maxlen, size_t *len);
+
+void sss_nss_lock(void);
+void sss_nss_unlock(void);
+void sss_pam_lock(void);
+void sss_pam_unlock(void);