[SCM] Samba Shared Repository - branch master updated -
release-4-0-0alpha7-1462-g78754ab
Jeremy Allison
jra at samba.org
Wed May 6 18:29:33 GMT 2009
On Wed, May 06, 2009 at 10:48:44AM -0700, Jeremy Allison wrote:
> On Wed, May 06, 2009 at 12:39:36PM -0500, Günther Deschner wrote:
> > The branch, master has been updated
> > via 78754ab2c9b28ea8ab09d3fd1f5450abe721a2c1 (commit)
> > from 730c91aaaad42c68fdb44bc51fee6c89e0c22910 (commit)
> >
> > http://gitweb.samba.org/?p=samba.git;a=shortlog;h=master
> >
> >
> > - Log -----------------------------------------------------------------
> > commit 78754ab2c9b28ea8ab09d3fd1f5450abe721a2c1
> > Author: Günther Deschner <gd at samba.org>
> > Date: Wed May 6 19:29:01 2009 +0200
> >
> > s3-netlogon: Fix NETLOGON credential chain. Fixes Bug #6099 (Windows 7 joining Samba3) and probably many, many more.
> >
> > Jeremy, with 9a5d5cc1db0ee60486f932e34cd7961b90c70a56 you alter the in negotiate
> > flags (which are a pointer to the out negotiate flags assigned in the generated
> > netlogon server code). So, while you wanted to just set the *out* negflags, you
> > did in fact reset the *in* negflags, effectively eliminating the
> > NETLOGON_NEG_STRONG_KEYS bit (formerly known as NETLOGON_NEG_128BIT) which then
> > caused creds_server_init() to generate 64bit creds instead of 128bit, causing
> > the whole chain to break. *Please* check.
>
> Wow - great catch ! Great work Guenther. I'm looking at it now.
> I think we probably need some comments here also to explain the
> details. Looks like I got caught by badly named auto-generated
> variable names (r->out.negotiate_flags actually being the "in"
> flags is not very obvious :-).
Ok, your patch is completely correct (IMHO - but as I'm the
one who messed up in the first place take this with a grain
of salt :-).
However, I'd like to add this aditional patch on top (attached)
to clarify some things. Firstly, I want to have a comment
warning that r->in.negotiate_flags is an aliased pointer to
r->out.negotiate_flags, so I (or others) don't mess up again.
Secondly, currently we're returning server negotiated flags
of 0x000001ff - which doesn't include the NETLOGON_NEG_STRONG_KEYS
bit even if the client requested it and we are supporting it
in the credential chain. This seems like a bug to me as according
to the MS docs the client should be checking the returned flags
for that bit and erroring out if we don't set it. So I'd like
to add that bit into our reply. See this change :
/* Ensure we support strong (128-bit) keys. */
if (in_neg_flags & NETLOGON_NEG_STRONG_KEYS) {
srv_flgs |= NETLOGON_NEG_STRONG_KEYS;
}
in my proposed additional patch. Thirdly I'd like to
clean up the error exits so we always return the same
way.
Please let me know what you think and if it passes
your Win7 testing.
Thanks !
Jeremy.
-------------- next part --------------
diff --git a/source3/rpc_server/srv_netlog_nt.c b/source3/rpc_server/srv_netlog_nt.c
index edd1321..333eabe 100644
--- a/source3/rpc_server/srv_netlog_nt.c
+++ b/source3/rpc_server/srv_netlog_nt.c
@@ -508,13 +508,16 @@ NTSTATUS _netr_ServerAuthenticate3(pipes_struct *p,
{
NTSTATUS status;
uint32_t srv_flgs;
+ /* r->in.negotiate_flags is an aliased pointer to r->out.negotiate_flags,
+ * so use a copy to avoid destroying the client values. */
+ uint32_t in_neg_flags = *r->in.negotiate_flags;
struct netr_Credential srv_chal_out;
const char *fn;
/* According to Microsoft (see bugid #6099)
* Windows 7 looks at the negotiate_flags
* returned in this structure *even if the
- * call fails with access denied ! So in order
+ * call fails with access denied* ! So in order
* to allow Win7 to connect to a Samba NT style
* PDC we set the flags before we know if it's
* an error or not.
@@ -531,6 +534,11 @@ NTSTATUS _netr_ServerAuthenticate3(pipes_struct *p,
NETLOGON_NEG_REDO |
NETLOGON_NEG_PASSWORD_CHANGE_REFUSAL;
+ /* Ensure we support strong (128-bit) keys. */
+ if (in_neg_flags & NETLOGON_NEG_STRONG_KEYS) {
+ srv_flgs |= NETLOGON_NEG_STRONG_KEYS;
+ }
+
if (lp_server_schannel() != false) {
srv_flgs |= NETLOGON_NEG_SCHANNEL;
}
@@ -552,19 +560,19 @@ NTSTATUS _netr_ServerAuthenticate3(pipes_struct *p,
if (!p->dc || !p->dc->challenge_sent) {
DEBUG(0,("%s: no challenge sent to client %s\n", fn,
r->in.computer_name));
- *r->out.negotiate_flags = srv_flgs;
- return NT_STATUS_ACCESS_DENIED;
+ status = NT_STATUS_ACCESS_DENIED;
+ goto out;
}
if ( (lp_server_schannel() == true) &&
- ((*r->in.negotiate_flags & NETLOGON_NEG_SCHANNEL) == 0) ) {
+ ((in_neg_flags & NETLOGON_NEG_SCHANNEL) == 0) ) {
/* schannel must be used, but client did not offer it. */
DEBUG(0,("%s: schannel required but client failed "
"to offer it. Client was %s\n",
fn, r->in.account_name));
- *r->out.negotiate_flags = srv_flgs;
- return NT_STATUS_ACCESS_DENIED;
+ status = NT_STATUS_ACCESS_DENIED;
+ goto out;
}
status = get_md4pw((char *)p->dc->mach_pw,
@@ -576,12 +584,12 @@ NTSTATUS _netr_ServerAuthenticate3(pipes_struct *p,
"account %s: %s\n",
fn, r->in.account_name, nt_errstr(status) ));
/* always return NT_STATUS_ACCESS_DENIED */
- *r->out.negotiate_flags = srv_flgs;
- return NT_STATUS_ACCESS_DENIED;
+ status = NT_STATUS_ACCESS_DENIED;
+ goto out;
}
/* From the client / server challenges and md4 password, generate sess key */
- creds_server_init(*r->in.negotiate_flags,
+ creds_server_init(in_neg_flags,
p->dc,
&p->dc->clnt_chal, /* Stored client chal. */
&p->dc->srv_chal, /* Stored server chal. */
@@ -594,8 +602,8 @@ NTSTATUS _netr_ServerAuthenticate3(pipes_struct *p,
"request from client %s machine account %s\n",
fn, r->in.computer_name,
r->in.account_name));
- *r->out.negotiate_flags = srv_flgs;
- return NT_STATUS_ACCESS_DENIED;
+ status = NT_STATUS_ACCESS_DENIED;
+ goto out;
}
/* set up the LSA AUTH 2 response */
memcpy(r->out.return_credentials->data, &srv_chal_out.data,
@@ -613,10 +621,12 @@ NTSTATUS _netr_ServerAuthenticate3(pipes_struct *p,
r->in.computer_name,
p->dc);
unbecome_root();
+ status = NT_STATUS_OK;
- *r->out.negotiate_flags = srv_flgs;
+ out:
- return NT_STATUS_OK;
+ *r->out.negotiate_flags = srv_flgs;
+ return status;
}
/*************************************************************************
More information about the samba-technical
mailing list