-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
md5 OpenSSL FIPS mode fix #7611 #7614
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with nits.
sphinx/util/__init__.py
Outdated
@@ -170,6 +170,22 @@ def __setstate__(self, state: Set[str]) -> None: | |||
self._existing = state | |||
|
|||
|
|||
def fips_safe_md5(data=b'', **kwargs): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be better to rename this to md5()
simply.
BTW, we've used |
I did not see any errors from sha1 when running the test suite on the FIPS enabled server. Thi is strange as sha1 is not allowed by FIPS either I think. It may depend on a policy on the server? I have not checked whether sha1 has the 'usedforsecurity' option. |
I guess you did not use features that uses SHA-1. Because Sphinx uses it only at 3 modules.
Could you check that |
I ran the sphinx test suite, does that not test it? Is it possible to handle possible sha1 issues in another patch, so that this can go in quickly? |
Yes, some external tools are needed to enable these modules. I guess you can check it via python console. Could you check this please?
I think FIPS also disallows to use SHA-1, it raises an error like MD5. |
I checked the hashlib source, it seems usedforsecurity is supported generically for all algorithms. |
I did the same change for sha1. I checked from console on the FIPS enabled server that it supports the flag. I ran the test suite on my laptop where all tests pass (ImageMagick is installed), so I think there should be no negative impact from this, but it may help in cases where FIPS forbids sha1. |
Great work! Thank you for your contribution :-) |
You're welcome. |
@tk0miya Can I ask when 3.0.4 is expected to be released? We are unable to build documentation at the moment. |
I don't have a plan for the release. But I understand your situation. Okay, let's ship it in this weekend. |
Thank you. Much appreciated.
Best regards / Venlig Hilsen
Lars Hupfeldt Nielsen
Hupfeldt IT
Phone: +45 2060 3546
Den tor. 21. maj 2020 kl. 18.02 skrev Takeshi KOMIYA <
[email protected]>:
… I don't have a plan for the release. But I understand your situation.
Okay, let's ship it in this weekend.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#7614 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAMCZSINVDGGGILPED44XO3RSVGC7ANCNFSM4MZESYZA>
.
|
This is a fix for #7611