{PATCH] store extra password hashes in supplemental credentials

[Andrew Bartlett]

On Wed, 2017-04-12 at 15:38 +1200, Andrew Bartlett via samba-technical
wrote:
> On Wed, 2017-04-12 at 13:39 +1200, Gary Lockyer via samba-technical
> wrote:
> > Patches reworked to use {Crypt}
> 
> Thanks Gary.
> 
> > On 12/04/17 11:33, Stefan Metzmacher wrote:
> > > Gi Gary and Andrew,
> > > 
> > > > Completed patch set to:
> > > > - Calculate SHA256 and SHA512 password hashes and store in
> > > >   supplementalCredentials Primary:userPassword
> > > > - add configuration options to control the generation of these
> > > >   hashes and the number of rounds used to calculate them.
> > > >   * 'password hash additional scheme'
> > > >   * 'password hash sha256 rounds'
> > > >   * 'password hash sha512 rounds'
> > > > - add new virtual attributes virtualWDigest01 to
> > > > virtualWDigest29
> > > > to
> > > >   make the WDigest values available
> > > > - change virtualCryptSHA256 and virtualCryptSHA512 to:
> > > >   * return the stored values in Primary:userPassword if
> > > > available
> > > >   * honor 'password hash sha256 rounds' and
> > > >     'password hash sha512 rounds' when calculating the hashes.
> > > > 
> > > > Review appreciated
> > > 
> > > I still think we should use {CRYPT} if we're using the crypt_r()
> > > function
> > > to generate the value.
> 
> I'm really sorry for not getting this the first few times you asked
> for
> it. 
> 
> > > Looking at the openldap code I see
> > > {SHA256} and {SHA512} only in contrib/slapd-modules/passwd/sha2/,
> > > but not in the main code libraries/liblutil/passwd.c
> > > 
> > > Looking at the code I can't see how the value you calculate
> > > using crypt_r() can be verified with the {SHA256} and {SHA512}
> > > code.
> > > If we don't want to use {CRYPT} we should implement {SSHA256} and
> > > {SSHA512} (with salt)
> > > instead of {SHA256} and {SHA512} (without salt) and match the
> > > actual
> > > implementation.
> 
> Indeed.  I'm embarrassed I pushed this so far without properly
> looking
> at the code/specification.  I'm also rather shocked that even in 2009
> that unsalted hash values were being considered, let alone
> implemented!
> 
> > > I still think it's confusing to have "password hash sha256
> > > rounds"
> > > and
> > > "password hash sha512 rounds" as separate options.
> 
> OK.  The patch Gary re-submitted keeps those (renamed) because it
> makes
> it easier to re-use them for your samba-tool user getpassword case,
> but
> I'm willing to be convinced.  If you could remind you 

Err..

If you could suggest a syntax that you like, we can code it up. 
Options include:

CryptSHA512:5500 CryptSHA256

or probably better:

CryptSHA512:rounds=5500 CryptSHA256

I'm not sure how to fit those in to the attributes for the 'samba-tool
user getpassword' case, but perhaps you have clearer ideas.

> What about using the names virtualSSHA, virtualCryptSHA256,
> > > virtualCryptSHA512
> 
> I've used CryptSHA256 and CryptSHA512 for now, matching these but
> without the virtual bit (which makes less sense in this context).  If
> we ever do SSHA we can decide if we want {SSHA} or so then.
> 
> > > Sorry, but we really have to get this sane before it can be
> > > pushed.
> 
> Thanks for the pushback.  Please do have a look over the patch set,
> and
> if you are OK with this, then we can proceed to review, but otherwise
> lets work out the final kinks at SambaXP. 
> 
> Hopefully that will be much less frustrating, and a much more natural
> extension of the work you did for SambaGPG.
> 
> Finally, I'm really sorry for rushing this with my basic facts so
> poorly researched.  
> 
> 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: 862 bytes
Desc: This is a digitally signed message part
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20170412/6977210d/signature.sig>