Skip to content

Commit 3b73a4a

Browse files
rjrudinrjrudin
committed
Refactored logic for determining SSL inputs
I was looking at this while analyzing support for 2-way SSL and I couldn't really understand what I wrote. So I refactored in the following way: 1. All construction for SSLContext/TrustManager is in buildSSLInputs. 2. There are 4 approaches, each clearly identified and implemented in its own method. 3. I moved the tests I use for verifying SSL support to a "test.ssl" package (no changes to the tests themselves). This may actually fix a bug but I'm not certain. The bug would have been that "newCertificateAuthContext" was called before all the SSL-input logic had occurred, meaning that e.g. "default" as an sslProtocol value would not have impacted certificate authentication.
1 parent 4bfb61b commit 3b73a4a

File tree

5 files changed

+105
-92
lines changed

5 files changed

+105
-92
lines changed

‎marklogic-client-api/src/main/java/com/marklogic/client/impl/DatabaseClientPropertySource.java‎

Lines changed: 90 additions & 88 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,6 @@
1919
import com.marklogic.client.DatabaseClientBuilder;
2020
import com.marklogic.client.DatabaseClientFactory;
2121
import com.marklogic.client.extra.okhttpclient.RemoveAcceptEncodingConfigurator;
22-
import org.slf4j.Logger;
23-
import org.slf4j.LoggerFactory;
2422

2523
import javax.net.ssl.SSLContext;
2624
import javax.net.ssl.X509TrustManager;
@@ -40,7 +38,6 @@
4038
*/
4139
public class DatabaseClientPropertySource {
4240

43-
private static final Logger logger = LoggerFactory.getLogger(DatabaseClientPropertySource.class);
4441
private static final String PREFIX = DatabaseClientBuilder.PREFIX;
4542

4643
private final Function<String, Object> propertySource;
@@ -97,7 +94,7 @@ public class DatabaseClientPropertySource {
9794
if (value instanceof Boolean && Boolean.TRUE.equals(value)) {
9895
disableGzippedResponses = true;
9996
} else if (value instanceof String) {
100-
disableGzippedResponses = Boolean.parseBoolean((String)value);
97+
disableGzippedResponses = Boolean.parseBoolean((String) value);
10198
}
10299
if (disableGzippedResponses) {
103100
DatabaseClientFactory.addConfigurator(new RemoveAcceptEncodingConfigurator());
@@ -152,20 +149,13 @@ private DatabaseClientFactory.SecurityContext newSecurityContext() {
152149
if (typeValue == null || !(typeValue instanceof String)) {
153150
throw new IllegalArgumentException("Security context should be set, or auth type must be of type String");
154151
}
155-
final String authType = (String)typeValue;
156-
final SSLInputs sslInputs = buildSSLInputs(authType);
152+
final String authType = (String) typeValue;
157153

154+
final SSLInputs sslInputs = buildSSLInputs(authType);
158155
DatabaseClientFactory.SecurityContext securityContext = newSecurityContext(authType, sslInputs);
159-
160-
X509TrustManager trustManager = determineTrustManager(sslInputs);
161-
SSLContext sslContext = sslInputs.getSslContext() != null ?
162-
sslInputs.getSslContext() :
163-
determineSSLContext(sslInputs, trustManager);
164-
165-
if (sslContext != null) {
166-
securityContext.withSSLContext(sslContext, trustManager);
156+
if (sslInputs.getSslContext() != null) {
157+
securityContext.withSSLContext(sslInputs.getSslContext(), sslInputs.getTrustManager());
167158
}
168-
169159
securityContext.withSSLHostnameVerifier(determineHostnameVerifier());
170160
return securityContext;
171161
}
@@ -202,7 +192,7 @@ private String getNullableStringValue(String propertyName) {
202192
if (value != null && !(value instanceof String)) {
203193
throw new IllegalArgumentException(propertyName + " must be of type String");
204194
}
205-
return (String)value;
195+
return (String) value;
206196
}
207197

208198
private DatabaseClientFactory.SecurityContext newBasicAuthContext() {
@@ -255,57 +245,6 @@ private DatabaseClientFactory.SecurityContext newSAMLAuthContext() {
255245
return new DatabaseClientFactory.SAMLAuthContext(getRequiredStringValue("saml.token"));
256246
}
257247

258-
private SSLContext determineSSLContext(SSLInputs sslInputs, X509TrustManager trustManager) {
259-
String protocol = sslInputs.getSslProtocol();
260-
if (protocol != null) {
261-
if ("default".equalsIgnoreCase(protocol)) {
262-
try {
263-
return SSLContext.getDefault();
264-
} catch (NoSuchAlgorithmException e) {
265-
throw new RuntimeException("Unable to obtain default SSLContext; cause: " + e.getMessage(), e);
266-
}
267-
}
268-
269-
SSLContext sslContext;
270-
try {
271-
sslContext = SSLContext.getInstance(protocol);
272-
} catch (NoSuchAlgorithmException e) {
273-
throw new RuntimeException("Unable to get SSLContext instance with protocol: " + protocol
274-
+ "; cause: " + e.getMessage(), e);
275-
}
276-
// Note that if only a protocol is specified, and not a TrustManager, an attempt will later be made
277-
// to use the JVM's default TrustManager
278-
if (trustManager != null) {
279-
try {
280-
sslContext.init(null, new X509TrustManager[]{trustManager}, null);
281-
} catch (KeyManagementException e) {
282-
throw new RuntimeException("Unable to initialize SSLContext; protocol: " + protocol + "; cause: " + e.getMessage(), e);
283-
}
284-
}
285-
return sslContext;
286-
}
287-
return null;
288-
}
289-
290-
private X509TrustManager determineTrustManager(SSLInputs sslInputs) {
291-
if (sslInputs.getTrustManager() != null) {
292-
return sslInputs.getTrustManager();
293-
}
294-
// If the user chooses the "default" SSLContext, then it's already been initialized - but OkHttp still
295-
// needs a separate X509TrustManager, so use the JVM's default trust manager. The assumption is that the
296-
// default SSLContext was initialized with the JVM's default trust manager. A user can of course always override
297-
// this by simply providing their own trust manager.
298-
if ("default".equalsIgnoreCase(sslInputs.getSslProtocol())) {
299-
X509TrustManager defaultTrustManager = SSLUtil.getDefaultTrustManager();
300-
if (logger.isDebugEnabled() && defaultTrustManager != null && defaultTrustManager.getAcceptedIssuers() != null) {
301-
logger.debug("Count of accepted issuers in default trust manager: {}",
302-
defaultTrustManager.getAcceptedIssuers().length);
303-
}
304-
return defaultTrustManager;
305-
}
306-
return null;
307-
}
308-
309248
private DatabaseClientFactory.SSLHostnameVerifier determineHostnameVerifier() {
310249
Object verifierObject = propertySource.apply(PREFIX + "sslHostnameVerifier");
311250
if (verifierObject instanceof DatabaseClientFactory.SSLHostnameVerifier) {
@@ -329,61 +268,124 @@ private DatabaseClientFactory.SSLHostnameVerifier determineHostnameVerifier() {
329268
* X509TrustManager.
330269
*
331270
* @param authType used for applying "default" as the SSL protocol for MarkLogic cloud authentication in
332-
* case the user does not define their own SSLContext or SSL protocol
271+
* case the user does not define their own SSLContext or SSL protocol
333272
* @return
334273
*/
335274
private SSLInputs buildSSLInputs(String authType) {
336-
SSLContext sslContext = null;
275+
X509TrustManager userTrustManager = getTrustManager();
276+
277+
// Approach 1 - user provides an SSLContext object, in which case there's nothing further to check.
278+
SSLContext sslContext = getSSLContext();
279+
if (sslContext != null) {
280+
return new SSLInputs(sslContext, userTrustManager);
281+
}
282+
283+
// Approaches 2 and 3 - user defines an SSL protocol.
284+
// Approach 2 - "default" is a convenience for using the JVM's default SSLContext.
285+
// Approach 3 - create a new SSLContext, and initialize it if the user-provided TrustManager is not null.
286+
final String sslProtocol = getSSLProtocol(authType);
287+
if (sslProtocol != null) {
288+
return "default".equalsIgnoreCase(sslProtocol) ?
289+
useDefaultSSLContext(userTrustManager) :
290+
useNewSSLContext(sslProtocol, userTrustManager);
291+
}
292+
293+
// Approach 4 - no SSL connection is needed.
294+
return new SSLInputs(null, null);
295+
}
296+
297+
private X509TrustManager getTrustManager() {
298+
Object val = propertySource.apply(PREFIX + "trustManager");
299+
if (val != null) {
300+
if (val instanceof X509TrustManager) {
301+
return (X509TrustManager) val;
302+
} else {
303+
throw new IllegalArgumentException("Trust manager must be an instanceof " + X509TrustManager.class.getName());
304+
}
305+
}
306+
return null;
307+
}
308+
309+
private SSLContext getSSLContext() {
337310
Object val = propertySource.apply(PREFIX + "sslContext");
338311
if (val != null) {
339312
if (val instanceof SSLContext) {
340-
sslContext = (SSLContext) val;
313+
return (SSLContext) val;
341314
} else {
342315
throw new IllegalArgumentException("SSL context must be an instanceof " + SSLContext.class.getName());
343316
}
344317
}
318+
return null;
319+
}
345320

321+
private String getSSLProtocol(String authType) {
346322
String sslProtocol = getNullableStringValue("sslProtocol");
347-
if (sslContext == null &&
348-
(sslProtocol == null || sslProtocol.trim().length() == 0) &&
349-
DatabaseClientBuilder.AUTH_TYPE_MARKLOGIC_CLOUD.equalsIgnoreCase(authType)) {
323+
if (sslProtocol != null) {
324+
sslProtocol = sslProtocol.trim();
325+
}
326+
// For convenience for MarkLogic Cloud users, assume the JVM's default SSLContext should trust the certificate
327+
// used by MarkLogic Cloud. A user can always override this default behavior by providing their own SSLContext.
328+
if ((sslProtocol == null || sslProtocol.length() == 0) && DatabaseClientBuilder.AUTH_TYPE_MARKLOGIC_CLOUD.equalsIgnoreCase(authType)) {
350329
sslProtocol = "default";
351330
}
331+
return sslProtocol;
332+
}
333+
334+
/**
335+
* Uses the JVM's default SSLContext. Because OkHttp requires a separate TrustManager, this approach will either
336+
* user the user-provided TrustManager or it will assume that the JVM's default TrustManager should be used.
337+
*/
338+
private SSLInputs useDefaultSSLContext(X509TrustManager userTrustManager) {
339+
SSLContext sslContext;
340+
try {
341+
sslContext = SSLContext.getDefault();
342+
} catch (NoSuchAlgorithmException e) {
343+
throw new RuntimeException("Unable to obtain default SSLContext; cause: " + e.getMessage(), e);
344+
}
345+
X509TrustManager trustManager = userTrustManager != null ? userTrustManager : SSLUtil.getDefaultTrustManager();
346+
return new SSLInputs(sslContext, trustManager);
347+
}
352348

353-
val = propertySource.apply(PREFIX + "trustManager");
354-
X509TrustManager trustManager = null;
355-
if (val != null) {
356-
if (val instanceof X509TrustManager) {
357-
trustManager = (X509TrustManager) val;
358-
} else {
359-
throw new IllegalArgumentException("Trust manager must be an instanceof " + X509TrustManager.class.getName());
349+
/**
350+
* Constructs a new SSLContext based on the given protocol (e.g. TLSv1.2). The SSLContext will be initialized if
351+
* the user's TrustManager is not null. Otherwise, OkHttpUtil will eventually initialize the SSLContext using the
352+
* JVM's default TrustManager.
353+
*/
354+
private SSLInputs useNewSSLContext(String sslProtocol, X509TrustManager userTrustManager) {
355+
SSLContext sslContext;
356+
try {
357+
sslContext = SSLContext.getInstance(sslProtocol);
358+
} catch (NoSuchAlgorithmException e) {
359+
throw new RuntimeException(String.format("Unable to get SSLContext instance with protocol: %s; cause: %s",
360+
sslProtocol, e.getMessage()), e);
361+
}
362+
if (userTrustManager != null) {
363+
try {
364+
sslContext.init(null, new X509TrustManager[]{userTrustManager}, null);
365+
} catch (KeyManagementException e) {
366+
throw new RuntimeException(String.format("Unable to initialize SSLContext; protocol: %s; cause: %s",
367+
sslProtocol, e.getMessage()), e);
360368
}
361369
}
362-
return new SSLInputs(sslContext, sslProtocol, trustManager);
370+
return new SSLInputs(sslContext, userTrustManager);
363371
}
364372

365373
/**
366374
* Captures the inputs provided by the caller that pertain to constructing an SSLContext.
367375
*/
368376
private static class SSLInputs {
369377
private final SSLContext sslContext;
370-
private final String sslProtocol;
371378
private final X509TrustManager trustManager;
372379

373-
public SSLInputs(SSLContext sslContext, String sslProtocol, X509TrustManager trustManager) {
380+
public SSLInputs(SSLContext sslContext, X509TrustManager trustManager) {
374381
this.sslContext = sslContext;
375-
this.sslProtocol = sslProtocol;
376382
this.trustManager = trustManager;
377383
}
378384

379385
public SSLContext getSslContext() {
380386
return sslContext;
381387
}
382388

383-
public String getSslProtocol() {
384-
return sslProtocol;
385-
}
386-
387389
public X509TrustManager getTrustManager() {
388390
return trustManager;
389391
}

‎marklogic-client-api/src/main/java/com/marklogic/client/impl/SSLUtil.java‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,9 @@
1515
*/
1616
package com.marklogic.client.impl;
1717

18+
import org.slf4j.Logger;
19+
import org.slf4j.LoggerFactory;
20+
1821
import javax.net.ssl.TrustManager;
1922
import javax.net.ssl.TrustManagerFactory;
2023
import javax.net.ssl.X509TrustManager;
@@ -25,7 +28,12 @@
2528
public interface SSLUtil {
2629

2730
static X509TrustManager getDefaultTrustManager() {
28-
return (X509TrustManager) getDefaultTrustManagers()[0];
31+
X509TrustManager trustManager = (X509TrustManager) getDefaultTrustManagers()[0];
32+
Logger logger = LoggerFactory.getLogger(SSLUtil.class);
33+
if (logger.isDebugEnabled() && trustManager.getAcceptedIssuers() != null) {
34+
logger.debug("Count of accepted issuers in default trust manager: {}", trustManager.getAcceptedIssuers().length);
35+
}
36+
return trustManager;
2937
}
3038

3139
/**

‎marklogic-client-api/src/test/java/com/marklogic/client/test/CheckSSLConnectionTest.java‎ renamed to ‎marklogic-client-api/src/test/java/com/marklogic/client/test/ssl/CheckSSLConnectionTest.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
1-
package com.marklogic.client.test;
1+
package com.marklogic.client.test.ssl;
22

33
import com.marklogic.client.DatabaseClient;
44
import com.marklogic.client.DatabaseClientFactory;
55
import com.marklogic.client.ForbiddenUserException;
66
import com.marklogic.client.MarkLogicIOException;
7+
import com.marklogic.client.test.Common;
78
import com.marklogic.client.test.junit5.RequireSSLExtension;
89
import org.junit.jupiter.api.Test;
910
import org.junit.jupiter.api.extension.ExtendWith;

‎marklogic-client-api/src/test/java/com/marklogic/client/test/SSLTest.java‎ renamed to ‎marklogic-client-api/src/test/java/com/marklogic/client/test/ssl/SSLTest.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,13 +13,14 @@
1313
* See the License for the specific language governing permissions and
1414
* limitations under the License.
1515
*/
16-
package com.marklogic.client.test;
16+
package com.marklogic.client.test.ssl;
1717

1818
import com.marklogic.client.DatabaseClient;
1919
import com.marklogic.client.DatabaseClientFactory.SSLHostnameVerifier;
2020
import com.marklogic.client.MarkLogicIOException;
2121
import com.marklogic.client.document.TextDocumentManager;
2222
import com.marklogic.client.io.StringHandle;
23+
import com.marklogic.client.test.Common;
2324
import org.junit.jupiter.api.Test;
2425

2526
import javax.net.ssl.*;

‎marklogic-client-api/src/test/java/com/marklogic/client/test/TwoWaySSLTest.java‎ renamed to ‎marklogic-client-api/src/test/java/com/marklogic/client/test/ssl/TwoWaySSLTest.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
package com.marklogic.client.test;
1+
package com.marklogic.client.test.ssl;
22

33
import com.fasterxml.jackson.databind.node.ObjectNode;
44
import com.marklogic.client.DatabaseClient;
@@ -8,6 +8,7 @@
88
import com.marklogic.client.document.DocumentDescriptor;
99
import com.marklogic.client.eval.EvalResultIterator;
1010
import com.marklogic.client.io.StringHandle;
11+
import com.marklogic.client.test.Common;
1112
import com.marklogic.client.test.junit5.RequireSSLExtension;
1213
import com.marklogic.mgmt.ManageClient;
1314
import com.marklogic.mgmt.resource.appservers.ServerManager;

0 commit comments

Comments
 (0)