test_hashlib.test_scrypt() fails on Fedora 29

[vstinner]

BPO	36263
Nosy	@vstinner, @tiran
PRs	[WIP] bpo-36263: Fix hashlib.scrypt() #12280

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/tiran'
closed_at = <Date 2019-04-16.16:17:01.650>
created_at = <Date 2019-03-11.15:15:11.043>
labels = ['expert-SSL', '3.7', '3.8']
title = 'test_hashlib.test_scrypt() fails on Fedora 29'
updated_at = <Date 2019-04-16.16:17:01.649>
user = 'https://github.com/vstinner'

bugs.python.org fields:

activity = <Date 2019-04-16.16:17:01.649>
actor = 'vstinner'
assignee = 'christian.heimes'
closed = True
closed_date = <Date 2019-04-16.16:17:01.650>
closer = 'vstinner'
components = ['SSL']
creation = <Date 2019-03-11.15:15:11.043>
creator = 'vstinner'
dependencies = []
files = []
hgrepos = []
issue_num = 36263
keywords = ['patch']
message_count = 9.0
messages = ['337674', '337681', '337682', '337684', '337685', '337848', '337919', '338381', '340353']
nosy_count = 2.0
nosy_names = ['vstinner', 'christian.heimes']
pr_nums = ['12280']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue36263'
versions = ['Python 3.7', 'Python 3.8']

[vstinner]

$ ./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.

[vstinner]

Python 3.7 is also affected, but not Python 2.7.

[vstinner]

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)

[vstinner]

Attached PR 12280 fix the issue: the salt wasn't passed to EVP_PBE_scrypt() and so the function fails with "missing salt".

[vstinner]

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;
}

[vstinner]

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

[vstinner]

I wrote a PR to fix OpenSSL:
openssl/openssl#8483

[vstinner]

Update: my OpenSSL PR openssl/openssl#8483 has been merged and new a new OpenSSL package for Fedora is being tested: https://bugzilla.redhat.com/show_bug.cgi?id=1688284

[vstinner]

OpenSSL regression has been fixed in Fedora:
https://bugzilla.redhat.com/show_bug.cgi?id=1688284