diff --git a/SOURCES/0001-SYSDB-merge_res_sysdb_attrs-fixed-to-avoid-NULL-ptr-.patch b/SOURCES/0001-SYSDB-merge_res_sysdb_attrs-fixed-to-avoid-NULL-ptr-.patch new file mode 100644 index 0000000..bc47f70 --- /dev/null +++ b/SOURCES/0001-SYSDB-merge_res_sysdb_attrs-fixed-to-avoid-NULL-ptr-.patch @@ -0,0 +1,64 @@ +From ff24d1538af88f83d0a3cc2817952cf70e7ca580 Mon Sep 17 00:00:00 2001 +From: Alexey Tikhonov +Date: Sun, 22 Nov 2020 17:44:07 +0100 +Subject: [PATCH] SYSDB: merge_res_sysdb_attrs() fixed to avoid NULL ptr in + msgs[] +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +This helps to avoid sssd_be segfaults at be_refresh_get_values_ex() due to NULL +ptrs in results of sysdb_search_with_ts_attr() + +Resolves: https://github.com/SSSD/sssd/issues/5412 + +Reviewed-by: Pavel Březina +--- + src/db/sysdb_search.c | 11 +++++++---- + 1 file changed, 7 insertions(+), 4 deletions(-) + +diff --git a/src/db/sysdb_search.c b/src/db/sysdb_search.c +index e616fd5bc..4ff65c1ae 100644 +--- a/src/db/sysdb_search.c ++++ b/src/db/sysdb_search.c +@@ -221,6 +221,7 @@ static errno_t merge_res_sysdb_attrs(TALLOC_CTX *mem_ctx, + const char *attrs[]) + { + errno_t ret; ++ size_t ts_cache_res_count = 0; + struct ldb_result *ts_cache_res = NULL; + + if (ts_res == NULL || ctx->ldb_ts == NULL) { +@@ -231,7 +232,6 @@ static errno_t merge_res_sysdb_attrs(TALLOC_CTX *mem_ctx, + if (ts_cache_res == NULL) { + return ENOMEM; + } +- ts_cache_res->count = ts_res->count; + ts_cache_res->msgs = talloc_zero_array(ts_cache_res, + struct ldb_message *, + ts_res->count); +@@ -244,15 +244,18 @@ static errno_t merge_res_sysdb_attrs(TALLOC_CTX *mem_ctx, + ret = merge_msg_sysdb_attrs(ts_cache_res->msgs, + ctx, + ts_res->msgs[c], +- &ts_cache_res->msgs[c], attrs); +- if (ret != EOK) { ++ &ts_cache_res->msgs[ts_cache_res_count], ++ attrs); ++ if ((ret != EOK) || (ts_cache_res->msgs[ts_cache_res_count] == NULL)) { + DEBUG(SSSDBG_MINOR_FAILURE, + "Cannot merge sysdb cache values for %s\n", + ldb_dn_get_linearized(ts_res->msgs[c]->dn)); +- /* non-fatal, we just get only the non-timestamp attrs */ ++ /* non-fatal, just skip */ + continue; + } ++ ts_cache_res_count += 1; + } ++ ts_cache_res->count = ts_cache_res_count; + + *_ts_cache_res = ts_cache_res; + return EOK; +-- +2.21.3 + diff --git a/SOURCES/0002-KCM-perf-improvements.patch b/SOURCES/0002-KCM-perf-improvements.patch new file mode 100644 index 0000000..3734ebe --- /dev/null +++ b/SOURCES/0002-KCM-perf-improvements.patch @@ -0,0 +1,3226 @@ +From 19c0cfe38670cc56219f0d9acdc2b3363e92616c Mon Sep 17 00:00:00 2001 +From: Alexey Tikhonov +Date: Fri, 4 Dec 2020 12:09:57 +0100 +Subject: [PATCH] Squashed commit of the following: +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +commit 325de5a5bb97ba026be6d22492bea8ab2605f1b5 +Author: Pavel Březina +Date: Thu Nov 26 12:07:06 2020 +0100 + + secrets: remove base64 enctype + + This was added as part of KCM performance improvements but never used. + Ldb is fully capable of holding binary data without the need for base64 + encoding so this is not needed. + + Reviewed-by: Alexey Tikhonov + +commit 39277cdadd317b0ab86cdd37de0616bc3eecbe6a +Author: Pavel Březina +Date: Thu Nov 26 11:55:39 2020 +0100 + + secrets: move attrs names to macros + + Reviewed-by: Alexey Tikhonov + +commit 9c1b51d057390fb5b26151f814a480911cda4cc9 +Author: Pavel Březina +Date: Thu Nov 26 11:47:24 2020 +0100 + + secrets: default to "plaintext" if "enctype" attr is missing + + This is a sane fallback behavior, however it should not happen since + the attribute should be always present. + + Reviewed-by: Alexey Tikhonov + +commit bf127d4f3f42e5b2afe25e512211439bc12a9904 +Author: Pavel Březina +Date: Tue Nov 3 13:35:33 2020 +0100 + + secrets: fix may_payload_size exceeded debug message + + The unit is bytes (B) not bits (b) and the conversion of the input + payload size to KiB was wrong (multiplying bytes * 1024). + + Reviewed-by: Alexey Tikhonov + +commit c3b314db57c34f64aaca7d74e76a9a955288bb51 +Author: Pavel Březina +Date: Mon Oct 19 12:40:07 2020 +0200 + + kcm: store credentials list in hash table to avoid cache lookups + + Iteration over ccache requires CRED_UUID_LIST and then calling + CRED_BY_UUID for each uuid in the obtained list. Each CRED_BY_UUID + operation invoked ldb_search and decryption. This was a substantional + bottle neck. + + Resolves: https://github.com/SSSD/sssd/issues/5349 + + :fixes: KCM performance has improved dramatically for cases where + large amount of credentials are stored in the ccache. + + Reviewed-by: Alexey Tikhonov + +commit a370553c90c2ed6df3b94c169c4960a6f978031f +Author: Pavel Březina +Date: Thu Oct 29 14:57:53 2020 +0100 + + sss_ptr_hash: fix double free for circular dependencies + + If the hash table delete callback deletes the stored item, + we can end up in double free in case when we try to override + an existing item (hash_enter(key) where key already exists). + + ```c + static void delete_cb(hash_entry_t *item, + hash_destroy_enum deltype, + void *pvt) + { + talloc_free(item->value.ptr); + } + + hash_enter(key); + hash_enter(key); + ``` + + The doble free it self is fine, since it is done via talloc destructor + and talloc can cope with that. However, the hash table fails to store + the new entry because hash_delete is called twice. + + ``` + _sss_ptr_hash_add -> hash_enter -> hash_delete(old) -> delete_cb -> sss_ptr_hash_value_destructor -> hash_delete + ``` + + Reviewed-by: Alexey Tikhonov + +commit 241ee30da12f564803793ee2b14c1522aabd9235 +Author: Pavel Březina +Date: Fri Oct 16 15:36:51 2020 +0200 + + kcm: add per-connection data to be shared between requests + + Resolves: https://github.com/SSSD/sssd/issues/5349 + + Reviewed-by: Alexey Tikhonov + +commit 194447d35c11eb914f54719491dc5cfaab01b9a1 +Author: Pavel Březina +Date: Tue Oct 27 16:21:31 2020 +0100 + + kcm: use binary format to store ccache instead of json + + JSON is computationally complex and the parser is a bottleneck which + consumes about 10% of time. It also create the ccache unnecessary + large because it requires lots of unneded character and base64 + encoding. + + Binary format is fast, simple and small. + + This is backwards compatible and there is no need to destroy existing + ccache. It will be stored in binary format at first write to the cache. + + Resolves: https://github.com/SSSD/sssd/issues/5349 + + Reviewed-by: Alexey Tikhonov + +commit f17740d831e16449495fff4ec57cc4800aaac83d +Author: Pavel Březina +Date: Tue Oct 27 17:09:43 2020 +0100 + + kcm: add spaces around operators in kcmsrv_ccache_key.c + + Reviewed-by: Alexey Tikhonov + +commit 15069a647ed6c7f1ead42baa1d421d953c9bc557 +Author: Pavel Březina +Date: Tue Oct 27 16:37:05 2020 +0100 + + kcm: avoid suppression of cppcheck warning + + Reviewed-by: Alexey Tikhonov + +commit e63a15038ac9c186626e4fdf681a6492031d1e40 +Author: Pavel Březina +Date: Tue Oct 27 16:18:11 2020 +0100 + + kcm: move sec key parser to separate file so it can be shared + + Reviewed-by: Alexey Tikhonov + +commit 9b1631defdcaa3ea7e87889eb136e7fa935ab4ce +Author: Pavel Březina +Date: Thu Oct 22 13:34:52 2020 +0200 + + kcm: add json suffix to existing searialization functions + + Reviewed-by: Alexey Tikhonov + +commit b6cc661b9f4162e590137430e945aa321fc13121 +Author: Pavel Březina +Date: Fri Oct 23 13:10:13 2020 +0200 + + iobuf: add more iobuf functions + + These will be used in later patches. + + Reviewed-by: Alexey Tikhonov + +commit ed08ba0023e63024bf1c52ae3f6596b9d804d0a5 +Author: Pavel Březina +Date: Thu Oct 22 12:18:38 2020 +0200 + + secrets: accept binary data instead of string + + Currently, both KCM and secrets responders store JSON formatted string + in the secrets database. One of the next commits makes KCM to store + binary format instead of JSON string to improve performance. We need + to be able to distinguish the formats to keep KCM update compatible + with existing ccache and also to keep secrets responder working. + + Secrets responder test had to be ammended to fit into a new maximum + payload which is now reduced by one byte for the secrets responder + to hold the ending zero of a secret string. + + This is a corner case in a long deprecated responder that is not even + built by default and has no known consumers so it is fine to fast fix + the test. + + Reviewed-by: Alexey Tikhonov + +commit 908c15af9a9f8f0556a588e368e4a0b2e24ace1b +Author: Pavel Březina +Date: Thu Oct 22 11:18:12 2020 +0200 + + secrets: allow to specify secret's data format + + Currently, both KCM and secrets responders store JSON formatted string + in the secrets database. One of the next commits makes KCM to store + binary format instead of JSON string to improve performance. We need + to be able to distinguish the formats to keep KCM update compatible + with existing ccache and also to keep secrets responder working. + + Reviewed-by: Alexey Tikhonov + +commit 74fdaa64b27e88a6e0f153f8cb59989c572d4294 +Author: Pavel Březina +Date: Tue Oct 27 16:45:22 2020 +0100 + + kcm: avoid multiple debug messages if sss_sec_put fails + + sec_put() already logs a message if the underlaying function fails + so this debug message is really unnecessary. + + Reviewed-by: Alexey Tikhonov + +commit b8f28d9aa9d862cf504691c9c3f92941a63fb0a4 +Author: Pavel Březina +Date: Mon Oct 19 12:59:48 2020 +0200 + + kcm: disable encryption + + Encryption was a huge bottleneck for the secdb backend. This is + backwards compatible and there is no need to destroy existing + ccache. It will be stored unencrypted at first write to the cache. + + Note that the encryption did not provide any security as the cache + is accessible only by root and the master key is stored together + with the cache. So once someone gains access to the file it can + be easily decrypted. Additionaly, there was also no encryption at + the memory level. + + Resolves: https://github.com/SSSD/sssd/issues/5349 + + Reviewed-by: Alexey Tikhonov + +commit 8edcea8c377e85d037e83065c1904fa4b92c4a39 +Author: Pavel Březina +Date: Fri Oct 16 15:33:42 2020 +0200 + + kcm: avoid name confusion in GET_CRED_UUID_LIST handlers + + The function name did not follow best practices and it got easily confused + with `kcm_op_get_cred_by_uuid_getbyname_done`. + + ``` + kcm_op_get_cred_uuid_getbyname_done + kcm_op_get_cred_by_uuid_getbyname_done + ``` + + Reviewed-by: Alexey Tikhonov + +commit 47a316c850107f12d406f27abb216e26383dfab7 +Author: Pavel Březina +Date: Mon Sep 14 12:44:57 2020 +0200 + + kcm: fix typos in debug messages + + Reviewed-by: Alexey Tikhonov +--- + Makefile.am | 14 +- + src/responder/kcm/kcmsrv_ccache.c | 66 ++++ + src/responder/kcm/kcmsrv_ccache.h | 47 ++- + src/responder/kcm/kcmsrv_ccache_binary.c | 308 ++++++++++++++++++ + src/responder/kcm/kcmsrv_ccache_json.c | 149 +-------- + src/responder/kcm/kcmsrv_ccache_key.c | 144 ++++++++ + src/responder/kcm/kcmsrv_ccache_mem.c | 30 +- + src/responder/kcm/kcmsrv_ccache_secdb.c | 128 +++----- + src/responder/kcm/kcmsrv_ccache_secrets.c | 9 +- + src/responder/kcm/kcmsrv_cmd.c | 23 +- + src/responder/kcm/kcmsrv_ops.c | 252 ++++++++++---- + src/responder/kcm/kcmsrv_ops.h | 8 + + src/responder/secrets/local.c | 5 +- + src/shared/safealign.h | 4 + + ...n_marshalling.c => test_kcm_marshalling.c} | 147 +++++++-- + src/tests/cmocka/test_sss_ptr_hash.c | 39 +++ + src/tests/cmocka/test_utils.c | 3 + + src/tests/cmocka/test_utils.h | 1 + + src/tests/intg/test_secrets.py | 3 +- + src/tests/multihost/basic/test_kcm.py | 12 +- + src/util/secrets/sec_pvt.h | 2 +- + src/util/secrets/secrets.c | 290 ++++++++++++----- + src/util/secrets/secrets.h | 20 +- + src/util/sss_iobuf.c | 141 ++++++++ + src/util/sss_iobuf.h | 46 +++ + src/util/sss_ptr_hash.c | 20 ++ + 26 files changed, 1457 insertions(+), 454 deletions(-) + create mode 100644 src/responder/kcm/kcmsrv_ccache_binary.c + create mode 100644 src/responder/kcm/kcmsrv_ccache_key.c + rename src/tests/cmocka/{test_kcm_json_marshalling.c => test_kcm_marshalling.c} (71%) + +diff --git a/Makefile.am b/Makefile.am +index 97aa1ec66..430b4e842 100644 +--- a/Makefile.am ++++ b/Makefile.am +@@ -311,7 +311,7 @@ endif # HAVE_INOTIFY + + if BUILD_KCM + non_interactive_cmocka_based_tests += \ +- test_kcm_json \ ++ test_kcm_marshalling \ + test_kcm_queue \ + $(NULL) + endif # BUILD_KCM +@@ -1817,8 +1817,10 @@ sssd_kcm_SOURCES = \ + src/responder/kcm/kcm.c \ + src/responder/kcm/kcmsrv_cmd.c \ + src/responder/kcm/kcmsrv_ccache.c \ ++ src/responder/kcm/kcmsrv_ccache_binary.c \ + src/responder/kcm/kcmsrv_ccache_mem.c \ + src/responder/kcm/kcmsrv_ccache_json.c \ ++ src/responder/kcm/kcmsrv_ccache_key.c \ + src/responder/kcm/kcmsrv_ccache_secdb.c \ + src/responder/kcm/kcmsrv_ops.c \ + src/responder/kcm/kcmsrv_op_queue.c \ +@@ -3927,18 +3929,20 @@ test_sssd_krb5_locator_plugin_LDADD = \ + $(NULL) + + if BUILD_KCM +-test_kcm_json_SOURCES = \ +- src/tests/cmocka/test_kcm_json_marshalling.c \ ++test_kcm_marshalling_SOURCES = \ ++ src/tests/cmocka/test_kcm_marshalling.c \ ++ src/responder/kcm/kcmsrv_ccache_binary.c \ + src/responder/kcm/kcmsrv_ccache_json.c \ ++ src/responder/kcm/kcmsrv_ccache_key.c \ + src/responder/kcm/kcmsrv_ccache.c \ + src/util/sss_krb5.c \ + src/util/sss_iobuf.c \ + $(NULL) +-test_kcm_json_CFLAGS = \ ++test_kcm_marshalling_CFLAGS = \ + $(AM_CFLAGS) \ + $(UUID_CFLAGS) \ + $(NULL) +-test_kcm_json_LDADD = \ ++test_kcm_marshalling_LDADD = \ + $(JANSSON_LIBS) \ + $(UUID_LIBS) \ + $(KRB5_LIBS) \ +diff --git a/src/responder/kcm/kcmsrv_ccache.c b/src/responder/kcm/kcmsrv_ccache.c +index 66e2752ba..60eacd451 100644 +--- a/src/responder/kcm/kcmsrv_ccache.c ++++ b/src/responder/kcm/kcmsrv_ccache.c +@@ -28,6 +28,9 @@ + #include "responder/kcm/kcmsrv_ccache_pvt.h" + #include "responder/kcm/kcmsrv_ccache_be.h" + ++static struct kcm_cred *kcm_cred_dup(TALLOC_CTX *mem_ctx, ++ struct kcm_cred *crd); ++ + static int kcm_cc_destructor(struct kcm_ccache *cc) + { + if (cc == NULL) { +@@ -94,6 +97,33 @@ done: + return ret; + } + ++struct kcm_ccache *kcm_cc_dup(TALLOC_CTX *mem_ctx, ++ const struct kcm_ccache *cc) ++{ ++ struct kcm_ccache *dup; ++ struct kcm_cred *crd_dup; ++ struct kcm_cred *crd; ++ ++ dup = talloc_zero(mem_ctx, struct kcm_ccache); ++ if (dup == NULL) { ++ return NULL; ++ } ++ memcpy(dup, cc, sizeof(struct kcm_ccache)); ++ ++ dup->creds = NULL; ++ DLIST_FOR_EACH(crd, cc->creds) { ++ crd_dup = kcm_cred_dup(dup, crd); ++ if (crd_dup == NULL) { ++ talloc_free(dup); ++ return NULL; ++ } ++ ++ DLIST_ADD(dup->creds, crd_dup); ++ } ++ ++ return dup; ++} ++ + const char *kcm_cc_get_name(struct kcm_ccache *cc) + { + return cc ? cc->name : NULL; +@@ -204,6 +234,22 @@ struct kcm_cred *kcm_cred_new(TALLOC_CTX *mem_ctx, + return kcreds; + } + ++static struct kcm_cred *kcm_cred_dup(TALLOC_CTX *mem_ctx, ++ struct kcm_cred *crd) ++{ ++ struct kcm_cred *dup; ++ ++ dup = talloc_zero(mem_ctx, struct kcm_cred); ++ if (dup == NULL) { ++ return NULL; ++ } ++ ++ uuid_copy(dup->uuid, crd->uuid); ++ dup->cred_blob = crd->cred_blob; ++ ++ return dup; ++} ++ + /* Add a cred to ccache */ + errno_t kcm_cc_store_creds(struct kcm_ccache *cc, + struct kcm_cred *crd) +@@ -213,6 +259,26 @@ errno_t kcm_cc_store_creds(struct kcm_ccache *cc, + return EOK; + } + ++errno_t kcm_cc_set_header(struct kcm_ccache *cc, ++ const char *sec_key, ++ struct cli_creds *client) ++{ ++ errno_t ret; ++ ++ ret = sec_key_parse(cc, sec_key, &cc->name, cc->uuid); ++ if (ret != EOK) { ++ return ret; ++ } ++ ++ /* We rely on sssd-secrets only searching the user's subtree so we ++ * set the ownership to the client ++ */ ++ cc->owner.uid = cli_creds_get_uid(client); ++ cc->owner.gid = cli_creds_get_gid(client); ++ ++ return EOK; ++} ++ + errno_t kcm_cred_get_uuid(struct kcm_cred *crd, uuid_t _uuid) + { + if (crd == NULL) { +diff --git a/src/responder/kcm/kcmsrv_ccache.h b/src/responder/kcm/kcmsrv_ccache.h +index d629923fa..77cf8f61d 100644 +--- a/src/responder/kcm/kcmsrv_ccache.h ++++ b/src/responder/kcm/kcmsrv_ccache.h +@@ -72,6 +72,13 @@ errno_t kcm_cc_new(TALLOC_CTX *mem_ctx, + krb5_principal princ, + struct kcm_ccache **_cc); + ++/* ++ * Duplicate the ccache. Only ccache and credentials are duplicated, ++ * but their data are a shallow copy. ++ */ ++struct kcm_ccache *kcm_cc_dup(TALLOC_CTX *mem_ctx, ++ const struct kcm_ccache *cc); ++ + /* + * Returns true if a client can access a ccache. + * +@@ -100,6 +107,11 @@ struct kcm_cred *kcm_cred_new(TALLOC_CTX *mem_ctx, + errno_t kcm_cc_store_creds(struct kcm_ccache *cc, + struct kcm_cred *crd); + ++/* Set cc header information from sec key and client */ ++errno_t kcm_cc_set_header(struct kcm_ccache *cc, ++ const char *sec_key, ++ struct cli_creds *client); ++ + errno_t kcm_cred_get_uuid(struct kcm_cred *crd, uuid_t uuid); + + /* +@@ -320,6 +332,11 @@ bool sec_key_match_name(const char *sec_key, + bool sec_key_match_uuid(const char *sec_key, + uuid_t uuid); + ++errno_t sec_key_parse(TALLOC_CTX *mem_ctx, ++ const char *sec_key, ++ const char **_name, ++ uuid_t uuid); ++ + const char *sec_key_get_name(const char *sec_key); + + errno_t sec_key_get_uuid(const char *sec_key, +@@ -333,16 +350,30 @@ const char *sec_key_create(TALLOC_CTX *mem_ctx, + * sec_key is a concatenation of the ccache's UUID and name + * sec_value is the JSON dump of the ccache contents + */ +-errno_t sec_kv_to_ccache(TALLOC_CTX *mem_ctx, +- const char *sec_key, +- const char *sec_value, +- struct cli_creds *client, +- struct kcm_ccache **_cc); ++errno_t sec_kv_to_ccache_json(TALLOC_CTX *mem_ctx, ++ const char *sec_key, ++ const char *sec_value, ++ struct cli_creds *client, ++ struct kcm_ccache **_cc); + + /* Convert a kcm_ccache to a key-value pair to be stored in secrets */ +-errno_t kcm_ccache_to_sec_input(TALLOC_CTX *mem_ctx, +- struct kcm_ccache *cc, ++errno_t kcm_ccache_to_sec_input_json(TALLOC_CTX *mem_ctx, ++ struct kcm_ccache *cc, ++ struct sss_iobuf **_payload); ++ ++/* ++ * sec_key is a concatenation of the ccache's UUID and name ++ * sec_value is the binary representation of ccache. ++ */ ++errno_t sec_kv_to_ccache_binary(TALLOC_CTX *mem_ctx, ++ const char *sec_key, ++ struct sss_iobuf *sec_value, + struct cli_creds *client, +- struct sss_iobuf **_payload); ++ struct kcm_ccache **_cc); ++ ++/* Convert a kcm_ccache to its binary representation. */ ++errno_t kcm_ccache_to_sec_input_binary(TALLOC_CTX *mem_ctx, ++ struct kcm_ccache *cc, ++ struct sss_iobuf **_payload); + + #endif /* _KCMSRV_CCACHE_H_ */ +diff --git a/src/responder/kcm/kcmsrv_ccache_binary.c b/src/responder/kcm/kcmsrv_ccache_binary.c +new file mode 100644 +index 000000000..7bfdbf13b +--- /dev/null ++++ b/src/responder/kcm/kcmsrv_ccache_binary.c +@@ -0,0 +1,308 @@ ++/* ++ Authors: ++ Pavel Březina ++ ++ Copyright (C) 2020 Red Hat ++ ++ This program is free software; you can redistribute it and/or modify ++ it under the terms of the GNU General Public License as published by ++ the Free Software Foundation; either version 3 of the License, or ++ (at your option) any later version. ++ ++ This program is distributed in the hope that it will be useful, ++ but WITHOUT ANY WARRANTY; without even the implied warranty of ++ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the ++ GNU General Public License for more details. ++ ++ You should have received a copy of the GNU General Public License ++ along with this program. If not, see . ++*/ ++ ++#include "config.h" ++ ++#include ++#include ++ ++#include "util/util.h" ++#include "util/util_creds.h" ++#include "util/crypto/sss_crypto.h" ++#include "responder/kcm/kcmsrv_ccache_pvt.h" ++ ++static errno_t krb_data_to_bin(krb5_data *data, struct sss_iobuf *buf) ++{ ++ return sss_iobuf_write_varlen(buf, (uint8_t *)data->data, data->length); ++} ++ ++static errno_t princ_to_bin(krb5_principal princ, struct sss_iobuf *buf) ++{ ++ errno_t ret; ++ ++ if (princ == NULL) { ++ return sss_iobuf_write_uint8(buf, 0); ++ } ++ ++ /* Mark that principal is not empty. */ ++ ret = sss_iobuf_write_uint8(buf, 1); ++ if (ret != EOK) { ++ return ret; ++ } ++ ++ ret = krb_data_to_bin(&princ->realm, buf); ++ if (ret != EOK) { ++ return ret; ++ } ++ ++ ret = sss_iobuf_write_int32(buf, princ->type); ++ if (ret != EOK) { ++ return ret; ++ } ++ ++ ret = sss_iobuf_write_int32(buf, princ->length); ++ if (ret != EOK) { ++ return ret; ++ } ++ ++ for (krb5_int32 i = 0; i < princ->length; i++) { ++ ret = krb_data_to_bin(&princ->data[i], buf); ++ if (ret != EOK) { ++ return ret; ++ } ++ } ++ ++ return EOK; ++} ++ ++static errno_t creds_to_bin(struct kcm_cred *creds, struct sss_iobuf *buf) ++{ ++ struct kcm_cred *crd; ++ uint32_t count = 0; ++ errno_t ret; ++ ++ DLIST_FOR_EACH(crd, creds) { ++ count++; ++ } ++ ++ ret = sss_iobuf_write_uint32(buf, count); ++ if (ret != EOK) { ++ return ret; ++ } ++ ++ DLIST_FOR_EACH(crd, creds) { ++ ret = sss_iobuf_write_len(buf, (uint8_t *)crd->uuid, sizeof(uuid_t)); ++ if (ret != EOK) { ++ return ret; ++ } ++ ++ ret = sss_iobuf_write_iobuf(buf, crd->cred_blob); ++ if (ret != EOK) { ++ return ret; ++ } ++ } ++ ++ return EOK; ++} ++ ++errno_t kcm_ccache_to_sec_input_binary(TALLOC_CTX *mem_ctx, ++ struct kcm_ccache *cc, ++ struct sss_iobuf **_payload) ++{ ++ struct sss_iobuf *buf; ++ errno_t ret; ++ ++ buf = sss_iobuf_init_empty(mem_ctx, sizeof(krb5_principal_data), 0); ++ if (buf == NULL) { ++ return ENOMEM; ++ } ++ ++ ret = sss_iobuf_write_int32(buf, cc->kdc_offset); ++ if (ret != EOK) { ++ goto done; ++ } ++ ++ ret = princ_to_bin(cc->client, buf); ++ if (ret != EOK) { ++ goto done; ++ } ++ ++ ret = creds_to_bin(cc->creds, buf); ++ if (ret != EOK) { ++ goto done; ++ } ++ ++ *_payload = buf; ++ ++ ret = EOK; ++ ++done: ++ if (ret != EOK) { ++ talloc_free(buf); ++ } ++ ++ return ret; ++} ++ ++static errno_t bin_to_krb_data(TALLOC_CTX *mem_ctx, ++ struct sss_iobuf *buf, ++ krb5_data *out) ++{ ++ uint8_t *data; ++ size_t len; ++ errno_t ret; ++ ++ ret = sss_iobuf_read_varlen(mem_ctx, buf, &data, &len); ++ if (ret != EOK) { ++ return ret; ++ } ++ ++ out->magic = 0; ++ out->data = (char*)data; ++ out->length = len; ++ ++ return EOK; ++} ++ ++static errno_t bin_to_princ(TALLOC_CTX *mem_ctx, ++ struct sss_iobuf *buf, ++ krb5_principal *_princ) ++{ ++ krb5_principal princ; ++ uint8_t non_empty; ++ krb5_int32 i; ++ errno_t ret; ++ ++ ret = sss_iobuf_read_uint8(buf, &non_empty); ++ if (ret != EOK) { ++ return ret; ++ } ++ ++ if (non_empty == 0) { ++ *_princ = NULL; ++ return EOK; ++ } ++ ++ princ = talloc_zero(mem_ctx, struct krb5_principal_data); ++ if (princ == NULL) { ++ return ENOMEM; ++ } ++ princ->magic = KV5M_PRINCIPAL; ++ ++ ret = bin_to_krb_data(princ, buf, &princ->realm); ++ if (ret != EOK) { ++ return ret; ++ } ++ ++ ret = sss_iobuf_read_int32(buf, &princ->type); ++ if (ret != EOK) { ++ return ret; ++ } ++ ++ ret = sss_iobuf_read_int32(buf, &princ->length); ++ if (ret != EOK) { ++ return ret; ++ } ++ ++ princ->data = talloc_zero_array(princ, krb5_data, princ->length); ++ if (princ->length > 0 && princ->data == NULL) { ++ return ENOMEM; ++ } ++ ++ for (i = 0; i < princ->length; i++) { ++ ret = bin_to_krb_data(princ, buf, &princ->data[i]); ++ if (ret != EOK) { ++ return ret; ++ } ++ } ++ ++ *_princ = princ; ++ ++ return EOK; ++} ++ ++static errno_t bin_to_creds(TALLOC_CTX *mem_ctx, ++ struct sss_iobuf *buf, ++ struct kcm_cred **_creds) ++{ ++ struct kcm_cred *creds = NULL; ++ struct kcm_cred *crd; ++ struct sss_iobuf *cred_blob; ++ uint32_t count; ++ uuid_t uuid; ++ errno_t ret; ++ ++ ret = sss_iobuf_read_uint32(buf, &count); ++ if (ret != EOK) { ++ return ret; ++ } ++ ++ for (uint32_t i = 0; i < count; i++) { ++ ret = sss_iobuf_read_len(buf, sizeof(uuid_t), (uint8_t*)uuid); ++ if (ret != EOK) { ++ return ret; ++ } ++ ++ ret = sss_iobuf_read_iobuf(NULL, buf, &cred_blob); ++ if (ret != EOK) { ++ return ret; ++ } ++ ++ crd = kcm_cred_new(mem_ctx, uuid, cred_blob); ++ if (crd == NULL) { ++ talloc_free(cred_blob); ++ return ENOMEM; ++ } ++ ++ DLIST_ADD(creds, crd); ++ } ++ ++ *_creds = creds; ++ ++ return EOK; ++} ++ ++errno_t sec_kv_to_ccache_binary(TALLOC_CTX *mem_ctx, ++ const char *sec_key, ++ struct sss_iobuf *sec_value, ++ struct cli_creds *client, ++ struct kcm_ccache **_cc) ++{ ++ struct kcm_ccache *cc; ++ errno_t ret; ++ ++ cc = talloc_zero(mem_ctx, struct kcm_ccache); ++ if (cc == NULL) { ++ return ENOMEM; ++ } ++ ++ ret = kcm_cc_set_header(cc, sec_key, client); ++ if (ret != EOK) { ++ DEBUG(SSSDBG_CRIT_FAILURE, "Cannot store ccache header [%d]: %s\n", ++ ret, sss_strerror(ret)); ++ goto done; ++ } ++ ++ ret = sss_iobuf_read_int32(sec_value, &cc->kdc_offset); ++ if (ret != EOK) { ++ goto done; ++ } ++ ++ ret = bin_to_princ(cc, sec_value, &cc->client); ++ if (ret != EOK) { ++ goto done; ++ } ++ ++ ret = bin_to_creds(cc, sec_value, &cc->creds); ++ if (ret != EOK) { ++ goto done; ++ } ++ ++ *_cc = cc; ++ ++ ret = EOK; ++ ++done: ++ if (ret != EOK) { ++ talloc_free(cc); ++ } ++ ++ return ret; ++} +diff --git a/src/responder/kcm/kcmsrv_ccache_json.c b/src/responder/kcm/kcmsrv_ccache_json.c +index f78e9f58c..e790cbea3 100644 +--- a/src/responder/kcm/kcmsrv_ccache_json.c ++++ b/src/responder/kcm/kcmsrv_ccache_json.c +@@ -37,12 +37,6 @@ + */ + #define KS_JSON_VERSION 1 + +-/* +- * The secrets store is a key-value store at heart. We store the UUID +- * and the name in the key to allow easy lookups be either key +- */ +-#define SEC_KEY_SEPARATOR '-' +- + /* Compat definition of json_array_foreach for older systems */ + #ifndef json_array_foreach + #define json_array_foreach(array, idx, value) \ +@@ -51,119 +45,6 @@ + idx++) + #endif + +-const char *sec_key_create(TALLOC_CTX *mem_ctx, +- const char *name, +- uuid_t uuid) +-{ +- char uuid_str[UUID_STR_SIZE]; +- +- uuid_unparse(uuid, uuid_str); +- return talloc_asprintf(mem_ctx, +- "%s%c%s", uuid_str, SEC_KEY_SEPARATOR, name); +-} +- +-static bool sec_key_valid(const char *sec_key) +-{ +- if (sec_key == NULL) { +- return false; +- } +- +- if (strlen(sec_key) < UUID_STR_SIZE + 1) { +- /* One char for separator (at UUID_STR_SIZE, because strlen doesn't +- * include the '\0', but UUID_STR_SIZE does) and at least one for +- * the name */ +- DEBUG(SSSDBG_CRIT_FAILURE, "Key %s is too short\n", sec_key); +- return false; +- } +- +- if (sec_key[UUID_STR_SIZE - 1] != SEC_KEY_SEPARATOR) { +- DEBUG(SSSDBG_CRIT_FAILURE, "Key doesn't contain the separator\n"); +- return false; +- } +- +- return true; +-} +- +-static errno_t sec_key_parse(TALLOC_CTX *mem_ctx, +- const char *sec_key, +- const char **_name, +- uuid_t uuid) +-{ +- char uuid_str[UUID_STR_SIZE]; +- +- if (!sec_key_valid(sec_key)) { +- return EINVAL; +- } +- +- strncpy(uuid_str, sec_key, sizeof(uuid_str)-1); +- if (sec_key[UUID_STR_SIZE - 1] != SEC_KEY_SEPARATOR) { +- DEBUG(SSSDBG_CRIT_FAILURE, "Key doesn't contain the separator\n"); +- return EINVAL; +- } +- uuid_str[UUID_STR_SIZE-1] = '\0'; +- +- *_name = talloc_strdup(mem_ctx, sec_key + UUID_STR_SIZE); +- if (*_name == NULL) { +- return ENOMEM; +- } +- uuid_parse(uuid_str, uuid); +- +- return EOK; +-} +- +-errno_t sec_key_get_uuid(const char *sec_key, +- uuid_t uuid) +-{ +- char uuid_str[UUID_STR_SIZE]; +- +- if (!sec_key_valid(sec_key)) { +- return EINVAL; +- } +- +- strncpy(uuid_str, sec_key, UUID_STR_SIZE-1); +- uuid_str[UUID_STR_SIZE-1] = '\0'; +- uuid_parse(uuid_str, uuid); +- return EOK; +-} +- +-const char *sec_key_get_name(const char *sec_key) +-{ +- if (!sec_key_valid(sec_key)) { +- return NULL; +- } +- +- return sec_key + UUID_STR_SIZE; +-} +- +-bool sec_key_match_name(const char *sec_key, +- const char *name) +-{ +- if (!sec_key_valid(sec_key) || name == NULL) { +- return false; +- } +- +- return strcmp(sec_key + UUID_STR_SIZE, name) == 0; +-} +- +-bool sec_key_match_uuid(const char *sec_key, +- uuid_t uuid) +-{ +- errno_t ret; +- uuid_t key_uuid; +- +- /* `key_uuid` is output arg and isn't read in sec_key_get_uuid() but +- * since libuuid is opaque for cppcheck it generates false positive here +- */ +- /* cppcheck-suppress uninitvar */ +- ret = sec_key_get_uuid(sec_key, key_uuid); +- if (ret != EOK) { +- DEBUG(SSSDBG_MINOR_FAILURE, "Cannot convert key to UUID\n"); +- return false; +- } +- +- return uuid_compare(key_uuid, uuid) == 0; +-} +- + /* + * Creates an array of principal elements that will be used later + * in the form of: +@@ -460,10 +341,9 @@ static errno_t ccache_to_sec_val(TALLOC_CTX *mem_ctx, + return EOK; + } + +-errno_t kcm_ccache_to_sec_input(TALLOC_CTX *mem_ctx, +- struct kcm_ccache *cc, +- struct cli_creds *client, +- struct sss_iobuf **_payload) ++errno_t kcm_ccache_to_sec_input_json(TALLOC_CTX *mem_ctx, ++ struct kcm_ccache *cc, ++ struct sss_iobuf **_payload) + { + errno_t ret; + const char *value; +@@ -897,11 +777,11 @@ static errno_t sec_json_value_to_ccache(struct kcm_ccache *cc, + * sec_key is a concatenation of the ccache's UUID and name + * sec_value is the JSON dump of the ccache contents + */ +-errno_t sec_kv_to_ccache(TALLOC_CTX *mem_ctx, +- const char *sec_key, +- const char *sec_value, +- struct cli_creds *client, +- struct kcm_ccache **_cc) ++errno_t sec_kv_to_ccache_json(TALLOC_CTX *mem_ctx, ++ const char *sec_key, ++ const char *sec_value, ++ struct cli_creds *client, ++ struct kcm_ccache **_cc) + { + errno_t ret; + json_t *root = NULL; +@@ -911,7 +791,7 @@ errno_t sec_kv_to_ccache(TALLOC_CTX *mem_ctx, + ret = sec_value_to_json(sec_value, &root); + if (ret != EOK) { + DEBUG(SSSDBG_CRIT_FAILURE, +- "Cannot store secret to JSN [%d]: %s\n", ++ "Cannot store secret to JSON [%d]: %s\n", + ret, sss_strerror(ret)); + goto done; + } +@@ -928,16 +808,9 @@ errno_t sec_kv_to_ccache(TALLOC_CTX *mem_ctx, + goto done; + } + +- /* We rely on sssd-secrets only searching the user's subtree so we +- * set the ownership to the client +- */ +- cc->owner.uid = cli_creds_get_uid(client); +- cc->owner.gid = cli_creds_get_gid(client); +- +- ret = sec_key_parse(cc, sec_key, &cc->name, cc->uuid); ++ ret = kcm_cc_set_header(cc, sec_key, client); + if (ret != EOK) { +- DEBUG(SSSDBG_CRIT_FAILURE, +- "Cannt parse secret key [%d]: %s\n", ++ DEBUG(SSSDBG_CRIT_FAILURE, "Cannot store ccache header [%d]: %s\n", + ret, sss_strerror(ret)); + goto done; + } +diff --git a/src/responder/kcm/kcmsrv_ccache_key.c b/src/responder/kcm/kcmsrv_ccache_key.c +new file mode 100644 +index 000000000..59d60453c +--- /dev/null ++++ b/src/responder/kcm/kcmsrv_ccache_key.c +@@ -0,0 +1,144 @@ ++/* ++ SSSD ++ ++ Copyright (C) Red Hat, 2020 ++ ++ This program is free software; you can redistribute it and/or modify ++ it under the terms of the GNU General Public License as published by ++ the Free Software Foundation; either version 3 of the License, or ++ (at your option) any later version. ++ ++ This program is distributed in the hope that it will be useful, ++ but WITHOUT ANY WARRANTY; without even the implied warranty of ++ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the ++ GNU General Public License for more details. ++ ++ You should have received a copy of the GNU General Public License ++ along with this program. If not, see . ++*/ ++ ++#include "config.h" ++ ++#include ++#include ++ ++#include "util/util.h" ++#include "responder/kcm/kcmsrv_ccache_pvt.h" ++ ++/* ++ * The secrets store is a key-value store at heart. We store the UUID ++ * and the name in the key to allow easy lookups by either part. ++ */ ++#define SEC_KEY_SEPARATOR '-' ++ ++const char *sec_key_create(TALLOC_CTX *mem_ctx, ++ const char *name, ++ uuid_t uuid) ++{ ++ char uuid_str[UUID_STR_SIZE]; ++ ++ uuid_unparse(uuid, uuid_str); ++ return talloc_asprintf(mem_ctx, ++ "%s%c%s", uuid_str, SEC_KEY_SEPARATOR, name); ++} ++ ++static bool sec_key_valid(const char *sec_key) ++{ ++ if (sec_key == NULL) { ++ return false; ++ } ++ ++ if (strlen(sec_key) < UUID_STR_SIZE + 1) { ++ /* One char for separator (at UUID_STR_SIZE, because strlen doesn't ++ * include the '\0', but UUID_STR_SIZE does) and at least one for ++ * the name */ ++ DEBUG(SSSDBG_CRIT_FAILURE, "Key %s is too short\n", sec_key); ++ return false; ++ } ++ ++ if (sec_key[UUID_STR_SIZE - 1] != SEC_KEY_SEPARATOR) { ++ DEBUG(SSSDBG_CRIT_FAILURE, "Key doesn't contain the separator\n"); ++ return false; ++ } ++ ++ return true; ++} ++ ++errno_t sec_key_parse(TALLOC_CTX *mem_ctx, ++ const char *sec_key, ++ const char **_name, ++ uuid_t uuid) ++{ ++ char uuid_str[UUID_STR_SIZE]; ++ ++ if (!sec_key_valid(sec_key)) { ++ return EINVAL; ++ } ++ ++ strncpy(uuid_str, sec_key, sizeof(uuid_str) - 1); ++ if (sec_key[UUID_STR_SIZE - 1] != SEC_KEY_SEPARATOR) { ++ DEBUG(SSSDBG_CRIT_FAILURE, "Key doesn't contain the separator\n"); ++ return EINVAL; ++ } ++ uuid_str[UUID_STR_SIZE - 1] = '\0'; ++ ++ *_name = talloc_strdup(mem_ctx, sec_key + UUID_STR_SIZE); ++ if (*_name == NULL) { ++ return ENOMEM; ++ } ++ uuid_parse(uuid_str, uuid); ++ ++ return EOK; ++} ++ ++errno_t sec_key_get_uuid(const char *sec_key, ++ uuid_t uuid) ++{ ++ char uuid_str[UUID_STR_SIZE]; ++ ++ if (!sec_key_valid(sec_key)) { ++ return EINVAL; ++ } ++ ++ strncpy(uuid_str, sec_key, UUID_STR_SIZE - 1); ++ uuid_str[UUID_STR_SIZE - 1] = '\0'; ++ uuid_parse(uuid_str, uuid); ++ return EOK; ++} ++ ++const char *sec_key_get_name(const char *sec_key) ++{ ++ if (!sec_key_valid(sec_key)) { ++ return NULL; ++ } ++ ++ return sec_key + UUID_STR_SIZE; ++} ++ ++bool sec_key_match_name(const char *sec_key, ++ const char *name) ++{ ++ if (!sec_key_valid(sec_key) || name == NULL) { ++ return false; ++ } ++ ++ return strcmp(sec_key + UUID_STR_SIZE, name) == 0; ++} ++ ++bool sec_key_match_uuid(const char *sec_key, ++ uuid_t uuid) ++{ ++ errno_t ret; ++ uuid_t key_uuid; ++ ++ /* Clear uuid value to avoid cppcheck warning. */ ++ uuid_clear(key_uuid); ++ ++ ret = sec_key_get_uuid(sec_key, key_uuid); ++ if (ret != EOK) { ++ DEBUG(SSSDBG_MINOR_FAILURE, "Cannot convert key to UUID\n"); ++ return false; ++ } ++ ++ return uuid_compare(key_uuid, uuid) == 0; ++} +diff --git a/src/responder/kcm/kcmsrv_ccache_mem.c b/src/responder/kcm/kcmsrv_ccache_mem.c +index baa698054..0e3a7b239 100644 +--- a/src/responder/kcm/kcmsrv_ccache_mem.c ++++ b/src/responder/kcm/kcmsrv_ccache_mem.c +@@ -49,24 +49,6 @@ struct ccdb_mem { + unsigned int nextid; + }; + +-/* In order to provide a consistent interface, we need to let the caller +- * of getbyXXX own the ccache, therefore the memory back end returns a shallow +- * copy of the ccache +- */ +-static struct kcm_ccache *kcm_ccache_dup(TALLOC_CTX *mem_ctx, +- struct kcm_ccache *in) +-{ +- struct kcm_ccache *out; +- +- out = talloc_zero(mem_ctx, struct kcm_ccache); +- if (out == NULL) { +- return NULL; +- } +- memcpy(out, in, sizeof(struct kcm_ccache)); +- +- return out; +-} +- + static struct ccache_mem_wrap *memdb_get_by_uuid(struct ccdb_mem *memdb, + struct cli_creds *client, + uuid_t uuid) +@@ -417,7 +399,11 @@ static struct tevent_req *ccdb_mem_getbyuuid_send(TALLOC_CTX *mem_ctx, + + ccwrap = memdb_get_by_uuid(memdb, client, uuid); + if (ccwrap != NULL) { +- state->cc = kcm_ccache_dup(state, ccwrap->cc); ++ /* In order to provide a consistent interface, we need to let the caller ++ * of getbyXXX own the ccache, therefore the memory back end returns a shallow ++ * copy of the ccache ++ */ ++ state->cc = kcm_cc_dup(state, ccwrap->cc); + if (state->cc == NULL) { + ret = ENOMEM; + goto immediate; +@@ -470,7 +456,11 @@ static struct tevent_req *ccdb_mem_getbyname_send(TALLOC_CTX *mem_ctx, + + ccwrap = memdb_get_by_name(memdb, client, name); + if (ccwrap != NULL) { +- state->cc = kcm_ccache_dup(state, ccwrap->cc); ++ /* In order to provide a consistent interface, we need to let the caller ++ * of getbyXXX own the ccache, therefore the memory back end returns a shallow ++ * copy of the ccache ++ */ ++ state->cc = kcm_cc_dup(state, ccwrap->cc); + if (state->cc == NULL) { + ret = ENOMEM; + goto immediate; +diff --git a/src/responder/kcm/kcmsrv_ccache_secdb.c b/src/responder/kcm/kcmsrv_ccache_secdb.c +index ed1c8247f..726711ac4 100644 +--- a/src/responder/kcm/kcmsrv_ccache_secdb.c ++++ b/src/responder/kcm/kcmsrv_ccache_secdb.c +@@ -35,15 +35,16 @@ + #define KCM_SECDB_CCACHE_FMT KCM_SECDB_BASE_FMT"ccache/" + #define KCM_SECDB_DFL_FMT KCM_SECDB_BASE_FMT"default" + +-static errno_t sec_get_b64(TALLOC_CTX *mem_ctx, +- struct sss_sec_req *req, +- struct sss_iobuf **_buf) ++static errno_t sec_get(TALLOC_CTX *mem_ctx, ++ struct sss_sec_req *req, ++ struct sss_iobuf **_buf, ++ char **_datatype) + { + errno_t ret; + TALLOC_CTX *tmp_ctx; +- char *b64_sec; ++ char *datatype; + uint8_t *data; +- size_t data_size; ++ size_t len; + struct sss_iobuf *buf; + + tmp_ctx = talloc_new(mem_ctx); +@@ -51,101 +52,61 @@ static errno_t sec_get_b64(TALLOC_CTX *mem_ctx, + return ENOMEM; + } + +- ret = sss_sec_get(tmp_ctx, req, &b64_sec); ++ ret = sss_sec_get(tmp_ctx, req, &data, &len, &datatype); + if (ret != EOK) { + DEBUG(SSSDBG_OP_FAILURE, + "Cannot retrieve the secret [%d]: %s\n", ret, sss_strerror(ret)); + goto done; + } + +- data = sss_base64_decode(tmp_ctx, b64_sec, &data_size); +- if (data == NULL) { +- DEBUG(SSSDBG_CRIT_FAILURE, "Cannot decode secret from base64\n"); +- ret = EIO; +- goto done; +- } +- +- buf = sss_iobuf_init_readonly(tmp_ctx, data, data_size); ++ buf = sss_iobuf_init_steal(tmp_ctx, data, len); + if (buf == NULL) { + DEBUG(SSSDBG_CRIT_FAILURE, "Cannot init the iobuf\n"); + ret = EIO; + goto done; + } + +- ret = EOK; + *_buf = talloc_steal(mem_ctx, buf); ++ if (_datatype != NULL) { ++ *_datatype = talloc_steal(mem_ctx, datatype); ++ } ++ ++ ret = EOK; ++ + done: + talloc_free(tmp_ctx); + return ret; + } + +-static errno_t sec_put_b64(TALLOC_CTX *mem_ctx, +- struct sss_sec_req *req, +- struct sss_iobuf *buf) ++static errno_t sec_put(TALLOC_CTX *mem_ctx, ++ struct sss_sec_req *req, ++ struct sss_iobuf *buf) + { + errno_t ret; +- TALLOC_CTX *tmp_ctx; +- char *secret; + +- tmp_ctx = talloc_new(mem_ctx); +- if (tmp_ctx == NULL) { +- return ENOMEM; +- } +- +- secret = sss_base64_encode(tmp_ctx, +- sss_iobuf_get_data(buf), +- sss_iobuf_get_size(buf)); +- if (secret == NULL) { +- DEBUG(SSSDBG_CRIT_FAILURE, "Cannot encode secret to base64\n"); +- ret = EIO; +- goto done; +- } +- +- ret = sss_sec_put(req, secret); ++ ret = sss_sec_put(req, sss_iobuf_get_data(buf), sss_iobuf_get_size(buf), ++ SSS_SEC_PLAINTEXT, "binary"); + if (ret != EOK) { + DEBUG(SSSDBG_OP_FAILURE, + "Cannot write the secret [%d]: %s\n", ret, sss_strerror(ret)); +- goto done; + } + +- ret = EOK; +-done: +- talloc_free(tmp_ctx); + return ret; + } + +-static errno_t sec_update_b64(TALLOC_CTX *mem_ctx, +- struct sss_sec_req *req, +- struct sss_iobuf *buf) ++static errno_t sec_update(TALLOC_CTX *mem_ctx, ++ struct sss_sec_req *req, ++ struct sss_iobuf *buf) + { + errno_t ret; +- TALLOC_CTX *tmp_ctx; +- char *secret; +- +- tmp_ctx = talloc_new(mem_ctx); +- if (tmp_ctx == NULL) { +- return ENOMEM; +- } +- +- secret = sss_base64_encode(tmp_ctx, +- sss_iobuf_get_data(buf), +- sss_iobuf_get_size(buf)); +- if (secret == NULL) { +- DEBUG(SSSDBG_CRIT_FAILURE, "Cannot encode secret to base64\n"); +- ret = EIO; +- goto done; +- } + +- ret = sss_sec_update(req, secret); ++ ret = sss_sec_update(req, sss_iobuf_get_data(buf), sss_iobuf_get_size(buf), ++ SSS_SEC_PLAINTEXT, "binary"); + if (ret != EOK) { + DEBUG(SSSDBG_OP_FAILURE, + "Cannot write the secret [%d]: %s\n", ret, sss_strerror(ret)); +- goto done; + } + +- ret = EOK; +-done: +- talloc_free(tmp_ctx); + return ret; + } + +@@ -206,7 +167,7 @@ static errno_t kcm_ccache_to_secdb_kv(TALLOC_CTX *mem_ctx, + goto done; + } + +- ret = kcm_ccache_to_sec_input(mem_ctx, cc, client, &payload); ++ ret = kcm_ccache_to_sec_input_binary(mem_ctx, cc, &payload); + if (ret != EOK) { + DEBUG(SSSDBG_CRIT_FAILURE, + "Cannot convert ccache to a secret [%d][%s]\n", ret, sss_strerror(ret)); +@@ -480,6 +441,7 @@ static errno_t secdb_get_cc(TALLOC_CTX *mem_ctx, + struct kcm_ccache *cc = NULL; + struct sss_sec_req *sreq = NULL; + struct sss_iobuf *ccbuf; ++ char *datatype; + + tmp_ctx = talloc_new(mem_ctx); + if (tmp_ctx == NULL) { +@@ -493,22 +455,23 @@ static errno_t secdb_get_cc(TALLOC_CTX *mem_ctx, + goto done; + } + +- ret = sec_get_b64(tmp_ctx, sreq, &ccbuf); ++ ret = sec_get(tmp_ctx, sreq, &ccbuf, &datatype); + if (ret != EOK) { + DEBUG(SSSDBG_OP_FAILURE, + "Cannot get the secret [%d][%s]\n", ret, sss_strerror(ret)); + goto done; + } + +- ret = sec_kv_to_ccache(tmp_ctx, +- secdb_key, +- (const char *) sss_iobuf_get_data(ccbuf), +- client, +- &cc); ++ if (strcmp(datatype, "binary") == 0) { ++ ret = sec_kv_to_ccache_binary(tmp_ctx, secdb_key, ccbuf, client, &cc); ++ } else { ++ ret = sec_kv_to_ccache_json(tmp_ctx, secdb_key, ++ (const char *)sss_iobuf_get_data(ccbuf), ++ client, &cc); ++ } + if (ret != EOK) { +- DEBUG(SSSDBG_OP_FAILURE, +- "Cannot convert JSON keyval to ccache blob [%d]: %s\n", +- ret, sss_strerror(ret)); ++ DEBUG(SSSDBG_OP_FAILURE, "Cannot convert %s data to ccache " ++ "[%d]: %s\n", datatype, ret, sss_strerror(ret)); + goto done; + } + +@@ -746,11 +709,11 @@ static struct tevent_req *ccdb_secdb_set_default_send(TALLOC_CTX *mem_ctx, + goto immediate; + } + +- ret = sss_sec_get(state, sreq, &cur_default); ++ ret = sss_sec_get(state, sreq, (uint8_t**)&cur_default, NULL, NULL); + if (ret == ENOENT) { +- ret = sec_put_b64(state, sreq, iobuf); ++ ret = sec_put(state, sreq, iobuf); + } else if (ret == EOK) { +- ret = sec_update_b64(state, sreq, iobuf); ++ ret = sec_update(state, sreq, iobuf); + } + + if (ret != EOK) { +@@ -804,7 +767,7 @@ static struct tevent_req *ccdb_secdb_get_default_send(TALLOC_CTX *mem_ctx, + goto immediate; + } + +- ret = sec_get_b64(state, sreq, &dfl_iobuf); ++ ret = sec_get(state, sreq, &dfl_iobuf, NULL); + if (ret == ENOENT) { + uuid_clear(state->uuid); + ret = EOK; +@@ -1230,9 +1193,8 @@ static struct tevent_req *ccdb_secdb_create_send(TALLOC_CTX *mem_ctx, + goto immediate; + } + +- ret = sec_put_b64(state, ccache_req, ccache_payload); ++ ret = sec_put(state, ccache_req, ccache_payload); + if (ret != EOK) { +- DEBUG(SSSDBG_OP_FAILURE, "Failed to add the payload\n"); + goto immediate; + } + +@@ -1298,7 +1260,7 @@ static struct tevent_req *ccdb_secdb_mod_send(TALLOC_CTX *mem_ctx, + goto immediate; + } + +- ret = kcm_ccache_to_sec_input(state, cc, client, &payload); ++ ret = kcm_ccache_to_sec_input_binary(state, cc, &payload); + if (ret != EOK) { + goto immediate; + } +@@ -1308,7 +1270,7 @@ static struct tevent_req *ccdb_secdb_mod_send(TALLOC_CTX *mem_ctx, + goto immediate; + } + +- ret = sec_update_b64(state, sreq, payload); ++ ret = sec_update(state, sreq, payload); + if (ret != EOK) { + goto immediate; + } +@@ -1374,7 +1336,7 @@ static struct tevent_req *ccdb_secdb_store_cred_send(TALLOC_CTX *mem_ctx, + goto immediate; + } + +- ret = kcm_ccache_to_sec_input(state, cc, client, &payload); ++ ret = kcm_ccache_to_sec_input_binary(state, cc, &payload); + if (ret != EOK) { + goto immediate; + } +@@ -1384,7 +1346,7 @@ static struct tevent_req *ccdb_secdb_store_cred_send(TALLOC_CTX *mem_ctx, + goto immediate; + } + +- ret = sec_update_b64(state, sreq, payload); ++ ret = sec_update(state, sreq, payload); + if (ret != EOK) { + goto immediate; + } +diff --git a/src/responder/kcm/kcmsrv_ccache_secrets.c b/src/responder/kcm/kcmsrv_ccache_secrets.c +index 440ab3bb9..f3d69842c 100644 +--- a/src/responder/kcm/kcmsrv_ccache_secrets.c ++++ b/src/responder/kcm/kcmsrv_ccache_secrets.c +@@ -195,7 +195,7 @@ static errno_t kcm_ccache_to_sec_kv(TALLOC_CTX *mem_ctx, + goto done; + } + +- ret = kcm_ccache_to_sec_input(mem_ctx, cc, client, &payload); ++ ret = kcm_ccache_to_sec_input_json(mem_ctx, cc, &payload); + if (ret != EOK) { + goto done; + } +@@ -489,11 +489,8 @@ static void sec_get_done(struct tevent_req *subreq) + return; + } + +- ret = sec_kv_to_ccache(state, +- state->sec_key, +- sec_value, +- state->client, +- &state->cc); ++ ret = sec_kv_to_ccache_json(state, state->sec_key, sec_value, state->client, ++ &state->cc); + if (ret != EOK) { + DEBUG(SSSDBG_OP_FAILURE, + "Cannot convert JSON keyval to ccache blob [%d]: %s\n", +diff --git a/src/responder/kcm/kcmsrv_cmd.c b/src/responder/kcm/kcmsrv_cmd.c +index 421bf4bc5..a1aa9aa20 100644 +--- a/src/responder/kcm/kcmsrv_cmd.c ++++ b/src/responder/kcm/kcmsrv_cmd.c +@@ -314,7 +314,7 @@ static void kcm_reply_error(struct cli_ctx *cctx, + krb5_error_code kerr; + + DEBUG(SSSDBG_OP_FAILURE, +- "KCM operation returs failure [%d]: %s\n", ++ "KCM operation returns failure [%d]: %s\n", + retcode, sss_strerror(retcode)); + kerr = sss2krb5_error(retcode); + +@@ -373,13 +373,16 @@ static errno_t kcm_cmd_dispatch(struct kcm_ctx *kctx, + { + struct tevent_req *req; + struct cli_ctx *cctx; ++ struct kcm_conn_data *conn_data; + + cctx = req_ctx->cctx; ++ conn_data = talloc_get_type(cctx->state_ctx, struct kcm_conn_data); + + req = kcm_cmd_send(req_ctx, + cctx->ev, + kctx->qctx, + req_ctx->kctx->kcm_data, ++ conn_data, + req_ctx->cctx->creds, + &req_ctx->op_io.request, + req_ctx->op_io.op); +@@ -492,7 +495,7 @@ static void kcm_recv(struct cli_ctx *cctx) + int ret; + + kctx = talloc_get_type(cctx->rctx->pvt_ctx, struct kcm_ctx); +- req = talloc_get_type(cctx->state_ctx, struct kcm_req_ctx); ++ req = talloc_get_type(cctx->protocol_ctx, struct kcm_req_ctx); + if (req == NULL) { + /* A new request comes in, setup data structures. */ + req = kcm_new_req(cctx, kctx); +@@ -503,7 +506,17 @@ static void kcm_recv(struct cli_ctx *cctx) + return; + } + +- cctx->state_ctx = req; ++ cctx->protocol_ctx = req; ++ } ++ ++ /* Shared data between requests that originates in the same connection. */ ++ if (cctx->state_ctx == NULL) { ++ cctx->state_ctx = talloc_zero(cctx, struct kcm_conn_data); ++ if (cctx->state_ctx == NULL) { ++ DEBUG(SSSDBG_CRIT_FAILURE, "Cannot set up client state\n"); ++ talloc_free(cctx); ++ return; ++ } + } + + ret = kcm_recv_data(req, cctx->cfd, &req->reqbuf); +@@ -558,7 +571,7 @@ static int kcm_send_data(struct cli_ctx *cctx) + struct kcm_req_ctx *req; + errno_t ret; + +- req = talloc_get_type(cctx->state_ctx, struct kcm_req_ctx); ++ req = talloc_get_type(cctx->protocol_ctx, struct kcm_req_ctx); + + ret = kcm_write_iovec(cctx->cfd, &req->repbuf.v_len); + if (ret != EOK) { +@@ -604,7 +617,7 @@ static void kcm_send(struct cli_ctx *cctx) + DEBUG(SSSDBG_TRACE_INTERNAL, "All data sent!\n"); + TEVENT_FD_NOT_WRITEABLE(cctx->cfde); + TEVENT_FD_READABLE(cctx->cfde); +- talloc_zfree(cctx->state_ctx); ++ talloc_zfree(cctx->protocol_ctx); + return; + } + +diff --git a/src/responder/kcm/kcmsrv_ops.c b/src/responder/kcm/kcmsrv_ops.c +index 6ac66c150..f458c724b 100644 +--- a/src/responder/kcm/kcmsrv_ops.c ++++ b/src/responder/kcm/kcmsrv_ops.c +@@ -22,9 +22,11 @@ + #include "config.h" + + #include ++#include + + #include "util/sss_iobuf.h" + #include "util/sss_krb5.h" ++#include "util/sss_ptr_hash.h" + #include "util/util_creds.h" + #include "responder/kcm/kcm.h" + #include "responder/kcm/kcmsrv_pvt.h" +@@ -38,6 +40,7 @@ + + struct kcm_op_ctx { + struct kcm_resp_ctx *kcm_data; ++ struct kcm_conn_data *conn_data; + struct cli_creds *client; + + struct sss_iobuf *input; +@@ -86,6 +89,7 @@ struct tevent_req *kcm_cmd_send(TALLOC_CTX *mem_ctx, + struct tevent_context *ev, + struct kcm_ops_queue_ctx *qctx, + struct kcm_resp_ctx *kcm_data, ++ struct kcm_conn_data *conn_data, + struct cli_creds *client, + struct kcm_data *input, + struct kcm_op *op) +@@ -135,6 +139,7 @@ struct tevent_req *kcm_cmd_send(TALLOC_CTX *mem_ctx, + } + + state->op_ctx->kcm_data = kcm_data; ++ state->op_ctx->conn_data = conn_data; + state->op_ctx->client = client; + + state->op_ctx->input = sss_iobuf_init_readonly(state->op_ctx, +@@ -1071,8 +1076,75 @@ static void kcm_op_get_principal_getbyname_done(struct tevent_req *subreq) + tevent_req_done(req); + } + ++static void ++kcm_creds_table_delete_cb(hash_entry_t *item, ++ hash_destroy_enum deltype, ++ void *pvt) ++{ ++ /* Delete the old credential if it is being overwritten. */ ++ talloc_free(item->value.ptr); ++} ++ ++/* Store credentials in a hash table. ++ * ++ * If the table already exist we add the new credentials to the table and ++ * overwrite the ones that already exist. This allows us to correctly serve ++ * also parallel GET_CRED_UUID_LIST requests from the same connection since ++ * it will have its own uuid list and cursor on the client side and we make ++ * all uuid (old, updated and newly added) available. ++ */ ++static errno_t ++kcm_creds_to_table(TALLOC_CTX *mem_ctx, ++ struct kcm_cred *creds, ++ hash_table_t **_table) ++{ ++ char str[UUID_STR_SIZE]; ++ uuid_t uuid; ++ errno_t ret; ++ ++ if (*_table == NULL) { ++ *_table = sss_ptr_hash_create(mem_ctx, kcm_creds_table_delete_cb, NULL); ++ if (*_table == NULL) { ++ return ENOMEM; ++ } ++ } ++ ++ for (struct kcm_cred *crd = creds; ++ crd != NULL; ++ crd = kcm_cc_next_cred(crd)) { ++ ret = kcm_cred_get_uuid(crd, uuid); ++ if (ret != EOK) { ++ DEBUG(SSSDBG_MINOR_FAILURE, "Credential has no UUID, skipping\n"); ++ continue; ++ } ++ uuid_unparse(uuid, str); ++ ++ ret = sss_ptr_hash_add_or_override(*_table, str, crd, struct kcm_cred); ++ if (ret != EOK) { ++ return ret; ++ } ++ ++ talloc_steal(*_table, crd); ++ } ++ ++ return EOK; ++} ++ ++static struct kcm_cred * ++kcm_creds_lookup(hash_table_t *table, uuid_t uuid) ++{ ++ char str[UUID_STR_SIZE]; ++ ++ if (uuid == NULL) { ++ return NULL; ++ } ++ ++ uuid_unparse(uuid, str); ++ return sss_ptr_hash_lookup(table, str, struct kcm_cred); ++} ++ + /* (name) -> (uuid, ...) */ +-static void kcm_op_get_cred_uuid_getbyname_done(struct tevent_req *subreq); ++static void kcm_op_get_cred_uuid_list_getbyname_done(struct tevent_req *subreq); + + static struct tevent_req * + kcm_op_get_cred_uuid_list_send(TALLOC_CTX *mem_ctx, +@@ -1106,7 +1178,7 @@ kcm_op_get_cred_uuid_list_send(TALLOC_CTX *mem_ctx, + ret = ENOMEM; + goto immediate; + } +- tevent_req_set_callback(subreq, kcm_op_get_cred_uuid_getbyname_done, req); ++ tevent_req_set_callback(subreq, kcm_op_get_cred_uuid_list_getbyname_done, req); + return req; + + immediate: +@@ -1115,17 +1187,20 @@ immediate: + return req; + } + +-static void kcm_op_get_cred_uuid_getbyname_done(struct tevent_req *subreq) ++static void kcm_op_get_cred_uuid_list_getbyname_done(struct tevent_req *subreq) + { + errno_t ret; + struct kcm_ccache *cc; + struct kcm_cred *crd; ++ struct kcm_conn_data *conn_data; + uuid_t uuid; + 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); + ++ conn_data = state->op_ctx->conn_data; ++ + ret = kcm_ccdb_getbyname_recv(subreq, state, &cc); + talloc_zfree(subreq); + if (ret != EOK) { +@@ -1137,12 +1212,20 @@ static void kcm_op_get_cred_uuid_getbyname_done(struct tevent_req *subreq) + } + + if (cc == NULL) { +- DEBUG(SSSDBG_MINOR_FAILURE, "No credentials by that UUID\n"); ++ DEBUG(SSSDBG_MINOR_FAILURE, "No ccache by that name\n"); + state->op_ret = ERR_NO_CREDS; + tevent_req_done(req); + return; + } + ++ ret = kcm_creds_to_table(conn_data, kcm_cc_get_cred(cc), &conn_data->creds); ++ if (ret != EOK) { ++ DEBUG(SSSDBG_OP_FAILURE, "Unable to build credentials hash table " ++ "[%d]: %s\n", ret, sss_strerror(ret)); ++ tevent_req_error(req, ret); ++ return; ++ } ++ + for (crd = kcm_cc_get_cred(cc); + crd != NULL; + crd = kcm_cc_next_cred(crd)) { +@@ -1169,6 +1252,34 @@ static void kcm_op_get_cred_uuid_getbyname_done(struct tevent_req *subreq) + tevent_req_done(req); + } + ++static errno_t ++kcm_op_get_cred_by_uuid_reply(struct kcm_cred *crd, ++ struct sss_iobuf *reply) ++{ ++ struct sss_iobuf *cred_blob; ++ errno_t ret; ++ ++ cred_blob = kcm_cred_get_creds(crd); ++ if (cred_blob == NULL) { ++ DEBUG(SSSDBG_CRIT_FAILURE, "Credentials lack the creds blob\n"); ++ return ERR_NO_CREDS; ++ } ++ ++ ret = sss_iobuf_write_len(reply, sss_iobuf_get_data(cred_blob), ++ sss_iobuf_get_size(cred_blob)); ++ if (ret != EOK) { ++ DEBUG(SSSDBG_OP_FAILURE, "Cannot write ccache blob [%d]: %s\n", ++ ret, sss_strerror(ret)); ++ } ++ ++ return ret; ++} ++ ++struct kcm_op_get_cred_by_uuid_state { ++ struct kcm_op_common_state common; ++ uuid_t uuid; ++}; ++ + /* (name, uuid) -> (cred) */ + static void kcm_op_get_cred_by_uuid_getbyname_done(struct tevent_req *subreq); + +@@ -1179,20 +1290,51 @@ kcm_op_get_cred_by_uuid_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_get_cred_by_uuid_state *state; ++ struct kcm_cred *crd; + 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_get_cred_by_uuid_state); + if (req == NULL) { + return NULL; + } +- state->op_ctx = op_ctx; ++ state->common.op_ctx = op_ctx; + + ret = sss_iobuf_read_stringz(op_ctx->input, &name); + if (ret != EOK) { + goto immediate; + } ++ ++ ret = sss_iobuf_read_len(state->common.op_ctx->input, UUID_BYTES, ++ state->uuid); ++ if (ret != EOK) { ++ DEBUG(SSSDBG_OP_FAILURE, "Cannot read input UUID [%d]: %s\n", ++ ret, sss_strerror(ret)); ++ goto immediate; ++ } ++ ++ if (op_ctx->conn_data->creds != NULL) { ++ crd = kcm_creds_lookup(op_ctx->conn_data->creds, state->uuid); ++ if (crd == NULL) { ++ /* This should not happen, it can only happen if wrong UUID was ++ * requested which suggests bug in the caller application. */ ++ DEBUG(SSSDBG_MINOR_FAILURE, "No credentials by that UUID\n"); ++ kcm_debug_uuid(state->uuid); ++ state->common.op_ret = ERR_KCM_CC_END; ++ ret = EOK; ++ goto immediate; ++ } else { ++ ret = kcm_op_get_cred_by_uuid_reply(crd, op_ctx->reply); ++ if (ret == ERR_NO_CREDS) { ++ state->common.op_ret = ret; ++ ret = EOK; ++ } ++ goto immediate; ++ } ++ } ++ + DEBUG(SSSDBG_TRACE_LIBS, "Returning creds by UUID for %s\n", name); + + subreq = kcm_ccdb_getbyname_send(state, ev, +@@ -1207,7 +1349,11 @@ kcm_op_get_cred_by_uuid_send(TALLOC_CTX *mem_ctx, + return req; + + immediate: +- tevent_req_error(req, ret); ++ if (ret == EOK) { ++ tevent_req_done(req); ++ } else { ++ tevent_req_error(req, ret); ++ } + tevent_req_post(req, ev); + return req; + } +@@ -1216,14 +1362,14 @@ static void kcm_op_get_cred_by_uuid_getbyname_done(struct tevent_req *subreq) + { + 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_get_cred_by_uuid_state *state = tevent_req_data(req, ++ struct kcm_op_get_cred_by_uuid_state); + errno_t ret; + struct kcm_ccache *cc; + struct kcm_cred *crd; +- uuid_t uuid_in; +- uuid_t uuid; +- struct sss_iobuf *cred_blob; ++ struct kcm_conn_data *conn_data; ++ ++ conn_data = state->common.op_ctx->conn_data; + + ret = kcm_ccdb_getbyname_recv(subreq, state, &cc); + talloc_zfree(subreq); +@@ -1235,67 +1381,43 @@ static void kcm_op_get_cred_by_uuid_getbyname_done(struct tevent_req *subreq) + return; + } + +- if (cc == NULL) { +- DEBUG(SSSDBG_MINOR_FAILURE, "No credentials by that name\n"); +- state->op_ret = ERR_NO_MATCHING_CREDS; +- tevent_req_done(req); +- return; +- } +- +- ret = sss_iobuf_read_len(state->op_ctx->input, +- UUID_BYTES, uuid_in); ++ ret = kcm_creds_to_table(conn_data, kcm_cc_get_cred(cc), &conn_data->creds); + if (ret != EOK) { +- DEBUG(SSSDBG_OP_FAILURE, +- "Cannot read input UUID [%d]: %s\n", +- ret, sss_strerror(ret)); ++ DEBUG(SSSDBG_OP_FAILURE, "Unable to build credentials hash table " ++ "[%d]: %s\n", ret, sss_strerror(ret)); + tevent_req_error(req, ret); + return; + } + +- for (crd = kcm_cc_get_cred(cc); +- crd != NULL; +- crd = kcm_cc_next_cred(crd)) { +- ret = kcm_cred_get_uuid(crd, uuid); +- if (ret != EOK) { +- DEBUG(SSSDBG_MINOR_FAILURE, +- "Cannot get UUID from creds, skipping\n"); +- continue; ++ if (conn_data->creds != NULL) { ++ crd = kcm_creds_lookup(conn_data->creds, state->uuid); ++ if (crd == NULL) { ++ DEBUG(SSSDBG_MINOR_FAILURE, "No credentials by that UUID\n"); ++ kcm_debug_uuid(state->uuid); ++ state->common.op_ret = ERR_KCM_CC_END; ++ } else { ++ ret = kcm_op_get_cred_by_uuid_reply(crd, state->common.op_ctx->reply); ++ if (ret != EOK && ret != ERR_NO_CREDS) { ++ tevent_req_error(req, ret); ++ return; ++ } ++ state->common.op_ret = ret; + } +- +- if (uuid_compare(uuid, uuid_in) == 0) { +- break; +- } +- kcm_debug_uuid(uuid); + } + +- if (crd == NULL) { +- state->op_ret = ERR_KCM_CC_END; +- DEBUG(SSSDBG_MINOR_FAILURE, "No credentials by that UUID\n"); +- tevent_req_done(req); +- return; +- } ++ tevent_req_done(req); ++} + +- cred_blob = kcm_cred_get_creds(crd); +- if (cred_blob == NULL) { +- DEBUG(SSSDBG_CRIT_FAILURE, "Credentials lack the creds blob\n"); +- state->op_ret = ERR_NO_CREDS; +- tevent_req_done(req); +- return; +- } ++static errno_t kcm_op_get_cred_by_uuid_recv(struct tevent_req *req, ++ uint32_t *_op_ret) ++{ ++ struct kcm_op_get_cred_by_uuid_state *state; + +- ret = sss_iobuf_write_len(state->op_ctx->reply, +- sss_iobuf_get_data(cred_blob), +- sss_iobuf_get_size(cred_blob)); +- if (ret != EOK) { +- DEBUG(SSSDBG_OP_FAILURE, +- "Cannot write ccache blob [%d]: %s\n", +- ret, sss_strerror(ret)); +- tevent_req_error(req, ret); +- return; +- } ++ state = tevent_req_data(req, struct kcm_op_get_cred_by_uuid_state); + +- state->op_ret = EOK; +- tevent_req_done(req); ++ TEVENT_REQ_RETURN_ON_ERROR(req); ++ *_op_ret = state->common.op_ret; ++ return EOK; + } + + /* (name, flags, credtag) -> () */ +@@ -1468,7 +1590,7 @@ static void kcm_op_get_cache_by_uuid_done(struct tevent_req *subreq) + talloc_zfree(subreq); + if (ret != EOK) { + DEBUG(SSSDBG_OP_FAILURE, +- "Cannot get ccahe by UUID [%d]: %s\n", ++ "Cannot get ccache by UUID [%d]: %s\n", + ret, sss_strerror(ret)); + tevent_req_error(req, ret); + return; +@@ -2153,7 +2275,7 @@ static struct kcm_op kcm_optable[] = { + { "RETRIEVE", NULL, NULL }, + { "GET_PRINCIPAL", kcm_op_get_principal_send, NULL }, + { "GET_CRED_UUID_LIST", kcm_op_get_cred_uuid_list_send, NULL }, +- { "GET_CRED_BY_UUID", kcm_op_get_cred_by_uuid_send, NULL }, ++ { "GET_CRED_BY_UUID", kcm_op_get_cred_by_uuid_send, kcm_op_get_cred_by_uuid_recv }, + { "REMOVE_CRED", kcm_op_remove_cred_send, NULL }, + { "SET_FLAGS", NULL, NULL }, + { "CHOWN", NULL, NULL }, +diff --git a/src/responder/kcm/kcmsrv_ops.h b/src/responder/kcm/kcmsrv_ops.h +index 67d9f8602..ab6c13791 100644 +--- a/src/responder/kcm/kcmsrv_ops.h ++++ b/src/responder/kcm/kcmsrv_ops.h +@@ -24,6 +24,7 @@ + + #include "config.h" + ++#include + #include + #include "util/sss_iobuf.h" + #include "responder/kcm/kcmsrv_pvt.h" +@@ -32,10 +33,17 @@ struct kcm_op; + struct kcm_op *kcm_get_opt(uint16_t opcode); + const char *kcm_opt_name(struct kcm_op *op); + ++struct kcm_conn_data { ++ /* Credentials obtained by GET_CRED_UUID_LIST. We use to improve performance ++ * by avoiding ccache lookups in GET_CRED_BY_UUID. */ ++ hash_table_t *creds; ++}; ++ + struct tevent_req *kcm_cmd_send(TALLOC_CTX *mem_ctx, + struct tevent_context *ev, + struct kcm_ops_queue_ctx *qctx, + struct kcm_resp_ctx *kcm_data, ++ struct kcm_conn_data *conn_data, + struct cli_creds *client, + struct kcm_data *input, + struct kcm_op *op); +diff --git a/src/responder/secrets/local.c b/src/responder/secrets/local.c +index eb37c08b7..252ef3a1d 100644 +--- a/src/responder/secrets/local.c ++++ b/src/responder/secrets/local.c +@@ -134,7 +134,7 @@ static struct tevent_req *local_secret_req(TALLOC_CTX *mem_ctx, + break; + } + +- ret = sss_sec_get(state, ssec_req, &secret); ++ ret = sss_sec_get(state, ssec_req, (uint8_t**)&secret, NULL, NULL); + if (ret) goto done; + + if (body_is_json) { +@@ -168,7 +168,8 @@ static struct tevent_req *local_secret_req(TALLOC_CTX *mem_ctx, + } + if (ret) goto done; + +- ret = sss_sec_put(ssec_req, secret); ++ ret = sss_sec_put(ssec_req, (uint8_t *)secret, strlen(secret) + 1, ++ SSS_SEC_MASTERKEY, "simple"); + if (ret) goto done; + break; + +diff --git a/src/shared/safealign.h b/src/shared/safealign.h +index b00c37f5b..35909faa2 100644 +--- a/src/shared/safealign.h ++++ b/src/shared/safealign.h +@@ -97,6 +97,10 @@ safealign_memcpy(void *dest, const void *src, size_t n, size_t *counter) + #define SAFEALIGN_SETMEM_UINT16(dest, value, pctr) \ + SAFEALIGN_SETMEM_VALUE(dest, value, uint16_t, pctr) + ++/* SAFEALIGN_SETMEM_UINT8(void *dest, uint8_t value, size_t *pctr) */ ++#define SAFEALIGN_SETMEM_UINT8(dest, value, pctr) \ ++ SAFEALIGN_SETMEM_VALUE(dest, value, uint8_t, pctr) ++ + /* These macros are the same as their equivalents without _CHECK suffix, + * but additionally make the caller return EINVAL immediately if *pctr + * would exceed len. */ +diff --git a/src/tests/cmocka/test_kcm_json_marshalling.c b/src/tests/cmocka/test_kcm_marshalling.c +similarity index 71% +rename from src/tests/cmocka/test_kcm_json_marshalling.c +rename to src/tests/cmocka/test_kcm_marshalling.c +index 48ee92bd6..cebebac80 100644 +--- a/src/tests/cmocka/test_kcm_json_marshalling.c ++++ b/src/tests/cmocka/test_kcm_marshalling.c +@@ -154,7 +154,7 @@ static void assert_cc_equal(struct kcm_ccache *cc1, + assert_cc_offset_equal(cc1, cc2); + } + +-static void test_kcm_ccache_marshall_unmarshall(void **state) ++static void test_kcm_ccache_marshall_unmarshall_json(void **state) + { + struct kcm_marshalling_test_ctx *test_ctx = talloc_get_type(*state, + struct kcm_marshalling_test_ctx); +@@ -182,10 +182,7 @@ static void test_kcm_ccache_marshall_unmarshall(void **state) + &cc); + assert_int_equal(ret, EOK); + +- ret = kcm_ccache_to_sec_input(test_ctx, +- cc, +- &owner, +- &payload); ++ ret = kcm_ccache_to_sec_input_json(test_ctx, cc, &payload); + assert_int_equal(ret, EOK); + + data = sss_iobuf_get_data(payload); +@@ -196,25 +193,19 @@ static void test_kcm_ccache_marshall_unmarshall(void **state) + 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); ++ ret = sec_kv_to_ccache_json(test_ctx, key, (const char *)data, &owner, ++ &cc2); + assert_int_equal(ret, EOK); + + assert_cc_equal(cc, cc2); + + /* This key is exactly one byte shorter than it should be */ +- ret = sec_kv_to_ccache(test_ctx, +- TEST_UUID_STR"-", +- (const char *) data, +- &owner, +- &cc2); ++ ret = sec_kv_to_ccache_json(test_ctx, TEST_UUID_STR "-", (const char *)data, ++ &owner, &cc2); + assert_int_equal(ret, EINVAL); + } + +-static void test_kcm_ccache_no_princ(void **state) ++static void test_kcm_ccache_no_princ_json(void **state) + { + struct kcm_marshalling_test_ctx *test_ctx = talloc_get_type(*state, + struct kcm_marshalling_test_ctx); +@@ -246,10 +237,7 @@ static void test_kcm_ccache_no_princ(void **state) + princ = kcm_cc_get_client_principal(cc); + assert_null(princ); + +- ret = kcm_ccache_to_sec_input(test_ctx, +- cc, +- &owner, +- &payload); ++ ret = kcm_ccache_to_sec_input_json(test_ctx, cc, &payload); + assert_int_equal(ret, EOK); + + data = sss_iobuf_get_data(payload); +@@ -260,11 +248,110 @@ static void test_kcm_ccache_no_princ(void **state) + 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); ++ ret = sec_kv_to_ccache_json(test_ctx, key, (const char *)data, &owner, ++ &cc2); ++ assert_int_equal(ret, EOK); ++ ++ assert_cc_equal(cc, cc2); ++} ++ ++static void test_kcm_ccache_marshall_unmarshall_binary(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; ++ struct kcm_ccache *cc; ++ struct kcm_ccache *cc2; ++ struct sss_iobuf *payload; ++ const char *name; ++ 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, ++ test_ctx->princ, ++ &cc); ++ assert_int_equal(ret, EOK); ++ ++ ret = kcm_ccache_to_sec_input_binary(test_ctx, cc, &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); ++ ++ sss_iobuf_cursor_reset(payload); ++ ret = sec_kv_to_ccache_binary(test_ctx, key, payload, &owner, &cc2); ++ assert_int_equal(ret, EOK); ++ ++ assert_cc_equal(cc, cc2); ++ ++ /* This key is exactly one byte shorter than it should be */ ++ sss_iobuf_cursor_reset(payload); ++ ret = sec_kv_to_ccache_binary(test_ctx, TEST_UUID_STR "-", payload, &owner, ++ &cc2); ++ assert_int_equal(ret, EINVAL); ++} ++ ++static void test_kcm_ccache_no_princ_binary(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_binary(test_ctx, cc, &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); ++ ++ sss_iobuf_cursor_reset(payload); ++ ret = sec_kv_to_ccache_binary(test_ctx, key, payload, &owner, &cc2); + assert_int_equal(ret, EOK); + + assert_cc_equal(cc, cc2); +@@ -340,10 +427,16 @@ int main(int argc, const char *argv[]) + }; + + const struct CMUnitTest tests[] = { +- cmocka_unit_test_setup_teardown(test_kcm_ccache_marshall_unmarshall, ++ cmocka_unit_test_setup_teardown(test_kcm_ccache_marshall_unmarshall_binary, ++ setup_kcm_marshalling, ++ teardown_kcm_marshalling), ++ cmocka_unit_test_setup_teardown(test_kcm_ccache_no_princ_binary, ++ setup_kcm_marshalling, ++ teardown_kcm_marshalling), ++ cmocka_unit_test_setup_teardown(test_kcm_ccache_marshall_unmarshall_json, + setup_kcm_marshalling, + teardown_kcm_marshalling), +- cmocka_unit_test_setup_teardown(test_kcm_ccache_no_princ, ++ cmocka_unit_test_setup_teardown(test_kcm_ccache_no_princ_json, + setup_kcm_marshalling, + teardown_kcm_marshalling), + cmocka_unit_test(test_sec_key_get_uuid), +diff --git a/src/tests/cmocka/test_sss_ptr_hash.c b/src/tests/cmocka/test_sss_ptr_hash.c +index 1458238f5..31cf8b705 100644 +--- a/src/tests/cmocka/test_sss_ptr_hash.c ++++ b/src/tests/cmocka/test_sss_ptr_hash.c +@@ -91,6 +91,45 @@ void test_sss_ptr_hash_with_free_cb(void **state) + assert_int_equal(free_counter, MAX_ENTRIES_AMOUNT*2); + } + ++void test_sss_ptr_hash_overwrite_with_free_cb(void **state) ++{ ++ hash_table_t *table; ++ int free_counter = 0; ++ unsigned long count; ++ char *payload; ++ char *value; ++ errno_t ret; ++ ++ table = sss_ptr_hash_create(global_talloc_context, ++ free_payload_cb, ++ &free_counter); ++ assert_non_null(table); ++ ++ payload = talloc_strdup(table, "test_value1"); ++ assert_non_null(payload); ++ talloc_set_name_const(payload, "char"); ++ ret = sss_ptr_hash_add_or_override(table, "test", payload, char); ++ assert_int_equal(ret, 0); ++ count = hash_count(table); ++ assert_int_equal(count, 1); ++ value = sss_ptr_hash_lookup(table, "test", char); ++ assert_ptr_equal(value, payload); ++ ++ ++ payload = talloc_strdup(table, "test_value2"); ++ assert_non_null(payload); ++ talloc_set_name_const(payload, "char"); ++ ret = sss_ptr_hash_add_or_override(table, "test", payload, char); ++ assert_int_equal(ret, 0); ++ count = hash_count(table); ++ assert_int_equal(count, 1); ++ value = sss_ptr_hash_lookup(table, "test", char); ++ assert_ptr_equal(value, payload); ++ ++ talloc_free(table); ++ assert_int_equal(free_counter, 2); ++} ++ + struct table_wrapper + { + hash_table_t **table; +diff --git a/src/tests/cmocka/test_utils.c b/src/tests/cmocka/test_utils.c +index d77a972c1..d258622fb 100644 +--- a/src/tests/cmocka/test_utils.c ++++ b/src/tests/cmocka/test_utils.c +@@ -2144,6 +2144,9 @@ int main(int argc, const char *argv[]) + cmocka_unit_test_setup_teardown(test_sss_ptr_hash_with_free_cb, + setup_leak_tests, + teardown_leak_tests), ++ cmocka_unit_test_setup_teardown(test_sss_ptr_hash_overwrite_with_free_cb, ++ setup_leak_tests, ++ teardown_leak_tests), + cmocka_unit_test_setup_teardown(test_sss_ptr_hash_with_lookup_cb, + setup_leak_tests, + teardown_leak_tests), +diff --git a/src/tests/cmocka/test_utils.h b/src/tests/cmocka/test_utils.h +index 44b9479f9..458bcb750 100644 +--- a/src/tests/cmocka/test_utils.h ++++ b/src/tests/cmocka/test_utils.h +@@ -35,6 +35,7 @@ void test_concatenate_string_array(void **state); + + /* from src/tests/cmocka/test_sss_ptr_hash.c */ + void test_sss_ptr_hash_with_free_cb(void **state); ++void test_sss_ptr_hash_overwrite_with_free_cb(void **state); + void test_sss_ptr_hash_with_lookup_cb(void **state); + void test_sss_ptr_hash_without_cb(void **state); + +diff --git a/src/tests/intg/test_secrets.py b/src/tests/intg/test_secrets.py +index 00933fb34..18d722c13 100644 +--- a/src/tests/intg/test_secrets.py ++++ b/src/tests/intg/test_secrets.py +@@ -438,7 +438,8 @@ def run_quota_test(cli, max_secrets, max_payload_size): + KILOBYTE = 1024 + kb_payload_size = max_payload_size * KILOBYTE + +- sec_value = "x" * kb_payload_size ++ # Adjust payload size to hold terminal zero byte. ++ sec_value = "x" * (kb_payload_size - 1) + + cli.set_secret("foo", sec_value) + +diff --git a/src/tests/multihost/basic/test_kcm.py b/src/tests/multihost/basic/test_kcm.py +index e5d315827..6f65431f8 100644 +--- a/src/tests/multihost/basic/test_kcm.py ++++ b/src/tests/multihost/basic/test_kcm.py +@@ -310,6 +310,12 @@ class TestSanityKCM(object): + set_param(multihost, 'kcm', 'max_ccache_size', '1') + self._restart_kcm(multihost) + +- with pytest.raises(paramiko.ssh_exception.AuthenticationException): +- ssh_foo3 = SSHClient(multihost.master[0].sys_hostname, +- username='foo3', password='Secret123') ++ # We use kinit to exceed the maximum ccache size as it creates payload ++ # of 1280 bytes by acquiring tgt and also some control credentials. ++ # SSH authentication is not sufficient as it stores only tgt. ++ ssh_foo3 = SSHClient(multihost.master[0].sys_hostname, ++ username='foo3', password='Secret123') ++ (_, _, exit_status) = ssh_foo3.execute_cmd( ++ 'kinit foo3@EXAMPLE.TEST', 'Secret123' ++ ) ++ assert exit_status != 0 +diff --git a/src/util/secrets/sec_pvt.h b/src/util/secrets/sec_pvt.h +index 92e2b8b25..0e77a660e 100644 +--- a/src/util/secrets/sec_pvt.h ++++ b/src/util/secrets/sec_pvt.h +@@ -33,7 +33,7 @@ + #define SSS_SEC_KCM_BASEPATH "/kcm/" + + struct sss_sec_data { +- char *data; ++ uint8_t *data; + size_t length; + }; + +diff --git a/src/util/secrets/secrets.c b/src/util/secrets/secrets.c +index d701face0..c6310b585 100644 +--- a/src/util/secrets/secrets.c ++++ b/src/util/secrets/secrets.c +@@ -36,9 +36,14 @@ + #define SECRETS_BASEDN "cn=secrets" + #define KCM_BASEDN "cn=kcm" + +-#define LOCAL_SIMPLE_FILTER "(type=simple)" ++#define LOCAL_SIMPLE_FILTER "(|(type=simple)(type=binary))" + #define LOCAL_CONTAINER_FILTER "(type=container)" + ++#define SEC_ATTR_SECRET "secret" ++#define SEC_ATTR_ENCTYPE "enctype" ++#define SEC_ATTR_TYPE "type" ++#define SEC_ATTR_CTIME "creationTime" ++ + typedef int (*url_mapper_fn)(TALLOC_CTX *mem_ctx, + const char *url, + uid_t client, +@@ -63,90 +68,136 @@ static struct sss_sec_quota default_kcm_quota = { + .containers_nest_level = DEFAULT_SEC_CONTAINERS_NEST_LEVEL, + }; + +-static int local_decrypt(struct sss_sec_ctx *sctx, TALLOC_CTX *mem_ctx, +- const char *secret, const char *enctype, +- char **plain_secret) ++static const char *sss_sec_enctype_to_str(enum sss_sec_enctype enctype) + { +- char *output; ++ switch (enctype) { ++ case SSS_SEC_PLAINTEXT: ++ return "plaintext"; ++ case SSS_SEC_MASTERKEY: ++ return "masterkey"; ++ default: ++ DEBUG(SSSDBG_CRIT_FAILURE, "Bug: unknown encryption type %d\n", ++ enctype); ++ return "unknown"; ++ } ++} + +- if (enctype && strcmp(enctype, "masterkey") == 0) { +- DEBUG(SSSDBG_TRACE_INTERNAL, "Decrypting with masterkey\n"); ++static enum sss_sec_enctype sss_sec_str_to_enctype(const char *str) ++{ ++ if (strcmp("plaintext", str) == 0) { ++ return SSS_SEC_PLAINTEXT; ++ } + +- struct sss_sec_data _secret; +- size_t outlen; +- int ret; ++ if (strcmp("masterkey", str) == 0) { ++ return SSS_SEC_MASTERKEY; ++ } ++ ++ return SSS_SEC_ENCTYPE_SENTINEL; ++} + +- _secret.data = (char *)sss_base64_decode(mem_ctx, secret, +- &_secret.length); ++static int local_decrypt(struct sss_sec_ctx *sctx, ++ TALLOC_CTX *mem_ctx, ++ uint8_t *secret, ++ size_t secret_len, ++ enum sss_sec_enctype enctype, ++ uint8_t **_output, ++ size_t *_output_len) ++{ ++ struct sss_sec_data _secret; ++ uint8_t *output; ++ size_t output_len; ++ int ret; ++ ++ switch (enctype) { ++ case SSS_SEC_PLAINTEXT: ++ output = talloc_memdup(mem_ctx, secret, secret_len); ++ output_len = secret_len; ++ break; ++ case SSS_SEC_MASTERKEY: ++ _secret.data = (uint8_t *)sss_base64_decode(mem_ctx, ++ (const char *)secret, ++ &_secret.length); + if (!_secret.data) { + DEBUG(SSSDBG_OP_FAILURE, "sss_base64_decode failed\n"); + return EINVAL; + } + ++ DEBUG(SSSDBG_TRACE_INTERNAL, "Decrypting with masterkey\n"); + ret = sss_decrypt(mem_ctx, AES256CBC_HMAC_SHA256, +- (uint8_t *)sctx->master_key.data, ++ sctx->master_key.data, + sctx->master_key.length, +- (uint8_t *)_secret.data, _secret.length, +- (uint8_t **)&output, &outlen); ++ _secret.data, _secret.length, ++ &output, &output_len); + talloc_free(_secret.data); + if (ret) { + DEBUG(SSSDBG_OP_FAILURE, + "sss_decrypt failed [%d]: %s\n", ret, sss_strerror(ret)); + return ret; + } ++ break; ++ default: ++ DEBUG(SSSDBG_CRIT_FAILURE, "Unknown encryption type '%d'\n", enctype); ++ return EINVAL; ++ } + +- if (((strnlen(output, outlen) + 1) != outlen) || +- output[outlen - 1] != '\0') { +- DEBUG(SSSDBG_CRIT_FAILURE, +- "Output length mismatch or output not NULL-terminated\n"); +- talloc_free(output); +- return EIO; +- } +- } else { +- DEBUG(SSSDBG_TRACE_INTERNAL, "Unexpected enctype (not 'masterkey')\n"); +- output = talloc_strdup(mem_ctx, secret); +- if (!output) return ENOMEM; ++ if (output == NULL) { ++ return ENOMEM; + } + +- *plain_secret = output; ++ *_output = output; ++ *_output_len = output_len; ++ + return EOK; + } + +-static int local_encrypt(struct sss_sec_ctx *sec_ctx, TALLOC_CTX *mem_ctx, +- const char *secret, const char *enctype, +- char **ciphertext) ++static int local_encrypt(struct sss_sec_ctx *sec_ctx, ++ TALLOC_CTX *mem_ctx, ++ uint8_t *secret, ++ size_t secret_len, ++ enum sss_sec_enctype enctype, ++ uint8_t **_output, ++ size_t *_output_len) + { + struct sss_sec_data _secret; +- char *output; ++ uint8_t *output; ++ size_t output_len; ++ char *b64; + int ret; + +- if (enctype == NULL) { +- DEBUG(SSSDBG_CRIT_FAILURE, "No encryption type\n"); +- return EINVAL; +- } ++ switch (enctype) { ++ case SSS_SEC_PLAINTEXT: ++ output = talloc_memdup(mem_ctx, secret, secret_len); ++ output_len = secret_len; ++ break; ++ case SSS_SEC_MASTERKEY: ++ ret = sss_encrypt(mem_ctx, AES256CBC_HMAC_SHA256, ++ sec_ctx->master_key.data, ++ sec_ctx->master_key.length, ++ secret, secret_len, ++ &_secret.data, &_secret.length); ++ if (ret) { ++ DEBUG(SSSDBG_OP_FAILURE, ++ "sss_encrypt failed [%d]: %s\n", ret, sss_strerror(ret)); ++ return ret; ++ } + +- if (strcmp(enctype, "masterkey") != 0) { +- DEBUG(SSSDBG_CRIT_FAILURE, "Unknown encryption type '%s'\n", enctype); ++ b64 = sss_base64_encode(mem_ctx, _secret.data, _secret.length); ++ output = (uint8_t*)b64; ++ output_len = strlen(b64) + 1; ++ talloc_free(_secret.data); ++ break; ++ default: ++ DEBUG(SSSDBG_CRIT_FAILURE, "Unknown encryption type '%d'\n", enctype); + return EINVAL; + } + +- ret = sss_encrypt(mem_ctx, AES256CBC_HMAC_SHA256, +- (uint8_t *)sec_ctx->master_key.data, +- sec_ctx->master_key.length, +- (const uint8_t *)secret, strlen(secret) + 1, +- (uint8_t **)&_secret.data, &_secret.length); +- if (ret) { +- DEBUG(SSSDBG_OP_FAILURE, +- "sss_encrypt failed [%d]: %s\n", ret, sss_strerror(ret)); +- return ret; ++ if (output == NULL) { ++ return ENOMEM; + } + +- output = sss_base64_encode(mem_ctx, +- (uint8_t *)_secret.data, _secret.length); +- talloc_free(_secret.data); +- if (!output) return ENOMEM; ++ *_output = output; ++ *_output_len = output_len; + +- *ciphertext = output; + return EOK; + } + +@@ -338,14 +389,14 @@ static int local_check_max_payload_size(struct sss_sec_req *req, + return EOK; + } + +- max_payload_size = req->quota->max_payload_size * 1024; /* kb */ ++ max_payload_size = req->quota->max_payload_size * 1024; /* KiB */ + if (payload_size > max_payload_size) { + DEBUG(SSSDBG_OP_FAILURE, +- "Secrets' payload size [%d kb (%d)] exceeds the maximum allowed " +- "payload size [%d kb (%d)]\n", +- payload_size * 1024, /* kb */ ++ "Secrets' payload size [%d KiB (%d B)] exceeds the maximum " ++ "allowed payload size [%d KiB (%d B)]\n", ++ payload_size / 1024, /* KiB */ + payload_size, +- req->quota->max_payload_size, /* kb */ ++ req->quota->max_payload_size, /* KiB */ + max_payload_size); + + return ERR_SEC_PAYLOAD_SIZE_IS_TOO_LARGE; +@@ -404,7 +455,7 @@ static int local_db_create(struct sss_sec_req *req) + ret = local_db_check_containers_nest_level(req, msg->dn); + if (ret != EOK) goto done; + +- ret = ldb_msg_add_string(msg, "type", "container"); ++ ret = ldb_msg_add_string(msg, SEC_ATTR_TYPE, "container"); + if (ret != EOK) { + DEBUG(SSSDBG_OP_FAILURE, + "ldb_msg_add_string failed adding type:container [%d]: %s\n", +@@ -412,7 +463,7 @@ static int local_db_create(struct sss_sec_req *req) + goto done; + } + +- ret = ldb_msg_add_fmt(msg, "creationTime", "%lu", time(NULL)); ++ ret = ldb_msg_add_fmt(msg, SEC_ATTR_CTIME, "%lu", time(NULL)); + if (ret != EOK) { + DEBUG(SSSDBG_OP_FAILURE, + "ldb_msg_add_string failed adding creationTime [%d]: %s\n", +@@ -892,7 +943,7 @@ errno_t sss_sec_list(TALLOC_CTX *mem_ctx, + size_t *_num_keys) + { + TALLOC_CTX *tmp_ctx; +- static const char *attrs[] = { "secret", NULL }; ++ static const char *attrs[] = { SEC_ATTR_SECRET, NULL }; + struct ldb_result *res; + char **keys; + int ret; +@@ -951,13 +1002,21 @@ done: + + errno_t sss_sec_get(TALLOC_CTX *mem_ctx, + struct sss_sec_req *req, +- char **_secret) ++ uint8_t **_secret, ++ size_t *_secret_len, ++ char **_datatype) + { + TALLOC_CTX *tmp_ctx; +- static const char *attrs[] = { "secret", "enctype", NULL }; ++ static const char *attrs[] = { SEC_ATTR_SECRET, SEC_ATTR_ENCTYPE, ++ SEC_ATTR_TYPE, NULL }; + struct ldb_result *res; +- const char *attr_secret; ++ const struct ldb_val *attr_secret; + const char *attr_enctype; ++ const char *attr_datatype; ++ enum sss_sec_enctype enctype; ++ char *datatype; ++ uint8_t *secret; ++ size_t secret_len; + int ret; + + if (req == NULL || _secret == NULL) { +@@ -996,21 +1055,38 @@ errno_t sss_sec_get(TALLOC_CTX *mem_ctx, + goto done; + } + +- attr_secret = ldb_msg_find_attr_as_string(res->msgs[0], "secret", NULL); ++ attr_secret = ldb_msg_find_ldb_val(res->msgs[0], SEC_ATTR_SECRET); + if (!attr_secret) { + DEBUG(SSSDBG_CRIT_FAILURE, "The 'secret' attribute is missing\n"); + ret = ENOENT; + goto done; + } + +- attr_enctype = ldb_msg_find_attr_as_string(res->msgs[0], "enctype", NULL); ++ attr_enctype = ldb_msg_find_attr_as_string(res->msgs[0], SEC_ATTR_ENCTYPE, ++ "plaintext"); ++ enctype = sss_sec_str_to_enctype(attr_enctype); ++ ret = local_decrypt(req->sctx, tmp_ctx, attr_secret->data, ++ attr_secret->length, enctype, &secret, &secret_len); ++ if (ret) goto done; + +- if (attr_enctype) { +- ret = local_decrypt(req->sctx, mem_ctx, attr_secret, attr_enctype, _secret); +- if (ret) goto done; +- } else { +- *_secret = talloc_strdup(mem_ctx, attr_secret); ++ if (_datatype != NULL) { ++ attr_datatype = ldb_msg_find_attr_as_string(res->msgs[0], SEC_ATTR_TYPE, ++ "simple"); ++ datatype = talloc_strdup(tmp_ctx, attr_datatype); ++ if (datatype == NULL) { ++ ret = ENOMEM; ++ goto done; ++ } ++ ++ *_datatype = talloc_steal(mem_ctx, datatype); + } ++ ++ *_secret = talloc_steal(mem_ctx, secret); ++ ++ if (_secret_len) { ++ *_secret_len = secret_len; ++ } ++ + ret = EOK; + + done: +@@ -1019,11 +1095,13 @@ done: + } + + errno_t sss_sec_put(struct sss_sec_req *req, +- const char *secret) ++ uint8_t *secret, ++ size_t secret_len, ++ enum sss_sec_enctype enctype, ++ const char *datatype) + { + struct ldb_message *msg; +- const char *enctype = "masterkey"; +- char *enc_secret; ++ struct ldb_val enc_secret; + int ret; + + if (req == NULL || secret == NULL) { +@@ -1064,7 +1142,7 @@ errno_t sss_sec_put(struct sss_sec_req *req, + goto done; + } + +- ret = local_check_max_payload_size(req, strlen(secret)); ++ ret = local_check_max_payload_size(req, secret_len); + if (ret != EOK) { + DEBUG(SSSDBG_OP_FAILURE, + "local_check_max_payload_size failed [%d]: %s\n", +@@ -1072,22 +1150,24 @@ errno_t sss_sec_put(struct sss_sec_req *req, + goto done; + } + +- ret = local_encrypt(req->sctx, msg, secret, enctype, &enc_secret); ++ ret = local_encrypt(req->sctx, msg, secret, secret_len, enctype, ++ &enc_secret.data, &enc_secret.length); + if (ret != EOK) { + DEBUG(SSSDBG_OP_FAILURE, + "local_encrypt failed [%d]: %s\n", ret, sss_strerror(ret)); + goto done; + } + +- ret = ldb_msg_add_string(msg, "type", "simple"); ++ ret = ldb_msg_add_string(msg, SEC_ATTR_TYPE, datatype); + if (ret != EOK) { + DEBUG(SSSDBG_OP_FAILURE, +- "ldb_msg_add_string failed adding type:simple [%d]: %s\n", +- ret, sss_strerror(ret)); ++ "ldb_msg_add_string failed adding type:%s [%d]: %s\n", ++ datatype, ret, sss_strerror(ret)); + goto done; + } + +- ret = ldb_msg_add_string(msg, "enctype", enctype); ++ ret = ldb_msg_add_string(msg, SEC_ATTR_ENCTYPE, ++ sss_sec_enctype_to_str(enctype)); + if (ret != EOK) { + DEBUG(SSSDBG_OP_FAILURE, + "ldb_msg_add_string failed adding enctype [%d]: %s\n", +@@ -1095,7 +1175,7 @@ errno_t sss_sec_put(struct sss_sec_req *req, + goto done; + } + +- ret = ldb_msg_add_string(msg, "secret", enc_secret); ++ ret = ldb_msg_add_value(msg, SEC_ATTR_SECRET, &enc_secret, NULL); + if (ret != EOK) { + DEBUG(SSSDBG_OP_FAILURE, + "ldb_msg_add_string failed adding secret [%d]: %s\n", +@@ -1103,7 +1183,7 @@ errno_t sss_sec_put(struct sss_sec_req *req, + goto done; + } + +- ret = ldb_msg_add_fmt(msg, "creationTime", "%lu", time(NULL)); ++ ret = ldb_msg_add_fmt(msg, SEC_ATTR_CTIME, "%lu", time(NULL)); + if (ret != EOK) { + DEBUG(SSSDBG_OP_FAILURE, + "ldb_msg_add_string failed adding creationTime [%d]: %s\n", +@@ -1132,11 +1212,13 @@ done: + } + + errno_t sss_sec_update(struct sss_sec_req *req, +- const char *secret) ++ uint8_t *secret, ++ size_t secret_len, ++ enum sss_sec_enctype enctype, ++ const char *datatype) + { + struct ldb_message *msg; +- const char *enctype = "masterkey"; +- char *enc_secret; ++ struct ldb_val enc_secret; + int ret; + + if (req == NULL || secret == NULL) { +@@ -1177,7 +1259,7 @@ errno_t sss_sec_update(struct sss_sec_req *req, + goto done; + } + +- ret = local_check_max_payload_size(req, strlen(secret)); ++ ret = local_check_max_payload_size(req, secret_len); + if (ret != EOK) { + DEBUG(SSSDBG_OP_FAILURE, + "local_check_max_payload_size failed [%d]: %s\n", +@@ -1185,15 +1267,49 @@ errno_t sss_sec_update(struct sss_sec_req *req, + goto done; + } + +- ret = local_encrypt(req->sctx, msg, secret, enctype, &enc_secret); ++ ret = local_encrypt(req->sctx, msg, secret, secret_len, enctype, ++ &enc_secret.data, &enc_secret.length); + if (ret != EOK) { + DEBUG(SSSDBG_OP_FAILURE, + "local_encrypt failed [%d]: %s\n", ret, sss_strerror(ret)); + goto done; + } + ++ ret = ldb_msg_add_empty(msg, SEC_ATTR_ENCTYPE, LDB_FLAG_MOD_REPLACE, NULL); ++ if (ret != LDB_SUCCESS) { ++ DEBUG(SSSDBG_MINOR_FAILURE, ++ "ldb_msg_add_empty failed: [%s]\n", ldb_strerror(ret)); ++ ret = EIO; ++ goto done; ++ } ++ ++ ret = ldb_msg_add_string(msg, SEC_ATTR_ENCTYPE, ++ sss_sec_enctype_to_str(enctype)); ++ if (ret != EOK) { ++ DEBUG(SSSDBG_OP_FAILURE, ++ "ldb_msg_add_string failed adding enctype [%d]: %s\n", ++ ret, sss_strerror(ret)); ++ goto done; ++ } ++ ++ ret = ldb_msg_add_empty(msg, SEC_ATTR_TYPE, LDB_FLAG_MOD_REPLACE, NULL); ++ if (ret != LDB_SUCCESS) { ++ DEBUG(SSSDBG_MINOR_FAILURE, ++ "ldb_msg_add_empty failed: [%s]\n", ldb_strerror(ret)); ++ ret = EIO; ++ goto done; ++ } ++ ++ ret = ldb_msg_add_string(msg, SEC_ATTR_TYPE, datatype); ++ if (ret != EOK) { ++ DEBUG(SSSDBG_OP_FAILURE, ++ "ldb_msg_add_string failed adding type:%s [%d]: %s\n", ++ datatype, ret, sss_strerror(ret)); ++ goto done; ++ } ++ + /* FIXME - should we have a lastUpdate timestamp? */ +- ret = ldb_msg_add_empty(msg, "secret", LDB_FLAG_MOD_REPLACE, NULL); ++ ret = ldb_msg_add_empty(msg, SEC_ATTR_SECRET, LDB_FLAG_MOD_REPLACE, NULL); + if (ret != LDB_SUCCESS) { + DEBUG(SSSDBG_MINOR_FAILURE, + "ldb_msg_add_empty failed: [%s]\n", ldb_strerror(ret)); +@@ -1201,7 +1317,7 @@ errno_t sss_sec_update(struct sss_sec_req *req, + goto done; + } + +- ret = ldb_msg_add_string(msg, "secret", enc_secret); ++ ret = ldb_msg_add_value(msg, SEC_ATTR_SECRET, &enc_secret, NULL); + if (ret != LDB_SUCCESS) { + DEBUG(SSSDBG_MINOR_FAILURE, + "ldb_msg_add_string failed: [%s]\n", ldb_strerror(ret)); +diff --git a/src/util/secrets/secrets.h b/src/util/secrets/secrets.h +index 9cf397516..f79bfaa4b 100644 +--- a/src/util/secrets/secrets.h ++++ b/src/util/secrets/secrets.h +@@ -43,6 +43,12 @@ + #define DEFAULT_SEC_KCM_MAX_UID_SECRETS 64 + #define DEFAULT_SEC_KCM_MAX_PAYLOAD_SIZE 65536 + ++enum sss_sec_enctype { ++ SSS_SEC_PLAINTEXT, ++ SSS_SEC_MASTERKEY, ++ SSS_SEC_ENCTYPE_SENTINEL ++}; ++ + struct sss_sec_ctx; + + struct sss_sec_req; +@@ -88,13 +94,21 @@ errno_t sss_sec_list(TALLOC_CTX *mem_ctx, + + errno_t sss_sec_get(TALLOC_CTX *mem_ctx, + struct sss_sec_req *req, +- char **_secret); ++ uint8_t **_secret, ++ size_t *_secret_len, ++ char **_datatype); + + errno_t sss_sec_put(struct sss_sec_req *req, +- const char *secret); ++ uint8_t *secret, ++ size_t secret_len, ++ enum sss_sec_enctype enctype, ++ const char *datatype); + + errno_t sss_sec_update(struct sss_sec_req *req, +- const char *secret); ++ uint8_t *secret, ++ size_t secret_len, ++ enum sss_sec_enctype enctype, ++ const char *datatype); + + errno_t sss_sec_create_container(struct sss_sec_req *req); + +diff --git a/src/util/sss_iobuf.c b/src/util/sss_iobuf.c +index 518713e4c..3056a7b0d 100644 +--- a/src/util/sss_iobuf.c ++++ b/src/util/sss_iobuf.c +@@ -66,6 +66,30 @@ struct sss_iobuf *sss_iobuf_init_readonly(TALLOC_CTX *mem_ctx, + return iobuf; + } + ++struct sss_iobuf *sss_iobuf_init_steal(TALLOC_CTX *mem_ctx, ++ uint8_t *data, ++ size_t size) ++{ ++ struct sss_iobuf *iobuf; ++ ++ iobuf = talloc_zero(mem_ctx, struct sss_iobuf); ++ if (iobuf == NULL) { ++ return NULL; ++ } ++ ++ iobuf->data = talloc_steal(iobuf, data); ++ iobuf->size = size; ++ iobuf->capacity = size; ++ iobuf->dp = 0; ++ ++ return iobuf; ++} ++ ++void sss_iobuf_cursor_reset(struct sss_iobuf *iobuf) ++{ ++ iobuf->dp = 0; ++} ++ + size_t sss_iobuf_get_len(struct sss_iobuf *iobuf) + { + if (iobuf == NULL) { +@@ -223,6 +247,109 @@ errno_t sss_iobuf_write_len(struct sss_iobuf *iobuf, + return EOK; + } + ++errno_t sss_iobuf_read_varlen(TALLOC_CTX *mem_ctx, ++ struct sss_iobuf *iobuf, ++ uint8_t **_out, ++ size_t *_len) ++{ ++ uint8_t *out; ++ uint32_t len; ++ size_t slen; ++ errno_t ret; ++ ++ if (iobuf == NULL || _out == NULL || _len == NULL) { ++ return EINVAL; ++ } ++ ++ ret = sss_iobuf_read_uint32(iobuf, &len); ++ if (ret != EOK) { ++ return ret; ++ } ++ ++ if (len == 0) { ++ *_out = NULL; ++ *_len = 0; ++ return EOK; ++ } ++ ++ out = talloc_array(mem_ctx, uint8_t, len); ++ if (out == NULL) { ++ return ENOMEM; ++ } ++ ++ slen = len; ++ ret = sss_iobuf_read_len(iobuf, slen, out); ++ if (ret != EOK) { ++ talloc_free(out); ++ return ret; ++ } ++ ++ *_out = out; ++ *_len = slen; ++ ++ return EOK; ++} ++ ++errno_t sss_iobuf_write_varlen(struct sss_iobuf *iobuf, ++ uint8_t *data, ++ size_t len) ++{ ++ errno_t ret; ++ ++ if (iobuf == NULL || (data == NULL && len != 0)) { ++ return EINVAL; ++ } ++ ++ ret = sss_iobuf_write_uint32(iobuf, len); ++ if (ret != EOK) { ++ return ret; ++ } ++ ++ if (len == 0) { ++ return EOK; ++ } ++ ++ return sss_iobuf_write_len(iobuf, data, len); ++} ++ ++errno_t sss_iobuf_read_iobuf(TALLOC_CTX *mem_ctx, ++ struct sss_iobuf *iobuf, ++ struct sss_iobuf **_out) ++{ ++ struct sss_iobuf *out; ++ uint8_t *data; ++ size_t len; ++ errno_t ret; ++ ++ ret = sss_iobuf_read_varlen(NULL, iobuf, &data, &len); ++ if (ret != EOK) { ++ return ret; ++ } ++ ++ out = sss_iobuf_init_steal(mem_ctx, data, len); ++ if (out == NULL) { ++ return ENOMEM; ++ } ++ ++ *_out = out; ++ ++ return EOK; ++} ++ ++errno_t sss_iobuf_write_iobuf(struct sss_iobuf *iobuf, ++ struct sss_iobuf *data) ++{ ++ return sss_iobuf_write_varlen(iobuf, data->data, data->size); ++} ++ ++errno_t sss_iobuf_read_uint8(struct sss_iobuf *iobuf, ++ uint8_t *_val) ++{ ++ SAFEALIGN_COPY_UINT8_CHECK(_val, iobuf_ptr(iobuf), ++ iobuf->capacity, &iobuf->dp); ++ return EOK; ++} ++ + errno_t sss_iobuf_read_uint32(struct sss_iobuf *iobuf, + uint32_t *_val) + { +@@ -239,6 +366,20 @@ errno_t sss_iobuf_read_int32(struct sss_iobuf *iobuf, + return EOK; + } + ++errno_t sss_iobuf_write_uint8(struct sss_iobuf *iobuf, ++ uint8_t val) ++{ ++ errno_t ret; ++ ++ ret = ensure_bytes(iobuf, sizeof(uint8_t)); ++ if (ret != EOK) { ++ return ret; ++ } ++ ++ SAFEALIGN_SETMEM_UINT8(iobuf_ptr(iobuf), val, &iobuf->dp); ++ return EOK; ++} ++ + errno_t sss_iobuf_write_uint32(struct sss_iobuf *iobuf, + uint32_t val) + { +diff --git a/src/util/sss_iobuf.h b/src/util/sss_iobuf.h +index cc3dfd1e9..159fbc0b9 100644 +--- a/src/util/sss_iobuf.h ++++ b/src/util/sss_iobuf.h +@@ -50,6 +50,29 @@ struct sss_iobuf *sss_iobuf_init_readonly(TALLOC_CTX *mem_ctx, + const uint8_t *data, + size_t size); + ++/* ++ * @brief Allocate an IO buffer with a fixed size, stealing input data. ++ * ++ * This function is useful for parsing an input buffer from an existing ++ * buffer pointed to by data. ++ * ++ * The iobuf assumes ownership of the data buffer. ++ * ++ * @param[in] mem_ctx The talloc context that owns the iobuf ++ * @param[in] data The data to initialize the IO buffer with. ++ * @param[in] size The size of the data buffer ++ * ++ * @return The newly created buffer on success or NULL on an error. ++ */ ++struct sss_iobuf *sss_iobuf_init_steal(TALLOC_CTX *mem_ctx, ++ uint8_t *data, ++ size_t size); ++ ++/* ++ * @brief Reset internal cursor of the IO buffer (seek to the start) ++ */ ++void sss_iobuf_cursor_reset(struct sss_iobuf *iobuf); ++ + /* + * @brief Returns the number of bytes currently stored in the iobuf + * +@@ -131,6 +154,28 @@ errno_t sss_iobuf_write_len(struct sss_iobuf *iobuf, + uint8_t *buf, + size_t len); + ++errno_t sss_iobuf_read_varlen(TALLOC_CTX *mem_ctx, ++ struct sss_iobuf *iobuf, ++ uint8_t **_out, ++ size_t *_len); ++ ++errno_t sss_iobuf_write_varlen(struct sss_iobuf *iobuf, ++ uint8_t *data, ++ size_t len); ++ ++errno_t sss_iobuf_read_iobuf(TALLOC_CTX *mem_ctx, ++ struct sss_iobuf *iobuf, ++ struct sss_iobuf **_out); ++ ++errno_t sss_iobuf_write_iobuf(struct sss_iobuf *iobuf, ++ struct sss_iobuf *data); ++ ++errno_t sss_iobuf_read_uint8(struct sss_iobuf *iobuf, ++ uint8_t *_val); ++ ++errno_t sss_iobuf_write_uint8(struct sss_iobuf *iobuf, ++ uint8_t val); ++ + errno_t sss_iobuf_read_uint32(struct sss_iobuf *iobuf, + uint32_t *_val); + +@@ -148,4 +193,5 @@ errno_t sss_iobuf_read_stringz(struct sss_iobuf *iobuf, + + errno_t sss_iobuf_write_stringz(struct sss_iobuf *iobuf, + const char *str); ++ + #endif /* __SSS_IOBUF_H_ */ +diff --git a/src/util/sss_ptr_hash.c b/src/util/sss_ptr_hash.c +index 6409236c7..e3805dac4 100644 +--- a/src/util/sss_ptr_hash.c ++++ b/src/util/sss_ptr_hash.c +@@ -54,6 +54,7 @@ struct sss_ptr_hash_value { + hash_table_t *table; + const char *key; + void *payload; ++ bool delete_in_progress; + }; + + static int +@@ -61,12 +62,22 @@ sss_ptr_hash_value_destructor(struct sss_ptr_hash_value *value) + { + hash_key_t table_key; + ++ /* Do not call hash_delete() if we got here from hash delete callback when ++ * the callback calls talloc_free(payload) which frees the value. This ++ * should not happen since talloc will avoid circular free but let's be ++ * over protective here. */ ++ if (value->delete_in_progress) { ++ return 0; ++ } ++ ++ value->delete_in_progress = true; + if (value->table && value->key) { + table_key.type = HASH_KEY_STRING; + table_key.str = discard_const_p(char, value->key); + if (hash_delete(value->table, &table_key) != HASH_SUCCESS) { + DEBUG(SSSDBG_CRIT_FAILURE, + "failed to delete entry with key '%s'\n", value->key); ++ value->delete_in_progress = false; + } + } + +@@ -127,6 +138,15 @@ sss_ptr_hash_delete_cb(hash_entry_t *item, + callback_entry.key = item->key; + callback_entry.value.type = HASH_VALUE_PTR; + callback_entry.value.ptr = value->payload; ++ ++ /* Delete the value in case this callback has been called directly ++ * from dhash (overwriting existing entry) instead of hash_delete() ++ * in value's destructor. */ ++ if (!value->delete_in_progress) { ++ talloc_set_destructor(value, NULL); ++ talloc_free(value); ++ } ++ + /* Even if execution is already in the context of + * talloc_free(payload) -> talloc_free(value) -> ... + * there still might be legitimate reasons to execute callback. +-- +2.21.3 + diff --git a/SOURCES/0003-DEBUG-journal_send-was-made-static.patch b/SOURCES/0003-DEBUG-journal_send-was-made-static.patch new file mode 100644 index 0000000..faa9c9e --- /dev/null +++ b/SOURCES/0003-DEBUG-journal_send-was-made-static.patch @@ -0,0 +1,29 @@ +From 833034f5332d2492d413a9c97fded1480b58bf14 Mon Sep 17 00:00:00 2001 +From: Alexey Tikhonov +Date: Wed, 21 Oct 2020 18:47:32 +0200 +Subject: [PATCH 3/4] DEBUG: journal_send() was made static +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +Reviewed-by: Tomáš Halman +--- + src/util/debug.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/src/util/debug.c b/src/util/debug.c +index 1d5f75e4d..c162987b9 100644 +--- a/src/util/debug.c ++++ b/src/util/debug.c +@@ -201,7 +201,7 @@ static void debug_printf(const char *format, ...) + } + + #ifdef WITH_JOURNALD +-errno_t journal_send(const char *file, ++static errno_t journal_send(const char *file, + long line, + const char *function, + int level, +-- +2.21.3 + diff --git a/SOURCES/0004-DEBUG-fixes-program-identifier-as-seen-in-syslog.patch b/SOURCES/0004-DEBUG-fixes-program-identifier-as-seen-in-syslog.patch new file mode 100644 index 0000000..8352ea6 --- /dev/null +++ b/SOURCES/0004-DEBUG-fixes-program-identifier-as-seen-in-syslog.patch @@ -0,0 +1,71 @@ +From 18233532b72e62452eac6886652fa633ba055d8c Mon Sep 17 00:00:00 2001 +From: Alexey Tikhonov +Date: Wed, 21 Oct 2020 19:20:03 +0200 +Subject: [PATCH 4/4] DEBUG: fixes program identifier as seen in syslog +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +Commit 225fe9950f2807d5fb226f6b3be1ff4cefd731f0 changed `debug_prg_name` +to accomodate needs of own SSSD logs, but this affected journal/syslog +as well. + +This patch amends situation: + - journal messages gets "umbrella" identifier "sssd[]" + - syslog uses default which is program name + +Resolves: https://github.com/SSSD/sssd/issues/5384 + +Reviewed-by: Tomáš Halman +--- + src/util/debug.c | 2 +- + src/util/sss_log.c | 12 +++--------- + 2 files changed, 4 insertions(+), 10 deletions(-) + +diff --git a/src/util/debug.c b/src/util/debug.c +index c162987b9..f05b26500 100644 +--- a/src/util/debug.c ++++ b/src/util/debug.c +@@ -250,7 +250,7 @@ static errno_t journal_send(const char *file, + "MESSAGE=%s", message, + "PRIORITY=%i", LOG_DEBUG, + "SSSD_DOMAIN=%s", domain, +- "SSSD_PRG_NAME=%s", debug_prg_name, ++ "SSSD_PRG_NAME=sssd[%s]", debug_prg_name, + "SSSD_DEBUG_LEVEL=%x", level, + NULL); + ret = -res; +diff --git a/src/util/sss_log.c b/src/util/sss_log.c +index 48e73dbea..c6b7435c6 100644 +--- a/src/util/sss_log.c ++++ b/src/util/sss_log.c +@@ -107,7 +107,7 @@ static void sss_log_internal(int priority, int facility, const char *format, + "SSSD_DOMAIN=%s", domain, + "PRIORITY=%i", syslog_priority, + "SYSLOG_FACILITY=%i", LOG_FAC(facility), +- "SYSLOG_IDENTIFIER=%s", debug_prg_name, ++ "SYSLOG_IDENTIFIER=sssd[%s]", debug_prg_name, + NULL); + + free(message); +@@ -118,15 +118,9 @@ static void sss_log_internal(int priority, int facility, const char *format, + static void sss_log_internal(int priority, int facility, const char *format, + va_list ap) + { +- int syslog_priority; +- +- syslog_priority = sss_to_syslog(priority); +- +- openlog(debug_prg_name, 0, facility); +- +- vsyslog(syslog_priority, format, ap); ++ int syslog_priority = sss_to_syslog(priority); + +- closelog(); ++ vsyslog(facility|syslog_priority, format, ap); + } + + #endif /* WITH_JOURNALD */ +-- +2.21.3 + diff --git a/SOURCES/0005-negcache-make-sure-domain-config-does-not-leak-into-.patch b/SOURCES/0005-negcache-make-sure-domain-config-does-not-leak-into-.patch new file mode 100644 index 0000000..8aeda8b --- /dev/null +++ b/SOURCES/0005-negcache-make-sure-domain-config-does-not-leak-into-.patch @@ -0,0 +1,36 @@ +From 0e1bcf77bd73baa0fea64830eb1f4f65a63c7afe Mon Sep 17 00:00:00 2001 +From: Sumit Bose +Date: Thu, 8 Oct 2020 12:18:41 +0200 +Subject: [PATCH 5/8] negcache: make sure domain config does not leak into + global + +Resolves: https://github.com/SSSD/sssd/issues/5238 + +Reviewed-by: Alexey Tikhonov +--- + src/responder/common/negcache.c | 2 ++ + 1 file changed, 2 insertions(+) + +diff --git a/src/responder/common/negcache.c b/src/responder/common/negcache.c +index ce1c0ab8c..139218420 100644 +--- a/src/responder/common/negcache.c ++++ b/src/responder/common/negcache.c +@@ -1050,6 +1050,7 @@ errno_t sss_ncache_prepopulate(struct sss_nc_ctx *ncache, + } + } + ++ talloc_zfree(filter_list); + /* Populate non domain-specific negative cache user entries */ + ret = confdb_get_string_as_list(cdb, tmpctx, CONFDB_NSS_CONF_ENTRY, + CONFDB_NSS_FILTER_USERS, &filter_list); +@@ -1185,6 +1186,7 @@ errno_t sss_ncache_prepopulate(struct sss_nc_ctx *ncache, + } + } + ++ talloc_zfree(filter_list); + /* Populate non domain-specific negative cache group entries */ + ret = confdb_get_string_as_list(cdb, tmpctx, CONFDB_NSS_CONF_ENTRY, + CONFDB_NSS_FILTER_GROUPS, &filter_list); +-- +2.21.3 + diff --git a/SOURCES/0006-utils-add-SSS_GND_SUBDOMAINS-flag-for-get_next_domai.patch b/SOURCES/0006-utils-add-SSS_GND_SUBDOMAINS-flag-for-get_next_domai.patch new file mode 100644 index 0000000..e3aeec3 --- /dev/null +++ b/SOURCES/0006-utils-add-SSS_GND_SUBDOMAINS-flag-for-get_next_domai.patch @@ -0,0 +1,106 @@ +From 385af99ff4d5a75d0c1edc9ad830da3eb7478295 Mon Sep 17 00:00:00 2001 +From: Sumit Bose +Date: Thu, 8 Oct 2020 17:57:29 +0200 +Subject: [PATCH 6/8] utils: add SSS_GND_SUBDOMAINS flag for get_next_domain() + +To allow to only iterate over a singel domain an its sub-domains a new +flag is added to get_next_domain(). + +Resolves: https://github.com/SSSD/sssd/issues/5238 + +Reviewed-by: Alexey Tikhonov +--- + src/tests/cmocka/test_utils.c | 31 +++++++++++++++++++++++++++++++ + src/util/domain_info_utils.c | 10 +++++++--- + src/util/util.h | 4 ++++ + 3 files changed, 42 insertions(+), 3 deletions(-) + +diff --git a/src/tests/cmocka/test_utils.c b/src/tests/cmocka/test_utils.c +index 945f5cb44..d77a972c1 100644 +--- a/src/tests/cmocka/test_utils.c ++++ b/src/tests/cmocka/test_utils.c +@@ -877,6 +877,37 @@ static void test_get_next_domain_flags(void **state) + + dom = get_next_domain(dom, gnd_flags); + assert_null(dom); ++ ++ /* Descend only to subdomains */ ++ gnd_flags = SSS_GND_SUBDOMAINS | SSS_GND_INCLUDE_DISABLED; ++ ++ dom = get_next_domain(test_ctx->dom_list, gnd_flags); ++ assert_non_null(dom); ++ assert_string_equal(dom->name, "sub1a"); ++ ++ dom = get_next_domain(dom, gnd_flags); ++ assert_null(dom); ++ ++ dom = find_domain_by_name_ex(test_ctx->dom_list, "dom2", true, ++ SSS_GND_ALL_DOMAINS); ++ assert_non_null(dom); ++ assert_string_equal(dom->name, "dom2"); ++ ++ dom = get_next_domain(dom, gnd_flags); ++ assert_non_null(dom); ++ assert_string_equal(dom->name, "sub2a"); ++ ++ dom = get_next_domain(dom, gnd_flags); ++ assert_non_null(dom); ++ assert_string_equal(dom->name, "sub2b"); ++ ++ dom = get_next_domain(dom, gnd_flags); ++ assert_null(dom); ++ ++ /* Expect NULL if the domain has no sub-domains */ ++ test_ctx->dom_list->subdomains = NULL; ++ dom = get_next_domain(test_ctx->dom_list, gnd_flags); ++ assert_null(dom); + } + + struct name_init_test_ctx { +diff --git a/src/util/domain_info_utils.c b/src/util/domain_info_utils.c +index aa3582f03..4d4726daa 100644 +--- a/src/util/domain_info_utils.c ++++ b/src/util/domain_info_utils.c +@@ -39,16 +39,20 @@ struct sss_domain_info *get_next_domain(struct sss_domain_info *domain, + uint32_t gnd_flags) + { + struct sss_domain_info *dom; +- bool descend = gnd_flags & SSS_GND_DESCEND; ++ bool descend = gnd_flags & (SSS_GND_DESCEND | SSS_GND_SUBDOMAINS); + bool include_disabled = gnd_flags & SSS_GND_INCLUDE_DISABLED; ++ bool only_subdomains = gnd_flags & SSS_GND_SUBDOMAINS; + + dom = domain; + while (dom) { + if (descend && dom->subdomains) { + dom = dom->subdomains; +- } else if (dom->next) { ++ } else if (dom->next && only_subdomains && IS_SUBDOMAIN(dom)) { + dom = dom->next; +- } else if (descend && IS_SUBDOMAIN(dom) && dom->parent->next) { ++ } else if (dom->next && !only_subdomains) { ++ dom = dom->next; ++ } else if (descend && !only_subdomains && IS_SUBDOMAIN(dom) ++ && dom->parent->next) { + dom = dom->parent->next; + } else { + dom = NULL; +diff --git a/src/util/util.h b/src/util/util.h +index fbcac5cd0..581c0edfb 100644 +--- a/src/util/util.h ++++ b/src/util/util.h +@@ -565,7 +565,11 @@ struct sss_domain_info *get_domains_head(struct sss_domain_info *domain); + + #define SSS_GND_DESCEND 0x01 + #define SSS_GND_INCLUDE_DISABLED 0x02 ++/* Descend to sub-domains of current domain but do not go to next parent */ ++#define SSS_GND_SUBDOMAINS 0x04 + #define SSS_GND_ALL_DOMAINS (SSS_GND_DESCEND | SSS_GND_INCLUDE_DISABLED) ++#define SSS_GND_ALL_SUBDOMAINS (SSS_GND_SUBDOMAINS | SSS_GND_INCLUDE_DISABLED) ++ + struct sss_domain_info *get_next_domain(struct sss_domain_info *domain, + uint32_t gnd_flags); + struct sss_domain_info *find_domain_by_name(struct sss_domain_info *domain, +-- +2.21.3 + diff --git a/SOURCES/0007-negcache-make-sure-short-names-are-added-to-sub-doma.patch b/SOURCES/0007-negcache-make-sure-short-names-are-added-to-sub-doma.patch new file mode 100644 index 0000000..9d405fc --- /dev/null +++ b/SOURCES/0007-negcache-make-sure-short-names-are-added-to-sub-doma.patch @@ -0,0 +1,443 @@ +From 0dc81a52e2836010974e9f71b1f3e47c20fd498d Mon Sep 17 00:00:00 2001 +From: Sumit Bose +Date: Fri, 9 Oct 2020 11:56:21 +0200 +Subject: [PATCH 7/8] negcache: make sure short names are added to sub-domains + +If short names are used with filter_users or filter_groups in a +[domain/...] section they should be added to the sub-domains of this +domain as well. + +Resolves: https://github.com/SSSD/sssd/issues/5238 + +Reviewed-by: Alexey Tikhonov +--- + src/responder/common/negcache.c | 105 +++++++------ + src/tests/cmocka/test_negcache.c | 254 +++++++++++++++++++++++++++++++ + 2 files changed, 312 insertions(+), 47 deletions(-) + +diff --git a/src/responder/common/negcache.c b/src/responder/common/negcache.c +index 139218420..9ee39ce3e 100644 +--- a/src/responder/common/negcache.c ++++ b/src/responder/common/negcache.c +@@ -971,6 +971,7 @@ errno_t sss_ncache_prepopulate(struct sss_nc_ctx *ncache, + char *name = NULL; + struct sss_domain_info *dom = NULL; + struct sss_domain_info *domain_list = rctx->domains; ++ struct sss_domain_info *ddom; + char *domainname = NULL; + char *conf_path = NULL; + TALLOC_CTX *tmpctx = talloc_new(NULL); +@@ -1013,39 +1014,44 @@ errno_t sss_ncache_prepopulate(struct sss_nc_ctx *ncache, + continue; + } + +- if (domainname && strcmp(domainname, dom->name)) { +- DEBUG(SSSDBG_TRACE_FUNC, +- "Mismatch between domain name (%s) and name " +- "set in FQN (%s), assuming %s is UPN\n", +- dom->name, domainname, filter_list[i]); +- ret = sss_ncache_set_upn(ncache, true, dom, filter_list[i]); ++ /* Check domain and its sub-domains */ ++ for (ddom = dom; ddom != NULL; ++ ddom = get_next_domain(ddom, SSS_GND_ALL_SUBDOMAINS)) { ++ ++ if (domainname && strcmp(domainname, ddom->name)) { ++ DEBUG(SSSDBG_TRACE_FUNC, ++ "Mismatch between domain name (%s) and name " ++ "set in FQN (%s), assuming %s is UPN\n", ++ ddom->name, domainname, filter_list[i]); ++ ret = sss_ncache_set_upn(ncache, true, ddom, filter_list[i]); ++ if (ret != EOK) { ++ DEBUG(SSSDBG_OP_FAILURE, ++ "sss_ncache_set_upn failed (%d [%s]), ignored\n", ++ ret, sss_strerror(ret)); ++ } ++ continue; ++ } ++ ++ fqname = sss_create_internal_fqname(tmpctx, name, ddom->name); ++ if (fqname == NULL) { ++ continue; ++ } ++ ++ ret = sss_ncache_set_upn(ncache, true, ddom, fqname); + if (ret != EOK) { + DEBUG(SSSDBG_OP_FAILURE, + "sss_ncache_set_upn failed (%d [%s]), ignored\n", + ret, sss_strerror(ret)); + } +- continue; +- } +- +- fqname = sss_create_internal_fqname(tmpctx, name, dom->name); +- if (fqname == NULL) { +- continue; +- } +- +- ret = sss_ncache_set_upn(ncache, true, dom, fqname); +- if (ret != EOK) { +- DEBUG(SSSDBG_OP_FAILURE, +- "sss_ncache_set_upn failed (%d [%s]), ignored\n", +- ret, sss_strerror(ret)); +- } +- ret = sss_ncache_set_user(ncache, true, dom, fqname); +- talloc_zfree(fqname); +- if (ret != EOK) { +- DEBUG(SSSDBG_CRIT_FAILURE, +- "Failed to store permanent user filter for [%s]" +- " (%d [%s])\n", filter_list[i], +- ret, sss_strerror(ret)); +- continue; ++ ret = sss_ncache_set_user(ncache, true, ddom, fqname); ++ talloc_zfree(fqname); ++ if (ret != EOK) { ++ DEBUG(SSSDBG_CRIT_FAILURE, ++ "Failed to store permanent user filter for [%s]" ++ " (%d [%s])\n", filter_list[i], ++ ret, sss_strerror(ret)); ++ continue; ++ } + } + } + } +@@ -1161,27 +1167,32 @@ errno_t sss_ncache_prepopulate(struct sss_nc_ctx *ncache, + continue; + } + +- if (domainname && strcmp(domainname, dom->name)) { +- DEBUG(SSSDBG_CRIT_FAILURE, +- "Mismatch between domain name (%s) and name " +- "set in FQN (%s), skipping group %s\n", +- dom->name, domainname, name); +- continue; +- } ++ /* Check domain and its sub-domains */ ++ for (ddom = dom; ++ ddom != NULL && (ddom == dom || ddom->parent != NULL); ++ ddom = get_next_domain(ddom, SSS_GND_ALL_DOMAINS)) { ++ if (domainname && strcmp(domainname, ddom->name)) { ++ DEBUG(SSSDBG_CRIT_FAILURE, ++ "Mismatch between domain name (%s) and name " ++ "set in FQN (%s), skipping group %s\n", ++ ddom->name, domainname, name); ++ continue; ++ } + +- fqname = sss_create_internal_fqname(tmpctx, name, dom->name); +- if (fqname == NULL) { +- continue; +- } ++ fqname = sss_create_internal_fqname(tmpctx, name, ddom->name); ++ if (fqname == NULL) { ++ continue; ++ } + +- ret = sss_ncache_set_group(ncache, true, dom, fqname); +- talloc_zfree(fqname); +- if (ret != EOK) { +- DEBUG(SSSDBG_CRIT_FAILURE, +- "Failed to store permanent group filter for [%s]" +- " (%d [%s])\n", filter_list[i], +- ret, strerror(ret)); +- continue; ++ ret = sss_ncache_set_group(ncache, true, ddom, fqname); ++ talloc_zfree(fqname); ++ if (ret != EOK) { ++ DEBUG(SSSDBG_CRIT_FAILURE, ++ "Failed to store permanent group filter for [%s]" ++ " (%d [%s])\n", filter_list[i], ++ ret, strerror(ret)); ++ continue; ++ } + } + } + } +diff --git a/src/tests/cmocka/test_negcache.c b/src/tests/cmocka/test_negcache.c +index b3a379227..fb306b110 100644 +--- a/src/tests/cmocka/test_negcache.c ++++ b/src/tests/cmocka/test_negcache.c +@@ -119,6 +119,8 @@ static int setup(void **state) + int ret; + struct test_state *ts; + ++ test_dom_suite_setup(TESTS_PATH); ++ + ts = talloc(NULL, struct test_state); + assert_non_null(ts); + +@@ -133,6 +135,7 @@ static int setup(void **state) + static int teardown(void **state) + { + struct test_state *ts = talloc_get_type_abort(*state, struct test_state); ++ test_dom_suite_cleanup(TESTS_PATH, TEST_CONF_DB, TEST_DOM_NAME); + talloc_free(ts); + return 0; + } +@@ -921,6 +924,255 @@ static void test_sss_ncache_reset_prepopulate(void **state) + assert_int_equal(ret, EEXIST); + } + ++/* The main purpose of test_sss_ncache_short_name_in_domain is to test that ++ * short names in the filter_users or filter_groups options in a [domain/...] ++ * section are properly added to the related sub-domains as well (if there are ++ * any) and not added to domains from other [domain/...] sections. For ++ * completeness entries with fully-qualified names of the parent and the ++ * sub-domain and the generic UPN are added as well. ++ * ++ * The result should of course be independent of the present domains. To ++ * verify this the domains are added one after the other and the negative ++ * cache is repopulated each time. ++ * ++ * With the given domains, users and group we have to following expectations: ++ * - the short name entry will be added to the domain and all sub-domains as ++ * name and as upn by expanding it to a fully-qualified name with the ++ * domain name or sub-domain name respectively ++ * - the fully-qualified name from the parent domain is added as name and upn ++ * to the parent domain and as upn to all sub-domains ++ * - the fully-qualified name from the sub-domain is added as name to the ++ * sub-domain and as upn to the parent and all sub-domains ++ * - the generic upn is nowhere added as name and as upn to the parent and all ++ * sub-domains ++ * - none of the names is added to a different parent domain ++ * ++ * The following table should illustrated the expectations: ++ * ++ * user (name): ++ * | shortuser | parentu@TEST_DOM_NAME | subdomu@subTEST_DOM_NAME | upn@upn.dom ++ *-----------------+-----------+-----------------------+--------------------------+------------ ++ * TEST_DOM_NAME | PRESENT | PRESENT | MISSING | MISSING ++ * subTEST_DOM_NAME| PRESENT | MISSING | PRESENT | MISSING ++ * TEST_DOM_NAME2 | MISSING | MISSING | MISSING | MISSING ++ * ++ * user (upn): ++ * | shortuser | parentu@TEST_DOM_NAME | subdomu@subTEST_DOM_NAME | upn@upn.dom ++ *-----------------+-----------+-----------------------+--------------------------+------------ ++ * TEST_DOM_NAME | PRESENT | PRESENT | PRESENT | PRESENT ++ * subTEST_DOM_NAME| PRESENT | PRESENT | PRESENT | PRESENT ++ * TEST_DOM_NAME2 | MISSING | MISSING | MISSING | MISSING ++ * ++ * ++ * ++ * groups: ++ * | shortgroup | parentg@TEST_DOM_NAME | subdomg@subTEST_DOM_NAME ++ *-----------------+------------+-----------------------+------------------------- ++ * TEST_DOM_NAME | PRESENT | PRESENT | MISSING ++ * subTEST_DOM_NAME| PRESENT | MISSING | PRESENT ++ * TEST_DOM_NAME2 | MISSING | MISSING | MISSING ++ * ++ * ++ * The following expect_*() implement checks for the expextations: ++ */ ++ ++static void expect_in_parent(struct sss_nc_ctx *ncache, ++ struct sss_domain_info *dom) ++{ ++ int ret; ++ ++ ret = check_user_in_ncache(ncache, dom, "shortuser"); ++ assert_int_equal(ret, EEXIST); ++ ret = sss_ncache_check_upn(ncache, dom, "shortuser@"TEST_DOM_NAME); ++ assert_int_equal(ret, EEXIST); ++ ++ ret = check_user_in_ncache(ncache, dom, "parentu"); ++ assert_int_equal(ret, EEXIST); ++ ret = sss_ncache_check_upn(ncache, dom, "parentu@"TEST_DOM_NAME); ++ assert_int_equal(ret, EEXIST); ++ ++ ret = check_user_in_ncache(ncache, dom, "subdomu"); ++ assert_int_equal(ret, ENOENT); ++ ret = sss_ncache_check_upn(ncache, dom, "subdomu@sub"TEST_DOM_NAME); ++ assert_int_equal(ret, EEXIST); ++ ++ ret = check_user_in_ncache(ncache, dom, "upn"); ++ assert_int_equal(ret, ENOENT); ++ ret = sss_ncache_check_upn(ncache, dom, "upn@upn.dom"); ++ assert_int_equal(ret, EEXIST); ++ ++ ret = check_group_in_ncache(ncache, dom, "shortgroup"); ++ assert_int_equal(ret, EEXIST); ++ ++ ret = check_group_in_ncache(ncache, dom, "parentg"); ++ assert_int_equal(ret, EEXIST); ++ ++ ret = check_group_in_ncache(ncache, dom, "subdomg"); ++ assert_int_equal(ret, ENOENT); ++} ++ ++static void expect_in_subdomain(struct sss_nc_ctx *ncache, ++ struct sss_domain_info *sub_dom) ++{ ++ int ret; ++ ++ ret = check_user_in_ncache(ncache, sub_dom, "shortuser"); ++ assert_int_equal(ret, EEXIST); ++ ret = sss_ncache_check_upn(ncache, sub_dom, "shortuser@sub"TEST_DOM_NAME); ++ assert_int_equal(ret, EEXIST); ++ ++ ret = check_user_in_ncache(ncache, sub_dom, "subdomu"); ++ assert_int_equal(ret, EEXIST); ++ ret = sss_ncache_check_upn(ncache, sub_dom, "subdomu@sub"TEST_DOM_NAME); ++ assert_int_equal(ret, EEXIST); ++ ++ ret = check_user_in_ncache(ncache, sub_dom, "upn"); ++ assert_int_equal(ret, ENOENT); ++ ret = sss_ncache_check_upn(ncache, sub_dom, "upn@upn.dom"); ++ assert_int_equal(ret, EEXIST); ++ ++ ret = check_user_in_ncache(ncache, sub_dom, "parentu"); ++ assert_int_equal(ret, ENOENT); ++ ret = sss_ncache_check_upn(ncache, sub_dom, "parentu@"TEST_DOM_NAME); ++ assert_int_equal(ret, EEXIST); ++ ++ ++ ret = check_group_in_ncache(ncache, sub_dom, "shortgroup"); ++ assert_int_equal(ret, EEXIST); ++ ++ ret = check_group_in_ncache(ncache, sub_dom, "parentg"); ++ assert_int_equal(ret, ENOENT); ++ ++ ret = check_group_in_ncache(ncache, sub_dom, "subdomg"); ++ assert_int_equal(ret, EEXIST); ++} ++static void expect_no_entries_in_dom(struct sss_nc_ctx *ncache, ++ struct sss_domain_info *dom2) ++{ ++ int ret; ++ ++ ret = check_user_in_ncache(ncache, dom2, "shortuser"); ++ assert_int_equal(ret, ENOENT); ++ ret = sss_ncache_check_upn(ncache, dom2, "shortuser"TEST_DOM_NAME); ++ assert_int_equal(ret, ENOENT); ++ ++ ret = check_user_in_ncache(ncache, dom2, "parentu"); ++ assert_int_equal(ret, ENOENT); ++ ret = sss_ncache_check_upn(ncache, dom2, "parentu@"TEST_DOM_NAME); ++ assert_int_equal(ret, ENOENT); ++ ++ ret = check_user_in_ncache(ncache, dom2, "subdomu"); ++ assert_int_equal(ret, ENOENT); ++ ret = sss_ncache_check_upn(ncache, dom2, "subdomu@sub"TEST_DOM_NAME); ++ assert_int_equal(ret, ENOENT); ++ ++ ret = check_user_in_ncache(ncache, dom2, "upn"); ++ assert_int_equal(ret, ENOENT); ++ ret = sss_ncache_check_upn(ncache, dom2, "upn@upn.dom"); ++ assert_int_equal(ret, ENOENT); ++ ++ ret = check_group_in_ncache(ncache, dom2, "shortgroup"); ++ assert_int_equal(ret, ENOENT); ++ ++ ret = check_group_in_ncache(ncache, dom2, "parentg"); ++ assert_int_equal(ret, ENOENT); ++ ++ ret = check_group_in_ncache(ncache, dom2, "subdomg"); ++ assert_int_equal(ret, ENOENT); ++} ++ ++static void test_sss_ncache_short_name_in_domain(void **state) ++{ ++ int ret; ++ struct test_state *ts; ++ struct tevent_context *ev; ++ struct sss_nc_ctx *ncache; ++ struct sss_test_ctx *tc; ++ struct sss_domain_info *dom; ++ struct sss_domain_info *dom2; ++ struct sss_domain_info *sub_dom; ++ ++ struct sss_test_conf_param params[] = { ++ { "filter_users", "shortuser, parentu@"TEST_DOM_NAME", " ++ "subdomu@sub"TEST_DOM_NAME", upn@upn.dom" }, ++ { "filter_groups", "shortgroup, parentg@"TEST_DOM_NAME", " ++ "subdomg@sub"TEST_DOM_NAME }, ++ { NULL, NULL }, ++ }; ++ ++ const char *nss_filter_users[] = { params[0].value, NULL}; ++ const char *nss_filter_groups[] = { params[1].value, NULL}; ++ ++ ts = talloc_get_type_abort(*state, struct test_state); ++ ++ ev = tevent_context_init(ts); ++ assert_non_null(ev); ++ ++ dom = talloc_zero(ts, struct sss_domain_info); ++ assert_non_null(dom); ++ dom->name = discard_const_p(char, TEST_DOM_NAME); ++ sss_domain_set_state(dom, DOM_ACTIVE); ++ ++ ts->nctx = mock_nctx(ts); ++ assert_non_null(ts->nctx); ++ ++ tc = create_dom_test_ctx(ts, TESTS_PATH, TEST_CONF_DB, ++ TEST_DOM_NAME, TEST_ID_PROVIDER, params); ++ assert_non_null(tc); ++ ++ ret = confdb_add_param(tc->confdb, true, "config/domain/"TEST_DOM_NAME, ++ "filter_users", nss_filter_users); ++ assert_int_equal(ret, EOK); ++ ++ ret = confdb_add_param(tc->confdb, true, "config/domain"TEST_DOM_NAME, ++ "filter_groups", nss_filter_groups); ++ assert_int_equal(ret, EOK); ++ ++ ncache = ts->ctx; ++ ts->rctx = mock_rctx(ts, ev, dom, ts->nctx); ++ assert_non_null(ts->rctx); ++ ts->rctx->cdb = tc->confdb; ++ ++ ret = sss_names_init(ts, tc->confdb, TEST_DOM_NAME, &dom->names); ++ assert_int_equal(ret, EOK); ++ ++ ret = sss_ncache_reset_repopulate_permanent(ts->rctx, ncache); ++ assert_int_equal(ret, EOK); ++ ++ /* Add another domain */ ++ dom2 = talloc_zero(ts, struct sss_domain_info); ++ assert_non_null(dom2); ++ dom2->name = discard_const_p(char, TEST_DOM_NAME"2"); ++ sss_domain_set_state(dom2, DOM_ACTIVE); ++ dom->next = dom2; ++ dom2->names = dom->names; ++ ++ expect_in_parent(ncache, dom); ++ expect_no_entries_in_dom(ncache, dom2); ++ ++ ret = sss_ncache_reset_repopulate_permanent(ts->rctx, ncache); ++ assert_int_equal(ret, EOK); ++ ++ expect_in_parent(ncache, dom); ++ expect_no_entries_in_dom(ncache, dom2); ++ ++ /* Add a sub domain */ ++ sub_dom = talloc_zero(ts, struct sss_domain_info); ++ assert_non_null(sub_dom); ++ sub_dom->name = discard_const_p(char, "sub"TEST_DOM_NAME); ++ sss_domain_set_state(sub_dom, DOM_ACTIVE); ++ sub_dom->parent = dom; ++ dom->subdomains = sub_dom; ++ sub_dom->names = dom->names; ++ ++ ret = sss_ncache_reset_repopulate_permanent(ts->rctx, ncache); ++ assert_int_equal(ret, EOK); ++ ++ expect_in_parent(ncache, dom); ++ expect_in_subdomain(ncache, sub_dom); ++ expect_no_entries_in_dom(ncache, dom2); ++} ++ + static void test_sss_ncache_reset(void **state) + { + errno_t ret; +@@ -1083,6 +1335,8 @@ int main(void) + setup, teardown), + cmocka_unit_test_setup_teardown(test_sss_ncache_reset_prepopulate, + setup, teardown), ++ cmocka_unit_test_setup_teardown(test_sss_ncache_short_name_in_domain, ++ setup, teardown), + cmocka_unit_test_setup_teardown(test_sss_ncache_reset, + setup, teardown), + cmocka_unit_test_setup_teardown(test_sss_ncache_locate_uid_gid, +-- +2.21.3 + diff --git a/SOURCES/0008-negcache-do-not-use-default_domain_suffix.patch b/SOURCES/0008-negcache-do-not-use-default_domain_suffix.patch new file mode 100644 index 0000000..17ce2db --- /dev/null +++ b/SOURCES/0008-negcache-do-not-use-default_domain_suffix.patch @@ -0,0 +1,154 @@ +From fa4b46e7de7297da3c0e37913eab8cba7f103629 Mon Sep 17 00:00:00 2001 +From: Sumit Bose +Date: Fri, 9 Oct 2020 15:26:39 +0200 +Subject: [PATCH 8/8] negcache: do not use default_domain_suffix + +When splitting the names from the filter_users and filter_groups options +do not use the default_domain_suffix because it will hide that the +original name is a short name and should be added everywhere. + +Additionally this patch fixes a typo where sss_parse_name() was used +instead of sss_parse_name_for_domains(). + +Resolves: https://github.com/SSSD/sssd/issues/5238 + +Reviewed-by: Alexey Tikhonov +--- + src/responder/common/negcache.c | 29 +++++++++++++++-------------- + src/tests/cmocka/test_negcache.c | 22 ++++++++++++++++++++-- + 2 files changed, 35 insertions(+), 16 deletions(-) + +diff --git a/src/responder/common/negcache.c b/src/responder/common/negcache.c +index 9ee39ce3e..59e8ad7e7 100644 +--- a/src/responder/common/negcache.c ++++ b/src/responder/common/negcache.c +@@ -1000,13 +1000,13 @@ errno_t sss_ncache_prepopulate(struct sss_nc_ctx *ncache, + + for (i = 0; (filter_list && filter_list[i]); i++) { + ret = sss_parse_name_for_domains(tmpctx, domain_list, +- rctx->default_domain, ++ NULL, + filter_list[i], + &domainname, &name); + if (ret == EAGAIN) { + DEBUG(SSSDBG_MINOR_FAILURE, +- "cannot add [%s] to negcache because the required or " +- "default domain are not known yet\n", filter_list[i]); ++ "Can add [%s] only as UPN to negcache because the " ++ "required domain is not known yet\n", filter_list[i]); + } else if (ret != EOK) { + DEBUG(SSSDBG_CRIT_FAILURE, + "Invalid name in filterUsers list: [%s] (%d)\n", +@@ -1066,12 +1066,12 @@ errno_t sss_ncache_prepopulate(struct sss_nc_ctx *ncache, + + for (i = 0; (filter_list && filter_list[i]); i++) { + ret = sss_parse_name_for_domains(tmpctx, domain_list, +- rctx->default_domain, filter_list[i], ++ NULL, filter_list[i], + &domainname, &name); + if (ret == EAGAIN) { + DEBUG(SSSDBG_MINOR_FAILURE, +- "Cannot add [%s] to negcache because the required or " +- "default domain are not known yet\n", filter_list[i]); ++ "Can add [%s] only as UPN to negcache because the " ++ "required domain is not known yet\n", filter_list[i]); + } else if (ret != EOK) { + DEBUG(SSSDBG_CRIT_FAILURE, + "Invalid name in filterUsers list: [%s] (%d)\n", +@@ -1158,9 +1158,12 @@ errno_t sss_ncache_prepopulate(struct sss_nc_ctx *ncache, + if (ret != EOK) goto done; + + for (i = 0; (filter_list && filter_list[i]); i++) { +- ret = sss_parse_name(tmpctx, dom->names, filter_list[i], +- &domainname, &name); ++ ret = sss_parse_name_for_domains(tmpctx, domain_list, ++ NULL, filter_list[i], ++ &domainname, &name); + if (ret != EOK) { ++ /* Groups do not have UPNs, so domain names, if present, ++ * must be known */ + DEBUG(SSSDBG_CRIT_FAILURE, + "Invalid name in filterGroups list: [%s] (%d)\n", + filter_list[i], ret); +@@ -1207,13 +1210,11 @@ errno_t sss_ncache_prepopulate(struct sss_nc_ctx *ncache, + + for (i = 0; (filter_list && filter_list[i]); i++) { + ret = sss_parse_name_for_domains(tmpctx, domain_list, +- rctx->default_domain, filter_list[i], ++ NULL, filter_list[i], + &domainname, &name); +- if (ret == EAGAIN) { +- DEBUG(SSSDBG_MINOR_FAILURE, +- "Cannot add [%s] to negcache because the required or " +- "default domain are not known yet\n", filter_list[i]); +- } else if (ret != EOK) { ++ if (ret != EOK) { ++ /* Groups do not have UPNs, so domain names, if present, ++ * must be known */ + DEBUG(SSSDBG_CRIT_FAILURE, + "Invalid name in filterGroups list: [%s] (%d)\n", + filter_list[i], ret); +diff --git a/src/tests/cmocka/test_negcache.c b/src/tests/cmocka/test_negcache.c +index fb306b110..30218d52a 100644 +--- a/src/tests/cmocka/test_negcache.c ++++ b/src/tests/cmocka/test_negcache.c +@@ -933,7 +933,9 @@ static void test_sss_ncache_reset_prepopulate(void **state) + * + * The result should of course be independent of the present domains. To + * verify this the domains are added one after the other and the negative +- * cache is repopulated each time. ++ * cache is repopulated each time. The result should be also independent of ++ * the setting of default_domain_suffix option which is tested by ++ * test_sss_ncache_short_name_in_domain_with_prefix. + * + * With the given domains, users and group we have to following expectations: + * - the short name entry will be added to the domain and all sub-domains as +@@ -1081,7 +1083,8 @@ static void expect_no_entries_in_dom(struct sss_nc_ctx *ncache, + assert_int_equal(ret, ENOENT); + } + +-static void test_sss_ncache_short_name_in_domain(void **state) ++static void run_sss_ncache_short_name_in_domain(void **state, ++ bool use_default_domain_prefix) + { + int ret; + struct test_state *ts; +@@ -1131,6 +1134,9 @@ static void test_sss_ncache_short_name_in_domain(void **state) + ncache = ts->ctx; + ts->rctx = mock_rctx(ts, ev, dom, ts->nctx); + assert_non_null(ts->rctx); ++ if (use_default_domain_prefix) { ++ ts->rctx->default_domain = discard_const(TEST_DOM_NAME); ++ } + ts->rctx->cdb = tc->confdb; + + ret = sss_names_init(ts, tc->confdb, TEST_DOM_NAME, &dom->names); +@@ -1173,6 +1179,16 @@ static void test_sss_ncache_short_name_in_domain(void **state) + expect_no_entries_in_dom(ncache, dom2); + } + ++static void test_sss_ncache_short_name_in_domain(void **state) ++{ ++ run_sss_ncache_short_name_in_domain(state, false); ++} ++ ++static void test_sss_ncache_short_name_in_domain_with_prefix(void **state) ++{ ++ run_sss_ncache_short_name_in_domain(state, true); ++} ++ + static void test_sss_ncache_reset(void **state) + { + errno_t ret; +@@ -1337,6 +1353,8 @@ int main(void) + setup, teardown), + cmocka_unit_test_setup_teardown(test_sss_ncache_short_name_in_domain, + setup, teardown), ++ cmocka_unit_test_setup_teardown(test_sss_ncache_short_name_in_domain_with_prefix, ++ setup, teardown), + cmocka_unit_test_setup_teardown(test_sss_ncache_reset, + setup, teardown), + cmocka_unit_test_setup_teardown(test_sss_ncache_locate_uid_gid, +-- +2.21.3 + diff --git a/SPECS/sssd.spec b/SPECS/sssd.spec index ffb49b8..26bb823 100644 --- a/SPECS/sssd.spec +++ b/SPECS/sssd.spec @@ -26,7 +26,7 @@ Name: sssd Version: 2.4.0 -Release: 2%{?dist} +Release: 3%{?dist} Group: Applications/System Summary: System Security Services Daemon License: GPLv3+ @@ -34,6 +34,14 @@ URL: https://pagure.io/SSSD/sssd/ Source0: https://releases.pagure.org/SSSD/sssd/%{name}-%{version}.tar.gz ### Patches ### +Patch0001: 0001-SYSDB-merge_res_sysdb_attrs-fixed-to-avoid-NULL-ptr-.patch +Patch0002: 0002-KCM-perf-improvements.patch +Patch0003: 0003-DEBUG-journal_send-was-made-static.patch +Patch0004: 0004-DEBUG-fixes-program-identifier-as-seen-in-syslog.patch +Patch0005: 0005-negcache-make-sure-domain-config-does-not-leak-into-.patch +Patch0006: 0006-utils-add-SSS_GND_SUBDOMAINS-flag-for-get_next_domai.patch +Patch0007: 0007-negcache-make-sure-short-names-are-added-to-sub-doma.patch +Patch0008: 0008-negcache-do-not-use-default_domain_suffix.patch ### Downstream Patches ### @@ -1207,6 +1215,12 @@ fi %{_libdir}/%{name}/modules/libwbclient.so %changelog +* Mon Dec 07 2020 Alexey Tikhonov - 2.4.0-3 +- Resolves: rhbz#1900733 - sssd_be segfaults at be_refresh_get_values_ex() due to NULL ptrs in results of sysdb_search_with_ts_attr() +- Resolves: rhbz#1876514 - High CPU utilization by the sssd_kcm process +- Resolves: rhbz#1894540 - sssd component logging is now too generic in syslog/journal +- Resolves: rhbz#1828483 - filtered ID is appearing due to strange negative cache behavior + * Thu Nov 12 2020 Alexey Tikhonov - 2.4.0-2 - This is to bump version to allow rebuild against rebased libldb.