Bug 1745499 - mod_wsgi: hashlib.get_fips_mode() fails with ValueError: [PEM routines: get_name] no start line
Summary: mod_wsgi: hashlib.get_fips_mode() fails with ValueError: [PEM routines: get_n...
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Red Hat Enterprise Linux 8
Classification: Red Hat
Component: python3
Version: 8.1
Hardware: Unspecified
OS: Unspecified
unspecified
urgent
Target Milestone: rc
: 8.1
Assignee: Python Maintainers
QA Contact: RHEL CS Apps Subsystem QE
URL:
Whiteboard:
: 1744614 (view as bug list)
Depends On:
Blocks: 1738779 1744614
TreeView+ depends on / blocked
 
Reported: 2019-08-26 09:00 UTC by Christian Heimes
Modified: 2019-11-05 22:04 UTC (History)
9 users (show)

Fixed In Version: python3-3.6.8-15.el8
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2019-11-05 22:03:44 UTC
Type: Bug
Target Upstream Version:


Attachments (Terms of Use)


Links
System ID Priority Status Summary Last Updated
Red Hat Product Errata RHSA-2019:3520 None None None 2019-11-05 22:04:04 UTC

Internal Links: 1760106

Description Christian Heimes 2019-08-26 09:00:06 UTC
Description of problem:
IPA's WSGI script /usr/share/ipa/wsgi.py fails to start with error message "ValueError: [PEM routines: get_name] no start line". The error message is coming from a call to get_fips_mode().


Version-Release number of selected component (if applicable):
python3-libs-3.6.8-14.el8.x86_64
ipa-server-4.8.0-9.module+el8.1.0+4011+fd4be199.x86_64
python3-mod_wsgi-4.6.4-3.el8.x86_64

How reproducible:
Always

Steps to Reproduce:
1. install IPA with ipa-server-install
2. check /var/log/httpd/error_log

Actual results:
# less /var/log/httpd/error_log
...
mod_wsgi (pid=14326): Failed to exec Python script file '/usr/share/ipa/wsgi.py'.
mod_wsgi (pid=14326): Exception occurred processing WSGI script '/usr/share/ipa/wsgi.py'.
Traceback (most recent call last):
  File "/usr/share/ipa/wsgi.py", line 44, in <module>
    from ipalib import api
  File "/usr/lib/python3.6/site-packages/ipalib/__init__.py", line 919, in <module>
    from ipalib import plugable
  File "/usr/lib/python3.6/site-packages/ipalib/plugable.py", line 41, in <module>
    from ipalib import errors
  File "/usr/lib/python3.6/site-packages/ipalib/errors.py", line 109, in <module>
    from ipalib.text import ngettext as ungettext
  File "/usr/lib/python3.6/site-packages/ipalib/text.py", line 139, in <module>
    from ipalib.request import context
  File "/usr/lib/python3.6/site-packages/ipalib/request.py", line 28, in <module>
    from ipalib.base import ReadOnly, lock
  File "/usr/lib/python3.6/site-packages/ipalib/base.py", line 26, in <module>
    from ipalib.constants import NAME_REGEX, NAME_ERROR
  File "/usr/lib/python3.6/site-packages/ipalib/constants.py", line 27, in <module>
    from ipapython.dn import DN
  File "/usr/lib/python3.6/site-packages/ipapython/dn.py", line 425, in <module>
    import cryptography.x509
  File "/usr/lib64/python3.6/site-packages/cryptography/x509/__init__.py", line 8, in <module>
    from cryptography.x509.base import (
  File "/usr/lib64/python3.6/site-packages/cryptography/x509/base.py", line 16, in <module>
    from cryptography.x509.extensions import Extension, ExtensionType
  File "/usr/lib64/python3.6/site-packages/cryptography/x509/extensions.py", line 9, in <module>
    import hashlib
  File "/usr/lib64/python3.6/hashlib.py", line 77, in <module>
    if not get_fips_mode():
ValueError: [PEM routines: get_name] no start line
mod_wsgi (pid=14325): Failed to exec Python script file '/usr/share/ipa/wsgi.py'.


Expected results:
No error

Additional info:
The problem looks like https://github.com/pyca/cryptography/issues/3368 again. The issue is probably caused by mod_ssl not cleanup up behind itself properly. It leaves an error message on OpenSSL's error stack. A library call from a different application in the same process space stumbles over the error on the error stack.

I suggest that get_fips_mode() clears the error stack before it executes any code.

Comment 6 Petr Viktorin 2019-08-26 11:43:30 UTC
Wait, are you really saying that hashlib should clear and ignore all errors?  How would that be a safe thing to do?

If we can't guarantee that mod_ssl would clean up the stack in all cases, should Python guarantee that it works with an error on the stack in all cases?
Are the expectations for OpenSSL API usage documented anywhere?

Comment 7 Christian Heimes 2019-08-26 12:25:07 UTC
No, I meant something different. get_fips_mode() should ignore and clear all *existing* errors on the error stack. Of course it should not ignore any error that is raised by any calls inside the function or any other hashlib function.

The general expectation is that application code should check all OpenSSL function results for error and then handle any error appropriately. An application may either decide to ignore an error or to report an error. In either case the application should deal with the error stack correctly. As common courtesy applications should clean up after themselves. The recommendation is not written down in the OpenSSL documentation or wiki.

It might be sufficient to clear the error stack when _ssl or _hashlib are imported.

Since it's not the first time mod_ssl is causing trouble, perhaps it's a good idea to get back to mod_ssl upstream and ask upstream to review the code?

Comment 8 Petr Viktorin 2019-08-26 15:34:25 UTC
> get_fips_mode() should ignore and clear all *existing* errors on the error stack

Should it? I'm not convinced; clearing and ignoring any error sounds very shaky. Could you point me to some upstream documentation/discussion saying this should be done?
I believe OpenSSL's own SSL_* code is, in this regard, in a very similar situation as Python's hashlib. The docs of SSL_get_error say that "The current thread's error queue must be empty before the TLS/SSL I/O operation is attempted".


Or I can add a workaround, but let's treat it as a workaround: please open issue(s) to clarify the intended usage and/or track the bug.

Comment 9 Christian Heimes 2019-08-26 16:04:18 UTC
The get_fips_mode() function from the downstream patch looks like this:

static PyObject *
_hashlib_get_fips_mode_impl(PyObject *module)
/*[clinic end generated code: output=ad8a7793310d3f98 input=f42a2135df2a5e11]*/
{
    int result = FIPS_mode();
    if (result == 0) {
        // "If the library was built without support of the FIPS Object Module,
        // then the function will return 0 with an error code of
        // CRYPTO_R_FIPS_MODE_NOT_SUPPORTED (0x0f06d065)."
        // But 0 is also a valid result value.

        unsigned long errcode = ERR_peek_last_error();
        if (errcode) {
            _setException(PyExc_ValueError);
            return NULL;
        }
    }
    return PyLong_FromLong(result);
}

Since the function uses the current error stack to determinate if result == 0 is an error or a valid result, it has to clear the error stack. This is the same design principal as using errno to figure out if a function call fails, because library function never set errno to 0. The fact is not explicitly written down in the documentation for libcrypto's error handling. Perhaps the documentation should be improved. The rest of _hashopenssl.c is not affected by the problem, because _setException() is only called whenever a libcrypto function call returns an error indicator.

There are two ways to address the issue:

1) Raise the issue as blocker+ with mod_ssl and have mod_ssl address all places that can leak an OpenSSL error on the error stack.
2) Change the downstream get_fips_mode() function to clear the error stack before it calls FIPS_mode().

Solution (2) is a mix of a workaround for a mod_ssl problem and following common practices for error handling. IMHO it's more best practices than workaround. It's definitely easier than reviewing the entire code base of mod_ssl.

Comment 10 Petr Viktorin 2019-08-26 17:54:21 UTC
Ah! Makes sense now. I'll clear the stack.
Thank you for your patience!

Comment 11 Christian Heimes 2019-08-26 17:59:11 UTC
Thanks for being thorough! :)

I didn't explain the issue properly at first.

Comment 12 Petr Viktorin 2019-08-27 12:10:44 UTC
It seems appropriate to use ERR_print_errors_cb, and give a RuntimeWarning for each error. Would that work for you?

Comment 13 Christian Heimes 2019-08-27 13:51:31 UTC
The use of ERR_print_errors_cb may change behavior of applications. I cannot foresee consequences and ill side effects of the callback.

I'm also worried that a RuntimeWarning in Apache HTTPd's error log is going to confuse customers and create additional support tickets. A customer or application developer cannot get rid of the root cause. At best a warning is going to annoy engineers and remind them to file bugs against mod_ssl.

I would just call ERR_clear_error() and maybe file a bug against mod_ssl.

{
    int result;

    /* mod_ssl leaves error on the OpenSSL error stack. Clear the error stack and ignore
     * left over errors, so we can use ERR_peek_last_error() to detect error condition,
     * see RHBZ#1745499. */
    ERR_clear_error();

    result = FIPS_mode();
    if (result == 0) {
        // "If the library was built without support of the FIPS Object Module,
        // then the function will return 0 with an error code of
        // CRYPTO_R_FIPS_MODE_NOT_SUPPORTED (0x0f06d065)."
        // But 0 is also a valid result value.

        unsigned long errcode = ERR_peek_last_error();
        if (errcode) {
            _setException(PyExc_ValueError);
            return NULL;
        }
    }
    return PyLong_FromLong(result);
}

Comment 14 Petr Viktorin 2019-08-28 07:40:04 UTC
> The use of ERR_print_errors_cb may change behavior of applications. I cannot foresee consequences and ill side effects of the callback.

I don't understand. How does this change behavior of applications?
According to the documentation, it calls the callback for each error on the stack, and clears the stack. It shouldn't change global state, apart from clearing the stack.

> I'm also worried that a RuntimeWarning in Apache HTTPd's error log is going to confuse customers and create additional support tickets. A customer or application developer cannot get rid of the root cause. At best a warning is going to annoy engineers and remind them to file bugs against mod_ssl.

But I'm changing hashlib for all users, not just FreeIPA. I'm quite worried about clearing and forgetting unknown errors.
Customers can't get rid of *this* root cause, but this can also hide any other causes in other applications.

Comment 15 Petr Viktorin 2019-08-28 13:44:36 UTC
Or, since this is a RHEL patch and I know we have the FIPS Object Module, I can just skip the error checking entirely:

{
    return PyLong_FromLong(FIPS_mode());
}

That wouldn't be acceptable for upstream, of course.

Comment 17 Christian Heimes 2019-08-28 21:16:07 UTC
> I don't understand. How does this change behavior of applications?
> According to the documentation, it calls the callback for each error on the stack, and clears the stack. It shouldn't change global state, apart from clearing the stack.

My fault, I thought that ERR_print_errors_cb() sets a global error handler callback. OpenSSL's naming convention is not consistent.

> Or, since this is a RHEL patch and I know we have the FIPS Object Module, I can just skip the error checking entirely:

I'm all in favor to keep the patch as simple as possible. Less code means less chances to screw things up and cause additional regressions. Since the patch is only applied to RHEL and RHEL always has FIPS module, the one liner should do the trick.

For upstream we can do something with ERR_peek_last_error() and then check for library == ERR_LIB_CRYPTO and reasons == CRYPTO_R_FIPS_MODE_NOT_SUPPORTED.

Comment 26 Kaleem 2019-09-04 13:04:20 UTC
*** Bug 1744614 has been marked as a duplicate of this bug. ***

Comment 34 errata-xmlrpc 2019-11-05 22:03:44 UTC
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHSA-2019:3520


Note You need to log in before you can comment on or make changes to this bug.