Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

[2.1] Added the scrypt key derivation algorithm in Zend\Crypt #2430

Closed
wants to merge 3 commits into
from

Conversation

Projects
None yet
5 participants
Owner

ezimuel commented Sep 27, 2012

Added the scrypt algorithm in Zend\Crypt\Key\Derivation.
The scrypt is designed to be more secure against hardware brute-force attacks than alternative functions such as PBKDF2 or bcrypt.
For more information: http://www.tarsnap.com/scrypt.html

@blanchonvincent blanchonvincent and 2 others commented on an outdated diff Sep 27, 2012

library/Zend/Crypt/Key/Derivation/Scrypt.php
+ * @package Zend_Crypt
+ */
+
+namespace Zend\Crypt\Key\Derivation;
+
+use Zend\Crypt\Key\Derivation\Pbkdf2;
+
+/**
+ * Scrypt key derivation function
+ *
+ * @category Zend
+ * @package Zend_Crypt
+ * @see http://www.tarsnap.com/scrypt.html
+ * @see https://tools.ietf.org/html/draft-josefsson-scrypt-kdf-01
+ */
+class Scrypt
@blanchonvincent

blanchonvincent Sep 27, 2012

Contributor

Class with only static methods should mark as "abstract" no ?

@ezimuel

ezimuel Sep 27, 2012

Owner

Why? I don't like the idea to use abstract here. Abstract is ment to define an class that implements some functionality, but needs more behaviour (added via inheritance) to function properly. If you want to prevent the possibility to instantiate the class you should protect the constructor, but this is not the case. I don't want to prevent evolution of this component.

@weierophinney

weierophinney Sep 27, 2012

Owner

@blanchonvincent I've discussed this with @ezimuel today, and pointed out that it has become a common practice in ZF2 to mark static classes as abstract, but not to use the "Abstract" prefix; this prevents having dead code (you don't need to have an empty constructor), and the lack of prefix indicates it can be consumed directly without need of extension. It also leaves the door open for extension later (as moving from abstract to concrete is not a BC break).

@padraic padraic was assigned Oct 1, 2012

Owner

weierophinney commented Oct 1, 2012

Assigning @padraic to review, as he likely has a better idea of correctness here.

Has this userland implementation of scrypt been reviewed for security and fully tested?

@weierophinney weierophinney added a commit to zendframework/zend-crypt that referenced this pull request May 15, 2015

@weierophinney weierophinney Merge branch 'feature/scrypt' into develop af0e743
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment