I'm not at all an expert but would these have passed FIPS if usedforsecurity was set to False, regardless of the algorithm? https://docs.python.org/3/library/hashlib.html

I'm not at all an expert but would these have passed FIPS if usedforsecurity was set to False, regardless of the algorithm? https://docs.python.org/3/library/hashlib.html

Great question. I primarily went this route following the precedent from #18193. If it worked there, I figure it should work here.

It was a few years ago, but I did run across a static compliance scanner that wasn't smart enough to respect (or perhaps intentionally didn't respect?) the usedforsecurity flag. I don't recall any specific details, it was a few years ago.

The other aspect is that I don't think that any of the instances I changed used md5 for any reason other than blind convenience. As such, unless there is a performance concern, I'd say that in these use cases sha256 is just as convenient. Correct me if I am wrong here, perhaps md5 may have been chosen intentionally over other algorithms in one of these use cases?

In any event I am happy to discuss or look into this further if there's a desire.

I'm sure it was indeed chosen for convenience. But the point of labelling as not for security is to prevent whack a mole with security checkers for something that is just being done to create a convenient label.

That's reasonable. Regardless of the algorithm used it can be added, which is helpful in-code documentation. I have added a follow-up commit.

I am not an expert in Github / Azure CI pipelining, but I do not think any of the failures are legitimate issues with my changes?

I'm happy to merge this as is. A quick search suggests that getting a GHA running in FIPS mode is not trivial.

The conventional wisdom is "md5 is insecure but fast" does not seem to hold up:

In [6]: %timeit hashlib.md5(data, usedforsecurity=False).hexdigest()
46.9 μs ± 69.9 ns per loop (mean ± std. dev. of 7 runs, 10,000 loops each)

In [7]: %timeit hashlib.sha256(data, usedforsecurity=False).hexdigest()
20.3 μs ± 19 ns per loop (mean ± std. dev. of 7 runs, 10,000 loops each)

where data is len 48407 (a small jpeg) and

In [12]: %timeit hashlib.sha256(b'some short text', usedforsecurity=False).hexdigest()
246 ns ± 1.09 ns per loop (mean ± std. dev. of 7 runs, 1,000,000 loops each)

In [13]: %timeit hashlib.md5(b'some short text', usedforsecurity=False).hexdigest()
259 ns ± 0.872 ns per loop (mean ± std. dev. of 7 runs, 1,000,000 loops each)

so I see no reason to not always prefer sha256

Something went wrong ... Please have a look at my logs.

For the sake of posterity:

Did you test if FIPS is happy with the flag even with md5 ?

In a quick test in my FIPS environment, FIPS is happy with just usedforsecurity=False

Although I agree with the decision to change the algorithm anyway.

Thanks all!

Congratulations on your first merged Maptlotlib PR @martinky24 🎉 Thank you for reporting and fixing this issue and I hope we hear from you again!

In a quick test in my FIPS environment, FIPS is happy with just usedforsecurity=False

Thats great - so we have somewhat future proofed ourselves against future changes to what is considered "secure"