Hermes supports JRE truststore #1196
Conversation
jewertow
changed the title
| @@ -122,18 +122,30 @@ | |||
| FRONTEND_SSL_PORT("frontend.ssl.port", 8443), | |||
| FRONTEND_SSL_CLIENT_AUTH_MODE("frontend.ssl.client.auth.mode", "not_requested"), | |||
| FRONTEND_SSL_PROTOCOL("frontend.ssl.protocol", "TLS"), | |||
|
|
|||
| FRONTEND_SSL_KEYSTORE_DEFAULT_JVM("frontend.ssl.keystore.default.jvm", true), | |||
There was a problem hiding this comment.
I am not sure whether this name is comprehensible.
Maybe frontend.ssl.keystore.jvm.based would be more appropriate...?
There was a problem hiding this comment.
I wonder If this should be a string type property (enum-like), say:
frontend.ssl.keystore.source, with default value equal to jvmDefault (the other option would be provided).
In this case there would be no possibility for empty value (both are false), plus no need for guessing which option has higher priority when both are set to true.
There was a problem hiding this comment.
@dankraw since we load trust store from JRE path maybe {consumer,frontend}.ssl.{key,trust}store.source=jre would be better name for this property?
| FRONTEND_SSL_TRUSTSTORE_LOCATION("frontend.ssl.truststore.location", "classpath:server.truststore"), | ||
| FRONTEND_SSL_TRUSTSTORE_PASSWORD("frontend.ssl.truststore.password", "password"), | ||
| FRONTEND_SSL_TRUSTSTORE_FORMAT("frontend.ssl.truststore.format", "JKS"), | ||
|
|
||
| CONSUMER_SSL_ENABLED("consumer.ssl.enabled", false), | ||
| CONSUMER_SSL_PROTOCOL("consumer.ssl.protocol", "TLS"), | ||
| CONSUMER_SSL_ENABLED("consumer.ssl.enabled", true), |
There was a problem hiding this comment.
Since property consumer.http2.enabled is enabled by default it should also be enabled.
hermes-common/src/main/java/pl/allegro/tech/hermes/common/config/Configs.java
Outdated
Show resolved
Hide resolved
| HttpClient client = new HttpClient(createSslContextFactory()); | ||
| HttpClient client = sslContextFactoryProvider.provideSslContextFactory() | ||
| .map(sslContextFactory -> new HttpClient(sslContextFactory)) | ||
| .orElseGet(() -> new HttpClient()); |
There was a problem hiding this comment.
It could be simplified to:
.map(HttpClient::new)
.orElseGet(HttpClient::new);
but I think it would be less understandable.
What is your opinion?
There was a problem hiding this comment.
I prefer the committed version. :)
| HttpClient client = new HttpClient(transport, createSslContextFactory()); | ||
| HttpClient client = sslContextFactoryProvider.provideSslContextFactory() | ||
| .map(sslContextFactory -> new HttpClient(transport, sslContextFactory)) | ||
| .orElseThrow(() -> new IllegalStateException("Cannot create http/2 client due to lack of ssl context factory")); |
There was a problem hiding this comment.
It is reasonable behaviour because http/2 client should be created only when both ssl and http/2 are enabled.
| @@ -67,27 +62,6 @@ public void shouldPublishUsingOkHttpClient() { | |||
| runTestSuiteForHermesClient(client); | |||
| } | |||
|
|
|||
| @Test | |||
| public void shouldPublishUsingOkHttpClientUsingSSL() { | |||
There was a problem hiding this comment.
I don't see any difference between this test and shouldCommunicateWithHermesUsingHttp2. I decided to remove it.
jewertow
changed the title
Background context
Hermes documentation says that hermes-consumers use JRE trust store to verify the ssl certificates, but in fact hermes-consumers use provided key store and trust store. It should work also with JRE trust store (
$JAVA_HOME/jre/lib/security/cacerts).Changes
consumer.ssl.enabledis enabled by defaultFrontend and Consumers allow to configure source of key store and trust store.
There are new configuration properties:
{consumers,frontend}.ssl.{key,trust}store.source- can be set tojre(default) orprovidedIn case of
providedHermes loads provided key/trust store from specified location ({consumer,frontend}.ssl{key,trust}store.location)In case of
jreHermes loads trust store from$JAVA_HOME/jre/lib/security/jssecacertsor$JAVA_HOME/jre/lib/security/cacerts.