Fix smartcard offline logon and NTLM authentication

[Andrew Bartlett]

On Mon, 2016-06-27 at 15:02 +1200, Andrew Bartlett wrote:
> On Mon, 2016-06-20 at 22:55 +0200, Stefan Metzmacher wrote:
> > 
> > Hi,
> > 
> > here're some patches to fix smartcard offline logons
> > and related bugs.
> > 
> > The key part is adding PAC_CREDENTIAL with the NTHASH.
> > 
> > In order to avoid an NTHASH based on a password,
> > I also implemented the UF_SMARTCARD_REQUIRED feature,
> > that generates a random NTHASH value, that is only
> > known to the KDC and the private key of the smartcard.
> > 
> > I may need to add some more BUG: markers, but you can start
> > with the review now:-)
> > 
> > See
> > https://git.samba.org/?p=metze/samba/wip.git;a=shortlog;h=refs/head
> > s/
> > master4-smart-ok
> > it's based on
> > https://git.samba.org/?p=metze/samba/wip.git;a=shortlog;h=refs/head
> > s/
> > master4-smart-base
> G'Day metze,
> 
> I can't see any tests for the critical components of this task, that
> is
> the changed PAC.  Can you add a test that confirms the returned PAC
> has
> the correct password, nor that these elements are present?
> 
> I'll keep looking over the rest of the changes.  I know you mention
> adding more BUG: markers, which is OK, but please don't backport
> these.
>  Samba 4.5 is coming soon enough, and I would really prefer not to
> see
> big backports made for pwdLastSet nor smart card login features.  
> 
> Finally, please ensure that you fix the code to pass the repl_move
> test.  This is sensitive to the exact repl_meta_data behaviour, in
> particular the number of password attributes with metadata, but it
> seems we still don't match Windows even with your changes. 

I've put review markers on some of the patches at: 

http://git.catalyst.net.nz/gitweb?p=samba.git;a=shortlog;h=refs/heads/m
etze-logoncount-smart

However, to review the others, I would like to see:
 - example values for supplementalCredentials put in
source4/torture/ndr/drsblobs.c to test the changed IDL and the new
manual parser. 
 - similar tests for the PAC changes.  In particular, I'm nervous
because of the issues last time we introduced PAC_UPN_DNS_INFO last
time, and because there are probably other more sloppy PAC parsers out
there in other products.
 - Tests to show we fill in the PAC correctly, both in the PKINIT and
not-PKINIT case (given we are adding the UPN stuff).
 - Confirmation that the password in the PAC is correct (as above).  
 - You could pull one password with GetNCChanges REPL_OBJECT if you
want to test the randomly generated case. 

I would also like to thank you for your incredible patience in this
area.  I have neglected your requests for review for too long, and I
hope this downpayment is some small way towards reducing the backlog.

I'm really quite excited by what Samba 4.5 looks like having for the AD
DC.  My team here at Catalyst are working on a lot of performance and
correctness issues right now around linked attributes and LDB, and we
are gaining some big improvements!

Thanks!

Andrew Bartlett

-- 
Andrew Bartlett
https://samba.org/~abartlet/
Authentication Developer, Samba Team         https://samba.org
Samba Development and Support, Catalyst IT   
https://catalyst.net.nz/services/samba





-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20160627/7b831aa9/signature.sig>