URL: https://github.com/SSSD/sssd/pull/5776
Author: pbrezina
Title: #5776: kcm: replace existing credentials to avoid unnecessary ccache
growth
Action: synchronized
To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5776/head:pr5776
git checkout pr5776
From 966ee10d89246925d647a712c64872bcfac4ee8d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrez...@redhat.com>
Date: Tue, 7 Sep 2021 11:16:22 +0200
Subject: [PATCH 1/2] krb5: remove unused mem_ctx from
get_krb5_data_from_cred()
Also don't return value since it is useless.
---
src/responder/kcm/kcmsrv_ccache.c | 7 +------
src/util/sss_krb5.c | 6 +-----
src/util/sss_krb5.h | 4 +---
3 files changed, 3 insertions(+), 14 deletions(-)
diff --git a/src/responder/kcm/kcmsrv_ccache.c b/src/responder/kcm/kcmsrv_ccache.c
index 8c447eacef..cc449877b3 100644
--- a/src/responder/kcm/kcmsrv_ccache.c
+++ b/src/responder/kcm/kcmsrv_ccache.c
@@ -333,12 +333,7 @@ krb5_creds **kcm_cc_unmarshal(TALLOC_CTX *mem_ctx,
cred_list = talloc_zero_array(tmp_ctx, krb5_creds *, count + 1);
for (cred = kcm_cc_get_cred(cc); cred != NULL; cred = kcm_cc_next_cred(cred), i++) {
- ret = get_krb5_data_from_cred(tmp_ctx, cred->cred_blob, &cred_data);
- if (ret != EOK) {
- DEBUG(SSSDBG_CRIT_FAILURE, "Failed to convert cred to krb5_data"
- "[%d]: %s\n", ret, sss_strerror(ret));
- goto done;
- }
+ get_krb5_data_from_cred(cred->cred_blob, &cred_data);
kerr = krb5_unmarshal_credentials(krb_context, &cred_data,
&cred_list[i]);
diff --git a/src/util/sss_krb5.c b/src/util/sss_krb5.c
index cf96f3ffc4..3a2c9d6496 100644
--- a/src/util/sss_krb5.c
+++ b/src/util/sss_krb5.c
@@ -1211,14 +1211,10 @@ static errno_t iobuf_get_len_bytes(TALLOC_CTX *mem_ctx,
return EOK;
}
-errno_t get_krb5_data_from_cred(TALLOC_CTX *mem_ctx,
- struct sss_iobuf *iobuf,
- krb5_data *k5data)
+void get_krb5_data_from_cred(struct sss_iobuf *iobuf, krb5_data *k5data)
{
k5data->data = (char *) sss_iobuf_get_data(iobuf);
k5data->length = sss_iobuf_get_size(iobuf);
-
- return EOK;
}
static errno_t get_krb5_data(TALLOC_CTX *mem_ctx,
diff --git a/src/util/sss_krb5.h b/src/util/sss_krb5.h
index aa1a258eb2..37158d803f 100644
--- a/src/util/sss_krb5.h
+++ b/src/util/sss_krb5.h
@@ -199,8 +199,6 @@ krb5_error_code sss_krb5_unmarshal_princ(TALLOC_CTX *mem_ctx,
krb5_error_code sss_krb5_init_context(krb5_context *context);
-errno_t get_krb5_data_from_cred(TALLOC_CTX *mem_ctx,
- struct sss_iobuf *iobuf,
- krb5_data *k5data);
+void get_krb5_data_from_cred(struct sss_iobuf *iobuf, krb5_data *k5data);
#endif /* __SSS_KRB5_H__ */
From 56af538e35acc643f81ad17ab70042587c199a4f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrez...@redhat.com>
Date: Tue, 7 Sep 2021 11:01:45 +0200
Subject: [PATCH 2/2] kcm: replace existing credentials to avoid unnecessary
ccache growth
Currently, we just append input credential to the ccache. This however
make the ccache grow over time as credentials expires and more control
credentials are stored.
Now we remove or credentials that are the same and overwrite them with
the input credential.
Resolves: https://github.com/SSSD/sssd/issues/5775
:fixes: KCM now replace the old credential with new one when storing
an update credential that is however already present in the ccache
to avoid unnecessary growth of the ccache.
---
src/responder/kcm/kcmsrv_ccache.c | 93 ++++++++++++++++++++++++++++---
src/util/sss_krb5.c | 13 +++++
src/util/sss_krb5.h | 2 +
3 files changed, 99 insertions(+), 9 deletions(-)
diff --git a/src/responder/kcm/kcmsrv_ccache.c b/src/responder/kcm/kcmsrv_ccache.c
index cc449877b3..522ccc138d 100644
--- a/src/responder/kcm/kcmsrv_ccache.c
+++ b/src/responder/kcm/kcmsrv_ccache.c
@@ -253,10 +253,91 @@ static struct kcm_cred *kcm_cred_dup(TALLOC_CTX *mem_ctx,
return dup;
}
+#ifdef HAVE_KRB5_UNMARSHAL_CREDENTIALS
+static krb5_creds *kcm_cred_to_krb5(krb5_context kctx, struct kcm_cred *kcm_crd)
+{
+ krb5_error_code kerr;
+ krb5_creds *kcrd;
+ krb5_data data;
+
+ get_krb5_data_from_cred(kcm_crd->cred_blob, &data);
+
+ kerr = krb5_unmarshal_credentials(kctx, &data, &kcrd);
+ if (kerr != 0) {
+ DEBUG(SSSDBG_CRIT_FAILURE, "Failed to unmarshal credentials\n");
+ return NULL;
+ }
+
+ return kcrd;
+}
+#endif
+
+static errno_t
+kcm_cc_remove_duplicates(struct kcm_ccache *cc,
+ struct kcm_cred *kcm_crd)
+{
+#ifdef HAVE_KRB5_UNMARSHAL_CREDENTIALS
+ struct kcm_cred *p, *q;
+ krb5_error_code kerr;
+ krb5_context kctx;
+ krb5_creds *kcrd_cc;
+ krb5_creds *kcrd;
+ errno_t ret;
+ bool bret;
+
+ kerr = krb5_init_context(&kctx);
+ if (kerr != 0) {
+ DEBUG(SSSDBG_CRIT_FAILURE, "Failed to init krb5 context\n");
+ return EIO;
+ }
+
+ kcrd = kcm_cred_to_krb5(kctx, kcm_crd);
+ if (kcrd == NULL) {
+ DEBUG(SSSDBG_CRIT_FAILURE, "Failed to convert kcm cred to krb5\n");
+ goto done;
+ }
+
+ DLIST_FOR_EACH_SAFE(p, q, cc->creds) {
+ kcrd_cc = kcm_cred_to_krb5(kctx, p);
+ if (kcrd_cc == NULL) {
+ DEBUG(SSSDBG_CRIT_FAILURE, "Failed to convert kcm cred to krb5\n");
+ goto done;
+ }
+
+ bret = sss_krb5_creds_compare(kctx, kcrd, kcrd_cc);
+ krb5_free_creds(kctx, kcrd_cc);
+ if (!bret) {
+ continue;
+ }
+
+ /* This cred is the same ticket. We will replace it with the new one. */
+ DLIST_REMOVE(cc->creds, p);
+ }
+
+ ret = EOK;
+
+done:
+ krb5_free_creds(kctx, kcrd);
+ krb5_free_context(kctx);
+
+ return ret;
+#else
+ return EOK;
+#endif
+}
+
/* Add a cred to ccache */
errno_t kcm_cc_store_creds(struct kcm_ccache *cc,
struct kcm_cred *crd)
{
+ errno_t ret;
+
+ ret = kcm_cc_remove_duplicates(cc, crd);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_MINOR_FAILURE, "Unable to remove duplicate credentials "
+ "[%d]: %s\n", ret, sss_strerror(ret));
+ }
+
DLIST_ADD(cc->creds, crd);
talloc_steal(cc, crd);
return EOK;
@@ -314,10 +395,7 @@ krb5_creds **kcm_cc_unmarshal(TALLOC_CTX *mem_ctx,
#else
TALLOC_CTX *tmp_ctx;
struct kcm_cred *cred;
- krb5_data cred_data;
krb5_creds **cred_list;
- errno_t ret;
- krb5_error_code kerr;
int i = 0;
int count = 0;
@@ -333,12 +411,9 @@ krb5_creds **kcm_cc_unmarshal(TALLOC_CTX *mem_ctx,
cred_list = talloc_zero_array(tmp_ctx, krb5_creds *, count + 1);
for (cred = kcm_cc_get_cred(cc); cred != NULL; cred = kcm_cc_next_cred(cred), i++) {
- get_krb5_data_from_cred(cred->cred_blob, &cred_data);
-
- kerr = krb5_unmarshal_credentials(krb_context, &cred_data,
- &cred_list[i]);
- if (kerr != 0) {
- DEBUG(SSSDBG_CRIT_FAILURE, "Failed to unmarshal credentials\n");
+ cred_list[i] = kcm_cred_to_krb5(krb_context, cred);
+ if (cred_list[i] == NULL) {
+ DEBUG(SSSDBG_CRIT_FAILURE, "Failed to convert kcm cred to krb5\n");
goto done;
}
}
diff --git a/src/util/sss_krb5.c b/src/util/sss_krb5.c
index 3a2c9d6496..940efd381b 100644
--- a/src/util/sss_krb5.c
+++ b/src/util/sss_krb5.c
@@ -1375,3 +1375,16 @@ krb5_error_code sss_krb5_init_context(krb5_context *context)
return kerr;
}
+
+bool sss_krb5_creds_compare(krb5_context kctx, krb5_creds *a, krb5_creds *b)
+{
+ if (!krb5_principal_compare(kctx, a->client, b->client)) {
+ return false;
+ }
+
+ if (!krb5_principal_compare(kctx, a->server, b->server)) {
+ return false;
+ }
+
+ return true;
+}
diff --git a/src/util/sss_krb5.h b/src/util/sss_krb5.h
index 37158d803f..a9521141e4 100644
--- a/src/util/sss_krb5.h
+++ b/src/util/sss_krb5.h
@@ -201,4 +201,6 @@ krb5_error_code sss_krb5_init_context(krb5_context *context);
void get_krb5_data_from_cred(struct sss_iobuf *iobuf, krb5_data *k5data);
+bool sss_krb5_creds_compare(krb5_context kctx, krb5_creds *a, krb5_creds *b);
+
#endif /* __SSS_KRB5_H__ */
_______________________________________________
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
Fedora Code of Conduct:
https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives:
https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org
Do not reply to spam on the list, report it:
https://pagure.io/fedora-infrastructure
