`SSLFactory.Builder.withDefaultIdentityMaterial` method

[hakky54]

`SSLFactory.Builder.withDefaultIdentityMaterial` method

[Hakky54]

Hi Sergey, @rymsha

Thank you for you suggestion. It sounds like a nice idea to have a default identity material just like the default trust material. However your suggestion is different than what the default trust material function provides.

The withDefaultTrustMaterial method gets the list of trusted certificates by the JDK from the JDK and does not use javax.net.ssl.trustStore as these two are different things. javax.net.ssl.trustStore is just a system property just like javax.net.ssl.keyStore. I think it can be misleading for the end-user as the withDefaultTrustMaterial does not use the system property and your suggestion of withDefaultIdentityMaterial uses the system property.

What do you think?

And alternative would be separate some of the logic to improve the readability.

import java.nio.file.Path;
import java.nio.file.Paths;

public class App {
    
    private static final Path KEYSTORE = Paths.get(System.getProperty("javax.net.ssl.keyStore"));
    private static final char[] KEYSTORE_PASSWORD = System.getProperty("javax.net.ssl.keyStorePassword").toCharArray();
    private static final String KEYSTORE_TYPE = System.getProperty("javax.net.ssl.keyStoreType");
    
    private static final Path TRUSTSTORE = Paths.get(System.getProperty("javax.net.ssl.trustStore"));
    private static final char[] TRUSTSTORE_PASSWORD = System.getProperty("javax.net.ssl.trustStorePassword").toCharArray();
    private static final String TRUSTSTORE_TYPE = System.getProperty("javax.net.ssl.trustStoreType");
    
    public static void main(String[] args) {
        SSLFactory sslFactory = SSLFactory.builder()
                .withIdentityMaterial(KEYSTORE, KEYSTORE_PASSWORD, KEYSTORE_TYPE)
                .withTrustMaterial(TRUSTSTORE, TRUSTSTORE_PASSWORD, TRUSTSTORE_TYPE)
                .build();
    }
    
}

[rymsha]

This is a call chain for withDefaultTrustMaterial:
SSLFactory.withDefaultTrustMaterial -> TrustManagerUtils.createTrustManagerWithJdkTrustedCertificates() -> createTrustManager(null) -> ... -> TrustManagerFactory.init(null) -> ... -> sun.security.ssl.TrustStoreManager.getTrustedCerts() that reads javax.net.ssl.trustStore system property and its siblings.
Of course it is from one vendor only. Other JSSE implementations don't have to read javax.net.ssl.trustStore

withDefaultIdentityMaterial name is confusing, I admit. There is no "Default Identity Material". For instance SSLContext.init JavaDoc states

Initializes this context. Either of the first two parameters may be null in which case the installed security providers will be searched for the highest priority implementation of the appropriate factory.

But in reality if null is specified for KeyManager[] km argument SSLContext always ends up using DummyX509KeyManager

// Dummy X509KeyManager implementation, never returns any certificates/keys.
// Used if the application did not specify a proper X509TrustManager.

The problem is described in detail here: https://stackoverflow.com/questions/15269419/how-do-i-provide-a-specific-truststore-while-using-the-default-keystore-in-java

BTW, there is already a method SSLFactory.withSystemTrustMaterial that uses non-standard KeyStore Types. That is why I thought it would be OK to provide a helper method that reads javax.net.ssl.keyStore

Maybe you can come up with a better method name?

[Hakky54]

I like the idea that it would be easier to setup SSLFactory with system properties by taking away the verbosity and null checks. I have worked on an initial implementation. But I am curious what your opinion is. Would this be useful for you? Please have a look here: #165

And another question for you, do you have some inspiration for the method name?
Currently I have named it temporally withPlaceHolderIdentity and withPlaceHolderTrustMaterial, but I plan to change it when I have something more meaningful.

[rymsha]

According to https://docs.oracle.com/en/java/javase/11/security/java-secure-socket-extension-jsse-reference-guide.html#GUID-7D9F43B8-AABF-4C5B-93E6-3AFB18B66150 and because "Default" is confusing, we can call it withCustomizedKeystoreIdentity.

As for TrustMaterial , I don't believe there should be a new method, because withDefaultTrustMaterial simply does what is needed already.

[Hakky54]

Thank you for your suggestion however I think withCustomizedKeystoreIdentity can also be confusing as it is not similar to the existing method names and next to that it is not so self explaining. I came up with the following name prefix: SystemPropertyDerived. It is verbose but it is more self explaining as it tells the library user that it will try to create the SSLFactory based on system properties. So to sum up, it will look like this:

SSLFactory sslFactory = SSLFactory.builder()
        .withSystemPropertyDerivedIdentityMaterial()
        .withSystemPropertyDerivedTrustMaterial()
        .withSystemPropertyDerivedProtocols()
        .withSystemPropertyDerivedCiphers()
        .build();

What do you think?

You also mentioned the following:

As for TrustMaterial , I don't believe there should be a new method, because withDefaultTrustMaterial simply does what is needed already.

I don't agree on this part as the method withDefaultTrustMaterial uses the JDK trusted certificates while withSystemPropertyDerivedTrustMaterial will create the trust material based on the system properties, which are:

-Djavax.net.ssl.trustStore=/path/to/truststore.jks
-Djavax.net.ssl.trustStoreType=jks
-Djavax.net.ssl.trustStorePassword=changeit
-Djavax.net.ssl.trustStoreProvider=SunJSSE

So these two have different responsibilities so I would prefer to keep them separate and not chain them as you suggested earlier. What do you think of the latest version? Is it something that would do the trick for your project?

[rymsha]

Good name suggestion!

Currently we decided to leave things as-is for our project - just wrote a note in the documentation that use of server certificates disables ability to customize client certificate via system properties. We'll see how it goes. If users don't like it - we well end-up using the new methods.

Thank you very much for all your work. It helped a lot.

[Hakky54]

Awesome, thank you again for your nice idea and contribution. I just made a release. These changes are available from version 7.4.0 onwards.