[Freeipa-devel] [PATCH] ipasam SASL bind callback fixes
Alexander Bokovoy
abokovoy at redhat.com
Wed Jul 4 20:10:25 UTC 2012
On Wed, 04 Jul 2012, Sumit Bose wrote:
>On Wed, Jul 04, 2012 at 08:57:44PM +0300, Alexander Bokovoy wrote:
>> 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
>
>This patch works for me and I have no objections using the in-memory
>ccache, so ACK from my side, but please wait for Simo's answer before
>pushing.
>
>Simo, you were in favour of using the default cache, do you
>agree as well?
>
>Before pushing please fix:
>
>ipa_sam.c:3164:8: warning: unused variable 'ccache_name' [-Wunused-variable]
After talking with Sumit and researching I've changed back to use
on-disk ccache and avoid re-initing on every callback call.
Attached patch should be good. The only trouble is that you'll need to
re-run 'ipa-adtrust-install' as smb.conf's parameter we use has been
changed to a different one. That or you can manually run
net conf setparm 'global' 'ipasam:principal template' '$FQDN@$REALM'
where $FQDN is the fully-qualified domain name of the IPA server where
smbd will run and $REALM is IPA realm name.
Note that I had to do this change to avoid manipulating by ipasam:principal
name as I need both client and server principals to use krb5_get_credentials().
With this change ipasam will build proper principal names by itself.
Note that we cannot use use ipasam_privates.realm value because it is
not available at the point when callback is called. It is fetched from
ldap later, after callback has been successfully used.
--
/ Alexander Bokovoy
-------------- next part --------------
>From de956384778bc7ebae8eec80c8e2127ed25f87df 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 2/2] 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.
Rework ccache handling to avoid re-initing every time callback is called
Please note that you need to re-run ipa-adtrust-install as smb.conf option
and its meaning has been changed (ipasam:principal vs ipasam:principal template)
---
daemons/ipa-sam/ipa_sam.c | 117 +++++++++++++++++++++++++--------------
install/share/smb.conf.template | 2 +-
2 files changed, 76 insertions(+), 43 deletions(-)
diff --git a/daemons/ipa-sam/ipa_sam.c b/daemons/ipa-sam/ipa_sam.c
index 9baac1b2d35a640c36fe7f95a6154ec8582649d8..4666e0f02c19c124f858b0c491af27f42e0a1519 100644
--- a/daemons/ipa-sam/ipa_sam.c
+++ b/daemons/ipa-sam/ipa_sam.c
@@ -177,6 +177,8 @@ struct ipasam_privates {
char *trust_dn;
char *flat_name;
char *fallback_primary_group;
+ char *server_princ;
+ char *client_princ;
};
static LDAP *priv2ld(struct ldapsam_privates *priv)
@@ -3125,12 +3127,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) {
@@ -3158,21 +3168,27 @@ static void bind_callback_cleanup(struct ipasam_sasl_interact_priv *data)
}
extern const char *lp_parm_const_string(int snum, const char *type, const char *option, const char *def);
-static int bind_callback(LDAP *ldap_struct, struct smbldap_state *ldap_state, void* ipasam_principal)
+static int bind_callback(LDAP *ldap_struct, struct smbldap_state *ldap_state, void* ipasam_priv)
{
- char *ccache_name = NULL;
krb5_error_code rc;
+ krb5_creds *out_creds = NULL;
+ krb5_creds in_creds;
struct ipasam_sasl_interact_priv data;
+ struct ipasam_privates *ipasam_private = NULL;
int ret;
memset(&data, 0, sizeof(struct ipasam_sasl_interact_priv));
- data.name = (const char*)ipasam_principal;
- if (data.name == NULL) {
- DEBUG(0, ("bind_callback: ipasam:principal is not set, cannot use GSSAPI bind\n"));
+ memset(&in_creds, 0, sizeof(krb5_creds));
+
+ ipasam_private = (struct ipasam_privates*)ipasam_priv;
+
+ if ((ipasam_private->client_princ == NULL) && (ipasam_private->server_princ == NULL)) {
+ DEBUG(0, ("bind_callback: 'ipasam:principal template' is not set, cannot use GSSAPI bind\n"));
return LDAP_LOCAL_ERROR;
}
+ data.name = ipasam_private->client_princ;
data.name_len = strlen(data.name);
rc = krb5_init_context(&data.context);
@@ -3182,60 +3198,60 @@ 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);
- 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);
+ rc = krb5_parse_name(data.context, ipasam_private->client_princ, &in_creds.client);
if (rc) {
- bind_callback_cleanup(&data);
+ krb5_free_principal(data.context, data.creds.client);
+ bind_callback_cleanup(&data, rc);
return LDAP_LOCAL_ERROR;
}
- rc = krb5_get_init_creds_opt_set_out_ccache(data.context, data.options, data.ccache);
+ rc = krb5_parse_name(data.context, ipasam_private->server_princ, &in_creds.server);
if (rc) {
- bind_callback_cleanup(&data);
+ krb5_free_principal(data.context, in_creds.server);
+ 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);
+ rc = krb5_get_credentials(data.context, KRB5_GC_CACHED, data.ccache, &in_creds, &out_creds);
+ krb5_free_principal(data.context, in_creds.server);
+ krb5_free_principal(data.context, in_creds.client);
+
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, 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, 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, rc);
+ return LDAP_LOCAL_ERROR;
+ }
}
ret = ldap_sasl_interactive_bind_s(ldap_struct,
@@ -3247,7 +3263,10 @@ 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);
+ if (out_creds) {
+ krb5_free_creds(data.context, out_creds);
+ }
+ bind_callback_cleanup(&data, 0);
return ret;
}
@@ -3263,7 +3282,7 @@ static NTSTATUS pdb_init_ipasam(struct pdb_methods **pdb_method,
struct dom_sid ldap_domain_sid;
char *bind_dn = NULL;
char *bind_secret = NULL;
- const char *service_principal = NULL;
+ const char *princ_template = NULL;
LDAPMessage *result = NULL;
LDAPMessage *entry = NULL;
@@ -3293,9 +3312,9 @@ static NTSTATUS pdb_init_ipasam(struct pdb_methods **pdb_method,
}
trim_char( uri, '\"', '\"' );
- service_principal = lp_parm_const_string(-1, "ipasam", "principal", NULL);
+ princ_template = lp_parm_const_string(-1, "ipasam", "principal template", NULL);
- if (service_principal == NULL) {
+ if (princ_template == NULL) {
if (!fetch_ldap_pw(&bind_dn, &bind_secret)) {
DEBUG(0, ("pdb_init_ipasam: Failed to retrieve LDAP password from secrets.tdb\n"));
return NT_STATUS_NO_MEMORY;
@@ -3304,13 +3323,27 @@ 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->client_princ = talloc_asprintf(ldap_state->ipasam_privates,
+ "cifs/%s",
+ princ_template);
+ if (ldap_state->ipasam_privates->client_princ == NULL) {
+ DEBUG(0, ("Failed to create ipasam client principal out of the template.\n"));
+ return NT_STATUS_NO_MEMORY;
+ }
+ ldap_state->ipasam_privates->server_princ = talloc_asprintf(ldap_state->ipasam_privates,
+ "ldap/%s",
+ princ_template);
+ if (ldap_state->ipasam_privates->client_princ == NULL) {
+ DEBUG(0, ("Failed to create ipasam server principal out of the template.\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;
}
}
diff --git a/install/share/smb.conf.template b/install/share/smb.conf.template
index 3107350aa6e94514354b73f0152846e1d01e1e68..e0954e1661542eb43ee78adb4675cc2e18250dcc 100644
--- a/install/share/smb.conf.template
+++ b/install/share/smb.conf.template
@@ -18,7 +18,7 @@ ldap suffix = $SUFFIX
ldap user suffix = cn=users,cn=accounts
ldap group suffix = cn=groups,cn=accounts
ldap machine suffix = cn=computers,cn=accounts
-ipasam:principal = cifs/$FQDN@$REALM
+ipasam:principal template = $FQDN@$REALM
rpc_server:epmapper = external
rpc_server:lsarpc = external
rpc_server:lsass = external
--
1.7.10.4
More information about the Freeipa-devel
mailing list