Ecwid / consul-api

Java client for Consul HTTP API
Apache License 2.0
417 stars 175 forks source link

DefaultHttpsTransport #129

Open domelz opened 7 years ago

domelz commented 7 years ago

com.ecwid.consul.transport.DefaultHttpsTransport currently requires both a key and trust store.

Should DefaultHttpsTransport be updated to only require the trust store and make the key store optional?

newmodelcoder commented 6 years ago

It's actually possible to use DefaultHttpTransport to do SSL - no key store required - as long as you are able to use standard system properties javax.net.ssl.trustStore and javax.net.ssl.trustStorePassword.

Looks like Apache HttpClient is used under the hood, which honors those system properties. The only trick is to include the scheme https in the agentHost string when creating a ConsulClient. E.g., for the constructor https://github.com/Ecwid/consul-api/blob/e3b4e3ac62171864d86b19545499128d37676a2d/src/main/java/com/ecwid/consul/v1/ConsulClient.java#L139, I had to use an agent host of https://127.0.0.1, and it worked when the system properties were set. No TLSConfig instance required.

The presence of the DefaultHttpTransport and DefaultHttpsTransport threw me off. The default constructor for the former will create an HttpClient that will handle BOTH http and https (using system properties for the truststore). The latter class is not required for https.

I was actually trying to do this with Spring Boot/Spring Cloud Consul, which has a pending PR to expose the TLSConfig properties, but that isn't required to get SSL working - I can just add the scheme to the host string and set the system properties.

It may be useful to others to document this a little more explicitly so people know they don't necessarily need a TLSConfig to enable TLS.