From 72c3725c29ecae7592e75ba75a3fcd0a0ad31cbb Mon Sep 17 00:00:00 2001 From: ZhenLian Date: Thu, 2 Sep 2021 15:24:21 -0700 Subject: [PATCH 1/4] fix a flaky test in advanced tls --- netty/src/test/java/io/grpc/netty/AdvancedTlsTest.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/netty/src/test/java/io/grpc/netty/AdvancedTlsTest.java b/netty/src/test/java/io/grpc/netty/AdvancedTlsTest.java index 9e0d8170c40..68e9f9b841c 100644 --- a/netty/src/test/java/io/grpc/netty/AdvancedTlsTest.java +++ b/netty/src/test/java/io/grpc/netty/AdvancedTlsTest.java @@ -181,6 +181,8 @@ public void advancedTlsKeyManagerTrustManagerMutualTlsTest() throws Exception { .keyManager(clientKeyManager).trustManager(clientTrustManager).build(); channel = Grpc.newChannelBuilderForAddress("localhost", server.getPort(), channelCredentials) .overrideAuthority("foo.test.google.com.au").build(); + // Wait 5 seconds for the client and server to load their credentials. + TimeUnit.SECONDS.sleep(5); // Start the connection. try { SimpleServiceGrpc.SimpleServiceBlockingStub client = From 49fd3f37a7783d33b02a8a3f55b2477fbd6f9857 Mon Sep 17 00:00:00 2001 From: ZhenLian Date: Fri, 3 Sep 2021 14:42:51 -0700 Subject: [PATCH 2/4] add the code to do a blocking call --- netty/src/test/java/io/grpc/netty/AdvancedTlsTest.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/netty/src/test/java/io/grpc/netty/AdvancedTlsTest.java b/netty/src/test/java/io/grpc/netty/AdvancedTlsTest.java index 68e9f9b841c..9e0d8170c40 100644 --- a/netty/src/test/java/io/grpc/netty/AdvancedTlsTest.java +++ b/netty/src/test/java/io/grpc/netty/AdvancedTlsTest.java @@ -181,8 +181,6 @@ public void advancedTlsKeyManagerTrustManagerMutualTlsTest() throws Exception { .keyManager(clientKeyManager).trustManager(clientTrustManager).build(); channel = Grpc.newChannelBuilderForAddress("localhost", server.getPort(), channelCredentials) .overrideAuthority("foo.test.google.com.au").build(); - // Wait 5 seconds for the client and server to load their credentials. - TimeUnit.SECONDS.sleep(5); // Start the connection. try { SimpleServiceGrpc.SimpleServiceBlockingStub client = From 5d55c270ca06a54703b6a5aac632df811aaa99c4 Mon Sep 17 00:00:00 2001 From: ZhenLian Date: Mon, 13 Sep 2021 17:02:22 -0700 Subject: [PATCH 3/4] AdvancedTls: add functions to load credentials from static files --- .../grpc/util/AdvancedTlsX509KeyManager.java | 16 ++++++++ .../util/AdvancedTlsX509TrustManager.java | 15 ++++++++ .../java/io/grpc/netty/AdvancedTlsTest.java | 38 +++++++++++++++++++ 3 files changed, 69 insertions(+) diff --git a/core/src/main/java/io/grpc/util/AdvancedTlsX509KeyManager.java b/core/src/main/java/io/grpc/util/AdvancedTlsX509KeyManager.java index 8541c6b5280..bad01e5819b 100644 --- a/core/src/main/java/io/grpc/util/AdvancedTlsX509KeyManager.java +++ b/core/src/main/java/io/grpc/util/AdvancedTlsX509KeyManager.java @@ -141,6 +141,22 @@ public Closeable updateIdentityCredentialsFromFile(File keyFile, File certFile, }; } + /** + * Loads the private key and certificate chains from the local file paths. The contents are only + * read at the construction time and won't be updated afterwards. + * + * @param keyFile the file on disk holding the private key + * @param certFile the file on disk holding the certificate chain + */ + public void loadIdentityCredentialsFromFile(File keyFile, File certFile) throws IOException, + GeneralSecurityException { + UpdateResult newResult = readAndUpdate(keyFile, certFile, 0, 0); + if (!newResult.success) { + throw new GeneralSecurityException( + "Files were unmodified before their initial update. Probably a bug."); + } + } + 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; diff --git a/core/src/main/java/io/grpc/util/AdvancedTlsX509TrustManager.java b/core/src/main/java/io/grpc/util/AdvancedTlsX509TrustManager.java index ad69fe4abfa..146ec94deec 100644 --- a/core/src/main/java/io/grpc/util/AdvancedTlsX509TrustManager.java +++ b/core/src/main/java/io/grpc/util/AdvancedTlsX509TrustManager.java @@ -255,6 +255,21 @@ public void run() { } } + /** + * Loads the trust certificates from a local file path. The contents are only read at the + * construction time and won't be updated afterwards. + * + * @param trustCertFile the file on disk holding the trust certificates + */ + public void loadTrustCredentialsFromFile(File trustCertFile) throws IOException, + GeneralSecurityException { + long updatedTime = readAndUpdate(trustCertFile, 0); + if (updatedTime == 0) { + throw new GeneralSecurityException( + "Files were unmodified before their initial update. Probably a bug."); + } + } + /** * Reads the trust certificates specified in the path location, and update the key store if the * modified time has changed since last read. diff --git a/netty/src/test/java/io/grpc/netty/AdvancedTlsTest.java b/netty/src/test/java/io/grpc/netty/AdvancedTlsTest.java index 9e0d8170c40..ceb5585c78c 100644 --- a/netty/src/test/java/io/grpc/netty/AdvancedTlsTest.java +++ b/netty/src/test/java/io/grpc/netty/AdvancedTlsTest.java @@ -388,6 +388,44 @@ public void onFileReloadingKeyManagerTrustManagerTest() throws Exception { clientTrustShutdown.close(); } + @Test + public void onFileLoadingKeyManagerTrustManagerTest() throws Exception { + // Create & start a server. + AdvancedTlsX509KeyManager serverKeyManager = new AdvancedTlsX509KeyManager(); + serverKeyManager.loadIdentityCredentialsFromFile(serverKey0File, serverCert0File); + AdvancedTlsX509TrustManager serverTrustManager = AdvancedTlsX509TrustManager.newBuilder() + .setVerification(Verification.CERTIFICATE_ONLY_VERIFICATION) + .build(); + serverTrustManager.loadTrustCredentialsFromFile(caCertFile); + 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.loadIdentityCredentialsFromFile(clientKey0File, clientCert0File); + AdvancedTlsX509TrustManager clientTrustManager = AdvancedTlsX509TrustManager.newBuilder() + .setVerification(Verification.CERTIFICATE_AND_HOST_NAME_VERIFICATION) + .build(); + clientTrustManager.loadTrustCredentialsFromFile(caCertFile); + 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); + // 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) { + e.printStackTrace(); + fail("Find error: " + e.getMessage()); + } + } + @Test public void onFileReloadingKeyManagerBadInitialContentTest() throws Exception { exceptionRule.expect(GeneralSecurityException.class); From 92da91688d1e77e998693b858ab6cdb83a0cee9f Mon Sep 17 00:00:00 2001 From: ZhenLian Date: Thu, 16 Sep 2021 14:55:32 -0700 Subject: [PATCH 4/4] change the function name; update comments --- .../main/java/io/grpc/util/AdvancedTlsX509KeyManager.java | 5 ++--- .../java/io/grpc/util/AdvancedTlsX509TrustManager.java | 5 ++--- netty/src/test/java/io/grpc/netty/AdvancedTlsTest.java | 8 ++++---- 3 files changed, 8 insertions(+), 10 deletions(-) diff --git a/core/src/main/java/io/grpc/util/AdvancedTlsX509KeyManager.java b/core/src/main/java/io/grpc/util/AdvancedTlsX509KeyManager.java index bad01e5819b..9c9102b12cb 100644 --- a/core/src/main/java/io/grpc/util/AdvancedTlsX509KeyManager.java +++ b/core/src/main/java/io/grpc/util/AdvancedTlsX509KeyManager.java @@ -142,13 +142,12 @@ public Closeable updateIdentityCredentialsFromFile(File keyFile, File certFile, } /** - * Loads the private key and certificate chains from the local file paths. The contents are only - * read at the construction time and won't be updated afterwards. + * Updates the private key and certificate chains from the local file paths. * * @param keyFile the file on disk holding the private key * @param certFile the file on disk holding the certificate chain */ - public void loadIdentityCredentialsFromFile(File keyFile, File certFile) throws IOException, + public void updateIdentityCredentialsFromFile(File keyFile, File certFile) throws IOException, GeneralSecurityException { UpdateResult newResult = readAndUpdate(keyFile, certFile, 0, 0); if (!newResult.success) { diff --git a/core/src/main/java/io/grpc/util/AdvancedTlsX509TrustManager.java b/core/src/main/java/io/grpc/util/AdvancedTlsX509TrustManager.java index 146ec94deec..51bf57aeb34 100644 --- a/core/src/main/java/io/grpc/util/AdvancedTlsX509TrustManager.java +++ b/core/src/main/java/io/grpc/util/AdvancedTlsX509TrustManager.java @@ -256,12 +256,11 @@ public void run() { } /** - * Loads the trust certificates from a local file path. The contents are only read at the - * construction time and won't be updated afterwards. + * Updates the trust certificates from a local file path. * * @param trustCertFile the file on disk holding the trust certificates */ - public void loadTrustCredentialsFromFile(File trustCertFile) throws IOException, + public void updateTrustCredentialsFromFile(File trustCertFile) throws IOException, GeneralSecurityException { long updatedTime = readAndUpdate(trustCertFile, 0); if (updatedTime == 0) { diff --git a/netty/src/test/java/io/grpc/netty/AdvancedTlsTest.java b/netty/src/test/java/io/grpc/netty/AdvancedTlsTest.java index ceb5585c78c..6b5a96b45ab 100644 --- a/netty/src/test/java/io/grpc/netty/AdvancedTlsTest.java +++ b/netty/src/test/java/io/grpc/netty/AdvancedTlsTest.java @@ -392,11 +392,11 @@ public void onFileReloadingKeyManagerTrustManagerTest() throws Exception { public void onFileLoadingKeyManagerTrustManagerTest() throws Exception { // Create & start a server. AdvancedTlsX509KeyManager serverKeyManager = new AdvancedTlsX509KeyManager(); - serverKeyManager.loadIdentityCredentialsFromFile(serverKey0File, serverCert0File); + serverKeyManager.updateIdentityCredentialsFromFile(serverKey0File, serverCert0File); AdvancedTlsX509TrustManager serverTrustManager = AdvancedTlsX509TrustManager.newBuilder() .setVerification(Verification.CERTIFICATE_ONLY_VERIFICATION) .build(); - serverTrustManager.loadTrustCredentialsFromFile(caCertFile); + serverTrustManager.updateTrustCredentialsFromFile(caCertFile); ServerCredentials serverCredentials = TlsServerCredentials.newBuilder() .keyManager(serverKeyManager).trustManager(serverTrustManager) .clientAuth(ClientAuth.REQUIRE).build(); @@ -404,11 +404,11 @@ public void onFileLoadingKeyManagerTrustManagerTest() throws Exception { new SimpleServiceImpl()).build().start(); // Create a client to connect. AdvancedTlsX509KeyManager clientKeyManager = new AdvancedTlsX509KeyManager(); - clientKeyManager.loadIdentityCredentialsFromFile(clientKey0File, clientCert0File); + clientKeyManager.updateIdentityCredentialsFromFile(clientKey0File, clientCert0File); AdvancedTlsX509TrustManager clientTrustManager = AdvancedTlsX509TrustManager.newBuilder() .setVerification(Verification.CERTIFICATE_AND_HOST_NAME_VERIFICATION) .build(); - clientTrustManager.loadTrustCredentialsFromFile(caCertFile); + clientTrustManager.updateTrustCredentialsFromFile(caCertFile); ChannelCredentials channelCredentials = TlsChannelCredentials.newBuilder() .keyManager(clientKeyManager).trustManager(clientTrustManager).build(); channel = Grpc.newChannelBuilderForAddress("localhost", server.getPort(), channelCredentials)