These patches improve the initgroups call. Previously any call to initgroups would go on the wire, now we cache initgroups calls the same way we do for other "queries".
0001 Adds initgroups caching (depends on check_cache patch sent earlier) This patch also simplifies the initgroups call. All backends already do a search for the user, so doing an explicit user search in the nss responder is pointless and wastes time. 0002 Change the pam code to do an online initgroups call instead of a search user call, an initgroups call also does a user search call, plus it also fetches all users groups so that, at login, access control against groups can be done against a properly refreshed group list. 0003 Store the initgroups expire time in the ldap driver. Simo. -- Simo Sorce * Red Hat, Inc * New York
>From 46c1baa9f29347e2be8e3789fd55f87fadb7396f Mon Sep 17 00:00:00 2001 From: Simo Sorce <[email protected]> Date: Wed, 11 Nov 2009 21:52:09 -0500 Subject: [PATCH 1/3] Change initgroups code to use and check the cache We were previously always ending up contacting the backend because we had no way to know if an initgroups call for the same user had ever been called. Add attribute to hold this information and rely on backends to update it. If they don't we fallback to the previous behvior of asking the backend. --- server/db/sysdb.h | 33 ++--- server/responder/nss/nsssrv_cmd.c | 294 ++++++++++++------------------------ 2 files changed, 109 insertions(+), 218 deletions(-) diff --git a/server/db/sysdb.h b/server/db/sysdb.h index 4c92c37..a329985 100644 --- a/server/db/sysdb.h +++ b/server/db/sysdb.h @@ -66,6 +66,7 @@ #define SYSDB_LAST_UPDATE "lastUpdate" #define SYSDB_CACHE_EXPIRE "dataExpireTimestamp" +#define SYSDB_INITGR_EXPIRE "initgrExpireTimestamp" #define SYSDB_CACHEDPWD "cachedPassword" @@ -101,41 +102,29 @@ #define SYSDB_GETCACHED_FILTER "(&"SYSDB_UC")("SYSDB_LAST_LOGIN">=%lu))" +#define SYSDB_DEFAULT_ATTRS SYSDB_LAST_UPDATE, \ + SYSDB_CACHE_EXPIRE, \ + SYSDB_INITGR_EXPIRE, \ + "objectClass" + #define SYSDB_PW_ATTRS {SYSDB_NAME, SYSDB_UIDNUM, \ SYSDB_GIDNUM, SYSDB_GECOS, \ SYSDB_HOMEDIR, SYSDB_SHELL, \ - SYSDB_LAST_UPDATE, SYSDB_CACHE_EXPIRE, \ - "objectClass", \ + SYSDB_DEFAULT_ATTRS, \ NULL} -#define SYSDB_USER_ATTRS {SYSDB_DEFAULTGROUP, \ - SYSDB_GECOS, \ - SYSDB_HOMEDIR, \ - SYSDB_SHELL, \ - SYSDB_FULLNAME, \ - SYSDB_LOCALE, \ - SYSDB_KEYBOARD, \ - SYSDB_SESSION, \ - SYSDB_LAST_LOGIN, \ - SYSDB_USERPIC, \ - SYSDB_LAST_UPDATE, SYSDB_CACHE_EXPIRE, \ - NULL} #define SYSDB_GRSRC_ATTRS {SYSDB_NAME, SYSDB_GIDNUM, \ - SYSDB_LAST_UPDATE, SYSDB_CACHE_EXPIRE, \ - "objectClass", \ + SYSDB_DEFAULT_ATTRS, \ NULL} #define SYSDB_GRPW_ATTRS {SYSDB_NAME, SYSDB_UIDNUM, \ - SYSDB_LAST_UPDATE, SYSDB_CACHE_EXPIRE, \ - "objectClass", \ + SYSDB_DEFAULT_ATTRS, \ NULL} #define SYSDB_GRENT_ATTRS {SYSDB_NAME, SYSDB_UIDNUM, SYSDB_MEMBEROF, \ - SYSDB_LAST_UPDATE, SYSDB_CACHE_EXPIRE, \ - "objectClass", \ + SYSDB_DEFAULT_ATTRS, \ NULL} #define SYSDB_INITGR_ATTR SYSDB_MEMBEROF #define SYSDB_INITGR_ATTRS {SYSDB_GIDNUM, \ - SYSDB_LAST_UPDATE, SYSDB_CACHE_EXPIRE, \ - "objectClass", \ + SYSDB_DEFAULT_ATTRS, \ NULL} #define SYSDB_TMPL_USER SYSDB_NAME"=%s,"SYSDB_TMPL_USER_BASE diff --git a/server/responder/nss/nsssrv_cmd.c b/server/responder/nss/nsssrv_cmd.c index b2a2035..a029baf 100644 --- a/server/responder/nss/nsssrv_cmd.c +++ b/server/responder/nss/nsssrv_cmd.c @@ -280,7 +280,7 @@ static errno_t check_cache(struct nss_dom_ctx *dctx, int timeout; time_t now; uint64_t lastUpdate; - uint64_t cacheExpire; + uint64_t cacheExpire = 0; uint64_t midpoint_refresh; struct nss_cmd_ctx *cmdctx = dctx->cmdctx; struct cli_ctx *cctx = cmdctx->cctx; @@ -305,8 +305,14 @@ static errno_t check_cache(struct nss_dom_ctx *dctx, lastUpdate = ldb_msg_find_attr_as_uint64(res->msgs[0], SYSDB_LAST_UPDATE, 0); - cacheExpire = ldb_msg_find_attr_as_uint64(res->msgs[0], - SYSDB_CACHE_EXPIRE, 0); + if (req_type == SSS_DP_INITGROUPS) { + cacheExpire = ldb_msg_find_attr_as_uint64(res->msgs[0], + SYSDB_INITGR_EXPIRE, 1); + } + if (cacheExpire == 0) { + cacheExpire = ldb_msg_find_attr_as_uint64(res->msgs[0], + SYSDB_CACHE_EXPIRE, 0); + } midpoint_refresh = 0; if(nctx->cache_refresh_percent) { @@ -2780,147 +2786,47 @@ done: return EOK; } -static void nss_cmd_initgr_callback(void *ptr, int status, - struct ldb_result *res) +static int fill_initgr(struct sss_packet *packet, struct ldb_result *res) { - struct nss_cmd_ctx *cmdctx = talloc_get_type(ptr, struct nss_cmd_ctx); - struct cli_ctx *cctx = cmdctx->cctx; uint8_t *body; size_t blen; - uint32_t gid; - uint32_t num; - int ret, i; + gid_t gid; + int ret, i, num; - /* create response packet */ - ret = sss_packet_new(cctx->creq, 0, - sss_packet_get_cmd(cctx->creq->in), - &cctx->creq->out); - if (ret != EOK) { - NSS_CMD_FATAL_ERROR(cctx); + if (res->count == 0) { + return ENOENT; } - if (status != LDB_SUCCESS) { - sss_packet_set_error(cctx->creq->out, status); - goto done; - } + /* one less, the first one is the user entry */ + num = res->count -1; - num = res->count; - ret = sss_packet_grow(cctx->creq->out, (2 + num) * sizeof(uint32_t)); + ret = sss_packet_grow(packet, (2 + res->count) * sizeof(uint32_t)); if (ret != EOK) { - sss_packet_set_error(cctx->creq->out, ret); - goto done; + return ret; } - sss_packet_get_body(cctx->creq->out, &body, &blen); + sss_packet_get_body(packet, &body, &blen); + /* skip first entry, it's the user entry */ for (i = 0; i < num; i++) { - gid = ldb_msg_find_attr_as_uint64(res->msgs[i], SYSDB_GIDNUM, 0); + gid = ldb_msg_find_attr_as_uint64(res->msgs[i + 1], SYSDB_GIDNUM, 0); if (!gid) { DEBUG(1, ("Incomplete group object for initgroups! Aborting\n")); - sss_packet_set_error(cctx->creq->out, EIO); - num = 0; - goto done; + return EFAULT; } - ((uint32_t *)body)[2+i] = gid; + ((uint32_t *)body)[2 + i] = gid; } ((uint32_t *)body)[0] = num; /* num results */ ((uint32_t *)body)[1] = 0; /* reserved */ -done: - sss_cmd_done(cctx, cmdctx); -} - -static void nss_cmd_getinitgr_callback(uint16_t err_maj, uint32_t err_min, - const char *err_msg, void *ptr) -{ - struct nss_dom_ctx *dctx = talloc_get_type(ptr, struct nss_dom_ctx); - struct nss_cmd_ctx *cmdctx = dctx->cmdctx; - struct cli_ctx *cctx = cmdctx->cctx; - struct sysdb_ctx *sysdb; - int ret; - - if (err_maj) { - DEBUG(2, ("Unable to get information from Data Provider\n" - "Error: %u, %u, %s\n" - "Will try to return what we have in cache\n", - (unsigned int)err_maj, (unsigned int)err_min, err_msg)); - } - - ret = sysdb_get_ctx_from_list(cctx->rctx->db_list, - dctx->domain, &sysdb); - if (ret != EOK) { - DEBUG(0, ("Fatal: Sysdb CTX not found for this domain!\n")); - NSS_CMD_FATAL_ERROR(cctx); - } - ret = sysdb_initgroups(cmdctx, sysdb, - dctx->domain, cmdctx->name, - nss_cmd_initgr_callback, cmdctx); - if (ret != EOK) { - DEBUG(1, ("Failed to make request to our cache!\n")); - - ret = nss_cmd_send_error(cmdctx, ret); - if (ret != EOK) { - NSS_CMD_FATAL_ERROR(cctx); - } - sss_cmd_done(cctx, cmdctx); - } + return EOK; } -static void nss_cmd_getinit_callback(void *ptr, int status, - struct ldb_result *res); - -static void nss_cmd_getinitnam_dp_callback(uint16_t err_maj, uint32_t err_min, - const char *err_msg, void *ptr) -{ - struct nss_dom_ctx *dctx = talloc_get_type(ptr, struct nss_dom_ctx); - struct nss_cmd_ctx *cmdctx = dctx->cmdctx; - struct cli_ctx *cctx = cmdctx->cctx; - struct sysdb_ctx *sysdb; - int ret; - - if (err_maj) { - DEBUG(2, ("Unable to get information from Data Provider\n" - "Error: %u, %u, %s\n" - "Will try to return what we have in cache\n", - (unsigned int)err_maj, (unsigned int)err_min, err_msg)); - - if (!dctx->res) { - /* return 0 results */ - dctx->res = talloc_zero(dctx, struct ldb_result); - if (!dctx->res) { - ret = ENOMEM; - goto done; - } - } - - nss_cmd_getinit_callback(dctx, LDB_SUCCESS, dctx->res); - return; - } - - ret = sysdb_get_ctx_from_list(cctx->rctx->db_list, - dctx->domain, &sysdb); - if (ret != EOK) { - DEBUG(0, ("Fatal: Sysdb CTX not found for this domain!\n")); - NSS_CMD_FATAL_ERROR(cctx); - } - ret = sysdb_getpwnam(cmdctx, sysdb, - dctx->domain, cmdctx->name, - nss_cmd_getinit_callback, dctx); +static void nss_cmd_getinitgr_dp_callback(uint16_t err_maj, uint32_t err_min, + const char *err_msg, void *ptr); -done: - if (ret != EOK) { - DEBUG(1, ("Failed to make request to our cache!\n")); - - ret = nss_cmd_send_error(cmdctx, ret); - if (ret != EOK) { - NSS_CMD_FATAL_ERROR(cctx); - } - sss_cmd_done(cctx, cmdctx); - } -} - -static void nss_cmd_getinit_callback(void *ptr, int status, - struct ldb_result *res) +static void nss_cmd_getinitgr_callback(void *ptr, int status, + struct ldb_result *res) { struct nss_dom_ctx *dctx = talloc_get_type(ptr, struct nss_dom_ctx); struct nss_cmd_ctx *cmdctx = dctx->cmdctx; @@ -2928,11 +2834,8 @@ static void nss_cmd_getinit_callback(void *ptr, int status, struct sss_domain_info *dom; struct sysdb_ctx *sysdb; struct nss_ctx *nctx; - int timeout; - uint64_t cacheExpire; uint8_t *body; size_t blen; - bool call_provider = false; bool neghit = false; int ncret; int ret; @@ -2949,55 +2852,15 @@ static void nss_cmd_getinit_callback(void *ptr, int status, } if (dctx->check_provider) { - switch (res->count) { - case 0: - call_provider = true; - break; - - case 1: - cacheExpire = ldb_msg_find_attr_as_uint64(res->msgs[0], - SYSDB_CACHE_EXPIRE, 0); - if (cacheExpire < time(NULL)) { - call_provider = true; - } - break; - - default: - DEBUG(1, ("getpwnam call returned more than one result !?!\n")); - ret = nss_cmd_send_error(cmdctx, ENOENT); - if (ret != EOK) { - NSS_CMD_FATAL_ERROR(cctx); - } - sss_cmd_done(cctx, cmdctx); - return; - } - } - - if (call_provider) { - - /* dont loop forever :-) */ - dctx->check_provider = false; - timeout = SSS_CLI_SOCKET_TIMEOUT/2; - - /* keep around current data in case backend is offline */ - if (res->count) { - dctx->res = talloc_steal(dctx, res); - } - - ret = sss_dp_send_acct_req(cctx->rctx, cmdctx, - nss_cmd_getinitnam_dp_callback, dctx, - timeout, dctx->domain->name, SSS_DP_USER, - cmdctx->name, 0); + ret = check_cache(dctx, nctx, res, + SSS_DP_INITGROUPS, cmdctx->name, 0, + nss_cmd_getinitgr_dp_callback); if (ret != EOK) { - DEBUG(3, ("Failed to dispatch request: %d(%s)\n", - ret, strerror(ret))); - ret = nss_cmd_send_error(cmdctx, ret); - if (ret != EOK) { - NSS_CMD_FATAL_ERROR(cctx); - } - sss_cmd_done(cctx, cmdctx); + /* Anything but EOK means we should reenter the mainloop + * because we may be refreshing the cache + */ + return; } - return; } switch (res->count) { @@ -3047,9 +2910,9 @@ static void nss_cmd_getinit_callback(void *ptr, int status, DEBUG(0, ("Fatal: Sysdb CTX not found for this domain!\n")); NSS_CMD_FATAL_ERROR(cctx); } - ret = sysdb_getpwnam(cmdctx, sysdb, - dctx->domain, cmdctx->name, - nss_cmd_getinit_callback, dctx); + ret = sysdb_initgroups(cmdctx, sysdb, + dctx->domain, cmdctx->name, + nss_cmd_getinitgr_callback, dctx); if (ret != EOK) { DEBUG(1, ("Failed to make request to our cache!\n")); } @@ -3081,35 +2944,74 @@ static void nss_cmd_getinit_callback(void *ptr, int status, ((uint32_t *)body)[1] = 0; /* reserved */ break; - case 1: + default: - timeout = SSS_CLI_SOCKET_TIMEOUT/2; - ret = sss_dp_send_acct_req(cctx->rctx, cmdctx, - nss_cmd_getinitgr_callback, dctx, - timeout, dctx->domain->name, - SSS_DP_INITGROUPS, - cmdctx->name, 0); + DEBUG(6, ("Returning initgr for user [%s]\n", cmdctx->name)); + + ret = sss_packet_new(cctx->creq, 0, + sss_packet_get_cmd(cctx->creq->in), + &cctx->creq->out); if (ret != EOK) { - DEBUG(3, ("Failed to dispatch request: %d(%s)\n", - ret, strerror(ret))); - ret = nss_cmd_send_error(cmdctx, ret); - if (ret != EOK) { - NSS_CMD_FATAL_ERROR(cctx); + NSS_CMD_FATAL_ERROR(cctx); + } + ret = fill_initgr(cctx->creq->out, res); + if (ret == ENOENT) { + ret = fill_empty(cctx->creq->out); + } + sss_packet_set_error(cctx->creq->out, ret); + } + + sss_cmd_done(cctx, cmdctx); +} + +static void nss_cmd_getinitgr_dp_callback(uint16_t err_maj, uint32_t err_min, + const char *err_msg, void *ptr) +{ + struct nss_dom_ctx *dctx = talloc_get_type(ptr, struct nss_dom_ctx); + struct nss_cmd_ctx *cmdctx = dctx->cmdctx; + struct cli_ctx *cctx = cmdctx->cctx; + struct sysdb_ctx *sysdb; + int ret; + + if (err_maj) { + DEBUG(2, ("Unable to get information from Data Provider\n" + "Error: %u, %u, %s\n" + "Will try to return what we have in cache\n", + (unsigned int)err_maj, (unsigned int)err_min, err_msg)); + + if (!dctx->res) { + /* return 0 results */ + dctx->res = talloc_zero(dctx, struct ldb_result); + if (!dctx->res) { + ret = ENOMEM; + goto done; } - sss_cmd_done(cctx, cmdctx); } + nss_cmd_getinitgr_callback(dctx, LDB_SUCCESS, dctx->res); return; + } - default: - DEBUG(1, ("getpwnam call returned more than one result !?!\n")); - ret = nss_cmd_send_error(cmdctx, ENOENT); + ret = sysdb_get_ctx_from_list(cctx->rctx->db_list, + dctx->domain, &sysdb); + if (ret != EOK) { + DEBUG(0, ("Fatal: Sysdb CTX not found for this domain!\n")); + NSS_CMD_FATAL_ERROR(cctx); + } + ret = sysdb_initgroups(cmdctx, sysdb, + dctx->domain, cmdctx->name, + nss_cmd_getinitgr_callback, dctx); + +done: + if (ret != EOK) { + DEBUG(1, ("Failed to make request to our cache!\n")); + + ret = nss_cmd_send_error(cmdctx, ret); if (ret != EOK) { NSS_CMD_FATAL_ERROR(cctx); } + sss_cmd_done(cctx, cmdctx); } - - sss_cmd_done(cctx, cmdctx); } /* for now, if we are online, try to always query the backend */ @@ -3227,9 +3129,9 @@ static int nss_cmd_initgroups(struct cli_ctx *cctx) ret = EFAULT; goto done; } - ret = sysdb_getpwnam(cmdctx, sysdb, - dctx->domain, cmdctx->name, - nss_cmd_getinit_callback, dctx); + ret = sysdb_initgroups(cmdctx, sysdb, + dctx->domain, cmdctx->name, + nss_cmd_getinitgr_callback, dctx); if (ret != EOK) { DEBUG(1, ("Failed to make request to our cache!\n")); } -- 1.6.2.5
>From 5e6b2c1905cb151ee689be4a374d4fa81f5ffe21 Mon Sep 17 00:00:00 2001 From: Simo Sorce <[email protected]> Date: Wed, 11 Nov 2009 21:52:46 -0500 Subject: [PATCH 2/3] Change the pam code to perform an initgroups call An initgroups call refreshes both the user and the user's groups, this is ideal for pam so that we don't need addiotnal initgroups calls (initgroups calls are cached too now) during the login process. --- server/responder/pam/pamsrv_cmd.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/server/responder/pam/pamsrv_cmd.c b/server/responder/pam/pamsrv_cmd.c index f4d9c4d..8627d5c 100644 --- a/server/responder/pam/pamsrv_cmd.c +++ b/server/responder/pam/pamsrv_cmd.c @@ -725,7 +725,7 @@ static int pam_forwarder(struct cli_ctx *cctx, int pam_cmd) ret = sss_dp_send_acct_req(preq->cctx->rctx, preq, pam_check_user_dp_callback, preq, - timeout, preq->domain->name, SSS_DP_USER, + timeout, preq->domain->name, SSS_DP_INITGROUPS, preq->pd->user, 0); } else { -- 1.6.2.5
>From 639e9cc012958c959003900ac8578bc5c69895a8 Mon Sep 17 00:00:00 2001 From: Simo Sorce <[email protected]> Date: Wed, 11 Nov 2009 21:28:22 -0500 Subject: [PATCH 3/3] Store initgr expire time on initgr call --- server/providers/ldap/sdap_async_accounts.c | 23 +++++++++++++++++------ 1 files changed, 17 insertions(+), 6 deletions(-) diff --git a/server/providers/ldap/sdap_async_accounts.c b/server/providers/ldap/sdap_async_accounts.c index bf6f852..292c85f 100644 --- a/server/providers/ldap/sdap_async_accounts.c +++ b/server/providers/ldap/sdap_async_accounts.c @@ -46,7 +46,8 @@ static struct tevent_req *sdap_save_user_send(TALLOC_CTX *memctx, struct sysdb_handle *handle, struct sdap_options *opts, struct sss_domain_info *dom, - struct sysdb_attrs *attrs) + struct sysdb_attrs *attrs, + bool is_initgr) { struct tevent_req *req, *subreq; struct sdap_save_user_state *state; @@ -63,6 +64,7 @@ static struct tevent_req *sdap_save_user_send(TALLOC_CTX *memctx, char *upn = NULL; int i; char *val = NULL; + int cache_timeout; DEBUG(9, ("Save user\n")); @@ -253,14 +255,23 @@ static struct tevent_req *sdap_save_user_send(TALLOC_CTX *memctx, } } + cache_timeout = dp_opt_get_int(opts->basic, SDAP_ENTRY_CACHE_TIMEOUT); + + if (is_initgr) { + ret = sysdb_attrs_add_time_t(user_attrs, SYSDB_INITGR_EXPIRE, + (cache_timeout ? + (time(NULL) + cache_timeout) : 0)); + if (ret) { + goto fail; + } + } + DEBUG(6, ("Storing info for user %s\n", state->name)); subreq = sysdb_store_user_send(state, state->ev, state->handle, state->dom, state->name, pwd, uid, gid, gecos, homedir, shell, - user_attrs, - dp_opt_get_int(opts->basic, - SDAP_ENTRY_CACHE_TIMEOUT)); + user_attrs, cache_timeout); if (!subreq) { ret = ENOMEM; goto fail; @@ -393,7 +404,7 @@ static void sdap_save_users_store(struct tevent_req *req) subreq = sdap_save_user_send(state, state->ev, state->handle, state->opts, state->dom, - state->users[state->cur]); + state->users[state->cur], false); if (!subreq) { tevent_req_error(req, ENOMEM); return; @@ -1955,7 +1966,7 @@ static void sdap_get_initgr_store(struct tevent_req *subreq) subreq = sdap_save_user_send(state, state->ev, state->handle, state->opts, state->dom, - state->orig_user); + state->orig_user, true); if (!subreq) { tevent_req_error(req, ENOMEM); return; -- 1.6.2.5
_______________________________________________ sssd-devel mailing list [email protected] https://fedorahosted.org/mailman/listinfo/sssd-devel
