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

HHH-18974 - initialize SecureRandom at runtime #9657

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

jrenaat
Copy link
Member

@jrenaat jrenaat commented Jan 21, 2025

inside the UuidVersion7Strategy constructor


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-18974

…ide the UuidVersion7Strategy constructor

Signed-off-by: Jan Schatteman <jschatte@redhat.com>
Copy link
Member

@beikov beikov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea is to rather remove Holder and adding a private final SecureRandom numberGenerator = new SecureRandom(); to UuidVersion7Strategy, which will require that State#getNextState accepts a SecureRandom parameter.

@jrenaat
Copy link
Member Author

jrenaat commented Jan 22, 2025

The idea is to rather remove Holder and adding a private final SecureRandom numberGenerator = new SecureRandom(); to UuidVersion7Strategy, which will require that State#getNextState accepts a SecureRandom parameter.

I've already talked with Yoann about a potential solution; the PR is currently only partial.

…ategy.Holder classes

Signed-off-by: Jan Schatteman <jschatte@redhat.com>
@jrenaat jrenaat changed the title HHH-18974 - remove the access to the SecureRandom numbergenerator HHH-18974 - initialize SecureRandom at runtime Jan 22, 2025
@jrenaat jrenaat requested a review from yrodiere January 22, 2025 21:56
Copy link
Member

@yrodiere yrodiere left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM assuming you managed to get tests to pass on Quarkus (as we discussed), thanks!


The idea is to rather remove Holder and adding a private final SecureRandom numberGenerator = new SecureRandom(); to UuidVersion7Strategy

Unfortunately if we do that, the UuidVersion7Strategy#INSTANCE, along with its SecureRandom, will end up in the heap just before native compilation, which will lead to the exact same problem.

Your suggestion could potentially work if the numberGenerator in UuidVersion7Strategy was lazily-initialized -- but even then I'm not entirely sure.

The removal of the numberGenerator access is indeed not enough, it's just the first part of a two-fold solution:

  1. First commit: making sure that no access is made to numbergenerator before we actually generate UUIDs (i.e., at runtime for native applications).
  2. Second commit: making sure that any native application using Hibernate ORM will initialize the Holder classes at runtime, ensuring the SecureRandom (and its seed) will be initialized at runtime.

This is enough to fix the issue in theory, and Jan checked this by running integration tests in my ORM 7 upgrade PR, so we should be fine.

…imeInitialization()

Signed-off-by: Jan Schatteman <jschatte@redhat.com>
Signed-off-by: Jan Schatteman <jschatte@redhat.com>
@jrenaat jrenaat marked this pull request as ready for review January 23, 2025 16:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants