[PATCH] addon patches for domain join failure (Re: [SCM] Samba Shared Repository - branch master updated)

Günther Deschner gd at samba.org
Fri Jun 17 11:59:20 UTC 2016

Hash: SHA1

Hi Michael,

On 16/06/16 11:30, Michael Adam wrote:
> - The place for early-success-out in the flow of this function is
> further up where valid_realm is set. This is the error treatment
> path of the function.
> ==> So this patch is *wrong* since it will let the config check 
> succeed when the workgroup is wrong!

Good catch, thanks! We certainly need to stop once lp_workgroup does
not match.

> - The comment is kind-of redundant: the actual code says it all!
> :-)

Well, kind of yes. But I think it is fine to add an additional comment
to explain why this is done.

> - The comment and debug msg are not quite correct: This will not
> only warn when realm is unset, but also when it is set to a
> different value than the detected dns domain name
> (r->out.dns_domain_name), which I think is OK.

Yes, this is intended, and I think it is OK as well :)

> - I question the content of actual warning message: Why *should* I
> configure a realm when using 'security=domain' ?

Because a lot of code relies on lp_realm() when talking to AD since we
initially moved all AD and RPC join routines to the libnetjoin
interfaces (e.g. the gensec gse_krb5 backend, some of the namelookup
code, etc.).

> I could buy this recommendation when also 'winbind rpc only = no'. 
> So if we add a warning, we should make it dependent on the value of
> this parameter, imho.
> ==> this makes me remember that I would like to change the default
> of 'winbind rpc only' to 'yes'. Sending a patch separately...

??? Sorry, but why should we change the default of this ?

>> libnet_join_set_error_string(mem_ctx, r, "Invalid configuration
>> (%s) and configuration modification " "was not requested",
>> wrong_conf); +
> Sneaking in an extra blank line? :-)

I think it reads nicer with having a newline here.

> Attached find 3 patches I propose for the fix:
> #1 reverts this patch #2 achieves the same intended difference in
> behavior #3 adds the debug message in early-success path

I think the other proposed patchset is not correct as it would then
emit the warning regardless whether we deal with AD or not, right?


- -- 
Günther Deschner                    GPG-ID: 8EE11688
Red Hat                         gdeschner at redhat.com
Samba Team                              gd at samba.org
Version: GnuPG v2


More information about the samba-technical mailing list