Set usedforsecurity=False for md5 call in utils.color_of #8979
Conversation
Unit Test ResultsSee test report for an extended history of previous test failures. This is useful for diagnosing flaky tests. 27 files ±0 27 suites ±0 11h 25m 31s ⏱️ - 11m 14s For more details on these failures, see this check. Results for commit 4d45cdf. ± Comparison against base commit 8d8c878. |
There was a problem hiding this comment.
LGTM. For the vast majority of users this will be a noop change. There's not really anything we can do in the way of testing as we don't have access to FIPS systems. The downside of this is there could be a regression in the future where FIPS support is broken again by some other change that we can't test for. So please consider FIPS support on a best efforts basis and not a guarantee.
The only test failures here appear unrelated to this change so I'mm happy to merge this in.
|
A few more related comments. I think that whether this helps you depends on how MD5 is controlled on your high security system. Looking at the hashlib docs, it appears sometimes there are python builds that simply omit md5. Luckily on my FIPS system, its "disabled" rather than omitted, and the change in this PR seems to fix the problem. If this ever becomes more of an issue, an alternative would be to change to a different more secure hashing algorithm altogether, but that would possibly have some kind of noticeable impact on other users (colors would probably change). The solution in this PR should not have any noticeable impact for most users, as @jacobtomlinson indicates. |
|
Yeah this method is only used in the dashboard to set colours of things like tasks in the task stream, or stack levels in the profile flame graph. Switching to a different hashing algorithm would change which colours get chosen for which item, but I don't think it would be a hugely noticeable change. If things are working for you then that's great and we should probably just leave it be. If someone raises an issue in the future to say they are trying to use Dask on a system that has a Python build that doesn't have md5 at all then we can look at coming back and changing the algorythm. But it feels like premature optimization to try and solve a niche edge case that hasn't been reported. |
Closes #8974
Adds
usedforsecurity=Falsetomd5call inutils.color_of. This will allow this function to work on FIPS systems where the md5 function is blocked forusedforsecurity=True.pre-commit run --all-files