Use PKCS5Padding instead of ISO10126Padding for secrets encryption #14193
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mpfz0r
changed the title
If Graylog is run in an FIPS environment there is no crypto provider available that supports "AES/CBC/ISO10126Padding". This is likely because this padding standard was withdrawn from ISO in 2007. It is considered bad practice to leave a subliminal channel in the padding. We tried to workaround this by explicitly using BouncyCastle for the encryption/decryption. This however creates problems with Oracle Java, because we strip the signature off the bouncy castle jar while repackaging it into our Uber-Jar. In contrast to OpenJDK, Oracle Java does not allow the use of unsigned Security Providers. Solution: Change our secret encryption to using PKCS5Padding instead. There is a FIPS compatible provider "SunPKCS11-NSS-FIPS" available which supports "AES/CBC/PKCS5Padding". For backwards compatibility, we decrypt the ISO10126Padded keys without stripping the padding, and do that manually. Fixes #14153 Refs #13525
bernd
approved these changes
Dec 13, 2022
|
I also tested on a FIPS-enabled RedHat. |
This seems to happen with FIPS enabled OpenJDK 17
bernd
requested changes
Dec 13, 2022
thll
approved these changes
Dec 13, 2022
There was a problem hiding this comment.
Looks good. Pending the one comment from @bernd, I think this is good to go.
Tested with RHEL 9 with fips mode disabled and also enabled. I was able to reproduce the issue with Oracle JDK 17 and Graylog version 5.0 and could observe the error being fixed when using the snapshot build for this PR. Reading encrypted values which have been written with Graylog version 5.0 worked flawlessly after the fix was applied.
bernd
approved these changes
Dec 14, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
If Graylog is run in an FIPS environment there is no crypto provider available that supports
AES/CBC/ISO10126Padding. This is likely because this padding standard was withdrawn from ISO in 2007.It is considered a bad practice to leave a subliminal channel in the padding.
We tried to workaround this by explicitly using BouncyCastle for the encryption/decryption.
This however creates problems with Oracle Java, because we strip the signature off the bouncy castle jar while repackaging it into our Uber-Jar.
In contrast to OpenJDK, Oracle Java does not allow the use of unsigned Security Providers.
Solution:
Change our secret encryption to using
PKCS5Paddinginstead. There is a FIPS compatible providerSunPKCS11-NSS-FIPSavailable which supportsAES/CBC/PKCS5Padding.For backwards compatibility, we decrypt the ISO10126Padded keys without stripping the padding, and do that manually.
Fixes #14153
Refs #13525