Skip to content

Commit

Permalink
feat: add mtls support for NetHttpTransport (#1147)
Browse files Browse the repository at this point in the history
* feat: support keystore in transport for mtls

* fix format

* update code

* add tests

* update test and doc

* update names

* create keystore from cert and key string

* change certAndKey from string to inputstream

* add mtls file

* Update google-http-client/src/main/java/com/google/api/client/http/javanet/NetHttpTransport.java

Co-authored-by: Jeff Ching <chingor@google.com>

* Update google-http-client/src/main/java/com/google/api/client/http/javanet/NetHttpTransport.java

Co-authored-by: Jeff Ching <chingor@google.com>

* Update google-http-client/src/main/java/com/google/api/client/util/SslUtils.java

Co-authored-by: Jeff Ching <chingor@google.com>

* Update google-http-client/src/main/java/com/google/api/client/util/SslUtils.java

Co-authored-by: Jeff Ching <chingor@google.com>

* Update google-http-client/src/test/java/com/google/api/client/util/SecurityUtilsTest.java

Co-authored-by: Jeff Ching <chingor@google.com>

* Update google-http-client/src/main/java/com/google/api/client/util/SslUtils.java

Co-authored-by: Jeff Ching <chingor@google.com>

* update the code

* fix name

Co-authored-by: Jeff Ching <chingor@google.com>
  • Loading branch information
arithmetic1728 and chingor13 authored Oct 30, 2020
1 parent b5754a4 commit 51762f2
Show file tree
Hide file tree
Showing 9 changed files with 278 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,15 @@ public boolean supportsMethod(String method) throws IOException {
return Arrays.binarySearch(SUPPORTED_METHODS, method) >= 0;
}

/**
* Returns whether the transport is mTLS.
*
* @return boolean indicating if the transport is mTLS.
*/
public boolean isMtls() {
return false;
}

/**
* Builds a low level HTTP request for the given HTTP method.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,13 +89,16 @@ private static Proxy defaultProxy() {
/** Host name verifier or {@code null} for the default. */
private final HostnameVerifier hostnameVerifier;

/** Whether the transport is mTLS. Default value is {@code false}. */
private final boolean isMtls;

/**
* Constructor with the default behavior.
*
* <p>Instead use {@link Builder} to modify behavior.
*/
public NetHttpTransport() {
this((ConnectionFactory) null, null, null);
this((ConnectionFactory) null, null, null, false);
}

/**
Expand All @@ -104,26 +107,33 @@ public NetHttpTransport() {
* system properties</a>
* @param sslSocketFactory SSL socket factory or {@code null} for the default
* @param hostnameVerifier host name verifier or {@code null} for the default
* @param isMtls Whether the transport is mTLS. Default value is {@code false}
*/
NetHttpTransport(
Proxy proxy, SSLSocketFactory sslSocketFactory, HostnameVerifier hostnameVerifier) {
this(new DefaultConnectionFactory(proxy), sslSocketFactory, hostnameVerifier);
Proxy proxy,
SSLSocketFactory sslSocketFactory,
HostnameVerifier hostnameVerifier,
boolean isMtls) {
this(new DefaultConnectionFactory(proxy), sslSocketFactory, hostnameVerifier, isMtls);
}

/**
* @param connectionFactory factory to produce connections from {@link URL}s; if {@code null} then
* {@link DefaultConnectionFactory} is used
* @param sslSocketFactory SSL socket factory or {@code null} for the default
* @param hostnameVerifier host name verifier or {@code null} for the default
* @param isMtls Whether the transport is mTLS. Default value is {@code false}
* @since 1.20
*/
NetHttpTransport(
ConnectionFactory connectionFactory,
SSLSocketFactory sslSocketFactory,
HostnameVerifier hostnameVerifier) {
HostnameVerifier hostnameVerifier,
boolean isMtls) {
this.connectionFactory = getConnectionFactory(connectionFactory);
this.sslSocketFactory = sslSocketFactory;
this.hostnameVerifier = hostnameVerifier;
this.isMtls = isMtls;
}

private ConnectionFactory getConnectionFactory(ConnectionFactory connectionFactory) {
Expand All @@ -141,6 +151,11 @@ public boolean supportsMethod(String method) {
return Arrays.binarySearch(SUPPORTED_METHODS, method) >= 0;
}

@Override
public boolean isMtls() {
return this.isMtls;
}

@Override
protected NetHttpRequest buildRequest(String method, String url) throws IOException {
Preconditions.checkArgument(supportsMethod(method), "HTTP method %s not supported", method);
Expand Down Expand Up @@ -189,6 +204,9 @@ public static final class Builder {
*/
private ConnectionFactory connectionFactory;

/** Whether the transport is mTLS. Default value is {@code false}. */
private boolean isMtls;

/**
* Sets the HTTP proxy or {@code null} to use the proxy settings from <a
* href="http://docs.oracle.com/javase/7/docs/api/java/net/doc-files/net-properties.html">system
Expand Down Expand Up @@ -275,6 +293,33 @@ public Builder trustCertificates(KeyStore trustStore) throws GeneralSecurityExce
return setSslSocketFactory(sslContext.getSocketFactory());
}

/**
* Sets the SSL socket factory based on a root certificate trust store and a client certificate
* key store. The client certificate key store will be used to establish mutual TLS.
*
* @param trustStore certificate trust store (use for example {@link SecurityUtils#loadKeyStore}
* or {@link SecurityUtils#loadKeyStoreFromCertificates})
* @param mtlsKeyStore key store for client certificate and key to establish mutual TLS. (use
* for example {@link SecurityUtils#createMtlsKeyStore(InputStream)})
* @param mtlsKeyStorePassword password for mtlsKeyStore parameter
*/
public Builder trustCertificates(
KeyStore trustStore, KeyStore mtlsKeyStore, String mtlsKeyStorePassword)
throws GeneralSecurityException {
if (mtlsKeyStore != null && mtlsKeyStore.size() > 0) {
this.isMtls = true;
}
SSLContext sslContext = SslUtils.getTlsSslContext();
SslUtils.initSslContext(
sslContext,
trustStore,
SslUtils.getPkixTrustManagerFactory(),
mtlsKeyStore,
mtlsKeyStorePassword,
SslUtils.getDefaultKeyManagerFactory());
return setSslSocketFactory(sslContext.getSocketFactory());
}

/**
* {@link Beta} <br>
* Disables validating server SSL certificates by setting the SSL socket factory using {@link
Expand Down Expand Up @@ -319,8 +364,8 @@ public NetHttpTransport build() {
setProxy(defaultProxy());
}
return this.proxy == null
? new NetHttpTransport(connectionFactory, sslSocketFactory, hostnameVerifier)
: new NetHttpTransport(this.proxy, sslSocketFactory, hostnameVerifier);
? new NetHttpTransport(connectionFactory, sslSocketFactory, hostnameVerifier, isMtls)
: new NetHttpTransport(this.proxy, sslSocketFactory, hostnameVerifier, isMtls);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import java.io.ByteArrayInputStream;
import java.io.IOException;
import java.io.InputStream;
import java.io.InputStreamReader;
import java.security.GeneralSecurityException;
import java.security.InvalidKeyException;
import java.security.KeyFactory;
Expand All @@ -31,6 +32,7 @@
import java.security.cert.CertificateException;
import java.security.cert.CertificateFactory;
import java.security.cert.X509Certificate;
import java.security.spec.PKCS8EncodedKeySpec;
import java.util.List;
import javax.net.ssl.X509TrustManager;

Expand Down Expand Up @@ -258,5 +260,59 @@ public static void loadKeyStoreFromCertificates(
}
}

/**
* Create a keystore for mutual TLS with the certificate and private key provided.
*
* @param certAndKey Certificate and private key input stream. The stream should contain one
* certificate and one unencrypted private key. If there are multiple certificates, only the
* first certificate will be used.
* @return keystore for mutual TLS.
*/
public static KeyStore createMtlsKeyStore(InputStream certAndKey)
throws GeneralSecurityException, IOException {
KeyStore keystore = KeyStore.getInstance("JKS");
keystore.load(null);

PemReader.Section certSection = null;
PemReader.Section keySection = null;
PemReader reader = new PemReader(new InputStreamReader(certAndKey));

while (certSection == null || keySection == null) {
// Read the certificate and private key.
PemReader.Section section = reader.readNextSection();
if (section == null) {
break;
}

if (certSection == null && "CERTIFICATE".equals(section.getTitle())) {
certSection = section;
} else if ("PRIVATE KEY".equals(section.getTitle())) {
keySection = section;
}
}

if (certSection == null) {
throw new IllegalArgumentException("certificate is missing from certAndKey string");
}
if (keySection == null) {
throw new IllegalArgumentException("private key is missing from certAndKey string");
}

CertificateFactory certFactory = CertificateFactory.getInstance("X.509");
X509Certificate cert =
(X509Certificate)
certFactory.generateCertificate(
new ByteArrayInputStream(certSection.getBase64DecodedBytes()));

PKCS8EncodedKeySpec keySpecPKCS8 = new PKCS8EncodedKeySpec(keySection.getBase64DecodedBytes());
PrivateKey key =
KeyFactory.getInstance(cert.getPublicKey().getAlgorithm()).generatePrivate(keySpecPKCS8);

This comment has been minimized.

Copy link
@Capstan

Capstan Jun 18, 2022

Contributor

@sophieschmieg indicated in the monorepo, "we should use a fixed algorithm here".

This comment has been minimized.

Copy link
@Capstan

Capstan Jun 19, 2022

Contributor

Her review was triggered by KeyFactory.getInstance being flagged by errorprone's InsecureCipherMode. The problem appears to be that some algorithms, which are controlled by the caller's passed in cert's public key, have vulnerabilities, namely DH and DSA at the time of this comment.

This comment has been minimized.

Copy link
@Capstan

Capstan Jun 19, 2022

Contributor

Filed #1675 to track this.


// Fit the certificate and private key into the keystore.
keystore.setKeyEntry("alias", key, new char[] {}, new X509Certificate[] {cert});

return keystore;
}

private SecurityUtils() {}
}
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,36 @@ public static SSLContext initSslContext(
return sslContext;
}

/**
* Initializes the SSL context to the trust managers supplied by the trust manager factory for the
* given trust store, and to the key managers supplied by the key manager factory for the given
* key store.
*
* @param sslContext SSL context (for example {@link SSLContext#getInstance})
* @param trustStore key store for certificates to trust (for example {@link
* SecurityUtils#getJavaKeyStore()})
* @param trustManagerFactory trust manager factory (for example {@link
* #getPkixTrustManagerFactory()})
* @param mtlsKeyStore key store for client certificate and key to establish mutual TLS
* @param mtlsKeyStorePassword password for mtlsKeyStore parameter
* @param keyManagerFactory key manager factory (for example {@link
* #getDefaultKeyManagerFactory()})
*/
public static SSLContext initSslContext(
SSLContext sslContext,
KeyStore trustStore,
TrustManagerFactory trustManagerFactory,
KeyStore mtlsKeyStore,
String mtlsKeyStorePassword,
KeyManagerFactory keyManagerFactory)
throws GeneralSecurityException {
trustManagerFactory.init(trustStore);
keyManagerFactory.init(mtlsKeyStore, mtlsKeyStorePassword.toCharArray());
sslContext.init(
keyManagerFactory.getKeyManagers(), trustManagerFactory.getTrustManagers(), null);
return sslContext;
}

/**
* {@link Beta} <br>
* Returns an SSL context in which all X.509 certificates are trusted.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import java.io.InputStream;
import java.net.HttpURLConnection;
import java.net.URL;
import java.security.KeyStore;
import junit.framework.TestCase;

/**
Expand All @@ -36,6 +37,32 @@ public class NetHttpTransportTest extends TestCase {
"GET", "POST", "HEAD", "OPTIONS", "PUT", "DELETE", "TRACE"
};

public void testNotMtlsWithoutClientCert() throws Exception {
KeyStore trustStore = KeyStore.getInstance("JKS");

NetHttpTransport transport =
new NetHttpTransport.Builder().trustCertificates(trustStore).build();
assertFalse(transport.isMtls());
}

public void testIsMtlsWithClientCert() throws Exception {
KeyStore trustStore = KeyStore.getInstance("JKS");
KeyStore keyStore = KeyStore.getInstance("PKCS12");

// Load client certificate and private key from secret.p12 file.
keyStore.load(
this.getClass()
.getClassLoader()
.getResourceAsStream("com/google/api/client/util/secret.p12"),
"notasecret".toCharArray());

NetHttpTransport transport =
new NetHttpTransport.Builder()
.trustCertificates(trustStore, keyStore, "notasecret")
.build();
assertTrue(transport.isMtls());
}

public void testExecute_mock() throws Exception {
for (String method : METHODS) {
boolean isPutOrPost = method.equals("PUT") || method.equals("POST");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
import com.google.api.client.testing.json.webtoken.TestCertificates;
import com.google.api.client.testing.util.SecurityTestUtils;
import java.io.ByteArrayInputStream;
import java.io.InputStream;
import java.security.KeyStore;
import java.security.PrivateKey;
import java.security.Signature;
import java.security.cert.X509Certificate;
Expand Down Expand Up @@ -160,4 +162,47 @@ public void testVerifyX509() throws Exception {
public void testVerifyX509WrongCa() throws Exception {
assertNull(verifyX509(TestCertificates.BOGUS_CA_CERT));
}

public void testCreateMtlsKeyStoreNoCert() throws Exception {
final InputStream certMissing =
getClass()
.getClassLoader()
.getResourceAsStream("com/google/api/client/util/privateKey.pem");

boolean thrown = false;
try {
SecurityUtils.createMtlsKeyStore(certMissing);
fail("should have thrown");
} catch (IllegalArgumentException e) {
assertTrue(e.getMessage().contains("certificate is missing from certAndKey string"));
thrown = true;
}
assertTrue("should have caught an IllegalArgumentException", thrown);
}

public void testCreateMtlsKeyStoreNoPrivateKey() throws Exception {
final InputStream privateKeyMissing =
getClass().getClassLoader().getResourceAsStream("com/google/api/client/util/cert.pem");

boolean thrown = false;
try {
SecurityUtils.createMtlsKeyStore(privateKeyMissing);
fail("should have thrown");
} catch (IllegalArgumentException e) {
assertTrue(e.getMessage().contains("private key is missing from certAndKey string"));
thrown = true;
}
assertTrue("should have caught an IllegalArgumentException", thrown);
}

public void testCreateMtlsKeyStoreSuccess() throws Exception {
InputStream certAndKey =
getClass()
.getClassLoader()
.getResourceAsStream("com/google/api/client/util/mtlsCertAndKey.pem");

KeyStore mtlsKeyStore = SecurityUtils.createMtlsKeyStore(certAndKey);

assertEquals(1, mtlsKeyStore.size());
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
-----BEGIN CERTIFICATE-----
MIICGzCCAYSgAwIBAgIIWrt6xtmHPs4wDQYJKoZIhvcNAQEFBQAwMzExMC8GA1UE
AxMoMTAwOTEyMDcyNjg3OC5hcHBzLmdvb2dsZXVzZXJjb250ZW50LmNvbTAeFw0x
MjEyMDExNjEwNDRaFw0yMjExMjkxNjEwNDRaMDMxMTAvBgNVBAMTKDEwMDkxMjA3
MjY4NzguYXBwcy5nb29nbGV1c2VyY29udGVudC5jb20wgZ8wDQYJKoZIhvcNAQEB
BQADgY0AMIGJAoGBAL1SdY8jTUVU7O4/XrZLYTw0ON1lV6MQRGajFDFCqD2Fd9tQ
GLW8Iftx9wfXe1zuaehJSgLcyCxazfyJoN3RiONBihBqWY6d3lQKqkgsRTNZkdFJ
Wdzl/6CxhK9sojh2p0r3tydtv9iwq5fuuWIvtODtT98EgphhncQAqkKoF3zVAgMB
AAGjODA2MAwGA1UdEwEB/wQCMAAwDgYDVR0PAQH/BAQDAgeAMBYGA1UdJQEB/wQM
MAoGCCsGAQUFBwMCMA0GCSqGSIb3DQEBBQUAA4GBAD8XQEqzGePa9VrvtEGpf+R4
fkxKbcYAzqYq202nKu0kfjhIYkYSBj6gi348YaxE64yu60TVl42l5HThmswUheW4
uQIaq36JvwvsDP5Zoj5BgiNSnDAFQp+jJFBRUA5vooJKgKgMDf/r/DCOsbO6VJF1
kWwa9n19NFiV0z3m6isj
-----END CERTIFICATE-----
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
-----BEGIN CERTIFICATE-----
MIICGzCCAYSgAwIBAgIIWrt6xtmHPs4wDQYJKoZIhvcNAQEFBQAwMzExMC8GA1UE
AxMoMTAwOTEyMDcyNjg3OC5hcHBzLmdvb2dsZXVzZXJjb250ZW50LmNvbTAeFw0x
MjEyMDExNjEwNDRaFw0yMjExMjkxNjEwNDRaMDMxMTAvBgNVBAMTKDEwMDkxMjA3
MjY4NzguYXBwcy5nb29nbGV1c2VyY29udGVudC5jb20wgZ8wDQYJKoZIhvcNAQEB
BQADgY0AMIGJAoGBAL1SdY8jTUVU7O4/XrZLYTw0ON1lV6MQRGajFDFCqD2Fd9tQ
GLW8Iftx9wfXe1zuaehJSgLcyCxazfyJoN3RiONBihBqWY6d3lQKqkgsRTNZkdFJ
Wdzl/6CxhK9sojh2p0r3tydtv9iwq5fuuWIvtODtT98EgphhncQAqkKoF3zVAgMB
AAGjODA2MAwGA1UdEwEB/wQCMAAwDgYDVR0PAQH/BAQDAgeAMBYGA1UdJQEB/wQM
MAoGCCsGAQUFBwMCMA0GCSqGSIb3DQEBBQUAA4GBAD8XQEqzGePa9VrvtEGpf+R4
fkxKbcYAzqYq202nKu0kfjhIYkYSBj6gi348YaxE64yu60TVl42l5HThmswUheW4
uQIaq36JvwvsDP5Zoj5BgiNSnDAFQp+jJFBRUA5vooJKgKgMDf/r/DCOsbO6VJF1
kWwa9n19NFiV0z3m6isj
-----END CERTIFICATE-----
-----BEGIN PRIVATE KEY-----
MIICdQIBADANBgkqhkiG9w0BAQEFAASCAl8wggJbAgEAAoGBAL1SdY8jTUVU7O4/
XrZLYTw0ON1lV6MQRGajFDFCqD2Fd9tQGLW8Iftx9wfXe1zuaehJSgLcyCxazfyJ
oN3RiONBihBqWY6d3lQKqkgsRTNZkdFJWdzl/6CxhK9sojh2p0r3tydtv9iwq5fu
uWIvtODtT98EgphhncQAqkKoF3zVAgMBAAECgYB51B9cXe4yiGTzJ4pOKpHGySAy
sC1F/IjXt2eeD3PuKv4m/hL4l7kScpLx0+NJuQ4j8U2UK/kQOdrGANapB1ZbMZAK
/q0xmIUzdNIDiGSoTXGN2mEfdsEpQ/Xiv0lyhYBBPC/K4sYIpHccnhSRQUZlWLLY
lE5cFNKC9b7226mNvQJBAPt0hfCNIN0kUYOA9jdLtx7CE4ySGMPf5KPBuzPd8ty1
fxaFm9PB7B76VZQYmHcWy8rT5XjoLJHrmGW1ZvP+iDsCQQDAvnKoarPOGb5iJfkq
RrA4flf1TOlf+1+uqIOJ94959jkkJeb0gv/TshDnm6/bWn+1kJylQaKygCizwPwB
Z84vAkA0Duur4YvsPJijoQ9YY1SGCagCcjyuUKwFOxaGpmyhRPIKt56LOJqpzyno
fy8ReKa4VyYq4eZYT249oFCwMwIBAkAROPNF2UL3x5UbcAkznd1hLujtIlI4IV4L
XUNjsJtBap7we/KHJq11XRPlniO4lf2TW7iji5neGVWJulTKS1xBAkAerktk4Hsw
ErUaUG1s/d+Sgc8e/KMeBElV+NxGhcWEeZtfHMn/6VOlbzY82JyvC9OKC80A5CAE
VUV6b25kqrcu
-----END PRIVATE KEY-----
Loading

0 comments on commit 51762f2

Please sign in to comment.