[PATCH] winbindd: support foreign domains when NTLM is restricted
Jeremy Allison
jra at samba.org
Fri Feb 19 00:32:21 UTC 2016
On Wed, Feb 10, 2016 at 12:57:18AM +0200, Uri Simchoni wrote:
> Hi,
>
> Attached is a patch to fix
> https://bugzilla.samba.org/show_bug.cgi?id=11691 - winbindd doesn't
> connect to a trusted domain if the DC of that domain refuses NTLM
> authentication. Review appreciated.
>
> The basic idea is to pre-mark the domain as AD domain based on trust
> properties, thus allowing the initial connection to be attempted
> using Kerberos.
>
> The patch is marked V2 because I circulated a similar patch a few
> days ago - basically it's a bit cleaner and possibly more correct in
> a couple of places (re-scanning root domain).
>
> In the previous message I expressed my desire to further clean up
> the code in this area, namely that foreign domain properties will be
> determined ONLY based on trust properties, not by contacting the
> domain, and possibly some other cleanups - this will wait for
> another time I'm afraid, and also this limited change is safer for
> backporting.
Sorry for the delay Uri, I will try and get
to this asap !
> From 70f9328a799a374a057db5651b9b17a26feca759 Mon Sep 17 00:00:00 2001
> From: Uri Simchoni <uri at samba.org>
> Date: Wed, 10 Feb 2016 00:26:45 +0200
> Subject: [PATCH v2 1/3] winbindd: introduce add_trusted_domain_from_tdc()
>
> This is purely a refactoring patch -
> Add a routine that adds a winbindd domain object based on
> domain trust cache entry. add_trusted_domain() becomes
> a wrapper for this new routine.
>
> BUG: https://bugzilla.samba.org/show_bug.cgi?id=11691
>
> Signed-off-by: Uri Simchoni <uri at samba.org>
> ---
> source3/winbindd/winbindd_util.c | 72 +++++++++++++++++++++++++---------------
> 1 file changed, 46 insertions(+), 26 deletions(-)
>
> diff --git a/source3/winbindd/winbindd_util.c b/source3/winbindd/winbindd_util.c
> index 8139daa..307ae6d 100644
> --- a/source3/winbindd/winbindd_util.c
> +++ b/source3/winbindd/winbindd_util.c
> @@ -34,6 +34,10 @@
> #undef DBGC_CLASS
> #define DBGC_CLASS DBGC_WINBIND
>
> +static struct winbindd_domain *
> +add_trusted_domain_from_tdc(const struct winbindd_tdc_domain *tdc,
> + struct winbindd_methods *methods);
> +
> extern struct winbindd_methods cache_methods;
>
> /**
> @@ -119,14 +123,40 @@ static bool is_in_internal_domain(const struct dom_sid *sid)
> If the domain already exists in the list,
> return it and don't re-initialize. */
>
> -static struct winbindd_domain *add_trusted_domain(const char *domain_name, const char *alt_name,
> - struct winbindd_methods *methods,
> - const struct dom_sid *sid)
> +static struct winbindd_domain *
> +add_trusted_domain(const char *domain_name, const char *alt_name,
> + struct winbindd_methods *methods, const struct dom_sid *sid)
> +{
> + struct winbindd_tdc_domain tdc;
> +
> + ZERO_STRUCT(tdc);
> +
> + tdc.domain_name = domain_name;
> + tdc.dns_name = alt_name;
> + if (sid) {
> + sid_copy(&tdc.sid, sid);
> + }
> +
> + return add_trusted_domain_from_tdc(&tdc, methods);
> +}
> +
> +/* Add a trusted domain out of a trusted domain cache
> + entry
> +*/
> +static struct winbindd_domain *
> +add_trusted_domain_from_tdc(const struct winbindd_tdc_domain *tdc,
> + struct winbindd_methods *methods)
> {
> struct winbindd_domain *domain;
> const char *alternative_name = NULL;
> const char **ignored_domains, **dom;
> int role = lp_server_role();
> + const char *domain_name = tdc->domain_name;
> + const struct dom_sid *sid = &tdc->sid;
> +
> + if (is_null_sid(sid)) {
> + sid = NULL;
> + }
>
> ignored_domains = lp_parm_string_list(-1, "winbind", "ignore domains", NULL);
> for (dom=ignored_domains; dom && *dom; dom++) {
> @@ -138,8 +168,8 @@ static struct winbindd_domain *add_trusted_domain(const char *domain_name, const
>
> /* use alt_name if available to allow DNS lookups */
>
> - if (alt_name && *alt_name) {
> - alternative_name = alt_name;
> + if (tdc->dns_name && *tdc->dns_name) {
> + alternative_name = tdc->dns_name;
> }
>
> /* We can't call domain_list() as this function is called from
> @@ -151,8 +181,7 @@ static struct winbindd_domain *add_trusted_domain(const char *domain_name, const
> break;
> }
>
> - if (alternative_name && *alternative_name)
> - {
> + if (alternative_name) {
> if (strequal(alternative_name, domain->name) ||
> strequal(alternative_name, domain->alt_name))
> {
> @@ -160,12 +189,7 @@ static struct winbindd_domain *add_trusted_domain(const char *domain_name, const
> }
> }
>
> - if (sid)
> - {
> - if (is_null_sid(sid)) {
> - continue;
> - }
> -
> + if (sid != NULL) {
> if (dom_sid_equal(sid, &domain->sid)) {
> break;
> }
> @@ -219,11 +243,11 @@ static struct winbindd_domain *add_trusted_domain(const char *domain_name, const
> domain->internal = is_internal_domain(sid);
> domain->sequence_number = DOM_SEQUENCE_NONE;
> domain->last_seq_check = 0;
> - domain->initialized = False;
> + domain->initialized = false;
> domain->online = is_internal_domain(sid);
> domain->check_online_timeout = 0;
> domain->dc_probe_pid = (pid_t)-1;
> - if (sid) {
> + if (sid != NULL) {
> sid_copy(&domain->sid, sid);
> }
>
> @@ -252,9 +276,9 @@ static struct winbindd_domain *add_trusted_domain(const char *domain_name, const
>
> setup_domain_child(domain);
>
> - DEBUG(2,("Added domain %s %s %s\n",
> - domain->name, domain->alt_name,
> - &domain->sid?sid_string_dbg(&domain->sid):""));
> + DEBUG(2,
> + ("Added domain %s %s %s\n", domain->name, domain->alt_name,
> + !is_null_sid(&domain->sid) ? sid_string_dbg(&domain->sid) : ""));
>
> return domain;
> }
> @@ -438,10 +462,8 @@ static void rescan_forest_root_trusts( void )
> d = find_domain_from_name_noinit( dom_list[i].domain_name );
>
> if ( !d ) {
> - d = add_trusted_domain( dom_list[i].domain_name,
> - dom_list[i].dns_name,
> - &cache_methods,
> - &dom_list[i].sid );
> + d = add_trusted_domain_from_tdc(&dom_list[i],
> + &cache_methods);
> }
>
> if (d == NULL) {
> @@ -507,10 +529,8 @@ static void rescan_forest_trusts( void )
> about it */
>
> if ( !d ) {
> - d = add_trusted_domain( dom_list[i].domain_name,
> - dom_list[i].dns_name,
> - &cache_methods,
> - &dom_list[i].sid );
> + d = add_trusted_domain_from_tdc(&dom_list[i],
> + &cache_methods);
> }
>
> if (d == NULL) {
> --
> 2.5.0
>
>
> From e3596024427fda6a02e1e5a5058455e45630cc88 Mon Sep 17 00:00:00 2001
> From: Uri Simchoni <uri at samba.org>
> Date: Wed, 10 Feb 2016 00:32:23 +0200
> Subject: [PATCH v2 2/3] winbindd: initialize foreign domain as AD based on
> trust
>
> Based on trust parameters, initialize the active_directory
> member of domain object to true.
>
> BUG: https://bugzilla.samba.org/show_bug.cgi?id=11691
>
> Signed-off-by: Uri Simchoni <uri at samba.org>
> ---
> source3/winbindd/winbindd_util.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/source3/winbindd/winbindd_util.c b/source3/winbindd/winbindd_util.c
> index 307ae6d..cb8d80f 100644
> --- a/source3/winbindd/winbindd_util.c
> +++ b/source3/winbindd/winbindd_util.c
> @@ -250,6 +250,9 @@ add_trusted_domain_from_tdc(const struct winbindd_tdc_domain *tdc,
> if (sid != NULL) {
> sid_copy(&domain->sid, sid);
> }
> + domain->domain_flags = tdc->trust_flags;
> + domain->domain_type = tdc->trust_type;
> + domain->domain_trust_attribs = tdc->trust_attribs;
>
> /* Is this our primary domain ? */
> if (strequal(domain_name, get_global_sam_name()) &&
> @@ -267,6 +270,10 @@ add_trusted_domain_from_tdc(const struct winbindd_tdc_domain *tdc,
> if (lp_security() == SEC_ADS) {
> domain->active_directory = true;
> }
> + } else if (!domain->internal) {
> + if (domain->domain_type == LSA_TRUST_TYPE_UPLEVEL) {
> + domain->active_directory = true;
> + }
> }
>
> /* Link to domain list */
> --
> 2.5.0
>
>
> From fa4b826b9d010f993ad6183cf9a1a63a2cf23ca1 Mon Sep 17 00:00:00 2001
> From: Uri Simchoni <uri at samba.org>
> Date: Wed, 10 Feb 2016 00:38:11 +0200
> Subject: [PATCH v2 3/3] winbindd: return trust parameters when listing trusts
>
> When asking a child domain process to list trusts on that domain,
> return (along with trust domain names and SID) the trust properties -
> flags, type, and attributes.
>
> Use those attributes to initialize domain object.
>
> BUG: https://bugzilla.samba.org/show_bug.cgi?id=11691
>
> Signed-off-by: Uri Simchoni <uri at samba.org>
> ---
> source3/winbindd/winbindd_misc.c | 11 +++---
> source3/winbindd/winbindd_util.c | 74 +++++++++++++++++++++++++++++-----------
> 2 files changed, 61 insertions(+), 24 deletions(-)
>
> diff --git a/source3/winbindd/winbindd_misc.c b/source3/winbindd/winbindd_misc.c
> index 29831aa..d32a71e 100644
> --- a/source3/winbindd/winbindd_misc.c
> +++ b/source3/winbindd/winbindd_misc.c
> @@ -181,11 +181,12 @@ enum winbindd_result winbindd_dual_list_trusted_domains(struct winbindd_domain *
> }
>
> extra_data = talloc_asprintf_append_buffer(
> - extra_data, "%s\\%s\\%s\n",
> - trusts.array[i].netbios_name,
> - trusts.array[i].dns_name,
> - sid_string_talloc(state->mem_ctx,
> - trusts.array[i].sid));
> + extra_data, "%s\\%s\\%s\\%u\\%u\\%u\n",
> + trusts.array[i].netbios_name, trusts.array[i].dns_name,
> + sid_string_talloc(state->mem_ctx, trusts.array[i].sid),
> + trusts.array[i].trust_flags,
> + (uint32_t)trusts.array[i].trust_type,
> + trusts.array[i].trust_attributes);
> }
>
> /* add our primary domain */
> diff --git a/source3/winbindd/winbindd_util.c b/source3/winbindd/winbindd_util.c
> index cb8d80f..dfc5ea3 100644
> --- a/source3/winbindd/winbindd_util.c
> +++ b/source3/winbindd/winbindd_util.c
> @@ -343,24 +343,37 @@ static void trustdom_list_done(struct tevent_req *req)
> struct winbindd_response *response;
> int res, err;
> char *p;
> + struct winbindd_tdc_domain trust_params = {0};
> + ptrdiff_t extra_len;
>
> res = wb_domain_request_recv(req, state, &response, &err);
> if ((res == -1) || (response->result != WINBINDD_OK)) {
> - DEBUG(1, ("Could not receive trustdoms\n"));
> + DBG_WARNING("Could not receive trustdoms\n");
> TALLOC_FREE(state);
> return;
> }
>
> + if (response->length < sizeof(struct winbindd_response)) {
> + DBG_ERR("ill-formed trustdom response - short length\n");
> + TALLOC_FREE(state);
> + return;
> + }
> +
> + extra_len = response->length - sizeof(struct winbindd_response);
> +
> p = (char *)response->extra_data.data;
>
> - while ((p != NULL) && (*p != '\0')) {
> + while ((p - (char *)response->extra_data.data) < extra_len) {
> char *q, *sidstr, *alt_name;
> - struct dom_sid sid;
> - char *alternate_name = NULL;
> +
> + DBG_DEBUG("parsing response line '%s'\n", p);
> +
> + ZERO_STRUCT(trust_params);
> + trust_params.domain_name = p;
>
> alt_name = strchr(p, '\\');
> if (alt_name == NULL) {
> - DEBUG(0, ("Got invalid trustdom response\n"));
> + DBG_ERR("Got invalid trustdom response\n");
> break;
> }
>
> @@ -369,26 +382,52 @@ static void trustdom_list_done(struct tevent_req *req)
>
> sidstr = strchr(alt_name, '\\');
> if (sidstr == NULL) {
> - DEBUG(0, ("Got invalid trustdom response\n"));
> + DBG_ERR("Got invalid trustdom response\n");
> break;
> }
>
> *sidstr = '\0';
> sidstr += 1;
>
> - q = strchr(sidstr, '\n');
> - if (q != NULL)
> - *q = '\0';
> + /* use the real alt_name if we have one, else pass in NULL */
> + if (!strequal(alt_name, "(null)")) {
> + trust_params.dns_name = alt_name;
> + }
> +
> + q = strtok(sidstr, "\\");
> + if (q == NULL) {
> + DBG_ERR("Got invalid trustdom response\n");
> + break;
> + }
>
> - if (!string_to_sid(&sid, sidstr)) {
> + if (!string_to_sid(&trust_params.sid, sidstr)) {
> DEBUG(0, ("Got invalid trustdom response\n"));
> break;
> }
>
> - /* use the real alt_name if we have one, else pass in NULL */
> + q = strtok(NULL, "\\");
> + if (q == NULL) {
> + DBG_ERR("Got invalid trustdom response\n");
> + break;
> + }
> +
> + trust_params.trust_flags = (uint32_t)strtoul(q, NULL, 10);
> +
> + q = strtok(NULL, "\\");
> + if (q == NULL) {
> + DBG_ERR("Got invalid trustdom response\n");
> + break;
> + }
> +
> + trust_params.trust_type = (uint32_t)strtoul(q, NULL, 10);
> +
> + q = strtok(NULL, "\n");
> + if (q == NULL) {
> + DBG_ERR("Got invalid trustdom response\n");
> + break;
> + }
>
> - if ( !strequal( alt_name, "(null)" ) )
> - alternate_name = alt_name;
> + trust_params.trust_attribs = (uint32_t)strtoul(q, NULL, 10);
>
> /*
> * We always call add_trusted_domain() cause on an existing
> @@ -396,13 +435,10 @@ static void trustdom_list_done(struct tevent_req *req)
> * This is important because we need the SID for sibling
> * domains.
> */
> - (void)add_trusted_domain(p, alternate_name,
> - &cache_methods,
> - &sid);
> + (void)add_trusted_domain_from_tdc(&trust_params,
> + &cache_methods);
>
> - p=q;
> - if (p != NULL)
> - p += 1;
> + p = q + strlen(q) + 1;
> }
>
> /*
> --
> 2.5.0
>
More information about the samba-technical
mailing list