Update SunJSSE fully qualified name for test cases #854
Conversation
In JDK11, the fully qualified name for SunJSSE provider is com.sun.net.ssl.internal.ssl.Provider. Update this for all the restricted security mode test cases. Signed-off-by: Tao Liu <[email protected]>
| @@ -36,7 +36,7 @@ RestrictedSecurity.TestBase.Version.jce.certpath.disabledAlgorithms = | |||
| RestrictedSecurity.TestBase.Version.jce.legacyAlgorithms = | |||
| RestrictedSecurity.TestBase.Version.jce.provider.1 = sun.security.provider.Sun | |||
| RestrictedSecurity.TestBase.Version.jce.provider.2 = com.sun.crypto.provider.SunJCE | |||
| RestrictedSecurity.TestBase.Version.jce.provider.3 = sun.security.ssl.SunJSSE | |||
| RestrictedSecurity.TestBase.Version.jce.provider.3 = com.sun.net.ssl.internal.ssl.Provider | |||
There was a problem hiding this comment.
Isn't a change to RestrictedSecurity.TestBase.Version.desc.hash required because of this update?
There was a problem hiding this comment.
In the TestProperties.java file, all profiles except for Test-Profile-SameStartWithoutVersion are designed to test error messages. In these profiles, the error message is returned before the hash value is checked, so the hash value is not used.
There was a problem hiding this comment.
It would be helpful to have comments to that effect directly in the property files and perhaps specify hashes of all zeros if they are not meaningful.
There was a problem hiding this comment.
Or they could say something like this:
RestrictedSecurity.Test-Profile.Base.desc.hash = SHA256:not-checked
There was a problem hiding this comment.
Since we don’t use these hash values, I think it’s fine to just leave them as they are. However, if we do want to make them more readable, like changing them to "SHA256:not-checked", I can update this by creating another PR. For two reasons:
-
This change "
SHA256:not-checked" would need to be ported to all JDK versions. -
I can combine this update with the "fixing/improving these comments" which suggested on the JDK8 PR Add the test cases for Restricted Security Mode openj9-openjdk-jdk8#784, starting from the head stream.
There was a problem hiding this comment.
Deferring to a separate pull request is fine, but please create an issue to track that work and the suggestions in ibmruntimes/openj9-openjdk-jdk8#784.
There was a problem hiding this comment.
Issue created: https://github.ibm.com/runtimes/jit-crypto/issues/615
There was a problem hiding this comment.
Please create an issue in a public repository, perhaps at https://github.com/eclipse-openj9/openj9/issues.
There was a problem hiding this comment.
Issue eclipse-openj9/openj9#20815 created.
| @@ -50,7 +50,7 @@ RestrictedSecurity.TestBase.Version-Extended.tls.disabledAlgorithms = | |||
| RestrictedSecurity.TestBase.Version-Extended.jce.provider.1 = sun.security.provider.Sun | |||
| RestrictedSecurity.TestBase.Version-Extended.jce.provider.2 = sun.security.rsa.SunRsaSign | |||
| RestrictedSecurity.TestBase.Version-Extended.jce.provider.3 = sun.security.ec.SunEC | |||
| RestrictedSecurity.TestBase.Version-Extended.jce.provider.4 = sun.security.ssl.SunJSSE | |||
| RestrictedSecurity.TestBase.Version-Extended.jce.provider.4 = com.sun.net.ssl.internal.ssl.Provider | |||
There was a problem hiding this comment.
Again, shouldn't RestrictedSecurity.Test-Profile.Base.desc.hash also need to change?
There was a problem hiding this comment.
Same as above, in the TestProperties.java file, all profiles except for Test-Profile-SameStartWithoutVersion are designed to test error messages. In these profiles, the error message is returned before the hash value is checked, so the hash value is not used.
There was a problem hiding this comment.
I think the .hash property should make that clear, for example:
RestrictedSecurity.TestBase.Version.desc.hash = irrelevant
|
@pshipton Could you help to approve this since Keith is on vacation? |
|
Does this fix any existing OpenJ9 issues? |
It updates the restricted security mode test cases for JDK 11 to correct the fully qualified name of the SunJSSE provider. This change is not a must for the current 0.49 release, it can be included in the next release if it doesn't make it for 0.49. |
I understand, but are there any issues opened at https://github.com/eclipse-openj9/openj9/issues that describe the failure and will be closed by merging this? |
|
@jasonkatonica maybe you can help with my question. Was the failure exposed by the triage of the jdk11 M2 builds? |
|
Hi Peter, In terms of the comment #854 (comment) I don't believe that this update is necessary. This is a cosmetic update to make it clear that the hash is not used ( lots of configuration is not used in the tests ). This update in general is not required for the M2 builds / .49. We do not currently run these tests as part of our automatons that I am aware of for this release. |
|
It will likely end up in 0.49 just for convenience. It would be harder to keep it out. |
In JDK11, the fully qualified name for SunJSSE provider is
com.sun.net.ssl.internal.ssl.Provider. Update this for all the restricted security mode test cases. This update only applies to JDK11.