risdenk commented on a change in pull request #69: KNOX-1812 - The Knox Gateway
truststore should be configurable
URL: https://github.com/apache/knox/pull/69#discussion_r264345382
##########
File path:
gateway-spi/src/main/java/org/apache/knox/gateway/dispatch/DefaultHttpClientFactory.java
##########
@@ -79,26 +77,21 @@ public HttpClient createHttpClient(FilterConfig
filterConfig) {
builder = HttpClients.custom();
}
if (Boolean.parseBoolean(filterConfig.getInitParameter("useTwoWaySsl"))) {
- char[] keypass = null;
- MasterService ms = services.getService(GatewayServices.MASTER_SERVICE);
AliasService as = services.getService(GatewayServices.ALIAS_SERVICE);
- try {
- keypass = as.getGatewayIdentityPassphrase();
- } catch (AliasServiceException e) {
- // nop - default passphrase will be used
- }
- if (keypass == null) {
- // there has been no alias created for the key - let's assume it is
the same as the keystore password
- keypass = ms.getMasterSecret();
- }
-
KeystoreService ks =
services.getService(GatewayServices.KEYSTORE_SERVICE);
final SSLContext sslcontext;
try {
- KeyStore keystoreForGateway = ks.getKeystoreForGateway();
+ KeyStore identityKeystore = ks.getKeystoreForGateway();
+ char[] identityKeyPassphrase = as.getGatewayIdentityPassphrase();
+
+ // The trustKeystore will be the same as the identityKeystore if a
truststore was not explicitly
+ // configured in gateway-site (gateway.truststore.password.alias,
gateway.truststore.path, gateway.truststore.type)
+ // This was the behavior before KNOX-1812
+ KeyStore trustKeystore = ks.getTruststoreForHttpClient();
+
sslcontext = SSLContexts.custom()
- .loadTrustMaterial(keystoreForGateway, new
TrustSelfSignedStrategy())
- .loadKeyMaterial(keystoreForGateway, keypass)
+ .loadTrustMaterial(trustKeystore, new TrustSelfSignedStrategy())
+ .loadKeyMaterial(identityKeystore, identityKeyPassphrase)
Review comment:
The below comment has nothing to do with this change, but this change is in
this area so commenting about it.
So I'm not sure the logic behind `loadTrustMaterial` and `loadKeyMaterial`
is sound. Here are the few scenarios that I'm not sure work:
* If we have a chain of certificates in the truststore, what happens? I am
not sure but this might only trust if there is a single certificate in the
chain?
* If there are multiple certificates in the keystore, do we need to specify
an alias to use? I think if you have multiple aliases that match it picks the
first one in the keystore.
Again the above behavior didn't change as part of this PR, but would be good
to follow up to make sure the scenarios aren't a problem.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
