From 574a615e61ca74b08e2bd7e1e820757f88150418 Mon Sep 17 00:00:00 2001

From: Sumit Bose <sbose@redhat.com>

Date: Fri, 14 Jun 2019 11:13:54 +0200

Subject: [PATCH 1/2] extdom: unify error code handling especially

 LDAP_NO_SUCH_OBJECT

A return code LDAP_NO_SUCH_OBJECT will tell SSSD on the IPA client to

remove the searched object from the cache. As a consequence

LDAP_NO_SUCH_OBJECT should only be returned if the object really does

not exists otherwise the data of existing objects might be removed form

the cache of the clients causing unexpected behaviour like

authentication errors.

Currently some code-paths use LDAP_NO_SUCH_OBJECT as default error code.

With this patch LDAP_NO_SUCH_OBJECT is only returned if the related

lookup functions return ENOENT. Timeout related error code will lead to

LDAP_TIMELIMIT_EXCEEDED and LDAP_OPERATIONS_ERROR is used as default

error code.

Fixes: https://pagure.io/freeipa/issue/8044

Reviewed-By: Alexander Bokovoy <abokovoy@redhat.com>

---

 .../ipa-extdom-extop/back_extdom_sss_idmap.c  |  4 +-

 .../ipa-extdom-extop/ipa_extdom_common.c      | 77 ++++++++++++++-----

 .../ipa-extdom-extop/ipa_extdom_extop.c       |  2 +

 3 files changed, 61 insertions(+), 22 deletions(-)

diff --git a/daemons/ipa-slapi-plugins/ipa-extdom-extop/back_extdom_sss_idmap.c b/daemons/ipa-slapi-plugins/ipa-extdom-extop/back_extdom_sss_idmap.c

index 89c58ca2d..64b90e3ae 100644

--- a/daemons/ipa-slapi-plugins/ipa-extdom-extop/back_extdom_sss_idmap.c

+++ b/daemons/ipa-slapi-plugins/ipa-extdom-extop/back_extdom_sss_idmap.c

@@ -47,10 +47,10 @@ static enum nss_status __convert_sss_nss2nss_status(int errcode) {

         return NSS_STATUS_SUCCESS;

     case ENOENT:

         return NSS_STATUS_NOTFOUND;

-    case ETIME:

-        /* fall-through */

     case ERANGE:

         return NSS_STATUS_TRYAGAIN;

+    case ETIME:

+        /* fall-through */

     case ETIMEDOUT:

         /* fall-through */

     default:

diff --git a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_common.c b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_common.c

index 1b93dce18..134b62377 100644

--- a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_common.c

+++ b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_common.c

@@ -523,7 +523,7 @@ int pack_ber_user(struct ipa_extdom_ctx *ctx,

         if (strcasecmp(locat+1, domain_name) == 0  ) {

             locat[0] = '\0';

         } else {

-            ret = LDAP_NO_SUCH_OBJECT;

+            ret = LDAP_INVALID_SYNTAX;

             goto done;

         }

     }

@@ -568,10 +568,12 @@ int pack_ber_user(struct ipa_extdom_ctx *ctx,

             ret = getgrgid_r_wrapper(ctx,

                                      groups[c], &grp, &buf, &buf_len);

             if (ret != 0) {

-                if (ret == ENOMEM || ret == ERANGE) {

-                    ret = LDAP_OPERATIONS_ERROR;

-                } else {

+                if (ret == ENOENT) {

                     ret = LDAP_NO_SUCH_OBJECT;

+                } else if (ret == ETIMEDOUT) {

+                    ret = LDAP_TIMELIMIT_EXCEEDED;

+                } else {

+                    ret = LDAP_OPERATIONS_ERROR;

                 }

                 goto done;

             }

@@ -634,7 +636,7 @@ int pack_ber_group(enum response_types response_type,

         if (strcasecmp(locat+1, domain_name) == 0  ) {

             locat[0] = '\0';

         } else {

-            ret = LDAP_NO_SUCH_OBJECT;

+            ret = LDAP_INVALID_SYNTAX;

             goto done;

         }

     }

@@ -836,6 +838,8 @@ static int handle_uid_request(struct ipa_extdom_ctx *ctx,

                             || id_type == SSS_ID_TYPE_BOTH)) {

             if (ret == ENOENT) {

                 ret = LDAP_NO_SUCH_OBJECT;

+            } else if (ret == ETIMEDOUT || ret == ETIME) {

+                ret = LDAP_TIMELIMIT_EXCEEDED;

             } else {

                 set_err_msg(req, "Failed to lookup SID by UID");

                 ret = LDAP_OPERATIONS_ERROR;

@@ -847,10 +851,12 @@ static int handle_uid_request(struct ipa_extdom_ctx *ctx,

     } else {

         ret = getpwuid_r_wrapper(ctx, uid, &pwd, &buf, &buf_len);

         if (ret != 0) {

-            if (ret == ENOMEM || ret == ERANGE) {

-                ret = LDAP_OPERATIONS_ERROR;

-            } else {

+            if (ret == ENOENT) {

                 ret = LDAP_NO_SUCH_OBJECT;

+            } else if (ret == ETIMEDOUT) {

+                ret = LDAP_TIMELIMIT_EXCEEDED;

+            } else {

+                ret = LDAP_OPERATIONS_ERROR;

             }

             goto done;

         }

@@ -862,6 +868,8 @@ static int handle_uid_request(struct ipa_extdom_ctx *ctx,

                 set_err_msg(req, "Failed to read original data");

                 if (ret == ENOENT) {

                     ret = LDAP_NO_SUCH_OBJECT;

+                } else if (ret == ETIMEDOUT || ret == ETIME) {

+                    ret = LDAP_TIMELIMIT_EXCEEDED;

                 } else {

                     ret = LDAP_OPERATIONS_ERROR;

                 }

@@ -907,6 +915,8 @@ static int handle_gid_request(struct ipa_extdom_ctx *ctx,

         if (ret != 0 || id_type != SSS_ID_TYPE_GID) {

             if (ret == ENOENT) {

                 ret = LDAP_NO_SUCH_OBJECT;

+            } else if (ret == ETIMEDOUT || ret == ETIME) {

+                ret = LDAP_TIMELIMIT_EXCEEDED;

             } else {

                 set_err_msg(req, "Failed to lookup SID by GID");

                 ret = LDAP_OPERATIONS_ERROR;

@@ -918,10 +928,12 @@ static int handle_gid_request(struct ipa_extdom_ctx *ctx,

     } else {

         ret = getgrgid_r_wrapper(ctx, gid, &grp, &buf, &buf_len);

         if (ret != 0) {

-            if (ret == ENOMEM || ret == ERANGE) {

-                ret = LDAP_OPERATIONS_ERROR;

-            } else {

+            if (ret == ENOENT) {

                 ret = LDAP_NO_SUCH_OBJECT;

+            } else if (ret == ETIMEDOUT) {

+                ret = LDAP_TIMELIMIT_EXCEEDED;

+            } else {

+                ret = LDAP_OPERATIONS_ERROR;

             }

             goto done;

         }

@@ -933,6 +945,8 @@ static int handle_gid_request(struct ipa_extdom_ctx *ctx,

                 set_err_msg(req, "Failed to read original data");

                 if (ret == ENOENT) {

                     ret = LDAP_NO_SUCH_OBJECT;

+                } else if (ret == ETIMEDOUT || ret == ETIME) {

+                    ret = LDAP_TIMELIMIT_EXCEEDED;

                 } else {

                     ret = LDAP_OPERATIONS_ERROR;

                 }

@@ -976,6 +990,8 @@ static int handle_cert_request(struct ipa_extdom_ctx *ctx,

     if (ret != 0) {

         if (ret == ENOENT) {

             ret = LDAP_NO_SUCH_OBJECT;

+        } else if (ret == ETIMEDOUT || ret == ETIME) {

+            ret = LDAP_TIMELIMIT_EXCEEDED;

         } else {

             set_err_msg(req, "Failed to lookup name by certificate");

             ret = LDAP_OPERATIONS_ERROR;

@@ -1020,6 +1036,8 @@ static int handle_sid_request(struct ipa_extdom_ctx *ctx,

     if (ret != 0) {

         if (ret == ENOENT) {

             ret = LDAP_NO_SUCH_OBJECT;

+        } else if (ret == ETIMEDOUT || ret == ETIME) {

+            ret = LDAP_TIMELIMIT_EXCEEDED;

         } else {

             set_err_msg(req, "Failed to lookup name by SID");

             ret = LDAP_OPERATIONS_ERROR;

@@ -1057,10 +1075,12 @@ static int handle_sid_request(struct ipa_extdom_ctx *ctx,

     case SSS_ID_TYPE_BOTH:

         ret = getpwnam_r_wrapper(ctx, fq_name, &pwd, &buf, &buf_len);

         if (ret != 0) {

-            if (ret == ENOMEM || ret == ERANGE) {

-                ret = LDAP_OPERATIONS_ERROR;

-            } else {

+            if (ret == ENOENT) {

                 ret = LDAP_NO_SUCH_OBJECT;

+            } else if (ret == ETIMEDOUT) {

+                ret = LDAP_TIMELIMIT_EXCEEDED;

+            } else {

+                ret = LDAP_OPERATIONS_ERROR;

             }

             goto done;

         }

@@ -1072,6 +1092,8 @@ static int handle_sid_request(struct ipa_extdom_ctx *ctx,

                 set_err_msg(req, "Failed to read original data");

                 if (ret == ENOENT) {

                     ret = LDAP_NO_SUCH_OBJECT;

+                } else if (ret == ETIMEDOUT || ret == ETIME) {

+                    ret = LDAP_TIMELIMIT_EXCEEDED;

                 } else {

                     ret = LDAP_OPERATIONS_ERROR;

                 }

@@ -1089,10 +1111,12 @@ static int handle_sid_request(struct ipa_extdom_ctx *ctx,

     case SSS_ID_TYPE_GID:

         ret = getgrnam_r_wrapper(ctx, fq_name, &grp, &buf, &buf_len);

         if (ret != 0) {

-            if (ret == ENOMEM || ret == ERANGE) {

-                ret = LDAP_OPERATIONS_ERROR;

-            } else {

+            if (ret == ENOENT) {

                 ret = LDAP_NO_SUCH_OBJECT;

+            } else if (ret == ETIMEDOUT) {

+                ret = LDAP_TIMELIMIT_EXCEEDED;

+            } else {

+                ret = LDAP_OPERATIONS_ERROR;

             }

             goto done;

         }

@@ -1104,6 +1128,8 @@ static int handle_sid_request(struct ipa_extdom_ctx *ctx,

                 set_err_msg(req, "Failed to read original data");

                 if (ret == ENOENT) {

                     ret = LDAP_NO_SUCH_OBJECT;

+                } else if (ret == ETIMEDOUT || ret == ETIME) {

+                    ret = LDAP_TIMELIMIT_EXCEEDED;

                 } else {

                     ret = LDAP_OPERATIONS_ERROR;

                 }

@@ -1167,6 +1193,8 @@ static int handle_name_request(struct ipa_extdom_ctx *ctx,

         if (ret != 0) {

             if (ret == ENOENT) {

                 ret = LDAP_NO_SUCH_OBJECT;

+            } else if (ret == ETIMEDOUT || ret == ETIME) {

+                ret = LDAP_TIMELIMIT_EXCEEDED;

             } else {

                 set_err_msg(req, "Failed to lookup SID by name");

                 ret = LDAP_OPERATIONS_ERROR;

@@ -1190,6 +1218,8 @@ static int handle_name_request(struct ipa_extdom_ctx *ctx,

                     set_err_msg(req, "Failed to read original data");

                     if (ret == ENOENT) {

                         ret = LDAP_NO_SUCH_OBJECT;

+                    } else if (ret == ETIMEDOUT || ret == ETIME) {

+                        ret = LDAP_TIMELIMIT_EXCEEDED;

                     } else {

                         ret = LDAP_OPERATIONS_ERROR;

                     }

@@ -1205,6 +1235,9 @@ static int handle_name_request(struct ipa_extdom_ctx *ctx,

         } else if (ret == ENOMEM || ret == ERANGE) {

             ret = LDAP_OPERATIONS_ERROR;

             goto done;

+        } else if (ret == ETIMEDOUT) {

+            ret = LDAP_TIMELIMIT_EXCEEDED;

+            goto done;

         } else { /* no user entry found */

             /* according to the getpwnam() man page there are a couple of

              * error codes which can indicate that the user was not found. To

@@ -1212,10 +1245,12 @@ static int handle_name_request(struct ipa_extdom_ctx *ctx,

              * errors. */

             ret = getgrnam_r_wrapper(ctx, fq_name, &grp, &buf, &buf_len);

             if (ret != 0) {

-                if (ret == ENOMEM || ret == ERANGE) {

-                    ret = LDAP_OPERATIONS_ERROR;

-                } else {

+                if (ret == ENOENT) {

                     ret = LDAP_NO_SUCH_OBJECT;

+                } else if (ret == ETIMEDOUT) {

+                    ret = LDAP_TIMELIMIT_EXCEEDED;

+                } else {

+                    ret = LDAP_OPERATIONS_ERROR;

                 }

                 goto done;

             }

@@ -1226,6 +1261,8 @@ static int handle_name_request(struct ipa_extdom_ctx *ctx,

                                     || id_type == SSS_ID_TYPE_BOTH)) {

                     if (ret == ENOENT) {

                         ret = LDAP_NO_SUCH_OBJECT;

+                    } else if (ret == ETIMEDOUT || ret == ETIME) {

+                        ret = LDAP_TIMELIMIT_EXCEEDED;

                     } else {

                         set_err_msg(req, "Failed to read original data");

                         ret = LDAP_OPERATIONS_ERROR;

diff --git a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_extop.c b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_extop.c

index 10d3f86eb..48fcecc1e 100644

--- a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_extop.c

+++ b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_extop.c

@@ -242,6 +242,8 @@ static int ipa_extdom_extop(Slapi_PBlock *pb)

     if (ret != LDAP_SUCCESS) {

         if (ret == LDAP_NO_SUCH_OBJECT) {

             rc = LDAP_NO_SUCH_OBJECT;

+        } else if (ret == LDAP_TIMELIMIT_EXCEEDED) {

+            rc = LDAP_TIMELIMIT_EXCEEDED;

         } else {

             rc = LDAP_OPERATIONS_ERROR;

             err_msg = "Failed to handle the request.\n";

-- 

2.21.0

From 387ed98e59ba4df8d3fd435cfc84f055970c064e Mon Sep 17 00:00:00 2001

From: Alexander Bokovoy <abokovoy@redhat.com>

Date: Mon, 19 Aug 2019 10:15:50 +0300

Subject: [PATCH 2/2] ipa-extdom-extop: test timed out getgrgid_r

Simulate getgrgid_r() timeout when packing list of groups user is a

member of in pack_ber_user().

Related: https://pagure.io/freeipa/issue/8044

Reviewed-By: Alexander Bokovoy <abokovoy@redhat.com>

---

 .../ipa_extdom_cmocka_tests.c                 | 29 +++++++++++++++++++

 1 file changed, 29 insertions(+)

diff --git a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_cmocka_tests.c b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_cmocka_tests.c

index 29699cfa3..1fa4c6af8 100644

--- a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_cmocka_tests.c

+++ b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_cmocka_tests.c

@@ -493,6 +493,34 @@ void test_set_err_msg(void **state)

 #define TEST_SID "S-1-2-3-4"

 #define TEST_DOMAIN_NAME "DOMAIN"

+/* Always time out for test */

+static

+enum nss_status getgrgid_r_timeout(gid_t gid, struct group *result,

+                                   char *buffer, size_t buflen, int *errnop) {

+    return NSS_STATUS_UNAVAIL;

+}

+

+void test_pack_ber_user_timeout(void **state)

+{

+    int ret;

+    struct berval *resp_val = NULL;

+    struct test_data *test_data;

+    enum nss_status (*oldgetgrgid_r)(gid_t gid, struct group *result,

+                                     char *buffer, size_t buflen, int *errnop);

+

+    test_data = (struct test_data *) *state;

+

+    oldgetgrgid_r = test_data->ctx->nss_ctx->getgrgid_r;

+    test_data->ctx->nss_ctx->getgrgid_r = getgrgid_r_timeout;

+

+    ret = pack_ber_user(test_data->ctx, RESP_USER_GROUPLIST,

+                        TEST_DOMAIN_NAME, "member001", 12345, 54321,

+                        "gecos", "homedir", "shell", NULL, &resp_val);

+    test_data->ctx->nss_ctx->getgrgid_r = oldgetgrgid_r;

+    assert_int_equal(ret, LDAP_TIMELIMIT_EXCEEDED);

+    ber_bvfree(resp_val);

+}

+

 char res_sid[] = {0x30, 0x0e, 0x0a, 0x01, 0x01, 0x04, 0x09, 0x53, 0x2d, 0x31, \

                   0x2d, 0x32, 0x2d, 0x33, 0x2d, 0x34};

 char res_nam[] = {0x30, 0x13, 0x0a, 0x01, 0x02, 0x30, 0x0e, 0x04, 0x06, 0x44, \

@@ -614,6 +642,7 @@ void test_decode(void **state)

 int main(int argc, const char *argv[])

 {

     const struct CMUnitTest tests[] = {

+        cmocka_unit_test(test_pack_ber_user_timeout),

         cmocka_unit_test(test_getpwnam_r_wrapper),

         cmocka_unit_test(test_getpwuid_r_wrapper),

         cmocka_unit_test(test_getgrnam_r_wrapper),

-- 

2.21.0