[Freeipa-devel] [PATCH] 0100 Enumerate UPN suffixes in ipasam
Alexander Bokovoy
abokovoy at redhat.com
Wed Mar 27 10:53:18 UTC 2013
Hi,
On Wed, 27 Mar 2013, Sumit Bose wrote:
>> Additionally, you can request Windows to update list of name suffixes
>> via UI. Here is how it looks in Windows 2012 Server:
>> http://abbra.fedorapeople.org/.paste/win2012-multiple-suffixes.png
>>
>> Part of ticket https://fedorahosted.org/freeipa/ticket/2848
>
>I've tested the attached patch with the samba packages mentioned above
>and everything works as expect.
>
>As can be seen in the figure Alexander linked above the new suffixes are
>disabled by default on the Windows side. This is expected and exactly
>the same behaviour can be found in AD-AD trusts. Nevertheless it would
>be good if you can make sure this behaviour is explicitly mentioned e.g.
>in the design page or other documents to avoid confusion when this
>feature is tested by others.
I'll add that, thanks for reminding.
>
>Review comments are in-line.
>
>bye,
>Sumit
>
>>
>>
>> --
>> / Alexander Bokovoy
>
>> >From f400d55eaec99b3e5440e90b6a6d055e26529e7e Mon Sep 17 00:00:00 2001
>> From: Alexander Bokovoy <abokovoy at redhat.com>
>> Date: Fri, 22 Mar 2013 17:30:41 +0200
>> Subject: [PATCH 2/2] ipasam: add enumeration of UPN suffixes based on the
>> realm domains
>>
>> PASSDB API in Samba adds support for specifying UPN suffixes. The change
>> in ipasam will allow to pass through list of realm domains as UPN suffixes
>> so that Active Directory domain controller will be able to recognize
>> non-primary UPN suffixes as belonging to IPA and properly find our KDC
>> for cross-realm TGT.
>>
>> Since Samba already returns primary DNS domain separately, filter it out
>> from list of UPN suffixes.
>>
>> Also enclose provider of UPN suffixes into #ifdef to support both
>> Samba with and without pdb_enum_upn_suffixes().
>>
>> Part of https://fedorahosted.org/freeipa/ticket/2848
>> ---
>> daemons/configure.ac | 10 +++
>> daemons/ipa-sam/ipa_sam.c | 172 ++++++++++++++++++++++++++++++++++++++++++++--
>> 2 files changed, 177 insertions(+), 5 deletions(-)
>>
>> diff --git a/daemons/configure.ac b/daemons/configure.ac
>> index d3b6b19..14dc04e 100644
>> --- a/daemons/configure.ac
>> +++ b/daemons/configure.ac
>> @@ -252,6 +252,16 @@ AC_CHECK_LIB([wbclient],
>> [$SAMBA40EXTRA_LIBPATH])
>> AC_SUBST(WBCLIENT_LIBS)
>>
>> +AC_CHECK_LIB([pdb],
>> + [make_pdb_method],
>> + [HAVE_LIBPDB=1],
>> + [AC_MSG_ERROR([libpdb does not have make_pdb_method])],
>> + [$SAMBA40EXTRA_LIBPATH])
>> +AC_CHECK_LIB([pdb],[pdb_enum_upn_suffixes],
>> + [AC_DEFINE([HAVE_PDB_ENUM_UPN_SUFFIXES], [1], [Ability to enumerate UPN suffixes])],
>> + [AC_MSG_WARN([libpdb does not have pdb_enum_upn_suffixes, no support for realm domains in ipasam])],
>> + [$SAMBA40EXTRA_LIBPATH])
>
>any reason for the extra space in the indentation?
No :)
>> +
>> dnl ---------------------------------------------------------------------------
>> dnl - Check for check unit test framework http://check.sourceforge.net/
>> dnl ---------------------------------------------------------------------------
>
>...
>
>> +static char **get_attribute_values(TALLOC_CTX *mem_ctx, LDAP *ldap_struct,
>> + LDAPMessage *entry, const char *attribute, int *num_values)
>> +{
>> + struct berval **values;
>> + int count, i;
>> + char **result = NULL;
>> + size_t conv_size;
>> +
>> + if (attribute == NULL || entry == NULL) {
>> + return NULL;
>> + }
>> +
>> + values = ldap_get_values_len(ldap_struct, entry, attribute);
>> + if (values == NULL) {
>> + DEBUG(10, ("Attribute [%s] not found.\n", attribute));
>> + return NULL;
>> + }
>
>if ldap_get_values_len() returns NULL in the case of an error (according
>to the man page), if there are no attributes, an empty array is
>returned. This also means that count can be 0 below.
I ended up specifically checking count to zero and returning before
allocating values.
>> +#ifdef HAVE_PDB_ENUM_UPN_SUFFIXES
>> +static NTSTATUS ipasam_enum_upn_suffixes(struct pdb_methods *pdb_methods,
>> + TALLOC_CTX *mem_ctx,
>> + uint32_t *num_suffixes,
>> + char ***suffixes)
>> +{
>> + int ret;
>> + LDAPMessage *result;
>> + LDAPMessage *entry = NULL;
>> + int count, i;
>> + char *realmdomains_dn = NULL;
>> + char **domains = NULL;
>> + struct ldapsam_privates *ldap_state;
>> + struct smbldap_state *smbldap_state;
>> + const char *attr_list[] = {
>> + "associatedDomain",
>
>would you mind to #define attribute names and directory paths at the
>beginning of ipa_sam.c?
Yes, I was wondering about that before. We have another places where we
use associatedDomain now. I've moved all of them to defines.
>> + NULL
>> + };
>> +
>> + if ((suffixes == NULL) || (num_suffixes == NULL)) {
>> + return NT_STATUS_UNSUCCESSFUL;
>> + }
>> +
>> + ldap_state = (struct ldapsam_privates *)pdb_methods->private_data;
>> + smbldap_state = ldap_state->smbldap_state;
>> +
>> + realmdomains_dn = talloc_asprintf(mem_ctx, "cn=Realm Domains,cn=ipa,cn=etc,%s",
>> + ldap_state->ipasam_privates->base_dn);
>> + if (realmdomains_dn == NULL) {
>> + return NT_STATUS_NO_MEMORY;
>> + }
>> +
>> + ret = smbldap_search(smbldap_state,
>> + realmdomains_dn,
>> + LDAP_SCOPE_BASE,
>> + "objectclass=domainRelatedObject", attr_list, 0,
>> + &result);
>> + if (ret != LDAP_SUCCESS) {
>> + DEBUG(1, ("Failed to get list of realm domains: %s\n",
>> + ldap_err2string (ret)));
>> + return NT_STATUS_UNSUCCESSFUL;
>> + }
>> +
>> + count = ldap_count_entries(smbldap_state->ldap_struct, result);
>> + if (count != 1) {
>> + DEBUG(1, ("Unexpected number of results [%d] for realm domains "
>> + "search.\n", count));
>> + ldap_msgfree(result);
>> + return NT_STATUS_OK;
>> + }
>
>Is there a reason to return NT_STATUS_OK for this particular error
>condition?
Actually, it should have been NT_STATUS_UNSUCCESSFUL and it is the same
error as in ipasam_get_domain_name. I've fixed both now.
>> +
>> + entry = ldap_first_entry(smbldap_state->ldap_struct, result);
>> + if (entry == NULL) {
>> + DEBUG(0, ("Could not get domainRelatedObject entry\n"));
>> + ldap_msgfree(result);
>> + return NT_STATUS_UNSUCCESSFUL;
>> + }
>> +
>> + domains = get_attribute_values(mem_ctx, smbldap_state->ldap_struct, entry,
>> + "associatedDomain", &count);
>> + if (domains == NULL) {
>> + ldap_msgfree(result);
>> + return NT_STATUS_UNSUCCESSFUL;
>> + }
>> +
>> + /* Since associatedDomain has attributeType MUST, there must be at least one domain */
>> + for (i = 0; i < count ; i++) {
>> + if (strcmp(ldap_state->domain_name, domains[i]) == 0) {
>> + break;
>> + }
>> + }
>
>Since we area handling DNS domain names here strcasecmp() would be more
>fault tolerant? OTOH I think mixed cases here can only happen if some
>modifies IPA LDAP object manually.
Technically it should be something that does utf-8 caseless lookups. We
can go with strcasecmp as it is for time being, I'll add TODO there for
future IDN handling.
>
>> +
>> + if (i < count) {
>> + /* If we found our primary domain in the list and it is alone, exit with empty list */
>> + if (count == 1) {
>> + ldap_msgfree(result);
>> + talloc_free(domains);
>> + return NT_STATUS_UNSUCCESSFUL;
>> + }
>> +
>> + talloc_free(domains[i]);
>> +
>> + /* if i is not last element, move everything down */
>> + if (i != (count - 1)) {
>> + memcpy(domains + i, domains + i + 1, sizeof(char *) * (count - i - 1));
>> + }
>
>I think you meant memmove() according to your comment, because memcpy()
>should not be used for overlapping areas.
Fixed, thanks!
>Maybe add domains[count - 1] = NULL; as a precaution measure?
Added this as well.
New patch attached. Survives Windows 2012 testing.
--
/ Alexander Bokovoy
-------------- next part --------------
>From 48906c4bb1226a174de3acf4a656fd6eb4466218 Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy <abokovoy at redhat.com>
Date: Fri, 22 Mar 2013 17:30:41 +0200
Subject: [PATCH 2/2] ipasam: add enumeration of UPN suffixes based on the
realm domains
PASSDB API in Samba adds support for specifying UPN suffixes. The change
in ipasam will allow to pass through list of realm domains as UPN suffixes
so that Active Directory domain controller will be able to recognize
non-primary UPN suffixes as belonging to IPA and properly find our KDC
for cross-realm TGT.
Since Samba already returns primary DNS domain separately, filter it out
from list of UPN suffixes.
Also enclose provider of UPN suffixes into #ifdef to support both
Samba with and without pdb_enum_upn_suffixes().
Part of https://fedorahosted.org/freeipa/ticket/2848
---
daemons/configure.ac | 10 +++
daemons/ipa-sam/ipa_sam.c | 192 +++++++++++++++++++++++++++++++++++++++++++---
2 files changed, 191 insertions(+), 11 deletions(-)
diff --git a/daemons/configure.ac b/daemons/configure.ac
index d3b6b19..0eb0646 100644
--- a/daemons/configure.ac
+++ b/daemons/configure.ac
@@ -252,6 +252,16 @@ AC_CHECK_LIB([wbclient],
[$SAMBA40EXTRA_LIBPATH])
AC_SUBST(WBCLIENT_LIBS)
+AC_CHECK_LIB([pdb],
+ [make_pdb_method],
+ [HAVE_LIBPDB=1],
+ [AC_MSG_ERROR([libpdb does not have make_pdb_method])],
+ [$SAMBA40EXTRA_LIBPATH])
+AC_CHECK_LIB([pdb],[pdb_enum_upn_suffixes],
+ [AC_DEFINE([HAVE_PDB_ENUM_UPN_SUFFIXES], [1], [Ability to enumerate UPN suffixes])],
+ [AC_MSG_WARN([libpdb does not have pdb_enum_upn_suffixes, no support for realm domains in ipasam])],
+ [$SAMBA40EXTRA_LIBPATH])
+
dnl ---------------------------------------------------------------------------
dnl - Check for check unit test framework http://check.sourceforge.net/
dnl ---------------------------------------------------------------------------
diff --git a/daemons/ipa-sam/ipa_sam.c b/daemons/ipa-sam/ipa_sam.c
index dd3ad61..4a2fca5 100644
--- a/daemons/ipa-sam/ipa_sam.c
+++ b/daemons/ipa-sam/ipa_sam.c
@@ -1,6 +1,7 @@
#define HAVE_IMMEDIATE_STRUCTURES 1
#define LDAP_DEPRECATED 1
+#include "config.h"
#include <stdbool.h>
#include <stdint.h>
#include <stdio.h>
@@ -127,6 +128,7 @@ bool secrets_store(const char *key, const void *data, size_t size); /* available
#define LDAP_ATTRIBUTE_NTHASH "ipaNTHash"
#define LDAP_ATTRIBUTE_UIDNUMBER "uidnumber"
#define LDAP_ATTRIBUTE_GIDNUMBER "gidnumber"
+#define LDAP_ATTRIBUTE_ASSOCIATED_DOMAIN "associatedDomain"
#define LDAP_OBJ_KRB_PRINCIPAL "krbPrincipal"
#define LDAP_OBJ_KRB_PRINCIPAL_AUX "krbPrincipalAux"
@@ -141,6 +143,9 @@ bool secrets_store(const char *key, const void *data, size_t size); /* available
#define LDAP_OBJ_NESTEDGROUP "nestedGroup"
#define LDAP_OBJ_IPAUSERGROUP "ipaUserGroup"
#define LDAP_OBJ_POSIXGROUP "posixGroup"
+#define LDAP_OBJ_DOMAINRELATED "domainRelatedObject"
+
+#define LDAP_CN_REALM_DOMAINS "cn=Realm Domains,cn=ipa,cn=etc"
#define HAS_KRB_PRINCIPAL (1<<0)
#define HAS_KRB_PRINCIPAL_AUX (1<<1)
@@ -167,6 +172,12 @@ struct ipasam_privates {
struct sss_idmap_ctx *idmap_ctx;
};
+
+static NTSTATUS ipasam_get_domain_name(struct ldapsam_privates *ldap_state,
+ TALLOC_CTX *mem_ctx,
+ char **domain_name);
+
+
static void *idmap_talloc(size_t size, void *pvt)
{
return talloc_size(pvt, size);
@@ -295,6 +306,56 @@ static LDAP *priv2ld(struct ldapsam_privates *priv)
return priv->smbldap_state->ldap_struct;
}
+/*
+ * get_attribute_values() returns array of all values of the attribute
+ * allocated over mem_ctx
+ */
+static char **get_attribute_values(TALLOC_CTX *mem_ctx, LDAP *ldap_struct,
+ LDAPMessage *entry, const char *attribute, int *num_values)
+{
+ struct berval **values;
+ int count, i;
+ char **result = NULL;
+ size_t conv_size;
+
+ if (attribute == NULL || entry == NULL) {
+ return NULL;
+ }
+
+ values = ldap_get_values_len(ldap_struct, entry, attribute);
+ if (values == NULL) {
+ DEBUG(10, ("Attribute [%s] not found.\n", attribute));
+ return NULL;
+ }
+
+ count = ldap_count_values_len(values);
+ if (count == 0) {
+ goto done;
+ }
+
+ result = talloc_array(mem_ctx, char *, count);
+ if (result == NULL) {
+ goto done;
+ }
+
+ *num_values = count;
+ for (i = 0; i < count; i++) {
+ if (!convert_string_talloc(result, CH_UTF8, CH_UNIX,
+ values[i]->bv_val, values[i]->bv_len,
+ &result[i], &conv_size)) {
+ DEBUG(10, ("Failed to convert %dth value of [%s] out of %d.\n",
+ i, attribute, count));
+ talloc_free(result);
+ result = NULL;
+ goto done;
+ }
+ }
+
+done:
+ ldap_value_free_len(values);
+ return result;
+}
+
static char *get_single_attribute(TALLOC_CTX *mem_ctx, LDAP *ldap_struct,
LDAPMessage *entry, const char *attribute)
{
@@ -3250,9 +3311,8 @@ static struct pdb_domain_info *pdb_ipasam_get_domain_info(struct pdb_methods *pd
goto fail;
}
- /* TODO: read dns_domain, dns_forest and guid from LDAP */
- info->dns_domain = strlower_talloc(info, ldap_state->ipasam_privates->realm);
- if (info->dns_domain == NULL) {
+ status = ipasam_get_domain_name(ldap_state, info, &info->dns_domain);
+ if (!NT_STATUS_IS_OK(status) || (info->dns_domain == NULL)) {
goto fail;
}
info->dns_forest = talloc_strdup(info, info->dns_domain);
@@ -3464,7 +3524,7 @@ static NTSTATUS ipasam_get_base_dn(struct smbldap_state *smbldap_state,
static NTSTATUS ipasam_get_domain_name(struct ldapsam_privates *ldap_state,
TALLOC_CTX *mem_ctx,
- const char **domain_name)
+ char **domain_name)
{
int ret;
LDAPMessage *result;
@@ -3473,14 +3533,14 @@ static NTSTATUS ipasam_get_domain_name(struct ldapsam_privates *ldap_state,
char *cn;
struct smbldap_state *smbldap_state = ldap_state->smbldap_state;
const char *attr_list[] = {
- "associatedDomain",
+ LDAP_ATTRIBUTE_ASSOCIATED_DOMAIN,
NULL
};
ret = smbldap_search(smbldap_state,
ldap_state->ipasam_privates->base_dn,
LDAP_SCOPE_BASE,
- "objectclass=domainRelatedObject", attr_list, 0,
+ "objectclass=" LDAP_OBJ_DOMAINRELATED, attr_list, 0,
&result);
if (ret != LDAP_SUCCESS) {
DEBUG(1, ("Failed to get domain name: %s\n",
@@ -3494,11 +3554,10 @@ static NTSTATUS ipasam_get_domain_name(struct ldapsam_privates *ldap_state,
DEBUG(1, ("Unexpected number of results [%d] for domain name "
"search.\n", count));
ldap_msgfree(result);
- return NT_STATUS_OK;
+ return NT_STATUS_UNSUCCESSFUL;
}
- entry = ldap_first_entry(smbldap_state->ldap_struct,
- result);
+ entry = ldap_first_entry(smbldap_state->ldap_struct, result);
if (entry == NULL) {
DEBUG(0, ("Could not get domainRelatedObject entry\n"));
ldap_msgfree(result);
@@ -3506,7 +3565,7 @@ static NTSTATUS ipasam_get_domain_name(struct ldapsam_privates *ldap_state,
}
cn = get_single_attribute(mem_ctx, smbldap_state->ldap_struct, entry,
- "associatedDomain");
+ LDAP_ATTRIBUTE_ASSOCIATED_DOMAIN);
if (cn == NULL) {
ldap_msgfree(result);
return NT_STATUS_UNSUCCESSFUL;
@@ -3572,6 +3631,112 @@ static NTSTATUS ipasam_get_realm(struct ldapsam_privates *ldap_state,
return NT_STATUS_OK;
}
+#ifdef HAVE_PDB_ENUM_UPN_SUFFIXES
+static NTSTATUS ipasam_enum_upn_suffixes(struct pdb_methods *pdb_methods,
+ TALLOC_CTX *mem_ctx,
+ uint32_t *num_suffixes,
+ char ***suffixes)
+{
+ int ret;
+ LDAPMessage *result;
+ LDAPMessage *entry = NULL;
+ int count, i;
+ char *realmdomains_dn = NULL;
+ char **domains = NULL;
+ struct ldapsam_privates *ldap_state;
+ struct smbldap_state *smbldap_state;
+ const char *attr_list[] = {
+ LDAP_ATTRIBUTE_ASSOCIATED_DOMAIN,
+ NULL
+ };
+
+ if ((suffixes == NULL) || (num_suffixes == NULL)) {
+ return NT_STATUS_UNSUCCESSFUL;
+ }
+
+ ldap_state = (struct ldapsam_privates *)pdb_methods->private_data;
+ smbldap_state = ldap_state->smbldap_state;
+
+ realmdomains_dn = talloc_asprintf(mem_ctx, "%s,%s", LDAP_CN_REALM_DOMAINS,
+ ldap_state->ipasam_privates->base_dn);
+ if (realmdomains_dn == NULL) {
+ return NT_STATUS_NO_MEMORY;
+ }
+
+ ret = smbldap_search(smbldap_state,
+ realmdomains_dn,
+ LDAP_SCOPE_BASE,
+ "objectclass=" LDAP_OBJ_DOMAINRELATED, attr_list, 0,
+ &result);
+ if (ret != LDAP_SUCCESS) {
+ DEBUG(1, ("Failed to get list of realm domains: %s\n",
+ ldap_err2string (ret)));
+ return NT_STATUS_UNSUCCESSFUL;
+ }
+
+ count = ldap_count_entries(smbldap_state->ldap_struct, result);
+ if (count != 1) {
+ DEBUG(1, ("Unexpected number of results [%d] for realm domains "
+ "search.\n", count));
+ ldap_msgfree(result);
+ return NT_STATUS_UNSUCCESSFUL;
+ }
+
+ entry = ldap_first_entry(smbldap_state->ldap_struct, result);
+ if (entry == NULL) {
+ DEBUG(0, ("Could not get domainRelatedObject entry\n"));
+ ldap_msgfree(result);
+ return NT_STATUS_UNSUCCESSFUL;
+ }
+
+ domains = get_attribute_values(mem_ctx, smbldap_state->ldap_struct, entry,
+ LDAP_ATTRIBUTE_ASSOCIATED_DOMAIN, &count);
+ if (domains == NULL) {
+ ldap_msgfree(result);
+ return NT_STATUS_UNSUCCESSFUL;
+ }
+
+ /* Since associatedDomain has attributeType MUST, there must be at least one domain */
+ for (i = 0; i < count ; i++) {
+ /* TODO: use comparison function friendly to IDN */
+ if (strcasecmp(ldap_state->domain_name, domains[i]) == 0) {
+ break;
+ }
+ }
+
+ if (i < count) {
+ /* If we found our primary domain in the list and it is alone, exit with empty list */
+ if (count == 1) {
+ ldap_msgfree(result);
+ talloc_free(domains);
+ return NT_STATUS_UNSUCCESSFUL;
+ }
+
+ talloc_free(domains[i]);
+
+ /* if i is not last element, move everything down */
+ if (i != (count - 1)) {
+ memmove(domains + i, domains + i + 1, sizeof(char *) * (count - i - 1));
+ }
+
+ /* we don't resize whole list, only reduce number of elements in it
+ * since sizing down a single pointer will not reduce memory usage in talloc
+ */
+ domains[count - 1] = NULL;
+ *suffixes = domains;
+ *num_suffixes = count - 1;
+ } else {
+ /* There is no our primary domain in the list */
+ *suffixes = domains;
+ *num_suffixes = count;
+ }
+
+ ldap_msgfree(result);
+ return NT_STATUS_OK;
+}
+#endif /* HAVE_PDB_ENUM_UPN_SUFFIXES */
+
+
#define SECRETS_DOMAIN_SID "SECRETS/SID"
static char *sec_key(TALLOC_CTX *mem_ctx, const char *d)
{
@@ -4030,7 +4195,7 @@ static NTSTATUS pdb_init_ipasam(struct pdb_methods **pdb_method,
}
status = ipasam_get_domain_name(ldap_state, ldap_state,
- &ldap_state->domain_name);
+ (char**) &ldap_state->domain_name);
if (!NT_STATUS_IS_OK(status)) {
DEBUG(0, ("Failed to get domain name.\n"));
return status;
@@ -4150,6 +4315,11 @@ static NTSTATUS pdb_init_ipasam(struct pdb_methods **pdb_method,
(*pdb_method)->set_trusted_domain = ipasam_set_trusted_domain;
(*pdb_method)->del_trusted_domain = ipasam_del_trusted_domain;
(*pdb_method)->enum_trusted_domains = ipasam_enum_trusted_domains;
+#ifdef HAVE_PDB_ENUM_UPN_SUFFIXES
+ (*pdb_method)->enum_upn_suffixes = ipasam_enum_upn_suffixes;
+ DEBUG(1, ("pdb_init_ipasam: support for pdb_enum_upn_suffixes "
+ "enabled for domain %s\n", ldap_state->domain_name));
+#endif
return NT_STATUS_OK;
}
--
1.8.1.4
More information about the Freeipa-devel
mailing list