Repository: SAP/cloud-security-xsuaa-integration (java)

Issue:
[token-client] extend SSLContextFactory with KeyStore getter method
For a case when application provides custom implementation for `HttpClientFactory` interface, it would be helpful to have access to `KeyStore` object to reduce code duplications for certificate and key parsing when initialising custom `SSLContext`.

Proposal to add public method `getKeyStore` to [SSLContextFactory](https://github.com/SAP/cloud-security-xsuaa-integration/blob/master/token-client/src/main/java/com/sap/cloud/security/mtls/SSLContextFactory.java) which would resolve accordingly the `certificate` and `key` attributes from `ClientIdentity` class and initialize the `KeyStore`.
```java
public KeyStore getKeyStore(ClientIdentity clientIdentity){...}
```

Pull request:
SSLContextFactory.createKeyStore(ClientIdentity)
fixes #621  (cloud sdk HttpDestinationProperties)

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index fe1c93d8..69d2222f 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,7 +2,8 @@
 All notable changes to this project will be documented in this file.
 
 ## 2.10.5
-- [token-client] 
+- [token-client]
+  - new method `SSLContextFactory.createKeyStore(ClientIdentity)`
   - `XsuaaTokenFlows` constructor accepts `com.sap.cloud.security.xsuaa.client.ClientCredentials` as argument.
   
 #### Dependency upgrades
diff --git a/env/pom.xml b/env/pom.xml
index 0f7f4019..5f6e09c3 100644
--- a/env/pom.xml
+++ b/env/pom.xml
@@ -1,5 +1,6 @@
 <?xml version="1.0" encoding="UTF-8"?>
-
+<!-- SPDX-FileCopyrightText: 2018-2021 SAP SE or an SAP affiliate company and Cloud Security Client Java contributors -->
+<!-- SPDX-License-Identifier: Apache-2.0 -->
 <project
         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd"
         xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance">
diff --git a/token-client/src/main/java/com/sap/cloud/security/client/DefaultHttpClientFactory.java b/token-client/src/main/java/com/sap/cloud/security/client/DefaultHttpClientFactory.java
index e63fb252..17b55f27 100644
--- a/token-client/src/main/java/com/sap/cloud/security/client/DefaultHttpClientFactory.java
+++ b/token-client/src/main/java/com/sap/cloud/security/client/DefaultHttpClientFactory.java
@@ -28,13 +28,10 @@ public class DefaultHttpClientFactory implements HttpClientFactory {
 	public CloseableHttpClient createClient(ClientIdentity clientIdentity) throws HttpClientException {
 		LOGGER.warn("In productive environment provide well configured HttpClientFactory service");
 		if (clientIdentity != null && clientIdentity.isCertificateBased()) {
-			LOGGER.debug("Setting up HTTPS client with: certificate: {}\nprivate key: {}\n",
-					clientIdentity.getCertificate(), clientIdentity.getKey());
-
+			LOGGER.debug("Setting up HTTPS client with: certificate: {}\n", clientIdentity.getCertificate());
 			SSLContext sslContext;
 			try {
-				sslContext = SSLContextFactory.getInstance().create(clientIdentity.getCertificate(),
-						clientIdentity.getKey());
+				sslContext = SSLContextFactory.getInstance().create(clientIdentity);
 			} catch (IOException | GeneralSecurityException e) {
 				throw new HttpClientException(
 						String.format("Couldn't set up https client for service provider. %s.",
diff --git a/token-client/src/main/java/com/sap/cloud/security/mtls/SSLContextFactory.java b/token-client/src/main/java/com/sap/cloud/security/mtls/SSLContextFactory.java
index ac502008..6d91b9cf 100644
--- a/token-client/src/main/java/com/sap/cloud/security/mtls/SSLContextFactory.java
+++ b/token-client/src/main/java/com/sap/cloud/security/mtls/SSLContextFactory.java
@@ -5,6 +5,9 @@
  */
 package com.sap.cloud.security.mtls;
 
+import com.sap.cloud.security.config.ClientCertificate;
+import com.sap.cloud.security.config.ClientIdentity;
+import com.sap.cloud.security.config.OAuth2ServiceConfiguration;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -23,6 +26,7 @@ import java.util.Base64;
 import java.util.Collection;
 
 import static com.sap.cloud.security.xsuaa.Assertions.assertHasText;
+import static com.sap.cloud.security.xsuaa.Assertions.assertNotNull;
 
 /**
  * Creates a SSLContext (without Bouncy Castle crypto lib).
@@ -46,9 +50,9 @@ public class SSLContextFactory {
 	 * order to support mutual TLS.
 	 *
 	 * @param x509Certificates
-	 *            you can get from your Service Configuration
+	 *            you can get from your Service Configuration {@link OAuth2ServiceConfiguration#getClientIdentity()}
 	 * @param rsaPrivateKey
-	 *            you can get from your Service Configuration
+	 *            you can get from your Service Configuration{@link OAuth2ServiceConfiguration#getClientIdentity()}
 	 * @return a new SSLContext instance
 	 * @throws GeneralSecurityException
 	 *             in case of key parsing errors
@@ -60,21 +64,57 @@ public class SSLContextFactory {
 		assertHasText(x509Certificates, "x509Certificate is required");
 		assertHasText(rsaPrivateKey, "rsaPrivateKey is required");
 
-		SSLContext sslContext = createDefaultSSLContext();
-
-		PrivateKey privateKey = getPrivateKeyFromString(rsaPrivateKey);
-		Certificate[] certificateChain = getCertificatesFromString(x509Certificates);
+		return create(new ClientCertificate(x509Certificates, rsaPrivateKey, null));
+	}
 
-		KeyStore keystore = initializeKeyStore(privateKey, certificateChain);
+	/**
+	 * Creates a SSLContext which can be used to parameterize your Rest client, in
+	 * order to support mutual TLS.
+	 *
+	 * @param clientIdentity
+	 *            you can get from your Service Configuration {@link OAuth2ServiceConfiguration#getClientIdentity()}
+	 * @return a new SSLContext instance
+	 * @throws GeneralSecurityException
+	 *             in case of key parsing errors
+	 * @throws IOException
+	 *             in case of KeyStore initialization errors
+	 */
+	public SSLContext create(ClientIdentity clientIdentity) throws GeneralSecurityException, IOException {
+		assertNotNull(clientIdentity, "clientIdentity must not be null");
+		assertHasText(clientIdentity.getCertificate(), "clientIdentity.getCertificate() must not return null");
+		assertHasText(clientIdentity.getKey(), "clientIdentity.getKey() must not return null");
 
+		KeyStore keystore = createKeyStore(clientIdentity);
 		KeyManagerFactory keyManagerFactory = KeyManagerFactory.getInstance("SunX509");
 		keyManagerFactory.init(keystore, noPassword);
-
+		SSLContext sslContext = createDefaultSSLContext();
 		sslContext.init(keyManagerFactory.getKeyManagers(), null, null);
-
 		return sslContext;
 	}
 
+	/**
+	 * Initializes a KeyStore which can be used to parameterize your Rest client, in
+	 * order to support mutual TLS.
+	 *
+	 * @param clientIdentity
+	 *            you can get from your Service Configuration {@link OAuth2ServiceConfiguration#getClientIdentity()}
+	 * @return a new KeyStore instance
+	 * @throws GeneralSecurityException
+	 *             in case of key parsing errors
+	 * @throws IOException
+	 *             in case of KeyStore initialization errors
+	 */
+	public KeyStore createKeyStore(ClientIdentity clientIdentity) throws GeneralSecurityException, IOException {
+		assertNotNull(clientIdentity, "clientIdentity must not be null");
+		assertHasText(clientIdentity.getCertificate(), "clientIdentity.getCertificate() must not return null");
+		assertHasText(clientIdentity.getKey(), "clientIdentity.getKey() must not return null");
+
+		PrivateKey privateKey = getPrivateKeyFromString(clientIdentity.getKey());
+		Certificate[] certificateChain = getCertificatesFromString(clientIdentity.getCertificate());
+
+		return initializeKeyStore(privateKey, certificateChain);
+	}
+
 	private KeyStore initializeKeyStore(PrivateKey privateKey, Certificate[] certificateChain)
 			throws KeyStoreException, IOException, NoSuchAlgorithmException, CertificateException {
 		KeyStore keystore = KeyStore.getInstance("jks");
@@ -146,7 +186,6 @@ public class SSLContextFactory {
 					primeExponentQ,
 					crtCoefficient);
 		} catch (IOException e) {
-			logger.error("Exception during parsing DER encoded private key ({})", e.getMessage());
 			throw new GeneralSecurityException("Exception during parsing DER encoded private key", e);
 		}
 		return keySpec;


Test patch:
diff --git a/token-client/src/test/java/com/sap/cloud/security/mtls/SSLContextFactoryTest.java b/token-client/src/test/java/com/sap/cloud/security/mtls/SSLContextFactoryTest.java
index 59b13302..b141e868 100644
--- a/token-client/src/test/java/com/sap/cloud/security/mtls/SSLContextFactoryTest.java
+++ b/token-client/src/test/java/com/sap/cloud/security/mtls/SSLContextFactoryTest.java
@@ -14,6 +14,8 @@ import java.io.IOException;
 import java.nio.charset.StandardCharsets;
 import java.security.GeneralSecurityException;
 
+import com.sap.cloud.security.config.ClientCertificate;
+import com.sap.cloud.security.config.ClientCredentials;
 import com.sap.cloud.security.mtls.SSLContextFactory;
 import org.apache.commons.io.IOUtils;
 import org.junit.Before;
@@ -43,6 +45,18 @@ public class SSLContextFactoryTest {
 		assertThatThrownBy(() -> {
 			cut.create(certificates, null);
 		}).isInstanceOf(IllegalArgumentException.class).hasMessageStartingWith("rsaPrivateKey");
+
+		assertThatThrownBy(() -> {
+			cut.create(null);
+		}).isInstanceOf(IllegalArgumentException.class).hasMessageStartingWith("clientIdentity");
+
+		assertThatThrownBy(() -> {
+			cut.create(new ClientCredentials("clientId", "clientSecret"));
+		}).isInstanceOf(IllegalArgumentException.class).hasMessageStartingWith("clientIdentity.getCertificate()");
+
+		assertThatThrownBy(() -> {
+			cut.create(new ClientCertificate("certificate", null, null));
+		}).isInstanceOf(IllegalArgumentException.class).hasMessageStartingWith("clientIdentity.getKey()");
 	}
 
 	@Test
@@ -54,7 +68,23 @@ public class SSLContextFactoryTest {
 		assertThat(cut.create(certificates, rsaPrivateKey), is(notNullValue()));
 	}
 
-	public String readFromFile(String file) throws IOException {
+	@Test
+	public void createKeyStore() throws GeneralSecurityException, IOException {
+		assertThat(cut.createKeyStore(new ClientCertificate(certificates, rsaPrivateKey, null)), is(notNullValue()));
+	}
+
+	@Test
+	public void createKeyStore_throwsOnNullValues() {
+		assertThatThrownBy(() -> {
+			cut.createKeyStore(new ClientCredentials("clientId", "clientSecret"));
+		}).isInstanceOf(IllegalArgumentException.class).hasMessageStartingWith("clientIdentity.getCertificate()");
+
+		assertThatThrownBy(() -> {
+			cut.createKeyStore(new ClientCertificate("certificate", null, null));
+		}).isInstanceOf(IllegalArgumentException.class).hasMessageStartingWith("clientIdentity.getKey()");
+	}
+
+	private String readFromFile(String file) throws IOException {
 		return IOUtils.resourceToString(file, StandardCharsets.UTF_8);
 	}