Bug 1688284

Summary: Regression of OpenSSL 1.1.1b-1 in EVP_PBE_scrypt() with salt=NULL
Product: [Fedora] Fedora Reporter: Victor Stinner <vstinner>
Component: opensslAssignee: Tomas Mraz <tmraz>
Status: CLOSED ERRATA QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: unspecified Docs Contact:
Priority: unspecified    
Version: rawhideCC: hkario, jorton, mhroncok, szidek, tmraz
Target Milestone: ---   
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: openssl-1.1.1b-3.fc29 openssl-1.1.1b-3.fc30 Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2019-03-21 14:40:59 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On:    
Bug Blocks: 1688546    

Description Victor Stinner 2019-03-13 13:10:44 UTC
Hi,

I'm working on the Python project and noticed a regression in Python
test_hashlib. The scrypt() function started to fail. First I read the
code upstream to notice a recent change in the 1.1.1 branch, but I
didn't see anything recent. After long time in gdb, I understood that
Fedora has downstream patches. Aaaah!

There is a downstream patch "openssl-1.1.1-evp-kdf.patch". I guess
that it's justified by "EVP_KDF API backport from master" of 1.1.1b-1
changelog entry (of the .spec file). By the way, it would help to add
a short comment to justify why we have such downstream changes ;-)

The patch contains code for backward compatibility with pass=NULL, but
not for salt=NULL:

+    /* Maintain existing behaviour. */
+    if (pass == NULL) {
+        pass = empty;
+        passlen = 0;
     }

Would it be possible to keep the backward compatibility by adding
similar code for salt=NULL?

For more info, see the Python bug report:
https://bugs.python.org/issue36263#msg337685

Python calls the function twice. The first call is to validate most
parameters except pass and salt:
---
    /* let OpenSSL validate the rest */
    retval = EVP_PBE_scrypt(NULL, 0, NULL, 0, n, r, p, maxmem, NULL, 0);
    if (!retval) {
        /* sorry, can't do much better */
        PyErr_SetString(PyExc_ValueError,
                        "Invalid parameter combination for n, r, p, maxmem.");
        return NULL;
    }
    ...

    Py_BEGIN_ALLOW_THREADS
    retval = EVP_PBE_scrypt(
        (const char*)password->buf, (size_t)password->len,
        (const unsigned char *)salt->buf, (size_t)salt->len,
        n, r, p, maxmem,
        (unsigned char *)key, (size_t)dklen
    );
    Py_END_ALLOW_THREADS
---

Victor

Comment 1 Victor Stinner 2019-03-13 13:13:19 UTC
The issue can be seen in Python 3 using command:

$ python3 -c 'import hashlib; print(repr(hashlib.scrypt(b"password", salt=b"salt", n=2, r=1, p=1)))'

Current behavior:

$ python3 -c 'import hashlib; print(repr(hashlib.scrypt(b"password", salt=b"salt", n=2, r=1, p=1)))'
Traceback (most recent call last):
  File "<string>", line 1, in <module>
ValueError: Invalid paramemter combination for n, r, p, maxmem.

Expected result:

$ ./python -c 'import hashlib; print(repr(hashlib.scrypt(b"password", salt=b"salt", n=2, r=1, p=1)))'
b'm\x1b\xb8x\xee\xe9\xceJ{w\xd7\xa4A\x03WML\xbf\xe3\xc1Z\xe3\x94\x0f\x0f\xfeu\xcd^\x1e\n\xfa\xdb\nUkH+]\xcb\r\x1aT\xb6\xe4\x07\x0b\xeb\x0c\x04\xbc\xdb\xee\xbfm0\x03\xf0g\x0cW\x1b\xf1\xc2'

Comment 5 Victor Stinner 2019-03-18 09:19:35 UTC
Update: I wrote an OpenSSL fix which has been merged upstream: https://github.com/openssl/openssl/pull/8483

Comment 6 Miro Hrončok 2019-03-18 09:25:21 UTC
Good. Can we have it backported in Fedora please?

Comment 7 Fedora Update System 2019-03-18 11:04:22 UTC
openssl-1.1.1b-3.fc29 has been submitted as an update to Fedora 29. https://bodhi.fedoraproject.org/updates/FEDORA-2019-74db223aa3

Comment 8 Fedora Update System 2019-03-18 11:04:33 UTC
openssl-1.1.1b-3.fc30 has been submitted as an update to Fedora 30. https://bodhi.fedoraproject.org/updates/FEDORA-2019-71c4c8b47e

Comment 9 Tomas Mraz 2019-03-18 11:05:33 UTC
I've already built the fixed package on Friday but did not manage to create the update.

Comment 10 Fedora Update System 2019-03-19 04:44:12 UTC
openssl-1.1.1b-3.fc29 has been pushed to the Fedora 29 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2019-74db223aa3

Comment 12 Fedora Update System 2019-03-21 14:40:59 UTC
openssl-1.1.1b-3.fc29 has been pushed to the Fedora 29 stable repository. If problems still persist, please make note of it in this bug report.

Comment 13 Fedora Update System 2019-03-21 19:10:29 UTC
openssl-1.1.1b-3.fc30 has been pushed to the Fedora 30 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2019-71c4c8b47e

Comment 14 Fedora Update System 2019-03-29 19:17:34 UTC
openssl-1.1.1b-3.fc30 has been pushed to the Fedora 30 stable repository. If problems still persist, please make note of it in this bug report.