Attached find a patch in the proper git format.
Adam can you push it if you think it is ok ?
Thanks,
Simo.
On Fri, 17 Dec 2010 18:15:22 +0100
Zoran Pericic <zperi...@inet.hr> wrote:
> On 12/16/2010 08:25 PM, Simo Sorce wrote:
>
> >> + (str_casecmp_char(ldap_inst->sasl_mech, "GSSAPI") == 0)) {
> >> + if((ldap_inst->krb5_principal == NULL)&&
> >> + (str_len(ldap_inst->krb5_principal) == 0)) {
> >> + if((ldap_inst->sasl_user == NULL)&&
> >> + (str_len(ldap_inst->sasl_user) == 0)) {
> > the above 2 statements seem wrong to me, the original one had:
> > if ((cond 1) || (cond 2)) {
> > while you changed it into:
> > if ((cond 1)&& (cond 2)) {
> > This fails to do the check that is intended.
> You are right. This is bug.
>
> >> + char hostname[255];
> >> + if(gethostname(hostname, 255) != 0) {
> >> + log_error("SASL mech GSSAPI defined but
> >> krb5_principal and sasl_user are empty. Could not get hostname");
> >> + result = ISC_R_FAILURE;
> >> + goto cleanup;
> >> + } else {
> >> + str_sprintf(ldap_inst->krb5_principal,
> >> "dns/%s", hostname);
> > This should probably be "DNS/%s", Kerberos is generally case
> > sensitive and t5he default for bind is to use the service name part
> > in all caps.
> ACK.
>
> Best regards,
>
> Zoran Pericic
>
> ---
> diff --git a/src/ldap_helper.c b/src/ldap_helper.c
> index
> 5eed8afba7a275a6ebb3a28c707639516ba9af41..d767571daa8e747833d598876541996280544916
> 100644 --- a/src/ldap_helper.c +++ b/src/ldap_helper.c
> @@ -128,6 +128,7 @@ struct ldap_instance {
> ldap_auth_t auth_method;
> ld_string_t *bind_dn;
> ld_string_t *password;
> + ld_string_t *krb5_principal;
> ld_string_t *sasl_mech;
> ld_string_t *sasl_user;
> ld_string_t *sasl_auth_name;
> @@ -293,6 +294,7 @@ new_ldap_instance(isc_mem_t *mctx, const char
> *db_name, { "auth_method", default_string("none") },
> { "bind_dn",
> default_string("") }, { "password",
> default_string("") },
> + { "krb5_principal",
> default_string("") }, { "sasl_mech",
> default_string("GSSAPI") }, { "sasl_user",
> default_string("") }, { "sasl_auth_name",
> default_string("") }, @@ -330,6 +332,7 @@
> new_ldap_instance(isc_mem_t *mctx, const char *db_name,
> CHECK(str_new(mctx,&ldap_inst->base));
> CHECK(str_new(mctx,&ldap_inst->bind_dn));
> CHECK(str_new(mctx,&ldap_inst->password));
> + CHECK(str_new(mctx,&ldap_inst->krb5_principal));
> CHECK(str_new(mctx,&ldap_inst->sasl_mech));
> CHECK(str_new(mctx,&ldap_inst->sasl_user));
> CHECK(str_new(mctx,&ldap_inst->sasl_auth_name));
> @@ -346,6 +349,7 @@ new_ldap_instance(isc_mem_t *mctx, const char
> *db_name, ldap_settings[i++].target = auth_method_str;
> ldap_settings[i++].target = ldap_inst->bind_dn;
> ldap_settings[i++].target = ldap_inst->password;
> + ldap_settings[i++].target = ldap_inst->krb5_principal;
> ldap_settings[i++].target = ldap_inst->sasl_mech;
> ldap_settings[i++].target = ldap_inst->sasl_user;
> ldap_settings[i++].target = ldap_inst->sasl_auth_name;
> @@ -381,12 +385,26 @@ new_ldap_instance(isc_mem_t *mctx, const char
> *db_name,
>
> /* check we have the right data when SASL/GSSAPI is
> selected */ if ((ldap_inst->auth_method == AUTH_SASL)&&
> - (str_casecmp_char(ldap_inst->sasl_mech, "GSSAPI") ==
> 0)) {
> - if ((ldap_inst->sasl_user == NULL) ||
> - (str_len(ldap_inst->sasl_user) == 0)) {
> - log_error("Sasl mech GSSAPI defined but
> sasl_user is empty");
> - result = ISC_R_FAILURE;
> - goto cleanup;
> + (str_casecmp_char(ldap_inst->sasl_mech, "GSSAPI") ==
> 0)) {
> + if ((ldap_inst->krb5_principal == NULL) ||
> + (str_len(ldap_inst->krb5_principal) == 0)) {
> + if ((ldap_inst->sasl_user == NULL) ||
> + (str_len(ldap_inst->sasl_user) ==
> 0)) {
> + char hostname[255];
> + if (gethostname(hostname, 255) != 0)
> {
> + log_error("SASL mech GSSAPI
> defined but krb5_principal"
> + "and sasl_user are
> empty. Could not get hostname");
> + result = ISC_R_FAILURE;
> + goto cleanup;
> + } else {
> +
> str_sprintf(ldap_inst->krb5_principal, "DNS/%s", hostname);
> + log_debug(2, "SASL mech
> GSSAPI defined but krb5_principal"
> + "and sasl_user are
> empty, using default %s",
> +
> str_buf(ldap_inst->krb5_principal));
> + }
> + } else {
> + str_copy(ldap_inst->krb5_principal,
> ldap_inst->sasl_user);
> + }
> }
> }
>
> @@ -447,6 +465,7 @@ destroy_ldap_instance(ldap_instance_t
> **ldap_instp) str_destroy(&ldap_inst->base);
> str_destroy(&ldap_inst->bind_dn);
> str_destroy(&ldap_inst->password);
> + str_destroy(&ldap_inst->krb5_principal);
> str_destroy(&ldap_inst->sasl_mech);
> str_destroy(&ldap_inst->sasl_user);
> str_destroy(&ldap_inst->sasl_auth_name);
> @@ -1618,7 +1637,7 @@ ldap_reconnect(ldap_connection_t *ldap_conn)
> isc_result_t result;
> LOCK(&ldap_inst->kinit_lock);
> result = get_krb5_tgt(ldap_inst->mctx,
> -
> str_buf(ldap_inst->sasl_user),
> +
> str_buf(ldap_inst->krb5_principal), str_buf(ldap_inst->krb5_keytab));
> UNLOCK(&ldap_inst->kinit_lock);
> if (result != ISC_R_SUCCESS)
--
Simo Sorce * Red Hat, Inc * New York
>From fa819bc901963bdb2ab5a1da2841f809598c28a3 Mon Sep 17 00:00:00 2001
From: Zoran Pericic <zperi...@inet.hr>
Date: Tue, 21 Dec 2010 20:12:10 -0500
Subject: [PATCH] Use separate variables for sasl_user and krb5_principal
---
src/ldap_helper.c | 31 +++++++++++++++++++++++++------
1 files changed, 25 insertions(+), 6 deletions(-)
diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index 5eed8afba7a275a6ebb3a28c707639516ba9af41..134a3e899bd413a8146dd19a68ab30fc26cec269 100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -128,6 +128,7 @@ struct ldap_instance {
ldap_auth_t auth_method;
ld_string_t *bind_dn;
ld_string_t *password;
+ ld_string_t *krb5_principal;
ld_string_t *sasl_mech;
ld_string_t *sasl_user;
ld_string_t *sasl_auth_name;
@@ -293,6 +294,7 @@ new_ldap_instance(isc_mem_t *mctx, const char *db_name,
{ "auth_method", default_string("none") },
{ "bind_dn", default_string("") },
{ "password", default_string("") },
+ { "krb5_principal", default_string("") },
{ "sasl_mech", default_string("GSSAPI") },
{ "sasl_user", default_string("") },
{ "sasl_auth_name", default_string("") },
@@ -330,6 +332,7 @@ new_ldap_instance(isc_mem_t *mctx, const char *db_name,
CHECK(str_new(mctx, &ldap_inst->base));
CHECK(str_new(mctx, &ldap_inst->bind_dn));
CHECK(str_new(mctx, &ldap_inst->password));
+ CHECK(str_new(mctx, &ldap_inst->krb5_principal));
CHECK(str_new(mctx, &ldap_inst->sasl_mech));
CHECK(str_new(mctx, &ldap_inst->sasl_user));
CHECK(str_new(mctx, &ldap_inst->sasl_auth_name));
@@ -346,6 +349,7 @@ new_ldap_instance(isc_mem_t *mctx, const char *db_name,
ldap_settings[i++].target = auth_method_str;
ldap_settings[i++].target = ldap_inst->bind_dn;
ldap_settings[i++].target = ldap_inst->password;
+ ldap_settings[i++].target = ldap_inst->krb5_principal;
ldap_settings[i++].target = ldap_inst->sasl_mech;
ldap_settings[i++].target = ldap_inst->sasl_user;
ldap_settings[i++].target = ldap_inst->sasl_auth_name;
@@ -382,11 +386,25 @@ new_ldap_instance(isc_mem_t *mctx, const char *db_name,
/* check we have the right data when SASL/GSSAPI is selected */
if ((ldap_inst->auth_method == AUTH_SASL) &&
(str_casecmp_char(ldap_inst->sasl_mech, "GSSAPI") == 0)) {
- if ((ldap_inst->sasl_user == NULL) ||
- (str_len(ldap_inst->sasl_user) == 0)) {
- log_error("Sasl mech GSSAPI defined but sasl_user is empty");
- result = ISC_R_FAILURE;
- goto cleanup;
+ if ((ldap_inst->krb5_principal == NULL) ||
+ (str_len(ldap_inst->krb5_principal) == 0)) {
+ if ((ldap_inst->sasl_user == NULL) ||
+ (str_len(ldap_inst->sasl_user) == 0)) {
+ char hostname[255];
+ if (gethostname(hostname, 255) != 0) {
+ log_error("SASL mech GSSAPI defined but krb5_principal"
+ "and sasl_user are empty. Could not get hostname");
+ result = ISC_R_FAILURE;
+ goto cleanup;
+ } else {
+ str_sprintf(ldap_inst->krb5_principal, "DNS/%s", hostname);
+ log_debug(2, "SASL mech GSSAPI defined but krb5_principal"
+ "and sasl_user are empty, using default %s",
+ str_buf(ldap_inst->krb5_principal));
+ }
+ } else {
+ str_copy(ldap_inst->krb5_principal, ldap_inst->sasl_user);
+ }
}
}
@@ -447,6 +465,7 @@ destroy_ldap_instance(ldap_instance_t **ldap_instp)
str_destroy(&ldap_inst->base);
str_destroy(&ldap_inst->bind_dn);
str_destroy(&ldap_inst->password);
+ str_destroy(&ldap_inst->krb5_principal);
str_destroy(&ldap_inst->sasl_mech);
str_destroy(&ldap_inst->sasl_user);
str_destroy(&ldap_inst->sasl_auth_name);
@@ -1618,7 +1637,7 @@ ldap_reconnect(ldap_connection_t *ldap_conn)
isc_result_t result;
LOCK(&ldap_inst->kinit_lock);
result = get_krb5_tgt(ldap_inst->mctx,
- str_buf(ldap_inst->sasl_user),
+ str_buf(ldap_inst->krb5_principal),
str_buf(ldap_inst->krb5_keytab));
UNLOCK(&ldap_inst->kinit_lock);
if (result != ISC_R_SUCCESS)
--
1.7.3.3
_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
