Skip to content
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

fix: use sha1 instead of md5 to make the installer work in FIPS #1260

Merged
merged 1 commit into from Mar 8, 2023

Conversation

ravitejb
Copy link
Contributor

@ravitejb ravitejb commented Mar 2, 2023

SUMMARY

New or Enhanced Feature

ISSUE TYPE

Use SHA1 instead of md5 to make the installer work in FIPS mode.

ADDITIONAL INFORMATION

Related to a new annotation added here:

@ravitejb
Copy link
Contributor Author

ravitejb commented Mar 3, 2023

@rooftopcellist @TheRealHaoLiu This is ready for review

@TheRealHaoLiu
Copy link
Member

Ur wonderful!

@ravitejb
Copy link
Contributor Author

ravitejb commented Mar 8, 2023

Thanks for the review @TheRealHaoLiu
Any idea who can help us merge this PR?

@rooftopcellist
Copy link
Member

Thanks for following up and contributing this @ravitejb !

@rooftopcellist rooftopcellist merged commit 6cae8df into ansible:devel Mar 8, 2023
@SiliconAlley
Copy link

Thanks you guys. We where dealing since weeks to get the awx-operator working on our FIPS enabled environment. After dealing with FQCN's already this finally let us successfully install AWX.

Also thanks for contributing this @ravitejb

@walterrowe
Copy link

walterrowe commented Mar 10, 2023

@ravitejb @rooftopcellist

It might be wise to use something more future proof than SHA1.

https://csrc.nist.gov/Projects/hash-functions

SP 800-131 says SHA1 is acceptable for checksums. SHA2 and SHA3 are preferred. See page 18.

https://nvlpubs.nist.gov/nistpubs/SpecialPublications/NIST.SP.800-131Ar2.pdf

@SiliconAlley
Copy link

@walterrowe

Agree, I thought the same in the morning. We adopted it and it made us happy. Can we easily change it to a prefered one?

@rooftopcellist
Copy link
Member

I thought about adding | checksum here instead, but it looks like that is also SHA1.

It looks like SHA2/3 are not available with the version of hashlib we currently have. SHA256 is available, but seems like overkill, plus it is 65 characters, so I think that we will have problems with the k8s-enforced annotation character length of 63 chars (maybe that's only on names, not annotations?).

So I am inclined to leave it as is, or if anything, change it to | checksum so that we pick up any new hash algorithms support added to ansible-core.

However if this is a no-go for install, we could look at a newer python hashlib dep and see if that makes SHA2 availalble. But it seems like FIPS still allows SHA1's use for checksums.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants