{PATCH] store extra password hashes in supplemental credentials

[Andrew Bartlett]

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 

> > 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/e6303c6c/signature.sig>