-
Notifications
You must be signed in to change notification settings - Fork 3.6k
HHH-19471 - Replace hashing a constraint name using SHA-256 instead of MD5 algorithm #10169
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
base: main
Are you sure you want to change the base?
Conversation
hibernate-core/src/main/java/org/hibernate/boot/model/naming/NamingHelper.java
Outdated
Show resolved
Hide resolved
Also @sebersole @beikov the only reason we ever used a hash here was that some platforms had silly limits on the length of FK names. Is that still relevant? Because if we're going to be messing about with this stuff, we should maybe think through the possibility of generating more semantic constraint names instead of the ugly hashes. |
The real trick is column name ordering and schema migrator/updater. But if you can work out a good solution for "nicer" names, I don't think anyone is going to argue. Now, breaking existing schemas with schema migrator/updater is a real concern and we'd have to have a good story there. |
hibernate-core/src/main/java/org/hibernate/boot/model/naming/NamingHelper.java
Show resolved
Hide resolved
@ShivamVerma380 please squash the commits. |
catch ( NoSuchAlgorithmException | UnsupportedEncodingException e ) { | ||
throw new HibernateException( "Unable to generate a hashed name", e ); | ||
catch (NoSuchAlgorithmException | UnsupportedEncodingException e) { | ||
log.warnf("MD5 algorithm failed for hashedName, falling back to SHA-256: %s", e.getMessage()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A WARN
is probably too aggressive here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gavinking I kept the log level as WARN so that people notice when the MD5 hashing constraint fails for some reason. If we set the level to INFO, it might go unnoticed. Would you like me to change it to INFO?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Typically, people don't notice
WARN
s until the program is already in prod. I never look at the log when I'm writing code in an IDE.WARN
should not be used to communicate with developers. WARN
means that something is dangerous and might go wrong. I don't see what's dangerous about this behavior.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed the log level to info.
Would it be more appropriate here to use a hibernate property or even java property flag to control exactly which hash alg to use? |
Oh gawd, no thanks, please not another property setting! Look, this is something that is already controllable via a property setting, namely I considered rejecting the proposal because of that. |
I mean, honestly, maybe that instinct was right, and it's better to just not fix this. It's easy to write a custom |
@gavinking We implemented a custom naming strategy using hibernate.implicit_naming_strategy, but it didn't resolve the issue because Hibernate still performs constraint name hashing internally. This hashing uses the MD5 algorithm, which is not allowed in FIPS mode, resulting in the following error: Algorithm not allowed in FIPS mode: MD5 Also I can see this in the code: In the ImplicitNamingStrategyJpaCompliantImpl class, the determineForeignKeyName() and determineUniqueKeyName() methods internally call the NamingHelper class methods generateHashedFkName() and generateHashedConstraintName(). ![]() Now we fallback to original problem i.e. hashedName() method of NamingHelper class uses MD5 algorithm for hashing. |
I don't understand. |
Okay, I'll check this. I just had an observation to share: @gavinking Do you see any concerns with this fix going in? This also aligns with the intended purpose of ImplicitNamingStrategy: it’s meant to be overridden when we want to apply consistent, customized naming conventions across entities — particularly for foreign keys, unique constraints, and indexes. However, if we don’t care about the specific names generated by Hibernate, there’s no real benefit in overriding it. It just creates an overhead of maintaining it. I might be off here, but just wanted to check with you :) |
Also, this won't be a breaking change because, for users not in FIPS mode, everything remains the same, and for those in FIPS mode, it wasn't working anyway — and will now work with SHA-256 hashing. |
I think it's fine, but I want to discuss it with others. |
…thm is not available Co-authored-by: Yitzchak Weiser <[email protected]> Co-authored-by: Ashwinkumar Rathod <[email protected]>
Hibernate currently uses the MD5 algorithm to generate unique constraint names and unique foreign-key names for @onetomany associations. This creates an issue in environments running in FIPS (Federal Information Processing Standards) mode, where MD5 is considered non-compliant and therefore disabled by default. As a result, server startup fails due to the use of MD5.
To ensure compatibility with FIPS 140-2/140-3 compliant environments, we have created a patch that replaces the usage of MD5 with SHA-256, a FIPS-approved cryptographic algorithm.
Note that MD5 is a required algorithm for Java <= 11. That requirement was dropped somewhere between Java 11 and Java 17. Hibernate 6 should not assume MD5 will be available.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license
and can be relicensed under the terms of the LGPL v2.1 license in the future at the maintainers' discretion.
For more information on licensing, please check here.
https://hibernate.atlassian.net/browse/HHH-19471