Review Request 115497: Replace SHA with PBKDF2-SHA512+Salt
Michael Pyne
mpyne at kde.org
Fri Feb 7 05:14:36 GMT 2014
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/115497/#review49163
-----------------------------------------------------------
A couple of minor things but definitely looks a lot better. Hopefully the kwallet integration review can be done soon by someone knowledgeable with kwallet but I'd run the code myself at this point. ;)
kwalletd/backend/kwalletbackend.cc
<https://git.reviewboard.kde.org/r/115497/#comment34719>
I didn't see specifics in libgcrypt's documentation but surely this memory needs to be freed (I'm assuming by the stdlib ::free()) after the QByteArray is constructed.
kwalletd/backend/kwalletbackend.cc
<https://git.reviewboard.kde.org/r/115497/#comment34720>
Again, might want to add error-checking here. If the salt can't be saved for whatever reason then we don't want to destroy an existing old-style wallet by mistake.
It looks like it would be as simple as returning an empty QByteArray if an error is detected.
- Michael Pyne
On Feb. 6, 2014, 3:28 p.m., Àlex Fiestas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/115497/
> -----------------------------------------------------------
>
> (Updated Feb. 6, 2014, 3:28 p.m.)
>
>
> Review request for KDE Runtime, Teo Mrnjavac and Valentin Rusu.
>
>
> Repository: kde-runtime
>
>
> Description
> -------
>
> Uses the MINOR_VERSION (which until now it was 0) to upgrade the hash from SHA to PBKDF2-SHA512+salt.
> I would have loved to completely replace it once the wallet is ported to the new hashing but because
> of kwalletd code that is not possible without a bigger rewrite.
>
> There are 2 reasons for this patch:
> 1-We avoid using our own implementation of SHA
> 2-We use a modern hashing technique
>
> I'm cooking more patches to use the system user password to open the wallet, we want that password to be
> hashed using PBKDF2_SHA512 for security reasons.
>
>
> Diffs
> -----
>
> CMakeLists.txt 275a6c7
> cmake/modules/FindLibGcrypt.cmake PRE-CREATION
> kwalletd/backend/CMakeLists.txt 5a5837c
> kwalletd/backend/backendpersisthandler.cpp bdef6ca
> kwalletd/backend/kwalletbackend.h 83ebf7f
> kwalletd/backend/kwalletbackend.cc e4d461c
>
> Diff: https://git.reviewboard.kde.org/r/115497/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Àlex Fiestas
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20140207/780d2204/attachment.htm>
More information about the kde-core-devel
mailing list