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
