From 6945fb46a2367b21fe19f7ea938ad9f09776e5d8 Mon Sep 17 00:00:00 2001 From: Sumit Bose Date: Mon, 13 Dec 2010 22:36:05 +0100 Subject: [PATCH 2/2] Fixes for automatic ticket renewal - do not recreate the ccache file when renewing the TGT - use username as hash key instead of ccfile name - let krb5_child return Kerberos error codes --- src/providers/krb5/krb5_auth.c | 45 ++++++++++++-------- src/providers/krb5/krb5_auth.h | 3 +- src/providers/krb5/krb5_child.c | 17 +++++++- src/providers/krb5/krb5_renew_tgt.c | 77 +++++++++++++++++++++++++---------- 4 files changed, 99 insertions(+), 43 deletions(-) diff --git a/src/providers/krb5/krb5_auth.c b/src/providers/krb5/krb5_auth.c index e6b680e..a0ac0e9 100644 --- a/src/providers/krb5/krb5_auth.c +++ b/src/providers/krb5/krb5_auth.c @@ -161,6 +161,8 @@ static int krb5_save_ccname(TALLOC_CTX *mem_ctx, return EINVAL; } + DEBUG(9, ("Save ccname [%s] for user [%s].\n", ccname, name)); + tmpctx = talloc_new(mem_ctx); if (!tmpctx) { return ENOMEM; @@ -349,8 +351,10 @@ struct tevent_req *krb5_auth_send(TALLOC_CTX *mem_ctx, } if (be_is_offline(be_ctx) && - (pd->cmd == SSS_PAM_CHAUTHTOK || pd->cmd == SSS_PAM_CHAUTHTOK_PRELIM)) { - DEBUG(9, ("Password changes are not possible while offline.\n")); + (pd->cmd == SSS_PAM_CHAUTHTOK || pd->cmd == SSS_PAM_CHAUTHTOK_PRELIM || + pd->cmd == SSS_CMD_RENEW)) { + DEBUG(9, ("Password changes and ticket renewal are not possible " + "while offline.\n")); state->pam_status = PAM_AUTHINFO_UNAVAIL; state->dp_err = DP_ERR_OFFLINE; ret = EOK; @@ -582,8 +586,10 @@ static void krb5_find_ccache_step(struct tevent_req *req) * is true: * - it doesn't exist (kr->ccname == NULL) * - the backend is online and the current ccache file is not used, i.e - * the related user is currently not logged in - * (!kr->is_offline && !kr->active_ccache_present) + * the related user is currently not logged in and it is not a renewal + * request + * (!kr->is_offline && !kr->active_ccache_present && + * pd->cmd != SSS_CMD_RENEW) * - the backend is offline and the current cache file not used and * it does not contain a valid tgt * (kr->is_offline && @@ -592,7 +598,8 @@ static void krb5_find_ccache_step(struct tevent_req *req) if (kr->ccname == NULL || (kr->is_offline && !kr->active_ccache_present && !kr->valid_tgt_present) || - (!kr->is_offline && !kr->active_ccache_present)) { + (!kr->is_offline && !kr->active_ccache_present && + pd->cmd != SSS_CMD_RENEW)) { DEBUG(9, ("Recreating ccache file.\n")); kr->ccname = expand_ccname_template(kr, kr, dp_opt_get_cstring(kr->krb5_ctx->opts, @@ -790,19 +797,6 @@ static void krb5_child_done(struct tevent_req *subreq) } } - if (msg_status == PAM_SUCCESS && - dp_opt_get_int(kr->krb5_ctx->opts, KRB5_RENEW_INTERVAL) > 0 && - (pd->cmd == SSS_PAM_AUTHENTICATE || pd->cmd == SSS_CMD_RENEW || - pd->cmd == SSS_PAM_CHAUTHTOK) && - tgtt.renew_till > tgtt.endtime && kr->ccname != NULL) { - DEBUG(7, ("Adding [%s] for automatic renewal.\n", kr->ccname)); - ret = add_tgt_to_renew_table(kr->krb5_ctx, kr->ccname, &tgtt, pd); - if (ret != EOK) { - DEBUG(1, ("add_tgt_to_renew_table failed, " - "automatic renewal not possible.\n")); - } - } - /* If the child request failed, but did not return an offline error code, * return with the status */ if (msg_status != PAM_SUCCESS && msg_status != PAM_AUTHINFO_UNAVAIL && @@ -889,7 +883,22 @@ static void krb5_child_done(struct tevent_req *subreq) goto done; } + if (msg_status == PAM_SUCCESS && + dp_opt_get_int(kr->krb5_ctx->opts, KRB5_RENEW_INTERVAL) > 0 && + (pd->cmd == SSS_PAM_AUTHENTICATE || pd->cmd == SSS_CMD_RENEW || + pd->cmd == SSS_PAM_CHAUTHTOK) && + tgtt.renew_till > tgtt.endtime && kr->ccname != NULL) { + DEBUG(7, ("Adding [%s] for automatic renewal.\n", kr->ccname)); + ret = add_tgt_to_renew_table(kr->krb5_ctx, kr->ccname, &tgtt, pd, + kr->upn); + if (ret != EOK) { + DEBUG(1, ("add_tgt_to_renew_table failed, " + "automatic renewal not possible.\n")); + } + } + krb5_save_ccname_done(req); + return; done: diff --git a/src/providers/krb5/krb5_auth.h b/src/providers/krb5/krb5_auth.h index f40cbcb..2f08ad9 100644 --- a/src/providers/krb5/krb5_auth.h +++ b/src/providers/krb5/krb5_auth.h @@ -83,7 +83,8 @@ errno_t init_delayed_online_authentication(struct krb5_ctx *krb5_ctx, errno_t init_renew_tgt(struct krb5_ctx *krb5_ctx, struct be_ctx *be_ctx, struct tevent_context *ev, time_t renew_intv); errno_t add_tgt_to_renew_table(struct krb5_ctx *krb5_ctx, const char *ccfile, - struct tgt_times *tgtt, struct pam_data *pd); + struct tgt_times *tgtt, struct pam_data *pd, + const char *upn); /* krb5_access.c */ struct tevent_req *krb5_access_send(TALLOC_CTX *mem_ctx, diff --git a/src/providers/krb5/krb5_child.c b/src/providers/krb5/krb5_child.c index b973c13..335da42 100644 --- a/src/providers/krb5/krb5_child.c +++ b/src/providers/krb5/krb5_child.c @@ -1046,12 +1046,14 @@ static errno_t renew_tgt_child(int fd, struct krb5_req *kr) if (kr->pd->authtok_type != SSS_AUTHTOK_TYPE_CCFILE) { DEBUG(1, ("Unsupported authtok type for TGT renewal [%d].\n", kr->pd->authtok_type)); + kerr = EINVAL; goto done; } ccname = talloc_strndup(kr, (char *) kr->pd->authtok, kr->pd->authtok_size); if (ccname == NULL) { DEBUG(1, ("talloc_strndup failed.\n")); + kerr = ENOMEM; goto done; } @@ -1064,6 +1066,9 @@ static errno_t renew_tgt_child(int fd, struct krb5_req *kr) kerr = krb5_get_renewed_creds(kr->ctx, kr->creds, kr->princ, ccache, NULL); if (kerr != 0) { KRB5_DEBUG(1, kerr); + if (kerr == KRB5_KDC_UNREACH) { + status = PAM_AUTHINFO_UNAVAIL; + } goto done; } @@ -1085,6 +1090,7 @@ static errno_t renew_tgt_child(int fd, struct krb5_req *kr) ret = become_user(kr->uid, kr->gid); if (ret != EOK) { DEBUG(1, ("become_user failed.\n")); + kerr = ret; goto done; } } @@ -1107,6 +1113,7 @@ static errno_t renew_tgt_child(int fd, struct krb5_req *kr) } status = PAM_SUCCESS; + kerr = 0; done: krb5_free_cred_contents(kr->ctx, kr->creds); @@ -1115,7 +1122,7 @@ done: krb5_cc_close(kr->ctx, ccache); } - ret = sendresponse(fd, 0, status, kr); + ret = sendresponse(fd, kerr, status, kr); if (ret != EOK) { DEBUG(1, ("sendresponse failed.\n")); } @@ -1424,7 +1431,13 @@ static int krb5_child_setup(struct krb5_req *kr, uint32_t offline) kr->child_req = kuserok_child; break; case SSS_CMD_RENEW: - kr->child_req = renew_tgt_child; + if (!offline) { + kr->child_req = renew_tgt_child; + } else { + DEBUG(1, ("Cannot renew TGT while offline.\n")); + kerr = KRB5_KDC_UNREACH; + goto failed; + } break; default: DEBUG(1, ("PAM command [%d] not supported.\n", kr->pd->cmd)); diff --git a/src/providers/krb5/krb5_renew_tgt.c b/src/providers/krb5/krb5_renew_tgt.c index be029fd..988cea4 100644 --- a/src/providers/krb5/krb5_renew_tgt.c +++ b/src/providers/krb5/krb5_renew_tgt.c @@ -40,6 +40,7 @@ struct renew_tgt_ctx { }; struct renew_data { + const char *ccfile; time_t start_time; time_t lifetime; time_t start_renew_at; @@ -50,6 +51,7 @@ struct auth_data { struct be_ctx *be_ctx; struct krb5_ctx *krb5_ctx; struct pam_data *pd; + struct renew_data *renew_data; hash_table_t *table; hash_key_t key; }; @@ -86,6 +88,10 @@ static void renew_tgt_done(struct tevent_req *req) talloc_free(req); if (ret) { DEBUG(1, ("krb5_auth request failed.\n")); + if (auth_data->renew_data != NULL) { + DEBUG(5, ("Giving back pam data.\n")); + talloc_steal(auth_data->renew_data, auth_data->pd); + } } else { switch (pam_status) { case PAM_SUCCESS: @@ -97,6 +103,10 @@ static void renew_tgt_done(struct tevent_req *req) DEBUG(4, ("Cannot renewed TGT for user [%s] while offline, " "will retry later.\n", auth_data->pd->user)); + if (auth_data->renew_data != NULL) { + DEBUG(5, ("Giving back pam data.\n")); + talloc_steal(auth_data->renew_data, auth_data->pd); + } break; default: DEBUG(1, ("Failed to renew TGT for user [%s].\n", @@ -132,17 +142,18 @@ static errno_t renew_all_tgts(struct renew_tgt_ctx *renew_tgt_ctx) for (c = 0; c < count; c++) { renew_data = talloc_get_type(entries[c].value.ptr, struct renew_data); - DEBUG(9, ("Checking [%s] for renewal at [%.24s].\n", entries[c].key.str, + DEBUG(9, ("Checking [%s] for renewal at [%.24s].\n", renew_data->ccfile, ctime(&renew_data->start_renew_at))); if (renew_data->start_renew_at < now) { auth_data = talloc_zero(renew_tgt_ctx, struct auth_data); if (auth_data == NULL) { DEBUG(1, ("talloc_zero failed.\n")); } else { - auth_data->pd = renew_data->pd; + auth_data->pd = talloc_steal(auth_data, renew_data->pd); auth_data->krb5_ctx = renew_tgt_ctx->krb5_ctx; auth_data->be_ctx = renew_tgt_ctx->be_ctx; auth_data->table = renew_tgt_ctx->tgt_table; + auth_data->renew_data = renew_data; auth_data->key.type = entries[c].key.type; auth_data->key.str = talloc_strdup(auth_data, entries[c].key.str); @@ -160,7 +171,7 @@ static errno_t renew_all_tgts(struct renew_tgt_ctx *renew_tgt_ctx) } if (auth_data == NULL || te == NULL) { - DEBUG(1, ("Failed to renew TGT in [%s].\n", entries[c].key.str)); + DEBUG(1, ("Failed to renew TGT in [%s].\n", renew_data->ccfile)); ret = hash_delete(renew_tgt_ctx->tgt_table, &entries[c].key); if (ret != HASH_SUCCESS) { DEBUG(1, ("hash_delete failed.\n")); @@ -242,6 +253,19 @@ static void renew_handler(struct renew_tgt_ctx *renew_tgt_ctx) return; } +static void renew_del_cb(hash_entry_t *entry, hash_destroy_enum type, void *pvt) +{ + struct renew_data *renew_data; + + if (entry->value.type == HASH_VALUE_PTR) { + renew_data = talloc_get_type(entry->value.ptr, struct renew_data); + talloc_zfree(renew_data); + return; + } + + DEBUG(1, ("Unexpected value type [%d].\n", entry->value.type)); +} + errno_t init_renew_tgt(struct krb5_ctx *krb5_ctx, struct be_ctx *be_ctx, struct tevent_context *ev, time_t renew_intv) { @@ -255,7 +279,8 @@ errno_t init_renew_tgt(struct krb5_ctx *krb5_ctx, struct be_ctx *be_ctx, } ret = sss_hash_create(krb5_ctx->renew_tgt_ctx, INITIAL_TGT_TABLE_SIZE, - &krb5_ctx->renew_tgt_ctx->tgt_table); + &krb5_ctx->renew_tgt_ctx->tgt_table, renew_del_cb, + NULL); if (ret != EOK) { DEBUG(1, ("sss_hash_create failed.\n")); goto fail; @@ -287,9 +312,9 @@ fail: } errno_t add_tgt_to_renew_table(struct krb5_ctx *krb5_ctx, const char *ccfile, - struct tgt_times *tgtt, struct pam_data *pd) + struct tgt_times *tgtt, struct pam_data *pd, + const char *upn) { - char *key_str = NULL; int ret; hash_key_t key; hash_value_t value; @@ -307,26 +332,34 @@ errno_t add_tgt_to_renew_table(struct krb5_ctx *krb5_ctx, const char *ccfile, return EINVAL; } - key.type = HASH_KEY_STRING; - if (ccfile[0] == '/') { - key_str = talloc_asprintf(NULL, "FILE:%s", ccfile); - if (key_str == NULL) { - DEBUG(1, ("talloc_asprintf doneed.\n")); - ret = ENOMEM; - goto done; - } - } else { - key_str = talloc_strdup(NULL, ccfile); + if (upn == NULL) { + DEBUG(1, ("Missing user principal name.\n")); + return EINVAL; } - key.str = key_str; + + /* hash_enter copies the content of the hash string, so it is safe to use + * discard_const_p here. */ + key.type = HASH_KEY_STRING; + key.str = discard_const_p(char, upn); renew_data = talloc_zero(krb5_ctx->renew_tgt_ctx, struct renew_data); if (renew_data == NULL) { - DEBUG(1, ("talloc_zero doneed.\n")); + DEBUG(1, ("talloc_zero failed.\n")); ret = ENOMEM; goto done; } + if (ccfile[0] == '/') { + renew_data->ccfile = talloc_asprintf(renew_data, "FILE:%s", ccfile); + if (renew_data->ccfile == NULL) { + DEBUG(1, ("talloc_asprintf failed.\n")); + ret = ENOMEM; + goto done; + } + } else { + renew_data->ccfile = talloc_strdup(renew_data, ccfile); + } + renew_data->start_time = tgtt->starttime; renew_data->lifetime = tgtt->endtime; renew_data->start_renew_at = (time_t) (tgtt->starttime + @@ -334,7 +367,7 @@ errno_t add_tgt_to_renew_table(struct krb5_ctx *krb5_ctx, const char *ccfile, ret = copy_pam_data(renew_data, pd, &renew_data->pd); if (ret != EOK) { - DEBUG(1, ("copy_pam_data doneed.\n")); + DEBUG(1, ("copy_pam_data failed.\n")); goto done; } @@ -345,7 +378,8 @@ errno_t add_tgt_to_renew_table(struct krb5_ctx *krb5_ctx, const char *ccfile, } talloc_zfree(renew_data->pd->authtok); - renew_data->pd->authtok = (uint8_t *) talloc_strdup(renew_data->pd, key.str); + renew_data->pd->authtok = (uint8_t *) talloc_strdup(renew_data->pd, + renew_data->ccfile); if (renew_data->pd->authtok == NULL) { DEBUG(1, ("talloc_strdup failed.\n")); ret = ENOMEM; @@ -366,13 +400,12 @@ errno_t add_tgt_to_renew_table(struct krb5_ctx *krb5_ctx, const char *ccfile, goto done; } - DEBUG(7, ("Added [%s] for renewal at [%.24s].\n", key_str, + DEBUG(7, ("Added [%s] for renewal at [%.24s].\n", renew_data->ccfile, ctime(&renew_data->start_renew_at))); ret = EOK; done: - talloc_free(key_str); if (ret != EOK) { talloc_free(renew_data); } -- 1.7.3.2