Question about joining machines to a domain.

[Jeremy Allison]

On Wed, Mar 15, 2017 at 11:47:07AM -0700, Jeremy Allison via samba-technical wrote:
> Hi guys,
> 
> In source3/libnet/libnet_join.c we have:
> 
>        /* Attempt to create the machine account and bail if this fails.
>            Assume that the admin wants exactly what they requested */
> 
>         status = ads_create_machine_acct(r->in.ads,
>                                          r->in.machine_name,
>                                          r->in.account_ou,
>                                          r->in.desired_encryption_types);
> 
>         if (ADS_ERR_OK(status)) {
>                 DEBUG(1,("machine account creation created\n"));
>                 return status;
>         } else  if ((status.error_type == ENUM_ADS_ERROR_LDAP) &&
>                     (status.err.rc == LDAP_ALREADY_EXISTS)) {
>                 status = ADS_SUCCESS;
>         }
> 
>         if (!ADS_ERR_OK(status)) {
>                 DEBUG(1,("machine account creation failed\n"));
>                 return status;
>         }
> 
>         status = ads_move_machine_acct(r->in.ads,
>                                        r->in.machine_name,
>                                        r->in.account_ou,
>                                        &moved);
>         if (!ADS_ERR_OK(status)) {
>                 DEBUG(1,("failure to locate/move pre-existing "
>                         "machine account\n"));
>                 return status;
>         }
> 
>         DEBUG(1,("The machine account %s the specified OU.\n",
>                 moved ? "was moved into" : "already exists in"));
> 
> As you can see it treats LDAP_ALREADY_EXISTS as "ok" and moves
> the account anyway. Should we move the account to the new
> OU if it already existed, or should we leave it where it
> was ?

Looking at this some more, at the top of this function we have:

static ADS_STATUS libnet_join_precreate_machine_acct(TALLOC_CTX *mem_ctx,
                                                     struct libnet_JoinCtx *r)
{
        ADS_STATUS status;
        LDAPMessage *res = NULL;
        const char *attrs[] = { "dn", NULL };
        bool moved = false;

        status = ads_check_ou_dn(mem_ctx, r->in.ads, &r->in.account_ou);
        if (!ADS_ERR_OK(status)) {
                return status;
        }

If someone just ran 'net ads join' without using the 'createcomputer' parameter,
then on entry to this function r->in.account_ou == NULL.

The call to ads_check_ou_dn() fills this in to a valid value for the following
ads_create_machine_acct() call.

Could we make the ads_move_machine_acct() conditional on r->in.account_ou not
initially being NULL ?

That way if someone does use createcomputer=dn on the command line, then we move
it if it already existed, but if they don't, then we leave it where it was.

We already do something similar inside the calling function libnet_DomainJoin(),
which has:

                ads_status = libnet_join_precreate_machine_acct(mem_ctx, r);
                if (ADS_ERR_OK(ads_status)) {

                        /*
                         * LDAP object create succeeded, now go to the rpc
                         * password set routines
                         */

                        r->in.join_flags &= ~WKSSVC_JOIN_FLAGS_ACCOUNT_CREATE;
                        goto rpc_join;
                }

                if (initial_account_ou != NULL) {
                        libnet_join_set_error_string(mem_ctx, r,
                                "failed to precreate account in ou %s: %s",
                                r->in.account_ou,
                                ads_errstr(ads_status));
                        return WERR_NERR_DEFAULTJOINREQUIRED;
                }

                DEBUG(5, ("failed to precreate account in ou %s: %s",
                        r->in.account_ou, ads_errstr(ads_status)));

so there appears to be precedence for this.

Thoughts ?

Jeremy.