search

nasa / trick

Trick Simulation Environment. Trick provides a common set of simulation capabilities and utilities to build simulations automatically.
Other
26 stars 15 forks source link

TrickOps: Use usedforsecurity=False when calling hashlib.md5 for compare mechanism #1662

Closed ddj116 closed 4 months ago

ddj116 commented 4 months ago

This clears the "ValueError: ... disabled for FIPS" exception for systems with FIPS mechanisms enabled. This is safe because hashlib is only used to compare baseline data to test data in trickops, it's not used for any security mechanisms.

FYI @hchen99 @jmpenn @sharmeye

Closes #1661

coveralls commented 4 months ago

Coverage Status

coverage: 55.923% (+0.01%) from 55.909% when pulling bbc1e9de6f1eaf94f9e90d11f343813a4def231e on 1661-hashlib-md5-fix into 2ac342cfc59fb400ba6421b4f17028dc5ffcd00c on master.

hchen99 commented 4 months ago

Thanks for the PR. Did some research and the solution seems to be the right way to fix it.

According to https://stackoverflow.com/questions/77425682/what-is-the-point-of-usedforsecurity:

One aspect of FIPS is restricting the hash functions you are allowed to use. In particular, MD5 is not allowed under FIPS. When you use MD5 in a FIPS environment, you will encounter an error. That is what the introduction of usedforsecurity is supposed to fix: give you an escape hatch in the case that you truly want to use MD5 in a FIPS environment. The parameter is designed to be specified at each call site so it can be audited on a case by case basis.

There seems to be confusion on many sides that usedforsecurity has anything to do with security. It's not. Having it set to False doesn't reduce your security. On FIPS enabled environments, it does however switch your implementation of the hash function to one that was explicitly certified.

In conclusion, for all intents and purposes, the parameter might as well have been called exceptionforfips because that's the singular purpose it serves: as an escape hatch if you happen to work under a FIPS environment and still need to use a FIPS non-compliant hash.

ddj116 commented 4 months ago

@hchen99 Any objection to me tacking on a quick fix for #1663 to this PR before we merge? I can do a separate PR for that if you prefer.

hchen99 commented 4 months ago

@hchen99 Any objection to me tacking on a quick fix for #1663 to this PR before we merge? I can do a separate PR for that if you prefer.

If you can do a separate pull request, that'd be great. Thanks!