-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On 02/23/2011 06:18 PM, Sumit Bose wrote:
> On Wed, Feb 23, 2011 at 05:59:13PM +0100, Jakub Hrozek wrote:
> On 02/23/2011 05:33 PM, Sumit Bose wrote:
>>>> On Wed, Feb 23, 2011 at 05:24:20PM +0100, Jakub Hrozek wrote:
>>>>
https://fedorahosted.org/sssd/ticket/807
>>>>
>>>>> NACK, please use the "new" option from 'Add krb5_realm
to the basic IPA
>>>>> options'
>>>>
>>>>> bye,
>>>>> Sumit
>>>>
>
> See, that's what I get for not reading today's patches.
>
> New one attached. I also explicitly set IPA_KRB5_REALM for cases where
> we use the uppercase domain as realm value so it's always available.
>
Sorry for not sseing this earlier, but would you mind to move the whole
setting of IPA_KRB5_REALM from ipa_service_init() to ipa_get_options()?
I think it it safer to set it here so that it is really always
available.
>> Patch looks good, would you mind to add a paragraph to the
man page of
>> the IPA provider which explains the special role of krb5_realm for the
>> IPA provider?
>
>> bye,
>> Sumit
Of course. A new patch is attached. I did one more modification - the
values of SDAP_KRB5_REALM and KRB5_REALM for ipa_id and ipa_auth
respectively now default to IPA_KRB5_REALM, not IPA_DOMAIN.toupper(). It
should technically be the same, but I think the new way is more consistent.
Good point. While reading 'toupper' here I start thinking if we should
add a 'tolower' to domain_to_basedn() as realm_to_suffix() does in
FreeIPA. This is a cosmetic change because the case shouldn't matter
here, but since we print the baseDN in the debug logs it might irritate
the untrained eye.
bye,
Sumit
Jakub
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Fedora -
http://enigmail.mozdev.org/
iEYEARECAAYFAk1mP5kACgkQHsardTLnvCVjigCfXS0ioFiWeSzYG051H/13Ri7r
iC4AnR9NNqcJ10gHZXNw09AgGdoYQC5R
=HOE/
-----END PGP SIGNATURE-----
From
2b4c66952a353c0b352401b6035e73506b3f1ac7 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek(a)redhat.com>
Date: Wed, 23 Feb 2011 17:40:44 +0100
Subject: [PATCH] Use realm for basedn instead of IPA domain
https://fedorahosted.org/sssd/ticket/807
---
src/man/sssd-ipa.5.xml | 15 +++++++++++++++
src/providers/ipa/ipa_access.c | 2 +-
src/providers/ipa/ipa_auth.c | 12 ++++++------
src/providers/ipa/ipa_common.c | 19 +++++++++----------
4 files changed, 31 insertions(+), 17 deletions(-)
diff --git a/src/man/sssd-ipa.5.xml b/src/man/sssd-ipa.5.xml
index 606581d..4604c55 100644
--- a/src/man/sssd-ipa.5.xml
+++ b/src/man/sssd-ipa.5.xml
@@ -161,6 +161,21 @@
</listitem>
</varlistentry>
+ <varlistentry>
+ <term>krb5_realm (string)</term>
+ <listitem>
+ <para>
+ The name of the Kerberos realm. This is optional and
+ defaults to the value of
<quote>ipa_domain</quote>.
+ </para>
+ <para>
+ The name of the Kerberos realm has a special
+ meaning in IPA - it is converted into the base
+ DN to use for performing LDAP operations.
+ </para>
+ </listitem>
+ </varlistentry>
+
</variablelist>
</para>
</refsect1>
diff --git a/src/providers/ipa/ipa_access.c b/src/providers/ipa/ipa_access.c
index 02b0a77..f07eb7b 100644
--- a/src/providers/ipa/ipa_access.c
+++ b/src/providers/ipa/ipa_access.c
@@ -74,7 +74,7 @@ static char *get_hbac_search_base(TALLOC_CTX *mem_ctx,
DEBUG(9, ("ipa_hbac_search_base not available, trying base DN.\n"));
ret = domain_to_basedn(mem_ctx,
- dp_opt_get_string(ipa_options, IPA_DOMAIN),
+ dp_opt_get_string(ipa_options, IPA_KRB5_REALM),
&base);
if (ret != EOK) {
DEBUG(1, ("domain_to_basedn failed.\n"));
diff --git a/src/providers/ipa/ipa_auth.c b/src/providers/ipa/ipa_auth.c
index eb7f291..d8d8ad5 100644
--- a/src/providers/ipa/ipa_auth.c
+++ b/src/providers/ipa/ipa_auth.c
@@ -46,7 +46,7 @@ struct get_password_migration_flag_state {
struct sdap_handle *sh;
enum sdap_result result;
struct fo_server *srv;
- char *ipa_domain;
+ char *ipa_realm;
bool password_migration;
};
@@ -56,13 +56,13 @@ static void get_password_migration_flag_done(struct tevent_req
*subreq);
static struct tevent_req *get_password_migration_flag_send(TALLOC_CTX *memctx,
struct tevent_context *ev,
struct sdap_auth_ctx *sdap_auth_ctx,
- char *ipa_domain)
+ char *ipa_realm)
{
int ret;
struct tevent_req *req, *subreq;
struct get_password_migration_flag_state *state;
- if (sdap_auth_ctx == NULL || ipa_domain == NULL) {
+ if (sdap_auth_ctx == NULL || ipa_realm == NULL) {
DEBUG(1, ("Missing parameter.\n"));
return NULL;
}
@@ -80,7 +80,7 @@ static struct tevent_req *get_password_migration_flag_send(TALLOC_CTX
*memctx,
state->result = SDAP_ERROR;
state->srv = NULL;
state->password_migration = false;
- state->ipa_domain = ipa_domain;
+ state->ipa_realm = ipa_realm;
/* We request to use StartTLS here, because if password migration is
* enabled we will use this connection for authentication, too. */
@@ -126,7 +126,7 @@ static void get_password_migration_flag_auth_done(struct tevent_req
*subreq)
return;
}
- ret = domain_to_basedn(state, state->ipa_domain, &ldap_basedn);
+ ret = domain_to_basedn(state, state->ipa_realm, &ldap_basedn);
if (ret != EOK) {
DEBUG(1, ("domain_to_basedn failed.\n"));
tevent_req_error(req, ret);
@@ -311,7 +311,7 @@ static void ipa_auth_handler_done(struct tevent_req *req)
state->ipa_auth_ctx->sdap_auth_ctx,
dp_opt_get_string(
state->ipa_auth_ctx->ipa_options,
- IPA_DOMAIN));
+ IPA_KRB5_REALM));
if (req == NULL) {
DEBUG(1, ("get_password_migration_flag failed.\n"));
goto done;
diff --git a/src/providers/ipa/ipa_common.c b/src/providers/ipa/ipa_common.c
index 397e418..59b6929 100644
--- a/src/providers/ipa/ipa_common.c
+++ b/src/providers/ipa/ipa_common.c
@@ -273,7 +273,7 @@ int ipa_get_id_options(struct ipa_options *ipa_opts,
}
ret = domain_to_basedn(tmpctx,
- dp_opt_get_string(ipa_opts->basic, IPA_DOMAIN),
+ dp_opt_get_string(ipa_opts->basic, IPA_KRB5_REALM),
&basedn);
if (ret != EOK) {
goto done;
@@ -319,16 +319,13 @@ int ipa_get_id_options(struct ipa_options *ipa_opts,
/* set krb realm */
if (NULL == dp_opt_get_string(ipa_opts->id->basic, SDAP_KRB5_REALM)) {
- realm = dp_opt_get_string(ipa_opts->basic, IPA_DOMAIN);
+ realm = dp_opt_get_string(ipa_opts->basic, IPA_KRB5_REALM);
value = talloc_strdup(tmpctx, realm);
if (value == NULL) {
DEBUG(1, ("talloc_strdup failed.\n"));
ret = ENOMEM;
goto done;
}
- for (i = 0; value[i]; i++) {
- value[i] = toupper(value[i]);
- }
ret = dp_opt_set_string(ipa_opts->id->basic,
SDAP_KRB5_REALM, value);
if (ret != EOK) {
@@ -467,7 +464,6 @@ int ipa_get_auth_options(struct ipa_options *ipa_opts,
char *value;
char *copy = NULL;
int ret;
- int i;
/* self check test, this should never fail, unless someone forgot
* to properly update the code after new ldap options have been added */
@@ -501,7 +497,7 @@ int ipa_get_auth_options(struct ipa_options *ipa_opts,
/* set krb realm */
if (NULL == dp_opt_get_string(ipa_opts->auth, KRB5_REALM)) {
- value = dp_opt_get_string(ipa_opts->basic, IPA_DOMAIN);
+ value = dp_opt_get_string(ipa_opts->basic, IPA_KRB5_REALM);
if (!value) {
ret = ENOMEM;
goto done;
@@ -512,9 +508,6 @@ int ipa_get_auth_options(struct ipa_options *ipa_opts,
ret = ENOMEM;
goto done;
}
- for (i = 0; copy[i]; i++) {
- copy[i] = toupper(copy[i]);
- }
ret = dp_opt_set_string(ipa_opts->auth, KRB5_REALM, copy);
if (ret != EOK) {
goto done;
@@ -673,6 +666,12 @@ int ipa_service_init(TALLOC_CTX *memctx, struct be_ctx *ctx,
service->krb5_service->realm[i] =
toupper(service->krb5_service->realm[i]);
}
+
+ ret = dp_opt_set_string(options->basic, IPA_KRB5_REALM,
+ service->krb5_service->realm);
+ if (ret != EOK) {
+ goto done;
+ }
}
if (!servers) {
--
1.7.4