Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add ChaCha20 Poly1305 to SSL ciphers #71953

Closed
tiran opened this issue Aug 15, 2016 · 14 comments
Closed

Add ChaCha20 Poly1305 to SSL ciphers #71953

tiran opened this issue Aug 15, 2016 · 14 comments
Assignees
@tiran
Labels
stdlib Python modules in the Lib dir topic-SSL type-security A security issue

Comments

@tiran
Copy link
Member

tiran commented Aug 15, 2016

BPO 27766
Nosy @birkenfeld, @larryhastings, @giampaolo, @tiran, @alex, @hynek, @dstufft, @Lukasa, @AraHaan
Dependencies
  • bpo-26470: Make OpenSSL module compatible with OpenSSL 1.1.0
    		•
    Files
  • Add-ChaCha20-Poly1305-to-SSL-ciphers.patch
    		•

    Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = 'https://github.com/tiran'
closed_at = <Date 2016-09-24.21:26:17.444>
created_at = <Date 2016-08-15.08:57:42.510>
labels = ['type-security', 'expert-SSL', 'library']
title = 'Add ChaCha20 Poly1305 to SSL ciphers'
updated_at = <Date 2016-09-24.21:26:17.443>
user = 'https://github.com/tiran'

    bugs.python.org fields:

    activity = <Date 2016-09-24.21:26:17.443>
actor = 'christian.heimes'
assignee = 'christian.heimes'
closed = True
closed_date = <Date 2016-09-24.21:26:17.444>
closer = 'christian.heimes'
components = ['Library (Lib)', 'SSL']
creation = <Date 2016-08-15.08:57:42.510>
creator = 'christian.heimes'
dependencies = ['26470']
files = ['44117']
hgrepos = []
issue_num = 27766
keywords = ['patch']
message_count = 14.0
messages = ['272740', '272742', '272749', '272750', '272751', '272753', '272758', '272759', '272760', '272761', '272762', '273150', '274583', '274585']
nosy_count = 12.0
nosy_names = ['georg.brandl', 'janssen', 'larry', 'giampaolo.rodola', 'christian.heimes', 'alex', 'python-dev', 'francismb', 'hynek', 'dstufft', 'Lukasa', 'Decorater']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'security'
url = 'https://bugs.python.org/issue27766'
versions = ['Python 2.7', 'Python 3.4', 'Python 3.5', 'Python 3.6']
    @tiran
    Copy link
    Member Author

    tiran commented Aug 15, 2016

    The ssl module has two cipher suite configurations, one for server-side and the other for client-side. Issue bpo-26470 will add OpenSSL 1.1.0 support, which will introduce new cipher suites with ChaCha 20 stream cipher and Poly1305 authenticator.

    CHAHA20 should be used when GCM is not available (AES GCM > CHACHA20 > AES CBC).

    $ bin/openssl ciphers 'ECDH+AESGCM:ECDH+CHACHA20:DH+AESGCM:DH+CHACHA20:ECDH+AES256:DH+AES256:ECDH+AES128:DH+AES:ECDH+HIGH:DH+HIGH:ECDH+3DES:DH+3DES:RSA+AESGCM:RSA+AES:RSA+HIGH:RSA+3DES:!aNULL:!eNULL:!MD5'
ECDHE-ECDSA-AES256-GCM-SHA384:ECDHE-RSA-AES256-GCM-SHA384:ECDHE-ECDSA-AES128-GCM-SHA256:ECDHE-RSA-AES128-GCM-SHA256:ECDHE-ECDSA-CHACHA20-POLY1305:ECDHE-RSA-CHACHA20-POLY1305:DHE-DSS-AES256-GCM-SHA384:DHE-RSA-AES256-GCM-SHA384:DHE-DSS-AES128-GCM-SHA256:DHE-RSA-AES128-GCM-SHA256:DHE-RSA-CHACHA20-POLY1305:ECDHE-ECDSA-AES256-CCM8:ECDHE-ECDSA-AES256-CCM:ECDHE-ECDSA-AES256-SHA384:ECDHE-RSA-AES256-SHA384:ECDHE-ECDSA-AES256-SHA:ECDHE-RSA-AES256-SHA:DHE-RSA-AES256-CCM8:DHE-RSA-AES256-CCM:DHE-RSA-AES256-SHA256:DHE-DSS-AES256-SHA256:DHE-RSA-AES256-SHA:DHE-DSS-AES256-SHA:ECDHE-ECDSA-AES128-CCM8:ECDHE-ECDSA-AES128-CCM:ECDHE-ECDSA-AES128-SHA256:ECDHE-RSA-AES128-SHA256:ECDHE-ECDSA-AES128-SHA:ECDHE-RSA-AES128-SHA:DHE-RSA-AES128-CCM8:DHE-RSA-AES128-CCM:DHE-RSA-AES128-SHA256:DHE-DSS-AES128-SHA256:DHE-RSA-AES128-SHA:DHE-DSS-AES128-SHA:ECDHE-ECDSA-CAMELLIA256-SHA384:ECDHE-RSA-CAMELLIA256-SHA384:ECDHE-ECDSA-CAMELLIA128-SHA256:ECDHE-RSA-CAMELLIA128-SHA256:DHE-RSA-CAMELLIA256-SHA256:DHE-DSS-CAMELLIA256-SHA256:DHE-RSA-CAMELLIA128-SHA256:DHE-DSS-CAMELLIA128-SHA256:DHE-RSA-CAMELLIA256-SHA:DHE-DSS-CAMELLIA256-SHA:DHE-RSA-CAMELLIA128-SHA:DHE-DSS-CAMELLIA128-SHA:ECDHE-ECDSA-DES-CBC3-SHA:ECDHE-RSA-DES-CBC3-SHA:DHE-RSA-DES-CBC3-SHA:DHE-DSS-DES-CBC3-SHA:AES256-GCM-SHA384:AES128-GCM-SHA256:AES256-CCM8:AES256-CCM:AES128-CCM8:AES128-CCM:AES256-SHA256:AES128-SHA256:AES256-SHA:AES128-SHA:CAMELLIA256-SHA256:CAMELLIA128-SHA256:CAMELLIA256-SHA:CAMELLIA128-SHA:DES-CBC3-SHA

    Bonus points:
    Prefer CHACHA20 over AESGCM on hardware without AES-NI and CLMUL CPU instructions.

    @tiran tiran added type-bug An unexpected behavior, bug, or error stdlib Python modules in the Lib dir type-security A security issue and removed type-bug An unexpected behavior, bug, or error labels Aug 15, 2016
    @tiran
    Copy link
    Member Author

    tiran commented Aug 15, 2016

    On X86 and X86_64 AES-NI and PCLMULQDQ can be detected with OPENSSL_ia32cap_loc(). https://www.openssl.org/docs/man1.0.2/crypto/OPENSSL_ia32cap_loc.html

    @alex
    Copy link
    Member

    alex commented Aug 15, 2016

    So, for servers really what we care about is if the _client_ has PCLMULQDQ/AESNI, not whether the server itself does. Unfortunately, there's no sane way to do this.

    Haven't reviewed this patch in terribly much detail, but conceptually fine. Cory, we should make sure this type of change propogates its way through requests, urllib3, hynek's blog post, and whatever else has a copy-pasted ciphersuite string.

    @Lukasa
    Copy link
    Mannequin

    Lukasa mannequin commented Aug 15, 2016

    Yup. So for Requests at least, the fix is easy: because OpenSSL kindly just quietly ignores cipher suites it doesn't know about we can unconditionally add it to the requests/urllib3 cipher string. In the first instance we'll just do it statically, and then we can consider down the road whether Python/cryptography could give us a way to ask whether we should prefer ChaCha20 over AES-GCM.

    In the short term, my expectation is that we'd still want to prioritise AES-GCM over ChaCha20 in Requests: is there any reason to think that I'm wrong there?

    @alex
    Copy link
    Member

    alex commented Aug 15, 2016

    Simply doing AES-GCM before ChaCha20 is probably the simplest thing to start with, can always get fancier later.

    @tiran
    Copy link
    Member Author

    tiran commented Aug 15, 2016

    On 2016-08-15 13:09, Alex Gaynor wrote:

    Alex Gaynor added the comment:

    So, for servers really what we care about is if the _client_ has PCLMULQDQ/AESNI, not whether the server itself does. Unfortunately, there's no sane way to do this.

    For servers we want to prefer CHACHA20 over AESGCM iff both sides have
    AES-NI and CLMUL. A server on a device such as a RPi benefits from
    CHACHA20, too. For that reason I also changed the server side cipher string.

    As you already said, there is no way to express this with OpenSSL cipher
    suite string.

    @Lukasa
    Copy link
    Mannequin

    Lukasa mannequin commented Aug 15, 2016

    Update for Requests+urllib3 is here: urllib3/urllib3#947

    Update for Twisted is here: https://twistedmatrix.com/trac/ticket/8760

    @AraHaan
    Copy link
    Mannequin

    AraHaan mannequin commented Aug 15, 2016

    tbh I personally perfer aiohttp over requests.

    @tiran
    Copy link
    Member Author

    tiran commented Aug 15, 2016

    Cory, Alex:

    Do you like to have a public API for CPU feature discovery? I don't mind to make OPENSSL_ia32cap_loc() a public API or even expose the bit set as structure with nice field names.

    Decorater:

    This ticket is not a vote on favorite packages. Please keep it on topic.

    @Lukasa
    Copy link
    Mannequin

    Lukasa mannequin commented Aug 15, 2016

    Christian: Certainly I'd like to be able to use that API from within urllib3 and Twisted. Having something public would be really convenient. Of course, it'd be good if OpenSSL exposed something useful here, but in the absence of that Python would be convenient.

    @alex
    Copy link
    Member

    alex commented Aug 15, 2016

    Exposing it in some way would be good, but we can make that a seperate issue.

    @francismb
    Copy link
    Mannequin

    francismb mannequin commented Aug 19, 2016

    Documentation cosmetic:

    # * Prefer ECDHE over DHE for better performance
    # * Prefer any AES-GCM over any AES-CBC for better performance and security
    +# * Prefer any AES-GCM over any AES-CBC for better performance and security

    The patch seems to be adding the same preference comment? or did you
    mean other preference?

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Sep 6, 2016

    New changeset d2111109fd77 by Christian Heimes in branch '3.5':
    Issues bpo-27850 and bpo-27766: Remove 3DES from ssl default cipher list and add ChaCha20 Poly1305.
    https://hg.python.org/cpython/rev/d2111109fd77

    New changeset 6f4f19217d9b by Christian Heimes in branch '2.7':
    Issues bpo-27850 and bpo-27766: Remove 3DES from ssl default cipher list and add ChaCha20 Poly1305.
    https://hg.python.org/cpython/rev/6f4f19217d9b

    New changeset f586742e56cb by Christian Heimes in branch 'default':
    Issues bpo-27850 and bpo-27766: Remove 3DES from ssl default cipher list and add ChaCha20 Poly1305.
    https://hg.python.org/cpython/rev/f586742e56cb

    @tiran
    Copy link
    Member Author

    tiran commented Sep 6, 2016

    See bpo-27850. ChaCha20 is even less relevant for 3.3 an 3.4. It either requires LibreSSL, patch bpo-26470 or a patched OpenSSL installation.

    @tiran tiran self-assigned this Sep 15, 2016
    @tiran tiran closed this as completed Sep 24, 2016
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees

    @tiran tiran

    Labels
    stdlib Python modules in the Lib dir topic-SSL type-security A security issue
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    2 participants
    @alex @tiran