Replace md5() with sha256() for hashing

[shaunbrady]

For FIPS[0] related installation, the MD5 modules are unavailable, to
the tune of md5() raising an exception. Using sha256() from the same
hashlib passes the same make test and is now FIPS compliant.

[0] http://csrc.nist.gov/groups/STM/cmvp/documents/140-1/140val-all.htm

[nicholasserra]

Nice, thank you. I'm concerned about backwards compatibility though, I know of some users who are checking for those md5 extractions using regex during the pre and post processing callbacks in this library.

This may have to go in before a major version bump.

[shaunbrady]

Understood. We use (and rely on) semver as well. Do you by chance have a major release slated/in the works?

[nicholasserra]

No major releases planned but it could be done.

[shaunbrady]

I'd hate to chew up your major version space just for this one change, but we'll have to fork until it's released. Because this loads as a part of another packages setup_requirement, we can't avoid the code path (we run down it just by installing our requirements). Keep me posted here on what you decide.

Thanks!

[nicholasserra]

Will do. The md5 stuff isn't really something that is exposed as a public interface to this package, so it wouldn't be terrible if we just landed this as a patch version. I know of only one downstream interacting with it directly.

[nicholasserra]

cc @trentm curious on your thoughts

[nicholasserra]

@shaunbrady another thought i'm having is to just keep the md5 string in the text, but use sha still. We'd have to truncate to 32 characters too. And maybe add a comment for this silliness. But it would maintain backwards compat and I think would be fine. This is basically for copying and pasting chunks of text, so i'm not concerned about collisions or anything.

Basically:

# We used to use md5 for this and kept the "md5" prefix for backwards compatibility.
return 'md5-' + sha256(SECRET_SALT + s.encode("utf-8")).hexdigest()[32:]

Thoughts?

[shaunbrady]

@nicholasserra Hm, seems a little odd, but I understand the angle (backwards compatibility). Maybe take out the silliness in a future major release? I'm fine wiht it; if you say go, I'll code/test this up and make another PR.

[nicholasserra]

Definitely odd, but it's an internal hash basically, so as long as it's commented I think it's fine. You can push another commit to this branch and the PR will update accordingly :) Thanks!

[shaunbrady]

Updated. Note, for a few seconds there, this branch had two commits that were for our internal fork. They aren't sensitive, but I wouldn't commit them if you happened to see them. If fixed it so it should look like a normal pull now.

[nicholasserra]

Looks good to me, thank you for this patch and the collaboration :)