From 713bc782502163251ef22eb81b09eed61a8407f7 Mon Sep 17 00:00:00 2001

From: Sumit Bose <sbose@redhat.com>

Date: Tue, 5 Jun 2018 17:44:59 +0200

Subject: [PATCH] krb5: refactor removal of krb5info files

Currently a persistent offline callback removes the krb5info files for

the configured main domain and those files were removed by a SIGTERM

signal handlers as well.

This does not scale if krb5info files are created for sub-domains as

well. To remove the files automatically the removal is moved into a

talloc destructor of an offline callback which is added if the file is

created and frees itself when the system goes offline. Due to the

talloc memory hierarchy we get removal on shutdown for free.

Related to https://pagure.io/SSSD/sssd/issue/3652

Reviewed-by: Jakub Hrozek <jhrozek@redhat.com>

(cherry picked from commit d91661e295c8e878f1bbf34e6f65f61e8301bf0e)

---

 src/providers/ad/ad_common.c          |   7 +-

 src/providers/ipa/ipa_common.c        |   5 +-

 src/providers/krb5/krb5_common.c      | 176 +++++++++++++-------------

 src/providers/krb5/krb5_common.h      |   7 +-

 src/providers/krb5/krb5_init_shared.c |   6 -

 src/providers/ldap/ldap_common.c      |  87 -------------

 6 files changed, 102 insertions(+), 186 deletions(-)

diff --git a/src/providers/ad/ad_common.c b/src/providers/ad/ad_common.c

index 0aea985e00faa996643fd7e7630d4264fb6cf233..8caaba6c0d06cfe83d9741536192d662fc936273 100644

--- a/src/providers/ad/ad_common.c

+++ b/src/providers/ad/ad_common.c

@@ -804,6 +804,8 @@ ad_failover_init(TALLOC_CTX *mem_ctx, struct be_ctx *bectx,

         goto done;

     }

+    service->krb5_service->be_ctx = bectx;

+

     if (!primary_servers) {

         DEBUG(SSSDBG_CONF_SETTINGS,

               "No primary servers defined, using service discovery\n");

@@ -984,8 +986,9 @@ ad_resolve_callback(void *private_data, struct fo_server *server)

             goto done;

         }

-        ret = write_krb5info_file(service->krb5_service->realm, safe_address,

-                                SSS_KRB5KDC_FO_SRV);

+        ret = write_krb5info_file(service->krb5_service,

+                                  safe_address,

+                                  SSS_KRB5KDC_FO_SRV);

         if (ret != EOK) {

             DEBUG(SSSDBG_MINOR_FAILURE,

                 "write_krb5info_file failed, authentication might fail.\n");

diff --git a/src/providers/ipa/ipa_common.c b/src/providers/ipa/ipa_common.c

index 87ed967673358bf833dae13c29b1f6a17b0fc19c..dcbb54a744358718e444972b9827ee64887e5e33 100644

--- a/src/providers/ipa/ipa_common.c

+++ b/src/providers/ipa/ipa_common.c

@@ -838,7 +838,8 @@ static void ipa_resolve_callback(void *private_data, struct fo_server *server)

             return;

         }

-        ret = write_krb5info_file(service->krb5_service->realm, safe_address,

+        ret = write_krb5info_file(service->krb5_service,

+                                  safe_address,

                                   SSS_KRB5KDC_FO_SRV);

         if (ret != EOK) {

             DEBUG(SSSDBG_OP_FAILURE,

@@ -1012,6 +1013,8 @@ int ipa_service_init(TALLOC_CTX *memctx, struct be_ctx *ctx,

         goto done;

     }

+    service->krb5_service->be_ctx = ctx;

+

     if (!primary_servers) {

         DEBUG(SSSDBG_CONF_SETTINGS,

               "No primary servers defined, using service discovery\n");

diff --git a/src/providers/krb5/krb5_common.c b/src/providers/krb5/krb5_common.c

index 520e7591ce1b37b4a8dea357b6dd0ec7afd76f58..c6896a6cd663da896075e72aa0a0602c198b45e8 100644

--- a/src/providers/krb5/krb5_common.c

+++ b/src/providers/krb5/krb5_common.c

@@ -389,7 +389,76 @@ done:

     return ret;

 }

-errno_t write_krb5info_file(const char *realm, const char *server,

+static int remove_info_files_destructor(void *p)

+{

+    int ret;

+    struct remove_info_files_ctx *ctx = talloc_get_type(p,

+                                                  struct remove_info_files_ctx);

+

+    ret = remove_krb5_info_files(ctx, ctx->realm);

+    if (ret != EOK) {

+        DEBUG(SSSDBG_CRIT_FAILURE, "remove_krb5_info_files failed.\n");

+    }

+

+    return 0;

+}

+

+static errno_t

+krb5_add_krb5info_offline_callback(struct krb5_service *krb5_service)

+{

+    int ret;

+    struct remove_info_files_ctx *ctx;

+

+    if (krb5_service == NULL || krb5_service->name == NULL

+                             || krb5_service->realm == NULL

+                             || krb5_service->be_ctx == NULL) {

+        DEBUG(SSSDBG_CRIT_FAILURE, "Missing KDC service name or realm!\n");

+        return EINVAL;

+    }

+

+    ctx = talloc_zero(krb5_service->be_ctx, struct remove_info_files_ctx);

+    if (ctx == NULL) {

+        DEBUG(SSSDBG_CRIT_FAILURE, "talloc_zfree failed.\n");

+        return ENOMEM;

+    }

+

+    ctx->realm = talloc_strdup(ctx, krb5_service->realm);

+    if (ctx->realm == NULL) {

+        DEBUG(SSSDBG_CRIT_FAILURE, "talloc_strdup failed!\n");

+        ret = ENOMEM;

+        goto done;

+    }

+

+    ctx->be_ctx = krb5_service->be_ctx;

+    ctx->kdc_service_name = talloc_strdup(ctx, krb5_service->name);

+    if (ctx->kdc_service_name == NULL) {

+        DEBUG(SSSDBG_CRIT_FAILURE, "talloc_strdup failed!\n");

+        ret = ENOMEM;

+        goto done;

+    }

+

+    ret = be_add_offline_cb(ctx, krb5_service->be_ctx,

+                            remove_krb5_info_files_callback, ctx, NULL);

+    if (ret != EOK) {

+        DEBUG(SSSDBG_CRIT_FAILURE, "be_add_offline_cb failed.\n");

+        goto done;

+    }

+

+    talloc_set_destructor((TALLOC_CTX *) ctx, remove_info_files_destructor);

+

+    ret = EOK;

+

+done:

+    if (ret != EOK) {

+        talloc_zfree(ctx);

+    }

+

+    return ret;

+}

+

+

+errno_t write_krb5info_file(struct krb5_service *krb5_service,

+                            const char *server,

                             const char *service)

 {

     int ret;

@@ -401,17 +470,19 @@ errno_t write_krb5info_file(const char *realm, const char *server,

     size_t server_len;

     ssize_t written;

-    if (realm == NULL || *realm == '\0' || server == NULL || *server == '\0' ||

-        service == NULL || *service == '\0') {

+    if (krb5_service == NULL || krb5_service->realm == NULL

+                             || *krb5_service->realm == '\0'

+                             || server == NULL || *server == '\0'

+                             || service == NULL || *service == '\0') {

         DEBUG(SSSDBG_CRIT_FAILURE,

               "Missing or empty realm, server or service.\n");

         return EINVAL;

     }

-    if (sss_krb5_realm_has_proxy(realm)) {

+    if (sss_krb5_realm_has_proxy(krb5_service->realm)) {

         DEBUG(SSSDBG_CONF_SETTINGS,

               "KDC Proxy available for realm [%s], no kdcinfo file created.\n",

-              realm);

+              krb5_service->realm);

         return EOK;

     }

@@ -439,7 +510,7 @@ errno_t write_krb5info_file(const char *realm, const char *server,

         goto done;

     }

-    krb5info_name = talloc_asprintf(tmp_ctx, name_tmpl, realm);

+    krb5info_name = talloc_asprintf(tmp_ctx, name_tmpl, krb5_service->realm);

     if (krb5info_name == NULL) {

         DEBUG(SSSDBG_CRIT_FAILURE, "talloc_asprintf failed.\n");

         ret = ENOMEM;

@@ -495,6 +566,12 @@ errno_t write_krb5info_file(const char *realm, const char *server,

         goto done;

     }

+    ret = krb5_add_krb5info_offline_callback(krb5_service);

+    if (ret != EOK) {

+        DEBUG(SSSDBG_OP_FAILURE, "Failed to add offline callback, krb5info "

+                                 "file might not be removed properly.\n");

+    }

+

     ret = EOK;

 done:

     if (fd != -1) {

@@ -561,7 +638,8 @@ static void krb5_resolve_callback(void *private_data, struct fo_server *server)

             return;

         }

-        ret = write_krb5info_file(krb5_service->realm, safe_address,

+        ret = write_krb5info_file(krb5_service,

+                                  safe_address,

                                   krb5_service->name);

         if (ret != EOK) {

             DEBUG(SSSDBG_OP_FAILURE,

@@ -761,6 +839,7 @@ int krb5_service_init(TALLOC_CTX *memctx, struct be_ctx *ctx,

     }

     service->write_kdcinfo = use_kdcinfo;

+    service->be_ctx = ctx;

     if (!primary_servers) {

         DEBUG(SSSDBG_CONF_SETTINGS,

@@ -839,7 +918,6 @@ errno_t remove_krb5_info_files(TALLOC_CTX *mem_ctx, const char *realm)

 void remove_krb5_info_files_callback(void *pvt)

 {

     int ret;

-    TALLOC_CTX *tmp_ctx = NULL;

     struct remove_info_files_ctx *ctx = talloc_get_type(pvt,

                                                   struct remove_info_files_ctx);

@@ -864,19 +942,10 @@ void remove_krb5_info_files_callback(void *pvt)

         }

     }

-    tmp_ctx = talloc_new(NULL);

-    if (tmp_ctx == NULL) {

-        DEBUG(SSSDBG_CRIT_FAILURE,

-              "talloc_new failed, cannot remove krb5 info files.\n");

-        return;

-    }

-

-    ret = remove_krb5_info_files(tmp_ctx, ctx->realm);

-    if (ret != EOK) {

-        DEBUG(SSSDBG_CRIT_FAILURE, "remove_krb5_info_files failed.\n");

-    }

-

-    talloc_zfree(tmp_ctx);

+    /* Freeing the remove_info_files_ctx will remove the related krb5info

+     * file. Additionally the callback from the list of callbacks is removed,

+     * it will be added again when a new krb5info file is created. */

+    talloc_free(ctx);

 }

 void krb5_finalize(struct tevent_context *ev,

@@ -886,74 +955,9 @@ void krb5_finalize(struct tevent_context *ev,

                    void *siginfo,

                    void *private_data)

 {

-    char *realm = (char *)private_data;

-    int ret;

-

-    ret = remove_krb5_info_files(se, realm);

-    if (ret != EOK) {

-        DEBUG(SSSDBG_CRIT_FAILURE, "remove_krb5_info_files failed.\n");

-    }

-

     orderly_shutdown(0);

 }

-errno_t krb5_install_offline_callback(struct be_ctx *be_ctx,

-                                      struct krb5_ctx *krb5_ctx)

-{

-    int ret;

-    struct remove_info_files_ctx *ctx;

-    const char *krb5_realm;

-

-    if (krb5_ctx->service == NULL || krb5_ctx->service->name == NULL) {

-        DEBUG(SSSDBG_CRIT_FAILURE, "Missing KDC service name!\n");

-        return EINVAL;

-    }

-

-    ctx = talloc_zero(krb5_ctx, struct remove_info_files_ctx);

-    if (ctx == NULL) {

-        DEBUG(SSSDBG_CRIT_FAILURE, "talloc_zfree failed.\n");

-        return ENOMEM;

-    }

-

-    krb5_realm = dp_opt_get_cstring(krb5_ctx->opts, KRB5_REALM);

-    if (krb5_realm == NULL) {

-        DEBUG(SSSDBG_CRIT_FAILURE, "Missing krb5_realm option!\n");

-        ret = EINVAL;

-        goto done;

-    }

-

-    ctx->realm = talloc_strdup(ctx, krb5_realm);

-    if (ctx->realm == NULL) {

-        DEBUG(SSSDBG_CRIT_FAILURE, "talloc_strdup failed!\n");

-        ret = ENOMEM;

-        goto done;

-    }

-

-    ctx->be_ctx = be_ctx;

-    ctx->kdc_service_name = krb5_ctx->service->name;

-    if (krb5_ctx->kpasswd_service == NULL) {

-        ctx->kpasswd_service_name =NULL;

-    } else {

-        ctx->kpasswd_service_name = krb5_ctx->kpasswd_service->name;

-    }

-

-    ret = be_add_offline_cb(ctx, be_ctx, remove_krb5_info_files_callback, ctx,

-                            NULL);

-    if (ret != EOK) {

-        DEBUG(SSSDBG_CRIT_FAILURE, "be_add_offline_cb failed.\n");

-        goto done;

-    }

-

-    ret = EOK;

-

-done:

-    if (ret != EOK) {

-        talloc_zfree(ctx);

-    }

-

-    return ret;

-}

-

 errno_t krb5_install_sigterm_handler(struct tevent_context *ev,

                                      struct krb5_ctx *krb5_ctx)

 {

diff --git a/src/providers/krb5/krb5_common.h b/src/providers/krb5/krb5_common.h

index 48368a528e75947102c74cb75bf7a74ec0dd258f..a2e47b0605debdffa28305dab4f7674707f713ac 100644

--- a/src/providers/krb5/krb5_common.h

+++ b/src/providers/krb5/krb5_common.h

@@ -67,6 +67,7 @@ enum krb5_opts {

 typedef enum { INIT_PW, INIT_KT, RENEW, VALIDATE } action_type;

 struct krb5_service {

+    struct be_ctx *be_ctx;

     char *name;

     char *realm;

     bool write_kdcinfo;

@@ -157,7 +158,8 @@ errno_t krb5_try_kdcip(struct confdb_ctx *cdb, const char *conf_path,

 errno_t sss_krb5_get_options(TALLOC_CTX *memctx, struct confdb_ctx *cdb,

                              const char *conf_path, struct dp_option **_opts);

-errno_t write_krb5info_file(const char *realm, const char *kdc,

+errno_t write_krb5info_file(struct krb5_service *krb5_service,

+                            const char *server,

                             const char *service);

 int krb5_service_init(TALLOC_CTX *memctx, struct be_ctx *ctx,

@@ -177,9 +179,6 @@ void krb5_finalize(struct tevent_context *ev,

                    void *siginfo,

                    void *private_data);

-errno_t krb5_install_offline_callback(struct be_ctx *be_ctx,

-                                      struct krb5_ctx *krb_ctx);

-

 errno_t krb5_install_sigterm_handler(struct tevent_context *ev,

                                      struct krb5_ctx *krb5_ctx);

diff --git a/src/providers/krb5/krb5_init_shared.c b/src/providers/krb5/krb5_init_shared.c

index 3901b7272119c32930c2b6b47279a2c685bf3cfb..368d6f7b0f2bc038e4cc4aa8f0970cd0e81d7b6b 100644

--- a/src/providers/krb5/krb5_init_shared.c

+++ b/src/providers/krb5/krb5_init_shared.c

@@ -71,12 +71,6 @@ errno_t krb5_child_init(struct krb5_ctx *krb5_auth_ctx,

         goto done;

     }

-    ret = krb5_install_offline_callback(bectx, krb5_auth_ctx);

-    if (ret != EOK) {

-        DEBUG(SSSDBG_CRIT_FAILURE, "krb5_install_offline_callback failed.\n");

-        goto done;

-    }

-

     ret = krb5_install_sigterm_handler(bectx->ev, krb5_auth_ctx);

     if (ret != EOK) {

         DEBUG(SSSDBG_CRIT_FAILURE, "krb5_install_sigterm_handler failed.\n");

diff --git a/src/providers/ldap/ldap_common.c b/src/providers/ldap/ldap_common.c

index 91e229243b9a1b43e7a57704824f5db0341f4ee9..15377ee1f062c0167aabee30ef0757ebe7271682 100644

--- a/src/providers/ldap/ldap_common.c

+++ b/src/providers/ldap/ldap_common.c

@@ -158,14 +158,6 @@ static void sdap_finalize(struct tevent_context *ev,

                           void *siginfo,

                           void *private_data)

 {

-    char *realm = (char *) private_data;

-    int ret;

-

-    ret = remove_krb5_info_files(se, realm);

-    if (ret != EOK) {

-        DEBUG(SSSDBG_CRIT_FAILURE, "remove_krb5_info_files failed.\n");

-    }

-

     orderly_shutdown(0);

 }

@@ -196,78 +188,6 @@ errno_t sdap_install_sigterm_handler(TALLOC_CTX *mem_ctx,

     return EOK;

 }

-void sdap_remove_kdcinfo_files_callback(void *pvt)

-{

-    int ret;

-    TALLOC_CTX *tmp_ctx = NULL;

-    struct remove_info_files_ctx *ctx = talloc_get_type(pvt,

-                                                  struct remove_info_files_ctx);

-

-    ret = be_fo_run_callbacks_at_next_request(ctx->be_ctx,

-                                              ctx->kdc_service_name);

-    if (ret != EOK) {

-        DEBUG(SSSDBG_CRIT_FAILURE,

-              "be_fo_run_callbacks_at_next_request failed, "

-                  "krb5 info files will not be removed, because "

-                  "it is unclear if they will be recreated properly.\n");

-        return;

-    }

-

-    tmp_ctx = talloc_new(NULL);

-    if (tmp_ctx == NULL) {

-        DEBUG(SSSDBG_CRIT_FAILURE,

-              "talloc_new failed, cannot remove krb5 info files.\n");

-        return;

-    }

-

-    ret = remove_krb5_info_files(tmp_ctx, ctx->realm);

-    if (ret != EOK) {

-        DEBUG(SSSDBG_CRIT_FAILURE, "remove_krb5_info_files failed.\n");

-    }

-

-    talloc_zfree(tmp_ctx);

-}

-

-

-errno_t sdap_install_offline_callback(TALLOC_CTX *mem_ctx,

-                                      struct be_ctx *be_ctx,

-                                      const char *realm,

-                                      const char *service_name)

-{

-    int ret;

-    struct remove_info_files_ctx *ctx;

-

-    ctx = talloc_zero(mem_ctx, struct remove_info_files_ctx);

-    if (ctx == NULL) {

-        DEBUG(SSSDBG_CRIT_FAILURE, "talloc_zfree failed.\n");

-        return ENOMEM;

-    }

-

-    ctx->be_ctx = be_ctx;

-    ctx->realm = talloc_strdup(ctx, realm);

-    ctx->kdc_service_name = talloc_strdup(ctx, service_name);

-    if (ctx->realm == NULL || ctx->kdc_service_name == NULL) {

-        DEBUG(SSSDBG_CRIT_FAILURE, "talloc_strdup failed!\n");

-        ret = ENOMEM;

-        goto done;

-    }

-

-    ret = be_add_offline_cb(ctx, be_ctx,

-                            sdap_remove_kdcinfo_files_callback,

-                            ctx, NULL);

-    if (ret != EOK) {

-        DEBUG(SSSDBG_CRIT_FAILURE, "be_add_offline_cb failed.\n");

-        goto done;

-    }

-

-    ret = EOK;

-done:

-    if (ret != EOK) {

-        talloc_zfree(ctx);

-    }

-    return ret;

-}

-

 errno_t

 sdap_set_sasl_options(struct sdap_options *id_opts,

                       char *default_primary,

@@ -458,13 +378,6 @@ int sdap_gssapi_init(TALLOC_CTX *mem_ctx,

         goto done;

     }

-    ret = sdap_install_offline_callback(mem_ctx, bectx,

-                                        krb5_realm, SSS_KRB5KDC_FO_SRV);

-    if (ret != EOK) {

-        DEBUG(SSSDBG_FATAL_FAILURE, "Failed to install sigterm handler\n");

-        goto done;

-    }

-

     sdap_service->kinit_service_name = talloc_strdup(sdap_service,

                                                      service->name);

     if (sdap_service->kinit_service_name == NULL) {

-- 

2.17.1