From c17527f4996614bd7fcefde6e1f8949c61d3c96c Mon Sep 17 00:00:00 2001 From: Ray Strode Date: Thu, 15 Dec 2022 16:27:27 -0500 Subject: [PATCH 1/2] goakerberosidentity: Fix crash when erasing credentials Right now when erasing an identity we erase the active credentials first and then the inactive ones. We neglect to take the active one out of the hash table, though, so it gets destroyed twice. This commit fixes that. --- src/goaidentity/goakerberosidentity.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/goaidentity/goakerberosidentity.c b/src/goaidentity/goakerberosidentity.c index 9e9b8a74..ac550f12 100644 --- a/src/goaidentity/goakerberosidentity.c +++ b/src/goaidentity/goakerberosidentity.c @@ -1686,60 +1686,62 @@ goa_kerberos_identity_renew (GoaKerberosIdentity *self, GError **error) { goto free_principal; } g_debug ("GoaKerberosIdentity: identity %s renewed", name); renewed = TRUE; free_principal: krb5_free_principal (self->kerberos_context, principal); out: g_free (name); return renewed; } gboolean goa_kerberos_identity_erase (GoaKerberosIdentity *self, GError **error) { GHashTableIter iter; const char *name; krb5_ccache credentials_cache; krb5_error_code error_code = 0; if (self->active_credentials_cache_name != NULL) { credentials_cache = (krb5_ccache) g_hash_table_lookup (self->credentials_caches, self->active_credentials_cache_name); g_debug ("GoaKerberosIdentity: Destroying active credentials cache %s", self->active_credentials_cache_name); error_code = krb5_cc_destroy (self->kerberos_context, credentials_cache); + g_hash_table_remove (self->credentials_caches, self->active_credentials_cache_name); + g_clear_pointer (&self->active_credentials_cache_name, g_free); if (error_code != 0) { set_and_prefix_error_from_krb5_error_code (self, error, GOA_IDENTITY_ERROR_REMOVING_CREDENTIALS, error_code, _("Could not erase identity: ")); } } g_hash_table_iter_init (&iter, self->credentials_caches); while (g_hash_table_iter_next (&iter, (gpointer *) &name, (gpointer*) &credentials_cache)) { g_debug ("GoaKerberosIdentity: Destroying inactive credentials cache %s", name); krb5_cc_destroy (self->kerberos_context, credentials_cache); } g_hash_table_remove_all (self->credentials_caches); return error_code == 0; } GoaIdentity * goa_kerberos_identity_new (krb5_context context, krb5_ccache cache, GError **error) { GoaKerberosIdentity *self; krb5_ccache copied_cache; self = GOA_KERBEROS_IDENTITY (g_object_new (GOA_TYPE_KERBEROS_IDENTITY, NULL)); self->kerberos_context = context; -- 2.37.0.rc1 From 2bb898f5137d09e998e2d5ad0127a65df8cf15e0 Mon Sep 17 00:00:00 2001 From: Ray Strode Date: Thu, 15 Dec 2022 15:35:49 -0500 Subject: [PATCH 2/2] goakerberosidentity: Explicitly switch to credentials cache when needed If we're updating a credentials cache and decide it should be the new default for an identity, and the old credentials cache was the default cache for the cache collection then we should make the new credential cache the default cache for the collection, too. This commit adds that. It also makes the new credentials cache the default if there wasn't a valid default set already. This brings consistency to differences in behavior from different kerberos ccache types. --- src/goaidentity/goakerberosidentity.c | 65 +++++++++++++++++++++++++-- 1 file changed, 62 insertions(+), 3 deletions(-) diff --git a/src/goaidentity/goakerberosidentity.c b/src/goaidentity/goakerberosidentity.c index ac550f12..71c1ff09 100644 --- a/src/goaidentity/goakerberosidentity.c +++ b/src/goaidentity/goakerberosidentity.c @@ -1524,98 +1524,157 @@ goa_kerberos_identity_compare (GoaKerberosIdentity *self, if (self->cached_verification_level < new_identity->cached_verification_level) return -100; if (self->cached_verification_level > new_identity->cached_verification_level) return 100; if (self->cached_verification_level != VERIFICATION_LEVEL_SIGNED_IN) return 50; if (self->expiration_time < new_identity->expiration_time) return -10; if (self->expiration_time > new_identity->expiration_time) return 10; if (self->start_time > new_identity->start_time) return -5; if (self->start_time < new_identity->start_time) return 5; if (self->renewal_time < new_identity->renewal_time) return -1; if (self->renewal_time > new_identity->renewal_time) return 1; return 0; } +static char * +get_default_cache_name (GoaKerberosIdentity *self) +{ + int error_code; + krb5_ccache default_cache; + krb5_principal principal; + char *default_cache_name; + char *principal_name; + + error_code = krb5_cc_default (self->kerberos_context, &default_cache); + + if (error_code != 0) + return NULL; + + /* Return NULL if the default cache doesn't pass basic sanity checks + */ + error_code = krb5_cc_get_principal (self->kerberos_context, default_cache, &principal); + + if (error_code != 0) + return NULL; + + error_code = krb5_unparse_name_flags (self->kerberos_context, principal, 0, &principal_name); + krb5_free_principal (self->kerberos_context, principal); + + if (error_code != 0) + return NULL; + + krb5_free_unparsed_name (self->kerberos_context, principal_name); + + default_cache_name = g_strdup (krb5_cc_get_name (self->kerberos_context, default_cache)); + krb5_cc_close (self->kerberos_context, default_cache); + + return default_cache_name; +} + void goa_kerberos_identity_update (GoaKerberosIdentity *self, GoaKerberosIdentity *new_identity) { VerificationLevel old_verification_level, new_verification_level; gboolean time_changed = FALSE; char *preauth_identity_source = NULL; int comparison; + G_LOCK (identity_lock); + + old_verification_level = self->cached_verification_level; + new_verification_level = new_identity->cached_verification_level; + comparison = goa_kerberos_identity_compare (self, new_identity); if (new_identity->active_credentials_cache_name != NULL) { + g_autofree char *default_cache_name = NULL; krb5_ccache credentials_cache; krb5_ccache copied_cache; + gboolean should_switch_to_new_credentials_cache = FALSE; + + default_cache_name = get_default_cache_name (self); + + if (default_cache_name == NULL) + should_switch_to_new_credentials_cache = TRUE; credentials_cache = (krb5_ccache) g_hash_table_lookup (new_identity->credentials_caches, new_identity->active_credentials_cache_name); krb5_cc_dup (new_identity->kerberos_context, credentials_cache, &copied_cache); + if (g_strcmp0 (default_cache_name, self->active_credentials_cache_name) == 0) + { + if ((comparison < 0) || + (comparison == 0 && old_verification_level != VERIFICATION_LEVEL_SIGNED_IN)) + should_switch_to_new_credentials_cache = TRUE; + } + if (comparison < 0) - g_clear_pointer (&self->active_credentials_cache_name, &g_free); + { + g_clear_pointer (&self->active_credentials_cache_name, g_free); + self->active_credentials_cache_name = g_strdup (new_identity->active_credentials_cache_name); + } goa_kerberos_identity_add_credentials_cache (self, copied_cache); + + if (should_switch_to_new_credentials_cache) + krb5_cc_switch (self->kerberos_context, copied_cache); } + G_UNLOCK (identity_lock); clear_alarms (new_identity); if (comparison >= 0) return; G_LOCK (identity_lock); update_identifier (self, new_identity); time_changed |= set_start_time (self, new_identity->start_time); time_changed |= set_renewal_time (self, new_identity->renewal_time); time_changed |= set_expiration_time (self, new_identity->expiration_time); - old_verification_level = self->cached_verification_level; - new_verification_level = new_identity->cached_verification_level; G_UNLOCK (identity_lock); if (time_changed) { if (new_verification_level == VERIFICATION_LEVEL_SIGNED_IN) reset_alarms (self); else clear_alarms (self); } G_LOCK (identity_lock); g_free (self->preauth_identity_source); self->preauth_identity_source = preauth_identity_source; G_UNLOCK (identity_lock); if (new_verification_level != old_verification_level) { if (old_verification_level == VERIFICATION_LEVEL_SIGNED_IN && new_verification_level == VERIFICATION_LEVEL_EXISTS) { G_LOCK (identity_lock); self->cached_verification_level = new_verification_level; G_UNLOCK (identity_lock); g_signal_emit (G_OBJECT (self), signals[EXPIRED], 0); } else if (old_verification_level == VERIFICATION_LEVEL_EXISTS && new_verification_level == VERIFICATION_LEVEL_SIGNED_IN) { G_LOCK (identity_lock); -- 2.37.0.rc1