[8u-dev] RFA (S): JDK-8210985: Update the default SSL session cache size to 20480

[Andrew John Hughes]

On 24/05/2019 22:55, Hohensee, Paul wrote:
> CSRs are definitely an Oracle thing. See https://wiki.openjdk.java.net/display/csr/Main.
> 
> At Oracle, if you want to change an interface or behavior, you file a JBS issue to do the work, then file a CSR to get approval for the interface change. Closing a CSR == approving it. Finalized state means it's ready for review. One is supposed to get consensus before finalizing a CSR.
> 
> If a backport involved a CSR, Oracle used to want a backport CSR, so the process was and maybe is: create a backport JBS issue, then a CSR for the backport issue, get the CSR approved, then get the backport approved. I don't know what Oracle's CSR backport process is now, but somehow I was under the impression that I should follow it. :) We could use Oracle's process for 8u and 11u, though it's a bit heavy-weight. It does have the advantage of thoroughly documenting interface/behavior changes in backports. I'm not sure who would do the backport CSR approvals: probably the Maintainers.

I don't have a strong opinion either way, as long as I know what boxes I
need to tick :-)

> 
> I was incorrect/hasty about 8u clean patch application. 8u doesn't have SSLLogger, so that code gets dropped from SSLSessionContextImpl.getDefaultCacheLimit().
> 
> 8u webrev: http://cr.openjdk.java.net/~phh/8210985/webrev.8u.00/

There's an issue here with the changes to SSLSessionContextImpl.java
where the doPrivileged is being lost. In the original version [0],
it is changed to GetIntegerAction.privilegedGetProperty which is
introduced by JDK-8154231 [1] & JDK-8155775 [2], which I think may be
worth backporting themselves:

    public static Integer privilegedGetProperty(String theProp,
            int defaultVal) {
        Integer value;
        if (System.getSecurityManager() == null) {
            value = Integer.getInteger(theProp);
        } else {
            value = AccessController.doPrivileged(
                    new GetIntegerAction(theProp));
        }
        return (value != null) ? value : defaultVal;
    }

In the 8u backport, the GetIntegerAction is created locally and not run
inside a doPrivileged call:

+            int defaultCacheLimit =
+                new GetIntegerAction("javax.net.ssl.sessionCacheSize",
+
DEFAULT_MAX_CACHE_SIZE).run().intValue();

which is incorrect. I suggest something along the lines of:

int defaultCacheLimit = java.security.AccessController.doPrivileged(new
GetIntegerAction("javax.net.ssl.sessionCacheSize",
DEFAULT_MAX_CACHE_SIZE)).intValue();

[0] https://hg.openjdk.java.net/jdk/jdk/rev/8a85d21d9616
[1] https://bugs.openjdk.java.net/browse/JDK-8154231
[2] https://bugs.openjdk.java.net/browse/JDK-8155775
-- 
Andrew :)

Senior Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)

PGP Key: ed25519/0xCFDA0F9B35964222 (hkp://keys.gnupg.net)
Fingerprint = 5132 579D D154 0ED2 3E04  C5A0 CFDA 0F9B 3596 4222
https://keybase.io/gnu_andrew