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

bpo-17258: use sha256 instead of md5 within multiprocessing.connection #16264

Closed
wants to merge 1 commit into from

Conversation

stratakis
Copy link
Contributor

@stratakis stratakis commented Sep 18, 2019

Within multiprocessing.connection, deliver_challenge() and
answer_challenge() use hmac for a challenge/response, however
hmac implicitly defaults to using MD5, which fails under FIPS
mode. Hardcode the digest value to sha256.

This PR is adapted from the patch provided at bpo-17258 and pulled from encukou@b819ca6

https://bugs.python.org/issue17258

Within multiprocessing.connection, deliver_challenge() and
answer_challenge() use hmac for a challenge/response, however
hmac implicitly defaults to using MD5, which fails under FIPS
mode. Hardcode the digest value to sha256.
@stratakis
Copy link
Contributor Author

cc @tiran

Copy link
Member

@tiran tiran left a comment

Choose a reason for hiding this comment

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

I would prefer a more flexible solution that allows us to change algorithms more easily in the future and perhaps allows different auth mechanisms than HMAC challenge and response.

@bedevere-bot
Copy link

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

@encukou
Copy link
Member

encukou commented Dec 5, 2019

I would prefer a more flexible solution that allows us to change algorithms more easily in the future

As fas as I can see, changing the algorithm would mean setting HMAC_DIGEST_NAME to a new value. That sounds very easy.
What am I missing?

and perhaps allows different auth mechanisms than HMAC challenge and response.

I don't see how this PR is blocking someone from working on that.

@tiran
Copy link
Member

tiran commented Dec 5, 2019

The change breaks backward compatibility. multiprocessing supports distributed computing across multiple machines and works with multiple Python versions. With the change a controller with Python 3.9 would no longer be able to talk to a 3.8 server or the other way around.

@stratakis
Copy link
Contributor Author

@tiran What would you propose to move this forward, essentially moving away from md5? Any approaches that would make sense? Something like implementing a client-server handshake negotiation to agree on a hash?

@stratakis
Copy link
Contributor Author

Closing this. Superseded by #20380

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants