Hi,
I'm sending corrected patches. All your suggestions and objections have been
addressed except maybe for this:
> If the SDAP_SASL_AUTHID has been explicitly set, but the SDAP_SASL_REALM
> hasn't, why are you overriding SDAP_SASL_AUTHID with
> select_principal_from_keytab()?
I agree with you that the code made a little sense before. I did a little
modification, so the SDAP_SASL_AUTHID isn't changed if possible. Here I'd like
to know your opinion. We might want to prioritize the configuration entered by
admin. My current approach prioritizes an actual content of the keytab if
either SDAP_SASL_AUTHID or SDAP_SASL_REALM isn't entered. That means in case
keytab doesn't contain principal matching the desired one, another one (based
on the preference in select_principal_from_keytab()) is selected.
If the user configuration had absolute priority, there would be a comparison
right after the best principal is selected by select_principal_from_keytab()
and in case the selected principal doesn't correspond to the configured one, an
error should be raised.
What do you think the best approach is for this?
Jan
From 94c6632a4ea8a574cdff725f3ae65e662869f213 Mon Sep 17 00:00:00 2001
From: Jan Zeleny <jzel...@redhat.com>
Date: Tue, 29 Mar 2011 02:51:29 -0400
Subject: [PATCH] Documentation update to the previous change
---
src/man/sssd-ldap.5.xml | 33 +++++++++++++++++++++++++++++++--
1 files changed, 31 insertions(+), 2 deletions(-)
diff --git a/src/man/sssd-ldap.5.xml b/src/man/sssd-ldap.5.xml
index 2a39732b62834b19e1a01422dc8d631de96faff2..3aa39b6f2f7336fac1221657a8480527b5f8bf85 100644
--- a/src/man/sssd-ldap.5.xml
+++ b/src/man/sssd-ldap.5.xml
@@ -1002,10 +1002,39 @@
<para>
Specify the SASL authorization id to use.
When GSSAPI is used, this represents the Kerberos
- principal used for authentication to the directory.
+ principal used for authentication to the directory
+ (without the REALM part).
</para>
<para>
- Default: host/machine.fqdn@REALM
+ See ldap_sasl_realm for information about default
+ value.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
+ <term>ldap_sasl_realm (string)</term>
+ <listitem>
+ <para>
+ Specify the SASL authorization realm to use.
+ When GSSAPI is used, this represents the Kerberos
+ principal realm used for authentication to the
+ directory.
+ </para>
+ <para>
+ If either ldap_sasl_realm or ldap_sasl_authid is
+ not set, autodetection takes place. This process
+ scans the keytab and selects the most convenient
+ principal to use. Priority of the chosen principal
+ is as follows:
+
+ <itemizedlist>
+ <listitem><para>foobar$@REALM (used in AD domain)</para></listitem>
+ <listitem><para>host/our.hostname@REALM</para></listitem>
+ <listitem><para>host/foobar@REALM</para></listitem>
+ <listitem><para>host/foo@BAR</para></listitem>
+ <listitem><para>any principal in the keytab</para></listitem>
+ </itemizedlist>
</para>
</listitem>
</varlistentry>
--
1.7.4.1
From f071f97d589c5ecffedb462f7382e3aae115b809 Mon Sep 17 00:00:00 2001
From: Jan Zeleny <jzel...@redhat.com>
Date: Tue, 29 Mar 2011 02:46:25 -0400
Subject: [PATCH] Modify principal selection for keytab authentication
Currently we construct the principal as host/fqdn@REALM. The problem
with this is that this principal doesn't have to be in the keytab. In
that case the provider fails to start. It is better to scan the keytab
and find the most suitable principal to use. Only in case no suitable
principal is found the backend should fail to start.
The second issue solved by this patch is that the realm we are
authenticating the machine to can be in general different from the realm
our users are part of (in case of cross Kerberos trust).
The patch adds new configuration option SDAP_SASL_REALM.
https://fedorahosted.org/sssd/ticket/781
---
src/config/SSSDConfig.py | 1 +
src/providers/ipa/ipa_common.c | 63 ++++++++----
src/providers/ipa/ipa_common.h | 2 +-
src/providers/ldap/ldap_child.c | 5 +-
src/providers/ldap/ldap_common.c | 1 +
src/providers/ldap/sdap.h | 1 +
src/providers/ldap/sdap_async_connection.c | 9 ++-
src/providers/ldap/sdap_child_helpers.c | 9 ++-
src/util/sss_krb5.c | 157 +++++++++++++++++++++++++++-
src/util/sss_krb5.h | 8 ++
10 files changed, 226 insertions(+), 30 deletions(-)
diff --git a/src/config/SSSDConfig.py b/src/config/SSSDConfig.py
index 5135174a87d13b2f0817a99e1b1d9f63e73d5673..93c03b365ca2d38020bbc86489e7593e8f734bb3 100644
--- a/src/config/SSSDConfig.py
+++ b/src/config/SSSDConfig.py
@@ -133,6 +133,7 @@ option_strings = {
'ldap_tls_reqcert' : _('Require TLS certificate verification'),
'ldap_sasl_mech' : _('Specify the sasl mechanism to use'),
'ldap_sasl_authid' : _('Specify the sasl authorization id to use'),
+ 'ldap_sasl_realm' : _('Specify the sasl authorization realm to use'),
'ldap_krb5_keytab' : _('Kerberos service keytab'),
'ldap_krb5_init_creds' : _('Use Kerberos auth for LDAP connection'),
'ldap_referrals' : _('Follow LDAP referrals'),
diff --git a/src/providers/ipa/ipa_common.c b/src/providers/ipa/ipa_common.c
index 067f2ee85c625a2a6e5dec77b07f9b2359a2dad0..6a392e397c6e7e7b6c285d5db7a3b73c1c7e89c0 100644
--- a/src/providers/ipa/ipa_common.c
+++ b/src/providers/ipa/ipa_common.c
@@ -28,6 +28,7 @@
#include "providers/ipa/ipa_common.h"
#include "providers/ldap/sdap_async_private.h"
+#include "util/sss_krb5.h"
struct dp_option ipa_basic_opts[] = {
{ "ipa_domain", DP_OPT_STRING, NULL_STRING, NULL_STRING },
@@ -69,6 +70,7 @@ struct dp_option ipa_def_ldap_opts[] = {
{ "ldap_id_use_start_tls", DP_OPT_BOOL, BOOL_FALSE, BOOL_FALSE },
{ "ldap_sasl_mech", DP_OPT_STRING, { "GSSAPI" } , NULL_STRING },
{ "ldap_sasl_authid", DP_OPT_STRING, NULL_STRING, NULL_STRING },
+ { "ldap_sasl_realm", DP_OPT_STRING, NULL_STRING, NULL_STRING },
{ "ldap_krb5_keytab", DP_OPT_STRING, NULL_STRING, NULL_STRING },
{ "ldap_krb5_init_creds", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE },
/* use the same parm name as the krb5 module so we set it only once */
@@ -262,10 +264,12 @@ int ipa_get_id_options(struct ipa_options *ipa_opts,
struct sdap_options **_opts)
{
TALLOC_CTX *tmpctx;
- char *hostname;
+ char *primary;
char *basedn;
char *realm;
char *value;
+ char *desired_realm;
+ char *desired_primary;
int ret;
int i;
@@ -322,26 +326,6 @@ int ipa_get_id_options(struct ipa_options *ipa_opts,
dp_opt_get_string(ipa_opts->id->basic, SDAP_SEARCH_BASE)));
}
- /* set the ldap_sasl_authid if the ipa_hostname override was specified */
- if (NULL == dp_opt_get_string(ipa_opts->id->basic, SDAP_SASL_AUTHID)) {
- hostname = dp_opt_get_string(ipa_opts->basic, IPA_HOSTNAME);
- if (hostname) {
- value = talloc_asprintf(tmpctx, "host/%s", hostname);
- if (!value) {
- ret = ENOMEM;
- goto done;
- }
- ret = dp_opt_set_string(ipa_opts->id->basic,
- SDAP_SASL_AUTHID, value);
- if (ret != EOK) {
- goto done;
- }
- }
- DEBUG(6, ("Option %s set to %s\n",
- ipa_opts->id->basic[SDAP_SASL_AUTHID].opt_name,
- dp_opt_get_string(ipa_opts->id->basic, SDAP_SASL_AUTHID)));
- }
-
/* 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_KRB5_REALM);
@@ -361,6 +345,43 @@ int ipa_get_id_options(struct ipa_options *ipa_opts,
dp_opt_get_string(ipa_opts->id->basic, SDAP_KRB5_REALM)));
}
+ if (NULL == dp_opt_get_string(ipa_opts->id->basic, SDAP_SASL_AUTHID) ||
+ NULL == dp_opt_get_string(ipa_opts->id->basic, SDAP_SASL_REALM)) {
+ desired_primary = dp_opt_get_string(ipa_opts->id->basic, SDAP_SASL_AUTHID);
+ if (!desired_primary)
+ desired_primary = dp_opt_get_string(ipa_opts->id->basic, IPA_HOSTNAME);
+ desired_realm = dp_opt_get_string(ipa_opts->id->basic, SDAP_SASL_REALM);
+ if (!desired_realm)
+ desired_realm = dp_opt_get_string(ipa_opts->id->basic, IPA_KRB5_REALM);
+
+ ret = select_principal_from_keytab(tmpctx,
+ dp_opt_get_string(ipa_opts->auth,
+ KRB5_KEYTAB),
+ desired_primary, desired_realm,
+ NULL, &primary, &realm);
+ if (ret == EOK) {
+ ret = dp_opt_set_string(ipa_opts->id->basic,
+ SDAP_SASL_AUTHID, primary);
+ if (ret != EOK) {
+ goto done;
+ }
+ DEBUG(6, ("Option %s set to %s\n",
+ ipa_opts->id->basic[SDAP_SASL_AUTHID].opt_name,
+ dp_opt_get_string(ipa_opts->id->basic, SDAP_SASL_AUTHID)));
+
+ ret = dp_opt_set_string(ipa_opts->id->basic,
+ SDAP_SASL_REALM, realm);
+ if (ret != EOK) {
+ goto done;
+ }
+ DEBUG(6, ("Option %s set to %s\n",
+ ipa_opts->id->basic[SDAP_SASL_REALM].opt_name,
+ dp_opt_get_string(ipa_opts->id->basic, SDAP_SASL_REALM)));
+ } else if (ret != ENOENT) {
+ goto done;
+ }
+ }
+
/* fix schema to IPAv1 for now */
ipa_opts->id->schema_type = SDAP_SCHEMA_IPA_V1;
diff --git a/src/providers/ipa/ipa_common.h b/src/providers/ipa/ipa_common.h
index 588aa63e412dc2ba006714729bb4710a4075ff25..922806234f76bad4b0d88907302c1e8a1d2f1020 100644
--- a/src/providers/ipa/ipa_common.h
+++ b/src/providers/ipa/ipa_common.h
@@ -35,7 +35,7 @@ struct ipa_service {
/* the following defines are used to keep track of the options in the ldap
* module, so that if they change and ipa is not updated correspondingly
* this will trigger a runtime abort error */
-#define IPA_OPTS_BASIC_TEST 48
+#define IPA_OPTS_BASIC_TEST 49
/* the following define is used to keep track of the options in the krb5
* module, so that if they change and ipa is not updated correspondingly
diff --git a/src/providers/ldap/ldap_child.c b/src/providers/ldap/ldap_child.c
index f4be18571a1a584270f6079173349eb92085fc4f..fb8dd8063c7bc6fb9e7bbdef9cb66c854e4040f9 100644
--- a/src/providers/ldap/ldap_child.c
+++ b/src/providers/ldap/ldap_child.c
@@ -196,8 +196,9 @@ static krb5_error_code ldap_child_get_tgt_sync(TALLOC_CTX *memctx,
}
hostname[511] = '\0';
- full_princ = talloc_asprintf(memctx, "host/%s@%s",
- hostname, realm_name);
+ ret = select_principal_from_keytab(memctx, hostname, realm_name,
+ keytab_name, &full_princ, NULL, NULL);
+ if (ret) goto done;
}
if (!full_princ) {
krberr = KRB5KRB_ERR_GENERIC;
diff --git a/src/providers/ldap/ldap_common.c b/src/providers/ldap/ldap_common.c
index 9eb9cc379f50ad31cdee4b6dc66298306169347a..2b395a03c0e385377002fa9112fd855a987f4c65 100644
--- a/src/providers/ldap/ldap_common.c
+++ b/src/providers/ldap/ldap_common.c
@@ -63,6 +63,7 @@ struct dp_option default_basic_opts[] = {
{ "ldap_id_use_start_tls", DP_OPT_BOOL, BOOL_FALSE, BOOL_FALSE },
{ "ldap_sasl_mech", DP_OPT_STRING, NULL_STRING, NULL_STRING },
{ "ldap_sasl_authid", DP_OPT_STRING, NULL_STRING, NULL_STRING },
+ { "ldap_sasl_realm", DP_OPT_STRING, NULL_STRING, NULL_STRING },
{ "ldap_krb5_keytab", DP_OPT_STRING, NULL_STRING, NULL_STRING },
{ "ldap_krb5_init_creds", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE },
/* use the same parm name as the krb5 module so we set it only once */
diff --git a/src/providers/ldap/sdap.h b/src/providers/ldap/sdap.h
index 32dc34448a9cd9a31e999081874338880bbfe0b6..41d542f6c5e74f087a84080e3f2ce9093d94bfba 100644
--- a/src/providers/ldap/sdap.h
+++ b/src/providers/ldap/sdap.h
@@ -182,6 +182,7 @@ enum sdap_basic_opt {
SDAP_ID_TLS,
SDAP_SASL_MECH,
SDAP_SASL_AUTHID,
+ SDAP_SASL_REALM,
SDAP_KRB5_KEYTAB,
SDAP_KRB5_KINIT,
SDAP_KRB5_KDC,
diff --git a/src/providers/ldap/sdap_async_connection.c b/src/providers/ldap/sdap_async_connection.c
index d2eadfa6a025180915f477aff237c512dfa26e29..af1dc059bcc8667f7d79f68ed27ee7c76247de6f 100644
--- a/src/providers/ldap/sdap_async_connection.c
+++ b/src/providers/ldap/sdap_async_connection.c
@@ -1318,6 +1318,12 @@ static void sdap_cli_kinit_step(struct tevent_req *req)
struct sdap_cli_connect_state *state = tevent_req_data(req,
struct sdap_cli_connect_state);
struct tevent_req *subreq;
+ const char *realm;
+
+ realm = dp_opt_get_string(state->opts->basic, SDAP_SASL_REALM);
+ if (!realm) {
+ realm = dp_opt_get_string(state->opts->basic, SDAP_KRB5_REALM);
+ }
subreq = sdap_kinit_send(state, state->ev,
state->be,
@@ -1329,8 +1335,7 @@ static void sdap_cli_kinit_step(struct tevent_req *req)
SDAP_KRB5_KEYTAB),
dp_opt_get_string(state->opts->basic,
SDAP_SASL_AUTHID),
- dp_opt_get_string(state->opts->basic,
- SDAP_KRB5_REALM),
+ realm,
dp_opt_get_int(state->opts->basic,
SDAP_KRB5_TICKET_LIFETIME));
if (!subreq) {
diff --git a/src/providers/ldap/sdap_child_helpers.c b/src/providers/ldap/sdap_child_helpers.c
index 5a15e661e3ca014e511dc5647dd199d9839100a8..d0f6caeb2ea71340406b39ed086a47a44414d85a 100644
--- a/src/providers/ldap/sdap_child_helpers.c
+++ b/src/providers/ldap/sdap_child_helpers.c
@@ -458,6 +458,12 @@ int setup_child(struct sdap_id_ctx *ctx)
const char *mech;
unsigned v;
FILE *debug_filep;
+ const char *realm;
+
+ realm = dp_opt_get_string(ctx->opts->basic, SDAP_SASL_REALM);
+ if (!realm) {
+ realm = dp_opt_get_string(ctx->opts->basic, SDAP_KRB5_REALM);
+ }
mech = dp_opt_get_string(ctx->opts->basic,
SDAP_SASL_MECH);
@@ -468,8 +474,7 @@ int setup_child(struct sdap_id_ctx *ctx)
if (mech && (strcasecmp(mech, "GSSAPI") == 0)) {
ret = sss_krb5_verify_keytab(dp_opt_get_string(ctx->opts->basic,
SDAP_SASL_AUTHID),
- dp_opt_get_string(ctx->opts->basic,
- SDAP_KRB5_REALM),
+ realm,
dp_opt_get_string(ctx->opts->basic,
SDAP_KRB5_KEYTAB));
diff --git a/src/util/sss_krb5.c b/src/util/sss_krb5.c
index e5d268ff027076f19ddb6d1e14498ed8128c464a..fb55871b7b87eda98072c466cc2d2fcf69e4f85a 100644
--- a/src/util/sss_krb5.c
+++ b/src/util/sss_krb5.c
@@ -26,6 +26,158 @@
#include "util/util.h"
#include "util/sss_krb5.h"
+errno_t select_principal_from_keytab(TALLOC_CTX *mem_ctx,
+ const char *hostname,
+ const char *desired_realm,
+ const char *keytab_name,
+ char **_principal,
+ char **_primary,
+ char **_realm)
+{
+ krb5_error_code kerr = 0;
+ krb5_context krb_ctx = NULL;
+ krb5_keytab keytab;
+ krb5_principal client_princ = NULL;
+ TALLOC_CTX *tmp_ctx;
+ char *primary = NULL;
+ char *realm = NULL;
+ int i = 0;
+ errno_t ret;
+ char *principal_string;
+
+ /**
+ * Priority of lookup:
+ * - foobar$@REALM (AD domain)
+ * - host/our.hostname@REALM
+ * - host/foobar@REALM
+ * - host/foo@BAR
+ * - pick the first principal in the keytab
+ */
+ const char *primary_patterns[] = {"%s$", "*$", "host/%s", "host/*", "host/*", NULL};
+ const char *realm_patterns[] = {"%s", "%s", "%s", "%s", NULL, NULL};
+
+ DEBUG(5, ("trying to select the most appropriate principal from keytab\n"));
+ tmp_ctx = talloc_new(NULL);
+ if (!tmp_ctx) {
+ DEBUG(1, ("talloc_new failed\n"));
+ return ENOMEM;
+ }
+
+ kerr = krb5_init_context(&krb_ctx);
+ if (kerr) {
+ DEBUG(2, ("Failed to init kerberos context\n"));
+ ret = EFAULT;
+ goto done;
+ }
+
+ if (keytab_name != NULL) {
+ kerr = krb5_kt_resolve(krb_ctx, keytab_name, &keytab);
+ } else {
+ kerr = krb5_kt_default(krb_ctx, &keytab);
+ }
+ if (kerr) {
+ DEBUG(0, ("Failed to read keytab file: %s\n",
+ sss_krb5_get_error_message(krb_ctx, kerr)));
+ ret = EFAULT;
+ goto done;
+ }
+
+ if (!desired_realm) {
+ desired_realm = "*";
+ }
+ if (!hostname) {
+ hostname = "*";
+ }
+
+ do {
+ if (primary_patterns[i]) {
+ primary = talloc_asprintf(tmp_ctx, primary_patterns[i], hostname);
+ } else {
+ primary = NULL;
+ }
+ if (realm_patterns[i]) {
+ realm = talloc_asprintf(tmp_ctx, realm_patterns[i], desired_realm);
+ } else {
+ realm = NULL;
+ }
+
+ kerr = find_principal_in_keytab(krb_ctx, keytab, primary, realm,
+ &client_princ);
+ talloc_zfree(primary);
+ talloc_zfree(realm);
+ if (kerr == 0) {
+ break;
+ }
+ if (client_princ != NULL) {
+ krb5_free_principal(krb_ctx, client_princ);
+ client_princ = NULL;
+ }
+ i++;
+ } while(primary_patterns[i-1] != NULL || realm_patterns[i-1] != NULL);
+
+ if (kerr == 0) {
+ if (_principal) {
+ kerr = krb5_unparse_name(krb_ctx, client_princ, &principal_string);
+ if (kerr) {
+ DEBUG(1, ("krb5_unparse_name failed"));
+ ret = EFAULT;
+ goto done;
+ }
+
+ *_principal = talloc_strdup(mem_ctx, principal_string);
+ if (!*_principal) {
+ DEBUG(1, ("talloc_strdup failed"));
+ }
+ free(principal_string);
+ DEBUG(5, ("Selected principal: %s\n", *_principal));
+ }
+
+ if (_primary) {
+ kerr = krb5_unparse_name_flags(krb_ctx, client_princ,
+ KRB5_PRINCIPAL_UNPARSE_NO_REALM,
+ &principal_string);
+ if (kerr) {
+ DEBUG(1, ("krb5_unparse_name failed"));
+ ret = EFAULT;
+ goto done;
+ }
+
+ *_primary = talloc_strdup(mem_ctx, principal_string);
+ if (!*_primary) {
+ DEBUG(1, ("talloc_strdup failed"));
+ }
+ free(principal_string);
+ DEBUG(5, ("Selected primary: %s\n", *_primary));
+ }
+
+ if (_realm) {
+ *_realm = talloc_asprintf(mem_ctx, "%.*s",
+ krb5_princ_realm(ctx, client_princ)->length,
+ krb5_princ_realm(ctx, client_princ)->data);
+ if (!*_realm) {
+ DEBUG(1, ("talloc_asprintf failed"));
+ }
+ DEBUG(5, ("Selected realm: %s\n", *_realm));
+ }
+
+ ret = EOK;
+ } else {
+ DEBUG(3, ("No suitable principal found in keytab\n"));
+ ret = ENOENT;
+ }
+
+done:
+ if (keytab) krb5_kt_close(krb_ctx, keytab);
+ if (krb_ctx) krb5_free_context(krb_ctx);
+ if (client_princ != NULL) {
+ krb5_free_principal(krb_ctx, client_princ);
+ client_princ = NULL;
+ }
+ talloc_free(tmp_ctx);
+ return ret;
+}
+
+
int sss_krb5_verify_keytab(const char *principal,
const char *realm_str,
const char *keytab_name)
@@ -104,8 +256,9 @@ int sss_krb5_verify_keytab(const char *principal,
}
hostname[511] = '\0';
- full_princ = talloc_asprintf(tmp_ctx, "host/%s@%s",
- hostname, realm_name);
+ ret = select_principal_from_keytab(tmp_ctx, hostname, realm_name,
+ keytab_name, &full_princ, NULL, NULL);
+ if (ret) goto done;
}
if (!full_princ) {
ret = ENOMEM;
diff --git a/src/util/sss_krb5.h b/src/util/sss_krb5.h
index f25f3285b2cac8b37fde73985775c276c6ea589d..d17bfe9691d1bcc6c4f04b71ed0392753c91f0f6 100644
--- a/src/util/sss_krb5.h
+++ b/src/util/sss_krb5.h
@@ -64,6 +64,14 @@ krb5_error_code find_principal_in_keytab(krb5_context ctx,
const char *pattern_realm,
krb5_principal *princ);
+errno_t select_principal_from_keytab(TALLOC_CTX *mem_ctx,
+ const char *hostname,
+ const char *desired_realm,
+ const char *keytab_name,
+ char **_principal,
+ char **_primary,
+ char **_realm);
+
#ifndef HAVE_KRB5_GET_INIT_CREDS_OPT_SET_EXPIRE_CALLBACK
typedef void krb5_expire_callback_func(krb5_context context, void *data,
krb5_timestamp password_expiration,
--
1.7.4.1
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel
