Update RestrictedSecurity flags, alter debug comments and profile name #701
Conversation
closed/src/java.base/share/classes/openj9/internal/security/RestrictedSecurity.java
Outdated
Show resolved
Hide resolved
closed/src/java.base/share/classes/openj9/internal/security/RestrictedSecurity.java
Outdated
Show resolved
Hide resolved
closed/src/java.base/share/classes/openj9/internal/security/RestrictedSecurity.java
Outdated
Show resolved
Hide resolved
closed/src/java.base/share/classes/openj9/internal/security/RestrictedSecurity.java
Outdated
Show resolved
Hide resolved
closed/src/java.base/share/classes/openj9/internal/security/RestrictedSecurity.java
Outdated
Show resolved
Hide resolved
closed/src/java.base/share/classes/openj9/internal/security/RestrictedSecurity.java
Outdated
Show resolved
Hide resolved
closed/src/java.base/share/classes/openj9/internal/security/RestrictedSecurity.java
Outdated
Show resolved
Hide resolved
closed/src/java.base/share/classes/openj9/internal/security/RestrictedSecurity.java
Outdated
Show resolved
Hide resolved
closed/src/java.base/share/classes/openj9/internal/security/RestrictedSecurity.java
Outdated
Show resolved
Hide resolved
closed/src/java.base/share/classes/openj9/internal/security/RestrictedSecurity.java
Outdated
Show resolved
Hide resolved
KostasTsiounis
force-pushed
the
rs_flags
branch
2 times, most recently
from
5c927cd
to
d637b49
Compare
| + selectedProfile); | ||
| } | ||
| for (Object keyObject : props.keySet()) { | ||
| // A matching property is found. |
There was a problem hiding this comment.
This comment doesn't seem appropriate here. I don't think it's needed at all. Same in the else case.
There was a problem hiding this comment.
I restructured the check for the property there and updated the comment to match it.
There was a problem hiding this comment.
The else case looks good now, but this comment in this place doesn't make sense to me. Maybe if you rewrote it. The next line is checking if the key is a String. The code seem readable enough without any comment here.
There was a problem hiding this comment.
It's about the if that comes after, so I can put it below that. I can also remove it altogether though.
There was a problem hiding this comment.
I'm fine with it removed, or if it's closer to the if that it's referring to.
There was a problem hiding this comment.
You seem to have removed a comment from the else case, but the comment on line 295 which I was discussing is still there.
There was a problem hiding this comment.
Your updated comment in the else case was fine, up to you if you want to put it back.
There was a problem hiding this comment.
It's fine as it is now.
There was a problem hiding this comment.
Are you going to remove the comment on line 296?
closed/src/java.base/share/classes/openj9/internal/security/RestrictedSecurity.java
Outdated
Show resolved
Hide resolved
closed/src/java.base/share/classes/openj9/internal/security/RestrictedSecurity.java
Outdated
Show resolved
Hide resolved
KostasTsiounis
force-pushed
the
rs_flags
branch
from
d54d2e9
to
de71e0f
Compare
closed/src/java.base/share/classes/openj9/internal/security/RestrictedSecurity.java
Outdated
Show resolved
Hide resolved
closed/src/java.base/share/classes/openj9/internal/security/RestrictedSecurity.java
Outdated
Show resolved
Hide resolved
closed/src/java.base/share/classes/openj9/internal/security/RestrictedSecurity.java
Outdated
Show resolved
Hide resolved
KostasTsiounis
force-pushed
the
rs_flags
branch
from
bdf576b
to
3860294
Compare
|
There is a merge conflict now that needs to be resolved. |
KostasTsiounis
force-pushed
the
rs_flags
branch
2 times, most recently
from
0e237c8
to
f619688
Compare
|
Resolved merge conflict. |
|
jenkins compile amac jdknext |
KostasTsiounis
force-pushed
the
rs_flags
branch
from
72c2f71
to
bef7d21
Compare
|
@pshipton I added a commit and it stopped the build. Should we restart it? |
|
If all you did was remove the comment, we don't need to rerun the build. Do you need to run any testing on the latest changes? Otherwise I'll go ahead and merge it and you can start backporting. |
|
No, nothing functional was changed so we should be fine. |
Always revert to Java impl when OpenSSL fails
The original flag to enable FIPS (i.e.,
-Dsemeru.fips=true) remains the same, but the one allowing a user to set a custom profile is changed to-Dsemeru.customprofile=<profile.version>.The debug messages have been altered a bit to only be enabled using the already known and used by similar components
-Djava.security.auth.debugflag. The information for available profiles, as well as the profile used in the particular run, is printed as part of the debug messages, instead of specifying additional properties in the custom profile flag.Further checks are added to ensure solutions are supported in the running platform and the profile is marked as FIPS compliant.
The flag for the custom profile allows the user to either specify the full name of the profile to be used (e.g.,
-Dsemeru.customprofile=NSS.FIPS140-2), or specify the solution to be used (e.g.,-Dsemeru.customprofile=NSS) and allowRestrictedSecurityto pick the default profile for that.The naming of profiles has, also, been altered to abide by the
<solution.version>template (e.g.,NSS.FIPS140-2), instead of an integer.Signed-off by: Kostas Tsiounis kostas.tsiounis@ibm.com