-
Notifications
You must be signed in to change notification settings - Fork 77
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
Add a check to detect the OpenJCEPlus module #924
base: openj9
Are you sure you want to change the base?
Conversation
2126df6
to
81d196e
Compare
if (module.isPresent()) { | ||
isOpenJCEPlusModuleExist = true; | ||
} |
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.
The field should be final
and initialized as:
isOpenJCEPlusModuleExist = module.isPresent();
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.
Done.
@@ -387,6 +397,11 @@ private static void checkIfKnownProfileSupported() { | |||
+ " on this platform."); | |||
} | |||
|
|||
if (profileID.contains("OpenJCEPlus") && !isOpenJCEPlusModuleExist) { |
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.
The operands of &&
should be reversed (doing the cheaper test first).
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.
Done.
Add a check to detect the OpenJCEPlus module. If the module is missing but the restricted security profile requires it, print an error message and exit. Signed-off-by: Tao Liu <[email protected]>
15ac91d
to
47366a5
Compare
@@ -387,6 +394,11 @@ private static void checkIfKnownProfileSupported() { | |||
+ " on this platform."); | |||
} | |||
|
|||
if (!isOpenJCEPlusModuleExist && profileID.contains("OpenJCEPlus")) { |
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.
This doesn't seem right. Trying to use a profile called "not-OpenJCEPlus" shouldn't demand the presence of the openjceplus module. Also profiles that don't contain that string might actually depend on the openjceplus module.
I would expect to just get "no such provider" or something similar if I try to use an algorithm that requires a module (perhaps openjceplus, but it could be another module) that isn't present in the runtime in use. What are the current symptoms of that situation? Are they really worth potentially breaking legitimate use-cases?
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.
The initially discussion about this is, we want to remove the OpenJCEPlus profile from the java.security file in the OpenJ9 builds. The OpenJCEPlus profile should only appear in the java.security file for the Semeru and personal builds. To do this, our first proposal was to modify how the java.security file is generated by introducing a new variable during the process. But, you suggested keeping the profile in the file and instead using a general mechanism to handle any broken profiles. So I am thinking about this current solution to detect the OpenJCEPlus module.
But after the team discussions last Friday, we realized that using this detect the OpenJCEPlus module solution won’t work for the OpenJCEPlus on z/OS. Since the OpenJCEPlus doesn’t exist as a Java module on z/OS.
So, we still think the best solution is to remove the OpenJCEPlus profile from the java.security file in the OpenJ9 builds. That said, it needs to add a new variable during the java.security file generation process. Or, do you have any other suggestions?
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.
Yes, we could conditionally include sections of java.security
depending on whether BUILD_OPENJCEPLUS
is true
or false
, but that wouldn't address the problem of a runtime created via jlink
that excludes the openjceplus
module.
It's not immediately obvious that we have a serious problem that needs to be solved. What are the symptoms if an algorithm is requested that is not present in the runtime?
Add a check to detect the OpenJCEPlus module. If the module is missing but the restricted security profile requires it, print an error message and exit.
Signed-off-by: Tao Liu [email protected]