#1690 pki.client may use PyOpenSSL or ssl
Closed: migrated 7 months ago by dmoluguw. Opened 5 years ago by cheimes.

Adam Williamson found a bug in FreeIPA after he upgraded his system from an old Fedora version. The FreeIPA web ui kept crashing and segfaulting with a SELinux violation. SELinux was preventing execmem in the context of Apache httpd. Eventually he was able to pin-point the cause of the crash to Dogtag's pki Python library, python-requests, python-cryptography and at last to a problem in python-cffi.

It turned out that python-requests was using PyOpenSSL in his system. On a default installation of Dogtag and FreeIPA, requests uses the stdlib ssl module of Python. It just happened that he had the necessary packages installed to trigger requests' pyopenssl.inject_into_urllib3(), which replace stdlib ssl with PyOpenSSL. I was able to reprodue the same bug and triggering SELinux violations by install python-ndg_httpsclient and pyOpenSSL.

# dnf install python-ndg_httpsclient pyOpenSSL
# systemctl restart httpd

The SELinux violation itself is out-of-scope for Dogtag and must be fixed in cffi. However some FreeIPA developers and me are concerned about the fact that the installation of a seemingly unrelated package switches the TLS/SSL backend of python-requests. All Dogtag and FreeIPA QE tests are run against stdlib ssl module. Apparently nobody has verified FreeIPA with PyOpenSSL yet.

I see three ways to handle the problem:

1) Modify python-requests to allow opt-out from the PyOpenSSL monkey-patch. I talked to core developers of requests and urllib3. That's not going to happen anytime soon.

2) Add a hack to prevent monkey-patching and enforce stdlib ssl. It's possible to trigger an ImportError for pyopenssl monkey-patching module with sys.modules['requests.packages.urllib3.contrib.pyopenssl'] = None. The monkey-patch can also be disabled with extract_from_urllib3(). The ssl module in Python 2.7.9+ and 3.4+ are as secure as PyOpenSSL.

3) Require PyOpenSSL once the cffi bug has been fixed. Instead of stdlib ssl Dogtag can also be tested with PyOpenSSL. PyOpenSSL has some interesting features for Dogtag, e.g. PKCS#12 files. In order to require OpenSSL, the RPM package has to depend on python-ndg_httpsclient and PyOpenSSL.

related IPA ticket: https://bugzilla.redhat.com/show_bug.cgi?id=1277224

upstream ticket in cffi: https://bitbucket.org/cffi/cffi/issues/231/writeable-memory-execution-execmem-with


I'm all in favor for option 3. For one I like to replace TLS and crypto related code with python-cryptography. PyOpenSSL is based on python-cryptography. The PKCS!#12 feature has some merit for Dogtag, too. With some slight improvements to PyOpenSSL it's possible to load user cert, private key, intermediate certs and trust anchor from a password-protected PKCS!#12 file. That's not possible with stdlib ssl.

Per discussions on 11/19/2015 on IRC, cheimes proposed marking this bug as 10.3 (although this could change due to dependencies on external component changes).

The issue is no longer pressing. Fedora's FreeIPA packages 4.2.3-2 and newer have a workaround for the issue. https://bodhi.fedoraproject.org/updates/freeipa-4.2.3-2.fc23 https://fedorahosted.org/freeipa/ticket/5442

I'm still in favor for option 3 (use PyOpenSSL) but it won't happen soon. PyOpenSSL still uses dynamic closures for some callbacks (passwords, verification hook and NPN). The latest version of cryptography only has code to address the password callback. I don't expect a fix in the next month or two.

Based on comment#4, I'm pushing this out to Dogtag 10.4.

Metadata Update from @cheimes:
- Issue set to the milestone: UNTRIAGED

4 years ago

Dogtag PKI is moving from Pagure issues to GitHub issues. This means that existing or new
issues will be reported and tracked through Dogtag PKI's GitHub Issue tracker.

This issue has been cloned to GitHub and is available here:
https://github.com/dogtagpki/pki/issues/2249

If you want to receive further updates on the issue, please navigate to the
GitHub issue and click on Subscribe button.

Thank you for understanding, and we apologize for any inconvenience.

Metadata Update from @dmoluguw:
- Issue close_status updated to: migrated
- Issue status updated to: Closed (was: Open)

7 months ago

Login to comment on this ticket.

Metadata