Skip to content

bpo-43522: Fix SSLContext.hostname_checks_common_name (GH-24899) #24899

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.

Sign up for GitHub

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
Merged
merged 1 commit into from
Apr 17, 2021
Merged

bpo-43522: Fix SSLContext.hostname_checks_common_name (GH-24899) #24899

merged 1 commit into from
Apr 17, 2021

Conversation

tiran
Copy link
Member

@tiran tiran commented Mar 16, 2021 •
edited by bedevere-bot
Loading

Fix problem with ssl.SSLContext.hostname_checks_common_name. OpenSSL does not
copy hostflags from struct SSL_CTX to struct SSL.

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

https://bugs.python.org/issue43522

@tiran tiran added type-bug An unexpected behavior, bug, or error needs backport to 3.7 needs backport to 3.9 only security fixes labels Mar 16, 2021
@tiran tiran mentioned this pull request Mar 16, 2021
Closed
pquentin
pquentin approved these changes Mar 17, 2021
View reviewed changes
Copy link

@pquentin pquentin left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, both the reproducer and the urllib3 test suite run fine with this change!

@pquentin pquentin mentioned this pull request Mar 18, 2021
Closed
@tiran
Copy link
Member Author

tiran commented Apr 9, 2021

The fix has landed in OpenSSL 3.0.0 and is flagged for backport to 1.1.1.

@tiran tiran force-pushed the bpo-43522-ssl-hostflags branch from dd0f554 to 5dcab4b Compare April 9, 2021 15:05
@tiran tiran marked this pull request as ready for review April 9, 2021 15:07
@tiran tiran requested review from 1st1 and asvetlov as code owners April 9, 2021 15:07
@tiran tiran force-pushed the bpo-43522-ssl-hostflags branch from 5dcab4b to c25e687 Compare April 13, 2021 13:07
@pquentin
Copy link

Thanks! Should we mention in the docs that the flag had no effect until now?

@tiran
Copy link
Member Author

tiran commented Apr 13, 2021

Thanks! Should we mention in the docs that the flag had no effect until now?

Something like this? I'll adjust the versionchanged to 3.8.9 and 3.9.3 in backports.

  .. versionchanged:: 3.10

     The flag had no effect with OpenSSL 1.1.1k and older.

@pquentin
Copy link

Yes, that would be perfect. That way we'll know for what Python and OpenSSL combinations this flag will be safe to use.

There's one thing I don't understand: is it enough to have either the CPython and OpenSSL fixes, or do we need both?

@tiran
Copy link
Member Author

tiran commented Apr 13, 2021

The workaround in this PR is only necessary for OpenSSL 1.1.1k and older. 1.1.1l and 3.0.0 are going to copy the flag correctly without the workaround.

#if ... OPENSSL_VERSION < 0x101010cf

>>> chr(0xc + ord('a') - 1)
'l'

@tiran tiran force-pushed the bpo-43522-ssl-hostflags branch from c25e687 to 8c652a6 Compare April 17, 2021 07:35
@tiran
bpo-43522: Fix SSLContext.hostname_checks_common_name
3cba59d 
Fix problem with ssl.SSLContext.hostname_checks_common_name. OpenSSL does not
copy hostflags from *struct SSL_CTX* to *struct SSL*.

Signed-off-by: Christian Heimes <christian@python.org>
@tiran tiran force-pushed the bpo-43522-ssl-hostflags branch from 8c652a6 to 3cba59d Compare April 17, 2021 07:41
@tiran tiran changed the title bpo-43522: Fix SSLContext.hostname_checks_common_name bpo-43522: Fix SSLContext.hostname_checks_common_name (GH-24899) Apr 17, 2021
@tiran tiran merged commit b467d9a into python:master Apr 17, 2021
@miss-islington
Copy link
Contributor

Thanks @tiran for the PR 🌮🎉.. I'm working now to backport this PR to: 3.8, 3.9.
🐍🍒⛏🤖

@tiran tiran deleted the bpo-43522-ssl-hostflags branch April 17, 2021 08:07
@miss-islington
Copy link
Contributor

Sorry, @tiran, I could not cleanly backport this to 3.9 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker b467d9a24011992242c95d9157d3455f8a84466b 3.9

@miss-islington
Copy link
Contributor

Sorry @tiran, I had trouble checking out the 3.8 backport branch.
Please backport using cherry_picker on command line.
cherry_picker b467d9a24011992242c95d9157d3455f8a84466b 3.8

@bedevere-bot
Copy link

GH-25451 is a backport of this pull request to the 3.9 branch.

@bedevere-bot bedevere-bot removed the needs backport to 3.9 only security fixes label Apr 17, 2021
tiran added a commit to tiran/cpython that referenced this pull request Apr 17, 2021
@tiran
[3.9] bpo-43522: Fix SSLContext.hostname_checks_common_name (pythonGH…
330b49e 
…-24899)

Fix problem with ssl.SSLContext.hostname_checks_common_name. OpenSSL does not
copy hostflags from *struct SSL_CTX* to *struct SSL*.

Signed-off-by: Christian Heimes <christian@python.org>.
(cherry picked from commit b467d9a)

Co-authored-by: Christian Heimes <christian@python.org>
tiran added a commit to tiran/cpython that referenced this pull request Apr 17, 2021
@tiran
[3.8] [3.9] bpo-43522: Fix SSLContext.hostname_checks_common_name (py…
5f38140 
…thonGH-24899)

Fix problem with ssl.SSLContext.hostname_checks_common_name. OpenSSL does not
copy hostflags from *struct SSL_CTX* to *struct SSL*.

Signed-off-by: Christian Heimes <christian@python.org>.
(cherry picked from commit b467d9a)

Co-authored-by: Christian Heimes <christian@python.org>
(cherry picked from commit 330b49e)

Co-authored-by: Christian Heimes <christian@python.org>
@bedevere-bot
Copy link

GH-25452 is a backport of this pull request to the 3.8 branch.

tiran added a commit that referenced this pull request Apr 17, 2021
@tiran
[3.9] bpo-43522: Fix SSLContext.hostname_checks_common_name (GH-24899) (
cdf0287 
GH-25451)

Fix problem with ssl.SSLContext.hostname_checks_common_name. OpenSSL does not
copy hostflags from *struct SSL_CTX* to *struct SSL*.

Signed-off-by: Christian Heimes <christian@python.org>.
(cherry picked from commit b467d9a)

Co-authored-by: Christian Heimes <christian@python.org>
tiran added a commit that referenced this pull request Apr 17, 2021
@tiran
[3.8] bpo-43522: Fix SSLContext.hostname_checks_common_name (GH-24899) (
f77ca86 
GH-25452)

Fix problem with ssl.SSLContext.hostname_checks_common_name. OpenSSL does not
copy hostflags from *struct SSL_CTX* to *struct SSL*.
(cherry picked from commit 330b49e)
@bedevere-app bedevere-app bot mentioned this pull request Jan 16, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@1st1 1st1 Awaiting requested review from 1st1

@asvetlov asvetlov Awaiting requested review from asvetlov

1 more reviewer

@pquentin pquentin pquentin approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees

@tiran tiran

Labels
type-bug An unexpected behavior, bug, or error
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@tiran @pquentin @miss-islington @bedevere-bot @the-knights-who-say-ni