Enable mutual auth utilising TLS config

[Farhie]

• Added testing around default config values
• Extract TLS config into its own object. We should look at deprecating path_to_cert, path_to_key, cert and key in XConfig in favour of TlsConfig
• Decided to build our own TLS config instead of utilising some of the ones already available such as com.xjeffrose.xio.SSL.TlsConfig as we only want to be configuring values that are utilised within the server to avoid confusion.
• BREAKING CHANGE release required.

Migration: config will have to be updated to utilise tls_config: Old config

path_to_cert = ${TLS_CERTIFICATE}
path_to_key = ${TLS_PRIVATE_KEY}

New config

tls_config {
    path_to_certificate = ${TLS_CERTIFICATE}
    path_to_private_key = ${TLS_PRIVATE_KEY}
}

For mutual auth to successfully work, this implementation requires that we add any custom CA's certs to the JRE's truststore (e.g. keytool -importcert -alias new-ca -keystore $JAVA_HOME/jre/lib/security/cacerts -storepass changeit -file new-ca.der). We could extend TLS config to support passing in of trusted CAs.

[pdex]

I have several problems with this Implementation.

1) You're not configuring the trust store and instead relying on the system installed certs. 2) You're not providing the same level of configuration as the xio TlsConfig: https://github.com/xjdr/xio/blob/master/xio-core/src/main/resources/tls-reference.conf 3) "Decided to build our own TLS config instead of utilising some of the ones already available such as com.xjeffrose.xio.SSL.TlsConfig as we only want to be configuring values that are utilised within the server to avoid confusion." This statement is incorrect, xio's TlsConfig provides configuration and sane defaults for ALL of the server settings. Your implementation relies on the netty defaults and will require a code change to address any security concerns around SSL Ciphers. 4) This implementation is entirely tied to JSSE. (keytool will not setup the OpenSSL trust store)

I would STRONGLY suggest utilizing TlsConfig and SslContextFactory from xio. Here you can see buildServer does all of the heavy lifting for you.

TlsConfig and SslContextFactory should be sufficient to get you up and running with mutual auth.

This line would be changed as so:

// old
SslContext sslCtx = null;

// new
SslContext sslCtx = SslContextFactory.buildServerContext(TlsConfig.fromConfig("path.to.your.tls.settings", config));

And most of the remaining code in that function can be eliminated.

[Farhie]

@pdex

1. Totally agree. However, this should be seen as a stepping stone towards not having to rely on system installed certs.
2. Agreed. Please see the initial comment for why we are not currently utilising xio
3. Again, I would expect us to add these configurations to TLS config.

That change seems straight forward. So if this is the easiest way to progress and for us to get mutual auth. Then that's fine by me. I would invite you to refactor/extend as you see fit 👍

[andyday]

I agree with @pdex assessment. I also understand that you have limited bandwidth. If his comments cannot be addressed at this time, can you open an issue for them to be addressed in the near future?

[Farhie]

Feel free to close this PR.