URL: https://github.com/SSSD/sssd/pull/5776
Author: pbrezina
 Title: #5776: kcm: replace existing credentials to avoid unnecessary ccache 
growth
Action: opened

PR body:
"""
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.
"""

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 128f09725cce510cef65aa1e11ae6b487f13fdee 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 4ea38b214d74af920b03e8a8730e227eb8405035 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 | 91 ++++++++++++++++++++++++++++---
 src/util/sss_krb5.c               | 13 +++++
 src/util/sss_krb5.h               |  2 +
 3 files changed, 97 insertions(+), 9 deletions(-)

diff --git a/src/responder/kcm/kcmsrv_ccache.c b/src/responder/kcm/kcmsrv_ccache.c
index cc449877b3..cdc78eb3dd 100644
--- a/src/responder/kcm/kcmsrv_ccache.c
+++ b/src/responder/kcm/kcmsrv_ccache.c
@@ -253,10 +253,89 @@ static struct kcm_cred *kcm_cred_dup(TALLOC_CTX *mem_ctx,
     return dup;
 }
 
+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;
+}
+
+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 +393,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 +409,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

Reply via email to