[PATCH] Allow GetDCNameEx to be called for arbitrary sites and trusted domains

[Stefan Metzmacher]

Am 04.04.2018 um 01:42 schrieb Garming Sam:
> Hi,
> 
> I think I've addressed everything. There's also an additional patch
> marked SQUASH that checks info and info[0] is non-NULL if you think
> that's appropriate.

In addition to the check of the 'status' variable, which just means
the response was correctly transmitted from the server (winbindd) to the
client (the netlogon server), we need to check the 'result' variable
before looking at other r->out.* elements at all.

Something like this (please don't try to safe lines by using '||' for
the if statements):

        if (!NT_STATUS_IS_OK(result)) {
		DBG_NOTICE("DC location via winbind returned - %s\n",
			   nt_errstr(result));
		state->r.out.result = result;
		goto finished;
        }

Shouldn't we use something other than WERR_NO_SUCH_DOMAIN?
This is more an internal error.

+	if (!NT_STATUS_IS_OK(status)) {
+		DBG_NOTICE("DC location via winbind failed - %s\n",
+			   nt_errstr(status));
+		state->r.out.result = WERR_NO_SUCH_DOMAIN;
+		goto finished;
+	}

info and info[0] is non-NULL should also be checked in a separate if
statement, with a DBG_ERR or DBG_WARNING message indicating something
like an internal error.

I also noticed that wb_irpc_GetDCName_done() should return the result of
wb_dsgetdcname_recv() as application result using:

state->r->out.result = status; and
irpc_send_reply(state->msg, NT_STATUS_OK);

and the memory context passed to wb_dsgetdcname_recv() should be
state->dcinfo (if it's a valid talloc pointer) or state->msg

> I've also modified some of the comments slightly to
> indicate that NETBIOS names actually do work (even before the patch
> where I clobber it), but not always, to which I'm still not at all sure why.
> 
> Cheers,
> 
> Garming
> 
> On 04/04/18 10:20, Garming Sam wrote:
>> On 04/04/18 02:14, Stefan Metzmacher wrote:
>>> Now a few logic things:
>>>
>>> - Can we really safely dereference state->r.out.info[0]->
>>>   in dcesrv_netr_DsRGetDCName_base_done() if result is not
>>>   NT_STATUS_OK?
>> It only dereferences info if the NTSTATUS of
>> dcerpc_winbind_DsGetDcName_recv is ok. winbind is supposed to return
>> DOMAIN_CONTROLLER_NOT_FOUND at this top level. If you want, I can
>> double-check the info before dereferencing it.
>>
>>> - dc_unc = talloc_asprintf(state->dce_call, uses the wrong
>>>   memory context, it should be state->r.out.info[0] (if we can assume a
>>>   valid talloc pointer) or state->mem_ctx.
>> I didn't have state->mem_ctx before, so yes, I will change that.
>>
>>> - Don't we need to check the result of samdb_client_site_name()
>>>   in dcesrv_netr_DsRGetDCName_base_call() ?
>> Not really. It can be NULL, and that doesn't mean anything bad has
>> happened. Returning NULL to the RPC call is also quite expected.

Ok.

>>> - Is "netlogon: Resolve calls to GetDCNameEx2 within the same
>>>   NETLOGON domain" really needed? "The return will have the DNS domain,
>>>   which is not quite as nice, but it does not seem to violate any
>>>   assumptions" sounds risky...
>> So, in general, when you specify the domain name as a NETBIOS name,
>> Windows seems to return the DC UNC in NETBIOS form (and this follows the
>> same pattern with DNS names). All this patch means is that we end up
>> returning DNS names (for DCs outside our site), but if you specify the
>> flag to return a particular format, everything is as expected.

To me the patch looks like it's forcing the dns name even if the client
asked for a netbios name (DS_IS_FLAT_NAME) explicitly.

>> We don't seem to follow this convention even when we didn't go to winbind. We
>> seem to need this patch, because against Windows, we must be getting
>> slightly different results via our DC locator calls and without it, we
>> get strange intermittent failures even while querying within the same
>> domain. I haven't really got the time to look at why this might be
>> happening.

I think we should better understand what's going on here.

metze

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: OpenPGP digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20180404/485b3b2b/signature.sig>