From 62aa4da7dc3a7826c5a942349e1af9631ca819f5 Mon Sep 17 00:00:00 2001 From: Jakub Hrozek Date: Fri, 2 Oct 2009 10:39:02 +0200 Subject: Fix error messages in tools Add getpwnam, getgrnam sync versions Fix ticket #164: Groupnames in non-local domains Fix ticket #100: Error Message Modifying a user that doesn't Exist Fix ticket #214: incorrect error message when MPG already exists Fix ticket #188: Deleting and modifying users in non-local domain Fix ticket #120: Adding a user to a full domain gives unhelpful error message --- server/tools/sss_groupadd.c | 4 + server/tools/sss_groupdel.c | 19 +---- server/tools/sss_groupmod.c | 34 ++++++++ server/tools/sss_sync_ops.c | 201 ++++++++++++++++++++++++++++++++++++++++++++ server/tools/sss_sync_ops.h | 14 +++ server/tools/sss_useradd.c | 17 +++- server/tools/sss_userdel.c | 18 +--- server/tools/sss_usermod.c | 35 +++++++- server/tools/tools_util.c | 31 +++++++ server/tools/tools_util.h | 4 + 10 files changed, 342 insertions(+), 35 deletions(-) (limited to 'server') diff --git a/server/tools/sss_groupadd.c b/server/tools/sss_groupadd.c index 1f6a2f117..82d4573db 100644 --- a/server/tools/sss_groupadd.c +++ b/server/tools/sss_groupadd.c @@ -125,6 +125,10 @@ done: if (tctx->error) { ret = tctx->error; switch (ret) { + case ERANGE: + ERROR("Could not allocate ID for the group - domain full?\n"); + break; + case EEXIST: ERROR("A group with the same name or GID already exists\n"); break; diff --git a/server/tools/sss_groupdel.c b/server/tools/sss_groupdel.c index a4451c43d..d6e3dfd6a 100644 --- a/server/tools/sss_groupdel.c +++ b/server/tools/sss_groupdel.c @@ -23,8 +23,6 @@ #include #include #include -#include -#include #include "db/sysdb.h" #include "util/util.h" @@ -35,7 +33,6 @@ int main(int argc, const char **argv) { int ret = EXIT_SUCCESS; int pc_debug = 0; - struct group *grp_info; const char *pc_groupname = NULL; struct tools_ctx *tctx = NULL; @@ -93,19 +90,6 @@ int main(int argc, const char **argv) goto fini; } - /* arguments processed, go on to actual work */ - grp_info = getgrnam(tctx->octx->name); - if (grp_info) { - tctx->octx->gid = grp_info->gr_gid; - } - - /* arguments processed, go on to actual work */ - if (id_in_range(tctx->octx->uid, tctx->octx->domain) != EOK) { - ERROR("The selected GID is outside the allowed range\n"); - ret = EXIT_FAILURE; - goto fini; - } - start_transaction(tctx); if (tctx->error != EOK) { goto done; @@ -129,7 +113,8 @@ done: DEBUG(1, ("sysdb operation failed (%d)[%s]\n", ret, strerror(ret))); switch (ret) { case ENOENT: - ERROR("No such group\n"); + ERROR("No such group in local domain. " + "Removing groups only allowed in local domain.\n"); break; default: diff --git a/server/tools/sss_groupmod.c b/server/tools/sss_groupmod.c index e821fdc39..d3a35988a 100644 --- a/server/tools/sss_groupmod.c +++ b/server/tools/sss_groupmod.c @@ -53,6 +53,7 @@ int main(int argc, const char **argv) char *addgroups = NULL, *rmgroups = NULL; int ret; const char *pc_groupname = NULL; + char *badgroup = NULL; debug_prg_name = argv[0]; @@ -117,6 +118,17 @@ int main(int argc, const char **argv) ret = EXIT_FAILURE; goto fini; } + /* check the username to be able to give sensible error message */ + ret = sysdb_getgrnam_sync(tctx, tctx->ev, tctx->sysdb, + tctx->octx->name, tctx->local, + &tctx->octx); + if (ret != EOK) { + ERROR("Cannot find group in local domain, " + "modifying groups is allowed only in local domain\n"); + ret = EXIT_FAILURE; + goto fini; + } + tctx->octx->gid = pc_gid; @@ -125,6 +137,7 @@ int main(int argc, const char **argv) if (ret != EOK) { DEBUG(1, ("Cannot parse groups to add the group to\n")); ERROR("Internal error while parsing parameters\n"); + ret = EXIT_FAILURE; goto fini; } @@ -132,6 +145,16 @@ int main(int argc, const char **argv) if (ret != EOK) { DEBUG(1, ("Cannot parse FQDN groups to add the group to\n")); ERROR("Member groups must be in the same domain as parent group\n"); + ret = EXIT_FAILURE; + goto fini; + } + + /* Check group names in the LOCAL domain */ + ret = check_group_names(tctx, tctx->octx->addgroups, &badgroup); + if (ret != EOK) { + ERROR("Cannot find group %s in local domain, " + "only groups in local domain are allowed\n", badgroup); + ret = EXIT_FAILURE; goto fini; } } @@ -141,6 +164,7 @@ int main(int argc, const char **argv) if (ret != EOK) { DEBUG(1, ("Cannot parse groups to remove the group from\n")); ERROR("Internal error while parsing parameters\n"); + ret = EXIT_FAILURE; goto fini; } @@ -148,6 +172,16 @@ int main(int argc, const char **argv) if (ret != EOK) { DEBUG(1, ("Cannot parse FQDN groups to remove the group from\n")); ERROR("Member groups must be in the same domain as parent group\n"); + ret = EXIT_FAILURE; + goto fini; + } + + /* Check group names in the LOCAL domain */ + ret = check_group_names(tctx, tctx->octx->rmgroups, &badgroup); + if (ret != EOK) { + ERROR("Cannot find group %s in local domain, " + "only groups in local domain are allowed\n", badgroup); + ret = EXIT_FAILURE; goto fini; } } diff --git a/server/tools/sss_sync_ops.c b/server/tools/sss_sync_ops.c index 68fe73e54..932a71222 100644 --- a/server/tools/sss_sync_ops.c +++ b/server/tools/sss_sync_ops.c @@ -44,6 +44,7 @@ } while(0) struct sync_op_res { + struct ops_ctx *data; int error; bool done; }; @@ -1138,6 +1139,10 @@ int useradd_defaults(TALLOC_CTX *mem_ctx, if (homedir) { data->home = talloc_strdup(data, homedir); + if (data->home == NULL) { + ret = ENOMEM; + goto done; + } } else { ret = confdb_get_string(confdb, mem_ctx, conf_path, CONFDB_LOCAL_DEFAULT_BASEDIR, @@ -1546,3 +1551,199 @@ static void end_transaction_done(struct tevent_req *req) talloc_zfree(req); } +/* + * getpwnam, getgrnam and friends + */ +static void sss_getpwnam_done(void *ptr, int status, + struct ldb_result *lrs); + +int sysdb_getpwnam_sync(TALLOC_CTX *mem_ctx, + struct tevent_context *ev, + struct sysdb_ctx *sysdb, + const char *name, + struct sss_domain_info *domain, + struct ops_ctx **out) +{ + int ret; + struct sync_op_res *res = NULL; + + res = talloc_zero(mem_ctx, struct sync_op_res); + if (!res) { + return ENOMEM; + } + + if (out == NULL) { + DEBUG(1, ("NULL passed for storage pointer\n")); + return EINVAL; + } + res->data = *out; + + ret = sysdb_getpwnam(mem_ctx, + sysdb, + domain, + name, + sss_getpwnam_done, + res); + + SYNC_LOOP(res, ret); + + return ret; +} + +static void sss_getpwnam_done(void *ptr, int status, + struct ldb_result *lrs) +{ + struct sync_op_res *res = talloc_get_type(ptr, struct sync_op_res ); + const char *str; + + res->done = true; + + if (status != LDB_SUCCESS) { + res->error = status; + return; + } + + switch (lrs->count) { + case 0: + DEBUG(1, ("No result for sysdb_getpwnam call\n")); + res->error = ENOENT; + break; + + case 1: + res->error = EOK; + /* fill ops_ctx */ + res->data->uid = ldb_msg_find_attr_as_uint64(lrs->msgs[0], + SYSDB_UIDNUM, 0); + + res->data->gid = ldb_msg_find_attr_as_uint64(lrs->msgs[0], + SYSDB_GIDNUM, 0); + + str = ldb_msg_find_attr_as_string(lrs->msgs[0], + SYSDB_NAME, NULL); + res->data->name = talloc_strdup(res, str); + if (res->data->name == NULL) { + res->error = ENOMEM; + return; + } + + str = ldb_msg_find_attr_as_string(lrs->msgs[0], + SYSDB_GECOS, NULL); + res->data->gecos = talloc_strdup(res, str); + if (res->data->gecos == NULL) { + res->error = ENOMEM; + return; + } + + str = ldb_msg_find_attr_as_string(lrs->msgs[0], + SYSDB_HOMEDIR, NULL); + res->data->home = talloc_strdup(res, str); + if (res->data->home == NULL) { + res->error = ENOMEM; + return; + } + + str = ldb_msg_find_attr_as_string(lrs->msgs[0], + SYSDB_SHELL, NULL); + res->data->shell = talloc_strdup(res, str); + if (res->data->shell == NULL) { + res->error = ENOMEM; + return; + } + + str = ldb_msg_find_attr_as_string(lrs->msgs[0], + SYSDB_DISABLED, NULL); + if (str == NULL) { + res->data->lock = DO_UNLOCK; + } else { + if (strcasecmp(str, "true") == 0) { + res->data->lock = DO_LOCK; + } else if (strcasecmp(str, "false") == 0) { + res->data->lock = DO_UNLOCK; + } else { /* Invalid value */ + DEBUG(2, ("Invalid value for %s attribute: %s\n", + SYSDB_DISABLED, str ? str : "NULL")); + res->error = EIO; + return; + } + } + break; + + default: + DEBUG(1, ("More than one result for sysdb_getpwnam call\n")); + res->error = EIO; + break; + } +} + +static void sss_getgrnam_done(void *ptr, int status, + struct ldb_result *lrs); + +int sysdb_getgrnam_sync(TALLOC_CTX *mem_ctx, + struct tevent_context *ev, + struct sysdb_ctx *sysdb, + const char *name, + struct sss_domain_info *domain, + struct ops_ctx **out) +{ + int ret; + struct sync_op_res *res = NULL; + + res = talloc_zero(mem_ctx, struct sync_op_res); + if (!res) { + return ENOMEM; + } + + if (out == NULL) { + DEBUG(1, ("NULL passed for storage pointer\n")); + return EINVAL; + } + res->data = *out; + + ret = sysdb_getgrnam(mem_ctx, + sysdb, + domain, + name, + sss_getgrnam_done, + res); + + SYNC_LOOP(res, ret); + + return ret; +} + +static void sss_getgrnam_done(void *ptr, int status, + struct ldb_result *lrs) +{ + struct sync_op_res *res = talloc_get_type(ptr, struct sync_op_res ); + const char *str; + + res->done = true; + + if (status != LDB_SUCCESS) { + res->error = status; + return; + } + + switch (lrs->count) { + case 0: + DEBUG(1, ("No result for sysdb_getgrnam call\n")); + res->error = ENOENT; + break; + + /* sysdb_getgrnam also returns members */ + default: + res->error = EOK; + /* fill ops_ctx */ + res->data->gid = ldb_msg_find_attr_as_uint64(lrs->msgs[0], + SYSDB_GIDNUM, 0); + str = ldb_msg_find_attr_as_string(lrs->msgs[0], + SYSDB_NAME, NULL); + res->data->name = talloc_strdup(res, str); + if (res->data->name == NULL) { + res->error = ENOMEM; + return; + } + break; + } +} + diff --git a/server/tools/sss_sync_ops.h b/server/tools/sss_sync_ops.h index c99b9b911..3988992e9 100644 --- a/server/tools/sss_sync_ops.h +++ b/server/tools/sss_sync_ops.h @@ -86,5 +86,19 @@ int groupmod(TALLOC_CTX *mem_ctx, void start_transaction(struct tools_ctx *tctx); void end_transaction(struct tools_ctx *tctx); +int sysdb_getpwnam_sync(TALLOC_CTX *mem_ctx, + struct tevent_context *ev, + struct sysdb_ctx *sysdb, + const char *name, + struct sss_domain_info *domain, + struct ops_ctx **out); + +int sysdb_getgrnam_sync(TALLOC_CTX *mem_ctx, + struct tevent_context *ev, + struct sysdb_ctx *sysdb, + const char *name, + struct sss_domain_info *domain, + struct ops_ctx **out); + #endif /* __SSS_OPS_H__ */ diff --git a/server/tools/sss_useradd.c b/server/tools/sss_useradd.c index 07e741e47..f805b242e 100644 --- a/server/tools/sss_useradd.c +++ b/server/tools/sss_useradd.c @@ -131,6 +131,7 @@ int main(int argc, const char **argv) poptContext pc = NULL; struct tools_ctx *tctx = NULL; char *groups = NULL; + char *badgroup = NULL; int ret; debug_prg_name = argv[0]; @@ -195,6 +196,7 @@ int main(int argc, const char **argv) if (ret != EOK) { DEBUG(1, ("Cannot parse groups to add the user to\n")); ERROR("Internal error while parsing parameters\n"); + ret = EXIT_FAILURE; goto fini; } @@ -202,6 +204,15 @@ int main(int argc, const char **argv) if (ret != EOK) { DEBUG(1, ("Cannot parse FQDN groups to add the user to\n")); ERROR("Groups must be in the same domain as user\n"); + ret = EXIT_FAILURE; + goto fini; + } + + /* Check group names in the LOCAL domain */ + ret = check_group_names(tctx, tctx->octx->addgroups, &badgroup); + if (ret != EOK) { + ERROR("Cannot find group %s in local domain\n", badgroup); + ret = EXIT_FAILURE; goto fini; } } @@ -256,8 +267,12 @@ int main(int argc, const char **argv) done: if (tctx->error) { switch (tctx->error) { + case ERANGE: + ERROR("Could not allocate ID for the user - domain full?\n"); + break; + case EEXIST: - ERROR("A user with the same name or UID already exists\n"); + ERROR("A user or group with the same name or ID already exists\n"); break; default: diff --git a/server/tools/sss_userdel.c b/server/tools/sss_userdel.c index 8c7c7bd70..d14ef3da3 100644 --- a/server/tools/sss_userdel.c +++ b/server/tools/sss_userdel.c @@ -23,8 +23,6 @@ #include #include #include -#include -#include #include "db/sysdb.h" #include "util/util.h" @@ -35,7 +33,6 @@ int main(int argc, const char **argv) { int ret = EXIT_SUCCESS; struct tools_ctx *tctx = NULL; - struct passwd *pwd_info; const char *pc_username = NULL; int pc_debug = 0; @@ -93,18 +90,6 @@ int main(int argc, const char **argv) goto fini; } - /* arguments processed, go on to actual work */ - pwd_info = getpwnam(tctx->octx->name); - if (pwd_info) { - tctx->octx->uid = pwd_info->pw_uid; - } - - if (id_in_range(tctx->octx->uid, tctx->octx->domain) != EOK) { - ERROR("The selected UID is outside the allowed range\n"); - ret = EXIT_FAILURE; - goto fini; - } - start_transaction(tctx); if (tctx->error != EOK) { goto done; @@ -128,7 +113,8 @@ done: DEBUG(1, ("sysdb operation failed (%d)[%s]\n", ret, strerror(ret))); switch (ret) { case ENOENT: - ERROR("No such user\n"); + ERROR("No such user in local domain. " + "Removing users only allowed in local domain.\n"); break; default: diff --git a/server/tools/sss_usermod.c b/server/tools/sss_usermod.c index 2c2b80ec2..733f9982e 100644 --- a/server/tools/sss_usermod.c +++ b/server/tools/sss_usermod.c @@ -60,6 +60,7 @@ int main(int argc, const char **argv) int ret; const char *pc_username = NULL; struct tools_ctx *tctx = NULL; + char *badgroup = NULL; debug_prg_name = argv[0]; @@ -133,6 +134,16 @@ int main(int argc, const char **argv) ret = EXIT_FAILURE; goto fini; } + /* check the username to be able to give sensible error message */ + ret = sysdb_getpwnam_sync(tctx, tctx->ev, tctx->sysdb, + tctx->octx->name, tctx->local, + &tctx->octx); + if (ret != EOK) { + ERROR("Cannot find user in local domain, " + "modifying users is allowed only in local domain\n"); + ret = EXIT_FAILURE; + goto fini; + } if (id_in_range(tctx->octx->uid, tctx->octx->domain) != EOK) { ERROR("The selected UID is outside the allowed range\n"); @@ -145,6 +156,7 @@ int main(int argc, const char **argv) if (ret != EOK) { DEBUG(1, ("Cannot parse groups to add the user to\n")); ERROR("Internal error while parsing parameters\n"); + ret = EXIT_FAILURE; goto fini; } @@ -152,6 +164,16 @@ int main(int argc, const char **argv) if (ret != EOK) { DEBUG(1, ("Cannot parse FQDN groups to add the user to\n")); ERROR("Groups must be in the same domain as user\n"); + ret = EXIT_FAILURE; + goto fini; + } + + /* Check group names in the LOCAL domain */ + ret = check_group_names(tctx, tctx->octx->addgroups, &badgroup); + if (ret != EOK) { + ERROR("Cannot find group %s in local domain, " + "only groups in local domain are allowed\n", badgroup); + ret = EXIT_FAILURE; goto fini; } } @@ -161,6 +183,7 @@ int main(int argc, const char **argv) if (ret != EOK) { DEBUG(1, ("Cannot parse groups to remove the user from\n")); ERROR("Internal error while parsing parameters\n"); + ret = EXIT_FAILURE; goto fini; } @@ -168,6 +191,16 @@ int main(int argc, const char **argv) if (ret != EOK) { DEBUG(1, ("Cannot parse FQDN groups to remove the user from\n")); ERROR("Groups must be in the same domain as user\n"); + ret = EXIT_FAILURE; + goto fini; + } + + /* Check group names in the LOCAL domain */ + ret = check_group_names(tctx, tctx->octx->rmgroups, &badgroup); + if (ret != EOK) { + ERROR("Cannot find group %s in local domain, " + "only groups in local domain are allowed\n", badgroup); + ret = EXIT_FAILURE; goto fini; } } @@ -205,7 +238,7 @@ done: break; case EFAULT: - ERROR("Could not modify user - check if username is correct\n"); + ERROR("Could not modify user - user already member of groups?\n"); break; default: diff --git a/server/tools/tools_util.c b/server/tools/tools_util.c index 777217213..bc6b76f47 100644 --- a/server/tools/tools_util.c +++ b/server/tools/tools_util.c @@ -191,6 +191,37 @@ int parse_name_domain(struct tools_ctx *tctx, return EOK; } +int check_group_names(struct tools_ctx *tctx, + char **grouplist, + char **badgroup) +{ + int ret; + int i; + struct ops_ctx *groupinfo; + + groupinfo = talloc_zero(tctx, struct ops_ctx); + if (!groupinfo) { + return ENOMEM; + } + + for (i=0; grouplist[i]; ++i) { + ret = sysdb_getgrnam_sync(tctx, + tctx->ev, + tctx->sysdb, + grouplist[i], + tctx->local, + &groupinfo); + if (ret) { + DEBUG(6, ("Cannot find group %s, ret: %d\n", grouplist[i], ret)); + break; + } + } + + talloc_zfree(groupinfo); + *badgroup = grouplist[i]; + return ret; +} + int id_in_range(uint32_t id, struct sss_domain_info *dom) { diff --git a/server/tools/tools_util.h b/server/tools/tools_util.h index c062decec..a56fc5eb7 100644 --- a/server/tools/tools_util.h +++ b/server/tools/tools_util.h @@ -72,4 +72,8 @@ int parse_groups(TALLOC_CTX *mem_ctx, int parse_group_name_domain(struct tools_ctx *tctx, char **groups); +int check_group_names(struct tools_ctx *tctx, + char **grouplist, + char **badgroup); + #endif /* __TOOLS_UTIL_H__ */ -- cgit