gh-61460: Stronger HMAC in multiprocessing by tiran · Pull Request #20380 · python/cpython (original) (raw)

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

tiran

@tiran

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

@florinspatar

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

gpshead

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.

pitrou

@gpshead

@gpshead

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.

gpshead

@gpshead gpshead marked this pull request as ready for review

November 20, 2022 21:32

@gpshead gpshead changed the titlebpo-17258: Stronger HMAC in multiprocessing gh-61460: Stronger HMAC in multiprocessing

Nov 20, 2022

@gpshead

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.

@gpshead

@netlify

This was referenced

Jan 5, 2023

@gpshead

@gpshead

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

@xnox

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

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 deliverchallenge not change its default)