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

gh-61460: Stronger HMAC in multiprocessing #20380

Merged
merged 5 commits into from
May 20, 2023

Conversation

tiran
Copy link
Member

@tiran tiran commented May 25, 2020

Signed-off-by: Christian Heimes <christian@python.org>
@tiran tiran force-pushed the bpo17258-multiproc-md5 branch from f4d7007 to c7f7680 Compare November 17, 2020 15:17
@florinspatar
Copy link
Contributor

I'm just wondering here, but is this still waiting for reviews before it can be merged?

Copy link
Member

@gpshead gpshead left a comment

Choose a reason for hiding this comment

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

Why jump through all the hoops to specify the digest in the protocol? Don't we always control both ends of the connection so there should never be a situation where negotiation and understanding of what was used is needed?

That'd be a lot less complicated.

And not prone to the potential problem this has of always stooping to the lowest level decided upon out by the challenge initiator rather than requiring a specific hash to be used on the channel.

Lib/multiprocessing/connection.py Outdated Show resolved Hide resolved
Lib/multiprocessing/connection.py Outdated Show resolved Hide resolved
The protocol modification idea remains, but we now take advantage of the
message length as an indicator of legacy vs modern protocol version.  No
more regular expression usage.  We now default to HMAC-SHA256, but do so
in a way that will be compatible when communicating with older clients
or older servers. No protocol transition period is needed.

More unittests to verify these claims remain true are required.
Lib/multiprocessing/connection.py Outdated Show resolved Hide resolved
Lib/multiprocessing/connection.py Outdated Show resolved Hide resolved
@gpshead gpshead self-assigned this Nov 20, 2022
@gpshead gpshead marked this pull request as ready for review November 20, 2022 21:32
@gpshead gpshead added the type-feature A feature request or enhancement label Nov 20, 2022
@gpshead gpshead changed the title bpo-17258: Stronger HMAC in multiprocessing gh-61460: Stronger HMAC in multiprocessing Nov 20, 2022
@gpshead
Copy link
Member

gpshead commented Nov 20, 2022

I believe this is in much better shape now, reviews appreciated @tiran & @pitrou.

This feature combined with #99309 will close the loop on #97514 - allowing people who oddly want to use Linux abstract namespace sockets for forkserver to do so "safely" again.

@netlify
Copy link

netlify bot commented Dec 11, 2022

Deploy Preview for python-cpython-preview ready!

Name Link
🔨 Latest commit ee5e6ff
🔍 Latest deploy log https://app.netlify.com/sites/python-cpython-preview/deploys/639526524ecd2e0009172e1f
😎 Deploy Preview https://deploy-preview-20380--python-cpython-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@gpshead gpshead enabled auto-merge (squash) May 20, 2023 23:24
@gpshead gpshead merged commit 3ed57e4 into python:main May 20, 2023
@gpshead
Copy link
Member

gpshead commented May 20, 2023

merged, we'll see how this goes during the betas!

@xnox
Copy link

xnox commented Sep 5, 2024

Given this is backwards and forwards compatible, and doesn't change default connectivity methods, has it been considered for backports to earlier python versions? as it would increase overlap as to which servers can interoperate, thus allowing to eventually change the default organically as md5 drops off in availability on the most modern servers/OSes.

@gpshead
Copy link
Member

gpshead commented Sep 5, 2024

There's no appetite to do it (see my comment on the original issue, this is a feature, not a bugfix, and not a security fix 3.11 and earlier are security fix only now).

People who think they need this are better off upgrading to 3.12. (the default does change in this PR FWIW, but if someone were backporting it into their own old runtime they could consider making deliver_challenge not change its default)

csabella pushed a commit to DataDog/cpython that referenced this pull request Dec 17, 2024
bpo-17258:  `multiprocessing` now supports stronger HMAC algorithms for inter-process connection authentication rather than only HMAC-MD5.

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

gpshead: I Reworked to be more robust while keeping the idea.

The protocol modification idea remains, but we now take advantage of the
message length as an indicator of legacy vs modern protocol version.  No
more regular expression usage.  We now default to HMAC-SHA256, but do so
in a way that will be compatible when communicating with older clients
or older servers. No protocol transition period is needed.

More integration tests to verify these claims remain true are required. I'm
unaware of anyone depending on multiprocessing connections between
different Python versions.

---------

(cherry picked from commit 3ed57e4)

Co-authored-by: Christian Heimes <christian@python.org>
Signed-off-by: Christian Heimes <christian@python.org>
Co-authored-by: Gregory P. Smith [Google] <greg@krypto.org>
csabella pushed a commit to DataDog/cpython that referenced this pull request Dec 17, 2024
bpo-17258:  `multiprocessing` now supports stronger HMAC algorithms for inter-process connection authentication rather than only HMAC-MD5.

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

gpshead: I Reworked to be more robust while keeping the idea.

The protocol modification idea remains, but we now take advantage of the
message length as an indicator of legacy vs modern protocol version.  No
more regular expression usage.  We now default to HMAC-SHA256, but do so
in a way that will be compatible when communicating with older clients
or older servers. No protocol transition period is needed.

More integration tests to verify these claims remain true are required. I'm
unaware of anyone depending on multiprocessing connections between
different Python versions.

---------

(cherry picked from commit 3ed57e4)

Co-authored-by: Christian Heimes <christian@python.org>
Signed-off-by: Christian Heimes <christian@python.org>
Co-authored-by: Gregory P. Smith [Google] <greg@krypto.org>
csabella added a commit to DataDog/cpython that referenced this pull request Dec 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic-multiprocessing type-feature A feature request or enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants