URL: https://github.com/SSSD/sssd/pull/736 Author: jhrozek Title: #736: KCM: Allow representing ccaches with a NULL principal Action: opened
PR body: """ Related: https://pagure.io/SSSD/sssd/issue/3873 We need to make it possible to create an internal ccache representation without passing in a principal. The principal is only assigned to the ccache with krb5_cc_initialize(), but some programs like openssh use the following sequence of calls: krb5_cc_new_unique krb5_cc_switch krb5_cc_initialize """ To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/736/head:pr736 git checkout pr736
From d4f0a4b3cf303f6d2f509d137673108cb0ac24bc Mon Sep 17 00:00:00 2001 From: Jakub Hrozek <jhro...@redhat.com> Date: Wed, 16 Jan 2019 13:06:10 +0100 Subject: [PATCH 1/3] KCM: Return a valid tevent error code if a request cannot be created Previously we were returning whatever was in 'ret' which is wrong, typically it would have been EOK as returned from a previous successfull call or even an uninitialized value. --- src/responder/kcm/kcmsrv_ops.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/responder/kcm/kcmsrv_ops.c b/src/responder/kcm/kcmsrv_ops.c index 9352909f4c..60b5677e93 100644 --- a/src/responder/kcm/kcmsrv_ops.c +++ b/src/responder/kcm/kcmsrv_ops.c @@ -527,7 +527,7 @@ static void kcm_op_initialize_create_step(struct tevent_req *req) state->op_ctx->client, state->new_cc); if (subreq == NULL) { - tevent_req_error(req, ret); + tevent_req_error(req, ENOMEM); return; } tevent_req_set_callback(subreq, kcm_op_initialize_cc_create_done, req); From d8bb375b81cdcdc2db9fca0dc1fdf3baf905022f Mon Sep 17 00:00:00 2001 From: Jakub Hrozek <jhro...@redhat.com> Date: Wed, 16 Jan 2019 13:02:01 +0100 Subject: [PATCH 2/3] KCM: Allow representing ccaches with a NULL principal Related: https://pagure.io/SSSD/sssd/issue/3873 We need to make it possible to create an internal ccache representation without passing in a principal. The principal is only assigned to the ccache with krb5_cc_initialize(), but some programs like openssh use the following sequence of calls: krb5_cc_new_unique krb5_cc_switch krb5_cc_initialize --- src/responder/kcm/kcmsrv_ccache.c | 18 +++-- src/responder/kcm/kcmsrv_ccache_json.c | 79 ++++++++++++++++--- src/tests/cmocka/test_kcm_json_marshalling.c | 83 ++++++++++++++++++-- 3 files changed, 153 insertions(+), 27 deletions(-) diff --git a/src/responder/kcm/kcmsrv_ccache.c b/src/responder/kcm/kcmsrv_ccache.c index af2bcf8bb5..e7800662ac 100644 --- a/src/responder/kcm/kcmsrv_ccache.c +++ b/src/responder/kcm/kcmsrv_ccache.c @@ -68,14 +68,16 @@ errno_t kcm_cc_new(TALLOC_CTX *mem_ctx, uuid_generate(cc->uuid); - kret = krb5_copy_principal(k5c, princ, &cc->client); - if (kret != 0) { - const char *err_msg = sss_krb5_get_error_message(k5c, kret); - DEBUG(SSSDBG_OP_FAILURE, - "krb5_copy_principal failed: [%d][%s]\n", kret, err_msg); - sss_krb5_free_error_message(k5c, err_msg); - ret = ERR_INTERNAL; - goto done; + if (princ) { + kret = krb5_copy_principal(k5c, princ, &cc->client); + if (kret != 0) { + const char *err_msg = sss_krb5_get_error_message(k5c, kret); + DEBUG(SSSDBG_OP_FAILURE, + "krb5_copy_principal failed: [%d][%s]\n", kret, err_msg); + sss_krb5_free_error_message(k5c, err_msg); + ret = ERR_INTERNAL; + goto done; + } } cc->owner.uid = cli_creds_get_uid(owner); diff --git a/src/responder/kcm/kcmsrv_ccache_json.c b/src/responder/kcm/kcmsrv_ccache_json.c index 6341530ee5..72e24c4304 100644 --- a/src/responder/kcm/kcmsrv_ccache_json.c +++ b/src/responder/kcm/kcmsrv_ccache_json.c @@ -229,6 +229,20 @@ static json_t *princ_to_json(TALLOC_CTX *mem_ctx, json_error_t error; char *str_realm_data; + if (princ == NULL) { + jprinc = json_pack_ex(&error, + JSON_STRICT, + "{}"); + if (jprinc == NULL) { + DEBUG(SSSDBG_CRIT_FAILURE, + "Failed to pack JSON princ structure on line %d: %s\n", + error.line, error.text); + return NULL; + } + + return jprinc; + } + components = princ_data_to_json(mem_ctx, princ); if (components == NULL) { DEBUG(SSSDBG_CRIT_FAILURE, @@ -587,13 +601,12 @@ static errno_t json_array_to_krb5_data(TALLOC_CTX *mem_ctx, return EOK; } -static errno_t json_to_princ(TALLOC_CTX *mem_ctx, - json_t *js_princ, - krb5_principal *_princ) +static errno_t json_to_nonempty_princ(TALLOC_CTX *mem_ctx, + json_t *js_princ, + krb5_principal *_princ) { errno_t ret; json_t *components = NULL; - int ok; krb5_principal princ = NULL; TALLOC_CTX *tmp_ctx = NULL; char *realm_str; @@ -601,13 +614,6 @@ static errno_t json_to_princ(TALLOC_CTX *mem_ctx, size_t comp_count; json_error_t error; - ok = json_is_object(js_princ); - if (!ok) { - DEBUG(SSSDBG_CRIT_FAILURE, "Json principal is not an object.\n"); - ret = ERR_JSON_DECODING; - goto done; - } - tmp_ctx = talloc_new(mem_ctx); if (tmp_ctx == NULL) { ret = ENOMEM; @@ -684,6 +690,57 @@ static errno_t json_to_princ(TALLOC_CTX *mem_ctx, return ret; } +static bool is_nonempty_principal(json_t *js_princ) +{ + errno_t ret; + json_error_t error; + + ret = json_unpack_ex(js_princ, + &error, + JSON_VALIDATE_ONLY, + "{s:i, s:s, s:o}", + "type", + "realm", + "components"); + + return ret == 0 ? true : false; +} + +static bool is_empty_principal(json_t *js_princ) +{ + errno_t ret; + json_error_t error; + + ret = json_unpack_ex(js_princ, + &error, + JSON_VALIDATE_ONLY, + "{}"); + + return ret == 0 ? true : false; +} + +static errno_t json_to_princ(TALLOC_CTX *mem_ctx, + json_t *js_princ, + krb5_principal *_princ) +{ + int ok; + + ok = json_is_object(js_princ); + if (!ok) { + DEBUG(SSSDBG_CRIT_FAILURE, "Json principal is not an object.\n"); + return ERR_JSON_DECODING; + } + + if (is_nonempty_principal(js_princ)) { + return json_to_nonempty_princ(mem_ctx, js_princ, _princ); + } else if (is_empty_principal(js_princ)) { + *_princ = NULL; + return EOK; + } + + return ERR_JSON_DECODING; +} + static errno_t json_elem_to_cred(TALLOC_CTX *mem_ctx, json_t *element, struct kcm_cred **_crd) diff --git a/src/tests/cmocka/test_kcm_json_marshalling.c b/src/tests/cmocka/test_kcm_json_marshalling.c index 05d4724993..48ee92bd67 100644 --- a/src/tests/cmocka/test_kcm_json_marshalling.c +++ b/src/tests/cmocka/test_kcm_json_marshalling.c @@ -116,14 +116,22 @@ static void assert_cc_princ_equal(struct kcm_ccache *cc1, p1 = kcm_cc_get_client_principal(cc1); p2 = kcm_cc_get_client_principal(cc2); - kerr = krb5_unparse_name(NULL, p1, &name1); - assert_int_equal(kerr, 0); - kerr = krb5_unparse_name(NULL, p2, &name2); - assert_int_equal(kerr, 0); - - assert_string_equal(name1, name2); - krb5_free_unparsed_name(NULL, name1); - krb5_free_unparsed_name(NULL, name2); + if (p1 != NULL && p2 != NULL) { + kerr = krb5_unparse_name(NULL, p1, &name1); + assert_int_equal(kerr, 0); + kerr = krb5_unparse_name(NULL, p2, &name2); + assert_int_equal(kerr, 0); + + assert_string_equal(name1, name2); + krb5_free_unparsed_name(NULL, name1); + krb5_free_unparsed_name(NULL, name2); + } else { + /* Either both principals must be NULL or both + * non-NULL and represent the same principals + */ + assert_null(p1); + assert_null(p2); + } } static void assert_cc_offset_equal(struct kcm_ccache *cc1, @@ -206,6 +214,62 @@ static void test_kcm_ccache_marshall_unmarshall(void **state) assert_int_equal(ret, EINVAL); } +static void test_kcm_ccache_no_princ(void **state) +{ + struct kcm_marshalling_test_ctx *test_ctx = talloc_get_type(*state, + struct kcm_marshalling_test_ctx); + errno_t ret; + struct cli_creds owner; + const char *name; + struct kcm_ccache *cc; + krb5_principal princ; + struct kcm_ccache *cc2; + struct sss_iobuf *payload; + const char *key; + uint8_t *data; + uuid_t uuid; + + owner.ucred.uid = getuid(); + owner.ucred.gid = getuid(); + + name = talloc_asprintf(test_ctx, "%"SPRIuid, getuid()); + assert_non_null(name); + + ret = kcm_cc_new(test_ctx, + test_ctx->kctx, + &owner, + name, + NULL, + &cc); + assert_int_equal(ret, EOK); + + princ = kcm_cc_get_client_principal(cc); + assert_null(princ); + + ret = kcm_ccache_to_sec_input(test_ctx, + cc, + &owner, + &payload); + assert_int_equal(ret, EOK); + + data = sss_iobuf_get_data(payload); + assert_non_null(data); + + ret = kcm_cc_get_uuid(cc, uuid); + assert_int_equal(ret, EOK); + key = sec_key_create(test_ctx, name, uuid); + assert_non_null(key); + + ret = sec_kv_to_ccache(test_ctx, + key, + (const char *) data, + &owner, + &cc2); + assert_int_equal(ret, EOK); + + assert_cc_equal(cc, cc2); +} + void test_sec_key_get_uuid(void **state) { errno_t ret; @@ -279,6 +343,9 @@ int main(int argc, const char *argv[]) cmocka_unit_test_setup_teardown(test_kcm_ccache_marshall_unmarshall, setup_kcm_marshalling, teardown_kcm_marshalling), + cmocka_unit_test_setup_teardown(test_kcm_ccache_no_princ, + setup_kcm_marshalling, + teardown_kcm_marshalling), cmocka_unit_test(test_sec_key_get_uuid), cmocka_unit_test(test_sec_key_get_name), cmocka_unit_test(test_sec_key_match_name), From 527d796e6c26038581ddf19375cbe1eb46775280 Mon Sep 17 00:00:00 2001 From: Jakub Hrozek <jhro...@redhat.com> Date: Wed, 16 Jan 2019 13:06:15 +0100 Subject: [PATCH 3/3] KCM: Create an empty ccache on switch to a non-existing one Related: https://pagure.io/SSSD/sssd/issue/3873 We need to make it possible to create an internal ccache representation without passing in a principal. The principal is only assigned to the ccache with krb5_cc_initialize(), but some programs like openssh use the following sequence of calls: cc = krb5_cc_new_unique krb5_cc_switch(cc) krb5_cc_initialize(cc, principal) Since switch changes the default ccache, we create a 'dummy' ccache with krb5_cc_switch() and then the initialize call just fills in the details. --- src/responder/kcm/kcmsrv_ops.c | 133 +++++++++++++++++++++++++++++---- 1 file changed, 118 insertions(+), 15 deletions(-) diff --git a/src/responder/kcm/kcmsrv_ops.c b/src/responder/kcm/kcmsrv_ops.c index 60b5677e93..6544c4ed0a 100644 --- a/src/responder/kcm/kcmsrv_ops.c +++ b/src/responder/kcm/kcmsrv_ops.c @@ -1601,8 +1601,21 @@ static errno_t kcm_op_get_default_ccache_recv(struct tevent_req *req, /* (name) -> () */ static void kcm_op_set_default_ccache_getbyname_done(struct tevent_req *subreq); +static void kcm_op_set_default_create_step(struct tevent_req *req); +static void kcm_op_set_default_create_step_done(struct tevent_req *subreq); +static void kcm_op_set_default_step(struct tevent_req *req); static void kcm_op_set_default_done(struct tevent_req *subreq); +struct kcm_op_set_default_ccache_state { + uint32_t op_ret; + struct kcm_op_ctx *op_ctx; + struct tevent_context *ev; + + const char *name; + uuid_t dfl_uuid; + struct kcm_ccache *new_cc; +}; + static struct tevent_req * kcm_op_set_default_ccache_send(TALLOC_CTX *mem_ctx, struct tevent_context *ev, @@ -1610,30 +1623,31 @@ kcm_op_set_default_ccache_send(TALLOC_CTX *mem_ctx, { struct tevent_req *req = NULL; struct tevent_req *subreq = NULL; - struct kcm_op_common_state *state = NULL; + struct kcm_op_set_default_ccache_state *state = NULL; errno_t ret; - const char *name; - req = tevent_req_create(mem_ctx, &state, struct kcm_op_common_state); + req = tevent_req_create(mem_ctx, + &state, + struct kcm_op_set_default_ccache_state); if (req == NULL) { return NULL; } state->op_ctx = op_ctx; state->ev = ev; - ret = sss_iobuf_read_stringz(op_ctx->input, &name); + ret = sss_iobuf_read_stringz(op_ctx->input, &state->name); if (ret != EOK) { DEBUG(SSSDBG_CRIT_FAILURE, "Cannot read input name [%d]: %s\n", ret, sss_strerror(ret)); goto immediate; } - DEBUG(SSSDBG_TRACE_LIBS, "Setting default ccache %s\n", name); + DEBUG(SSSDBG_TRACE_LIBS, "Setting default ccache %s\n", state->name); subreq = kcm_ccdb_uuid_by_name_send(state, ev, op_ctx->kcm_data->db, op_ctx->client, - name); + state->name); if (subreq == NULL) { ret = ENOMEM; goto immediate; @@ -1652,13 +1666,16 @@ static void kcm_op_set_default_ccache_getbyname_done(struct tevent_req *subreq) errno_t ret; struct tevent_req *req = tevent_req_callback_data(subreq, struct tevent_req); - struct kcm_op_common_state *state = tevent_req_data(req, - struct kcm_op_common_state); - uuid_t dfl_uuid; + struct kcm_op_set_default_ccache_state *state = tevent_req_data(req, + struct kcm_op_set_default_ccache_state); - ret = kcm_ccdb_uuid_by_name_recv(subreq, state, dfl_uuid); + ret = kcm_ccdb_uuid_by_name_recv(subreq, state, state->dfl_uuid); talloc_zfree(subreq); - if (ret != EOK) { + if (ret == ERR_NO_CREDS) { + DEBUG(SSSDBG_TRACE_LIBS, + "The ccache does not exist, creating a new one\n"); + kcm_op_set_default_create_step(req); + } else if (ret != EOK) { DEBUG(SSSDBG_OP_FAILURE, "Cannot get ccache by name [%d]: %s\n", ret, sss_strerror(ret)); @@ -1666,11 +1683,91 @@ static void kcm_op_set_default_ccache_getbyname_done(struct tevent_req *subreq) return; } + kcm_op_set_default_step(req); +} + +static void kcm_op_set_default_create_step(struct tevent_req *req) +{ + errno_t ret; + struct tevent_req *subreq; + struct kcm_op_set_default_ccache_state *state = tevent_req_data(req, + struct kcm_op_set_default_ccache_state); + + /* Only allow to create ccaches for 'self' */ + ret = kcm_check_name(state->name, state->op_ctx->client); + if (ret != EOK) { + DEBUG(SSSDBG_CRIT_FAILURE, + "Name %s is malformed [%d]: %s\n", + state->name, ret, sss_strerror(ret)); + tevent_req_error(req, ret); + return; + } + + ret = kcm_cc_new(state->op_ctx, + state->op_ctx->kcm_data->k5c, + state->op_ctx->client, + state->name, + NULL, + &state->new_cc); + if (ret != EOK) { + DEBUG(SSSDBG_OP_FAILURE, + "Cannot create new ccache %d: %s\n", ret, sss_strerror(ret)); + tevent_req_error(req, ret); + return; + } + + subreq = kcm_ccdb_create_cc_send(state, + state->ev, + state->op_ctx->kcm_data->db, + state->op_ctx->client, + state->new_cc); + if (subreq == NULL) { + tevent_req_error(req, ENOMEM); + return; + } + tevent_req_set_callback(subreq, kcm_op_set_default_create_step_done, req); +} + +static void kcm_op_set_default_create_step_done(struct tevent_req *subreq) +{ + errno_t ret; + struct tevent_req *req = tevent_req_callback_data(subreq, + struct tevent_req); + struct kcm_op_set_default_ccache_state *state = tevent_req_data(req, + struct kcm_op_set_default_ccache_state); + + ret = kcm_ccdb_create_cc_recv(subreq); + talloc_zfree(subreq); + if (ret != EOK) { + DEBUG(SSSDBG_OP_FAILURE, + "Cannot add ccache to db %d: %s\n", ret, sss_strerror(ret)); + tevent_req_error(req, ret); + return; + } + + ret = kcm_cc_get_uuid(state->new_cc, state->dfl_uuid); + if (ret != EOK) { + DEBUG(SSSDBG_OP_FAILURE, + "Cannot get new ccache UUID [%d]: %s\n", + ret, sss_strerror(ret)); + tevent_req_error(req, ret); + return; + } + + kcm_op_set_default_step(req); +} + +static void kcm_op_set_default_step(struct tevent_req *req) +{ + struct kcm_op_set_default_ccache_state *state = tevent_req_data(req, + struct kcm_op_set_default_ccache_state); + struct tevent_req *subreq; + subreq = kcm_ccdb_set_default_send(state, state->ev, state->op_ctx->kcm_data->db, state->op_ctx->client, - dfl_uuid); + state->dfl_uuid); if (subreq == NULL) { tevent_req_error(req, ENOMEM); return; @@ -1684,8 +1781,8 @@ static void kcm_op_set_default_done(struct tevent_req *subreq) errno_t ret; struct tevent_req *req = tevent_req_callback_data(subreq, struct tevent_req); - struct kcm_op_common_state *state = tevent_req_data(req, - struct kcm_op_common_state); + struct kcm_op_set_default_ccache_state *state = tevent_req_data(req, + struct kcm_op_set_default_ccache_state); ret = kcm_ccdb_set_default_recv(subreq); talloc_zfree(subreq); @@ -1700,6 +1797,12 @@ static void kcm_op_set_default_done(struct tevent_req *subreq) tevent_req_done(req); } +static errno_t kcm_op_set_default_ccache_recv(struct tevent_req *req, + uint32_t *_op_ret) +{ + KCM_OP_RET_FROM_TYPE(req, struct kcm_op_set_default_ccache_state, _op_ret); +} + /* (name) -> (offset) */ static void kcm_op_get_kdc_offset_getbyname_done(struct tevent_req *subreq); @@ -1948,7 +2051,7 @@ static struct kcm_op kcm_optable[] = { { "GET_CACHE_UUID_LIST", kcm_op_get_cache_uuid_list_send, NULL }, { "GET_CACHE_BY_UUID", kcm_op_get_cache_by_uuid_send, NULL }, { "GET_DEFAULT_CACHE", kcm_op_get_default_ccache_send, kcm_op_get_default_ccache_recv }, - { "SET_DEFAULT_CACHE", kcm_op_set_default_ccache_send, NULL }, + { "SET_DEFAULT_CACHE", kcm_op_set_default_ccache_send, kcm_op_set_default_ccache_recv }, { "GET_KDC_OFFSET", kcm_op_get_kdc_offset_send, NULL }, { "SET_KDC_OFFSET", kcm_op_set_kdc_offset_send, kcm_op_set_kdc_offset_recv }, { "ADD_NTLM_CRED", NULL, NULL },
_______________________________________________ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org