[SSSD] [PATCH] mmap_cache: "Override" functions for initgr mmap cache

Lukas Slebodnik lslebodn at redhat.com
Mon Aug 3 12:08:29 UTC 2015


On (31/07/15 17:07), Michal Židek wrote:
>On 07/31/2015 04:54 PM, Michal Židek wrote:
>>On 07/30/2015 05:23 PM, Michal Židek wrote:
>>>On 07/30/2015 05:06 PM, Michal Židek wrote:
>>>>On 07/30/2015 04:15 PM, Lukas Slebodnik wrote:
>>>>>On (29/07/15 17:05), Lukas Slebodnik wrote:
>>>>>>Attached is a new patch set. I also contains fix for #2712 and not
>>>>>>just for
>>>>>>#2716. Otherwise there would be a crash. So it would be better
>>>>>>push them together.
>>>>>>
>>>>>
>>>>>Integration test were failing sporadically due to bug in clien code.
>>>>>3rd patch fix this problem.
>>>>>
>>>>>Other patches are the same.
>>>>>
>>>>>LS
>>>>>
>>>>
>>>>Integration tests still do not work for me even with
>>>>this version.
>>>>
>>>>$ contrib/ci/run -n -r
>>>>autoreconf:             success  00:00:14 ci-autoreconf.log
>>>>DEBUG BUILD:                              ci-build-debug
>>>>configure:              success  00:00:10
>>>>ci-build-debug/ci-configure.log
>>>>make-tests:             success  00:02:18
>>>>ci-build-debug/ci-make-tests.log
>>>>make-check-valgrind:    success  00:01:41
>>>>ci-build-debug/ci-make-check-valgrind.log
>>>>make-intgcheck:         failure  00:01:55
>>>>ci-build-debug/ci-make-intgcheck.log
>>>>FAILURE
>>>>
>>>>See sanitized log. These are the newly added _with_mc tests.
>>>>
>>>>Michal
>>>>
>>>
>>>Ah... nevermind, I had exported SSS_NSS_USE_MEMCACHE=no in
>>>the terminal where I was running the tests.
>>>
>>
>>On the other hand this proves that your tests really
>>catch situation when memcache does not work :)
>>
>>Thank you for expanding the tests coverage.
>>
>>Tentative ACK to the patches (will fully ack when
>>CI results finish).
>>
>>I filled a ticket to track one issue,
>>but it does not seem to be related to the memcache.
>>https://fedorahosted.org/sssd/ticket/2741
>>
>
>Well, it is related to the memcache. But not that
>memcache itself would not work. We somewhere miss
>call to clear the memcache in the code branch
>that 'su' triggers.
>
The bug #2761 was fixed just partialy by the second patch
"mmap_cache: "Override" functions for initgr mmap cache"

Initgroups mmap cache was update but it was a coincidence
caused by bug #2743 with validation of ingegrity check.
Which was fixed by another commit.

This is a reason why I opened ticket #2743. It is an important
bug which should be tracked and probably fixed in stable branches.
There are two possible solutions. Remove integrity check for sssd >= 1.12
or fix integrity check for sssd >= 1.11

Updated patches are attached.
BTW I would catch this bug as well with integration test but we do not have
a pam wrapper yet.

Thank you for review.

LS
-------------- next part --------------
>From 0d7a657a2ada0b0a90c3ecbf242de19a5f9bcfca Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik <lslebodn at redhat.com>
Date: Thu, 16 Jul 2015 16:54:00 +0200
Subject: [PATCH 1/8] mmap_cache: Rename variables

---
 src/responder/nss/nsssrv_mmap_cache.c | 16 ++++++++--------
 src/responder/nss/nsssrv_mmap_cache.h |  4 ++--
 src/sss_client/nss_mc_initgr.c        | 10 +++++-----
 src/util/mmap_cache.h                 |  2 +-
 4 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/src/responder/nss/nsssrv_mmap_cache.c b/src/responder/nss/nsssrv_mmap_cache.c
index ebda8ac6fab3dd87f5a1d8e43717bf7a5b5a9878..16bc926f3ba4f5ab096bd0fb43895edef2b57c50 100644
--- a/src/responder/nss/nsssrv_mmap_cache.c
+++ b/src/responder/nss/nsssrv_mmap_cache.c
@@ -959,8 +959,8 @@ done:
 
 errno_t sss_mmap_cache_initgr_store(struct sss_mc_ctx **_mcc,
                                     struct sized_string *name,
-                                    uint32_t memnum,
-                                    uint8_t *membuf)
+                                    uint32_t num_groups,
+                                    uint8_t *gids_buf)
 {
     struct sss_mc_ctx *mcc = *_mcc;
     struct sss_mc_rec *rec;
@@ -974,8 +974,8 @@ errno_t sss_mmap_cache_initgr_store(struct sss_mc_ctx **_mcc,
         return EINVAL;
     }
 
-    /* memnum + reserved + array of members + name*/
-    data_len = (2 + memnum) * sizeof(uint32_t) + name->len;
+    /* num_groups + reserved + array of gids + name*/
+    data_len = (2 + num_groups) * sizeof(uint32_t) + name->len;
     rec_len = sizeof(struct sss_mc_rec) + sizeof(struct sss_mc_initgr_data)
               + data_len;
     if (rec_len > mcc->dt_size) {
@@ -998,10 +998,10 @@ errno_t sss_mmap_cache_initgr_store(struct sss_mc_ctx **_mcc,
                             name->str, name->len, name->str, name->len);
 
     /* initgroups struct */
-    data->members = memnum;
-    memcpy(data->gids, membuf, memnum * sizeof(uint32_t));
-    memcpy(&data->gids[memnum], name->str, name->len);
-    data->name = MC_PTR_DIFF(&data->gids[memnum], data);
+    data->num_groups = num_groups;
+    memcpy(data->gids, gids_buf, num_groups * sizeof(uint32_t));
+    memcpy(&data->gids[num_groups], name->str, name->len);
+    data->name = MC_PTR_DIFF(&data->gids[num_groups], data);
 
     MC_LOWER_BARRIER(rec);
 
diff --git a/src/responder/nss/nsssrv_mmap_cache.h b/src/responder/nss/nsssrv_mmap_cache.h
index 3a6764dd36bf3346d789bb287b3a94df120f36ee..b09e4a6f8efa364ae7a6407bab9d8a2a2143c812 100644
--- a/src/responder/nss/nsssrv_mmap_cache.h
+++ b/src/responder/nss/nsssrv_mmap_cache.h
@@ -53,8 +53,8 @@ errno_t sss_mmap_cache_gr_store(struct sss_mc_ctx **_mcc,
 
 errno_t sss_mmap_cache_initgr_store(struct sss_mc_ctx **_mcc,
                                     struct sized_string *name,
-                                    uint32_t memnum,
-                                    uint8_t *membuf);
+                                    uint32_t num_groups,
+                                    uint8_t *gids_buf);
 
 errno_t sss_mmap_cache_pw_invalidate(struct sss_mc_ctx *mcc,
                                      struct sized_string *name);
diff --git a/src/sss_client/nss_mc_initgr.c b/src/sss_client/nss_mc_initgr.c
index bfb09d6550c310fbab254dc9b3ab7b306b7d3f06..e21b9f40aba00f9cc2385a561fc2bcc163c5791a 100644
--- a/src/sss_client/nss_mc_initgr.c
+++ b/src/sss_client/nss_mc_initgr.c
@@ -42,7 +42,7 @@ static errno_t sss_nss_mc_parse_result(struct sss_mc_rec *rec,
     struct sss_mc_initgr_data *data;
     time_t expire;
     long int i;
-    uint32_t gid_count;
+    uint32_t num_groups;
     long int max_ret;
 
     /* additional checks before filling result*/
@@ -53,15 +53,15 @@ static errno_t sss_nss_mc_parse_result(struct sss_mc_rec *rec,
     }
 
     data = (struct sss_mc_initgr_data *)rec->data;
-    gid_count = data->members;
-    max_ret = gid_count;
+    num_groups = data->num_groups;
+    max_ret = num_groups;
 
     /* check we have enough space in the buffer */
-    if ((*size - *start) < gid_count) {
+    if ((*size - *start) < num_groups) {
         long int newsize;
         gid_t *newgroups;
 
-        newsize = *size + gid_count;
+        newsize = *size + num_groups;
         if ((limit > 0) && (newsize > limit)) {
             newsize = limit;
             max_ret = newsize - *start;
diff --git a/src/util/mmap_cache.h b/src/util/mmap_cache.h
index 438e28a3d217041278fc1bb60aa553d098516035..a8703d5da4022e11c4bc2f3e2f8710e7f9d982bf 100644
--- a/src/util/mmap_cache.h
+++ b/src/util/mmap_cache.h
@@ -139,7 +139,7 @@ struct sss_mc_grp_data {
 
 struct sss_mc_initgr_data {
     rel_ptr_t name;         /* ptr to name string, rel. to struct base addr */
-    uint32_t members;       /* number of members in groups */
+    uint32_t num_groups;    /* number of groups */
     uint32_t gids[0];       /* array of all groups
                              * string with name is stored after gids */
 };
-- 
2.4.6

-------------- next part --------------
>From 946fed7fd5d7963e6f6eb21bce5cd1d05bc95838 Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik <lslebodn at redhat.com>
Date: Thu, 16 Jul 2015 17:00:12 +0200
Subject: [PATCH 2/8] mmap_cache: "Override" functions for initgr mmap cache

Functions sss_mc_get_strs_offset and sss_mc_get_strs_len provides
data about strings for individual memory caches (passwd, ...)
Their are used in generic responder mmap cache code to find a record
in mmap cache (sss_mc_find_record). Data provided from functions sss_mc_get_*
are used for checking the validity of record. So in case of corrupted record
the whole mmap cache can be invalidated.

Functions sss_mc_get_strs_offset and sss_mc_get_strs_len did not provide
data for initgroups mmap cache and therefore particular record could not be
invalidated.

Resolves:
https://fedorahosted.org/sssd/ticket/2716
---
 src/responder/nss/nsssrv_mmap_cache.c | 15 ++++++++++++---
 src/sss_client/nss_mc_initgr.c        | 16 +++++++++++-----
 src/util/mmap_cache.h                 |  6 +++++-
 3 files changed, 28 insertions(+), 9 deletions(-)

diff --git a/src/responder/nss/nsssrv_mmap_cache.c b/src/responder/nss/nsssrv_mmap_cache.c
index 16bc926f3ba4f5ab096bd0fb43895edef2b57c50..ab9e769b1f4d5d17a8c41429afce292298239bc5 100644
--- a/src/responder/nss/nsssrv_mmap_cache.c
+++ b/src/responder/nss/nsssrv_mmap_cache.c
@@ -475,6 +475,9 @@ static errno_t sss_mc_get_strs_offset(struct sss_mc_ctx *mcc,
     case SSS_MC_GROUP:
         *_offset = offsetof(struct sss_mc_grp_data, strs);
         return EOK;
+    case SSS_MC_INITGROUPS:
+        *_offset = offsetof(struct sss_mc_initgr_data, gids);
+        return EOK;
     default:
         DEBUG(SSSDBG_FATAL_FAILURE, "Unknown memory cache type.\n");
         return EINVAL;
@@ -492,6 +495,9 @@ static errno_t sss_mc_get_strs_len(struct sss_mc_ctx *mcc,
     case SSS_MC_GROUP:
         *_len = ((struct sss_mc_grp_data *)&rec->data)->strs_len;
         return EOK;
+    case SSS_MC_INITGROUPS:
+        *_len = ((struct sss_mc_initgr_data *)&rec->data)->data_len;
+        return EOK;
     default:
         DEBUG(SSSDBG_FATAL_FAILURE, "Unknown memory cache type.\n");
         return EINVAL;
@@ -974,8 +980,8 @@ errno_t sss_mmap_cache_initgr_store(struct sss_mc_ctx **_mcc,
         return EINVAL;
     }
 
-    /* num_groups + reserved + array of gids + name*/
-    data_len = (2 + num_groups) * sizeof(uint32_t) + name->len;
+    /* array of gids + name */
+    data_len = num_groups * sizeof(uint32_t) + name->len;
     rec_len = sizeof(struct sss_mc_rec) + sizeof(struct sss_mc_initgr_data)
               + data_len;
     if (rec_len > mcc->dt_size) {
@@ -998,10 +1004,13 @@ errno_t sss_mmap_cache_initgr_store(struct sss_mc_ctx **_mcc,
                             name->str, name->len, name->str, name->len);
 
     /* initgroups struct */
+    data->strs_len = name->len;
+    data->data_len = data_len;
+    data->reserved = MC_INVALID_VAL32;
     data->num_groups = num_groups;
     memcpy(data->gids, gids_buf, num_groups * sizeof(uint32_t));
     memcpy(&data->gids[num_groups], name->str, name->len);
-    data->name = MC_PTR_DIFF(&data->gids[num_groups], data);
+    data->strs = data->name = MC_PTR_DIFF(&data->gids[num_groups], data);
 
     MC_LOWER_BARRIER(rec);
 
diff --git a/src/sss_client/nss_mc_initgr.c b/src/sss_client/nss_mc_initgr.c
index e21b9f40aba00f9cc2385a561fc2bcc163c5791a..153617ea9c6489b7439b9676904b42b042f6697c 100644
--- a/src/sss_client/nss_mc_initgr.c
+++ b/src/sss_client/nss_mc_initgr.c
@@ -93,6 +93,7 @@ errno_t sss_nss_mc_initgroups_dyn(const char *name, size_t name_len,
     uint32_t hash;
     uint32_t slot;
     int ret;
+    const size_t data_offset = offsetof(struct sss_mc_initgr_data, gids);
     uint8_t *max_addr;
 
     ret = sss_nss_mc_get_ctx("initgroups", &initgr_mc_ctx);
@@ -128,16 +129,21 @@ errno_t sss_nss_mc_initgroups_dyn(const char *name, size_t name_len,
         }
 
         data = (struct sss_mc_initgr_data *)rec->data;
+        rec_name = (char *)data + data->name;
         /* Integrity check
-         * - array with gids must be within data_table
-         * - string must be within data_table */
-        if ((uint8_t *)data->gids > max_addr
-                || (uint8_t *)data + data->name + name_len > max_addr) {
+         * - name_len cannot be longer than all strings or data
+         * - data->name cannot point outside strings
+         * - all data must be within data_table
+         * - name must be within data_table */
+        if (name_len > data->data_len
+            || name_len > data->strs_len
+            || (data->strs + name_len) > (data_offset + data->data_len)
+            || (uint8_t *)data->gids + data->data_len > max_addr
+            || (uint8_t *)rec_name + name_len > max_addr) {
             ret = ENOENT;
             goto done;
         }
 
-        rec_name = (char *)data + data->name;
         if (strcmp(name, rec_name) == 0) {
             break;
         }
diff --git a/src/util/mmap_cache.h b/src/util/mmap_cache.h
index a8703d5da4022e11c4bc2f3e2f8710e7f9d982bf..b5917b3c0973276e43e9fe160cec7528b1224f8f 100644
--- a/src/util/mmap_cache.h
+++ b/src/util/mmap_cache.h
@@ -79,7 +79,7 @@ typedef uint32_t rel_ptr_t;
 
 
 #define SSS_MC_MAJOR_VNO    1
-#define SSS_MC_MINOR_VNO    0
+#define SSS_MC_MINOR_VNO    1
 
 #define SSS_MC_HEADER_UNINIT    0   /* after ftruncate or before reset */
 #define SSS_MC_HEADER_ALIVE     1   /* current and in use */
@@ -139,6 +139,10 @@ struct sss_mc_grp_data {
 
 struct sss_mc_initgr_data {
     rel_ptr_t name;         /* ptr to name string, rel. to struct base addr */
+    rel_ptr_t strs;         /* ptr to concatenation of all strings */
+    uint32_t reserved;
+    uint32_t strs_len;      /* length of strs */
+    uint32_t data_len;      /* all initgroups data len */
     uint32_t num_groups;    /* number of groups */
     uint32_t gids[0];       /* array of all groups
                              * string with name is stored after gids */
-- 
2.4.6

-------------- next part --------------
>From 48dbdc36badc6548a8dfa910c6a605b224823df4 Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik <lslebodn at redhat.com>
Date: Mon, 3 Aug 2015 12:58:03 +0200
Subject: [PATCH 3/8] mmap: Invalidate initgroups memory cache after any change

Initgroups memory cache was invalidated only in case on removed user.
it should be invalidated also after changes in group membership.

Resolves:
https://fedorahosted.org/sssd/ticket/2716
---
 src/responder/nss/nsssrv_cmd.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/src/responder/nss/nsssrv_cmd.c b/src/responder/nss/nsssrv_cmd.c
index 4505f2a990d8711aa880e20f9cf5c982a773921d..778618e4080b1342535408e1720349f5226b3252 100644
--- a/src/responder/nss/nsssrv_cmd.c
+++ b/src/responder/nss/nsssrv_cmd.c
@@ -3939,14 +3939,6 @@ void nss_update_initgr_memcache(struct nss_ctx *nctx,
                   ret, strerror(ret));
         }
 
-        ret = sss_mmap_cache_initgr_invalidate(nctx->initgr_mc_ctx,
-                                               &delete_name);
-        if (ret != EOK && ret != ENOENT) {
-            DEBUG(SSSDBG_CRIT_FAILURE,
-                  "Internal failure in memory cache code: %d [%s]\n",
-                  ret, strerror(ret));
-        }
-
         /* Also invalidate his groups */
         changed = true;
     } else {
@@ -3994,6 +3986,15 @@ void nss_update_initgr_memcache(struct nss_ctx *nctx,
                        ret, strerror(ret));
             }
         }
+
+        to_sized_string(&delete_name, name);
+        ret = sss_mmap_cache_initgr_invalidate(nctx->initgr_mc_ctx,
+                                               &delete_name);
+        if (ret != EOK && ret != ENOENT) {
+            DEBUG(SSSDBG_CRIT_FAILURE,
+                  "Internal failure in memory cache code: %d [%s]\n",
+                  ret, strerror(ret));
+        }
     }
 
 done:
-- 
2.4.6

-------------- next part --------------
>From fa360247690c3099589f0b3bc06eddeef1d3821d Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik <lslebodn at redhat.com>
Date: Thu, 30 Jul 2015 10:50:47 +0200
Subject: [PATCH 4/8] sss_client: Update integrity check of records in mmap
 cache

The function sss_nss_mc_get_record return copy of record from memory
cache in last argument. Because we should not access data directly
to avoid problems with consistency of record.
The function sss_nss_mc_get_record also check whether length of record
is within data area (with macro MC_CHECK_RECORD_LENGTH)

However we also tried to do the same check in functions sss_nss_mc_get{gr, pw}*
Pointer to end of strings in record was compared to pointer to the end
of data table. But these two pointers are not within the same allocated area
and does not make sense to compare them. Sometimes record can be allocated
before mmaped area and sometime after. Sometimes it will return cached data
and other time will fall back to responder.

Resolves:
https://fedorahosted.org/sssd/ticket/2743
---
 src/sss_client/nss_mc_group.c  | 19 ++++++++++---------
 src/sss_client/nss_mc_initgr.c | 26 +++++++++++++-------------
 src/sss_client/nss_mc_passwd.c | 20 ++++++++++----------
 3 files changed, 33 insertions(+), 32 deletions(-)

diff --git a/src/sss_client/nss_mc_group.c b/src/sss_client/nss_mc_group.c
index e0fdb97f628ac19741409be29566e4af5a391f74..aacf59d9fd8b81ea895f4660de08f3e44f0ce645 100644
--- a/src/sss_client/nss_mc_group.c
+++ b/src/sss_client/nss_mc_group.c
@@ -112,16 +112,16 @@ errno_t sss_nss_mc_getgrnam(const char *name, size_t name_len,
     uint32_t hash;
     uint32_t slot;
     int ret;
-    size_t strs_offset;
-    uint8_t *max_addr;
+    const size_t strs_offset = offsetof(struct sss_mc_grp_data, strs);
+    size_t data_size;
 
     ret = sss_nss_mc_get_ctx("group", &gr_mc_ctx);
     if (ret) {
         return ret;
     }
 
-    /* Get max address of data table. */
-    max_addr = gr_mc_ctx.data_table + gr_mc_ctx.dt_size;
+    /* Get max size of data table. */
+    data_size = gr_mc_ctx.dt_size;
 
     /* hashes are calculated including the NULL terminator */
     hash = sss_nss_mc_hash(&gr_mc_ctx, name, name_len + 1);
@@ -130,7 +130,7 @@ errno_t sss_nss_mc_getgrnam(const char *name, size_t name_len,
     /* If slot is not within the bounds of mmaped region and
      * it's value is not MC_INVALID_VAL, then the cache is
      * probbably corrupted. */
-    while (MC_SLOT_WITHIN_BOUNDS(slot, gr_mc_ctx.dt_size)) {
+    while (MC_SLOT_WITHIN_BOUNDS(slot, data_size)) {
         /* free record from previous iteration */
         free(rec);
         rec = NULL;
@@ -147,15 +147,16 @@ errno_t sss_nss_mc_getgrnam(const char *name, size_t name_len,
             continue;
         }
 
-        strs_offset = offsetof(struct sss_mc_grp_data, strs);
         data = (struct sss_mc_grp_data *)rec->data;
         /* Integrity check
          * - name_len cannot be longer than all strings
          * - data->name cannot point outside strings
-         * - all strings must be within data_table */
+         * - all strings must be within copy of record
+         * - size of record must be lower that data table size */
         if (name_len > data->strs_len
             || (data->name + name_len) > (strs_offset + data->strs_len)
-            || (uint8_t *)data->strs + data->strs_len > max_addr) {
+            || data->strs_len > rec->len
+            || rec->len > data_size) {
             ret = ENOENT;
             goto done;
         }
@@ -168,7 +169,7 @@ errno_t sss_nss_mc_getgrnam(const char *name, size_t name_len,
         slot = sss_nss_mc_next_slot_with_hash(rec, hash);
     }
 
-    if (!MC_SLOT_WITHIN_BOUNDS(slot, gr_mc_ctx.dt_size)) {
+    if (!MC_SLOT_WITHIN_BOUNDS(slot, data_size)) {
         ret = ENOENT;
         goto done;
     }
diff --git a/src/sss_client/nss_mc_initgr.c b/src/sss_client/nss_mc_initgr.c
index 153617ea9c6489b7439b9676904b42b042f6697c..74143d9fb3c674c3116d7f4cf0b4c03d993743a2 100644
--- a/src/sss_client/nss_mc_initgr.c
+++ b/src/sss_client/nss_mc_initgr.c
@@ -94,15 +94,15 @@ errno_t sss_nss_mc_initgroups_dyn(const char *name, size_t name_len,
     uint32_t slot;
     int ret;
     const size_t data_offset = offsetof(struct sss_mc_initgr_data, gids);
-    uint8_t *max_addr;
+    size_t data_size;
 
     ret = sss_nss_mc_get_ctx("initgroups", &initgr_mc_ctx);
     if (ret) {
         return ret;
     }
 
-    /* Get max address of data table. */
-    max_addr = initgr_mc_ctx.data_table + initgr_mc_ctx.dt_size;
+    /* Get max size of data table. */
+    data_size = initgr_mc_ctx.dt_size;
 
     /* hashes are calculated including the NULL terminator */
     hash = sss_nss_mc_hash(&initgr_mc_ctx, name, name_len + 1);
@@ -111,7 +111,7 @@ errno_t sss_nss_mc_initgroups_dyn(const char *name, size_t name_len,
     /* If slot is not within the bounds of mmaped region and
      * it's value is not MC_INVALID_VAL, then the cache is
      * probbably corrupted. */
-    while (MC_SLOT_WITHIN_BOUNDS(slot, initgr_mc_ctx.dt_size)) {
+    while (MC_SLOT_WITHIN_BOUNDS(slot, data_size)) {
         /* free record from previous iteration */
         free(rec);
         rec = NULL;
@@ -132,14 +132,14 @@ errno_t sss_nss_mc_initgroups_dyn(const char *name, size_t name_len,
         rec_name = (char *)data + data->name;
         /* Integrity check
          * - name_len cannot be longer than all strings or data
-         * - data->name cannot point outside strings
-         * - all data must be within data_table
-         * - name must be within data_table */
-        if (name_len > data->data_len
-            || name_len > data->strs_len
-            || (data->strs + name_len) > (data_offset + data->data_len)
-            || (uint8_t *)data->gids + data->data_len > max_addr
-            || (uint8_t *)rec_name + name_len > max_addr) {
+         * - all data must be within copy of record
+         * - size of record must be lower that data table size
+         * - data->strs cannot point outside strings */
+        if (name_len > data->strs_len
+            || data->strs_len > data->data_len
+            || data->data_len > rec->len
+            || rec->len > data_size
+            || (data->strs + name_len) > (data_offset + data->data_len)) {
             ret = ENOENT;
             goto done;
         }
@@ -151,7 +151,7 @@ errno_t sss_nss_mc_initgroups_dyn(const char *name, size_t name_len,
         slot = sss_nss_mc_next_slot_with_hash(rec, hash);
     }
 
-    if (!MC_SLOT_WITHIN_BOUNDS(slot, initgr_mc_ctx.dt_size)) {
+    if (!MC_SLOT_WITHIN_BOUNDS(slot, data_size)) {
         ret = ENOENT;
         goto done;
     }
diff --git a/src/sss_client/nss_mc_passwd.c b/src/sss_client/nss_mc_passwd.c
index 10e43e2af43c5e7f1738e281b3ed260d89f3a004..0da7ad0aeece7d38ca34bb3fde64adc898eaf0ae 100644
--- a/src/sss_client/nss_mc_passwd.c
+++ b/src/sss_client/nss_mc_passwd.c
@@ -105,16 +105,16 @@ errno_t sss_nss_mc_getpwnam(const char *name, size_t name_len,
     uint32_t hash;
     uint32_t slot;
     int ret;
-    size_t strs_offset;
-    uint8_t *max_addr;
+    const size_t strs_offset = offsetof(struct sss_mc_pwd_data, strs);
+    size_t data_size;
 
     ret = sss_nss_mc_get_ctx("passwd", &pw_mc_ctx);
     if (ret) {
         return ret;
     }
 
-    /* Get max address of data table. */
-    max_addr = pw_mc_ctx.data_table + pw_mc_ctx.dt_size;
+    /* Get max size of data table. */
+    data_size = pw_mc_ctx.dt_size;
 
     /* hashes are calculated including the NULL terminator */
     hash = sss_nss_mc_hash(&pw_mc_ctx, name, name_len + 1);
@@ -123,7 +123,7 @@ errno_t sss_nss_mc_getpwnam(const char *name, size_t name_len,
     /* If slot is not within the bounds of mmaped region and
      * it's value is not MC_INVALID_VAL, then the cache is
      * probbably corrupted. */
-    while (MC_SLOT_WITHIN_BOUNDS(slot, pw_mc_ctx.dt_size)) {
+    while (MC_SLOT_WITHIN_BOUNDS(slot, data_size)) {
         /* free record from previous iteration */
         free(rec);
         rec = NULL;
@@ -140,16 +140,16 @@ errno_t sss_nss_mc_getpwnam(const char *name, size_t name_len,
             continue;
         }
 
-        strs_offset = offsetof(struct sss_mc_pwd_data, strs);
-
         data = (struct sss_mc_pwd_data *)rec->data;
         /* Integrity check
          * - name_len cannot be longer than all strings
          * - data->name cannot point outside strings
-         * - all strings must be within data_table */
+         * - all strings must be within copy of record
+         * - size of record must be lower that data table size */
         if (name_len > data->strs_len
             || (data->name + name_len) > (strs_offset + data->strs_len)
-            || (uint8_t *)data->strs + data->strs_len > max_addr) {
+            || data->strs_len > rec->len
+            || rec->len > data_size) {
             ret = ENOENT;
             goto done;
         }
@@ -162,7 +162,7 @@ errno_t sss_nss_mc_getpwnam(const char *name, size_t name_len,
         slot = sss_nss_mc_next_slot_with_hash(rec, hash);
     }
 
-    if (!MC_SLOT_WITHIN_BOUNDS(slot, pw_mc_ctx.dt_size)) {
+    if (!MC_SLOT_WITHIN_BOUNDS(slot, data_size)) {
         ret = ENOENT;
         goto done;
     }
-- 
2.4.6

-------------- next part --------------
>From 015a07af3fd706bfccaf6be32ad7ccda727ce3cb Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik <lslebodn at redhat.com>
Date: Wed, 29 Jul 2015 14:35:52 +0200
Subject: [PATCH 5/8] intg_test: Add module for simulation of utility id

---
 src/tests/intg/Makefile.am |   1 +
 src/tests/intg/sssd_id.py  | 119 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 120 insertions(+)
 create mode 100644 src/tests/intg/sssd_id.py

diff --git a/src/tests/intg/Makefile.am b/src/tests/intg/Makefile.am
index 9383e112002baa7b0d200a3dc205aa2856089c97..7488022ed59dd894ba7b6808c5573706412098c7 100644
--- a/src/tests/intg/Makefile.am
+++ b/src/tests/intg/Makefile.am
@@ -1,5 +1,6 @@
 dist_noinst_DATA = \
     config.py.m4 \
+    sssd_id.py \
     ds.py \
     ds_openldap.py \
     ent.py \
diff --git a/src/tests/intg/sssd_id.py b/src/tests/intg/sssd_id.py
new file mode 100644
index 0000000000000000000000000000000000000000..45f2822b5b33d99b6a7cbbbba0450c774e05ff11
--- /dev/null
+++ b/src/tests/intg/sssd_id.py
@@ -0,0 +1,119 @@
+#
+# Module for simulation of utility "id" from coreutils
+#
+# Copyright (c) 2015 Red Hat, Inc.
+# Author: Lukas Slebodnik <lslebodn at redhat.com>
+#
+# This is free software; you can redistribute it and/or modify it
+# under the terms of the GNU General Public License as published by
+# the Free Software Foundation; version 2 only
+#
+# This program is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+import config
+import pwd
+import grp
+from ctypes import (cdll, c_int, c_char, c_uint32, c_long, c_char_p,
+                    POINTER, pointer)
+
+
+class NssReturnCode(object):
+    """ 'enum' class for name service switch retrn code """
+    TRYAGAIN = -2,
+    UNAVAIL = -1
+    NOTFOUND = 0
+    SUCCESS = 1
+    RETURN = 2
+
+
+def call_sssd_initgroups(user, gid):
+    """
+    Function will initialize the supplementary group access list
+    for given user. It will gather groups only provided by sssd.
+
+    Arguments are the same as for C function initgroups
+    @param string user name of user
+    @param int gid the additional gid will be also added to the list.
+
+    @return (int, int, List[int]) (err, errno, gids)
+        gids shoudl contain user group IDs if err is NssReturnCode.SUCCESS
+        otherwise errno will contain non-zero vlaue.
+    """
+    libnss_sss_path = config.PREFIX + "/lib/libnss_sss.so.2"
+    libnss_sss = cdll.LoadLibrary(libnss_sss_path)
+
+    func = libnss_sss._nss_sss_initgroups_dyn
+    func.restype = c_int
+    func.argtypes = [POINTER(c_char), c_uint32, POINTER(c_long),
+                     POINTER(c_long), POINTER(POINTER(c_uint32)), c_long,
+                     POINTER(c_int)]
+
+    start = POINTER(c_long)(c_long(0))
+    size = POINTER(c_long)(c_long(0))
+    groups = POINTER(c_uint32)()
+    p_groups = pointer(groups)
+    limit = c_long(-1)
+    errno = POINTER(c_int)(c_int(0))
+
+    res = func(c_char_p(user), c_uint32(gid), start, size, p_groups, limit,
+               errno)
+
+    gids = []
+    if res == NssReturnCode.SUCCESS:
+        gids_count = size[0]
+        assert gids_count > 0, "_nss_sss_initgroups_dyn shoulld return " \
+                               "one gid"
+
+        for i in range(0, gids_count):
+            gids.append(int(p_groups.contents[i]))
+
+    return (int(res), errno[0], gids)
+
+
+def get_user_gids(user):
+    """
+    Function will initialize the supplementary group access list
+    for given user. It will gather groups only provided by sssd.
+
+    Arguments are the same as for C function initgroups
+    @param string user name of user
+
+    @return (int, int, List[int]) (err, errno, gids)
+        gids shoudl contain user group IDs if err is NssReturnCode.SUCCESS
+        otherwise errno will contain non-zero vlaue.
+    """
+    pwd_user = pwd.getpwnam(user)
+    uid = pwd_user.pw_uid
+    gid = pwd_user.pw_gid
+
+    user = pwd.getpwuid(uid).pw_name
+
+    return call_sssd_initgroups(user, gid)
+
+
+def get_user_groups(user):
+    """
+    Function will initialize the supplementary group access list
+    for given user. It will gather groups only provided by sssd.
+
+    Arguments are the same as for C function initgroups
+    @param string user name of user
+
+    @return (int, int, List[string]) (err, errno, groups)
+        roups shoudl contain names of user groups
+        if err is NssReturnCode.SUCCESS
+        otherwise errno will contain non-zero vlaue.
+    """
+    (res, errno, gids) = get_user_gids(user)
+    groups = []
+
+    if res == NssReturnCode.SUCCESS:
+        groups = [grp.getgrgid(gid).gr_name for gid in gids]
+
+    return (res, errno, groups)
-- 
2.4.6

-------------- next part --------------
>From a1d964a9dd67b230f2a9cac2d2abc63fe929f378 Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik <lslebodn at redhat.com>
Date: Tue, 21 Jul 2015 08:55:14 +0200
Subject: [PATCH 6/8] intg_test: Add integration test for memory cache

---
 src/tests/intg/Makefile.am          |   1 +
 src/tests/intg/test_memory_cache.py | 347 ++++++++++++++++++++++++++++++++++++
 2 files changed, 348 insertions(+)
 create mode 100644 src/tests/intg/test_memory_cache.py

diff --git a/src/tests/intg/Makefile.am b/src/tests/intg/Makefile.am
index 7488022ed59dd894ba7b6808c5573706412098c7..6819c2f2cea137c7544e94b0b29391611533b20c 100644
--- a/src/tests/intg/Makefile.am
+++ b/src/tests/intg/Makefile.am
@@ -8,6 +8,7 @@ dist_noinst_DATA = \
     ldap_ent.py \
     ldap_test.py \
     util.py \
+    test_memory_cache.py \
     $(NULL)
 
 config.py: config.py.m4
diff --git a/src/tests/intg/test_memory_cache.py b/src/tests/intg/test_memory_cache.py
new file mode 100644
index 0000000000000000000000000000000000000000..5a1f07651b70a5bf1fbacceeae6825ea4341e3b5
--- /dev/null
+++ b/src/tests/intg/test_memory_cache.py
@@ -0,0 +1,347 @@
+#
+# LDAP integration test
+#
+# Copyright (c) 2015 Red Hat, Inc.
+# Author: Lukas Slebodnik <lslebodn at redhat.com>
+#
+# This is free software; you can redistribute it and/or modify it
+# under the terms of the GNU General Public License as published by
+# the Free Software Foundation; version 2 only
+#
+# This program is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+import os
+import stat
+import ent
+import config
+import signal
+import subprocess
+import time
+import pytest
+import ds_openldap
+import ldap_ent
+import sssd_id
+from util import unindent
+
+LDAP_BASE_DN = "dc=example,dc=com"
+
+
+ at pytest.fixture(scope="module")
+def ds_inst(request):
+    """LDAP server instance fixture"""
+    ds_inst = ds_openldap.DSOpenLDAP(
+        config.PREFIX, 10389, LDAP_BASE_DN,
+        "cn=admin", "Secret123")
+    try:
+        ds_inst.setup()
+    except:
+        ds_inst.teardown()
+        raise
+    request.addfinalizer(lambda: ds_inst.teardown())
+    return ds_inst
+
+
+ at pytest.fixture(scope="module")
+def ldap_conn(request, ds_inst):
+    """LDAP server connection fixture"""
+    ldap_conn = ds_inst.bind()
+    ldap_conn.ds_inst = ds_inst
+    request.addfinalizer(lambda: ldap_conn.unbind_s())
+    return ldap_conn
+
+
+def create_ldap_fixture(request, ldap_conn, ent_list):
+    """Add LDAP entries and add teardown for removing them"""
+    for entry in ent_list:
+        ldap_conn.add_s(entry[0], entry[1])
+
+    def teardown():
+        for entry in ent_list:
+            ldap_conn.delete_s(entry[0])
+    request.addfinalizer(teardown)
+
+
+def create_conf_fixture(request, contents):
+    """Generate sssd.conf and add teardown for removing it"""
+    conf = open(config.CONF_PATH, "w")
+    conf.write(contents)
+    conf.close()
+    os.chmod(config.CONF_PATH, stat.S_IRUSR | stat.S_IWUSR)
+    request.addfinalizer(lambda: os.unlink(config.CONF_PATH))
+
+
+def stop_sssd():
+    pid_file = open(config.PIDFILE_PATH, "r")
+    pid = int(pid_file.read())
+    os.kill(pid, signal.SIGTERM)
+    while True:
+        try:
+            os.kill(pid, signal.SIGCONT)
+        except:
+            break
+        time.sleep(1)
+
+
+def create_sssd_fixture(request):
+    """Start sssd and add teardown for stopping it and removing state"""
+    if subprocess.call(["sssd", "-D", "-f"]) != 0:
+        raise Exception("sssd start failed")
+
+    def teardown():
+        try:
+            stop_sssd()
+        except:
+            pass
+        subprocess.call(["sss_cache", "-E"])
+        for path in os.listdir(config.DB_PATH):
+            os.unlink(config.DB_PATH + "/" + path)
+        for path in os.listdir(config.MCACHE_PATH):
+            os.unlink(config.MCACHE_PATH + "/" + path)
+    request.addfinalizer(teardown)
+
+
+ at pytest.fixture
+def sanity_rfc2307(request, ldap_conn):
+    ent_list = ldap_ent.List(LDAP_BASE_DN)
+    ent_list.add_user("user1", 1001, 2001)
+    ent_list.add_user("user2", 1002, 2002)
+    ent_list.add_user("user3", 1003, 2003)
+    ent_list.add_user("user11", 1011, 2001)
+    ent_list.add_user("user12", 1012, 2002)
+    ent_list.add_user("user13", 1013, 2003)
+    ent_list.add_user("user21", 1021, 2001)
+    ent_list.add_user("user22", 1022, 2002)
+    ent_list.add_user("user23", 1023, 2003)
+
+    ent_list.add_group("group1", 2001, ["user1", "user11", "user21"])
+    ent_list.add_group("group2", 2002, ["user2", "user12", "user22"])
+    ent_list.add_group("group3", 2003, ["user3", "user13", "user23"])
+
+    ent_list.add_group("group0x", 2000, ["user1", "user2", "user3"])
+    ent_list.add_group("group1x", 2010, ["user11", "user12", "user13"])
+    ent_list.add_group("group2x", 2020, ["user21", "user22", "user23"])
+    create_ldap_fixture(request, ldap_conn, ent_list)
+
+    conf = unindent("""\
+        [sssd]
+        config_file_version = 2
+        domains             = LDAP
+        services            = nss
+
+        [nss]
+
+        [domain/LDAP]
+        ldap_auth_disable_tls_never_use_in_production = true
+        ldap_schema         = rfc2307
+        id_provider         = ldap
+        auth_provider       = ldap
+        sudo_provider       = ldap
+        ldap_uri            = {ldap_conn.ds_inst.ldap_url}
+        ldap_search_base    = {ldap_conn.ds_inst.base_dn}
+    """).format(**locals())
+    create_conf_fixture(request, conf)
+    create_sssd_fixture(request)
+    return None
+
+
+def test_getpwnam(ldap_conn, sanity_rfc2307):
+    ent.assert_passwd_by_name(
+        'user1',
+        dict(name='user1', passwd='*', uid=1001, gid=2001,
+             gecos='1001', shell='/bin/bash'))
+    ent.assert_passwd_by_uid(
+        1001,
+        dict(name='user1', passwd='*', uid=1001, gid=2001,
+             gecos='1001', shell='/bin/bash'))
+
+    ent.assert_passwd_by_name(
+        'user2',
+        dict(name='user2', passwd='*', uid=1002, gid=2002,
+             gecos='1002', shell='/bin/bash'))
+    ent.assert_passwd_by_uid(
+        1002,
+        dict(name='user2', passwd='*', uid=1002, gid=2002,
+             gecos='1002', shell='/bin/bash'))
+
+    ent.assert_passwd_by_name(
+        'user3',
+        dict(name='user3', passwd='*', uid=1003, gid=2003,
+             gecos='1003', shell='/bin/bash'))
+    ent.assert_passwd_by_uid(
+        1003,
+        dict(name='user3', passwd='*', uid=1003, gid=2003,
+             gecos='1003', shell='/bin/bash'))
+
+    ent.assert_passwd_by_name(
+        'user11',
+        dict(name='user11', passwd='*', uid=1011, gid=2001,
+             gecos='1011', shell='/bin/bash'))
+    ent.assert_passwd_by_uid(
+        1011,
+        dict(name='user11', passwd='*', uid=1011, gid=2001,
+             gecos='1011', shell='/bin/bash'))
+
+    ent.assert_passwd_by_name(
+        'user12',
+        dict(name='user12', passwd='*', uid=1012, gid=2002,
+             gecos='1012', shell='/bin/bash'))
+    ent.assert_passwd_by_uid(
+        1012,
+        dict(name='user12', passwd='*', uid=1012, gid=2002,
+             gecos='1012', shell='/bin/bash'))
+
+    ent.assert_passwd_by_name(
+        'user13',
+        dict(name='user13', passwd='*', uid=1013, gid=2003,
+             gecos='1013', shell='/bin/bash'))
+    ent.assert_passwd_by_uid(
+        1013,
+        dict(name='user13', passwd='*', uid=1013, gid=2003,
+             gecos='1013', shell='/bin/bash'))
+
+    ent.assert_passwd_by_name(
+        'user21',
+        dict(name='user21', passwd='*', uid=1021, gid=2001,
+             gecos='1021', shell='/bin/bash'))
+    ent.assert_passwd_by_uid(
+        1021,
+        dict(name='user21', passwd='*', uid=1021, gid=2001,
+             gecos='1021', shell='/bin/bash'))
+
+    ent.assert_passwd_by_name(
+        'user22',
+        dict(name='user22', passwd='*', uid=1022, gid=2002,
+             gecos='1022', shell='/bin/bash'))
+    ent.assert_passwd_by_uid(
+        1022,
+        dict(name='user22', passwd='*', uid=1022, gid=2002,
+             gecos='1022', shell='/bin/bash'))
+
+    ent.assert_passwd_by_name(
+        'user23',
+        dict(name='user23', passwd='*', uid=1023, gid=2003,
+             gecos='1023', shell='/bin/bash'))
+    ent.assert_passwd_by_uid(
+        1023,
+        dict(name='user23', passwd='*', uid=1023, gid=2003,
+             gecos='1023', shell='/bin/bash'))
+
+
+def test_getpwnam_with_mc(ldap_conn, sanity_rfc2307):
+    test_getpwnam(ldap_conn, sanity_rfc2307)
+    stop_sssd()
+    test_getpwnam(ldap_conn, sanity_rfc2307)
+
+
+def test_getgrnam_simple(ldap_conn, sanity_rfc2307):
+    ent.assert_group_by_name("group1", dict(name="group1", gid=2001))
+    ent.assert_group_by_gid(2001, dict(name="group1", gid=2001))
+
+    ent.assert_group_by_name("group2", dict(name="group2", gid=2002))
+    ent.assert_group_by_gid(2002, dict(name="group2", gid=2002))
+
+    ent.assert_group_by_name("group3", dict(name="group3", gid=2003))
+    ent.assert_group_by_gid(2003, dict(name="group3", gid=2003))
+
+    ent.assert_group_by_name("group0x", dict(name="group0x", gid=2000))
+    ent.assert_group_by_gid(2000, dict(name="group0x", gid=2000))
+
+    ent.assert_group_by_name("group1x", dict(name="group1x", gid=2010))
+    ent.assert_group_by_gid(2010, dict(name="group1x", gid=2010))
+
+    ent.assert_group_by_name("group2x", dict(name="group2x", gid=2020))
+    ent.assert_group_by_gid(2020, dict(name="group2x", gid=2020))
+
+
+def test_getgrnam_simple_with_mc(ldap_conn, sanity_rfc2307):
+    test_getgrnam_simple(ldap_conn, sanity_rfc2307)
+    stop_sssd()
+    test_getgrnam_simple(ldap_conn, sanity_rfc2307)
+
+
+def test_getgrnam_membership(ldap_conn, sanity_rfc2307):
+    ent.assert_group_by_name(
+        "group1",
+        dict(mem=ent.contains_only("user1", "user11", "user21")))
+    ent.assert_group_by_gid(
+        2001,
+        dict(mem=ent.contains_only("user1", "user11", "user21")))
+
+    ent.assert_group_by_name(
+        "group2",
+        dict(mem=ent.contains_only("user2", "user12", "user22")))
+    ent.assert_group_by_gid(
+        2002,
+        dict(mem=ent.contains_only("user2", "user12", "user22")))
+
+    ent.assert_group_by_name(
+        "group3",
+        dict(mem=ent.contains_only("user3", "user13", "user23")))
+    ent.assert_group_by_gid(
+        2003,
+        dict(mem=ent.contains_only("user3", "user13", "user23")))
+
+    ent.assert_group_by_name(
+        "group0x",
+        dict(mem=ent.contains_only("user1", "user2", "user3")))
+    ent.assert_group_by_gid(
+        2000,
+        dict(mem=ent.contains_only("user1", "user2", "user3")))
+
+    ent.assert_group_by_name(
+        "group1x",
+        dict(mem=ent.contains_only("user11", "user12", "user13")))
+    ent.assert_group_by_gid(
+        2010,
+        dict(mem=ent.contains_only("user11", "user12", "user13")))
+
+    ent.assert_group_by_name(
+        "group2x",
+        dict(mem=ent.contains_only("user21", "user22", "user23")))
+    ent.assert_group_by_gid(
+        2020,
+        dict(mem=ent.contains_only("user21", "user22", "user23")))
+
+
+def test_getgrnam_membership_with_mc(ldap_conn, sanity_rfc2307):
+    test_getgrnam_membership(ldap_conn, sanity_rfc2307)
+    stop_sssd()
+    test_getgrnam_membership(ldap_conn, sanity_rfc2307)
+
+
+def assert_user_gids_equal(user, expected_gids):
+    (res, errno, gids) = sssd_id.get_user_gids(user)
+    assert res == sssd_id.NssReturnCode.SUCCESS, \
+        "Could not find groups for user %s, %d" % (user, errno)
+
+    assert sorted(gids) == sorted(expected_gids), \
+        "result: %s\n expected %s" % (
+            ", ".join(["%s" % s for s in sorted(gids)]),
+            ", ".join(["%s" % s for s in sorted(expected_gids)])
+        )
+
+
+def test_initgroups(ldap_conn, sanity_rfc2307):
+    assert_user_gids_equal('user1', [2000, 2001])
+    assert_user_gids_equal('user2', [2000, 2002])
+    assert_user_gids_equal('user3', [2000, 2003])
+
+    assert_user_gids_equal('user11', [2010, 2001])
+    assert_user_gids_equal('user12', [2010, 2002])
+    assert_user_gids_equal('user13', [2010, 2003])
+
+    assert_user_gids_equal('user21', [2020, 2001])
+    assert_user_gids_equal('user22', [2020, 2002])
+    assert_user_gids_equal('user23', [2020, 2003])
+
+
+def test_initgroups_with_mc(ldap_conn, sanity_rfc2307):
+    test_initgroups(ldap_conn, sanity_rfc2307)
+    stop_sssd()
+    test_initgroups(ldap_conn, sanity_rfc2307)
-- 
2.4.6

-------------- next part --------------
>From 42ffe0a7aee3be0049b2cb424b5c25ae140c0633 Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik <lslebodn at redhat.com>
Date: Mon, 13 Jul 2015 10:40:06 +0200
Subject: [PATCH 7/8] NSS: Initgr memory cache should work with fq names

We need to stored two versions of name to the initgroups memory cache.
Otherwise it could be stored many times if sssd is configured with
case_sensitive = false. It would be impossible to invalidate all
version of names after user login. As a result of this wrong user
groups could be returned from initgroups memory cache.

Therefore we store raw name provided by glibc function
and internal sanitized fully qualified name,
which is unique for particular user.

This patch also increase average space for initgroups
because there are also stored two quite long names in case of
fq names.

Resolves:
https://fedorahosted.org/sssd/ticket/2712
---
 src/responder/nss/nsssrv_cmd.c        | 30 +++++++++++++++++++++++++-----
 src/responder/nss/nsssrv_mmap_cache.c | 32 +++++++++++++++++++++-----------
 src/responder/nss/nsssrv_mmap_cache.h |  1 +
 src/responder/nss/nsssrv_private.h    |  2 ++
 src/util/mmap_cache.h                 |  7 ++++---
 5 files changed, 53 insertions(+), 19 deletions(-)

diff --git a/src/responder/nss/nsssrv_cmd.c b/src/responder/nss/nsssrv_cmd.c
index 778618e4080b1342535408e1720349f5226b3252..e754245eac7200c2f667760540a1d04c9203843d 100644
--- a/src/responder/nss/nsssrv_cmd.c
+++ b/src/responder/nss/nsssrv_cmd.c
@@ -1387,6 +1387,7 @@ static int nss_cmd_getbynam(enum sss_cli_command cmd, struct cli_ctx *cctx)
     }
 
     rawname = (const char *)body;
+    dctx->mc_name = rawname;
 
     DEBUG(SSSDBG_TRACE_FUNC, "Running command [%d] with input [%s].\n",
                                dctx->cmdctx->cmd, rawname);
@@ -3976,6 +3977,13 @@ void nss_update_initgr_memcache(struct nss_ctx *nctx,
     }
 
     if (changed) {
+        char *fq_name = sss_tc_fqname(tmp_ctx, dom->names, dom, name);
+        if (!fq_name) {
+            DEBUG(SSSDBG_CRIT_FAILURE,
+                  "Could not create fq name\n");
+            goto done;
+        }
+
         for (i = 0; i < gnum; i++) {
             id = groups[i];
 
@@ -3987,7 +3995,7 @@ void nss_update_initgr_memcache(struct nss_ctx *nctx,
             }
         }
 
-        to_sized_string(&delete_name, name);
+        to_sized_string(&delete_name, fq_name);
         ret = sss_mmap_cache_initgr_invalidate(nctx->initgr_mc_ctx,
                                                &delete_name);
         if (ret != EOK && ret != ENOENT) {
@@ -4007,6 +4015,7 @@ static int fill_initgr(struct sss_packet *packet,
                        struct sss_domain_info *dom,
                        struct ldb_result *res,
                        struct nss_ctx *nctx,
+                       const char *mc_name,
                        const char *name)
 {
     uint8_t *body;
@@ -4095,9 +4104,18 @@ static int fill_initgr(struct sss_packet *packet,
     }
 
     if (nctx->initgr_mc_ctx) {
-        to_sized_string(&rawname, name);
+        struct sized_string unique_name;
+        char *fq_name = sss_tc_fqname(packet, dom->names, dom, name);
+        if (!fq_name) {
+            DEBUG(SSSDBG_CRIT_FAILURE,
+                  "Could not create fq name\n");
+            return ENOMEM;
+        }
+
+        to_sized_string(&rawname, mc_name);
+        to_sized_string(&unique_name, fq_name);
         ret = sss_mmap_cache_initgr_store(&nctx->initgr_mc_ctx, &rawname,
-                                          num - skipped, gids);
+                                          &unique_name, num - skipped, gids);
         if (ret != EOK && ret != ENOMEM) {
             DEBUG(SSSDBG_CRIT_FAILURE,
                   "Failed to store user %s(%s) in mmap cache!\n",
@@ -4125,7 +4143,7 @@ static int nss_cmd_initgr_send_reply(struct nss_dom_ctx *dctx)
     }
 
     ret = fill_initgr(cctx->creq->out, dctx->domain, dctx->res, nctx,
-                      dctx->rawname);
+                      dctx->mc_name, cmdctx->name);
     if (ret) {
         return ret;
     }
@@ -4173,8 +4191,10 @@ static int nss_cmd_initgroups_search(struct nss_dom_ctx *dctx)
         name = sss_get_cased_name(dctx, cmdctx->name, dom->case_sensitive);
         if (!name) return ENOMEM;
 
-        name = sss_reverse_replace_space(dctx, name,
+        name = sss_reverse_replace_space(cmdctx, name,
                                          nctx->rctx->override_space);
+        /* save name so it can be used in initgr reply */
+        cmdctx->name = name;
         if (name == NULL) {
             DEBUG(SSSDBG_CRIT_FAILURE,
                   "sss_reverse_replace_space failed\n");
diff --git a/src/responder/nss/nsssrv_mmap_cache.c b/src/responder/nss/nsssrv_mmap_cache.c
index ab9e769b1f4d5d17a8c41429afce292298239bc5..62f4c543c628712810b6dfbc669c586c39ca609d 100644
--- a/src/responder/nss/nsssrv_mmap_cache.c
+++ b/src/responder/nss/nsssrv_mmap_cache.c
@@ -31,8 +31,8 @@
 #define SSS_AVG_PASSWD_PAYLOAD (MC_SLOT_SIZE * 4)
 /* short group name and no gids (private user group */
 #define SSS_AVG_GROUP_PAYLOAD (MC_SLOT_SIZE * 3)
-/* average place for 40 supplementary groups */
-#define SSS_AVG_INITGROUP_PAYLOAD (MC_SLOT_SIZE * 4)
+/* average place for 40 supplementary groups + 2 names */
+#define SSS_AVG_INITGROUP_PAYLOAD (MC_SLOT_SIZE * 5)
 
 #define MC_NEXT_BARRIER(val) ((((val) + 1) & 0x00ffffff) | 0xf0000000)
 
@@ -965,6 +965,7 @@ done:
 
 errno_t sss_mmap_cache_initgr_store(struct sss_mc_ctx **_mcc,
                                     struct sized_string *name,
+                                    struct sized_string *unique_name,
                                     uint32_t num_groups,
                                     uint8_t *gids_buf)
 {
@@ -973,6 +974,7 @@ errno_t sss_mmap_cache_initgr_store(struct sss_mc_ctx **_mcc,
     struct sss_mc_initgr_data *data;
     size_t data_len;
     size_t rec_len;
+    size_t pos;
     int ret;
 
     if (mcc == NULL) {
@@ -980,20 +982,22 @@ errno_t sss_mmap_cache_initgr_store(struct sss_mc_ctx **_mcc,
         return EINVAL;
     }
 
-    /* array of gids + name */
-    data_len = num_groups * sizeof(uint32_t) + name->len;
+    /* array of gids + name + unique_name */
+    data_len = num_groups * sizeof(uint32_t) + name->len + unique_name->len;
     rec_len = sizeof(struct sss_mc_rec) + sizeof(struct sss_mc_initgr_data)
               + data_len;
     if (rec_len > mcc->dt_size) {
         return ENOMEM;
     }
 
-    ret = sss_mc_get_record(_mcc, rec_len, name, &rec);
+    /* use unique name for searching potential old records */
+    ret = sss_mc_get_record(_mcc, rec_len, unique_name, &rec);
     if (ret != EOK) {
         return ret;
     }
 
     data = (struct sss_mc_initgr_data *)rec->data;
+    pos = 0;
 
     MC_RAISE_BARRIER(rec);
 
@@ -1001,16 +1005,22 @@ errno_t sss_mmap_cache_initgr_store(struct sss_mc_ctx **_mcc,
      * Use the first key twice.
      */
     sss_mmap_set_rec_header(mcc, rec, rec_len, mcc->valid_time_slot,
-                            name->str, name->len, name->str, name->len);
+                            name->str, name->len,
+                            unique_name->str, unique_name->len);
 
     /* initgroups struct */
-    data->strs_len = name->len;
+    data->strs_len = name->len + unique_name->len;
     data->data_len = data_len;
-    data->reserved = MC_INVALID_VAL32;
     data->num_groups = num_groups;
-    memcpy(data->gids, gids_buf, num_groups * sizeof(uint32_t));
-    memcpy(&data->gids[num_groups], name->str, name->len);
-    data->strs = data->name = MC_PTR_DIFF(&data->gids[num_groups], data);
+    memcpy((char *)data->gids + pos, gids_buf, num_groups * sizeof(uint32_t));
+    pos += num_groups * sizeof(uint32_t);
+
+    memcpy((char *)data->gids + pos, unique_name->str, unique_name->len);
+    data->strs = data->unique_name = MC_PTR_DIFF((char *)data->gids + pos, data);
+    pos += unique_name->len;
+
+    memcpy((char *)data->gids + pos, name->str, name->len);
+    data->name = MC_PTR_DIFF((char *)data->gids + pos, data);
 
     MC_LOWER_BARRIER(rec);
 
diff --git a/src/responder/nss/nsssrv_mmap_cache.h b/src/responder/nss/nsssrv_mmap_cache.h
index b09e4a6f8efa364ae7a6407bab9d8a2a2143c812..b84fbc8edef69db96c17a48a4862d63942297c66 100644
--- a/src/responder/nss/nsssrv_mmap_cache.h
+++ b/src/responder/nss/nsssrv_mmap_cache.h
@@ -53,6 +53,7 @@ errno_t sss_mmap_cache_gr_store(struct sss_mc_ctx **_mcc,
 
 errno_t sss_mmap_cache_initgr_store(struct sss_mc_ctx **_mcc,
                                     struct sized_string *name,
+                                    struct sized_string *unique_name,
                                     uint32_t num_groups,
                                     uint8_t *gids_buf);
 
diff --git a/src/responder/nss/nsssrv_private.h b/src/responder/nss/nsssrv_private.h
index e9f00b114975ef760aaaac45e6e91f6e40c52b9a..e5a2486f1fb9a8de39ec90f802f596b2c2f6af7f 100644
--- a/src/responder/nss/nsssrv_private.h
+++ b/src/responder/nss/nsssrv_private.h
@@ -79,6 +79,8 @@ struct nss_dom_ctx {
 
     /* Service-specific */
     const char *protocol;
+
+    const char *mc_name;
 };
 
 struct setent_step_ctx {
diff --git a/src/util/mmap_cache.h b/src/util/mmap_cache.h
index b5917b3c0973276e43e9fe160cec7528b1224f8f..22c1ae62d1ff0c816c23bd8b26140990d692134c 100644
--- a/src/util/mmap_cache.h
+++ b/src/util/mmap_cache.h
@@ -138,14 +138,15 @@ struct sss_mc_grp_data {
 };
 
 struct sss_mc_initgr_data {
-    rel_ptr_t name;         /* ptr to name string, rel. to struct base addr */
+    rel_ptr_t unique_name;  /* ptr to unique name string, rel. to struct base addr */
+    rel_ptr_t name;         /* ptr to raw name string, rel. to struct base addr */
     rel_ptr_t strs;         /* ptr to concatenation of all strings */
-    uint32_t reserved;
     uint32_t strs_len;      /* length of strs */
     uint32_t data_len;      /* all initgroups data len */
     uint32_t num_groups;    /* number of groups */
     uint32_t gids[0];       /* array of all groups
-                             * string with name is stored after gids */
+                             * string with name and unique_name is stored
+                             * after gids */
 };
 
 #pragma pack()
-- 
2.4.6

-------------- next part --------------
>From 0602835da62471ecef2f14b0a49c086708254ee3 Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik <lslebodn at redhat.com>
Date: Fri, 24 Jul 2015 15:13:01 +0200
Subject: [PATCH 8/8] test_memory_cache: Add test for initgroups mc with fq
 names

---
 src/tests/intg/test_memory_cache.py | 140 +++++++++++++++++++++++++++++++++++-
 1 file changed, 138 insertions(+), 2 deletions(-)

diff --git a/src/tests/intg/test_memory_cache.py b/src/tests/intg/test_memory_cache.py
index 5a1f07651b70a5bf1fbacceeae6825ea4341e3b5..12ce3c5054fe76560e31137d88043baf20641d3a 100644
--- a/src/tests/intg/test_memory_cache.py
+++ b/src/tests/intg/test_memory_cache.py
@@ -106,8 +106,7 @@ def create_sssd_fixture(request):
     request.addfinalizer(teardown)
 
 
- at pytest.fixture
-def sanity_rfc2307(request, ldap_conn):
+def load_data_to_ldap(request, ldap_conn):
     ent_list = ldap_ent.List(LDAP_BASE_DN)
     ent_list.add_user("user1", 1001, 2001)
     ent_list.add_user("user2", 1002, 2002)
@@ -128,6 +127,64 @@ def sanity_rfc2307(request, ldap_conn):
     ent_list.add_group("group2x", 2020, ["user21", "user22", "user23"])
     create_ldap_fixture(request, ldap_conn, ent_list)
 
+
+ at pytest.fixture
+def sanity_rfc2307(request, ldap_conn):
+    load_data_to_ldap(request, ldap_conn)
+
+    conf = unindent("""\
+        [sssd]
+        config_file_version = 2
+        domains             = LDAP
+        services            = nss
+
+        [nss]
+
+        [domain/LDAP]
+        ldap_auth_disable_tls_never_use_in_production = true
+        ldap_schema         = rfc2307
+        id_provider         = ldap
+        auth_provider       = ldap
+        sudo_provider       = ldap
+        ldap_uri            = {ldap_conn.ds_inst.ldap_url}
+        ldap_search_base    = {ldap_conn.ds_inst.base_dn}
+    """).format(**locals())
+    create_conf_fixture(request, conf)
+    create_sssd_fixture(request)
+    return None
+
+
+ at pytest.fixture
+def fqname_rfc2307(request, ldap_conn):
+    load_data_to_ldap(request, ldap_conn)
+
+    conf = unindent("""\
+        [sssd]
+        config_file_version = 2
+        domains             = LDAP
+        services            = nss
+
+        [nss]
+
+        [domain/LDAP]
+        ldap_auth_disable_tls_never_use_in_production = true
+        ldap_schema         = rfc2307
+        id_provider         = ldap
+        auth_provider       = ldap
+        sudo_provider       = ldap
+        ldap_uri            = {ldap_conn.ds_inst.ldap_url}
+        ldap_search_base    = {ldap_conn.ds_inst.base_dn}
+        use_fully_qualified_names = true
+    """).format(**locals())
+    create_conf_fixture(request, conf)
+    create_sssd_fixture(request)
+    return None
+
+
+ at pytest.fixture
+def fqname_case_insensitive_rfc2307(request, ldap_conn):
+    load_data_to_ldap(request, ldap_conn)
+
     conf = unindent("""\
         [sssd]
         config_file_version = 2
@@ -144,6 +201,8 @@ def sanity_rfc2307(request, ldap_conn):
         sudo_provider       = ldap
         ldap_uri            = {ldap_conn.ds_inst.ldap_url}
         ldap_search_base    = {ldap_conn.ds_inst.base_dn}
+        use_fully_qualified_names = true
+        case_sensitive = false
     """).format(**locals())
     create_conf_fixture(request, conf)
     create_sssd_fixture(request)
@@ -345,3 +404,80 @@ def test_initgroups_with_mc(ldap_conn, sanity_rfc2307):
     test_initgroups(ldap_conn, sanity_rfc2307)
     stop_sssd()
     test_initgroups(ldap_conn, sanity_rfc2307)
+
+
+def test_initgroups_fqname_with_mc(ldap_conn, fqname_rfc2307):
+    assert_user_gids_equal('user1 at LDAP', [2000, 2001])
+    stop_sssd()
+    assert_user_gids_equal('user1 at LDAP', [2000, 2001])
+
+
+def assert_initgroups_equal(user, primary_gid, expected_gids):
+    (res, errno, gids) = sssd_id.call_sssd_initgroups(user, primary_gid)
+    assert res == sssd_id.NssReturnCode.SUCCESS, \
+        "Could not find groups for user %s, %d" % (user, errno)
+
+    assert sorted(gids) == sorted(expected_gids), \
+        "result: %s\n expected %s" % (
+            ", ".join(["%s" % s for s in sorted(gids)]),
+            ", ".join(["%s" % s for s in sorted(expected_gids)])
+        )
+
+
+def assert_stored_last_initgroups(user1_case1, user1_case2, user1_case_last,
+                                  primary_gid, expected_gids):
+
+    assert_initgroups_equal(user1_case1, primary_gid, expected_gids)
+    assert_initgroups_equal(user1_case2, primary_gid, expected_gids)
+    assert_initgroups_equal(user1_case_last, primary_gid, expected_gids)
+    stop_sssd()
+
+    user = user1_case1
+    (res, errno, gids) = sssd_id.call_sssd_initgroups(user, primary_gid)
+    assert res == sssd_id.NssReturnCode.UNAVAIL, \
+        "Initgroups for user shoudl fail user %s, %d" % (user, res)
+
+    user = user1_case2
+    (res, errno, gids) = sssd_id.call_sssd_initgroups(user, primary_gid)
+    assert res == sssd_id.NssReturnCode.UNAVAIL, \
+        "Initgroups for user shoudl fail user %s, %d" % (user, res)
+
+    # Just last invocation of initgroups shoudl PASS
+    # Otherwise, we would not be able to invalidate it
+    assert_initgroups_equal(user1_case_last, primary_gid, expected_gids)
+
+
+def test_initgroups_case_insensitive_with_mc1(ldap_conn,
+                                              fqname_case_insensitive_rfc2307):
+    user1_case1 = 'User1 at LDAP'
+    user1_case2 = 'uSer1 at LDAP'
+    user1_case_last = 'usEr1 at LDAP'
+    primary_gid = 2001
+    expected_gids = [2000, 2001]
+
+    assert_stored_last_initgroups(user1_case1, user1_case2, user1_case_last,
+                                  primary_gid, expected_gids)
+
+
+def test_initgroups_case_insensitive_with_mc2(ldap_conn,
+                                              fqname_case_insensitive_rfc2307):
+    user1_case1 = 'usEr1 at LDAP'
+    user1_case2 = 'User1 at LDAP'
+    user1_case_last = 'uSer1 at LDAP'
+    primary_gid = 2001
+    expected_gids = [2000, 2001]
+
+    assert_stored_last_initgroups(user1_case1, user1_case2, user1_case_last,
+                                  primary_gid, expected_gids)
+
+
+def test_initgroups_case_insensitive_with_mc3(ldap_conn,
+                                              fqname_case_insensitive_rfc2307):
+    user1_case1 = 'uSer1 at LDAP'
+    user1_case2 = 'usEr1 at LDAP'
+    user1_case_last = 'User1 at LDAP'
+    primary_gid = 2001
+    expected_gids = [2000, 2001]
+
+    assert_stored_last_initgroups(user1_case1, user1_case2, user1_case_last,
+                                  primary_gid, expected_gids)
-- 
2.4.6



More information about the sssd-devel mailing list