URL: https://github.com/freeipa/freeipa/pull/3542 Author: sumit-bose Title: #3542: extdom: unify error code handling especially LDAP_NO_SUCH_OBJECT Action: opened
PR body: """ 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. Related to https://pagure.io/freeipa/issue/8044 """ To pull the PR as Git branch: git remote add ghfreeipa https://github.com/freeipa/freeipa git fetch ghfreeipa pull/3542/head:pr3542 git checkout pr3542
From 8f1f89858c48da27ce1b9f7c46281b61c744b4f1 Mon Sep 17 00:00:00 2001 From: Sumit Bose <sb...@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. Related to https://pagure.io/freeipa/issue/8044 --- .../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";
_______________________________________________ FreeIPA-devel mailing list -- freeipa-devel@lists.fedorahosted.org To unsubscribe send an email to freeipa-devel-le...@lists.fedorahosted.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/freeipa-devel@lists.fedorahosted.org
