bpo-31399: Let OpenSSL verify hostname and IP address#3462

tiran

Replace Python's custom ssl.match_hostname() with OpenSSL 1.0.2 APIs. The hostname or IP address are now verified by OpenSSL during the TLS handshake. A failure to verify the name results in a proper handshake abort with TLS Alert bad cert.

Signed-off-by: Christian Heimes christian@python.org

https://bugs.python.org/issue31399

alex

Mostly looks good, from a first skim

tiran

Thanks @alex

I have updated the comment for hostflaggs and created openssl/openssl#5061.

tiran

Blocked by #5180

@asvetlov @1st1 The PR touches asyncio. Please review the test changes.

asvetlov

asyncio test change is correct (but please update assertion as suggested)

tiran

@asvetlov Thanks, I changed the assert as requested

I also forgot to remove the call to match_hostname. How should I handle the call in asyncio? If I remove it, the code will no worker be secure on 3.6.

1st1

I also forgot to remove the call to match_hostname. How should I handle the call in asyncio? If I remove it, the code will no worker be secure on 3.6.

This change is for 3.7 only, right? If so just remove the call in the master branch. We no longer try to use asyncio CPython code with other CPython versions.

tiran

Yes, it's 3.7 only.

tiran

This branch is now based on PR #5242. Please review and ACK the other PR first.

asvetlov

Are there stoppers for the issue left?

tiran

@asvetlov No stoppers, I'm just waiting for a final review. The PR has changed a bit since @alex reviewed it. I have added more documentation and additional checks for LibreSSL.

asvetlov

Good to know.
Just for note -- LGTM.

alex

Code looks fine, I have some API design concerns though.

The ssl module now uses OpenSSL's X509_VERIFY_PARAM_set1_host() and X509_VERIFY_PARAM_set1_ip() API to verify hostname and IP addresses. Signed-off-by: Christian Heimes <christian@python.org>

Signed-off-by: Christian Heimes <christian@python.org>

libssl must provide X509_VERIFY_PARAM_set1_host() Signed-off-by: Christian Heimes <christian@python.org>

Signed-off-by: Christian Heimes <christian@python.org>

Remove all hostflags except for NO_PARTIAL_WILDCARDS and NEVER_CHECK_SUBJECT. The other flags aren't that useful at the moment. Don't support OpenSSL special mode with a leading dot, e.g. ".example.org" matches "www.example.org". It's not standard conform. Signed-off-by: Christian Heimes <christian@python.org>

Host flags are now in internal API. Public API is a new attribute hostname_checks_common_name. Signed-off-by: Christian Heimes <christian@python.org>

bedevere-bot

@tiran: Please replace # with GH- in the commit message next time. Thanks!

asottile

Running into this patch on travis-ci as ubuntu trusty ships with openssl 1.0.1 -- this patch causes python3.7 to build without _ssl support on trusty. See also:

I agree that the patch is the right move going forward 👍, figured I'd post this here for visibility though.

the-knights-who-say-ni added the CLA signed label Sep 8, 2017

bedevere-bot added the awaiting merge label Sep 8, 2017

Mariatta added and removed needs rebase labels Oct 9, 2017

tiran force-pushed the openssl_check_hostname branch from a3fa2cf to 75d33a3 Compare January 11, 2018 20:51

tiran requested a review from a team as a code owner January 11, 2018 20:51

tiran force-pushed the openssl_check_hostname branch from 75d33a3 to 35176ba Compare January 11, 2018 21:14

tiran requested review from 1st1 and asvetlov as code owners January 11, 2018 21:14

tiran force-pushed the openssl_check_hostname branch from 35176ba to 65faab4 Compare January 11, 2018 21:18

alex reviewed Jan 11, 2018

View reviewed changes

tiran mentioned this pull request Jan 11, 2018

Feature: Remove support for common names in certificates urllib3/urllib3#497

Closed

tiran force-pushed the openssl_check_hostname branch from 65faab4 to c9f4c6f Compare January 11, 2018 22:02

tiran force-pushed the openssl_check_hostname branch 6 times, most recently from e499eca to 89a28a5 Compare January 12, 2018 14:54

tiran changed the title ~~bpo-31399: [WIP] Let OpenSSL verify hostname and IP address~~ Jan 16, 2018

asvetlov approved these changes Jan 16, 2018

View reviewed changes

1st1 approved these changes Jan 16, 2018

View reviewed changes

tiran force-pushed the openssl_check_hostname branch 2 times, most recently from c400837 to d1dd4f3 Compare January 16, 2018 21:09

tiran force-pushed the openssl_check_hostname branch from d1dd4f3 to d07134d Compare January 17, 2018 10:20

tiran force-pushed the openssl_check_hostname branch from d07134d to 7b5edd5 Compare January 20, 2018 09:43

tiran requested a review from a team as a code owner January 20, 2018 09:43

tiran force-pushed the openssl_check_hostname branch 3 times, most recently from 7c4fc70 to 197d51f Compare January 20, 2018 12:25

tiran mentioned this pull request Jan 20, 2018

bpo-31429: Define TLS cipher suite on build time #3532

Merged

tiran mentioned this pull request Jan 20, 2018

bpo-32185: Don't send IP in SNI TLS extension #4938

Closed

alex reviewed Jan 22, 2018

View reviewed changes

tiran force-pushed the openssl_check_hostname branch 2 times, most recently from 2199b99 to e819fbd Compare January 23, 2018 11:17

tiran added 7 commits January 26, 2018 15:27

bpo-31399: Let OpenSSL verify hostname and IP …

f289cab

The ssl module now uses OpenSSL's X509_VERIFY_PARAM_set1_host() and X509_VERIFY_PARAM_set1_ip() API to verify hostname and IP addresses. Signed-off-by: Christian Heimes <christian@python.org>

Remove match_hostname calls …

9dbf5f7

Signed-off-by: Christian Heimes <christian@python.org>

Document host flags …

802807d

Signed-off-by: Christian Heimes <christian@python.org>

Check for libssl with set1_host …

bb1f7aa

libssl must provide X509_VERIFY_PARAM_set1_host() Signed-off-by: Christian Heimes <christian@python.org>

bpo-31399: Add documentation for OpenSSL 1.0.2 requirement …

a48ae16

Signed-off-by: Christian Heimes <christian@python.org>

Replace SSLContext.host_flags …

845c149

Host flags are now in internal API. Public API is a new attribute hostname_checks_common_name. Signed-off-by: Christian Heimes <christian@python.org>

tiran force-pushed the openssl_check_hostname branch from e819fbd to 845c149 Compare January 26, 2018 15:35

alex approved these changes Jan 27, 2018

View reviewed changes

tiran merged commit 61d478c into python:master Jan 27, 2018

bedevere-bot removed the awaiting merge label Jan 27, 2018

tiran deleted the openssl_check_hostname branch January 27, 2018 14:51

awarecan mentioned this pull request Feb 26, 2019

TheThings Network Component throws SSL errors after update home-assistant/core#21367

Closed

Conversation

tiran commented Sep 8, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alex left a comment

Choose a reason for hiding this comment

Uh oh!

tiran commented Jan 11, 2018

Uh oh!

tiran commented Jan 16, 2018

Uh oh!

asvetlov left a comment

Choose a reason for hiding this comment

Uh oh!

tiran commented Jan 16, 2018

Uh oh!

1st1 commented Jan 16, 2018

Uh oh!

tiran commented Jan 17, 2018

Uh oh!

tiran commented Jan 20, 2018

Uh oh!

asvetlov commented Jan 20, 2018

Uh oh!

tiran commented Jan 20, 2018

Uh oh!

asvetlov commented Jan 20, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alex left a comment

Choose a reason for hiding this comment

Uh oh!

bedevere-bot commented Jan 27, 2018

Uh oh!

asottile commented Apr 27, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

tiran commented Sep 8, 2017 •

edited

Loading

asvetlov commented Jan 20, 2018 •

edited

Loading