[Freeipa-devel] [PATCH] ipasam SASL bind callback fixes
Alexander Bokovoy
abokovoy at redhat.com
Wed Jul 4 17:57:44 UTC 2012
Hi,
when chasing what looked like ccache corruption with Sumit, I've found
yet another issue: use of local stack variable in long-time living code.
This local stack use was absent in the original patch version and was
proposed by Sumit on one of reviews. It worked for us by luck, it should
not have.
Hence, the patch that fixes the issue by moving service principal to a
longer term storage (ipasam private struct).
In order to avoid ccache corruption we also need to move back to
in-memory ccache. When multiple LSASD and smbd processes try to auth
against LDAP in ipasam, they may write to the same ccache (common
ccache) when another process reads from it. This is not what we need.
And writing to a persistent on-disk ccache is not needed anyway, as
smbldap connections re-authenticate themselves (smbldap connections
expire in few minutes).
The patch also removes kerberos operations that are not needed when
using memory ccache.
--
/ Alexander Bokovoy
-------------- next part --------------
>From 9fc235938045c4d98ca8c6f62fc56da4d7b2280f Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy <abokovoy at redhat.com>
Date: Wed, 4 Jul 2012 20:47:03 +0300
Subject: [PATCH 4/4] ipasam: improve SASL bind callback
SASL bind callback due to refactoring was referencing local variable which
didn't exist all the time. Fix that by including a copy of service principal
into ipasam long term private struct.
Also remove unneeded default ccache management and use in-memory ccache to
avoid stepping over multiple LSASD processes.
---
daemons/ipa-sam/ipa_sam.c | 58 +++++++++++++++++++++------------------------
1 file changed, 27 insertions(+), 31 deletions(-)
diff --git a/daemons/ipa-sam/ipa_sam.c b/daemons/ipa-sam/ipa_sam.c
index 9baac1b2d35a640c36fe7f95a6154ec8582649d8..63f2506a3acdbb739a7cb227bfb6f0ffa723a0ab 100644
--- a/daemons/ipa-sam/ipa_sam.c
+++ b/daemons/ipa-sam/ipa_sam.c
@@ -177,6 +177,7 @@ struct ipasam_privates {
char *trust_dn;
char *flat_name;
char *fallback_primary_group;
+ char *service_principal;
};
static LDAP *priv2ld(struct ldapsam_privates *priv)
@@ -3125,12 +3126,20 @@ static int ldap_sasl_interact(LDAP *ld, unsigned flags, void *priv_data, void *s
return ret;
}
-static void bind_callback_cleanup(struct ipasam_sasl_interact_priv *data)
+static void bind_callback_cleanup(struct ipasam_sasl_interact_priv *data, krb5_error_code rc)
{
+ const char *errstring = NULL;
+
if (!data->context) {
return;
}
+ if (rc) {
+ errstring = krb5_get_error_message(data->context, rc);
+ DEBUG(0,("kerberos error: code=%d, message=%s\n", rc, errstring));
+ krb5_free_error_message(data->context, errstring);
+ }
+
krb5_free_cred_contents(data->context, &data->creds);
if (data->options) {
@@ -3182,59 +3191,38 @@ static int bind_callback(LDAP *ldap_struct, struct smbldap_state *ldap_state, vo
rc = krb5_parse_name(data.context, data.name, &data.principal);
if (rc) {
- bind_callback_cleanup(&data);
+ bind_callback_cleanup(&data, rc);
return LDAP_LOCAL_ERROR;
}
- rc = krb5_cc_default(data.context, &data.ccache);
+ rc = krb5_cc_new_unique(data.context, "MEMORY", NULL, &data.ccache);
if (rc) {
- bind_callback_cleanup(&data);
- return LDAP_LOCAL_ERROR;
- }
-
- rc = krb5_cc_initialize(data.context, data.ccache, data.principal);
- if (rc) {
- bind_callback_cleanup(&data);
- return LDAP_LOCAL_ERROR;
- }
-
- rc = krb5_cc_get_full_name(data.context, data.ccache, &ccache_name);
- if (rc) {
- if (ccache_name) {
- krb5_free_string(data.context, ccache_name);
- }
- bind_callback_cleanup(&data);
- return LDAP_LOCAL_ERROR;
- }
-
- rc = krb5_cc_set_default_name(data.context, ccache_name);
- if (rc) {
- bind_callback_cleanup(&data);
+ bind_callback_cleanup(&data, rc);
return LDAP_LOCAL_ERROR;
}
rc = krb5_kt_resolve(data.context, "FILE:/etc/samba/samba.keytab", &data.keytab);
if (rc) {
- bind_callback_cleanup(&data);
+ bind_callback_cleanup(&data, rc);
return LDAP_LOCAL_ERROR;
}
rc = krb5_get_init_creds_opt_alloc(data.context, &data.options);
if (rc) {
- bind_callback_cleanup(&data);
+ bind_callback_cleanup(&data, rc);
return LDAP_LOCAL_ERROR;
}
rc = krb5_get_init_creds_opt_set_out_ccache(data.context, data.options, data.ccache);
if (rc) {
- bind_callback_cleanup(&data);
+ bind_callback_cleanup(&data, rc);
return LDAP_LOCAL_ERROR;
}
rc = krb5_get_init_creds_keytab(data.context, &data.creds, data.principal, data.keytab,
0, NULL, data.options);
if (rc) {
- bind_callback_cleanup(&data);
+ bind_callback_cleanup(&data, rc);
return LDAP_LOCAL_ERROR;
}
@@ -3247,7 +3235,7 @@ static int bind_callback(LDAP *ldap_struct, struct smbldap_state *ldap_state, vo
DEBUG(0, ("bind_callback: cannot perform interactive SASL bind with GSSAPI\n"));
}
- bind_callback_cleanup(&data);
+ bind_callback_cleanup(&data, 0);
return ret;
}
@@ -3304,13 +3292,21 @@ static NTSTATUS pdb_init_ipasam(struct pdb_methods **pdb_method,
uri, false, bind_dn, bind_secret,
&ldap_state->smbldap_state);
} else {
+ ldap_state->ipasam_privates->service_principal = talloc_strdup(
+ ldap_state->ipasam_privates,
+ service_principal);
+ if (ldap_state->ipasam_privates->service_principal == NULL) {
+ DEBUG(0, ("Failed to create service principal.\n"));
+ return NT_STATUS_NO_MEMORY;
+ }
/* We authenticate via GSSAPI and thus will use kerberos principal to bind our access */
status = smbldap_init(*pdb_method, pdb_get_tevent_context(),
uri, false, NULL, NULL,
&ldap_state->smbldap_state);
if (NT_STATUS_IS_OK(status)) {
ldap_state->smbldap_state->bind_callback = bind_callback;
- ldap_state->smbldap_state->bind_callback_data = service_principal;
+ ldap_state->smbldap_state->bind_callback_data =
+ ldap_state->ipasam_privates->service_principal;
}
}
--
1.7.10.4
More information about the Freeipa-devel
mailing list