From 55b78f7c38fcdd88c40cd441ade8a46a6ef8c1e4 Mon Sep 17 00:00:00 2001 From: ZhenLian Date: Thu, 13 May 2021 09:39:42 -0700 Subject: [PATCH 01/12] a basic test using new credential API --- .../java/io/grpc/netty/AdvancedTlsTest.java | 183 ++++++++++++++++++ 1 file changed, 183 insertions(+) create mode 100644 netty/src/test/java/io/grpc/netty/AdvancedTlsTest.java diff --git a/netty/src/test/java/io/grpc/netty/AdvancedTlsTest.java b/netty/src/test/java/io/grpc/netty/AdvancedTlsTest.java new file mode 100644 index 00000000000..86adf82c7cc --- /dev/null +++ b/netty/src/test/java/io/grpc/netty/AdvancedTlsTest.java @@ -0,0 +1,183 @@ +/* + * Copyright 2021 The gRPC Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.grpc.netty; + +import static org.junit.Assert.fail; + +import com.google.common.util.concurrent.MoreExecutors; +import io.grpc.ChannelCredentials; +import io.grpc.Grpc; +import io.grpc.ManagedChannel; +import io.grpc.Server; +import io.grpc.ServerCredentials; +import io.grpc.StatusRuntimeException; +import io.grpc.TlsChannelCredentials; +import io.grpc.TlsServerCredentials; +import io.grpc.TlsServerCredentials.ClientAuth; +import io.grpc.internal.testing.TestUtils; +import io.grpc.stub.StreamObserver; +import io.grpc.testing.protobuf.SimpleRequest; +import io.grpc.testing.protobuf.SimpleResponse; +import io.grpc.testing.protobuf.SimpleServiceGrpc; +import io.grpc.util.AdvancedTlsX509KeyManager; +import io.grpc.util.AdvancedTlsX509TrustManager; +import io.grpc.util.AdvancedTlsX509TrustManager.Verification; +import io.grpc.util.CertificateUtils; + +import io.netty.handler.ssl.OpenSsl; +import io.netty.handler.ssl.SslContextBuilder; +import io.netty.handler.ssl.SslProvider; +import java.io.File; +import java.security.PrivateKey; +import java.security.cert.X509Certificate; +import java.security.NoSuchAlgorithmException; +import java.security.Security; +import java.util.Arrays; +import java.util.concurrent.Executors; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.TimeUnit; +import javax.net.ssl.SSLContext; +import org.junit.After; +import org.junit.Assume; +import org.junit.Before; +import org.junit.Test; + +public class AdvancedTlsTest { + + + public static final String SERVER_0_KEY_FILE = "server0.key"; + public static final String SERVER_0_PEM_FILE = "server0.pem"; + public static final String CLIENT_0_KEY_FILE = "client.key"; + public static final String CLIENT_0_PEM_FILE = "client.pem"; + public static final String CA_PEM_FILE = "ca.pem"; + + private ScheduledExecutorService executor; + private Server server; + private ManagedChannel channel; + + @Before + public void setUp() throws NoSuchAlgorithmException { + executor = Executors.newSingleThreadScheduledExecutor(); + } + + @After + public void tearDown() { + if (server != null) { + server.shutdown(); + } + if (channel != null) { + channel.shutdown(); + } + MoreExecutors.shutdownAndAwaitTermination(executor, 5, TimeUnit.SECONDS); + } + + @Test + public void basicMutualTlsTest() throws Exception { + File caCert = TestUtils.loadCert("ca.pem"); + File serverKey0 = TestUtils.loadCert("server0.key"); + File serverCert0 = TestUtils.loadCert("server0.pem"); + File clientKey0 = TestUtils.loadCert("client.key"); + File clientCert0 = TestUtils.loadCert("client.pem"); + + // Create & start a server. + + ServerCredentials serverCredentials = TlsServerCredentials.newBuilder() + .keyManager(serverCert0, serverKey0).trustManager(caCert) + .clientAuth(ClientAuth.REQUIRE).build(); + server = Grpc.newServerBuilderForPort(0, serverCredentials).addService( + new SimpleServiceImpl()).build().start(); + + // Create a client to connect. + ChannelCredentials channelCredentials = TlsChannelCredentials.newBuilder() + .keyManager(clientCert0, clientKey0).trustManager(caCert).build(); + channel = Grpc.newChannelBuilderForAddress( + "localhost", server.getPort(), channelCredentials).build(); + + + try { + SimpleServiceGrpc.SimpleServiceBlockingStub client = + SimpleServiceGrpc.newBlockingStub(channel); + // Send an actual request, via the full GRPC & network stack, and check that a proper + // response comes back. + client.unaryRpc(SimpleRequest.getDefaultInstance()); + } catch (StatusRuntimeException e) { + fail("Find error: " + e.getMessage()); + } + } + + /*@Test + public void AdvancedTlsKeyManagerTrustManagerMutualTlsTest() throws Exception { + X509Certificate[] caCert = CertificateUtils.getX509Certificates( + TestUtils.class.getResourceAsStream("/certs/" + CA_PEM_FILE)); + PrivateKey serverKey0 = CertificateUtils.getPrivateKey( + TestUtils.class.getResourceAsStream("/certs/" + SERVER_0_KEY_FILE)); + X509Certificate[] serverCert0 = CertificateUtils.getX509Certificates( + TestUtils.class.getResourceAsStream("/certs/" + SERVER_0_PEM_FILE)); + PrivateKey clientKey0 = CertificateUtils.getPrivateKey( + TestUtils.class.getResourceAsStream("/certs/" + CLIENT_0_KEY_FILE)); + X509Certificate[] clientCert0 = CertificateUtils.getX509Certificates( + TestUtils.class.getResourceAsStream("/certs/" + CLIENT_0_PEM_FILE)); + + + // Create & start a server. + AdvancedTlsX509KeyManager serverKeyManager = new AdvancedTlsX509KeyManager(); + serverKeyManager.updateIdentityCredentials(serverKey0, serverCert0); + AdvancedTlsX509TrustManager serverTrustManager = AdvancedTlsX509TrustManager.newBuilder() + .setVerification(Verification.CertificateOnlyVerification) + .build(); + serverTrustManager.updateTrustCredentials(caCert); + ServerCredentials serverCredentials = TlsServerCredentials.newBuilder() + .keyManager(serverKeyManager).trustManager(serverTrustManager) + .clientAuth(ClientAuth.REQUIRE).build(); + Server server = Grpc.newServerBuilderForPort(0, serverCredentials).addService( + new SimpleServiceImpl()).build().start(); + + // Create a client to connect. + AdvancedTlsX509KeyManager clientKeyManager = new AdvancedTlsX509KeyManager(); + clientKeyManager.updateIdentityCredentials(clientKey0, clientCert0); + AdvancedTlsX509TrustManager clientTrustManager = AdvancedTlsX509TrustManager.newBuilder() + .setVerification(Verification.CertificateAndHostNameVerification) + .build(); + clientTrustManager.updateTrustCredentials(caCert); + ChannelCredentials channelCredentials = TlsChannelCredentials.newBuilder() + .keyManager(clientKeyManager).trustManager(clientTrustManager).build(); + ManagedChannel channel = Grpc.newChannelBuilderForAddress( + "localhost", server.getPort(), channelCredentials).build(); + + + try { + SimpleServiceGrpc.SimpleServiceBlockingStub client = + SimpleServiceGrpc.newBlockingStub(channel); + // Send an actual request, via the full GRPC & network stack, and check that a proper + // response comes back. + client.unaryRpc(SimpleRequest.getDefaultInstance()); + } catch (StatusRuntimeException e) { + fail("Find error: " + e.getMessage()); + } + + server.shutdown(); + channel.shutdown(); + }*/ + + private static class SimpleServiceImpl extends SimpleServiceGrpc.SimpleServiceImplBase { + @Override + public void unaryRpc(SimpleRequest req, StreamObserver respOb) { + respOb.onNext(SimpleResponse.getDefaultInstance()); + respOb.onCompleted(); + } + } +} From 67b657b000be8bf9fa8617a75bac91aa84877b50 Mon Sep 17 00:00:00 2001 From: ZhenLian Date: Thu, 13 May 2021 13:14:41 -0700 Subject: [PATCH 02/12] add advanced TLS classes and more tests --- .../grpc/util/AdvancedTlsX509KeyManager.java | 201 +++++++++ .../util/AdvancedTlsX509TrustManager.java | 391 ++++++++++++++++++ .../java/io/grpc/netty/AdvancedTlsTest.java | 261 +++++++++--- 3 files changed, 807 insertions(+), 46 deletions(-) create mode 100644 core/src/main/java/io/grpc/util/AdvancedTlsX509KeyManager.java create mode 100644 core/src/main/java/io/grpc/util/AdvancedTlsX509TrustManager.java diff --git a/core/src/main/java/io/grpc/util/AdvancedTlsX509KeyManager.java b/core/src/main/java/io/grpc/util/AdvancedTlsX509KeyManager.java new file mode 100644 index 00000000000..6cf797d45c0 --- /dev/null +++ b/core/src/main/java/io/grpc/util/AdvancedTlsX509KeyManager.java @@ -0,0 +1,201 @@ +/* + * Copyright 2021 The gRPC Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.grpc.util; + +import io.grpc.ExperimentalApi; +import java.io.File; +import java.io.FileInputStream; +import java.io.IOException; +import java.io.InputStream; +import java.net.Socket; +import java.security.NoSuchAlgorithmException; +import java.security.Principal; +import java.security.PrivateKey; +import java.security.cert.CertificateException; +import java.security.cert.X509Certificate; +import java.security.spec.InvalidKeySpecException; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.ScheduledFuture; +import java.util.concurrent.TimeUnit; +import java.util.logging.Level; +import java.util.logging.Logger; +import javax.net.ssl.SSLEngine; +import javax.net.ssl.X509ExtendedKeyManager; + +@ExperimentalApi("https://github.com/grpc/grpc-java/issues/8024") +/** + * AdvancedTlsX509KeyManager is an {@code X509ExtendedKeyManager} that allows users to configure + * advanced TLS features, such as private key and certificate chain reloading, etc. + */ +public final class AdvancedTlsX509KeyManager extends X509ExtendedKeyManager { + private static final Logger log = Logger.getLogger(AdvancedTlsX509KeyManager.class.getName()); + + // The private key and the cert chain we will use to send to peers to prove our identity. + private volatile PrivateKey key; + private volatile X509Certificate[] certs; + + public AdvancedTlsX509KeyManager() { } + + @Override + public PrivateKey getPrivateKey(String alias) { + // Same question as the trust manager: is it really thread-safe to do this? + return this.key; + } + + @Override + public X509Certificate[] getCertificateChain(String alias) { + return this.certs; + } + + @Override + public String[] getClientAliases(String keyType, Principal[] issuers) { + return new String[0]; + } + + @Override + public String chooseClientAlias(String[] keyType, Principal[] issuers, Socket socket) { + return "default"; + } + + @Override + public String chooseEngineClientAlias(String[] keyType, Principal[] issuers, SSLEngine engine) { + return "default"; + } + + @Override + public String[] getServerAliases(String keyType, Principal[] issuers) { + return new String[0]; + } + + @Override + public String chooseServerAlias(String keyType, Principal[] issuers, Socket socket) { + return "default"; + } + + @Override + public String chooseEngineServerAlias(String keyType, Principal[] issuers, + SSLEngine engine) { + return "default"; + } + + // Update the current cached private key and cert chains. + // Users should make sure the private key matches the public key on the leaf certificate of the + // certificate chain. + // TODO(ZhenLian): explore possibilities to do a crypto check here. + public void updateIdentityCredentials(PrivateKey key, X509Certificate[] certs) { + // Same question as the trust manager: what to do if copy is not feasible here? + this.key = key; + this.certs = certs; + } + + /** + * starts a {@code ScheduledExecutorService} to read private key and certificate chains from the + * local file paths periodically, and update the cached identity credentials if they are both + * updated. + * + * @param keyFile the file on disk holding the private key + * @param certFile the file on disk holding the certificate chain + * @param initialDelay the time to delay first read-and-update execution + * @param delay the period between successive read-and-update executions + * @param unit the time unit of the initialDelay and period parameters + * @param executor the execute service we use to start the read-and-update thread + * @return an object that caller should close when the scheduled execution is not needed + */ + public Closeable updateIdentityCredentialsFromFile(File keyFile, File certFile, + long initialDelay, long delay, TimeUnit unit, ScheduledExecutorService executor) { + final ScheduledFuture future = + executor.scheduleWithFixedDelay( + new LoadFilePathExecution(keyFile, certFile), + initialDelay, delay, unit); + return new Closeable() { + @Override public void close() { + future.cancel(false); + } + }; + } + + private class LoadFilePathExecution implements Runnable { + File keyFile; + File certFile; + long currentKeyTime; + long currentCertTime; + + public LoadFilePathExecution(File keyFile, File certFile) { + this.keyFile = keyFile; + this.certFile = certFile; + this.currentKeyTime = 0; + this.currentCertTime = 0; + } + + @Override + public void run() { + try { + UpdateResult newResult = readAndUpdate(this.keyFile, this.certFile, this.currentKeyTime, + this.currentCertTime); + if (newResult.success) { + this.currentKeyTime = newResult.keyTime; + this.currentCertTime = newResult.certTime; + } + } catch (CertificateException | IOException | NoSuchAlgorithmException | + InvalidKeySpecException e) { + log.log(Level.SEVERE, "readAndUpdate() execution thread failed", e); + } + } + } + + private static class UpdateResult { + boolean success; + long keyTime; + long certTime; + + public UpdateResult(boolean success, long keyTime, long certTime) { + this.success = success; + this.keyTime = keyTime; + this.certTime = certTime; + } + } + + /** + * reads the private key and certificates specified in the path locations. Updates |key| and |cert| + * if both of their modified time changed since last read. + * + * @param keyFile the file on disk holding the private key + * @param certFile the file on disk holding the certificate chain + * @param oldKeyTime the time when the private key file is modified during last execution + * @param oldCertTime the time when the certificate chain file is modified during last execution + * @return the result of this update execution + */ + private UpdateResult readAndUpdate(File keyFile, File certFile, long oldKeyTime, long oldCertTime) + throws IOException, CertificateException, NoSuchAlgorithmException, InvalidKeySpecException { + long newKeyTime = keyFile.lastModified(); + long newCertTime = certFile.lastModified(); + // We only update when both the key and the certs are updated. + if (newKeyTime != oldKeyTime && newCertTime != oldCertTime) { + PrivateKey key = CertificateUtils.getPrivateKey(new FileInputStream(keyFile)); + X509Certificate[] certs = CertificateUtils.getX509Certificates(new FileInputStream(certFile)); + updateIdentityCredentials(key, certs); + return new UpdateResult(true, newKeyTime, newCertTime); + } + return new UpdateResult(false, -1, -1); + } + + // Mainly used to avoid throwing IO Exceptions in java.io.Closeable. + public interface Closeable extends java.io.Closeable { + @Override public void close(); + } +} + diff --git a/core/src/main/java/io/grpc/util/AdvancedTlsX509TrustManager.java b/core/src/main/java/io/grpc/util/AdvancedTlsX509TrustManager.java new file mode 100644 index 00000000000..33e7a072975 --- /dev/null +++ b/core/src/main/java/io/grpc/util/AdvancedTlsX509TrustManager.java @@ -0,0 +1,391 @@ +/* + * Copyright 2021 The gRPC Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.grpc.util; + +import io.grpc.ExperimentalApi; +import java.io.File; +import java.io.FileInputStream; +import java.io.FileNotFoundException; +import java.io.IOException; +import java.net.Socket; +import java.security.KeyStore; +import java.security.KeyStoreException; +import java.security.NoSuchAlgorithmException; +import java.security.cert.CertificateException; +import java.security.cert.X509Certificate; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.ScheduledFuture; +import java.util.concurrent.TimeUnit; +import java.util.logging.Level; +import java.util.logging.Logger; +import javax.net.ssl.SSLEngine; +import javax.net.ssl.SSLParameters; +import javax.net.ssl.TrustManager; +import javax.net.ssl.TrustManagerFactory; +import javax.net.ssl.X509ExtendedTrustManager; + +@ExperimentalApi("https://github.com/grpc/grpc-java/issues/8024") +/** + * AdvancedTlsX509TrustManager is an {@code X509ExtendedTrustManager} that allows users to configure + * advanced TLS features, such as root certificate reloading, peer cert custom verification, etc. + */ +public final class AdvancedTlsX509TrustManager extends X509ExtendedTrustManager { + private static final Logger log = Logger.getLogger(AdvancedTlsX509TrustManager.class.getName()); + + private Verification verification; + private PeerVerifier peerVerifier; + private SSLSocketPeerVerifier sslSocketPeerVerifier; + private SSLEnginePeerVerifier sslEnginePeerVerifier; + // The trust certs we will use to verify peer certificate chain. + private volatile X509Certificate[] trustCerts; + + private AdvancedTlsX509TrustManager(Verification verification, PeerVerifier peerVerifier, + SSLSocketPeerVerifier socketPeerVerifier, SSLEnginePeerVerifier enginePeerVerifier) { + this.verification = verification; + this.peerVerifier = peerVerifier; + this.sslSocketPeerVerifier = socketPeerVerifier; + this.sslEnginePeerVerifier = enginePeerVerifier; + } + + @Override + public void checkClientTrusted(X509Certificate[] chain, String authType) + throws CertificateException { + checkTrusted(chain, authType, null, null, false); + } + + @Override + public void checkServerTrusted(X509Certificate[] chain, String authType) + throws CertificateException { + checkTrusted(chain, authType, null, null, true); + } + + @Override + public void checkClientTrusted(X509Certificate[] chain, String authType, Socket socket) + throws CertificateException { + checkTrusted(chain, authType, null, socket, false); + } + + @Override + public void checkServerTrusted(X509Certificate[] chain, String authType, Socket socket) + throws CertificateException { + checkTrusted(chain, authType, null, socket, true); + } + + @Override + public void checkClientTrusted(X509Certificate[] chain, String authType, SSLEngine engine) + throws CertificateException { + checkTrusted(chain, authType, engine, null, false); + } + + @Override + public void checkServerTrusted(X509Certificate[] chain, String authType, SSLEngine engine) + throws CertificateException { + checkTrusted(chain, authType, engine, null, true); + } + + @Override + public X509Certificate[] getAcceptedIssuers() { + return trustCerts; + } + + // If this is set, we will use the default trust certificates stored on user's local system. + // After this is used, functions that will update this.trustCerts(e.g. updateTrustCredentials(), + // updateTrustCredentialsFromFile()) should not be called. + public void useSystemDefaultTrustCerts() { + this.trustCerts = new X509Certificate[0]; + } + + // Update the current cached trust certs. + public void updateTrustCredentials(X509Certificate[] trustCerts) { + // Question: in design, we will do an copy of trustCerts, but that seems not easy, since + // X509Certificate is an abstract class without copy constructor. + // Can we document in the API that trustCerts shouldn't be modified once set? + this.trustCerts = trustCerts; + } + + public void checkTrusted(X509Certificate[] chain, String authType, SSLEngine sslEngine, + Socket socket, boolean checkingServer) throws CertificateException { + if (this.verification == Verification.CertificateAndHostNameVerification + || this.verification == Verification.CertificateOnlyVerification) { + if (chain == null || chain.length == 0) { + throw new CertificateException( + "Want certificate verification but got null or empty certificates"); + } + // Set the cached trust certificates into a key store. + KeyStore ks; + try { + ks = KeyStore.getInstance(KeyStore.getDefaultType()); + ks.load(null, null); + } catch (KeyStoreException | IOException | NoSuchAlgorithmException e) { + throw new CertificateException("Failed to create KeyStore", e); + } + int i = 1; + // Question: I know we've already made this.trustCerts volatile, but shouldn't we require a + // lock around it as well? Otherwise, while we are reading here, the updating thread might + // change it as well? + // I could be wrong, but my current understanding about "volatile" is that it guarantees we + // store value in memory instead of cache, so there wouldn't be problems like dirty read, + // etc, but it doesn't eliminate race conditions. + for (X509Certificate cert: this.trustCerts) { + String alias = Integer.toString(i); + try { + ks.setCertificateEntry(alias, cert); + } catch (KeyStoreException e) { + throw new CertificateException("Failed to load trust certificate into KeyStore", e); + } + i++; + } + // Use the key store to create a delegated trust manager. + X509ExtendedTrustManager delegateManager = null; + TrustManagerFactory tmf; + try { + tmf = TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm()); + tmf.init(ks); + } catch (KeyStoreException | NoSuchAlgorithmException e) { + throw new CertificateException("Failed to create delegate TrustManagerFactory", e); + } + TrustManager[] tms = tmf.getTrustManagers(); + // Iterate over the returned trust managers, looking for an instance of X509TrustManager. + // If found, use that as the delegate trust manager. + for (i = 0; i < tms.length; i++) { + if (tms[i] instanceof X509ExtendedTrustManager) { + delegateManager = (X509ExtendedTrustManager) tms[i]; + break; + } + } + if (delegateManager == null) { + throw new CertificateException( + "Instance delegateX509TrustManager is null. Failed to initialize"); + } + // Configure the delegated trust manager based on users' input. + if (checkingServer) { + if (this.verification == Verification.CertificateAndHostNameVerification || + this.verification == Verification.CertificateOnlyVerification) { + if (sslEngine == null) { + throw new CertificateException( + "SSLEngine is null. Couldn't check host name"); + } + String algorithm = this.verification == Verification.CertificateAndHostNameVerification + ? "HTTPS" : ""; + SSLParameters sslParams = sslEngine.getSSLParameters(); + sslParams.setEndpointIdentificationAlgorithm(algorithm); + sslEngine.setSSLParameters(sslParams); + } + delegateManager.checkServerTrusted(chain, authType, sslEngine); + } else { + delegateManager.checkClientTrusted(chain, authType, sslEngine); + } + } + // Perform the additional peer cert check. + if (sslEngine != null && sslEnginePeerVerifier != null) { + sslEnginePeerVerifier.verifyPeerCertificate(chain, authType, sslEngine); + } + if (socket != null && sslSocketPeerVerifier != null) { + sslSocketPeerVerifier.verifyPeerCertificate(chain, authType, socket); + } + if (peerVerifier != null) { + peerVerifier.verifyPeerCertificate(chain, authType); + } + if (sslEnginePeerVerifier == null && sslSocketPeerVerifier == null && peerVerifier == null) { + log.log(Level.INFO, "No peer verifier is specified"); + } + } + + /** + * starts a {@code ScheduledExecutorService} to read trust certificates from a local file path + * periodically, and update the cached trust certs if there is an update. + * + * @param trustCertFile the file on disk holding the trust certificates + * @param initialDelay the time to delay first read-and-update execution + * @param delay the period between successive read-and-update executions + * @param unit the time unit of the initialDelay and period parameters + * @param executor the execute service we use to start the read-and-update thread + * @return an object that caller should close when the scheduled execution is not needed + */ + public Closeable updateTrustCredentialsFromFile(File trustCertFile, long initialDelay, + long delay, TimeUnit unit, ScheduledExecutorService executor) { + final ScheduledFuture future = + executor.scheduleWithFixedDelay( + new LoadFilePathExecution(trustCertFile), + initialDelay, delay, unit); + return new Closeable() { + @Override public void close() { + future.cancel(false); + } + }; + } + + private class LoadFilePathExecution implements Runnable { + File file; + long currentTime; + + public LoadFilePathExecution(File file) { + this.file = file; + this.currentTime = 0; + } + + @Override + public void run() { + try { + long newUpdateTime = readAndUpdate(this.file, this.currentTime); + if (newUpdateTime != 0) { + this.currentTime = newUpdateTime; + } + } catch (FileNotFoundException | CertificateException e) { + log.log(Level.SEVERE, "readAndUpdate() execution thread failed", e); + + } + } + } + + /** + * reads the trust certificates specified in the path location, and update |trustCerts| if the + * modified time has changed since last read. + * + * @param trustCertFile the file on disk holding the trust certificates + * @param oldTime the time when the trust file is modified during last execution + * @return 0 if something wrong with this execution or the modified time is not changed since last + * execution, otherwise the modified time during this execution + */ + private long readAndUpdate(File trustCertFile, long oldTime) + throws FileNotFoundException, CertificateException { + long newTime = trustCertFile.lastModified(); + if (newTime != oldTime) { + X509Certificate[] certificates = CertificateUtils.getX509Certificates( + new FileInputStream(trustCertFile)); + updateTrustCredentials(certificates); + return newTime; + } + return 0; + } + + // Mainly used to avoid throwing IO Exceptions in java.io.Closeable. + public interface Closeable extends java.io.Closeable { + @Override public void close(); + } + + public static Builder newBuilder() { + return new Builder().setVerification(Verification.CertificateAndHostNameVerification); + } + + // The verification mode when authenticating the peer certificate. + public enum Verification { + // This is the DEFAULT and RECOMMENDED mode for most applications. + // Setting this on the client side will do the certificate and hostname verification, while + // setting this on the server side will only do the certificate verification. + CertificateAndHostNameVerification, + // This SHOULD be chosen only when you know what the implication this will bring, and have a + // basic understanding about TLS. + // It SHOULD be accompanied with proper additional peer identity checks set through + // {@code PeerVerifier}(nit: why this @code not working?). Failing to do so will leave + // applications to MITM attack. + // Also note that this will only take effect if the underlying SDK implementation invokes + // checkClientTrusted/checkServerTrusted with the {@code SSLEngine} parameter while doing + // verification. + // Setting this on either side will only do the certificate verification. + CertificateOnlyVerification, + // Setting is very DANGEROUS. Please try to avoid this in a real production environment, unless + // you are a super advanced user intended to re-implement the whole verification logic on your + // own. A secure verification might include: + // 1. proper verification on the peer certificate chain + // 2. proper checks on the identity of the peer certificate + InsecurelySkipAllVerification, + } + + // Additional custom peer verification check. + // It will be used when checkClientTrusted/checkServerTrusted is called without + // {@code Socket}/{@code SSLEngine} parameter. + public interface PeerVerifier { + /** + * Verifies the peer certificate chain. For more information, please refer to + * {@code X509ExtendedTrustManager}. + * + * @param peerCertChain the certificate chain sent from the peer + * @param authType the key exchange algorithm used, e.g. "RSA", "DHE_DSS", etc + */ + void verifyPeerCertificate(X509Certificate[] peerCertChain, String authType) + throws CertificateException; + } + // Additional custom peer verification check. + // It will be used when checkClientTrusted/checkServerTrusted is called with the {@code Socket} + // parameter. + public interface SSLSocketPeerVerifier { + /** + * Verifies the peer certificate chain. For more information, please refer to + * {@code X509ExtendedTrustManager}. + * + * @param peerCertChain the certificate chain sent from the peer + * @param authType the key exchange algorithm used, e.g. "RSA", "DHE_DSS", etc + * @param socket the socket used for this connection. This parameter can be null, which + * indicates that implementations need not check the ssl parameters + */ + void verifyPeerCertificate(X509Certificate[] peerCertChain, String authType, Socket socket) + throws CertificateException; + } + // Additional custom peer verification check. + // It will be used when checkClientTrusted/checkServerTrusted is called with the {@code SSLEngine} + // parameter. + public interface SSLEnginePeerVerifier { + /** + * Verifies the peer certificate chain. For more information, please refer to + * {@code X509ExtendedTrustManager}. + * + * @param peerCertChain the certificate chain sent from the peer + * @param authType the key exchange algorithm used, e.g. "RSA", "DHE_DSS", etc + * @param engine the engine used for this connection. This parameter can be null, which + * indicates that implementations need not check the ssl parameters + */ + void verifyPeerCertificate(X509Certificate[] peerCertChain, String authType, SSLEngine engine) + throws CertificateException; + } + + public static final class Builder { + + private Verification verification; + private PeerVerifier peerVerifier; + private SSLSocketPeerVerifier socketPeerVerifier; + private SSLEnginePeerVerifier enginePeerVerifier; + + public Builder setVerification(Verification verification) { + this.verification = verification; + return this; + } + + public Builder setPeerVerifier(PeerVerifier peerVerifier) { + this.peerVerifier = peerVerifier; + return this; + } + + public Builder setSSLSocketPeerVerifier(SSLSocketPeerVerifier peerVerifier) { + this.socketPeerVerifier = peerVerifier; + return this; + } + + public Builder setSSLEnginePeerVerifier(SSLEnginePeerVerifier peerVerifier) { + this.enginePeerVerifier = peerVerifier; + return this; + } + + // Question: shall we do some sanity checks here, to make sure all three verifiers are set, to + // be more secure? + public AdvancedTlsX509TrustManager build() { + return new AdvancedTlsX509TrustManager(this.verification, this.peerVerifier, + this.socketPeerVerifier, this.enginePeerVerifier); + } + } +} + diff --git a/netty/src/test/java/io/grpc/netty/AdvancedTlsTest.java b/netty/src/test/java/io/grpc/netty/AdvancedTlsTest.java index 86adf82c7cc..aac22a85133 100644 --- a/netty/src/test/java/io/grpc/netty/AdvancedTlsTest.java +++ b/netty/src/test/java/io/grpc/netty/AdvancedTlsTest.java @@ -35,30 +35,30 @@ import io.grpc.testing.protobuf.SimpleServiceGrpc; import io.grpc.util.AdvancedTlsX509KeyManager; import io.grpc.util.AdvancedTlsX509TrustManager; +import io.grpc.util.AdvancedTlsX509TrustManager.PeerVerifier; +import io.grpc.util.AdvancedTlsX509TrustManager.SSLEnginePeerVerifier; +import io.grpc.util.AdvancedTlsX509TrustManager.SSLSocketPeerVerifier; import io.grpc.util.AdvancedTlsX509TrustManager.Verification; import io.grpc.util.CertificateUtils; -import io.netty.handler.ssl.OpenSsl; -import io.netty.handler.ssl.SslContextBuilder; -import io.netty.handler.ssl.SslProvider; +import java.io.Closeable; import java.io.File; +import java.io.IOException; +import java.net.Socket; import java.security.PrivateKey; +import java.security.cert.CertificateException; import java.security.cert.X509Certificate; import java.security.NoSuchAlgorithmException; -import java.security.Security; -import java.util.Arrays; +import java.security.spec.InvalidKeySpecException; import java.util.concurrent.Executors; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; -import javax.net.ssl.SSLContext; +import javax.net.ssl.SSLEngine; import org.junit.After; -import org.junit.Assume; import org.junit.Before; import org.junit.Test; public class AdvancedTlsTest { - - public static final String SERVER_0_KEY_FILE = "server0.key"; public static final String SERVER_0_PEM_FILE = "server0.pem"; public static final String CLIENT_0_KEY_FILE = "client.key"; @@ -69,9 +69,36 @@ public class AdvancedTlsTest { private Server server; private ManagedChannel channel; + private File caCertFile; + private File serverKey0File; + private File serverCert0File; + private File clientKey0File; + private File clientCert0File; + private X509Certificate[] caCert; + private PrivateKey serverKey0; + private X509Certificate[] serverCert0; + private PrivateKey clientKey0; + private X509Certificate[] clientCert0; + @Before - public void setUp() throws NoSuchAlgorithmException { + public void setUp() + throws NoSuchAlgorithmException, IOException, CertificateException, InvalidKeySpecException { executor = Executors.newSingleThreadScheduledExecutor(); + caCertFile = TestUtils.loadCert(CA_PEM_FILE); + serverKey0File = TestUtils.loadCert(SERVER_0_KEY_FILE); + serverCert0File = TestUtils.loadCert(SERVER_0_PEM_FILE); + clientKey0File = TestUtils.loadCert(CLIENT_0_KEY_FILE); + clientCert0File = TestUtils.loadCert(CLIENT_0_PEM_FILE); + caCert = CertificateUtils.getX509Certificates( + TestUtils.class.getResourceAsStream("/certs/" + CA_PEM_FILE)); + serverKey0 = CertificateUtils.getPrivateKey( + TestUtils.class.getResourceAsStream("/certs/" + SERVER_0_KEY_FILE)); + serverCert0 = CertificateUtils.getX509Certificates( + TestUtils.class.getResourceAsStream("/certs/" + SERVER_0_PEM_FILE)); + clientKey0 = CertificateUtils.getPrivateKey( + TestUtils.class.getResourceAsStream("/certs/" + CLIENT_0_KEY_FILE)); + clientCert0 = CertificateUtils.getX509Certificates( + TestUtils.class.getResourceAsStream("/certs/" + CLIENT_0_PEM_FILE)); } @After @@ -87,27 +114,18 @@ public void tearDown() { @Test public void basicMutualTlsTest() throws Exception { - File caCert = TestUtils.loadCert("ca.pem"); - File serverKey0 = TestUtils.loadCert("server0.key"); - File serverCert0 = TestUtils.loadCert("server0.pem"); - File clientKey0 = TestUtils.loadCert("client.key"); - File clientCert0 = TestUtils.loadCert("client.pem"); - // Create & start a server. - ServerCredentials serverCredentials = TlsServerCredentials.newBuilder() - .keyManager(serverCert0, serverKey0).trustManager(caCert) + .keyManager(serverCert0File, serverKey0File).trustManager(caCertFile) .clientAuth(ClientAuth.REQUIRE).build(); server = Grpc.newServerBuilderForPort(0, serverCredentials).addService( new SimpleServiceImpl()).build().start(); - // Create a client to connect. ChannelCredentials channelCredentials = TlsChannelCredentials.newBuilder() - .keyManager(clientCert0, clientKey0).trustManager(caCert).build(); - channel = Grpc.newChannelBuilderForAddress( - "localhost", server.getPort(), channelCredentials).build(); - - + .keyManager(clientCert0File, clientKey0File).trustManager(caCertFile).build(); + channel = Grpc.newChannelBuilderForAddress("localhost", server.getPort(), channelCredentials) + .overrideAuthority("foo.test.google.com.au").build(); + // Start the connection. try { SimpleServiceGrpc.SimpleServiceBlockingStub client = SimpleServiceGrpc.newBlockingStub(channel); @@ -115,24 +133,14 @@ public void basicMutualTlsTest() throws Exception { // response comes back. client.unaryRpc(SimpleRequest.getDefaultInstance()); } catch (StatusRuntimeException e) { - fail("Find error: " + e.getMessage()); + e.printStackTrace(); + fail("Failed to make a connection"); + e.printStackTrace(); } } - /*@Test + @Test public void AdvancedTlsKeyManagerTrustManagerMutualTlsTest() throws Exception { - X509Certificate[] caCert = CertificateUtils.getX509Certificates( - TestUtils.class.getResourceAsStream("/certs/" + CA_PEM_FILE)); - PrivateKey serverKey0 = CertificateUtils.getPrivateKey( - TestUtils.class.getResourceAsStream("/certs/" + SERVER_0_KEY_FILE)); - X509Certificate[] serverCert0 = CertificateUtils.getX509Certificates( - TestUtils.class.getResourceAsStream("/certs/" + SERVER_0_PEM_FILE)); - PrivateKey clientKey0 = CertificateUtils.getPrivateKey( - TestUtils.class.getResourceAsStream("/certs/" + CLIENT_0_KEY_FILE)); - X509Certificate[] clientCert0 = CertificateUtils.getX509Certificates( - TestUtils.class.getResourceAsStream("/certs/" + CLIENT_0_PEM_FILE)); - - // Create & start a server. AdvancedTlsX509KeyManager serverKeyManager = new AdvancedTlsX509KeyManager(); serverKeyManager.updateIdentityCredentials(serverKey0, serverCert0); @@ -143,9 +151,8 @@ public void AdvancedTlsKeyManagerTrustManagerMutualTlsTest() throws Exception { ServerCredentials serverCredentials = TlsServerCredentials.newBuilder() .keyManager(serverKeyManager).trustManager(serverTrustManager) .clientAuth(ClientAuth.REQUIRE).build(); - Server server = Grpc.newServerBuilderForPort(0, serverCredentials).addService( + server = Grpc.newServerBuilderForPort(0, serverCredentials).addService( new SimpleServiceImpl()).build().start(); - // Create a client to connect. AdvancedTlsX509KeyManager clientKeyManager = new AdvancedTlsX509KeyManager(); clientKeyManager.updateIdentityCredentials(clientKey0, clientCert0); @@ -155,10 +162,169 @@ public void AdvancedTlsKeyManagerTrustManagerMutualTlsTest() throws Exception { clientTrustManager.updateTrustCredentials(caCert); ChannelCredentials channelCredentials = TlsChannelCredentials.newBuilder() .keyManager(clientKeyManager).trustManager(clientTrustManager).build(); - ManagedChannel channel = Grpc.newChannelBuilderForAddress( - "localhost", server.getPort(), channelCredentials).build(); + channel = Grpc.newChannelBuilderForAddress("localhost", server.getPort(), channelCredentials) + .overrideAuthority("foo.test.google.com.au").build(); + // Start the connection. + try { + SimpleServiceGrpc.SimpleServiceBlockingStub client = + SimpleServiceGrpc.newBlockingStub(channel); + // Send an actual request, via the full GRPC & network stack, and check that a proper + // response comes back. + client.unaryRpc(SimpleRequest.getDefaultInstance()); + } catch (StatusRuntimeException e) { + fail("Failed to make a connection"); + e.printStackTrace(); + } + } + @Test + public void TrustManagerCustomVerifierMutualTlsTest() throws Exception { + // Create & start a server. + AdvancedTlsX509KeyManager serverKeyManager = new AdvancedTlsX509KeyManager(); + serverKeyManager.updateIdentityCredentials(serverKey0, serverCert0); + AdvancedTlsX509TrustManager serverTrustManager = AdvancedTlsX509TrustManager.newBuilder() + .setVerification(Verification.CertificateOnlyVerification) + .setSSLEnginePeerVerifier( + new SSLEnginePeerVerifier() { + @Override + public void verifyPeerCertificate(X509Certificate[] peerCertChain, String authType, + SSLEngine engine) throws CertificateException { + if (peerCertChain == null || peerCertChain.length == 0) { + throw new CertificateException("peerCertChain is empty"); + } + X509Certificate leafCert = peerCertChain[0]; + if (!leafCert.getSubjectDN().getName().contains("testclient")) { + throw new CertificateException("SSLEnginePeerVerifier failed"); + } + } + }) + .setSSLSocketPeerVerifier(new SSLSocketPeerVerifier() { + @Override + public void verifyPeerCertificate(X509Certificate[] peerCertChain, String authType, + Socket socket) throws CertificateException { + if (peerCertChain == null || peerCertChain.length == 0) { + throw new CertificateException("peerCertChain is empty"); + } + X509Certificate leafCert = peerCertChain[0]; + if (!leafCert.getSubjectDN().getName().contains("testclient")) { + throw new CertificateException("SSLSocketPeerVerifier failed"); + } + } + }) + .setPeerVerifier(new PeerVerifier() { + @Override + public void verifyPeerCertificate(X509Certificate[] peerCertChain, String authType) + throws CertificateException { + if (peerCertChain == null || peerCertChain.length == 0) { + throw new CertificateException("peerCertChain is empty"); + } + X509Certificate leafCert = peerCertChain[0]; + if (!leafCert.getSubjectDN().getName().contains("testclient")) { + throw new CertificateException("PeerVerifier failed"); + } + } + }) + .build(); + serverTrustManager.updateTrustCredentials(caCert); + ServerCredentials serverCredentials = TlsServerCredentials.newBuilder() + .keyManager(serverKeyManager).trustManager(serverTrustManager) + .clientAuth(ClientAuth.REQUIRE).build(); + server = Grpc.newServerBuilderForPort(0, serverCredentials).addService( + new SimpleServiceImpl()).build().start(); + // Create a client to connect. + AdvancedTlsX509KeyManager clientKeyManager = new AdvancedTlsX509KeyManager(); + clientKeyManager.updateIdentityCredentials(clientKey0, clientCert0); + AdvancedTlsX509TrustManager clientTrustManager = AdvancedTlsX509TrustManager.newBuilder() + .setVerification(Verification.CertificateOnlyVerification) + .setSSLEnginePeerVerifier( + new SSLEnginePeerVerifier() { + @Override + public void verifyPeerCertificate(X509Certificate[] peerCertChain, String authType, + SSLEngine engine) throws CertificateException { + if (peerCertChain == null || peerCertChain.length == 0) { + throw new CertificateException("peerCertChain is empty"); + } + X509Certificate leafCert = peerCertChain[0]; + if (!leafCert.getSubjectDN().getName().contains("*.test.google.com.au")) { + throw new CertificateException("SSLEnginePeerVerifier failed"); + } + } + }) + .setSSLSocketPeerVerifier(new SSLSocketPeerVerifier() { + @Override + public void verifyPeerCertificate(X509Certificate[] peerCertChain, String authType, + Socket socket) throws CertificateException { + if (peerCertChain == null || peerCertChain.length == 0) { + throw new CertificateException("peerCertChain is empty"); + } + X509Certificate leafCert = peerCertChain[0]; + if (!leafCert.getSubjectDN().getName().contains("*.test.google.com.au")) { + throw new CertificateException("SSLSocketPeerVerifier failed"); + } + } + }) + .setPeerVerifier(new PeerVerifier() { + @Override + public void verifyPeerCertificate(X509Certificate[] peerCertChain, String authType) + throws CertificateException { + if (peerCertChain == null || peerCertChain.length == 0) { + throw new CertificateException("peerCertChain is empty"); + } + X509Certificate leafCert = peerCertChain[0]; + if (!leafCert.getSubjectDN().getName().contains("*.test.google.com.au")) { + throw new CertificateException("PeerVerifier failed"); + } + } + }) + .build(); + clientTrustManager.updateTrustCredentials(caCert); + ChannelCredentials channelCredentials = TlsChannelCredentials.newBuilder() + .keyManager(clientKeyManager).trustManager(clientTrustManager).build(); + channel = Grpc.newChannelBuilderForAddress( + "localhost", server.getPort(), channelCredentials).build(); + // Start the connection. + try { + SimpleServiceGrpc.SimpleServiceBlockingStub client = + SimpleServiceGrpc.newBlockingStub(channel); + // Send an actual request, via the full GRPC & network stack, and check that a proper + // response comes back. + client.unaryRpc(SimpleRequest.getDefaultInstance()); + } catch (StatusRuntimeException e) { + fail("Failed to make a connection"); + e.printStackTrace(); + } + } + @Test + public void OnFileReloadingKeyManagerTrustManagerTest() throws Exception { + // Create & start a server. + AdvancedTlsX509KeyManager serverKeyManager = new AdvancedTlsX509KeyManager(); + Closeable serverKeyShutdown = serverKeyManager.updateIdentityCredentialsFromFile(serverKey0File, + serverCert0File, 0, 100, TimeUnit.MILLISECONDS, executor); + AdvancedTlsX509TrustManager serverTrustManager = AdvancedTlsX509TrustManager.newBuilder() + .setVerification(Verification.CertificateOnlyVerification) + .build(); + Closeable serverTrustShutdown = serverTrustManager.updateTrustCredentialsFromFile(caCertFile, + 0, 100, TimeUnit.MILLISECONDS, executor); + ServerCredentials serverCredentials = TlsServerCredentials.newBuilder() + .keyManager(serverKeyManager).trustManager(serverTrustManager) + .clientAuth(ClientAuth.REQUIRE).build(); + server = Grpc.newServerBuilderForPort(0, serverCredentials).addService( + new SimpleServiceImpl()).build().start(); + // Create a client to connect. + AdvancedTlsX509KeyManager clientKeyManager = new AdvancedTlsX509KeyManager(); + Closeable clientKeyShutdown = clientKeyManager.updateIdentityCredentialsFromFile(clientKey0File, + clientCert0File, 0, 100, TimeUnit.MILLISECONDS, executor); + AdvancedTlsX509TrustManager clientTrustManager = AdvancedTlsX509TrustManager.newBuilder() + .setVerification(Verification.CertificateAndHostNameVerification) + .build(); + Closeable clientTrustShutdown = clientTrustManager.updateTrustCredentialsFromFile(caCertFile, + 0, 100, TimeUnit.MILLISECONDS, executor); + ChannelCredentials channelCredentials = TlsChannelCredentials.newBuilder() + .keyManager(clientKeyManager).trustManager(clientTrustManager).build(); + channel = Grpc.newChannelBuilderForAddress("localhost", server.getPort(), channelCredentials) + .overrideAuthority("foo.test.google.com.au").build(); + // Start the connection. try { SimpleServiceGrpc.SimpleServiceBlockingStub client = SimpleServiceGrpc.newBlockingStub(channel); @@ -166,12 +332,15 @@ public void AdvancedTlsKeyManagerTrustManagerMutualTlsTest() throws Exception { // response comes back. client.unaryRpc(SimpleRequest.getDefaultInstance()); } catch (StatusRuntimeException e) { + e.printStackTrace(); fail("Find error: " + e.getMessage()); } - - server.shutdown(); - channel.shutdown(); - }*/ + // Clean up. + serverKeyShutdown.close(); + serverTrustShutdown.close(); + clientKeyShutdown.close(); + clientTrustShutdown.close(); + } private static class SimpleServiceImpl extends SimpleServiceGrpc.SimpleServiceImplBase { @Override From 043bd08538d9411bc411ec59c2c76c746323cb6d Mon Sep 17 00:00:00 2001 From: ZhenLian Date: Wed, 19 May 2021 23:53:14 -0700 Subject: [PATCH 03/12] fix style errors --- .../grpc/util/AdvancedTlsX509KeyManager.java | 22 ++++--- .../util/AdvancedTlsX509TrustManager.java | 58 +++++++++++-------- .../java/io/grpc/netty/AdvancedTlsTest.java | 24 ++++---- 3 files changed, 58 insertions(+), 46 deletions(-) diff --git a/core/src/main/java/io/grpc/util/AdvancedTlsX509KeyManager.java b/core/src/main/java/io/grpc/util/AdvancedTlsX509KeyManager.java index 6cf797d45c0..9b971033870 100644 --- a/core/src/main/java/io/grpc/util/AdvancedTlsX509KeyManager.java +++ b/core/src/main/java/io/grpc/util/AdvancedTlsX509KeyManager.java @@ -20,7 +20,6 @@ import java.io.File; import java.io.FileInputStream; import java.io.IOException; -import java.io.InputStream; import java.net.Socket; import java.security.NoSuchAlgorithmException; import java.security.Principal; @@ -92,10 +91,15 @@ public String chooseEngineServerAlias(String keyType, Principal[] issuers, return "default"; } - // Update the current cached private key and cert chains. - // Users should make sure the private key matches the public key on the leaf certificate of the - // certificate chain. - // TODO(ZhenLian): explore possibilities to do a crypto check here. + /** + * Updates the current cached private key and cert chains. + * Right now, callers should make sure |key| matches the public key on the leaf certificate of + * |certs|. + * TODO(ZhenLian): explore possibilities to do a crypto check here. + * + * @param key the private key that is going to be used + * @param certs the certificate chain that is going to be used + */ public void updateIdentityCredentials(PrivateKey key, X509Certificate[] certs) { // Same question as the trust manager: what to do if copy is not feasible here? this.key = key; @@ -150,8 +154,8 @@ public void run() { this.currentKeyTime = newResult.keyTime; this.currentCertTime = newResult.certTime; } - } catch (CertificateException | IOException | NoSuchAlgorithmException | - InvalidKeySpecException e) { + } catch (CertificateException | IOException | NoSuchAlgorithmException + | InvalidKeySpecException e) { log.log(Level.SEVERE, "readAndUpdate() execution thread failed", e); } } @@ -170,8 +174,8 @@ public UpdateResult(boolean success, long keyTime, long certTime) { } /** - * reads the private key and certificates specified in the path locations. Updates |key| and |cert| - * if both of their modified time changed since last read. + * reads the private key and certificates specified in the path locations. Updates |key| and + * |cert| if both of their modified time changed since last read. * * @param keyFile the file on disk holding the private key * @param certFile the file on disk holding the certificate chain diff --git a/core/src/main/java/io/grpc/util/AdvancedTlsX509TrustManager.java b/core/src/main/java/io/grpc/util/AdvancedTlsX509TrustManager.java index 33e7a072975..135ca6e6d33 100644 --- a/core/src/main/java/io/grpc/util/AdvancedTlsX509TrustManager.java +++ b/core/src/main/java/io/grpc/util/AdvancedTlsX509TrustManager.java @@ -37,24 +37,27 @@ import javax.net.ssl.TrustManager; import javax.net.ssl.TrustManagerFactory; import javax.net.ssl.X509ExtendedTrustManager; +import org.codehaus.mojo.animal_sniffer.IgnoreJRERequirement; @ExperimentalApi("https://github.com/grpc/grpc-java/issues/8024") /** + * For Android users: this class is only supported in API level 24 and above. * AdvancedTlsX509TrustManager is an {@code X509ExtendedTrustManager} that allows users to configure * advanced TLS features, such as root certificate reloading, peer cert custom verification, etc. */ +@IgnoreJRERequirement public final class AdvancedTlsX509TrustManager extends X509ExtendedTrustManager { private static final Logger log = Logger.getLogger(AdvancedTlsX509TrustManager.class.getName()); private Verification verification; private PeerVerifier peerVerifier; - private SSLSocketPeerVerifier sslSocketPeerVerifier; - private SSLEnginePeerVerifier sslEnginePeerVerifier; + private SslSocketPeerVerifier sslSocketPeerVerifier; + private SslEnginePeerVerifier sslEnginePeerVerifier; // The trust certs we will use to verify peer certificate chain. private volatile X509Certificate[] trustCerts; private AdvancedTlsX509TrustManager(Verification verification, PeerVerifier peerVerifier, - SSLSocketPeerVerifier socketPeerVerifier, SSLEnginePeerVerifier enginePeerVerifier) { + SslSocketPeerVerifier socketPeerVerifier, SslEnginePeerVerifier enginePeerVerifier) { this.verification = verification; this.peerVerifier = peerVerifier; this.sslSocketPeerVerifier = socketPeerVerifier; @@ -68,33 +71,33 @@ public void checkClientTrusted(X509Certificate[] chain, String authType) } @Override - public void checkServerTrusted(X509Certificate[] chain, String authType) + public void checkClientTrusted(X509Certificate[] chain, String authType, Socket socket) throws CertificateException { - checkTrusted(chain, authType, null, null, true); + checkTrusted(chain, authType, null, socket, false); } @Override - public void checkClientTrusted(X509Certificate[] chain, String authType, Socket socket) + public void checkClientTrusted(X509Certificate[] chain, String authType, SSLEngine engine) throws CertificateException { - checkTrusted(chain, authType, null, socket, false); + checkTrusted(chain, authType, engine, null, false); } @Override - public void checkServerTrusted(X509Certificate[] chain, String authType, Socket socket) + public void checkServerTrusted(X509Certificate[] chain, String authType, SSLEngine engine) throws CertificateException { - checkTrusted(chain, authType, null, socket, true); + checkTrusted(chain, authType, engine, null, true); } @Override - public void checkClientTrusted(X509Certificate[] chain, String authType, SSLEngine engine) + public void checkServerTrusted(X509Certificate[] chain, String authType) throws CertificateException { - checkTrusted(chain, authType, engine, null, false); + checkTrusted(chain, authType, null, null, true); } @Override - public void checkServerTrusted(X509Certificate[] chain, String authType, SSLEngine engine) + public void checkServerTrusted(X509Certificate[] chain, String authType, Socket socket) throws CertificateException { - checkTrusted(chain, authType, engine, null, true); + checkTrusted(chain, authType, null, socket, true); } @Override @@ -109,7 +112,11 @@ public void useSystemDefaultTrustCerts() { this.trustCerts = new X509Certificate[0]; } - // Update the current cached trust certs. + /** + * Updates the current cached trust certificates. + * + * @param trustCerts the trust certificates that are going to be used + */ public void updateTrustCredentials(X509Certificate[] trustCerts) { // Question: in design, we will do an copy of trustCerts, but that seems not easy, since // X509Certificate is an abstract class without copy constructor. @@ -117,7 +124,7 @@ public void updateTrustCredentials(X509Certificate[] trustCerts) { this.trustCerts = trustCerts; } - public void checkTrusted(X509Certificate[] chain, String authType, SSLEngine sslEngine, + private void checkTrusted(X509Certificate[] chain, String authType, SSLEngine sslEngine, Socket socket, boolean checkingServer) throws CertificateException { if (this.verification == Verification.CertificateAndHostNameVerification || this.verification == Verification.CertificateOnlyVerification) { @@ -173,8 +180,8 @@ public void checkTrusted(X509Certificate[] chain, String authType, SSLEngine ssl } // Configure the delegated trust manager based on users' input. if (checkingServer) { - if (this.verification == Verification.CertificateAndHostNameVerification || - this.verification == Verification.CertificateOnlyVerification) { + if (this.verification == Verification.CertificateAndHostNameVerification + || this.verification == Verification.CertificateOnlyVerification) { if (sslEngine == null) { throw new CertificateException( "SSLEngine is null. Couldn't check host name"); @@ -258,8 +265,7 @@ public void run() { * * @param trustCertFile the file on disk holding the trust certificates * @param oldTime the time when the trust file is modified during last execution - * @return 0 if something wrong with this execution or the modified time is not changed since last - * execution, otherwise the modified time during this execution + * @return 0 if failed or the modified time is not changed, otherwise the new modified time */ private long readAndUpdate(File trustCertFile, long oldTime) throws FileNotFoundException, CertificateException { @@ -320,10 +326,11 @@ public interface PeerVerifier { void verifyPeerCertificate(X509Certificate[] peerCertChain, String authType) throws CertificateException; } + // Additional custom peer verification check. // It will be used when checkClientTrusted/checkServerTrusted is called with the {@code Socket} // parameter. - public interface SSLSocketPeerVerifier { + public interface SslSocketPeerVerifier { /** * Verifies the peer certificate chain. For more information, please refer to * {@code X509ExtendedTrustManager}. @@ -336,10 +343,11 @@ public interface SSLSocketPeerVerifier { void verifyPeerCertificate(X509Certificate[] peerCertChain, String authType, Socket socket) throws CertificateException; } + // Additional custom peer verification check. // It will be used when checkClientTrusted/checkServerTrusted is called with the {@code SSLEngine} // parameter. - public interface SSLEnginePeerVerifier { + public interface SslEnginePeerVerifier { /** * Verifies the peer certificate chain. For more information, please refer to * {@code X509ExtendedTrustManager}. @@ -357,8 +365,8 @@ public static final class Builder { private Verification verification; private PeerVerifier peerVerifier; - private SSLSocketPeerVerifier socketPeerVerifier; - private SSLEnginePeerVerifier enginePeerVerifier; + private SslSocketPeerVerifier socketPeerVerifier; + private SslEnginePeerVerifier enginePeerVerifier; public Builder setVerification(Verification verification) { this.verification = verification; @@ -370,12 +378,12 @@ public Builder setPeerVerifier(PeerVerifier peerVerifier) { return this; } - public Builder setSSLSocketPeerVerifier(SSLSocketPeerVerifier peerVerifier) { + public Builder setSslSocketPeerVerifier(SslSocketPeerVerifier peerVerifier) { this.socketPeerVerifier = peerVerifier; return this; } - public Builder setSSLEnginePeerVerifier(SSLEnginePeerVerifier peerVerifier) { + public Builder setSslEnginePeerVerifier(SslEnginePeerVerifier peerVerifier) { this.enginePeerVerifier = peerVerifier; return this; } diff --git a/netty/src/test/java/io/grpc/netty/AdvancedTlsTest.java b/netty/src/test/java/io/grpc/netty/AdvancedTlsTest.java index aac22a85133..ad5a0c22050 100644 --- a/netty/src/test/java/io/grpc/netty/AdvancedTlsTest.java +++ b/netty/src/test/java/io/grpc/netty/AdvancedTlsTest.java @@ -36,8 +36,8 @@ import io.grpc.util.AdvancedTlsX509KeyManager; import io.grpc.util.AdvancedTlsX509TrustManager; import io.grpc.util.AdvancedTlsX509TrustManager.PeerVerifier; -import io.grpc.util.AdvancedTlsX509TrustManager.SSLEnginePeerVerifier; -import io.grpc.util.AdvancedTlsX509TrustManager.SSLSocketPeerVerifier; +import io.grpc.util.AdvancedTlsX509TrustManager.SslEnginePeerVerifier; +import io.grpc.util.AdvancedTlsX509TrustManager.SslSocketPeerVerifier; import io.grpc.util.AdvancedTlsX509TrustManager.Verification; import io.grpc.util.CertificateUtils; @@ -45,10 +45,10 @@ import java.io.File; import java.io.IOException; import java.net.Socket; +import java.security.NoSuchAlgorithmException; import java.security.PrivateKey; import java.security.cert.CertificateException; import java.security.cert.X509Certificate; -import java.security.NoSuchAlgorithmException; import java.security.spec.InvalidKeySpecException; import java.util.concurrent.Executors; import java.util.concurrent.ScheduledExecutorService; @@ -140,7 +140,7 @@ public void basicMutualTlsTest() throws Exception { } @Test - public void AdvancedTlsKeyManagerTrustManagerMutualTlsTest() throws Exception { + public void advancedTlsKeyManagerTrustManagerMutualTlsTest() throws Exception { // Create & start a server. AdvancedTlsX509KeyManager serverKeyManager = new AdvancedTlsX509KeyManager(); serverKeyManager.updateIdentityCredentials(serverKey0, serverCert0); @@ -178,14 +178,14 @@ public void AdvancedTlsKeyManagerTrustManagerMutualTlsTest() throws Exception { } @Test - public void TrustManagerCustomVerifierMutualTlsTest() throws Exception { + public void trustManagerCustomVerifierMutualTlsTest() throws Exception { // Create & start a server. AdvancedTlsX509KeyManager serverKeyManager = new AdvancedTlsX509KeyManager(); serverKeyManager.updateIdentityCredentials(serverKey0, serverCert0); AdvancedTlsX509TrustManager serverTrustManager = AdvancedTlsX509TrustManager.newBuilder() .setVerification(Verification.CertificateOnlyVerification) - .setSSLEnginePeerVerifier( - new SSLEnginePeerVerifier() { + .setSslEnginePeerVerifier( + new SslEnginePeerVerifier() { @Override public void verifyPeerCertificate(X509Certificate[] peerCertChain, String authType, SSLEngine engine) throws CertificateException { @@ -198,7 +198,7 @@ public void verifyPeerCertificate(X509Certificate[] peerCertChain, String authTy } } }) - .setSSLSocketPeerVerifier(new SSLSocketPeerVerifier() { + .setSslSocketPeerVerifier(new SslSocketPeerVerifier() { @Override public void verifyPeerCertificate(X509Certificate[] peerCertChain, String authType, Socket socket) throws CertificateException { @@ -236,8 +236,8 @@ public void verifyPeerCertificate(X509Certificate[] peerCertChain, String authTy clientKeyManager.updateIdentityCredentials(clientKey0, clientCert0); AdvancedTlsX509TrustManager clientTrustManager = AdvancedTlsX509TrustManager.newBuilder() .setVerification(Verification.CertificateOnlyVerification) - .setSSLEnginePeerVerifier( - new SSLEnginePeerVerifier() { + .setSslEnginePeerVerifier( + new SslEnginePeerVerifier() { @Override public void verifyPeerCertificate(X509Certificate[] peerCertChain, String authType, SSLEngine engine) throws CertificateException { @@ -250,7 +250,7 @@ public void verifyPeerCertificate(X509Certificate[] peerCertChain, String authTy } } }) - .setSSLSocketPeerVerifier(new SSLSocketPeerVerifier() { + .setSslSocketPeerVerifier(new SslSocketPeerVerifier() { @Override public void verifyPeerCertificate(X509Certificate[] peerCertChain, String authType, Socket socket) throws CertificateException { @@ -296,7 +296,7 @@ public void verifyPeerCertificate(X509Certificate[] peerCertChain, String authTy } @Test - public void OnFileReloadingKeyManagerTrustManagerTest() throws Exception { + public void onFileReloadingKeyManagerTrustManagerTest() throws Exception { // Create & start a server. AdvancedTlsX509KeyManager serverKeyManager = new AdvancedTlsX509KeyManager(); Closeable serverKeyShutdown = serverKeyManager.updateIdentityCredentialsFromFile(serverKey0File, From e17a2e5a8d6e6b3d8932b2a091a47b75a6cc6bd6 Mon Sep 17 00:00:00 2001 From: ZhenLian Date: Thu, 20 May 2021 11:39:32 -0700 Subject: [PATCH 04/12] add sleep period --- netty/src/test/java/io/grpc/netty/AdvancedTlsTest.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/netty/src/test/java/io/grpc/netty/AdvancedTlsTest.java b/netty/src/test/java/io/grpc/netty/AdvancedTlsTest.java index ad5a0c22050..cc9ebdb7a5d 100644 --- a/netty/src/test/java/io/grpc/netty/AdvancedTlsTest.java +++ b/netty/src/test/java/io/grpc/netty/AdvancedTlsTest.java @@ -153,6 +153,7 @@ public void advancedTlsKeyManagerTrustManagerMutualTlsTest() throws Exception { .clientAuth(ClientAuth.REQUIRE).build(); server = Grpc.newServerBuilderForPort(0, serverCredentials).addService( new SimpleServiceImpl()).build().start(); + TimeUnit.SECONDS.sleep(5); // Create a client to connect. AdvancedTlsX509KeyManager clientKeyManager = new AdvancedTlsX509KeyManager(); clientKeyManager.updateIdentityCredentials(clientKey0, clientCert0); @@ -231,6 +232,7 @@ public void verifyPeerCertificate(X509Certificate[] peerCertChain, String authTy .clientAuth(ClientAuth.REQUIRE).build(); server = Grpc.newServerBuilderForPort(0, serverCredentials).addService( new SimpleServiceImpl()).build().start(); + TimeUnit.SECONDS.sleep(5); // Create a client to connect. AdvancedTlsX509KeyManager clientKeyManager = new AdvancedTlsX509KeyManager(); clientKeyManager.updateIdentityCredentials(clientKey0, clientCert0); @@ -311,6 +313,7 @@ public void onFileReloadingKeyManagerTrustManagerTest() throws Exception { .clientAuth(ClientAuth.REQUIRE).build(); server = Grpc.newServerBuilderForPort(0, serverCredentials).addService( new SimpleServiceImpl()).build().start(); + TimeUnit.SECONDS.sleep(5); // Create a client to connect. AdvancedTlsX509KeyManager clientKeyManager = new AdvancedTlsX509KeyManager(); Closeable clientKeyShutdown = clientKeyManager.updateIdentityCredentialsFromFile(clientKey0File, From 33885482a4f83d5247ae5a331a49413713cc788b Mon Sep 17 00:00:00 2001 From: ZhenLian Date: Fri, 28 May 2021 22:40:48 -0700 Subject: [PATCH 05/12] resolve some of the comments --- .../grpc/util/AdvancedTlsX509KeyManager.java | 49 ++++++----- .../util/AdvancedTlsX509TrustManager.java | 88 ++++++++----------- .../java/io/grpc/netty/AdvancedTlsTest.java | 73 ++++++++------- 3 files changed, 99 insertions(+), 111 deletions(-) diff --git a/core/src/main/java/io/grpc/util/AdvancedTlsX509KeyManager.java b/core/src/main/java/io/grpc/util/AdvancedTlsX509KeyManager.java index 9b971033870..facbe13e514 100644 --- a/core/src/main/java/io/grpc/util/AdvancedTlsX509KeyManager.java +++ b/core/src/main/java/io/grpc/util/AdvancedTlsX509KeyManager.java @@ -27,6 +27,7 @@ import java.security.cert.CertificateException; import java.security.cert.X509Certificate; import java.security.spec.InvalidKeySpecException; +import java.util.Arrays; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.ScheduledFuture; import java.util.concurrent.TimeUnit; @@ -35,11 +36,11 @@ import javax.net.ssl.SSLEngine; import javax.net.ssl.X509ExtendedKeyManager; -@ExperimentalApi("https://github.com/grpc/grpc-java/issues/8024") /** * AdvancedTlsX509KeyManager is an {@code X509ExtendedKeyManager} that allows users to configure * advanced TLS features, such as private key and certificate chain reloading, etc. */ +@ExperimentalApi("https://github.com/grpc/grpc-java/issues/8024") public final class AdvancedTlsX509KeyManager extends X509ExtendedKeyManager { private static final Logger log = Logger.getLogger(AdvancedTlsX509KeyManager.class.getName()); @@ -57,12 +58,12 @@ public PrivateKey getPrivateKey(String alias) { @Override public X509Certificate[] getCertificateChain(String alias) { - return this.certs; + return Arrays.copyOf(this.certs, this.certs.length); } @Override public String[] getClientAliases(String keyType, Principal[] issuers) { - return new String[0]; + return new String[] {"default"}; } @Override @@ -77,7 +78,7 @@ public String chooseEngineClientAlias(String[] keyType, Principal[] issuers, SSL @Override public String[] getServerAliases(String keyType, Principal[] issuers) { - return new String[0]; + return new String[] {"default"}; } @Override @@ -93,38 +94,36 @@ public String chooseEngineServerAlias(String keyType, Principal[] issuers, /** * Updates the current cached private key and cert chains. - * Right now, callers should make sure |key| matches the public key on the leaf certificate of - * |certs|. - * TODO(ZhenLian): explore possibilities to do a crypto check here. * * @param key the private key that is going to be used * @param certs the certificate chain that is going to be used */ public void updateIdentityCredentials(PrivateKey key, X509Certificate[] certs) { // Same question as the trust manager: what to do if copy is not feasible here? + // Right now, callers should make sure {@code key} matches the public key on the leaf + // certificate of {@code certs}. + // TODO(ZhenLian): explore possibilities to do a crypto check here. this.key = key; - this.certs = certs; + this.certs = Arrays.copyOf(certs, certs.length); } /** - * starts a {@code ScheduledExecutorService} to read private key and certificate chains from the - * local file paths periodically, and update the cached identity credentials if they are both + * Schedules a {@code ScheduledExecutorService} to read private key and certificate chains from + * the local file paths periodically, and update the cached identity credentials if they are both * updated. * * @param keyFile the file on disk holding the private key * @param certFile the file on disk holding the certificate chain - * @param initialDelay the time to delay first read-and-update execution - * @param delay the period between successive read-and-update executions + * @param period the period between successive read-and-update executions * @param unit the time unit of the initialDelay and period parameters - * @param executor the execute service we use to start the read-and-update thread - * @return an object that caller should close when the scheduled execution is not needed + * @param executor the execute service we use to read and update the credentials + * @return an object that caller should close when the file refreshes are not needed */ public Closeable updateIdentityCredentialsFromFile(File keyFile, File certFile, - long initialDelay, long delay, TimeUnit unit, ScheduledExecutorService executor) { + long period, TimeUnit unit, ScheduledExecutorService executor) { final ScheduledFuture future = executor.scheduleWithFixedDelay( - new LoadFilePathExecution(keyFile, certFile), - initialDelay, delay, unit); + new LoadFilePathExecution(keyFile, certFile), 0, period, unit); return new Closeable() { @Override public void close() { future.cancel(false); @@ -174,8 +173,8 @@ public UpdateResult(boolean success, long keyTime, long certTime) { } /** - * reads the private key and certificates specified in the path locations. Updates |key| and - * |cert| if both of their modified time changed since last read. + * Reads the private key and certificates specified in the path locations. Updates {@code key} and + * {@code cert} if both of their modified time changed since last read. * * @param keyFile the file on disk holding the private key * @param certFile the file on disk holding the certificate chain @@ -189,15 +188,21 @@ private UpdateResult readAndUpdate(File keyFile, File certFile, long oldKeyTime, long newCertTime = certFile.lastModified(); // We only update when both the key and the certs are updated. if (newKeyTime != oldKeyTime && newCertTime != oldCertTime) { - PrivateKey key = CertificateUtils.getPrivateKey(new FileInputStream(keyFile)); - X509Certificate[] certs = CertificateUtils.getX509Certificates(new FileInputStream(certFile)); + FileInputStream keyInputStream = new FileInputStream(keyFile); + FileInputStream certInputStream = new FileInputStream(certFile); + PrivateKey key = CertificateUtils.getPrivateKey(keyInputStream); + X509Certificate[] certs = CertificateUtils.getX509Certificates(certInputStream); + keyInputStream.close(); + certInputStream.close(); updateIdentityCredentials(key, certs); return new UpdateResult(true, newKeyTime, newCertTime); } return new UpdateResult(false, -1, -1); } - // Mainly used to avoid throwing IO Exceptions in java.io.Closeable. + /** + * Mainly used to avoid throwing IO Exceptions in java.io.Closeable. + */ public interface Closeable extends java.io.Closeable { @Override public void close(); } diff --git a/core/src/main/java/io/grpc/util/AdvancedTlsX509TrustManager.java b/core/src/main/java/io/grpc/util/AdvancedTlsX509TrustManager.java index 135ca6e6d33..21997cffd4f 100644 --- a/core/src/main/java/io/grpc/util/AdvancedTlsX509TrustManager.java +++ b/core/src/main/java/io/grpc/util/AdvancedTlsX509TrustManager.java @@ -19,7 +19,6 @@ import io.grpc.ExperimentalApi; import java.io.File; import java.io.FileInputStream; -import java.io.FileNotFoundException; import java.io.IOException; import java.net.Socket; import java.security.KeyStore; @@ -27,6 +26,7 @@ import java.security.NoSuchAlgorithmException; import java.security.cert.CertificateException; import java.security.cert.X509Certificate; +import java.util.Arrays; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.ScheduledFuture; import java.util.concurrent.TimeUnit; @@ -39,29 +39,27 @@ import javax.net.ssl.X509ExtendedTrustManager; import org.codehaus.mojo.animal_sniffer.IgnoreJRERequirement; -@ExperimentalApi("https://github.com/grpc/grpc-java/issues/8024") /** - * For Android users: this class is only supported in API level 24 and above. * AdvancedTlsX509TrustManager is an {@code X509ExtendedTrustManager} that allows users to configure * advanced TLS features, such as root certificate reloading, peer cert custom verification, etc. + * For Android users: this class is only supported in API level 24 and above. */ +@ExperimentalApi("https://github.com/grpc/grpc-java/issues/8024") @IgnoreJRERequirement public final class AdvancedTlsX509TrustManager extends X509ExtendedTrustManager { private static final Logger log = Logger.getLogger(AdvancedTlsX509TrustManager.class.getName()); - private Verification verification; - private PeerVerifier peerVerifier; - private SslSocketPeerVerifier sslSocketPeerVerifier; - private SslEnginePeerVerifier sslEnginePeerVerifier; + private final Verification verification; + private final PeerVerifier peerVerifier; + private final SslSocketAndEnginePeerVerifier socketAndEnginePeerVerifier; // The trust certs we will use to verify peer certificate chain. private volatile X509Certificate[] trustCerts; private AdvancedTlsX509TrustManager(Verification verification, PeerVerifier peerVerifier, - SslSocketPeerVerifier socketPeerVerifier, SslEnginePeerVerifier enginePeerVerifier) { + SslSocketAndEnginePeerVerifier socketAndEnginePeerVerifier) { this.verification = verification; this.peerVerifier = peerVerifier; - this.sslSocketPeerVerifier = socketPeerVerifier; - this.sslEnginePeerVerifier = enginePeerVerifier; + this.socketAndEnginePeerVerifier = socketAndEnginePeerVerifier; } @Override @@ -121,7 +119,7 @@ public void updateTrustCredentials(X509Certificate[] trustCerts) { // Question: in design, we will do an copy of trustCerts, but that seems not easy, since // X509Certificate is an abstract class without copy constructor. // Can we document in the API that trustCerts shouldn't be modified once set? - this.trustCerts = trustCerts; + this.trustCerts = Arrays.copyOf(trustCerts, trustCerts.length); } private void checkTrusted(X509Certificate[] chain, String authType, SSLEngine sslEngine, @@ -198,37 +196,33 @@ private void checkTrusted(X509Certificate[] chain, String authType, SSLEngine ss } } // Perform the additional peer cert check. - if (sslEngine != null && sslEnginePeerVerifier != null) { - sslEnginePeerVerifier.verifyPeerCertificate(chain, authType, sslEngine); - } - if (socket != null && sslSocketPeerVerifier != null) { - sslSocketPeerVerifier.verifyPeerCertificate(chain, authType, socket); + if (sslEngine != null && socket != null && socketAndEnginePeerVerifier != null) { + socketAndEnginePeerVerifier.verifyPeerCertificate(chain, authType, socket); + socketAndEnginePeerVerifier.verifyPeerCertificate(chain, authType, sslEngine); } if (peerVerifier != null) { peerVerifier.verifyPeerCertificate(chain, authType); } - if (sslEnginePeerVerifier == null && sslSocketPeerVerifier == null && peerVerifier == null) { + if (socketAndEnginePeerVerifier == null && peerVerifier == null) { log.log(Level.INFO, "No peer verifier is specified"); } } /** - * starts a {@code ScheduledExecutorService} to read trust certificates from a local file path + * Schedules a {@code ScheduledExecutorService} to read trust certificates from a local file path * periodically, and update the cached trust certs if there is an update. * * @param trustCertFile the file on disk holding the trust certificates - * @param initialDelay the time to delay first read-and-update execution - * @param delay the period between successive read-and-update executions + * @param period the period between successive read-and-update executions * @param unit the time unit of the initialDelay and period parameters - * @param executor the execute service we use to start the read-and-update thread - * @return an object that caller should close when the scheduled execution is not needed + * @param executor the execute service we use to read and update the credentials + * @return an object that caller should close when the file refreshes are not needed */ - public Closeable updateTrustCredentialsFromFile(File trustCertFile, long initialDelay, - long delay, TimeUnit unit, ScheduledExecutorService executor) { + public Closeable updateTrustCredentialsFromFile(File trustCertFile, long period, TimeUnit unit, + ScheduledExecutorService executor) { final ScheduledFuture future = executor.scheduleWithFixedDelay( - new LoadFilePathExecution(trustCertFile), - initialDelay, delay, unit); + new LoadFilePathExecution(trustCertFile), 0, period, unit); return new Closeable() { @Override public void close() { future.cancel(false); @@ -252,7 +246,7 @@ public void run() { if (newUpdateTime != 0) { this.currentTime = newUpdateTime; } - } catch (FileNotFoundException | CertificateException e) { + } catch (CertificateException | IOException e) { log.log(Level.SEVERE, "readAndUpdate() execution thread failed", e); } @@ -268,11 +262,12 @@ public void run() { * @return 0 if failed or the modified time is not changed, otherwise the new modified time */ private long readAndUpdate(File trustCertFile, long oldTime) - throws FileNotFoundException, CertificateException { + throws CertificateException, IOException { long newTime = trustCertFile.lastModified(); if (newTime != oldTime) { - X509Certificate[] certificates = CertificateUtils.getX509Certificates( - new FileInputStream(trustCertFile)); + FileInputStream inputStream = new FileInputStream(trustCertFile); + X509Certificate[] certificates = CertificateUtils.getX509Certificates(inputStream); + inputStream.close(); updateTrustCredentials(certificates); return newTime; } @@ -328,9 +323,9 @@ void verifyPeerCertificate(X509Certificate[] peerCertChain, String authType) } // Additional custom peer verification check. - // It will be used when checkClientTrusted/checkServerTrusted is called with the {@code Socket} - // parameter. - public interface SslSocketPeerVerifier { + // It will be used when checkClientTrusted/checkServerTrusted is called with the {@code Socket} or + // the {@code SSLEngine} parameter. + public interface SslSocketAndEnginePeerVerifier { /** * Verifies the peer certificate chain. For more information, please refer to * {@code X509ExtendedTrustManager}. @@ -342,12 +337,7 @@ public interface SslSocketPeerVerifier { */ void verifyPeerCertificate(X509Certificate[] peerCertChain, String authType, Socket socket) throws CertificateException; - } - // Additional custom peer verification check. - // It will be used when checkClientTrusted/checkServerTrusted is called with the {@code SSLEngine} - // parameter. - public interface SslEnginePeerVerifier { /** * Verifies the peer certificate chain. For more information, please refer to * {@code X509ExtendedTrustManager}. @@ -363,28 +353,24 @@ void verifyPeerCertificate(X509Certificate[] peerCertChain, String authType, SSL public static final class Builder { - private Verification verification; + private Verification verification = Verification.CertificateAndHostNameVerification; private PeerVerifier peerVerifier; - private SslSocketPeerVerifier socketPeerVerifier; - private SslEnginePeerVerifier enginePeerVerifier; + private SslSocketAndEnginePeerVerifier socketAndEnginePeerVerifier; + + private Builder() {} public Builder setVerification(Verification verification) { this.verification = verification; return this; } - public Builder setPeerVerifier(PeerVerifier peerVerifier) { - this.peerVerifier = peerVerifier; - return this; - } - - public Builder setSslSocketPeerVerifier(SslSocketPeerVerifier peerVerifier) { - this.socketPeerVerifier = peerVerifier; + public Builder setPeerVerifier(PeerVerifier verifier) { + this.peerVerifier = verifier; return this; } - public Builder setSslEnginePeerVerifier(SslEnginePeerVerifier peerVerifier) { - this.enginePeerVerifier = peerVerifier; + public Builder setSslSocketAndEnginePeerVerifier(SslSocketAndEnginePeerVerifier verifier) { + this.socketAndEnginePeerVerifier = verifier; return this; } @@ -392,7 +378,7 @@ public Builder setSslEnginePeerVerifier(SslEnginePeerVerifier peerVerifier) { // be more secure? public AdvancedTlsX509TrustManager build() { return new AdvancedTlsX509TrustManager(this.verification, this.peerVerifier, - this.socketPeerVerifier, this.enginePeerVerifier); + this.socketAndEnginePeerVerifier); } } } diff --git a/netty/src/test/java/io/grpc/netty/AdvancedTlsTest.java b/netty/src/test/java/io/grpc/netty/AdvancedTlsTest.java index cc9ebdb7a5d..c134cde6ffe 100644 --- a/netty/src/test/java/io/grpc/netty/AdvancedTlsTest.java +++ b/netty/src/test/java/io/grpc/netty/AdvancedTlsTest.java @@ -36,8 +36,7 @@ import io.grpc.util.AdvancedTlsX509KeyManager; import io.grpc.util.AdvancedTlsX509TrustManager; import io.grpc.util.AdvancedTlsX509TrustManager.PeerVerifier; -import io.grpc.util.AdvancedTlsX509TrustManager.SslEnginePeerVerifier; -import io.grpc.util.AdvancedTlsX509TrustManager.SslSocketPeerVerifier; +import io.grpc.util.AdvancedTlsX509TrustManager.SslSocketAndEnginePeerVerifier; import io.grpc.util.AdvancedTlsX509TrustManager.Verification; import io.grpc.util.CertificateUtils; @@ -185,8 +184,20 @@ public void trustManagerCustomVerifierMutualTlsTest() throws Exception { serverKeyManager.updateIdentityCredentials(serverKey0, serverCert0); AdvancedTlsX509TrustManager serverTrustManager = AdvancedTlsX509TrustManager.newBuilder() .setVerification(Verification.CertificateOnlyVerification) - .setSslEnginePeerVerifier( - new SslEnginePeerVerifier() { + .setSslSocketAndEnginePeerVerifier( + new SslSocketAndEnginePeerVerifier() { + @Override + public void verifyPeerCertificate(X509Certificate[] peerCertChain, String authType, + Socket socket) throws CertificateException { + if (peerCertChain == null || peerCertChain.length == 0) { + throw new CertificateException("peerCertChain is empty"); + } + X509Certificate leafCert = peerCertChain[0]; + if (!leafCert.getSubjectDN().getName().contains("testclient")) { + throw new CertificateException("SslSocketAndEnginePeerVerifier failed"); + } + } + @Override public void verifyPeerCertificate(X509Certificate[] peerCertChain, String authType, SSLEngine engine) throws CertificateException { @@ -195,23 +206,10 @@ public void verifyPeerCertificate(X509Certificate[] peerCertChain, String authTy } X509Certificate leafCert = peerCertChain[0]; if (!leafCert.getSubjectDN().getName().contains("testclient")) { - throw new CertificateException("SSLEnginePeerVerifier failed"); + throw new CertificateException("SslSocketAndEnginePeerVerifier failed"); } } }) - .setSslSocketPeerVerifier(new SslSocketPeerVerifier() { - @Override - public void verifyPeerCertificate(X509Certificate[] peerCertChain, String authType, - Socket socket) throws CertificateException { - if (peerCertChain == null || peerCertChain.length == 0) { - throw new CertificateException("peerCertChain is empty"); - } - X509Certificate leafCert = peerCertChain[0]; - if (!leafCert.getSubjectDN().getName().contains("testclient")) { - throw new CertificateException("SSLSocketPeerVerifier failed"); - } - } - }) .setPeerVerifier(new PeerVerifier() { @Override public void verifyPeerCertificate(X509Certificate[] peerCertChain, String authType) @@ -238,8 +236,20 @@ public void verifyPeerCertificate(X509Certificate[] peerCertChain, String authTy clientKeyManager.updateIdentityCredentials(clientKey0, clientCert0); AdvancedTlsX509TrustManager clientTrustManager = AdvancedTlsX509TrustManager.newBuilder() .setVerification(Verification.CertificateOnlyVerification) - .setSslEnginePeerVerifier( - new SslEnginePeerVerifier() { + .setSslSocketAndEnginePeerVerifier( + new SslSocketAndEnginePeerVerifier() { + @Override + public void verifyPeerCertificate(X509Certificate[] peerCertChain, String authType, + Socket socket) throws CertificateException { + if (peerCertChain == null || peerCertChain.length == 0) { + throw new CertificateException("peerCertChain is empty"); + } + X509Certificate leafCert = peerCertChain[0]; + if (!leafCert.getSubjectDN().getName().contains("*.test.google.com.au")) { + throw new CertificateException("SslSocketAndEnginePeerVerifier failed"); + } + } + @Override public void verifyPeerCertificate(X509Certificate[] peerCertChain, String authType, SSLEngine engine) throws CertificateException { @@ -248,23 +258,10 @@ public void verifyPeerCertificate(X509Certificate[] peerCertChain, String authTy } X509Certificate leafCert = peerCertChain[0]; if (!leafCert.getSubjectDN().getName().contains("*.test.google.com.au")) { - throw new CertificateException("SSLEnginePeerVerifier failed"); + throw new CertificateException("SslSocketAndEnginePeerVerifier failed"); } } }) - .setSslSocketPeerVerifier(new SslSocketPeerVerifier() { - @Override - public void verifyPeerCertificate(X509Certificate[] peerCertChain, String authType, - Socket socket) throws CertificateException { - if (peerCertChain == null || peerCertChain.length == 0) { - throw new CertificateException("peerCertChain is empty"); - } - X509Certificate leafCert = peerCertChain[0]; - if (!leafCert.getSubjectDN().getName().contains("*.test.google.com.au")) { - throw new CertificateException("SSLSocketPeerVerifier failed"); - } - } - }) .setPeerVerifier(new PeerVerifier() { @Override public void verifyPeerCertificate(X509Certificate[] peerCertChain, String authType) @@ -302,12 +299,12 @@ public void onFileReloadingKeyManagerTrustManagerTest() throws Exception { // Create & start a server. AdvancedTlsX509KeyManager serverKeyManager = new AdvancedTlsX509KeyManager(); Closeable serverKeyShutdown = serverKeyManager.updateIdentityCredentialsFromFile(serverKey0File, - serverCert0File, 0, 100, TimeUnit.MILLISECONDS, executor); + serverCert0File, 100, TimeUnit.MILLISECONDS, executor); AdvancedTlsX509TrustManager serverTrustManager = AdvancedTlsX509TrustManager.newBuilder() .setVerification(Verification.CertificateOnlyVerification) .build(); Closeable serverTrustShutdown = serverTrustManager.updateTrustCredentialsFromFile(caCertFile, - 0, 100, TimeUnit.MILLISECONDS, executor); + 100, TimeUnit.MILLISECONDS, executor); ServerCredentials serverCredentials = TlsServerCredentials.newBuilder() .keyManager(serverKeyManager).trustManager(serverTrustManager) .clientAuth(ClientAuth.REQUIRE).build(); @@ -317,12 +314,12 @@ public void onFileReloadingKeyManagerTrustManagerTest() throws Exception { // Create a client to connect. AdvancedTlsX509KeyManager clientKeyManager = new AdvancedTlsX509KeyManager(); Closeable clientKeyShutdown = clientKeyManager.updateIdentityCredentialsFromFile(clientKey0File, - clientCert0File, 0, 100, TimeUnit.MILLISECONDS, executor); + clientCert0File, 100, TimeUnit.MILLISECONDS, executor); AdvancedTlsX509TrustManager clientTrustManager = AdvancedTlsX509TrustManager.newBuilder() .setVerification(Verification.CertificateAndHostNameVerification) .build(); Closeable clientTrustShutdown = clientTrustManager.updateTrustCredentialsFromFile(caCertFile, - 0, 100, TimeUnit.MILLISECONDS, executor); + 100, TimeUnit.MILLISECONDS, executor); ChannelCredentials channelCredentials = TlsChannelCredentials.newBuilder() .keyManager(clientKeyManager).trustManager(clientTrustManager).build(); channel = Grpc.newChannelBuilderForAddress("localhost", server.getPort(), channelCredentials) From 76ee5b628ec49e2fd8c3bda9361af6379d343497 Mon Sep 17 00:00:00 2001 From: ZhenLian Date: Tue, 22 Jun 2021 23:12:16 -0700 Subject: [PATCH 06/12] resolve comments; add more tests --- .../util/AdvancedTlsX509TrustManager.java | 85 ++++++++------ .../java/io/grpc/netty/AdvancedTlsTest.java | 107 ++++++++++++++++++ 2 files changed, 155 insertions(+), 37 deletions(-) diff --git a/core/src/main/java/io/grpc/util/AdvancedTlsX509TrustManager.java b/core/src/main/java/io/grpc/util/AdvancedTlsX509TrustManager.java index 21997cffd4f..c16f15b4894 100644 --- a/core/src/main/java/io/grpc/util/AdvancedTlsX509TrustManager.java +++ b/core/src/main/java/io/grpc/util/AdvancedTlsX509TrustManager.java @@ -54,12 +54,21 @@ public final class AdvancedTlsX509TrustManager extends X509ExtendedTrustManager private final SslSocketAndEnginePeerVerifier socketAndEnginePeerVerifier; // The trust certs we will use to verify peer certificate chain. private volatile X509Certificate[] trustCerts; + // The key store that is used to hold the trust certificates. It will always keep synced with + // trustCerts. + private volatile KeyStore ks; private AdvancedTlsX509TrustManager(Verification verification, PeerVerifier peerVerifier, - SslSocketAndEnginePeerVerifier socketAndEnginePeerVerifier) { + SslSocketAndEnginePeerVerifier socketAndEnginePeerVerifier) throws CertificateException { this.verification = verification; this.peerVerifier = peerVerifier; this.socketAndEnginePeerVerifier = socketAndEnginePeerVerifier; + try { + ks = KeyStore.getInstance(KeyStore.getDefaultType()); + ks.load(null, null); + } catch (KeyStoreException | IOException | NoSuchAlgorithmException | CertificateException e) { + throw new CertificateException("Failed to create KeyStore", e); + } } @Override @@ -108,18 +117,41 @@ public X509Certificate[] getAcceptedIssuers() { // updateTrustCredentialsFromFile()) should not be called. public void useSystemDefaultTrustCerts() { this.trustCerts = new X509Certificate[0]; + this.ks = null; } /** - * Updates the current cached trust certificates. + * Updates the current cached trust certificates as well as the key store. * * @param trustCerts the trust certificates that are going to be used */ - public void updateTrustCredentials(X509Certificate[] trustCerts) { - // Question: in design, we will do an copy of trustCerts, but that seems not easy, since - // X509Certificate is an abstract class without copy constructor. - // Can we document in the API that trustCerts shouldn't be modified once set? + public void updateTrustCredentials(X509Certificate[] trustCerts) throws KeyStoreException, + CertificateException { this.trustCerts = Arrays.copyOf(trustCerts, trustCerts.length); + // Clear the key store by re-creating it. + try { + ks = KeyStore.getInstance(KeyStore.getDefaultType()); + ks.load(null, null); + } catch (KeyStoreException | IOException | NoSuchAlgorithmException | CertificateException e) { + throw new CertificateException("Failed to create KeyStore", e); + } + // Update the ks with the new credential contents. + int i = 1; + // Question: I know we've already made this.trustCerts volatile, but shouldn't we require a + // lock around it as well? Otherwise, while we are reading here, the updating thread might + // change it as well? + // I could be wrong, but my current understanding about "volatile" is that it guarantees we + // store value in memory instead of cache, so there wouldn't be problems like dirty read, + // etc, but it doesn't eliminate race conditions. + for (X509Certificate cert: this.trustCerts) { + String alias = Integer.toString(i); + try { + ks.setCertificateEntry(alias, cert); + } catch (KeyStoreException e) { + throw new CertificateException("Failed to load trust certificate into KeyStore", e); + } + i++; + } } private void checkTrusted(X509Certificate[] chain, String authType, SSLEngine sslEngine, @@ -130,30 +162,6 @@ private void checkTrusted(X509Certificate[] chain, String authType, SSLEngine ss throw new CertificateException( "Want certificate verification but got null or empty certificates"); } - // Set the cached trust certificates into a key store. - KeyStore ks; - try { - ks = KeyStore.getInstance(KeyStore.getDefaultType()); - ks.load(null, null); - } catch (KeyStoreException | IOException | NoSuchAlgorithmException e) { - throw new CertificateException("Failed to create KeyStore", e); - } - int i = 1; - // Question: I know we've already made this.trustCerts volatile, but shouldn't we require a - // lock around it as well? Otherwise, while we are reading here, the updating thread might - // change it as well? - // I could be wrong, but my current understanding about "volatile" is that it guarantees we - // store value in memory instead of cache, so there wouldn't be problems like dirty read, - // etc, but it doesn't eliminate race conditions. - for (X509Certificate cert: this.trustCerts) { - String alias = Integer.toString(i); - try { - ks.setCertificateEntry(alias, cert); - } catch (KeyStoreException e) { - throw new CertificateException("Failed to load trust certificate into KeyStore", e); - } - i++; - } // Use the key store to create a delegated trust manager. X509ExtendedTrustManager delegateManager = null; TrustManagerFactory tmf; @@ -166,7 +174,7 @@ private void checkTrusted(X509Certificate[] chain, String authType, SSLEngine ss TrustManager[] tms = tmf.getTrustManagers(); // Iterate over the returned trust managers, looking for an instance of X509TrustManager. // If found, use that as the delegate trust manager. - for (i = 0; i < tms.length; i++) { + for (int i = 0; i < tms.length; i++) { if (tms[i] instanceof X509ExtendedTrustManager) { delegateManager = (X509ExtendedTrustManager) tms[i]; break; @@ -180,7 +188,8 @@ private void checkTrusted(X509Certificate[] chain, String authType, SSLEngine ss if (checkingServer) { if (this.verification == Verification.CertificateAndHostNameVerification || this.verification == Verification.CertificateOnlyVerification) { - if (sslEngine == null) { + if (this.verification == Verification.CertificateAndHostNameVerification + && sslEngine == null) { throw new CertificateException( "SSLEngine is null. Couldn't check host name"); } @@ -196,10 +205,12 @@ private void checkTrusted(X509Certificate[] chain, String authType, SSLEngine ss } } // Perform the additional peer cert check. - if (sslEngine != null && socket != null && socketAndEnginePeerVerifier != null) { - socketAndEnginePeerVerifier.verifyPeerCertificate(chain, authType, socket); + if (sslEngine != null && socketAndEnginePeerVerifier != null) { socketAndEnginePeerVerifier.verifyPeerCertificate(chain, authType, sslEngine); } + if (socket != null && socketAndEnginePeerVerifier != null) { + socketAndEnginePeerVerifier.verifyPeerCertificate(chain, authType, socket); + } if (peerVerifier != null) { peerVerifier.verifyPeerCertificate(chain, authType); } @@ -246,7 +257,7 @@ public void run() { if (newUpdateTime != 0) { this.currentTime = newUpdateTime; } - } catch (CertificateException | IOException e) { + } catch (CertificateException | IOException | KeyStoreException e) { log.log(Level.SEVERE, "readAndUpdate() execution thread failed", e); } @@ -262,7 +273,7 @@ public void run() { * @return 0 if failed or the modified time is not changed, otherwise the new modified time */ private long readAndUpdate(File trustCertFile, long oldTime) - throws CertificateException, IOException { + throws CertificateException, IOException, KeyStoreException { long newTime = trustCertFile.lastModified(); if (newTime != oldTime) { FileInputStream inputStream = new FileInputStream(trustCertFile); @@ -376,7 +387,7 @@ public Builder setSslSocketAndEnginePeerVerifier(SslSocketAndEnginePeerVerifier // Question: shall we do some sanity checks here, to make sure all three verifiers are set, to // be more secure? - public AdvancedTlsX509TrustManager build() { + public AdvancedTlsX509TrustManager build() throws CertificateException { return new AdvancedTlsX509TrustManager(this.verification, this.peerVerifier, this.socketAndEnginePeerVerifier); } diff --git a/netty/src/test/java/io/grpc/netty/AdvancedTlsTest.java b/netty/src/test/java/io/grpc/netty/AdvancedTlsTest.java index c134cde6ffe..739065303e2 100644 --- a/netty/src/test/java/io/grpc/netty/AdvancedTlsTest.java +++ b/netty/src/test/java/io/grpc/netty/AdvancedTlsTest.java @@ -16,6 +16,8 @@ package io.grpc.netty; +import static org.junit.Assert.assertArrayEquals; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.fail; import com.google.common.util.concurrent.MoreExecutors; @@ -55,7 +57,9 @@ import javax.net.ssl.SSLEngine; import org.junit.After; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.ExpectedException; public class AdvancedTlsTest { public static final String SERVER_0_KEY_FILE = "server0.key"; @@ -79,6 +83,9 @@ public class AdvancedTlsTest { private PrivateKey clientKey0; private X509Certificate[] clientCert0; + @Rule + public ExpectedException exceptionRule = ExpectedException.none(); + @Before public void setUp() throws NoSuchAlgorithmException, IOException, CertificateException, InvalidKeySpecException { @@ -342,6 +349,106 @@ public void onFileReloadingKeyManagerTrustManagerTest() throws Exception { clientTrustShutdown.close(); } + @Test + public void keyManagerAliasesTest() throws Exception { + AdvancedTlsX509KeyManager km = new AdvancedTlsX509KeyManager(); + assertArrayEquals( + new String[] {"default"}, km.getClientAliases("", null)); + assertEquals( + "default", km.chooseClientAlias(new String[] {"default"}, null, null)); + assertArrayEquals( + new String[] {"default"}, km.getServerAliases("", null)); + assertEquals( + "default", km.chooseServerAlias("default", null, null)); + } + + @Test + public void trustManagerCheckTrustTest() throws Exception { + AdvancedTlsX509TrustManager tm = AdvancedTlsX509TrustManager.newBuilder() + .setVerification(Verification.InsecurelySkipAllVerification) + .setSslSocketAndEnginePeerVerifier( + new SslSocketAndEnginePeerVerifier() { + @Override + public void verifyPeerCertificate(X509Certificate[] peerCertChain, String authType, + Socket socket) throws CertificateException { } + + @Override + public void verifyPeerCertificate(X509Certificate[] peerCertChain, String authType, + SSLEngine engine) throws CertificateException { } + }) + .setPeerVerifier(new PeerVerifier() { + @Override + public void verifyPeerCertificate(X509Certificate[] peerCertChain, String authType) + throws CertificateException { } + }) + .build(); + tm.updateTrustCredentials(caCert); + tm.checkClientTrusted(serverCert0, "RSA", new Socket()); + tm.checkClientTrusted(serverCert0, "RSA"); + tm.useSystemDefaultTrustCerts(); + tm.checkServerTrusted(clientCert0, "RSA", new Socket()); + tm.checkServerTrusted(clientCert0, "RSA"); + } + + @Test + public void trustManagerEmptyChainTest() throws Exception { + exceptionRule.expect(CertificateException.class); + exceptionRule.expectMessage( + "Want certificate verification but got null or empty certificates"); + AdvancedTlsX509TrustManager tm = AdvancedTlsX509TrustManager.newBuilder() + .setVerification(Verification.CertificateOnlyVerification) + .setSslSocketAndEnginePeerVerifier( + new SslSocketAndEnginePeerVerifier() { + @Override + public void verifyPeerCertificate(X509Certificate[] peerCertChain, String authType, + Socket socket) throws CertificateException { } + + @Override + public void verifyPeerCertificate(X509Certificate[] peerCertChain, String authType, + SSLEngine engine) throws CertificateException { } + }) + .setPeerVerifier(new PeerVerifier() { + @Override + public void verifyPeerCertificate(X509Certificate[] peerCertChain, String authType) + throws CertificateException { } + }) + .build(); + tm.updateTrustCredentials(caCert); + tm.checkClientTrusted(null, "RSA"); + } + + @Test + public void trustManagerBadCustomVerificationTest() throws Exception { + exceptionRule.expect(CertificateException.class); + exceptionRule.expectMessage("Bad Custom Verification"); + AdvancedTlsX509TrustManager tm = AdvancedTlsX509TrustManager.newBuilder() + .setVerification(Verification.CertificateOnlyVerification) + .setSslSocketAndEnginePeerVerifier( + new SslSocketAndEnginePeerVerifier() { + @Override + public void verifyPeerCertificate(X509Certificate[] peerCertChain, String authType, + Socket socket) throws CertificateException { + throw new CertificateException("Bad Custom Verification"); + } + + @Override + public void verifyPeerCertificate(X509Certificate[] peerCertChain, String authType, + SSLEngine engine) throws CertificateException { + throw new CertificateException("Bad Custom Verification"); + } + }) + .setPeerVerifier(new PeerVerifier() { + @Override + public void verifyPeerCertificate(X509Certificate[] peerCertChain, String authType) + throws CertificateException { + throw new CertificateException("Bad Custom Verification"); + } + }) + .build(); + tm.updateTrustCredentials(caCert); + tm.checkClientTrusted(serverCert0, "RSA", new Socket()); + } + private static class SimpleServiceImpl extends SimpleServiceGrpc.SimpleServiceImplBase { @Override public void unaryRpc(SimpleRequest req, StreamObserver respOb) { From b196a1208f6058aa9e1f532ad2923b60167eda10 Mon Sep 17 00:00:00 2001 From: ZhenLian Date: Wed, 23 Jun 2021 10:32:37 -0700 Subject: [PATCH 07/12] fix the build issue --- core/BUILD.bazel | 1 + 1 file changed, 1 insertion(+) diff --git a/core/BUILD.bazel b/core/BUILD.bazel index c6e1ffbb6d7..03bdd13189e 100644 --- a/core/BUILD.bazel +++ b/core/BUILD.bazel @@ -58,6 +58,7 @@ java_library( "@com_google_code_findbugs_jsr305//jar", "@com_google_guava_guava//jar", "@com_google_j2objc_j2objc_annotations//jar", + "@org_codehaus_mojo_animal_sniffer_annotations//jar", ], ) From 321b2c4948923ac71bdf80e14574c3fc5d961c0c Mon Sep 17 00:00:00 2001 From: ZhenLian Date: Fri, 13 Aug 2021 14:49:50 -0700 Subject: [PATCH 08/12] change to use key store only; fix the latest comments --- .../grpc/util/AdvancedTlsX509KeyManager.java | 132 ++++++++++++++---- .../util/AdvancedTlsX509TrustManager.java | 93 ++++++------ .../java/io/grpc/netty/AdvancedTlsTest.java | 12 +- 3 files changed, 167 insertions(+), 70 deletions(-) diff --git a/core/src/main/java/io/grpc/util/AdvancedTlsX509KeyManager.java b/core/src/main/java/io/grpc/util/AdvancedTlsX509KeyManager.java index facbe13e514..0c01b75417b 100644 --- a/core/src/main/java/io/grpc/util/AdvancedTlsX509KeyManager.java +++ b/core/src/main/java/io/grpc/util/AdvancedTlsX509KeyManager.java @@ -21,13 +21,19 @@ import java.io.FileInputStream; import java.io.IOException; import java.net.Socket; +import java.security.KeyStore; +import java.security.KeyStore.PrivateKeyEntry; +import java.security.KeyStoreException; import java.security.NoSuchAlgorithmException; import java.security.Principal; import java.security.PrivateKey; +import java.security.UnrecoverableEntryException; +import java.security.cert.Certificate; import java.security.cert.CertificateException; import java.security.cert.X509Certificate; import java.security.spec.InvalidKeySpecException; -import java.util.Arrays; +import java.util.ArrayList; +import java.util.List; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.ScheduledFuture; import java.util.concurrent.TimeUnit; @@ -44,21 +50,64 @@ public final class AdvancedTlsX509KeyManager extends X509ExtendedKeyManager { private static final Logger log = Logger.getLogger(AdvancedTlsX509KeyManager.class.getName()); - // The private key and the cert chain we will use to send to peers to prove our identity. - private volatile PrivateKey key; - private volatile X509Certificate[] certs; + // The key store that is used to hold the private key and the certificate chain. + private volatile KeyStore ks; + // The password associated with the private key. + private volatile String password = null; - public AdvancedTlsX509KeyManager() { } + /** + * Constructs an AdvancedTlsX509KeyManager. + */ + public AdvancedTlsX509KeyManager() throws CertificateException { + try { + ks = KeyStore.getInstance(KeyStore.getDefaultType()); + ks.load(null, null); + } catch (KeyStoreException | IOException | NoSuchAlgorithmException | CertificateException e) { + throw new CertificateException("Failed to create KeyStore", e); + } + } @Override public PrivateKey getPrivateKey(String alias) { - // Same question as the trust manager: is it really thread-safe to do this? - return this.key; + KeyStore.Entry entry = null; + try { + entry = ks.getEntry(alias, new KeyStore.PasswordProtection(this.password.toCharArray())); + } catch (NoSuchAlgorithmException | UnrecoverableEntryException | KeyStoreException e) { + log.log(Level.SEVERE, "Unable to get the key entry from the key store", e); + return null; + } + if (entry == null || !(entry instanceof PrivateKeyEntry)) { + log.log(Level.SEVERE, "Failed to get the actual entry"); + return null; + } + PrivateKeyEntry privateKeyEntry = (PrivateKeyEntry) entry; + return privateKeyEntry.getPrivateKey(); } @Override public X509Certificate[] getCertificateChain(String alias) { - return Arrays.copyOf(this.certs, this.certs.length); + KeyStore.Entry entry = null; + try { + entry = ks.getEntry(alias, new KeyStore.PasswordProtection(this.password.toCharArray())); + } catch (NoSuchAlgorithmException | UnrecoverableEntryException | KeyStoreException e) { + log.log(Level.SEVERE, "Unable to get the key entry from the key store", e); + return null; + } + if (entry == null || !(entry instanceof PrivateKeyEntry)) { + log.log(Level.SEVERE, "Failed to get the actual entry"); + return null; + } + PrivateKeyEntry privateKeyEntry = (PrivateKeyEntry) entry; + Certificate[] certs = privateKeyEntry.getCertificateChain(); + List returnedCerts = new ArrayList<>(); + for (int i = 0; i < certs.length; ++i) { + if (certs[i] instanceof X509Certificate) { + returnedCerts.add((X509Certificate) certs[i]); + } else { + log.log(Level.SEVERE, "The certificate is not type of X509Certificate. Skip it"); + } + } + return returnedCerts.toArray(new X509Certificate[0]); } @Override @@ -97,14 +146,32 @@ public String chooseEngineServerAlias(String keyType, Principal[] issuers, * * @param key the private key that is going to be used * @param certs the certificate chain that is going to be used + * @param password the password associated with the key */ - public void updateIdentityCredentials(PrivateKey key, X509Certificate[] certs) { - // Same question as the trust manager: what to do if copy is not feasible here? - // Right now, callers should make sure {@code key} matches the public key on the leaf - // certificate of {@code certs}. + public void updateIdentityCredentials(PrivateKey key, X509Certificate[] certs, String password) + throws CertificateException { // TODO(ZhenLian): explore possibilities to do a crypto check here. - this.key = key; - this.certs = Arrays.copyOf(certs, certs.length); + // Clear the key store by re-creating it. + KeyStore newKeyStore = null; + try { + newKeyStore = KeyStore.getInstance(KeyStore.getDefaultType()); + newKeyStore.load(null, null); + } catch (KeyStoreException | IOException | NoSuchAlgorithmException | CertificateException e) { + throw new CertificateException("Failed to create KeyStore", e); + } + if (newKeyStore != null) { + this.ks = newKeyStore; + } + this.password = password; + // Update the ks with the new credential contents. + try { + PrivateKeyEntry privateKeyEntry = new PrivateKeyEntry(key, certs); + ks.setEntry("default", privateKeyEntry, new KeyStore.PasswordProtection( + this.password.toCharArray())); + } catch (KeyStoreException e) { + throw new CertificateException( + "Failed to load private key and certificates into KeyStore", e); + } } /** @@ -114,13 +181,15 @@ public void updateIdentityCredentials(PrivateKey key, X509Certificate[] certs) { * * @param keyFile the file on disk holding the private key * @param certFile the file on disk holding the certificate chain + * @param password the password associated with the key * @param period the period between successive read-and-update executions * @param unit the time unit of the initialDelay and period parameters * @param executor the execute service we use to read and update the credentials * @return an object that caller should close when the file refreshes are not needed */ public Closeable updateIdentityCredentialsFromFile(File keyFile, File certFile, - long period, TimeUnit unit, ScheduledExecutorService executor) { + String password, long period, TimeUnit unit, ScheduledExecutorService executor) { + this.password = password; final ScheduledFuture future = executor.scheduleWithFixedDelay( new LoadFilePathExecution(keyFile, certFile), 0, period, unit); @@ -155,7 +224,8 @@ public void run() { } } catch (CertificateException | IOException | NoSuchAlgorithmException | InvalidKeySpecException e) { - log.log(Level.SEVERE, "readAndUpdate() execution thread failed", e); + log.log(Level.SEVERE, "Failed refreshing private key and certificate chain from files. " + + "Using previous ones", e); } } } @@ -188,16 +258,28 @@ private UpdateResult readAndUpdate(File keyFile, File certFile, long oldKeyTime, long newCertTime = certFile.lastModified(); // We only update when both the key and the certs are updated. if (newKeyTime != oldKeyTime && newCertTime != oldCertTime) { - FileInputStream keyInputStream = new FileInputStream(keyFile); - FileInputStream certInputStream = new FileInputStream(certFile); - PrivateKey key = CertificateUtils.getPrivateKey(keyInputStream); - X509Certificate[] certs = CertificateUtils.getX509Certificates(certInputStream); - keyInputStream.close(); - certInputStream.close(); - updateIdentityCredentials(key, certs); - return new UpdateResult(true, newKeyTime, newCertTime); + FileInputStream keyInputStream = null; + try { + keyInputStream = new FileInputStream(keyFile); + PrivateKey key = CertificateUtils.getPrivateKey(keyInputStream); + FileInputStream certInputStream = null; + try { + certInputStream = new FileInputStream(certFile); + X509Certificate[] certs = CertificateUtils.getX509Certificates(certInputStream); + updateIdentityCredentials(key, certs, this.password); + return new UpdateResult(true, newKeyTime, newCertTime); + } finally { + if (certInputStream != null) { + certInputStream.close(); + } + } + } finally { + if (keyInputStream != null) { + keyInputStream.close(); + } + } } - return new UpdateResult(false, -1, -1); + return new UpdateResult(false, oldKeyTime, oldCertTime); } /** diff --git a/core/src/main/java/io/grpc/util/AdvancedTlsX509TrustManager.java b/core/src/main/java/io/grpc/util/AdvancedTlsX509TrustManager.java index c16f15b4894..1a1da50c9d6 100644 --- a/core/src/main/java/io/grpc/util/AdvancedTlsX509TrustManager.java +++ b/core/src/main/java/io/grpc/util/AdvancedTlsX509TrustManager.java @@ -24,9 +24,11 @@ import java.security.KeyStore; import java.security.KeyStoreException; import java.security.NoSuchAlgorithmException; +import java.security.cert.Certificate; import java.security.cert.CertificateException; import java.security.cert.X509Certificate; -import java.util.Arrays; +import java.util.ArrayList; +import java.util.List; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.ScheduledFuture; import java.util.concurrent.TimeUnit; @@ -52,14 +54,15 @@ public final class AdvancedTlsX509TrustManager extends X509ExtendedTrustManager private final Verification verification; private final PeerVerifier peerVerifier; private final SslSocketAndEnginePeerVerifier socketAndEnginePeerVerifier; - // The trust certs we will use to verify peer certificate chain. - private volatile X509Certificate[] trustCerts; + // The aliases we use in the key store to keep track of all the trust certificates. + private volatile List aliases; // The key store that is used to hold the trust certificates. It will always keep synced with // trustCerts. - private volatile KeyStore ks; + private volatile KeyStore ks; private AdvancedTlsX509TrustManager(Verification verification, PeerVerifier peerVerifier, SslSocketAndEnginePeerVerifier socketAndEnginePeerVerifier) throws CertificateException { + this.aliases = new ArrayList<>(); this.verification = verification; this.peerVerifier = peerVerifier; this.socketAndEnginePeerVerifier = socketAndEnginePeerVerifier; @@ -109,42 +112,53 @@ public void checkServerTrusted(X509Certificate[] chain, String authType, Socket @Override public X509Certificate[] getAcceptedIssuers() { - return trustCerts; + List trustCerts = new ArrayList<>(); + for (String alias : this.aliases) { + try { + Certificate cert = this.ks.getCertificate(alias); + if (cert instanceof X509Certificate) { + trustCerts.add((X509Certificate) cert); + } else { + log.log(Level.SEVERE, "The certificate is not type of X509Certificate. Skip it"); + } + } catch (KeyStoreException e) { + log.log(Level.SEVERE, "Failed to get the certificate from the key store", e); + } + } + return trustCerts.toArray(new X509Certificate[0]); + } // If this is set, we will use the default trust certificates stored on user's local system. - // After this is used, functions that will update this.trustCerts(e.g. updateTrustCredentials(), + // After this is used, functions that will update this.ks(e.g. updateTrustCredentials(), // updateTrustCredentialsFromFile()) should not be called. public void useSystemDefaultTrustCerts() { - this.trustCerts = new X509Certificate[0]; this.ks = null; } /** * Updates the current cached trust certificates as well as the key store. * - * @param trustCerts the trust certificates that are going to be used + * @param trustCerts the trust certificates that are going to be used */ - public void updateTrustCredentials(X509Certificate[] trustCerts) throws KeyStoreException, - CertificateException { - this.trustCerts = Arrays.copyOf(trustCerts, trustCerts.length); + public void updateTrustCredentials(X509Certificate[] trustCerts) throws CertificateException { // Clear the key store by re-creating it. + KeyStore newKeyStore = null; try { - ks = KeyStore.getInstance(KeyStore.getDefaultType()); - ks.load(null, null); + newKeyStore = KeyStore.getInstance(KeyStore.getDefaultType()); + newKeyStore.load(null, null); } catch (KeyStoreException | IOException | NoSuchAlgorithmException | CertificateException e) { throw new CertificateException("Failed to create KeyStore", e); } + if (newKeyStore != null) { + this.ks = newKeyStore; + } + this.aliases.clear(); // Update the ks with the new credential contents. int i = 1; - // Question: I know we've already made this.trustCerts volatile, but shouldn't we require a - // lock around it as well? Otherwise, while we are reading here, the updating thread might - // change it as well? - // I could be wrong, but my current understanding about "volatile" is that it guarantees we - // store value in memory instead of cache, so there wouldn't be problems like dirty read, - // etc, but it doesn't eliminate race conditions. - for (X509Certificate cert: this.trustCerts) { + for (X509Certificate cert: trustCerts) { String alias = Integer.toString(i); + this.aliases.add(alias); try { ks.setCertificateEntry(alias, cert); } catch (KeyStoreException e) { @@ -207,14 +221,11 @@ private void checkTrusted(X509Certificate[] chain, String authType, SSLEngine ss // Perform the additional peer cert check. if (sslEngine != null && socketAndEnginePeerVerifier != null) { socketAndEnginePeerVerifier.verifyPeerCertificate(chain, authType, sslEngine); - } - if (socket != null && socketAndEnginePeerVerifier != null) { + } else if (socket != null && socketAndEnginePeerVerifier != null) { socketAndEnginePeerVerifier.verifyPeerCertificate(chain, authType, socket); - } - if (peerVerifier != null) { + } else if (peerVerifier != null) { peerVerifier.verifyPeerCertificate(chain, authType); - } - if (socketAndEnginePeerVerifier == null && peerVerifier == null) { + } else { log.log(Level.INFO, "No peer verifier is specified"); } } @@ -254,35 +265,41 @@ public LoadFilePathExecution(File file) { public void run() { try { long newUpdateTime = readAndUpdate(this.file, this.currentTime); - if (newUpdateTime != 0) { + if (this.currentTime != newUpdateTime) { this.currentTime = newUpdateTime; } } catch (CertificateException | IOException | KeyStoreException e) { - log.log(Level.SEVERE, "readAndUpdate() execution thread failed", e); + log.log(Level.SEVERE, "Failed refreshing trust CAs from file. Using previous CAs", e); } } } /** - * reads the trust certificates specified in the path location, and update |trustCerts| if the + * Reads the trust certificates specified in the path location, and update the key store if the * modified time has changed since last read. * * @param trustCertFile the file on disk holding the trust certificates * @param oldTime the time when the trust file is modified during last execution - * @return 0 if failed or the modified time is not changed, otherwise the new modified time + * @return oldTime if failed or the modified time is not changed, otherwise the new modified time */ private long readAndUpdate(File trustCertFile, long oldTime) throws CertificateException, IOException, KeyStoreException { long newTime = trustCertFile.lastModified(); if (newTime != oldTime) { - FileInputStream inputStream = new FileInputStream(trustCertFile); - X509Certificate[] certificates = CertificateUtils.getX509Certificates(inputStream); - inputStream.close(); - updateTrustCredentials(certificates); - return newTime; + FileInputStream inputStream = null; + try { + inputStream = new FileInputStream(trustCertFile); + X509Certificate[] certificates = CertificateUtils.getX509Certificates(inputStream); + updateTrustCredentials(certificates); + return newTime; + } finally { + if (inputStream != null) { + inputStream.close(); + } + } } - return 0; + return oldTime; } // Mainly used to avoid throwing IO Exceptions in java.io.Closeable. @@ -291,7 +308,7 @@ public interface Closeable extends java.io.Closeable { } public static Builder newBuilder() { - return new Builder().setVerification(Verification.CertificateAndHostNameVerification); + return new Builder(); } // The verification mode when authenticating the peer certificate. @@ -385,8 +402,6 @@ public Builder setSslSocketAndEnginePeerVerifier(SslSocketAndEnginePeerVerifier return this; } - // Question: shall we do some sanity checks here, to make sure all three verifiers are set, to - // be more secure? public AdvancedTlsX509TrustManager build() throws CertificateException { return new AdvancedTlsX509TrustManager(this.verification, this.peerVerifier, this.socketAndEnginePeerVerifier); diff --git a/netty/src/test/java/io/grpc/netty/AdvancedTlsTest.java b/netty/src/test/java/io/grpc/netty/AdvancedTlsTest.java index 739065303e2..7fd52abde14 100644 --- a/netty/src/test/java/io/grpc/netty/AdvancedTlsTest.java +++ b/netty/src/test/java/io/grpc/netty/AdvancedTlsTest.java @@ -149,7 +149,7 @@ public void basicMutualTlsTest() throws Exception { public void advancedTlsKeyManagerTrustManagerMutualTlsTest() throws Exception { // Create & start a server. AdvancedTlsX509KeyManager serverKeyManager = new AdvancedTlsX509KeyManager(); - serverKeyManager.updateIdentityCredentials(serverKey0, serverCert0); + serverKeyManager.updateIdentityCredentials(serverKey0, serverCert0, ""); AdvancedTlsX509TrustManager serverTrustManager = AdvancedTlsX509TrustManager.newBuilder() .setVerification(Verification.CertificateOnlyVerification) .build(); @@ -162,7 +162,7 @@ public void advancedTlsKeyManagerTrustManagerMutualTlsTest() throws Exception { TimeUnit.SECONDS.sleep(5); // Create a client to connect. AdvancedTlsX509KeyManager clientKeyManager = new AdvancedTlsX509KeyManager(); - clientKeyManager.updateIdentityCredentials(clientKey0, clientCert0); + clientKeyManager.updateIdentityCredentials(clientKey0, clientCert0, ""); AdvancedTlsX509TrustManager clientTrustManager = AdvancedTlsX509TrustManager.newBuilder() .setVerification(Verification.CertificateAndHostNameVerification) .build(); @@ -188,7 +188,7 @@ public void advancedTlsKeyManagerTrustManagerMutualTlsTest() throws Exception { public void trustManagerCustomVerifierMutualTlsTest() throws Exception { // Create & start a server. AdvancedTlsX509KeyManager serverKeyManager = new AdvancedTlsX509KeyManager(); - serverKeyManager.updateIdentityCredentials(serverKey0, serverCert0); + serverKeyManager.updateIdentityCredentials(serverKey0, serverCert0, ""); AdvancedTlsX509TrustManager serverTrustManager = AdvancedTlsX509TrustManager.newBuilder() .setVerification(Verification.CertificateOnlyVerification) .setSslSocketAndEnginePeerVerifier( @@ -240,7 +240,7 @@ public void verifyPeerCertificate(X509Certificate[] peerCertChain, String authTy TimeUnit.SECONDS.sleep(5); // Create a client to connect. AdvancedTlsX509KeyManager clientKeyManager = new AdvancedTlsX509KeyManager(); - clientKeyManager.updateIdentityCredentials(clientKey0, clientCert0); + clientKeyManager.updateIdentityCredentials(clientKey0, clientCert0, ""); AdvancedTlsX509TrustManager clientTrustManager = AdvancedTlsX509TrustManager.newBuilder() .setVerification(Verification.CertificateOnlyVerification) .setSslSocketAndEnginePeerVerifier( @@ -306,7 +306,7 @@ public void onFileReloadingKeyManagerTrustManagerTest() throws Exception { // Create & start a server. AdvancedTlsX509KeyManager serverKeyManager = new AdvancedTlsX509KeyManager(); Closeable serverKeyShutdown = serverKeyManager.updateIdentityCredentialsFromFile(serverKey0File, - serverCert0File, 100, TimeUnit.MILLISECONDS, executor); + serverCert0File, "", 100, TimeUnit.MILLISECONDS, executor); AdvancedTlsX509TrustManager serverTrustManager = AdvancedTlsX509TrustManager.newBuilder() .setVerification(Verification.CertificateOnlyVerification) .build(); @@ -321,7 +321,7 @@ public void onFileReloadingKeyManagerTrustManagerTest() throws Exception { // Create a client to connect. AdvancedTlsX509KeyManager clientKeyManager = new AdvancedTlsX509KeyManager(); Closeable clientKeyShutdown = clientKeyManager.updateIdentityCredentialsFromFile(clientKey0File, - clientCert0File, 100, TimeUnit.MILLISECONDS, executor); + clientCert0File, "",100, TimeUnit.MILLISECONDS, executor); AdvancedTlsX509TrustManager clientTrustManager = AdvancedTlsX509TrustManager.newBuilder() .setVerification(Verification.CertificateAndHostNameVerification) .build(); From 8765c1ff1b4e060d9b917ca1bceba29b70551f6a Mon Sep 17 00:00:00 2001 From: ZhenLian Date: Sat, 14 Aug 2021 10:43:40 -0700 Subject: [PATCH 09/12] updates based on the discussion offline; add a new test --- .../grpc/util/AdvancedTlsX509KeyManager.java | 116 +++---------- .../util/AdvancedTlsX509TrustManager.java | 160 +++++++----------- .../java/io/grpc/netty/AdvancedTlsTest.java | 102 +++++++---- 3 files changed, 154 insertions(+), 224 deletions(-) diff --git a/core/src/main/java/io/grpc/util/AdvancedTlsX509KeyManager.java b/core/src/main/java/io/grpc/util/AdvancedTlsX509KeyManager.java index 0c01b75417b..ec6a5a9beae 100644 --- a/core/src/main/java/io/grpc/util/AdvancedTlsX509KeyManager.java +++ b/core/src/main/java/io/grpc/util/AdvancedTlsX509KeyManager.java @@ -21,19 +21,13 @@ import java.io.FileInputStream; import java.io.IOException; import java.net.Socket; -import java.security.KeyStore; -import java.security.KeyStore.PrivateKeyEntry; -import java.security.KeyStoreException; import java.security.NoSuchAlgorithmException; import java.security.Principal; import java.security.PrivateKey; -import java.security.UnrecoverableEntryException; -import java.security.cert.Certificate; import java.security.cert.CertificateException; import java.security.cert.X509Certificate; import java.security.spec.InvalidKeySpecException; -import java.util.ArrayList; -import java.util.List; +import java.util.Arrays; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.ScheduledFuture; import java.util.concurrent.TimeUnit; @@ -50,64 +44,22 @@ public final class AdvancedTlsX509KeyManager extends X509ExtendedKeyManager { private static final Logger log = Logger.getLogger(AdvancedTlsX509KeyManager.class.getName()); - // The key store that is used to hold the private key and the certificate chain. - private volatile KeyStore ks; - // The password associated with the private key. - private volatile String password = null; + // The credential information sent to peers to prove our identity. + private volatile KeyInfo keyInfo; /** * Constructs an AdvancedTlsX509KeyManager. */ - public AdvancedTlsX509KeyManager() throws CertificateException { - try { - ks = KeyStore.getInstance(KeyStore.getDefaultType()); - ks.load(null, null); - } catch (KeyStoreException | IOException | NoSuchAlgorithmException | CertificateException e) { - throw new CertificateException("Failed to create KeyStore", e); - } - } + public AdvancedTlsX509KeyManager() throws CertificateException { } @Override public PrivateKey getPrivateKey(String alias) { - KeyStore.Entry entry = null; - try { - entry = ks.getEntry(alias, new KeyStore.PasswordProtection(this.password.toCharArray())); - } catch (NoSuchAlgorithmException | UnrecoverableEntryException | KeyStoreException e) { - log.log(Level.SEVERE, "Unable to get the key entry from the key store", e); - return null; - } - if (entry == null || !(entry instanceof PrivateKeyEntry)) { - log.log(Level.SEVERE, "Failed to get the actual entry"); - return null; - } - PrivateKeyEntry privateKeyEntry = (PrivateKeyEntry) entry; - return privateKeyEntry.getPrivateKey(); + return this.keyInfo.key; } @Override public X509Certificate[] getCertificateChain(String alias) { - KeyStore.Entry entry = null; - try { - entry = ks.getEntry(alias, new KeyStore.PasswordProtection(this.password.toCharArray())); - } catch (NoSuchAlgorithmException | UnrecoverableEntryException | KeyStoreException e) { - log.log(Level.SEVERE, "Unable to get the key entry from the key store", e); - return null; - } - if (entry == null || !(entry instanceof PrivateKeyEntry)) { - log.log(Level.SEVERE, "Failed to get the actual entry"); - return null; - } - PrivateKeyEntry privateKeyEntry = (PrivateKeyEntry) entry; - Certificate[] certs = privateKeyEntry.getCertificateChain(); - List returnedCerts = new ArrayList<>(); - for (int i = 0; i < certs.length; ++i) { - if (certs[i] instanceof X509Certificate) { - returnedCerts.add((X509Certificate) certs[i]); - } else { - log.log(Level.SEVERE, "The certificate is not type of X509Certificate. Skip it"); - } - } - return returnedCerts.toArray(new X509Certificate[0]); + return Arrays.copyOf(this.keyInfo.certs, this.keyInfo.certs.length); } @Override @@ -146,32 +98,11 @@ public String chooseEngineServerAlias(String keyType, Principal[] issuers, * * @param key the private key that is going to be used * @param certs the certificate chain that is going to be used - * @param password the password associated with the key */ - public void updateIdentityCredentials(PrivateKey key, X509Certificate[] certs, String password) + public void updateIdentityCredentials(PrivateKey key, X509Certificate[] certs) throws CertificateException { // TODO(ZhenLian): explore possibilities to do a crypto check here. - // Clear the key store by re-creating it. - KeyStore newKeyStore = null; - try { - newKeyStore = KeyStore.getInstance(KeyStore.getDefaultType()); - newKeyStore.load(null, null); - } catch (KeyStoreException | IOException | NoSuchAlgorithmException | CertificateException e) { - throw new CertificateException("Failed to create KeyStore", e); - } - if (newKeyStore != null) { - this.ks = newKeyStore; - } - this.password = password; - // Update the ks with the new credential contents. - try { - PrivateKeyEntry privateKeyEntry = new PrivateKeyEntry(key, certs); - ks.setEntry("default", privateKeyEntry, new KeyStore.PasswordProtection( - this.password.toCharArray())); - } catch (KeyStoreException e) { - throw new CertificateException( - "Failed to load private key and certificates into KeyStore", e); - } + this.keyInfo = new KeyInfo(key, certs); } /** @@ -181,15 +112,13 @@ public void updateIdentityCredentials(PrivateKey key, X509Certificate[] certs, S * * @param keyFile the file on disk holding the private key * @param certFile the file on disk holding the certificate chain - * @param password the password associated with the key * @param period the period between successive read-and-update executions * @param unit the time unit of the initialDelay and period parameters * @param executor the execute service we use to read and update the credentials * @return an object that caller should close when the file refreshes are not needed */ public Closeable updateIdentityCredentialsFromFile(File keyFile, File certFile, - String password, long period, TimeUnit unit, ScheduledExecutorService executor) { - this.password = password; + long period, TimeUnit unit, ScheduledExecutorService executor) { final ScheduledFuture future = executor.scheduleWithFixedDelay( new LoadFilePathExecution(keyFile, certFile), 0, period, unit); @@ -200,6 +129,17 @@ public Closeable updateIdentityCredentialsFromFile(File keyFile, File certFile, }; } + private static class KeyInfo { + // The private key and the cert chain we will use to send to peers to prove our identity. + final PrivateKey key; + final X509Certificate[] certs; + + public KeyInfo(PrivateKey key, X509Certificate[] certs) { + this.key = key; + this.certs = certs; + } + } + private class LoadFilePathExecution implements Runnable { File keyFile; File certFile; @@ -258,25 +198,19 @@ private UpdateResult readAndUpdate(File keyFile, File certFile, long oldKeyTime, long newCertTime = certFile.lastModified(); // We only update when both the key and the certs are updated. if (newKeyTime != oldKeyTime && newCertTime != oldCertTime) { - FileInputStream keyInputStream = null; + FileInputStream keyInputStream = new FileInputStream(keyFile); try { - keyInputStream = new FileInputStream(keyFile); PrivateKey key = CertificateUtils.getPrivateKey(keyInputStream); - FileInputStream certInputStream = null; + FileInputStream certInputStream = new FileInputStream(certFile); try { - certInputStream = new FileInputStream(certFile); X509Certificate[] certs = CertificateUtils.getX509Certificates(certInputStream); - updateIdentityCredentials(key, certs, this.password); + updateIdentityCredentials(key, certs); return new UpdateResult(true, newKeyTime, newCertTime); } finally { - if (certInputStream != null) { - certInputStream.close(); - } + certInputStream.close(); } } finally { - if (keyInputStream != null) { - keyInputStream.close(); - } + keyInputStream.close(); } } return new UpdateResult(false, oldKeyTime, oldCertTime); diff --git a/core/src/main/java/io/grpc/util/AdvancedTlsX509TrustManager.java b/core/src/main/java/io/grpc/util/AdvancedTlsX509TrustManager.java index 1a1da50c9d6..1f30177900a 100644 --- a/core/src/main/java/io/grpc/util/AdvancedTlsX509TrustManager.java +++ b/core/src/main/java/io/grpc/util/AdvancedTlsX509TrustManager.java @@ -24,11 +24,8 @@ import java.security.KeyStore; import java.security.KeyStoreException; import java.security.NoSuchAlgorithmException; -import java.security.cert.Certificate; import java.security.cert.CertificateException; import java.security.cert.X509Certificate; -import java.util.ArrayList; -import java.util.List; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.ScheduledFuture; import java.util.concurrent.TimeUnit; @@ -54,24 +51,15 @@ public final class AdvancedTlsX509TrustManager extends X509ExtendedTrustManager private final Verification verification; private final PeerVerifier peerVerifier; private final SslSocketAndEnginePeerVerifier socketAndEnginePeerVerifier; - // The aliases we use in the key store to keep track of all the trust certificates. - private volatile List aliases; - // The key store that is used to hold the trust certificates. It will always keep synced with - // trustCerts. - private volatile KeyStore ks; + + // The delegated trust manager used to perform traditional certificate verification. + private volatile X509ExtendedTrustManager delegateManager = null; private AdvancedTlsX509TrustManager(Verification verification, PeerVerifier peerVerifier, SslSocketAndEnginePeerVerifier socketAndEnginePeerVerifier) throws CertificateException { - this.aliases = new ArrayList<>(); this.verification = verification; this.peerVerifier = peerVerifier; this.socketAndEnginePeerVerifier = socketAndEnginePeerVerifier; - try { - ks = KeyStore.getInstance(KeyStore.getDefaultType()); - ks.load(null, null); - } catch (KeyStoreException | IOException | NoSuchAlgorithmException | CertificateException e) { - throw new CertificateException("Failed to create KeyStore", e); - } } @Override @@ -112,28 +100,24 @@ public void checkServerTrusted(X509Certificate[] chain, String authType, Socket @Override public X509Certificate[] getAcceptedIssuers() { - List trustCerts = new ArrayList<>(); - for (String alias : this.aliases) { - try { - Certificate cert = this.ks.getCertificate(alias); - if (cert instanceof X509Certificate) { - trustCerts.add((X509Certificate) cert); - } else { - log.log(Level.SEVERE, "The certificate is not type of X509Certificate. Skip it"); - } - } catch (KeyStoreException e) { - log.log(Level.SEVERE, "Failed to get the certificate from the key store", e); - } + if (this.delegateManager == null) { + log.log(Level.SEVERE, "Credential hasn't been provided yet"); + return new X509Certificate[0]; } - return trustCerts.toArray(new X509Certificate[0]); - + return this.delegateManager.getAcceptedIssuers(); } - // If this is set, we will use the default trust certificates stored on user's local system. - // After this is used, functions that will update this.ks(e.g. updateTrustCredentials(), - // updateTrustCredentialsFromFile()) should not be called. - public void useSystemDefaultTrustCerts() { - this.ks = null; + /** + * Uses the default trust certificates stored on user's local system. + * After this is used, functions that will provide new credential + * data(e.g. updateTrustCredentials(), updateTrustCredentialsFromFile()) should not be called. + */ + public void useSystemDefaultTrustCerts() throws CertificateException, KeyStoreException, + NoSuchAlgorithmException { + // Passing a null value of KeyStore would make {@code TrustManagerFactory} attempt to use + // system-default trust CA certs. + X509ExtendedTrustManager newDelegateManager = createDelegateTrustManager(null); + this.delegateManager = newDelegateManager; } /** @@ -141,78 +125,63 @@ public void useSystemDefaultTrustCerts() { * * @param trustCerts the trust certificates that are going to be used */ - public void updateTrustCredentials(X509Certificate[] trustCerts) throws CertificateException { - // Clear the key store by re-creating it. - KeyStore newKeyStore = null; - try { - newKeyStore = KeyStore.getInstance(KeyStore.getDefaultType()); - newKeyStore.load(null, null); - } catch (KeyStoreException | IOException | NoSuchAlgorithmException | CertificateException e) { - throw new CertificateException("Failed to create KeyStore", e); - } - if (newKeyStore != null) { - this.ks = newKeyStore; - } - this.aliases.clear(); - // Update the ks with the new credential contents. + public void updateTrustCredentials(X509Certificate[] trustCerts) throws CertificateException, + KeyStoreException, NoSuchAlgorithmException, IOException { + KeyStore keyStore = KeyStore.getInstance(KeyStore.getDefaultType()); + keyStore.load(null, null); int i = 1; for (X509Certificate cert: trustCerts) { String alias = Integer.toString(i); - this.aliases.add(alias); - try { - ks.setCertificateEntry(alias, cert); - } catch (KeyStoreException e) { - throw new CertificateException("Failed to load trust certificate into KeyStore", e); - } + keyStore.setCertificateEntry(alias, cert); i++; } + X509ExtendedTrustManager newDelegateManager = createDelegateTrustManager(keyStore); + this.delegateManager = newDelegateManager; + } + + private static X509ExtendedTrustManager createDelegateTrustManager(KeyStore keyStore) + throws CertificateException, KeyStoreException, NoSuchAlgorithmException { + TrustManagerFactory tmf = TrustManagerFactory.getInstance( + TrustManagerFactory.getDefaultAlgorithm()); + tmf.init(keyStore); + X509ExtendedTrustManager delegateManager = null; + TrustManager[] tms = tmf.getTrustManagers(); + // Iterate over the returned trust managers, looking for an instance of X509TrustManager. + // If found, use that as the delegate trust manager. + for (int j = 0; j < tms.length; j++) { + if (tms[j] instanceof X509ExtendedTrustManager) { + delegateManager = (X509ExtendedTrustManager) tms[j]; + break; + } + } + if (delegateManager == null) { + throw new CertificateException( + "Instance delegateX509TrustManager is null. Failed to initialize"); + } + return delegateManager; } private void checkTrusted(X509Certificate[] chain, String authType, SSLEngine sslEngine, Socket socket, boolean checkingServer) throws CertificateException { - if (this.verification == Verification.CertificateAndHostNameVerification - || this.verification == Verification.CertificateOnlyVerification) { + if (this.verification != Verification.InsecurelySkipAllVerification) { if (chain == null || chain.length == 0) { throw new CertificateException( "Want certificate verification but got null or empty certificates"); } - // Use the key store to create a delegated trust manager. - X509ExtendedTrustManager delegateManager = null; - TrustManagerFactory tmf; - try { - tmf = TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm()); - tmf.init(ks); - } catch (KeyStoreException | NoSuchAlgorithmException e) { - throw new CertificateException("Failed to create delegate TrustManagerFactory", e); - } - TrustManager[] tms = tmf.getTrustManagers(); - // Iterate over the returned trust managers, looking for an instance of X509TrustManager. - // If found, use that as the delegate trust manager. - for (int i = 0; i < tms.length; i++) { - if (tms[i] instanceof X509ExtendedTrustManager) { - delegateManager = (X509ExtendedTrustManager) tms[i]; - break; - } - } - if (delegateManager == null) { - throw new CertificateException( - "Instance delegateX509TrustManager is null. Failed to initialize"); + if (this.delegateManager == null) { + throw new CertificateException("Credential hasn't been provided yet"); } - // Configure the delegated trust manager based on users' input. if (checkingServer) { if (this.verification == Verification.CertificateAndHostNameVerification - || this.verification == Verification.CertificateOnlyVerification) { - if (this.verification == Verification.CertificateAndHostNameVerification - && sslEngine == null) { - throw new CertificateException( - "SSLEngine is null. Couldn't check host name"); - } - String algorithm = this.verification == Verification.CertificateAndHostNameVerification - ? "HTTPS" : ""; - SSLParameters sslParams = sslEngine.getSSLParameters(); - sslParams.setEndpointIdentificationAlgorithm(algorithm); - sslEngine.setSSLParameters(sslParams); + && sslEngine == null) { + throw new CertificateException( + "SSLEngine is null. Couldn't check host name"); } + String algorithm = this.verification == Verification.CertificateAndHostNameVerification + ? "HTTPS" : ""; + SSLParameters sslParams = sslEngine.getSSLParameters(); + sslParams.setEndpointIdentificationAlgorithm(algorithm); + sslEngine.setSSLParameters(sslParams); delegateManager.checkServerTrusted(chain, authType, sslEngine); } else { delegateManager.checkClientTrusted(chain, authType, sslEngine); @@ -268,9 +237,9 @@ public void run() { if (this.currentTime != newUpdateTime) { this.currentTime = newUpdateTime; } - } catch (CertificateException | IOException | KeyStoreException e) { + } catch (CertificateException | IOException | KeyStoreException + | NoSuchAlgorithmException e) { log.log(Level.SEVERE, "Failed refreshing trust CAs from file. Using previous CAs", e); - } } } @@ -284,19 +253,16 @@ public void run() { * @return oldTime if failed or the modified time is not changed, otherwise the new modified time */ private long readAndUpdate(File trustCertFile, long oldTime) - throws CertificateException, IOException, KeyStoreException { + throws CertificateException, IOException, KeyStoreException, NoSuchAlgorithmException { long newTime = trustCertFile.lastModified(); if (newTime != oldTime) { - FileInputStream inputStream = null; + FileInputStream inputStream = new FileInputStream(trustCertFile); try { - inputStream = new FileInputStream(trustCertFile); X509Certificate[] certificates = CertificateUtils.getX509Certificates(inputStream); updateTrustCredentials(certificates); return newTime; } finally { - if (inputStream != null) { - inputStream.close(); - } + inputStream.close(); } } return oldTime; diff --git a/netty/src/test/java/io/grpc/netty/AdvancedTlsTest.java b/netty/src/test/java/io/grpc/netty/AdvancedTlsTest.java index 7fd52abde14..3719ccc7f98 100644 --- a/netty/src/test/java/io/grpc/netty/AdvancedTlsTest.java +++ b/netty/src/test/java/io/grpc/netty/AdvancedTlsTest.java @@ -67,6 +67,8 @@ public class AdvancedTlsTest { public static final String CLIENT_0_KEY_FILE = "client.key"; public static final String CLIENT_0_PEM_FILE = "client.pem"; public static final String CA_PEM_FILE = "ca.pem"; + public static final String SERVER_BAD_KEY_FILE = "badserver.key"; + public static final String SERVER_BAD_PEM_FILE = "badserver.pem"; private ScheduledExecutorService executor; private Server server; @@ -82,6 +84,8 @@ public class AdvancedTlsTest { private X509Certificate[] serverCert0; private PrivateKey clientKey0; private X509Certificate[] clientCert0; + private PrivateKey serverKeyBad; + private X509Certificate[] serverCertBad; @Rule public ExpectedException exceptionRule = ExpectedException.none(); @@ -105,6 +109,10 @@ public void setUp() TestUtils.class.getResourceAsStream("/certs/" + CLIENT_0_KEY_FILE)); clientCert0 = CertificateUtils.getX509Certificates( TestUtils.class.getResourceAsStream("/certs/" + CLIENT_0_PEM_FILE)); + serverKeyBad = CertificateUtils.getPrivateKey( + TestUtils.class.getResourceAsStream("/certs/" + SERVER_BAD_KEY_FILE)); + serverCertBad = CertificateUtils.getX509Certificates( + TestUtils.class.getResourceAsStream("/certs/" + SERVER_BAD_PEM_FILE)); } @After @@ -147,9 +155,9 @@ public void basicMutualTlsTest() throws Exception { @Test public void advancedTlsKeyManagerTrustManagerMutualTlsTest() throws Exception { - // Create & start a server. + // Create a server with the key manager and trust manager. AdvancedTlsX509KeyManager serverKeyManager = new AdvancedTlsX509KeyManager(); - serverKeyManager.updateIdentityCredentials(serverKey0, serverCert0, ""); + serverKeyManager.updateIdentityCredentials(serverKey0, serverCert0); AdvancedTlsX509TrustManager serverTrustManager = AdvancedTlsX509TrustManager.newBuilder() .setVerification(Verification.CertificateOnlyVerification) .build(); @@ -160,9 +168,9 @@ public void advancedTlsKeyManagerTrustManagerMutualTlsTest() throws Exception { server = Grpc.newServerBuilderForPort(0, serverCredentials).addService( new SimpleServiceImpl()).build().start(); TimeUnit.SECONDS.sleep(5); - // Create a client to connect. + // Create a client with the key manager and trust manager. AdvancedTlsX509KeyManager clientKeyManager = new AdvancedTlsX509KeyManager(); - clientKeyManager.updateIdentityCredentials(clientKey0, clientCert0, ""); + clientKeyManager.updateIdentityCredentials(clientKey0, clientCert0); AdvancedTlsX509TrustManager clientTrustManager = AdvancedTlsX509TrustManager.newBuilder() .setVerification(Verification.CertificateAndHostNameVerification) .build(); @@ -175,8 +183,6 @@ public void advancedTlsKeyManagerTrustManagerMutualTlsTest() throws Exception { try { SimpleServiceGrpc.SimpleServiceBlockingStub client = SimpleServiceGrpc.newBlockingStub(channel); - // Send an actual request, via the full GRPC & network stack, and check that a proper - // response comes back. client.unaryRpc(SimpleRequest.getDefaultInstance()); } catch (StatusRuntimeException e) { fail("Failed to make a connection"); @@ -186,9 +192,9 @@ public void advancedTlsKeyManagerTrustManagerMutualTlsTest() throws Exception { @Test public void trustManagerCustomVerifierMutualTlsTest() throws Exception { - // Create & start a server. AdvancedTlsX509KeyManager serverKeyManager = new AdvancedTlsX509KeyManager(); - serverKeyManager.updateIdentityCredentials(serverKey0, serverCert0, ""); + serverKeyManager.updateIdentityCredentials(serverKey0, serverCert0); + // Set server's custom verification based on the information of clientCert0. AdvancedTlsX509TrustManager serverTrustManager = AdvancedTlsX509TrustManager.newBuilder() .setVerification(Verification.CertificateOnlyVerification) .setSslSocketAndEnginePeerVerifier( @@ -217,19 +223,6 @@ public void verifyPeerCertificate(X509Certificate[] peerCertChain, String authTy } } }) - .setPeerVerifier(new PeerVerifier() { - @Override - public void verifyPeerCertificate(X509Certificate[] peerCertChain, String authType) - throws CertificateException { - if (peerCertChain == null || peerCertChain.length == 0) { - throw new CertificateException("peerCertChain is empty"); - } - X509Certificate leafCert = peerCertChain[0]; - if (!leafCert.getSubjectDN().getName().contains("testclient")) { - throw new CertificateException("PeerVerifier failed"); - } - } - }) .build(); serverTrustManager.updateTrustCredentials(caCert); ServerCredentials serverCredentials = TlsServerCredentials.newBuilder() @@ -238,9 +231,10 @@ public void verifyPeerCertificate(X509Certificate[] peerCertChain, String authTy server = Grpc.newServerBuilderForPort(0, serverCredentials).addService( new SimpleServiceImpl()).build().start(); TimeUnit.SECONDS.sleep(5); - // Create a client to connect. + AdvancedTlsX509KeyManager clientKeyManager = new AdvancedTlsX509KeyManager(); - clientKeyManager.updateIdentityCredentials(clientKey0, clientCert0, ""); + clientKeyManager.updateIdentityCredentials(clientKey0, clientCert0); + // Set client's custom verification based on the information of serverCert0. AdvancedTlsX509TrustManager clientTrustManager = AdvancedTlsX509TrustManager.newBuilder() .setVerification(Verification.CertificateOnlyVerification) .setSslSocketAndEnginePeerVerifier( @@ -269,18 +263,56 @@ public void verifyPeerCertificate(X509Certificate[] peerCertChain, String authTy } } }) + .build(); + clientTrustManager.updateTrustCredentials(caCert); + ChannelCredentials channelCredentials = TlsChannelCredentials.newBuilder() + .keyManager(clientKeyManager).trustManager(clientTrustManager).build(); + channel = Grpc.newChannelBuilderForAddress( + "localhost", server.getPort(), channelCredentials).build(); + // Start the connection. + try { + SimpleServiceGrpc.SimpleServiceBlockingStub client = + SimpleServiceGrpc.newBlockingStub(channel); + client.unaryRpc(SimpleRequest.getDefaultInstance()); + } catch (StatusRuntimeException e) { + fail("Failed to make a connection"); + e.printStackTrace(); + } + } + + @Test + public void trustManagerInsecurelySkipAllTest() throws Exception { + AdvancedTlsX509KeyManager serverKeyManager = new AdvancedTlsX509KeyManager(); + // Even if we provide bad credentials for the server, the test should still pass, because we + // will configure the client to skip all checks later. + serverKeyManager.updateIdentityCredentials(serverKeyBad, serverCertBad); + AdvancedTlsX509TrustManager serverTrustManager = AdvancedTlsX509TrustManager.newBuilder() + .setVerification(Verification.CertificateOnlyVerification) .setPeerVerifier(new PeerVerifier() { @Override public void verifyPeerCertificate(X509Certificate[] peerCertChain, String authType) - throws CertificateException { - if (peerCertChain == null || peerCertChain.length == 0) { - throw new CertificateException("peerCertChain is empty"); - } - X509Certificate leafCert = peerCertChain[0]; - if (!leafCert.getSubjectDN().getName().contains("*.test.google.com.au")) { - throw new CertificateException("PeerVerifier failed"); - } - } + throws CertificateException { } + }) + .build(); + serverTrustManager.updateTrustCredentials(caCert); + ServerCredentials serverCredentials = TlsServerCredentials.newBuilder() + .keyManager(serverKeyManager).trustManager(serverTrustManager) + .clientAuth(ClientAuth.REQUIRE).build(); + server = Grpc.newServerBuilderForPort(0, serverCredentials).addService( + new SimpleServiceImpl()).build().start(); + TimeUnit.SECONDS.sleep(5); + + AdvancedTlsX509KeyManager clientKeyManager = new AdvancedTlsX509KeyManager(); + clientKeyManager.updateIdentityCredentials(clientKey0, clientCert0); + // Set the client to skip all checks, including traditional certificate verification. + // Note this is very dangerous in production environment - only do so if you are confident on + // what you are doing! + AdvancedTlsX509TrustManager clientTrustManager = AdvancedTlsX509TrustManager.newBuilder() + .setVerification(Verification.InsecurelySkipAllVerification) + .setPeerVerifier(new PeerVerifier() { + @Override + public void verifyPeerCertificate(X509Certificate[] peerCertChain, String authType) + throws CertificateException { } }) .build(); clientTrustManager.updateTrustCredentials(caCert); @@ -292,8 +324,6 @@ public void verifyPeerCertificate(X509Certificate[] peerCertChain, String authTy try { SimpleServiceGrpc.SimpleServiceBlockingStub client = SimpleServiceGrpc.newBlockingStub(channel); - // Send an actual request, via the full GRPC & network stack, and check that a proper - // response comes back. client.unaryRpc(SimpleRequest.getDefaultInstance()); } catch (StatusRuntimeException e) { fail("Failed to make a connection"); @@ -306,7 +336,7 @@ public void onFileReloadingKeyManagerTrustManagerTest() throws Exception { // Create & start a server. AdvancedTlsX509KeyManager serverKeyManager = new AdvancedTlsX509KeyManager(); Closeable serverKeyShutdown = serverKeyManager.updateIdentityCredentialsFromFile(serverKey0File, - serverCert0File, "", 100, TimeUnit.MILLISECONDS, executor); + serverCert0File, 100, TimeUnit.MILLISECONDS, executor); AdvancedTlsX509TrustManager serverTrustManager = AdvancedTlsX509TrustManager.newBuilder() .setVerification(Verification.CertificateOnlyVerification) .build(); @@ -321,7 +351,7 @@ public void onFileReloadingKeyManagerTrustManagerTest() throws Exception { // Create a client to connect. AdvancedTlsX509KeyManager clientKeyManager = new AdvancedTlsX509KeyManager(); Closeable clientKeyShutdown = clientKeyManager.updateIdentityCredentialsFromFile(clientKey0File, - clientCert0File, "",100, TimeUnit.MILLISECONDS, executor); + clientCert0File,100, TimeUnit.MILLISECONDS, executor); AdvancedTlsX509TrustManager clientTrustManager = AdvancedTlsX509TrustManager.newBuilder() .setVerification(Verification.CertificateAndHostNameVerification) .build(); From 19ee458db0039e4750bcbb1597c6b64278777b21 Mon Sep 17 00:00:00 2001 From: ZhenLian Date: Tue, 17 Aug 2021 09:31:30 -0700 Subject: [PATCH 10/12] fix the latest comments --- .../grpc/util/AdvancedTlsX509KeyManager.java | 14 +- .../util/AdvancedTlsX509TrustManager.java | 123 ++++++++---------- .../java/io/grpc/netty/AdvancedTlsTest.java | 100 +++++++------- 3 files changed, 108 insertions(+), 129 deletions(-) diff --git a/core/src/main/java/io/grpc/util/AdvancedTlsX509KeyManager.java b/core/src/main/java/io/grpc/util/AdvancedTlsX509KeyManager.java index ec6a5a9beae..e2addcfdca9 100644 --- a/core/src/main/java/io/grpc/util/AdvancedTlsX509KeyManager.java +++ b/core/src/main/java/io/grpc/util/AdvancedTlsX509KeyManager.java @@ -16,6 +16,8 @@ package io.grpc.util; +import static com.google.common.base.Preconditions.checkNotNull; + import io.grpc.ExperimentalApi; import java.io.File; import java.io.FileInputStream; @@ -54,12 +56,18 @@ public AdvancedTlsX509KeyManager() throws CertificateException { } @Override public PrivateKey getPrivateKey(String alias) { - return this.keyInfo.key; + if (alias.equals("default")) { + return this.keyInfo.key; + } + return null; } @Override public X509Certificate[] getCertificateChain(String alias) { - return Arrays.copyOf(this.keyInfo.certs, this.keyInfo.certs.length); + if (alias.equals("default")) { + return Arrays.copyOf(this.keyInfo.certs, this.keyInfo.certs.length); + } + return null; } @Override @@ -102,7 +110,7 @@ public String chooseEngineServerAlias(String keyType, Principal[] issuers, public void updateIdentityCredentials(PrivateKey key, X509Certificate[] certs) throws CertificateException { // TODO(ZhenLian): explore possibilities to do a crypto check here. - this.keyInfo = new KeyInfo(key, certs); + this.keyInfo = new KeyInfo(checkNotNull(key), checkNotNull(certs)); } /** diff --git a/core/src/main/java/io/grpc/util/AdvancedTlsX509TrustManager.java b/core/src/main/java/io/grpc/util/AdvancedTlsX509TrustManager.java index 1f30177900a..ca248749b73 100644 --- a/core/src/main/java/io/grpc/util/AdvancedTlsX509TrustManager.java +++ b/core/src/main/java/io/grpc/util/AdvancedTlsX509TrustManager.java @@ -33,6 +33,7 @@ import java.util.logging.Logger; import javax.net.ssl.SSLEngine; import javax.net.ssl.SSLParameters; +import javax.net.ssl.SSLSocket; import javax.net.ssl.TrustManager; import javax.net.ssl.TrustManagerFactory; import javax.net.ssl.X509ExtendedTrustManager; @@ -49,23 +50,22 @@ public final class AdvancedTlsX509TrustManager extends X509ExtendedTrustManager private static final Logger log = Logger.getLogger(AdvancedTlsX509TrustManager.class.getName()); private final Verification verification; - private final PeerVerifier peerVerifier; private final SslSocketAndEnginePeerVerifier socketAndEnginePeerVerifier; // The delegated trust manager used to perform traditional certificate verification. private volatile X509ExtendedTrustManager delegateManager = null; - private AdvancedTlsX509TrustManager(Verification verification, PeerVerifier peerVerifier, + private AdvancedTlsX509TrustManager(Verification verification, SslSocketAndEnginePeerVerifier socketAndEnginePeerVerifier) throws CertificateException { this.verification = verification; - this.peerVerifier = peerVerifier; this.socketAndEnginePeerVerifier = socketAndEnginePeerVerifier; } @Override public void checkClientTrusted(X509Certificate[] chain, String authType) throws CertificateException { - checkTrusted(chain, authType, null, null, false); + throw new CertificateException( + "Either SSLEngine or Socket should be available. Consider upgrading your java version"); } @Override @@ -89,7 +89,8 @@ public void checkServerTrusted(X509Certificate[] chain, String authType, SSLEng @Override public void checkServerTrusted(X509Certificate[] chain, String authType) throws CertificateException { - checkTrusted(chain, authType, null, null, true); + throw new CertificateException( + "Either SSLEngine or Socket should be available. Consider upgrading your java version"); } @Override @@ -101,7 +102,6 @@ public void checkServerTrusted(X509Certificate[] chain, String authType, Socket @Override public X509Certificate[] getAcceptedIssuers() { if (this.delegateManager == null) { - log.log(Level.SEVERE, "Credential hasn't been provided yet"); return new X509Certificate[0]; } return this.delegateManager.getAcceptedIssuers(); @@ -116,8 +116,7 @@ public void useSystemDefaultTrustCerts() throws CertificateException, KeyStoreEx NoSuchAlgorithmException { // Passing a null value of KeyStore would make {@code TrustManagerFactory} attempt to use // system-default trust CA certs. - X509ExtendedTrustManager newDelegateManager = createDelegateTrustManager(null); - this.delegateManager = newDelegateManager; + this.delegateManager = createDelegateTrustManager(null); } /** @@ -156,46 +155,55 @@ private static X509ExtendedTrustManager createDelegateTrustManager(KeyStore keyS } if (delegateManager == null) { throw new CertificateException( - "Instance delegateX509TrustManager is null. Failed to initialize"); + "Failed to find X509ExtendedTrustManager with default TrustManager algorithm " + + TrustManagerFactory.getDefaultAlgorithm()); } return delegateManager; } private void checkTrusted(X509Certificate[] chain, String authType, SSLEngine sslEngine, Socket socket, boolean checkingServer) throws CertificateException { + if (chain == null || chain.length == 0) { + throw new CertificateException( + "Want certificate verification but got null or empty certificates"); + } + if (sslEngine == null && socket == null) { + throw new CertificateException("Either SSLEngine or socket should be available"); + } if (this.verification != Verification.InsecurelySkipAllVerification) { - if (chain == null || chain.length == 0) { - throw new CertificateException( - "Want certificate verification but got null or empty certificates"); - } - if (this.delegateManager == null) { - throw new CertificateException("Credential hasn't been provided yet"); + X509ExtendedTrustManager currentDelegateManager = this.delegateManager; + if (currentDelegateManager == null) { + throw new CertificateException("No trust roots configured"); } if (checkingServer) { - if (this.verification == Verification.CertificateAndHostNameVerification - && sslEngine == null) { - throw new CertificateException( - "SSLEngine is null. Couldn't check host name"); - } String algorithm = this.verification == Verification.CertificateAndHostNameVerification ? "HTTPS" : ""; - SSLParameters sslParams = sslEngine.getSSLParameters(); - sslParams.setEndpointIdentificationAlgorithm(algorithm); - sslEngine.setSSLParameters(sslParams); - delegateManager.checkServerTrusted(chain, authType, sslEngine); + if (sslEngine != null) { + SSLParameters sslParams = sslEngine.getSSLParameters(); + sslParams.setEndpointIdentificationAlgorithm(algorithm); + sslEngine.setSSLParameters(sslParams); + currentDelegateManager.checkServerTrusted(chain, authType, sslEngine); + } else { + if (!(socket instanceof SSLSocket)) { + throw new CertificateException("socket is not a type of SSLSocket"); + } + SSLSocket sslSocket = (SSLSocket)socket; + SSLParameters sslParams = sslSocket.getSSLParameters(); + sslParams.setEndpointIdentificationAlgorithm(algorithm); + sslSocket.setSSLParameters(sslParams); + currentDelegateManager.checkServerTrusted(chain, authType, sslSocket); + } } else { - delegateManager.checkClientTrusted(chain, authType, sslEngine); + currentDelegateManager.checkClientTrusted(chain, authType, sslEngine); } } // Perform the additional peer cert check. - if (sslEngine != null && socketAndEnginePeerVerifier != null) { - socketAndEnginePeerVerifier.verifyPeerCertificate(chain, authType, sslEngine); - } else if (socket != null && socketAndEnginePeerVerifier != null) { - socketAndEnginePeerVerifier.verifyPeerCertificate(chain, authType, socket); - } else if (peerVerifier != null) { - peerVerifier.verifyPeerCertificate(chain, authType); - } else { - log.log(Level.INFO, "No peer verifier is specified"); + if (socketAndEnginePeerVerifier != null) { + if (sslEngine != null) { + socketAndEnginePeerVerifier.verifyPeerCertificate(chain, authType, sslEngine); + } else { + socketAndEnginePeerVerifier.verifyPeerCertificate(chain, authType, socket); + } } } @@ -233,10 +241,7 @@ public LoadFilePathExecution(File file) { @Override public void run() { try { - long newUpdateTime = readAndUpdate(this.file, this.currentTime); - if (this.currentTime != newUpdateTime) { - this.currentTime = newUpdateTime; - } + this.currentTime = readAndUpdate(this.file, this.currentTime); } catch (CertificateException | IOException | KeyStoreException | NoSuchAlgorithmException e) { log.log(Level.SEVERE, "Failed refreshing trust CAs from file. Using previous CAs", e); @@ -255,17 +260,17 @@ public void run() { private long readAndUpdate(File trustCertFile, long oldTime) throws CertificateException, IOException, KeyStoreException, NoSuchAlgorithmException { long newTime = trustCertFile.lastModified(); - if (newTime != oldTime) { - FileInputStream inputStream = new FileInputStream(trustCertFile); - try { - X509Certificate[] certificates = CertificateUtils.getX509Certificates(inputStream); - updateTrustCredentials(certificates); - return newTime; - } finally { - inputStream.close(); - } + if (newTime == oldTime) { + return oldTime; + } + FileInputStream inputStream = new FileInputStream(trustCertFile); + try { + X509Certificate[] certificates = CertificateUtils.getX509Certificates(inputStream); + updateTrustCredentials(certificates); + return newTime; + } finally { + inputStream.close(); } - return oldTime; } // Mainly used to avoid throwing IO Exceptions in java.io.Closeable. @@ -301,21 +306,6 @@ public enum Verification { InsecurelySkipAllVerification, } - // Additional custom peer verification check. - // It will be used when checkClientTrusted/checkServerTrusted is called without - // {@code Socket}/{@code SSLEngine} parameter. - public interface PeerVerifier { - /** - * Verifies the peer certificate chain. For more information, please refer to - * {@code X509ExtendedTrustManager}. - * - * @param peerCertChain the certificate chain sent from the peer - * @param authType the key exchange algorithm used, e.g. "RSA", "DHE_DSS", etc - */ - void verifyPeerCertificate(X509Certificate[] peerCertChain, String authType) - throws CertificateException; - } - // Additional custom peer verification check. // It will be used when checkClientTrusted/checkServerTrusted is called with the {@code Socket} or // the {@code SSLEngine} parameter. @@ -348,7 +338,6 @@ void verifyPeerCertificate(X509Certificate[] peerCertChain, String authType, SSL public static final class Builder { private Verification verification = Verification.CertificateAndHostNameVerification; - private PeerVerifier peerVerifier; private SslSocketAndEnginePeerVerifier socketAndEnginePeerVerifier; private Builder() {} @@ -358,19 +347,13 @@ public Builder setVerification(Verification verification) { return this; } - public Builder setPeerVerifier(PeerVerifier verifier) { - this.peerVerifier = verifier; - return this; - } - public Builder setSslSocketAndEnginePeerVerifier(SslSocketAndEnginePeerVerifier verifier) { this.socketAndEnginePeerVerifier = verifier; return this; } public AdvancedTlsX509TrustManager build() throws CertificateException { - return new AdvancedTlsX509TrustManager(this.verification, this.peerVerifier, - this.socketAndEnginePeerVerifier); + return new AdvancedTlsX509TrustManager(this.verification, this.socketAndEnginePeerVerifier); } } } diff --git a/netty/src/test/java/io/grpc/netty/AdvancedTlsTest.java b/netty/src/test/java/io/grpc/netty/AdvancedTlsTest.java index 3719ccc7f98..05869635c4e 100644 --- a/netty/src/test/java/io/grpc/netty/AdvancedTlsTest.java +++ b/netty/src/test/java/io/grpc/netty/AdvancedTlsTest.java @@ -37,7 +37,6 @@ import io.grpc.testing.protobuf.SimpleServiceGrpc; import io.grpc.util.AdvancedTlsX509KeyManager; import io.grpc.util.AdvancedTlsX509TrustManager; -import io.grpc.util.AdvancedTlsX509TrustManager.PeerVerifier; import io.grpc.util.AdvancedTlsX509TrustManager.SslSocketAndEnginePeerVerifier; import io.grpc.util.AdvancedTlsX509TrustManager.Verification; import io.grpc.util.CertificateUtils; @@ -288,11 +287,16 @@ public void trustManagerInsecurelySkipAllTest() throws Exception { serverKeyManager.updateIdentityCredentials(serverKeyBad, serverCertBad); AdvancedTlsX509TrustManager serverTrustManager = AdvancedTlsX509TrustManager.newBuilder() .setVerification(Verification.CertificateOnlyVerification) - .setPeerVerifier(new PeerVerifier() { - @Override - public void verifyPeerCertificate(X509Certificate[] peerCertChain, String authType) - throws CertificateException { } - }) + .setSslSocketAndEnginePeerVerifier( + new SslSocketAndEnginePeerVerifier() { + @Override + public void verifyPeerCertificate(X509Certificate[] peerCertChain, String authType, + Socket socket) throws CertificateException { } + + @Override + public void verifyPeerCertificate(X509Certificate[] peerCertChain, String authType, + SSLEngine engine) throws CertificateException { } + }) .build(); serverTrustManager.updateTrustCredentials(caCert); ServerCredentials serverCredentials = TlsServerCredentials.newBuilder() @@ -309,11 +313,16 @@ public void verifyPeerCertificate(X509Certificate[] peerCertChain, String authTy // what you are doing! AdvancedTlsX509TrustManager clientTrustManager = AdvancedTlsX509TrustManager.newBuilder() .setVerification(Verification.InsecurelySkipAllVerification) - .setPeerVerifier(new PeerVerifier() { - @Override - public void verifyPeerCertificate(X509Certificate[] peerCertChain, String authType) - throws CertificateException { } - }) + .setSslSocketAndEnginePeerVerifier( + new SslSocketAndEnginePeerVerifier() { + @Override + public void verifyPeerCertificate(X509Certificate[] peerCertChain, String authType, + Socket socket) throws CertificateException { } + + @Override + public void verifyPeerCertificate(X509Certificate[] peerCertChain, String authType, + SSLEngine engine) throws CertificateException { } + }) .build(); clientTrustManager.updateTrustCredentials(caCert); ChannelCredentials channelCredentials = TlsChannelCredentials.newBuilder() @@ -393,31 +402,33 @@ public void keyManagerAliasesTest() throws Exception { } @Test - public void trustManagerCheckTrustTest() throws Exception { + public void trustManagerCheckTrustedWithSocketTest() throws Exception { AdvancedTlsX509TrustManager tm = AdvancedTlsX509TrustManager.newBuilder() - .setVerification(Verification.InsecurelySkipAllVerification) - .setSslSocketAndEnginePeerVerifier( - new SslSocketAndEnginePeerVerifier() { - @Override - public void verifyPeerCertificate(X509Certificate[] peerCertChain, String authType, - Socket socket) throws CertificateException { } - - @Override - public void verifyPeerCertificate(X509Certificate[] peerCertChain, String authType, - SSLEngine engine) throws CertificateException { } - }) - .setPeerVerifier(new PeerVerifier() { - @Override - public void verifyPeerCertificate(X509Certificate[] peerCertChain, String authType) - throws CertificateException { } - }) - .build(); + .setVerification(Verification.InsecurelySkipAllVerification).build(); tm.updateTrustCredentials(caCert); tm.checkClientTrusted(serverCert0, "RSA", new Socket()); - tm.checkClientTrusted(serverCert0, "RSA"); tm.useSystemDefaultTrustCerts(); tm.checkServerTrusted(clientCert0, "RSA", new Socket()); - tm.checkServerTrusted(clientCert0, "RSA"); + } + + @Test + public void trustManagerCheckClientTrustedWithoutParameterTest() throws Exception { + exceptionRule.expect(CertificateException.class); + exceptionRule.expectMessage("Either SSLEngine or Socket should be available. " + + "Consider upgrading your java version"); + AdvancedTlsX509TrustManager tm = AdvancedTlsX509TrustManager.newBuilder() + .setVerification(Verification.InsecurelySkipAllVerification).build(); + tm.checkClientTrusted(serverCert0, "RSA"); + } + + @Test + public void trustManagerCheckServerTrustedWithoutParameterTest() throws Exception { + exceptionRule.expect(CertificateException.class); + exceptionRule.expectMessage("Either SSLEngine or Socket should be available. " + + "Consider upgrading your java version"); + AdvancedTlsX509TrustManager tm = AdvancedTlsX509TrustManager.newBuilder() + .setVerification(Verification.InsecurelySkipAllVerification).build(); + tm.checkServerTrusted(serverCert0, "RSA"); } @Test @@ -427,24 +438,9 @@ public void trustManagerEmptyChainTest() throws Exception { "Want certificate verification but got null or empty certificates"); AdvancedTlsX509TrustManager tm = AdvancedTlsX509TrustManager.newBuilder() .setVerification(Verification.CertificateOnlyVerification) - .setSslSocketAndEnginePeerVerifier( - new SslSocketAndEnginePeerVerifier() { - @Override - public void verifyPeerCertificate(X509Certificate[] peerCertChain, String authType, - Socket socket) throws CertificateException { } - - @Override - public void verifyPeerCertificate(X509Certificate[] peerCertChain, String authType, - SSLEngine engine) throws CertificateException { } - }) - .setPeerVerifier(new PeerVerifier() { - @Override - public void verifyPeerCertificate(X509Certificate[] peerCertChain, String authType) - throws CertificateException { } - }) .build(); tm.updateTrustCredentials(caCert); - tm.checkClientTrusted(null, "RSA"); + tm.checkClientTrusted(null, "RSA", (SSLEngine) null); } @Test @@ -466,15 +462,7 @@ public void verifyPeerCertificate(X509Certificate[] peerCertChain, String authTy SSLEngine engine) throws CertificateException { throw new CertificateException("Bad Custom Verification"); } - }) - .setPeerVerifier(new PeerVerifier() { - @Override - public void verifyPeerCertificate(X509Certificate[] peerCertChain, String authType) - throws CertificateException { - throw new CertificateException("Bad Custom Verification"); - } - }) - .build(); + }).build(); tm.updateTrustCredentials(caCert); tm.checkClientTrusted(serverCert0, "RSA", new Socket()); } From 378e2ec8143fd9d96dd876acea18747e0b27fba4 Mon Sep 17 00:00:00 2001 From: ZhenLian Date: Tue, 17 Aug 2021 14:21:59 -0700 Subject: [PATCH 11/12] fix latest comment --- .../java/io/grpc/util/AdvancedTlsX509KeyManager.java | 2 +- .../io/grpc/util/AdvancedTlsX509TrustManager.java | 11 ++++++----- .../src/test/java/io/grpc/netty/AdvancedTlsTest.java | 10 +++++----- 3 files changed, 12 insertions(+), 11 deletions(-) diff --git a/core/src/main/java/io/grpc/util/AdvancedTlsX509KeyManager.java b/core/src/main/java/io/grpc/util/AdvancedTlsX509KeyManager.java index e2addcfdca9..adaa1e6e69a 100644 --- a/core/src/main/java/io/grpc/util/AdvancedTlsX509KeyManager.java +++ b/core/src/main/java/io/grpc/util/AdvancedTlsX509KeyManager.java @@ -110,7 +110,7 @@ public String chooseEngineServerAlias(String keyType, Principal[] issuers, public void updateIdentityCredentials(PrivateKey key, X509Certificate[] certs) throws CertificateException { // TODO(ZhenLian): explore possibilities to do a crypto check here. - this.keyInfo = new KeyInfo(checkNotNull(key), checkNotNull(certs)); + this.keyInfo = new KeyInfo(checkNotNull(key, "key"), checkNotNull(certs, "certs")); } /** diff --git a/core/src/main/java/io/grpc/util/AdvancedTlsX509TrustManager.java b/core/src/main/java/io/grpc/util/AdvancedTlsX509TrustManager.java index ca248749b73..dd83253aa0c 100644 --- a/core/src/main/java/io/grpc/util/AdvancedTlsX509TrustManager.java +++ b/core/src/main/java/io/grpc/util/AdvancedTlsX509TrustManager.java @@ -65,7 +65,7 @@ private AdvancedTlsX509TrustManager(Verification verification, public void checkClientTrusted(X509Certificate[] chain, String authType) throws CertificateException { throw new CertificateException( - "Either SSLEngine or Socket should be available. Consider upgrading your java version"); + "Not enough information to validate peer. SSLEngine or Socket required."); } @Override @@ -90,7 +90,7 @@ public void checkServerTrusted(X509Certificate[] chain, String authType, SSLEng public void checkServerTrusted(X509Certificate[] chain, String authType) throws CertificateException { throw new CertificateException( - "Either SSLEngine or Socket should be available. Consider upgrading your java version"); + "Not enough information to validate peer. SSLEngine or Socket required."); } @Override @@ -162,13 +162,14 @@ private static X509ExtendedTrustManager createDelegateTrustManager(KeyStore keyS } private void checkTrusted(X509Certificate[] chain, String authType, SSLEngine sslEngine, - Socket socket, boolean checkingServer) throws CertificateException { + Socket socket, boolean checkingServer) throws CertificateException, IllegalArgumentException { if (chain == null || chain.length == 0) { - throw new CertificateException( + throw new IllegalArgumentException( "Want certificate verification but got null or empty certificates"); } if (sslEngine == null && socket == null) { - throw new CertificateException("Either SSLEngine or socket should be available"); + throw new CertificateException( + "Not enough information to validate peer. SSLEngine or Socket required."); } if (this.verification != Verification.InsecurelySkipAllVerification) { X509ExtendedTrustManager currentDelegateManager = this.delegateManager; diff --git a/netty/src/test/java/io/grpc/netty/AdvancedTlsTest.java b/netty/src/test/java/io/grpc/netty/AdvancedTlsTest.java index 05869635c4e..294fbcd4a9a 100644 --- a/netty/src/test/java/io/grpc/netty/AdvancedTlsTest.java +++ b/netty/src/test/java/io/grpc/netty/AdvancedTlsTest.java @@ -414,8 +414,8 @@ public void trustManagerCheckTrustedWithSocketTest() throws Exception { @Test public void trustManagerCheckClientTrustedWithoutParameterTest() throws Exception { exceptionRule.expect(CertificateException.class); - exceptionRule.expectMessage("Either SSLEngine or Socket should be available. " - + "Consider upgrading your java version"); + exceptionRule.expectMessage( + "Not enough information to validate peer. SSLEngine or Socket required."); AdvancedTlsX509TrustManager tm = AdvancedTlsX509TrustManager.newBuilder() .setVerification(Verification.InsecurelySkipAllVerification).build(); tm.checkClientTrusted(serverCert0, "RSA"); @@ -424,8 +424,8 @@ public void trustManagerCheckClientTrustedWithoutParameterTest() throws Exceptio @Test public void trustManagerCheckServerTrustedWithoutParameterTest() throws Exception { exceptionRule.expect(CertificateException.class); - exceptionRule.expectMessage("Either SSLEngine or Socket should be available. " - + "Consider upgrading your java version"); + exceptionRule.expectMessage( + "Not enough information to validate peer. SSLEngine or Socket required."); AdvancedTlsX509TrustManager tm = AdvancedTlsX509TrustManager.newBuilder() .setVerification(Verification.InsecurelySkipAllVerification).build(); tm.checkServerTrusted(serverCert0, "RSA"); @@ -433,7 +433,7 @@ public void trustManagerCheckServerTrustedWithoutParameterTest() throws Exceptio @Test public void trustManagerEmptyChainTest() throws Exception { - exceptionRule.expect(CertificateException.class); + exceptionRule.expect(IllegalArgumentException.class); exceptionRule.expectMessage( "Want certificate verification but got null or empty certificates"); AdvancedTlsX509TrustManager tm = AdvancedTlsX509TrustManager.newBuilder() From 810eaae11e2c56ae70987fb1864a6ed02f68773d Mon Sep 17 00:00:00 2001 From: ZhenLian Date: Tue, 17 Aug 2021 14:52:09 -0700 Subject: [PATCH 12/12] small fix --- .../src/main/java/io/grpc/util/AdvancedTlsX509TrustManager.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/java/io/grpc/util/AdvancedTlsX509TrustManager.java b/core/src/main/java/io/grpc/util/AdvancedTlsX509TrustManager.java index dd83253aa0c..ea8e74b1a9e 100644 --- a/core/src/main/java/io/grpc/util/AdvancedTlsX509TrustManager.java +++ b/core/src/main/java/io/grpc/util/AdvancedTlsX509TrustManager.java @@ -162,7 +162,7 @@ private static X509ExtendedTrustManager createDelegateTrustManager(KeyStore keyS } private void checkTrusted(X509Certificate[] chain, String authType, SSLEngine sslEngine, - Socket socket, boolean checkingServer) throws CertificateException, IllegalArgumentException { + Socket socket, boolean checkingServer) throws CertificateException { if (chain == null || chain.length == 0) { throw new IllegalArgumentException( "Want certificate verification but got null or empty certificates");