Opened 12 months ago

Closed 3 months ago

#9210 defect closed fixed (fixed)

TLS handshake failure with OpenSSL 1.1.0f

Reported by: Paul Tremberth Owned by: hawkowl
Priority: normal Milestone:
Component: core Keywords:
Cc: paul.tremberth@…, kurt@… Branch:
Author:

Description

(Continuation of bug report on the mailing list)

Debian 9 currently uses OpenSSL 1.1.0f. And some websites refuse incoming TLS connection requests. The original report (on scrapy) was about https://www.skelbiu.lt/. Apparently, the handshake fails because of advertized Elliptic Curves in the ClientHello.

I've compared Debian 8 (which uses OpenSSL 1.0.1t) and Debian 9 using official Docker images.

What I ran on both:

docker run -it debian:{8|9} /bin/bash
    ... install dependencies for twisted[tls]...
    $ virtualenv try-twisted
    $ . try-twisted/bin/activate
    $ pip install twisted[tls]

    $ echo "from __future__ import print_function

from twisted.internet import reactor
from twisted.web.client import Agent
from twisted.web.http_headers import Headers

agent = Agent(reactor)

d = agent.request(
    'GET',
    'https://www.skelbiu.lt/',
    Headers({'User-Agent': ['Twisted Web Client Example']}),
    None)

def cbResponse(ignored):
    print('Response received')
d.addCallback(cbResponse)

def cbShutdown(ignored):
    print(ignored)
    reactor.stop()
d.addBoth(cbShutdown)

reactor.run()" > test.py

    $ python test.py

From ClientHello on Debian 8:

Extension: elliptic_curves
    Type: elliptic_curves (0x000a)
    Length: 52
    Elliptic Curves Length: 50
    Elliptic curves (25 curves)
        Elliptic curve: sect571r1 (0x000e)
        Elliptic curve: sect571k1 (0x000d)
        Elliptic curve: secp521r1 (0x0019)
        Elliptic curve: sect409k1 (0x000b)
        Elliptic curve: sect409r1 (0x000c)
        Elliptic curve: secp384r1 (0x0018)
        Elliptic curve: sect283k1 (0x0009)
        Elliptic curve: sect283r1 (0x000a)
        Elliptic curve: secp256k1 (0x0016)
        Elliptic curve: secp256r1 (0x0017)
        Elliptic curve: sect239k1 (0x0008)
        Elliptic curve: sect233k1 (0x0006)
        Elliptic curve: sect233r1 (0x0007)
        Elliptic curve: secp224k1 (0x0014)
        Elliptic curve: secp224r1 (0x0015)
        Elliptic curve: sect193r1 (0x0004)
        Elliptic curve: sect193r2 (0x0005)
        Elliptic curve: secp192k1 (0x0012)
        Elliptic curve: secp192r1 (0x0013)
        Elliptic curve: sect163k1 (0x0001)
        Elliptic curve: sect163r1 (0x0002)
        Elliptic curve: sect163r2 (0x0003)
        Elliptic curve: secp160k1 (0x000f)
        Elliptic curve: secp160r1 (0x0010)
        Elliptic curve: secp160r2 (0x0011)

From Debian 9:

Extension: elliptic_curves
    Type: elliptic_curves (0x000a)
    Length: 4
    Elliptic Curves Length: 2
    Elliptic curves (1 curve)
        Elliptic curve: secp256r1 (0x0017)

I've also pushed the Wireshark captures at https://github.com/redapple/scrapy-issues/tree/master/2717/debian

Attachments (2)

debian8.pcap.pcapng (46.4 KB) - added by Paul Tremberth 12 months ago.
debian9.pcap.pcapng (2.0 KB) - added by Paul Tremberth 12 months ago.

Download all attachments as: .zip

Change History (20)

Changed 12 months ago by Paul Tremberth

Attachment: debian8.pcap.pcapng added

Changed 12 months ago by Paul Tremberth

Attachment: debian9.pcap.pcapng added

comment:1 Changed 12 months ago by Cory Benfield

Huh, bizarre: this web server doesn't support secp256r1. I wonder why: it's the most-common curve.

comment:2 Changed 12 months ago by Cory Benfield

What are the pyopenssl versions here?

comment:3 in reply to:  2 Changed 12 months ago by Paul Tremberth

Replying to Cory Benfield:

What are the pyopenssl versions here?

Debian 8 pip freeze:

(try-twisted)root@f1ff550bbf21:/tmp# pip freeze
Automat==0.6.0
Twisted==17.5.0
argparse==1.2.1
asn1crypto==0.22.0
attrs==17.2.0
cffi==1.10.0
constantly==15.1.0
cryptography==1.9
enum34==1.1.6
hyperlink==17.2.1
idna==2.5
incremental==17.5.0
ipaddress==1.0.18
pyOpenSSL==17.1.0
pyasn1==0.2.3
pyasn1-modules==0.0.9
pycparser==2.18
service-identity==17.0.0
six==1.10.0
wsgiref==0.1.2
zope.interface==4.4.2

Debian 9 pip freeze:

(try-twisted) root@bb1bb693d050:/# pip freeze
asn1crypto==0.22.0
attrs==17.2.0
Automat==0.6.0
cffi==1.10.0
constantly==15.1.0
cryptography==1.9
enum34==1.1.6
hyperlink==17.2.1
idna==2.5
incremental==17.5.0
ipaddress==1.0.18
pkg-resources==0.0.0
pyasn1==0.2.3
pyasn1-modules==0.0.9
pycparser==2.18
pyOpenSSL==17.1.0
service-identity==17.0.0
six==1.10.0
Twisted==17.5.0
zope.interface==4.4.2

comment:4 in reply to:  1 ; Changed 12 months ago by Paul Tremberth

Replying to Cory Benfield:

Huh, bizarre: this web server doesn't support secp256r1. I wonder why: it's the most-common curve.

Weird perhaps.

Note that as I mention [on the mailing list https://twistedmatrix.com/pipermail/twisted-web/2017-April/005293.html], openssl v1.1 client uses 4 by default: ecdh_x25519, secp256r1, secp521r1, secp384r1 But Twisted Agent uses only 1.

comment:5 in reply to:  4 Changed 12 months ago by Paul Tremberth

Changing _defaultCurveName = u"secp384r1" solved it for me at the time, in April. I haven't tried again.

Replying to Paul Tremberth:

Replying to Cory Benfield:

Huh, bizarre: this web server doesn't support secp256r1. I wonder why: it's the most-common curve.

Weird perhaps.

Note that as I mention [on the mailing list https://twistedmatrix.com/pipermail/twisted-web/2017-April/005293.html], openssl v1.1 client uses 4 by default: ecdh_x25519, secp256r1, secp521r1, secp384r1 But Twisted Agent uses only 1.

comment:6 Changed 11 months ago by Cory Benfield

Yeah, so I think this requires an enhancement to use the default curves when that functionality is available.

comment:7 Changed 11 months ago by Cory Benfield

PyOpenSSL DTRT automatically for PyOpenSSL 17.0 and later, when using OpenSSL 1.0.2 or later. We can probably feature detect this and extend the function as needed there.

comment:8 Changed 11 months ago by Paul Tremberth

Cc: paul.tremberth@… added

comment:9 Changed 8 months ago by Richard van der Hoff

This sounds like https://bugs.python.org/issue29697 to me? (TL;DR: ECDH support with OpenSSL 1.1 was broken; fixed in Python 2.7.14 / 3.5.4).

comment:10 Changed 7 months ago by Kurt Roeckx

Twisted seems to be doing the same thing wrong as python was doing. After having fixed python2.7, just using python worked, but synapse (which uses twisted) was still having this issue. After removing the lines that set _ecCurve things work.

Last edited 7 months ago by Kurt Roeckx (previous) (diff)

comment:11 Changed 7 months ago by Kurt Roeckx

Cc: kurt@… added
Keywords: review added

I've created a pull request at: https://github.com/twisted/twisted/pull/927

comment:12 Changed 7 months ago by mark williams

Keywords: review removed

comment:13 Changed 7 months ago by mark williams

Keywords: review added

comment:14 Changed 7 months ago by Adi Roiban

Keywords: review removed
Owner: set to mark williams

Latest PR activity is at https://github.com/twisted/twisted/pull/935

See PR for comments. This needs some clarification and I think more work :)

In this ticket's or the PR's description it is not clear for me, how I can manually check that the changes will produce the expected results.

comment:15 Changed 6 months ago by mark williams

There are several axes of variation that affect this bug:

  • OpenSSL versions: 1.0.1, 1.0.2, 1.1.0
  • OpenSSL configurations: ECDH supported or not
  • pyOpenSSL versions: 16.0.0 to latest

Twisted's existing test suite runs under OpenSSL 1.0.1 (because Travis jobs come with 1.0.1 and cryptography doesn't build wheels for Python 3.3) and 1.1.0 (because everything else gets cryptography wheels with 1.1.0), and the latest versions of cryptography and pyOpenSSL.

pyOpenSSL 16.2.0 is what's currently available on stretch, and it does not use OpenSSL 1.0.2's automatic ECDH curve API.

To have confidence that any patch fixes this issue, it needs to be tested against pyOpenSSL 16.2.0 and OpenSSL 1.1.0 on Debian 9. It also needs to work on the other supported points on the above axes.

It turns out that Twisted cannot run its tests against pyOpenSSL < 16.2.0 (see #9354), so that limits the axes to OpenSSL versions and configurations. I put together a set of Docker containers to test 1.0.1 with and without ECDH, 1.0.2, 1.1.0, and finally Debian 9 with the reproducer script:

https://github.com/markrwilliams/twisted-ssl-environments

I ran all the containers against https://github.com/twisted/twisted/pull/935 (specifically https://github.com/twisted/twisted/pull/935/commits/cfa00498a1748e0168e590df126e626dad775c98).

The reproducing script succeeds; its ClientHello includes the following curves:

TLSv1.2 Record Layer: Handshake Protocol: Client Hello
    Content Type: Handshake (22)
    Version: TLS 1.0 (0x0301)
    Length: 214
    Handshake Protocol: Client Hello
        Extension: supported_groups (len=10)
            Type: supported_groups (10)
            Length: 10
            Supported Groups List Length: 8
            Supported Groups (4 groups)
                Supported Group: x25519 (0x001d)
                Supported Group: secp256r1 (0x0017)
                Supported Group: secp521r1 (0x0019)
                Supported Group: secp384r1 (0x0018)

the elliptic_curves extension was renamed to supported_groups

And it negotiates secp384r1:

TLSv1.2 Record Layer: Handshake Protocol: Server Key Exchange
    Content Type: Handshake (22)
    Version: TLS 1.2 (0x0303)
    Length: 365
    Handshake Protocol: Server Key Exchange
        Handshake Type: Server Key Exchange (12)
        Length: 361
        EC Diffie-Hellman Server Params
            Curve Type: named_curve (0x03)
            Named Curve: secp384r1 (0x0018)

The collected code coverage results in a diff coverage of 100%.

comment:16 Changed 6 months ago by mark williams

Keywords: review added

comment:17 Changed 3 months ago by hawkowl

Keywords: review removed
Owner: changed from mark williams to hawkowl

I am reasonably happy with this. I fixed the one (minor) review comment and the builds pass, so I'm happy.

comment:18 Changed 3 months ago by Amber Brown <hawkowl@…>

Resolution: fixed
Status: newclosed

In dbb84d72:

Merge 9210-ecdh-selection-1: Fix TLS handshake failure with OpenSSL 1.1.0f

Author: kroeckx, markrwilliams
Reviewers: adiroiban, hawkowl
Fixes: ticket:9210

Note: See TracTickets for help on using tickets.