[PATCH] support for kerberos in plugin DC code

[Stefan (metze) Metzmacher]

Am 02.08.2011 01:01, schrieb Andrew Bartlett:
> On Fri, 2011-07-29 at 17:09 +0200, Stefan (metze) Metzmacher wrote:
>>>> I'd like you to give quite some time to review and decide if it is ok.
>>>> I have been opposed on introducing gensec in s3 for a few reasons. One
>>>> is dependencies, the other is that IIRC gensec does not create new event
>>>> loops bu allows nesting of loops. That is something too dangerous for
>>>> the file server imho.
>>>
>>> Yes, this needs a lot of review, I hope to get some time in the next days.
>>
>> Here're my first result, but I'll do more review on monday:
> 
> I've addressed all these issues (including the
> auth_ntlmssp_steal_session_info(), but not removing the auth4_context
> for the reasons I've explained).  Can you please let me know soon if you
> have any further comments that cannot be addressed in-tree, as I would
> like to move forward with this work.

Thanks, here some more comments:

- Please avoid } else { after return statements,
  neesting can be avoided in that case and it makes the
  code easier to read.

  +                       return nt_status;
  +               } else {

- Please use (at least in future)

  /*
   * some multi line
   * text...
   */

  instead of
  /* some multi line
   * text ... */

  It makes the code much more readable.

- In source4/utils/ntlm_auth.c
  nt_status = gensec_session_info(state->gensec_state, NULL, &session_info);
  NULL => mem_ctx to avoid memory leaks.

- In s3-ntlmssp Add hooks to optionally call into GENSEC in auth_ntlmssp
  In auth_ntlmssp_want_feature this comment seems to be wrong:
  /* You need to negotiate signing to get a windows server to calculate
a session
   * key */

- In s3-ntlmssp Add mem_ctx argument to auth_ntlmssp_update
  auth_ntlmssp_update doesn't handle MORE_PROCESSING_REQUIRED.

- It would be nice to have
  (blob1.length > 7 && strncmp((char *)(blob1.data), "NTLMSSP", 7) == 0)
  in a helper function.

The rest looks good to me, did you run your wintest?

metze

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 262 bytes
Desc: OpenPGP digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20110802/03c043ec/attachment.pgp>