This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: test_hashlib.test_scrypt() fails on Fedora 29
Type: Stage: resolved
Components: SSL Versions: Python 3.8, Python 3.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: christian.heimes Nosy List: christian.heimes, vstinner
Priority: normal Keywords: patch

Created on 2019-03-11 15:15 by vstinner, last changed 2022-04-11 14:59 by admin. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 12280 closed vstinner, 2019-03-11 15:56
Messages (9)
msg337674 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-03-11 15:15
$ ./python -m test -v test_hashlib -m test_scrypt
...
ERROR: test_scrypt (test.test_hashlib.KDFTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/vstinner/prog/python/master/Lib/test/test_hashlib.py", line 941, in test_scrypt
    result = hashlib.scrypt(password, salt=salt, n=n, r=r, p=p)
ValueError: Invalid parameter combination for n, r, p, maxmem.
...

The patch pass with the following patch:
---
diff --git a/Modules/_hashopenssl.c b/Modules/_hashopenssl.c
index e560c18b63..22682ea86d 100644
--- a/Modules/_hashopenssl.c
+++ b/Modules/_hashopenssl.c
@@ -770,6 +770,7 @@ _hashlib_scrypt_impl(PyObject *module, Py_buffer *password, Py_buffer *salt,
         return NULL;
     }
 
+#if 0
     /* let OpenSSL validate the rest */
     retval = EVP_PBE_scrypt(NULL, 0, NULL, 0, n, r, p, maxmem, NULL, 0);
     if (!retval) {
@@ -778,6 +779,7 @@ _hashlib_scrypt_impl(PyObject *module, Py_buffer *password, Py_buffer *salt,
                         "Invalid parameter combination for n, r, p, maxmem.");
         return NULL;
    }
+#endif
 
     key_obj = PyBytes_FromStringAndSize(NULL, dklen);
     if (key_obj == NULL) {
---

$ ./python -m test.pythoninfo|grep -E '^ssl|libc_ver'
platform.libc_ver: glibc 2.28
ssl.HAS_SNI: True
ssl.OPENSSL_VERSION: OpenSSL 1.1.1b FIPS  26 Feb 2019
ssl.OPENSSL_VERSION_INFO: (1, 1, 1, 2, 15)
ssl.OP_ALL: 0x80000054
ssl.OP_NO_TLSv1_1: 0x10000000

Note: I don't think that libxcrypt is related, but just in case:

$ rpm -q libxcrypt
libxcrypt-4.4.4-1.fc29.x86_64
libxcrypt-4.4.4-1.fc29.i686
vstinner@apu$ 

Note 2: It seems like bpo-27928 "Add hashlib.scrypt" is still open whereas it has been implemented in Python 3.6.
msg337681 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-03-11 15:56
Python 3.7 is also affected, but not Python 2.7.
msg337682 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-03-11 16:03
I tried but failed to identify when test_hashlib started to fail whereas it was fine previously. IMHO it can only be a changed in OpenSSL.

It might be a different between Fedora packages openssl-1.1.1b-1.fc29 (built at 2019-02-28) and openssl-1.1.1b-2.fc29 (built at 2019-03-01). 

Other older packages:
* openssl-1.1.1a-1.fc29 (2019-01-15)
* openssl-1.1.1-3.fc29 (2018-09-17)
msg337684 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-03-11 16:18
Attached PR 12280 fix the issue: the salt wasn't passed to EVP_PBE_scrypt() and so the function fails with "missing salt".
msg337685 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-03-11 16:29
Oh, the Fedora package of OpenSSL 1.1.1b includes this downstream patch:

https://src.fedoraproject.org/rpms/openssl/blob/master/f/openssl-1.1.1-evp-kdf.patch

Extract of the changelog:

* Thu Feb 28 2019 Tomáš Mráz <tmraz@redhat.com> 1.1.1b-1
- update to the 1.1.1b release
- EVP_KDF API backport from master
- SSH KDF implementation for EVP_KDF API backport from master

The patch changes the behavior for (salt=NULL, saltlen=0). Previously, it was handled as (salt="", saltlen=0), but now the function fails with "missing salt".

The patch has code to handle (pass=NULL, passlen=any value) as (pass="", passlen=0):

https://src.fedoraproject.org/rpms/openssl/blob/master/f/openssl-1.1.1-evp-kdf.patch#_705

+    /* Maintain existing behaviour. */
+    if (pass == NULL) {
+        pass = empty;
+        passlen = 0;
     }
msg337848 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-03-13 13:20
> Oh, the Fedora package of OpenSSL 1.1.1b includes this downstream patch (...) The patch changes the behavior for (salt=NULL, saltlen=0) (...)

I reported the issue to OpenSSL in Fedora:
https://bugzilla.redhat.com/show_bug.cgi?id=1688284
msg337919 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-03-14 14:34
I wrote a PR to fix OpenSSL:
https://github.com/openssl/openssl/pull/8483
msg338381 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-03-19 16:39
Update: my OpenSSL PR https://github.com/openssl/openssl/pull/8483 has been merged and new a new OpenSSL package for Fedora is being tested: https://bugzilla.redhat.com/show_bug.cgi?id=1688284
msg340353 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-04-16 16:17
OpenSSL regression has been fixed in Fedora:
https://bugzilla.redhat.com/show_bug.cgi?id=1688284
History
Date User Action Args
2022-04-11 14:59:12adminsetgithub: 80444
2019-04-16 16:17:01vstinnersetstatus: open -> closed
resolution: fixed
messages: + msg340353

stage: patch review -> resolved
2019-03-19 16:39:09vstinnersetmessages: + msg338381
2019-03-14 14:34:50vstinnersetmessages: + msg337919
2019-03-13 13:20:54vstinnersetmessages: + msg337848
2019-03-11 16:29:19vstinnersetmessages: + msg337685
2019-03-11 16:18:29vstinnersetmessages: + msg337684
2019-03-11 16:03:14vstinnersetmessages: + msg337682
2019-03-11 15:56:57vstinnersetkeywords: + patch
stage: patch review
pull_requests: + pull_request12260
2019-03-11 15:56:05vstinnersetmessages: + msg337681
versions: + Python 3.7
2019-03-11 15:15:11vstinnercreate