From 3bb72545fc337564e0843b0c72906a9a1e3f6a06 Mon Sep 17 00:00:00 2001

From: Sumit Bose <sbose@redhat.com>

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

Subject: [PATCH] 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 ef552a9a37..163e8e1371 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

@@ -62,10 +62,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 525487c9e4..65c723ce65 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 10d3f86eba..48fcecc1ee 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";

From 0ead6f59732e8b3370c5d8d05acd29f2d56c52bb Mon Sep 17 00:00:00 2001

From: Alexander Bokovoy <abokovoy@redhat.com>

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

Subject: [PATCH] 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 29699cfa39..1fa4c6af82 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),