[Freeipa-devel] [PATCH] 0055 Add error condition handling to SASL bind callback in ipasam module

[Alexander Bokovoy]

Hi,

attached patch adds comprehensive error condition handling to SASL bind
callback in ipasam module. The callback is doing keytab-based auth
against FreeIPA LDAP server and original version lacked error checks on
purpose.

Now it is time to fix the purpose. :)

-- 
/ Alexander Bokovoy
-------------- next part --------------
>From 041d1b8e49398e991f49b6709d8ae5d57f7dc93f Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy <abokovoy at redhat.com>
Date: Wed, 27 Jun 2012 17:11:33 +0300
Subject: [PATCH 13/13] Add error condition handling to the SASL bind callback
 in ipasam

https://fedorahosted.org/freeipa/ticket/2877
---
 daemons/ipa-sam/ipa_sam.c |   92 +++++++++++++++++++++++++++++++++++++++------
 1 file changed, 80 insertions(+), 12 deletions(-)

diff --git a/daemons/ipa-sam/ipa_sam.c b/daemons/ipa-sam/ipa_sam.c
index f63ea1899e6eb994c1ef03487e0477dac6c7e504..e41cb966837da9dc73ee9a5571caa36e682903cd 100644
--- a/daemons/ipa-sam/ipa_sam.c
+++ b/daemons/ipa-sam/ipa_sam.c
@@ -1453,7 +1453,6 @@ static bool set_krb_princ(struct ldapsam_privates *ldap_state,
 	int ret;
 	uint32_t has_objectclass = 0;
 	NTSTATUS status;
-	char *inp;
 
 	if (!search_krb_princ(ldap_state, mem_ctx, princ, base_dn, &entry)) {
 		return false;
@@ -3102,9 +3101,6 @@ static int ldap_sasl_interact(LDAP *ld, unsigned flags, void *priv_data, void *s
 	sasl_interact_t *in = NULL;
 	int ret = LDAP_OTHER;
 	struct ipasam_sasl_interact_priv *data = (struct ipasam_sasl_interact_priv*) priv_data;
-	krb5_context krbctx;
-	char *outname = NULL;
-	krb5_error_code krberr;
 
 	if (!ld) return LDAP_PARAM_ERROR;
 
@@ -3129,10 +3125,40 @@ 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)
+{
+	if (!data->context) {
+		return;
+	}
+
+	krb5_free_cred_contents(data->context, &data->creds);
+
+	if (data->options) {
+		krb5_get_init_creds_opt_free(data->context, data->options);
+		data->options = NULL;
+	}
+
+	if (data->keytab) {
+		krb5_kt_close(data->context, data->keytab);
+		data->keytab = NULL;
+	}
+
+	if (data->ccache) {
+		krb5_cc_close(data->context, data->ccache);
+		data->ccache = NULL;
+	}
+
+	if (data->principal) {
+		krb5_free_principal(data->context, data->principal);
+		data->principal = NULL;
+	}
+
+	krb5_free_context(data->context);
+	data->context = NULL;
+}
+
 extern const char *lp_parm_const_string(int snum, const char *type, const char *option, const char *def);
-extern void become_root();
-extern void unbecome_root();
-static int bind_callback(LDAP *ldap_struct, struct smbldap_state *ldap_state)
+static int bind_callback(LDAP *ldap_struct, struct smbldap_state *ldap_state, void* ipasam_principal)
 {
 	char *ccache_name = NULL;
 	krb5_error_code rc;
@@ -3140,7 +3166,7 @@ static int bind_callback(LDAP *ldap_struct, struct smbldap_state *ldap_state)
 	struct ipasam_sasl_interact_priv data;
 	int ret;
 
-	data.name = lp_parm_const_string(-1, "ipasam", "principal", NULL);
+	data.name = (const char*)ipasam_principal;
 	if (data.name == NULL) {
 		DEBUG(0, ("bind_callback: ipasam:principal is not set, cannot use GSSAPI bind\n"));
 		return LDAP_LOCAL_ERROR;
@@ -3149,24 +3175,67 @@ static int bind_callback(LDAP *ldap_struct, struct smbldap_state *ldap_state)
 	data.name_len = strlen(data.name);
 
 	rc = krb5_init_context(&data.context);
+	if (rc) {
+		return LDAP_LOCAL_ERROR;
+	}
 
 	rc = krb5_parse_name(data.context, data.name, &data.principal);
+	if (rc) {
+		bind_callback_cleanup(&data);
+		return LDAP_LOCAL_ERROR;
+	}
 
 	rc = krb5_cc_default(data.context, &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);
+		return LDAP_LOCAL_ERROR;
+	}
 
 	rc = krb5_kt_resolve(data.context, "FILE:/etc/samba/samba.keytab", &data.keytab);
+	if (rc) {
+		bind_callback_cleanup(&data);
+		return LDAP_LOCAL_ERROR;
+	}
 
 	rc = krb5_get_init_creds_opt_alloc(data.context, &data.options);
+	if (rc) {
+		bind_callback_cleanup(&data);
+		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);
+		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);
+		return LDAP_LOCAL_ERROR;
+	}
 
 	ret = ldap_sasl_interactive_bind_s(ldap_struct,
 					   NULL, "GSSAPI",
@@ -3177,10 +3246,7 @@ static int bind_callback(LDAP *ldap_struct, struct smbldap_state *ldap_state)
 		DEBUG(0, ("bind_callback: cannot perform interactive SASL bind with GSSAPI\n"));
 	}
 
-	krb5_get_init_creds_opt_free(data.context, data.options);
-	krb5_kt_close(data.context, data.keytab);
-	krb5_cc_close(data.context, data.ccache);
-	krb5_free_context(data.context);
+	bind_callback_cleanup(&data);
 	return ret;
 }
 
@@ -3243,6 +3309,8 @@ static NTSTATUS pdb_init_ipasam(struct pdb_methods **pdb_method,
 			      &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 = 
+						(void*)lp_parm_const_string(-1, "ipasam", "principal", NULL);
 		}
 	}
 
-- 
1.7.10.4