classification
Title: Add hashlib.scrypt
Type: enhancement Stage: patch review
Components: Versions: Python 3.6, Python 2.7
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: alex, benjamin.peterson, christian.heimes, gregory.p.smith, python-dev, xiang.zhang
Priority: normal Keywords: patch

Created on 2016-09-01 12:29 by christian.heimes, last changed 2016-09-07 17:01 by benjamin.peterson.

Files
File name Uploaded Description Edit
Add-hashlib.scrypt-4.patch christian.heimes, 2016-09-04 15:16
hashlib.scrypt.patch christian.heimes, 2016-09-05 22:24 review
Messages (18)
msg274118 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2016-09-01 12:29
OpenSSL 1.1 has EVP_PBE_scrypt(). hashlib.scrypt() is a low-hanging fruit for Python 3.6. I have a working patch with some tests. I need to write more tests and documentation:

https://github.com/tiran/cpython/commits/feature/openssl110_scrypt
msg274175 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2016-09-01 19:44
Rather than PyArg_ParseTupleAndKeywords can you have it use argument clinic?

Also, how about making all arguments other than password be keyword only so
that code calling the function is more clear.  Otherwise it's a bit of
positional argument soup with a lot of integers and potential to invert
password and salt without realizing it.
msg274180 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2016-09-01 20:27
Argument is easy.

Your second request is a very good idea but also harder to implement. Neither PyArg_Parse nor clinic have a way to declare arguments that required and keyword only but have no default value. I have a workaround but it ain't beautiful.
msg274181 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2016-09-01 20:32
If clinic doesn't support required keyword only args then don't worry about it for now. :)
msg274217 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2016-09-02 09:07
Here is a new patch with argument clinic, more tests and required keyword arguments.
msg274225 - (view) Author: Xiang Zhang (xiang.zhang) * (Python committer) Date: 2016-09-02 10:41
It looks that new patch when used like this hashlib.scrypt(b'password') will generate a "an integer is required" exception message which is misleading. I don't test it since I don't get openssl 1.1.

And the phrase "interpreted as buffers of bytes" in the doc may better be "bytes-like objects".
msg274253 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2016-09-02 16:30
You are right. Let's try this again. How do you like:

>>> hashlib.scrypt(b'', n=2, r=2, p=3)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: salt is required
>>> hashlib.scrypt(b'', salt=b'')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: n is required and must be an unsigned int
>>> hashlib.scrypt(b'', n=None, r=2, p=3)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: scrypt() argument 3 must be int, not None
msg274258 - (view) Author: Xiang Zhang (xiang.zhang) * (Python committer) Date: 2016-09-02 16:53
It looks good. But Christian, may I ask how do you generate the argument clinic? It looks from me that the declaration cannot give you such a format "y*|$y*O!O!O!ll:scrypt". I rerun clinic.py and the .c.h file is altered. Maybe it's better to abandon AC for right now?
msg274271 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2016-09-02 20:26
It's not a limitation of the argument clinic. PyArg_Parse*() does not support required, keyword-only arguments without a default value. I'm using None as default value, require PyLong_Type and added some extra checks.
msg274281 - (view) Author: Alex Gaynor (alex) * (Python committer) Date: 2016-09-02 23:57
Bug in the error message "n must be a multiple of 2." it should say "n must be a power of 2."
msg274358 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2016-09-04 11:45
Thanks Alex, multiple is the wrong term. The argument 'n' must be 2^m for m > 1.
msg274589 - (view) Author: Roundup Robot (python-dev) Date: 2016-09-06 18:23
New changeset d926fa1a833c by Christian Heimes in branch 'default':
Issue #27928: Add scrypt (password-based key derivation function) to hashlib module (requires OpenSSL 1.1.0).
https://hg.python.org/cpython/rev/d926fa1a833c
msg274788 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2016-09-07 10:25
Benjamin, what's your take on Alex's suggestion?

<Crys> gutworth: Alex_Gaynor has asked me if hashlib.scrypt() can go into 2.7, too. It's a password-based KDF like hashlib.pbkdf2() but more secure than PBKDF2. It requires OpenSSL 1.1.0.
<Alex_Gaynor> gutworth: I think it'd be good if this were approved, for the same reasons as PEP466
<Crys> contrary to PKBDF2 it doesn't make sense to have a pure-Python implementation. scrypt uses ChaCha20 cipher. I don't want to add a cipher to CPython core (possible legal issue) and it's not available in OpenSSL < 1.1.0.
msg274820 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2016-09-07 15:22
No, scrypt is a simple new feature.  An extension module on PyPI is the appropriate place for that for 2.6 through 3.5.  Wholly unrelated to PEP466.
msg274824 - (view) Author: Alex Gaynor (alex) * (Python committer) Date: 2016-09-07 15:49
PEP466 includes hashlib.pbkdf2_hmac(). Any reasoning that includes that surely is applicable to scrypt as well.
msg274830 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2016-09-07 16:28
Why are we adding scrypt and not argon2 anyway?

On Wed, Sep 7, 2016, at 03:25, Christian Heimes wrote:
> 
> Christian Heimes added the comment:
> 
> Benjamin, what's your take on Alex's suggestion?
> 
> <Crys> gutworth: Alex_Gaynor has asked me if hashlib.scrypt() can go into
> 2.7, too. It's a password-based KDF like hashlib.pbkdf2() but more secure
> than PBKDF2. It requires OpenSSL 1.1.0.
> <Alex_Gaynor> gutworth: I think it'd be good if this were approved, for
> the same reasons as PEP466
> <Crys> contrary to PKBDF2 it doesn't make sense to have a pure-Python
> implementation. scrypt uses ChaCha20 cipher. I don't want to add a cipher
> to CPython core (possible legal issue) and it's not available in OpenSSL
> < 1.1.0.
> 
> ----------
> nosy: +benjamin.peterson
> versions: +Python 2.7
> 
> _______________________________________
> Python tracker <report@bugs.python.org>
> <http://bugs.python.org/issue27928>
> _______________________________________
msg274833 - (view) Author: Alex Gaynor (alex) * (Python committer) Date: 2016-09-07 16:44
OpenSSL supports scrypt

On Sep 7, 2016 12:28 PM, "Benjamin Peterson" <report@bugs.python.org> wrote:

>
> Benjamin Peterson added the comment:
>
> Why are we adding scrypt and not argon2 anyway?
>
> On Wed, Sep 7, 2016, at 03:25, Christian Heimes wrote:
> >
> > Christian Heimes added the comment:
> >
> > Benjamin, what's your take on Alex's suggestion?
> >
> > <Crys> gutworth: Alex_Gaynor has asked me if hashlib.scrypt() can go into
> > 2.7, too. It's a password-based KDF like hashlib.pbkdf2() but more secure
> > than PBKDF2. It requires OpenSSL 1.1.0.
> > <Alex_Gaynor> gutworth: I think it'd be good if this were approved, for
> > the same reasons as PEP466
> > <Crys> contrary to PKBDF2 it doesn't make sense to have a pure-Python
> > implementation. scrypt uses ChaCha20 cipher. I don't want to add a cipher
> > to CPython core (possible legal issue) and it's not available in OpenSSL
> > < 1.1.0.
> >
> > ----------
> > nosy: +benjamin.peterson
> > versions: +Python 2.7
> >
> > _______________________________________
> > Python tracker <report@bugs.python.org>
> > <http://bugs.python.org/issue27928>
> > _______________________________________
>
> ----------
>
> _______________________________________
> Python tracker <report@bugs.python.org>
> <http://bugs.python.org/issue27928>
> _______________________________________
>
msg274838 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2016-09-07 17:01
PEP 466 is explicitly not blanket approval for backporting All The
Things to 2.7. The only justification for pbkdf2 in PEP 466 is to "lower
the barriers to secure password storage and checking in Python 2 server
applications". While scrypt is probably a bit better, applications using
pkbdf2 are still in a much better situation than ones using, e.g., a
naïve salted hash.

There is a self-contained, easily-installable scrypt module on PyPI.
History
Date User Action Args
2016-09-07 17:01:57benjamin.petersonsetmessages: + msg274838
2016-09-07 16:44:08alexsetmessages: + msg274833
2016-09-07 16:28:56benjamin.petersonsetmessages: + msg274830
2016-09-07 15:49:44alexsetmessages: + msg274824
2016-09-07 15:22:25gregory.p.smithsetmessages: + msg274820
2016-09-07 10:25:18christian.heimessetnosy: + benjamin.peterson

messages: + msg274788
versions: + Python 2.7
2016-09-06 18:23:59python-devsetnosy: + python-dev
messages: + msg274589
2016-09-05 22:24:39christian.heimessetfiles: + hashlib.scrypt.patch
2016-09-05 22:09:25christian.heimessetfiles: - Add-hashlib.scrypt-3.patch
2016-09-05 22:09:18christian.heimessetfiles: - Add-hashlib.scrypt-2.patch
2016-09-05 22:09:13christian.heimessetfiles: - Add-hashlib.scrypt.patch
2016-09-04 15:16:49christian.heimessetfiles: + Add-hashlib.scrypt-4.patch
2016-09-04 11:45:33christian.heimessetmessages: + msg274358
2016-09-02 23:57:33alexsetnosy: + alex
messages: + msg274281
2016-09-02 20:26:48christian.heimessetfiles: + Add-hashlib.scrypt-3.patch

messages: + msg274271
2016-09-02 16:53:03xiang.zhangsetmessages: + msg274258
2016-09-02 16:30:15christian.heimessetmessages: + msg274253
2016-09-02 10:41:29xiang.zhangsetnosy: + xiang.zhang
messages: + msg274225
2016-09-02 09:07:36christian.heimessetfiles: + Add-hashlib.scrypt-2.patch

messages: + msg274217
2016-09-01 20:32:39gregory.p.smithsetmessages: + msg274181
2016-09-01 20:27:50christian.heimessetmessages: + msg274180
2016-09-01 19:44:06gregory.p.smithsetmessages: + msg274175
2016-09-01 16:18:21christian.heimessetfiles: + Add-hashlib.scrypt.patch
keywords: + patch
2016-09-01 12:29:19christian.heimescreate