[SSSD] [PATCH] LDAP: Make it possible to use full principal in ldap_sasl_authid again

Jakub Hrozek jhrozek at redhat.com
Mon Nov 19 10:53:20 UTC 2012


The behaviour of ldap_sasl_authid was changed in
e81a816cddab4a62f263d1a0274d5d3f101e8e0f so that it no longer accepted full
principal, but only hostname. The realm was read from the (undocumented)
ldap_sasl_realm option.

Moreover the "hostname" had to be specified in exactly the format that would
match the keytab later, ie if the first match was host/hostname, the option
would have to be specified as host/hostname, otherwise the user got an error.

I also found out that the principal selection is only used in IPA and AD
providers, but I'll file a separate ticket to track that. Right now, I'm
mostly interested in fixing the regression.

The attached patched modify that behaviour:

[PATCH 1/3] MAN: document the ldap_sasl_realm option
The option was completely undocumented.

[PATCH 2/3] LDAP: Provide a common sdap_set_sasl_options init function
The AD and IPA initialization functions shared the same code. This patch
moves the code into a common initialization function.

[PATCH 3/3] LDAP: Make it possible to use full principal in ldap_sasl_authid again
When the guessing of principals was introduced, we changed the existing
behaviour and instead of allowing both host/hostname at REALM and
host/hostname, only the latter worked and the realm was always appended
from the (then undocumented) ldap_sasl_realm option.

This patch changes the behaviour to be more backwards-compatible -- if
the ldap_sasl_authid contains the @-sign, then the ldap_sasl_realm is
not always appended.

The strict requirement of checking the requested authid/realm against
the one found in keytab was also kind of relaxed because it didn't
really work. For example when hostname was requested and the keytab
matched host/hostname first, the code blew up.

https://fedorahosted.org/sssd/ticket/1635
-------------- next part --------------
>From bf2d11e79abd0f31b3fad250315fb0aceefaa7b9 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Mon, 19 Nov 2012 10:32:53 +0100
Subject: [PATCH 1/3] MAN: document the ldap_sasl_realm option

The option was completely undocumented.
---
 src/man/sssd-ldap.5.xml | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/src/man/sssd-ldap.5.xml b/src/man/sssd-ldap.5.xml
index 70ada226298efda427b94c352e6e2352104a5a85..2d62c11f232c4f1fd15b547b9ac3f4cfb7c0e170 100644
--- a/src/man/sssd-ldap.5.xml
+++ b/src/man/sssd-ldap.5.xml
@@ -1426,6 +1426,19 @@
                 </varlistentry>
 
                 <varlistentry>
+                    <term>ldap_sasl_realm (string)</term>
+                    <listitem>
+                        <para>
+                            Specify the SASL realm to use. When not specified,
+                            this option defaults to the value of krb5_realm.
+                        </para>
+                        <para>
+                            Default: the value of krb5_realm.
+                        </para>
+                    </listitem>
+                </varlistentry>
+
+                <varlistentry>
                     <term>ldap_sasl_canonicalize (boolean)</term>
                     <listitem>
                         <para>
-- 
1.8.0

-------------- next part --------------
>From 09359efd4ffc56707f25c6f940c9fe84628f62ab Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Mon, 19 Nov 2012 10:26:18 +0100
Subject: [PATCH 2/3] LDAP: Provide a common sdap_set_sasl_options init
 function

The AD and IPA initialization functions shared the same code. This patch
moves the code into a common initialization function.
---
 src/providers/ad/ad_common.c     | 52 +++++------------------------
 src/providers/ipa/ipa_common.c   | 55 +++++-------------------------
 src/providers/ldap/ldap_common.c | 72 ++++++++++++++++++++++++++++++++++++++++
 src/providers/ldap/ldap_common.h |  7 ++++
 4 files changed, 95 insertions(+), 91 deletions(-)

diff --git a/src/providers/ad/ad_common.c b/src/providers/ad/ad_common.c
index 21a7b53467cb6eeb426ce5266e771bd3fc5537d2..8600dab22a6392367fd6a4465177a49bffb81f8e 100644
--- a/src/providers/ad/ad_common.c
+++ b/src/providers/ad/ad_common.c
@@ -422,13 +422,7 @@ ad_get_id_options(struct ad_options *ad_opts,
     TALLOC_CTX *tmp_ctx;
     struct sdap_options *id_opts;
     char *krb5_realm;
-    char *sasl_primary;
-    char *desired_primary;
-    char *sasl_realm;
-    char *desired_realm;
     char *keytab_path;
-    bool primary_requested = true;
-    bool realm_requested = true;
 
     tmp_ctx = talloc_new(NULL);
     if (!tmp_ctx) return ENOMEM;
@@ -478,19 +472,6 @@ ad_get_id_options(struct ad_options *ad_opts,
            id_opts->basic[SDAP_KRB5_REALM].opt_name,
            krb5_realm));
 
-    /* Configuration of SASL auth ID and realm */
-    desired_primary = dp_opt_get_string(id_opts->basic, SDAP_SASL_AUTHID);
-    if (!desired_primary) {
-        primary_requested = false;
-        desired_primary = dp_opt_get_string(ad_opts->basic, AD_HOSTNAME);
-    }
-
-    desired_realm = dp_opt_get_string(id_opts->basic, SDAP_SASL_REALM);
-    if (!desired_realm) {
-        realm_requested = false;
-        desired_realm = dp_opt_get_string(ad_opts->basic, AD_KRB5_REALM);
-    }
-
     keytab_path = dp_opt_get_string(ad_opts->basic, AD_KEYTAB);
     if (keytab_path) {
         ret = dp_opt_set_string(id_opts->basic, SDAP_KRB5_KEYTAB,
@@ -502,34 +483,17 @@ ad_get_id_options(struct ad_options *ad_opts,
                keytab_path));
     }
 
-    ret = select_principal_from_keytab(tmp_ctx,
-                                       desired_primary, desired_realm,
-                                       keytab_path, NULL,
-                                       &sasl_primary, &sasl_realm);
-    if (ret != EOK) goto done;
-
-    if ((primary_requested && strcmp(desired_primary, sasl_primary) != 0) ||
-        (realm_requested && strcmp(desired_realm, sasl_realm) != 0)) {
-        DEBUG(SSSDBG_FATAL_FAILURE,
-              ("Configured SASL auth ID/realm not found in keytab.\n"));
-        ret = ENOENT;
+    ret = sdap_set_sasl_options(id_opts,
+                                dp_opt_get_string(ad_opts->basic,
+                                                  AD_HOSTNAME),
+                                dp_opt_get_string(ad_opts->basic,
+                                                  AD_KRB5_REALM),
+                                keytab_path);
+    if (ret != EOK) {
+        DEBUG(SSSDBG_OP_FAILURE, ("Cannot set the SASL-related options\n"));
         goto done;
     }
 
-    ret = dp_opt_set_string(id_opts->basic, SDAP_SASL_AUTHID, sasl_primary);
-    if (ret != EOK) goto done;
-    DEBUG(SSSDBG_CONF_SETTINGS,
-          ("Option %s set to %s\n",
-           id_opts->basic[SDAP_SASL_AUTHID].opt_name,
-           sasl_primary));
-
-    ret = dp_opt_set_string(id_opts->basic, SDAP_SASL_REALM, sasl_realm);
-    if (ret != EOK) goto done;
-    DEBUG(SSSDBG_CONF_SETTINGS,
-          ("Option %s set to %s\n",
-           id_opts->basic[SDAP_SASL_REALM].opt_name,
-           sasl_realm));
-
     /* fix schema to AD  */
     id_opts->schema_type = SDAP_SCHEMA_AD;
 
diff --git a/src/providers/ipa/ipa_common.c b/src/providers/ipa/ipa_common.c
index db736921eaf02666fce24a57a297885fac23bfef..4c68f61d5c4f91f06c193ceb40631af2c242e332 100644
--- a/src/providers/ipa/ipa_common.c
+++ b/src/providers/ipa/ipa_common.c
@@ -168,14 +168,9 @@ int ipa_get_id_options(struct ipa_options *ipa_opts,
                        struct sdap_options **_opts)
 {
     TALLOC_CTX *tmpctx;
-    char *primary;
     char *basedn;
     char *realm;
     char *value;
-    char *desired_realm;
-    char *desired_primary;
-    bool primary_requested = true;
-    bool realm_requested = true;
     int ret;
     int i;
 
@@ -248,51 +243,17 @@ int ipa_get_id_options(struct ipa_options *ipa_opts,
                   dp_opt_get_string(ipa_opts->id->basic, SDAP_KRB5_REALM)));
     }
 
-    /* Configuration of SASL auth ID and realm */
-    desired_primary = dp_opt_get_string(ipa_opts->id->basic, SDAP_SASL_AUTHID);
-    if (!desired_primary) {
-        primary_requested = false;
-        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) {
-        realm_requested = false;
-        desired_realm = dp_opt_get_string(ipa_opts->id->basic, SDAP_KRB5_REALM);
-    }
-
-    ret = select_principal_from_keytab(tmpctx,
-                                       desired_primary, desired_realm,
-                                       dp_opt_get_string(ipa_opts->id->basic,
-                                                         SDAP_KRB5_KEYTAB),
-                                       NULL, &primary, &realm);
-    if (ret != EOK) {
-        goto done;
-    }
-
-    if ((primary_requested && strcmp(desired_primary, primary) != 0) ||
-        (realm_requested && strcmp(desired_realm, realm) != 0)) {
-        DEBUG(1, ("Configured SASL auth ID/realm not found in keytab.\n"));
-        ret = ENOENT;
-        goto done;
-    }
-
-    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);
+    ret = sdap_set_sasl_options(ipa_opts->id,
+                                dp_opt_get_string(ipa_opts->id->basic,
+                                                  IPA_HOSTNAME),
+                                dp_opt_get_string(ipa_opts->id->basic,
+                                                  SDAP_KRB5_REALM),
+                                dp_opt_get_string(ipa_opts->id->basic,
+                                                  SDAP_KRB5_KEYTAB));
     if (ret != EOK) {
+        DEBUG(SSSDBG_OP_FAILURE, ("Cannot set the SASL-related options\n"));
         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)));
 
     /* fix schema to IPAv1 for now */
     ipa_opts->id->schema_type = SDAP_SCHEMA_IPA_V1;
diff --git a/src/providers/ldap/ldap_common.c b/src/providers/ldap/ldap_common.c
index da5786fbf2acb89d3ffecfe2af1a4e035423bf4d..07e9c5d4fd09358e4157bf189703f7f7266bf164 100644
--- a/src/providers/ldap/ldap_common.c
+++ b/src/providers/ldap/ldap_common.c
@@ -999,6 +999,78 @@ done:
     return ret;
 }
 
+errno_t
+sdap_set_sasl_options(struct sdap_options *id_opts,
+                      char *default_primary,
+                      char *default_realm,
+                      const char *keytab_path)
+{
+    errno_t ret;
+    TALLOC_CTX *tmp_ctx;
+    char *sasl_primary;
+    char *desired_primary;
+    char *sasl_realm;
+    char *desired_realm;
+    bool primary_requested = true;
+    bool realm_requested = true;
+
+    tmp_ctx = talloc_new(NULL);
+    if (!tmp_ctx) return ENOMEM;
+
+    /* Configuration of SASL auth ID and realm */
+    desired_primary = dp_opt_get_string(id_opts->basic, SDAP_SASL_AUTHID);
+    if (!desired_primary) {
+        primary_requested = false;
+        desired_primary = default_primary;
+    }
+
+    desired_realm = dp_opt_get_string(id_opts->basic, SDAP_SASL_REALM);
+    if (!desired_realm) {
+        realm_requested = false;
+        desired_realm = default_realm;
+    }
+
+    ret = select_principal_from_keytab(tmp_ctx,
+                                       desired_primary, desired_realm,
+                                       keytab_path,
+                                       NULL, &sasl_primary, &sasl_realm);
+    if (ret != EOK) {
+        goto done;
+    }
+
+    if ((primary_requested && strcmp(desired_primary, sasl_primary) != 0) ||
+        (realm_requested && strcmp(desired_realm, sasl_realm) != 0)) {
+        DEBUG(SSSDBG_CRIT_FAILURE,
+               ("Configured SASL auth ID/realm not found in keytab.\n"));
+        ret = ENOENT;
+        goto done;
+    }
+
+    ret = dp_opt_set_string(id_opts->basic,
+                            SDAP_SASL_AUTHID, sasl_primary);
+    if (ret != EOK) {
+        goto done;
+    }
+    DEBUG(SSSDBG_CONF_SETTINGS, ("Option %s set to %s\n",
+          id_opts->basic[SDAP_SASL_AUTHID].opt_name,
+          dp_opt_get_string(id_opts->basic, SDAP_SASL_AUTHID)));
+
+    ret = dp_opt_set_string(id_opts->basic,
+                            SDAP_SASL_REALM, sasl_realm);
+    if (ret != EOK) {
+        goto done;
+    }
+
+    DEBUG(SSSDBG_CONF_SETTINGS, ("Option %s set to %s\n",
+          id_opts->basic[SDAP_SASL_REALM].opt_name,
+          dp_opt_get_string(id_opts->basic, SDAP_SASL_REALM)));
+
+    ret = EOK;
+done:
+    talloc_free(tmp_ctx);
+    return ret;
+}
+
 static const char *
 sdap_gssapi_get_default_realm(TALLOC_CTX *mem_ctx)
 {
diff --git a/src/providers/ldap/ldap_common.h b/src/providers/ldap/ldap_common.h
index 034dc995bfa6969dd01955744735d9d6214b9c19..86079fa6ad2aedcf220d38251adc94d60d654bd1 100644
--- a/src/providers/ldap/ldap_common.h
+++ b/src/providers/ldap/ldap_common.h
@@ -228,4 +228,11 @@ sdap_attrs_get_sid_str(TALLOC_CTX *mem_ctx,
                        struct sysdb_attrs *sysdb_attrs,
                        const char *sid_attr,
                        char **_sid_str);
+
+errno_t
+sdap_set_sasl_options(struct sdap_options *id_opts,
+                      char *default_primary,
+                      char *default_realm,
+                      const char *keytab_path);
+
 #endif /* _LDAP_COMMON_H_ */
-- 
1.8.0

-------------- next part --------------
>From 56452e4a92df0891b86621d50ae737ada77bad45 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Mon, 19 Nov 2012 10:43:07 +0100
Subject: [PATCH 3/3] LDAP: Make it possible to use full principal in
 ldap_sasl_authid again

When the guessing of principals was introduced, we changed the existing
behaviour and instead of allowing both host/hostname at REALM and
host/hostname, only the latter worked and the realm was always appended
from the (then undocumented) ldap_sasl_realm option.

This patch changes the behaviour to be more backwards-compatible -- if
the ldap_sasl_authid contains the @-sign, then the ldap_sasl_realm is
not always appended.

The strict requirement of checking the requested authid/realm against
the one found in keytab was also kind of relaxed because it didn't
really work. For example when hostname was requested and the keytab
matched host/hostname first, the code blew up.

https://fedorahosted.org/sssd/ticket/1635
---
 src/man/sssd-ldap.5.xml          |  5 +++++
 src/providers/ldap/ldap_common.c | 36 ++++++++++++++++++++++++++----------
 2 files changed, 31 insertions(+), 10 deletions(-)

diff --git a/src/man/sssd-ldap.5.xml b/src/man/sssd-ldap.5.xml
index 2d62c11f232c4f1fd15b547b9ac3f4cfb7c0e170..b1be45fe2eb7c5900bcaca83d5d2ccb2335c8600 100644
--- a/src/man/sssd-ldap.5.xml
+++ b/src/man/sssd-ldap.5.xml
@@ -1418,6 +1418,9 @@
                             Specify the SASL authorization id to use.
                             When GSSAPI is used, this represents the Kerberos
                             principal used for authentication to the directory.
+                            This option can either contain the full principal (for
+                            example host/myhost at EXAMPLE.COM) or just the principal name
+                            (for example host/myhost).
                         </para>
                         <para>
                             Default: host/hostname at REALM
@@ -1431,6 +1434,8 @@
                         <para>
                             Specify the SASL realm to use. When not specified,
                             this option defaults to the value of krb5_realm.
+                            If the ldap_sasl_authid contains the realm as well,
+                            this option is ignored.
                         </para>
                         <para>
                             Default: the value of krb5_realm.
diff --git a/src/providers/ldap/ldap_common.c b/src/providers/ldap/ldap_common.c
index 07e9c5d4fd09358e4157bf189703f7f7266bf164..f8b921adfe0aabc8f7ab70caab4b0201f7959c46 100644
--- a/src/providers/ldap/ldap_common.c
+++ b/src/providers/ldap/ldap_common.c
@@ -1009,6 +1009,7 @@ sdap_set_sasl_options(struct sdap_options *id_opts,
     TALLOC_CTX *tmp_ctx;
     char *sasl_primary;
     char *desired_primary;
+    char *primary_realm;
     char *sasl_realm;
     char *desired_realm;
     bool primary_requested = true;
@@ -1024,12 +1025,23 @@ sdap_set_sasl_options(struct sdap_options *id_opts,
         desired_primary = default_primary;
     }
 
-    desired_realm = dp_opt_get_string(id_opts->basic, SDAP_SASL_REALM);
-    if (!desired_realm) {
-        realm_requested = false;
-        desired_realm = default_realm;
+    if ((primary_realm = strchr(desired_primary, '@'))) {
+        *primary_realm = '\0';
+        desired_realm = primary_realm+1;
+        DEBUG(SSSDBG_TRACE_INTERNAL,
+              ("authid contains realm [%s]\n", desired_realm));
+    } else {
+        desired_realm = dp_opt_get_string(id_opts->basic, SDAP_SASL_REALM);
+        if (!desired_realm) {
+            realm_requested = false;
+            desired_realm = default_realm;
+        }
     }
 
+    DEBUG(SSSDBG_CONF_SETTINGS, ("Will look for %s@%s in %s\n",
+          desired_primary, desired_realm,
+          keytab_path ? keytab_path : "default keytab"));
+
     ret = select_principal_from_keytab(tmp_ctx,
                                        desired_primary, desired_realm,
                                        keytab_path,
@@ -1038,12 +1050,16 @@ sdap_set_sasl_options(struct sdap_options *id_opts,
         goto done;
     }
 
-    if ((primary_requested && strcmp(desired_primary, sasl_primary) != 0) ||
-        (realm_requested && strcmp(desired_realm, sasl_realm) != 0)) {
-        DEBUG(SSSDBG_CRIT_FAILURE,
-               ("Configured SASL auth ID/realm not found in keytab.\n"));
-        ret = ENOENT;
-        goto done;
+    if (primary_requested && strcmp(desired_primary, sasl_primary) != 0) {
+        DEBUG(SSSDBG_MINOR_FAILURE,
+               ("Configured SASL auth ID not found in keytab. "
+                "Requested %s, found %s\n", desired_primary, sasl_primary));
+    }
+
+    if (realm_requested && strcmp(desired_realm, sasl_realm) != 0) {
+        DEBUG(SSSDBG_MINOR_FAILURE,
+               ("Configured SASL realm not found in keytab. "
+                "Requested %s, found %s\n", desired_realm, sasl_realm));
     }
 
     ret = dp_opt_set_string(id_opts->basic,
-- 
1.8.0



More information about the sssd-devel mailing list