Opened 4 years ago

Closed 4 years 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 4 years ago.
debian9.pcap.pcapng (2.0 KB) - added by Paul Tremberth 4 years ago.

Download all attachments as: .zip

Change History (20)

Changed 4 years ago by Paul Tremberth

Attachment: debian8.pcap.pcapng added

Changed 4 years ago by Paul Tremberth

Attachment: debian9.pcap.pcapng added

comment:1 Changed 4 years 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 4 years ago by Cory Benfield

What are the pyopenssl versions here?

comment:3 in reply to:  2 Changed 4 years 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 4 years 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 4 years 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 4 years 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 4 years 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 4 years ago by Paul Tremberth

Cc: paul.tremberth@… added

comment:9 Changed 4 years 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 4 years 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 4 years ago by Kurt Roeckx (previous) (diff)

comment:11 Changed 4 years 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 4 years ago by mark williams

Keywords: review removed

comment:13 Changed 4 years ago by mark williams

Keywords: review added

comment:14 Changed 4 years 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 4 years 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 4 years ago by mark williams

Keywords: review added

comment:17 Changed 4 years 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 4 years ago by Amber Brown <hawkowl@…>

Resolution: fixed
Status: newclosed

In dbb84d72:

Error: Processor CommitTicketReference failed
 does not appear to be a Git repository. See the log for more information.
Note: See TracTickets for help on using tickets.