Issue2390

classification
Title: Support SSLContext
Type: Severity: normal
Components: Versions: Jython 2.7
Milestone: Jython 2.7.1
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: darjus Nosy List: ajdavis, darjus, zyasoft
Priority: urgent Keywords:

Created on 2015-09-04.01:33:58 by zyasoft, last changed 2017-04-10.19:32:28 by ajdavis.

Messages
msg10221 (view) Author: Jim Baker (zyasoft) Date: 2015-09-04.01:33:58
This is new functionality as of CPython 2.7.10. Expose the equivalent functionality in Java.
msg10233 (view) Author: Jim Baker (zyasoft) Date: 2015-09-04.17:54:02
After upgrading the current release of pip from the bundled wheel specific to Jython 2.7.0, we get problems like the following:

$ dist/bin/jython -m pip install nose
Collecting nose
/Users/jbaker/jythondev/jython27/dist/Lib/site-packages/pip/_vendor/requests/packages/urllib3/util/ssl_.py:84: InsecurePlatformWarning: A true SSLContext object is not available. This prevents urllib3 from configuring SSL appropriately and may cause certain SSL connections to fail. For more information, see https://urllib3.readthedocs.org/en/latest/security.html#insecureplatformwarning.
...

I should point out that Jython is in fact *secure*, and uses the CAs bundled as part of the Java release, so this is a false positive. But we can readily fix by supporting SSLContext. There are other advantages, including SSLContext avoids reading CA cert files on every verified SSL connection.
msg10354 (view) Author: Jim Baker (zyasoft) Date: 2015-10-16.20:49:20
Running 2.7.1b2, we see that there is additional SSLContext work to do. Need to figure out scope, but here are the user visible problems:

First, let's upgrade pip:

$ bin/jython -m pip install --upgrade pip
/Users/jbaker/jython2.7.1b2/Lib/site-packages/pip/_vendor/requests/cacert.pem
Downloading/unpacking pip from https://pypi.python.org/packages/py2.py3/p/pip/pip-7.1.2-py2.py3-none-any.whl#md5=5ff9fec0be479e4e36df467556deed4d
  Downloading pip-7.1.2-py2.py3-none-any.whl (1.1MB): 1.1MB downloaded
Installing collected packages: pip
  Found existing installation: pip 1.6.dev1
    Uninstalling pip:
      Successfully uninstalled pip
Successfully installed pip
Cleaning up...
jimbaker:jython2.7.1b2 jbaker$ bin/jython -m pip install pytest
Collecting pytest
/usr/local/etc/openssl/cert.pem
/usr/local/etc/openssl/cert.pem
  Could not find a version that satisfies the requirement pytest (from versions: )
No matching distribution found for pytest
/usr/local/etc/openssl/cert.pem

I tried with the develop branch of pip as well:

$ /Users/jbaker/jython2.7.1b2/bin/jython -m pip install bottle
Collecting bottle
/usr/local/etc/openssl/cert.pem
  Could not find a version that satisfies the requirement bottle (from versions: )
No matching distribution found for bottle
/usr/local/etc/openssl/cert.pem

requests is the other main target, especially since pip vendors it; using the master branch: 

$ /Users/jbaker/jython2.7.1b2/bin/jython test_requests.py
.............................................................../Users/jbaker/opensource/requests/requests/cacert.pem
............/Users/jbaker/opensource/requests/requests/cacert.pem
...........F...............................................................
======================================================================
FAIL: test_response_iter_lines_reentrant (__main__.RequestsTestCase)
Response.iter_lines() is not reentrant safe
----------------------------------------------------------------------
Traceback (most recent call last):
  File "test_requests.py", line 1127, in test_response_iter_lines_reentrant
    assert len(list(r.iter_lines())) == 3
AssertionError

----------------------------------------------------------------------
Ran 150 tests in 28.703s

FAILED (failures=1)
msg10355 (view) Author: Jim Baker (zyasoft) Date: 2015-10-17.04:21:55
We really need to get this fixed for 2.7.1, so marking high so we get in the RC. Darjus, thanks again for a good start on SSLContext, I will look at this in more detail next week.
msg10356 (view) Author: Darjus Loktevic (darjus) Date: 2015-10-17.06:01:05
Hey Jim,

Can you try removing /usr/local/etc/openssl/cert.pem ? I assume you're running on Mac and i had some issues with it (the SSLContext code is not failing at the moment, but i should probably raise an exception for it). It probably comes with homebrew and Java does not like it as is. You can also replace it with one from certifi, which seems to just work.
msg10357 (view) Author: Jim Baker (zyasoft) Date: 2015-10-17.06:36:51
Darjus, tried what you suggested by moving /usr/local/etc/openssl/cert.pem out of the way (I'm indeed running homebrew on this OS X 10.11 box), and that makes the upgraded pip happy. Sounds like we might be close!

We still need to look at requests, but could be something else.
msg10359 (view) Author: Darjus Loktevic (darjus) Date: 2015-10-18.02:12:52
Hey Jim, this issue about "is not reentrant safe" is not Jython specific:
http://docs.python-requests.org/en/latest/api/#requests.Response.iter_lines

➜  requests git:(master) ✗ python -V
Python 2.7.10

➜  requests git:(master) ✗ python test_requests.py
......................................................................................F..............................................................
======================================================================
FAIL: test_response_iter_lines_reentrant (__main__.RequestsTestCase)
Response.iter_lines() is not reentrant safe
----------------------------------------------------------------------
Traceback (most recent call last):
  File "test_requests.py", line 1116, in test_response_iter_lines_reentrant
    assert len(list(r.iter_lines())) == 3
AssertionError

----------------------------------------------------------------------
Ran 149 tests in 82.430s

FAILED (failures=1)

Same with Py3K
msg10365 (view) Author: Jim Baker (zyasoft) Date: 2015-10-20.20:40:45
Darjus, that was a good idea to run it on CPython 2/3. Strange it fails in this fashion for CPython, but it's looking good so far. Some further thoughts on looking at the code:

re SSLContext.cert_store_stats, we should be able to obtain via BC; see 
http://www.bouncycastle.org/wiki/display/JA1/X.509+Certificate+Revocation+Lists

re SSLContext.set_default_verify_paths, I'm pretty sure we can get this functionality via CompositeX509TrustManager as defined in _sslcerts. Maybe this can also resolve the issue we see with /usr/local/etc/openssl/cert.pem for brew users? TBD.

re SSLContext.set_alpn_protocols, etc, raising NotImplemented is fine for now. (It's possible we can use Netty to support.) Note that per the docs on SSLContext.set_*, we need to set values for the following:

>>> ssl.HAS_ALPN
False
>>> ssl.HAS_NPN
False
>>> ssl.HAS_ECDH
True
>>> ssl.HAS_SNI
True

Maybe we can provide support for ALPN/NPN by digging into Netty and what we can do to support SSL negotiation, but just based
 on what OS X 10.11 returns for their support, I think we are safe in deferring to 2.7.2 or later.
msg10366 (view) Author: Darjus Loktevic (darjus) Date: 2015-10-20.21:55:41
Hey Jim, thanks for the pointers. Will take a look.
msg10440 (view) Author: Darjus Loktevic (darjus) Date: 2015-11-08.22:17:38
Seems like supporting alpn/npn is a tad more involved http://www.smartjava.org/content/using-spdy-and-http-transparently-using-netty

Should we package alpn/npn and set our bootclasspath? Or detect if they were provided and enable then?
msg10511 (view) Author: Jim Baker (zyasoft) Date: 2015-11-28.02:16:08
Now blocking 2.7.1

In looking at #2437, I reviewed specifics of our SSL negotiation as changed for implementing SSLContext support and noticed we have introduced a regression compared to Jython 2.7.0:

    def _createSSLEngine(self):
        trust_managers = [NoVerifyX509TrustManager()]
        if self.verify_mode == CERT_REQUIRED:
            tmf = TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm())
            tmf.init(self._trust_store)
            trust_managers = tmf.getTrustManagers()

        if self._key_managers is None:  # get an e
            self._context.init(_get_openssl_key_manager().getKeyManagers(), trust_managers, None)
        else:
            self._context.init(self._key_managers.getKeyManagers(), trust_managers, None)

        engine = self._context.createSSLEngine()

        if self._ciphers is not None:
            engine.setEnabledCipherSuites(self._ciphers)

        return engine

The problem is in the use of NoVerifyX509TrustManager. This was intended to support being able to turn off SSL validation, but it should only be done in the context of implementing https://www.python.org/dev/peps/pep-0476, specifically support in https://www.python.org/dev/peps/pep-0476/#opting-out for monkey patching SSL so that it is possible to not validate certs, but only with some work by a developer. Such opt-in monkeypatching should use ssl._create_unverified_context (to be implemented).

This validation change is a regression, because Jython in the past used Java's underlying cert validation by default, which matches *latest* CPython support, again as specified by PEP 476.

One thing we should also consider is backporting CPython 2.7.10 (or latest) version of test_ssl and supporting files, which adds support for testing cert verification.
msg10526 (view) Author: Darjus Loktevic (darjus) Date: 2015-12-09.10:59:31
Hey Jim, quick update. I've ingested newest test_ssl.py and I'm working through it. Lots of changes/fixes out of it, so thanks for the tip.

Also just saw this interesting article: http://lwn.net/SubscriberLink/666353/2ec5359a5e424716/ regarding ssl and distro packaging.
msg10527 (view) Author: Jim Baker (zyasoft) Date: 2015-12-09.15:48:37
Darjus, thanks for pointing out Jake's article, which helped me also recall the discussion around CPython backwards compatibility at PyCon.

I think it makes sense to implement the PYTHONHTTPSVERIFY environment variable for turning off SSL verification - this does come up in testing at the very least. (It was a question I got when talking to a PyMongo maintainer, Jesse Davis.) I'm not so sure about the other functionality in PEP 493, especially since it's in draft for CPython 2.7.12.
msg10558 (view) Author: Jim Baker (zyasoft) Date: 2015-12-28.18:52:24
Just need to get Java 8 support working. (We should also test against Java 9.) Getting test_ssl to pass on 8/9 should presumably suffice to prove compliance - it is currently failing loading certs.
msg10564 (view) Author: Jim Baker (zyasoft) Date: 2015-12-29.14:34:19
Closed as sufficiently complete for our original goals, namely supporting latest pip and requests. But see these related bugs, which correspond to a continuation of this work to get full CPython-level compliance for SSL:

Support SNI for SSL/TLS #2446
Support server-side STARTTLS for SSL #2447
msg10571 (view) Author: Jim Baker (zyasoft) Date: 2015-12-29.18:06:17
Should also mention:

Java 8 fixes in https://hg.python.org/jython/rev/a7b30c51e754
Java 9 fixes in https://hg.python.org/jython/rev/5194b3d04dca

as verified with both test_ssl and using jython -m pip install PKG as an integration test
History
Date User Action Args
2017-04-10 19:32:28ajdavissetnosy: + ajdavis
2016-01-06 15:59:53zyasoftsetstatus: pending -> closed
2015-12-29 18:06:17zyasoftsetmessages: + msg10571
2015-12-29 14:34:19zyasoftsetstatus: open -> pending
resolution: accepted -> fixed
messages: + msg10564
2015-12-28 18:52:25zyasoftsetmessages: + msg10558
2015-12-09 15:48:38zyasoftsetmessages: + msg10527
2015-12-09 10:59:32darjussetmessages: + msg10526
2015-11-28 02:16:09zyasoftsetpriority: high -> urgent
messages: + msg10511
2015-11-08 22:17:38darjussetmessages: + msg10440
2015-10-20 21:55:41darjussetmessages: + msg10366
2015-10-20 20:41:11zyasoftsetresolution: accepted
2015-10-20 20:40:54zyasoftsetassignee: zyasoft -> darjus
2015-10-20 20:40:46zyasoftsetmessages: + msg10365
2015-10-18 02:12:52darjussetmessages: + msg10359
2015-10-17 06:36:53zyasoftsetmessages: + msg10357
2015-10-17 06:01:05darjussetmessages: + msg10356
2015-10-17 04:21:56zyasoftsetpriority: high
messages: + msg10355
2015-10-16 20:49:22zyasoftsetnosy: + darjus
messages: + msg10354
2015-09-04 17:54:02zyasoftsetmessages: + msg10233
2015-09-04 01:33:58zyasoftcreate