[PATCH] allow passdb backend to change trusted domain object password with clear text

[Jeremy Allison]

On Sat, Apr 08, 2017 at 07:46:23AM +1200, Andrew Bartlett wrote:
> On Fri, 2017-04-07 at 12:12 -0700, Jeremy Allison via samba-technical
> wrote:
> > On Thu, Apr 06, 2017 at 06:46:06PM +0300, Alexander Bokovoy via
> > samba-technical wrote:
> > > Hi,
> > > 
> > > attached patch switches _netr_ServerPasswordSet2 to use
> > > SetUserInfo2
> > > info level 26. This allows us to pass through clear text password
> > > change
> > > down to passdb backend. This is critical for AD-like configurations
> > > (FreeIPA) where it is not enough to change NT or LM hashes for TDO,
> > > as
> > > one needs to generate Kerberos keys as well.
> > > 
> > > I'm working on a corresponding change in FreeIPA ipasam module as
> > > well.
> > > It currently does not provide pdb_update_sam_account() callback so
> > > end
> > > result is still NT_STATUS_NOT_IMPLEMENTED as can be witnessed with
> > > 'nltest /sc_change_pwd:ipa.domain' but we are getting closer.
> > > +							const char
> > > *account_name,
> > > +							DATA_BLOB
> > > *plain_text)
> > > +{
> > > +	NTSTATUS status;
> > > +	NTSTATUS result = NT_STATUS_OK;
> > > +	struct dcerpc_binding_handle *h = NULL;
> > > +	struct tsocket_address *local;
> > > +	struct policy_handle user_handle;
> > > +	uint32_t acct_ctrl;
> > > +	union samr_UserInfo *info;
> > > +	struct samr_UserInfo26 info26;
> > > +	int rc;
> > > +	DATA_BLOB session_key;
> > > +
> > > +	ZERO_STRUCT(user_handle);
> > > +
> > > +	status = session_extract_session_key(session_info,
> > > +					     &session_key,
> > > +					     KEY_USE_16BYTES);
> > > +	if (!NT_STATUS_IS_OK(status)) {
> > > +		goto out;
> > > +	}
> > > +
> > > +	rc = tsocket_address_inet_from_strings(mem_ctx,
> > > +					       "ip",
> > > +					       "127.0.0.1",
> > > +					       0,
> > > +					       &local);
> > 
> > Alexander - is the above going to work on an IPv6-only
> > box ?
> > 
> > Can you test that please.
> 
> Those strings are just used for logging

Nope. The code looks like:

+       rc = tsocket_address_inet_from_strings(mem_ctx,
+                                              "ip",
+                                              "127.0.0.1",
+                                              0,
+                                              &local);
+       if (rc < 0) {
+               status = NT_STATUS_NO_MEMORY;
+               goto out;
+       }

Third argument is "addr" which gets passed internally
to getaddrinfo(). So it's calling getaddrinfo("127.0.0.1",...).

What does that do an an IPv6-only box ? The right thing ?
Maybe. But if it returns any error then we exit here, despite IPv6-localhost
existing ::1 and being valid.

Maybe. But we shouldn't be encoding IPv4-specific addresses
anymore on a machine that potentially doesn't use them.

That's what I was complaining about :-). I spent a long time
making us IPv6 clean and I don't want to see regressions.

Jeremy.