Listener specific keystore not used

[ratcashdev]

Description

---

Keystore explicitely assigned to be used by a listener is ignored.

Expected Outcome

the SSL implementation used by the listener (by default com.sun.enterprise.security.ssl.GlassfishSSLImpl) would read the keystore explicitely assigned to the listener.

Current Outcome

The listener is using the default keystore specified as a -Djavax.net.ssl.keyStore= JVM parameter.

Steps to reproduce (Only for bug reports)

1. copy the default keystore keystore.jks in the config folder. Name it keystore3.jks
2. remove the s1as alias from keystore3
3. add a new keypair to keystore-http2, name it ssl12.
4. configure http-listener-2 to use keystore3 and use alias ssl12, i.e. the domain/config.xml should look like:
   <protocol name="http-listener-2" security-enabled="true"> <http max-connections="250" default-virtual-server="server"> <file-cache></file-cache> </http> <ssl classname="fish.payara.letsencrypt.ssl.PayaraSSLImpl" cert-nickname="ssl12" key-store="${com.sun.aas.instanceRoot}/config/keystore3.jks"></ssl> </protocol>
   
   Note: disregard the classname: it is a direct copy of the com.sun.enterprise.security.ssl.GlassfishSSLImpl with some logging enhancement.
5. start the domain. It will go all right
6. try to access https://localhost:8181. The browser will show Site can't be reached. (Chrome will show This site can’t be reached. localhost unexpectedly closed the connection.). This should however, work, since keystore3.jks contains the necessary alias and keypair.
7. If you add alias ssl12 to the default keystore (keystore.jks) it will work correctly.

Context (Optional)

This was discovered while trying to develop a keystore implementation that would facilitate hot-reload of the keystore and certificates for the purposes of #1047
However, as it can be seen, it is not being called for the keystore in question,only for the default keystore (visible from the logs).

The actual GlassfishServerSocketFactory uses the following code snippet to retrieve the keystore inside methods getKeyManagers(...):

String keystoreFile = (String) attributes.get("keystore");
        if (logger.isLoggable(Level.FINE)) {
            logger.log(Level.FINE, "Keystore file= {0}", keystoreFile);
        }

Clearly, the keystore attribute for the given listener must have been set incorrectly.
If the -Djavax.net.ssl.keyStore= JVM option is not given, the keystoreFile retrieved is null

Environment

• Payara Version: 5.1.181.x
• Edition: FULL
• JDK Version: Oracle JDK 8
• Operating System: Windows
• Database: N/A

[ratcashdev]

While investigating this, I could also confirm #499 as still being an issue.

[ratcashdev]

Tracked this down to: com.sun.enterprise.security.ssl.impl.SecuritySupportImpl#initJKS that only cares about settings in the system properties:

keyStoreFileName = System.getProperty(keyStoreProp);
trustStoreFileName = System.getProperty(trustStoreProp);

The keystore parameter passed using ServerSocketFactory#setAttributes to GlassfishServerSocketFactory are not taken into account. Actually, those parameters are not passed at all: HK2 while instantiating the class does not know about them.

[ratcashdev]

Short brainstorming with some (probably bad) ideas how to fix this:

1. Change the com.sun.enterprise.server.pluggable.SecuritySupport API to be able to specify the keystores needed. May break existing uses of this Contract. Also, strangely ssl-impl contains the SecuritySupport contract, not security/core.
2. Change the SecuritySupportImpl from @Singleton to @Dependent and inject the relevant configuration (the attributes field from GlassfishServerSocketFactory) into it. I have limited experience with HK2. Care to provide a hint how to do this?
3. (Dirty) move the call to initJKS() out of the constructor. Add a configure() method to set the keystore and truststore in question (or pass as constructor parameters) and call initJKS() only afterwards, explicitly. Basically wrapping it into a Factory. This would also mean changing the SSlUtils class as well (also a @Service). This breaks the SecuritySupport contract (being ready right after injection or calling the postConstruct() method)
4. (very dirty) Move parts of the code of SSLUtil and SecuritySupportImpl into the GlassfishServerSocketFactory.

@smillidge So the big question is, can we break contract(s) in order to fix this? Or leave it as it is, and create our own (PayaraSSLImpl) implementation that will not be bound by those contracts.

[smillidge]

breaking contracts is doable as long as default remains as is so won't hurt people just upgrading. Assuming this class insn't in a public api jar ( I haven't checked)

[smillidge]

Also have you tried with a listener that is not the default ssl listener?

[ratcashdev]

Not sure what you mean by "not the default ssl listener". You mean creating an entirely new listener/protocol combo?

[smillidge]

Yes I was wondering if a new listener had the same problem

[ratcashdev]

Confirmed: a new listener/protocol has the same behavior.

[ratcashdev]

Assuming this class insn't in a public api jar

The SecurityService contract is inside
<groupId>org.glassfish.main.security</groupId> <artifactId>ssl-impl</artifactId>
and is exported by osgi. Seems public enough to me.
Anyway. Thanks to the classname parameter of SSL config we can create our own implementation not bound by this contract. Even though this seems to be an upstream bug, not payara specific.

[ratcashdev]

It seems the bug is specific to the GlassfishServerSocketFactory. Its superclass JSSE14SocketFactory does not exhibit this bug.

[ratcashdev]

@smillidge submitted #2635 as a suggestion for a fix.

[ratcashdev]

#2635 got merged, I consider this issue resolved.