From b38a73324b85e36782c2caef8457f97ddc449b0f Mon Sep 17 00:00:00 2001 From: Albumen Kevin Date: Sun, 5 Jan 2025 18:32:25 +0800 Subject: [PATCH 1/7] Add UnitTest to verify updateTrustCredentials rotate --- .../util/AdvancedTlsX509TrustManagerTest.java | 49 ++++++++++++++++++- 1 file changed, 48 insertions(+), 1 deletion(-) diff --git a/util/src/test/java/io/grpc/util/AdvancedTlsX509TrustManagerTest.java b/util/src/test/java/io/grpc/util/AdvancedTlsX509TrustManagerTest.java index 91159d121ad..33b490a2236 100644 --- a/util/src/test/java/io/grpc/util/AdvancedTlsX509TrustManagerTest.java +++ b/util/src/test/java/io/grpc/util/AdvancedTlsX509TrustManagerTest.java @@ -28,9 +28,14 @@ import io.grpc.internal.testing.TestUtils; import io.grpc.testing.TlsTesting; import io.grpc.util.AdvancedTlsX509TrustManager.Verification; +import java.io.BufferedInputStream; +import java.io.BufferedOutputStream; import java.io.File; import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; import java.net.Socket; +import java.nio.file.Files; import java.security.GeneralSecurityException; import java.security.cert.CertificateException; import java.security.cert.X509Certificate; @@ -57,21 +62,28 @@ public class AdvancedTlsX509TrustManagerTest { private static final String CA_PEM_FILE = "ca.pem"; private static final String SERVER_0_PEM_FILE = "server0.pem"; + private static final String SERVER_1_PEM_FILE = "server1.pem"; private File caCertFile; private File serverCert0File; + private File serverCert1File; private X509Certificate[] caCert; private X509Certificate[] serverCert0; + private X509Certificate[] serverCert1; + private FakeClock fakeClock; private ScheduledExecutorService executor; @Before public void setUp() throws IOException, GeneralSecurityException { - executor = new FakeClock().getScheduledExecutorService(); + fakeClock = new FakeClock(); + executor = fakeClock.getScheduledExecutorService(); caCertFile = TestUtils.loadCert(CA_PEM_FILE); caCert = CertificateUtils.getX509Certificates(TlsTesting.loadCert(CA_PEM_FILE)); serverCert0File = TestUtils.loadCert(SERVER_0_PEM_FILE); serverCert0 = CertificateUtils.getX509Certificates(TlsTesting.loadCert(SERVER_0_PEM_FILE)); + serverCert1File = TestUtils.loadCert(SERVER_1_PEM_FILE); + serverCert1 = CertificateUtils.getX509Certificates(TlsTesting.loadCert(SERVER_1_PEM_FILE)); } @Test @@ -147,6 +159,41 @@ public void clientTrustedWithSocketTest() throws Exception { assertEquals("No handshake session", ce.getMessage()); } + @Test + public void updateTrustCredentials_rotate() throws GeneralSecurityException, IOException { + AdvancedTlsX509TrustManager trustManager = AdvancedTlsX509TrustManager.newBuilder().build(); + trustManager.updateTrustCredentials(serverCert0File); + assertArrayEquals(serverCert0, trustManager.getAcceptedIssuers()); + + trustManager.updateTrustCredentials(serverCert0File, 1, TimeUnit.MINUTES, + executor); + assertArrayEquals(serverCert0, trustManager.getAcceptedIssuers()); + + fakeClock.forwardTime(1, TimeUnit.MINUTES); + assertArrayEquals(serverCert0, trustManager.getAcceptedIssuers()); + + serverCert0File.setLastModified(serverCert0File.lastModified() - 10); + + fakeClock.forwardTime(1, TimeUnit.MINUTES); + assertArrayEquals(serverCert0, trustManager.getAcceptedIssuers()); + + replaceFile(serverCert1File, serverCert0File); + fakeClock.forwardTime(1, TimeUnit.MINUTES); + assertArrayEquals(serverCert1, trustManager.getAcceptedIssuers()); + } + + private void replaceFile(File source, File target) throws IOException { + try (InputStream in = new BufferedInputStream( + Files.newInputStream(source.toPath())); OutputStream os = new BufferedOutputStream( + Files.newOutputStream(target.toPath()))) { + int b; + while ((b = in.read()) != -1) { + os.write(b); + } + os.flush(); + } + } + private static class TestHandler extends Handler { private final List records = new ArrayList<>(); From b762db6507b013c9b1f0a4c00388fac6b763980b Mon Sep 17 00:00:00 2001 From: Albumen Kevin Date: Sun, 5 Jan 2025 18:42:21 +0800 Subject: [PATCH 2/7] Fix jdk8 --- .../java/io/grpc/util/AdvancedTlsX509TrustManagerTest.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/util/src/test/java/io/grpc/util/AdvancedTlsX509TrustManagerTest.java b/util/src/test/java/io/grpc/util/AdvancedTlsX509TrustManagerTest.java index 33b490a2236..658a8c3db1d 100644 --- a/util/src/test/java/io/grpc/util/AdvancedTlsX509TrustManagerTest.java +++ b/util/src/test/java/io/grpc/util/AdvancedTlsX509TrustManagerTest.java @@ -31,6 +31,8 @@ import java.io.BufferedInputStream; import java.io.BufferedOutputStream; import java.io.File; +import java.io.FileInputStream; +import java.io.FileOutputStream; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; @@ -184,8 +186,8 @@ public void updateTrustCredentials_rotate() throws GeneralSecurityException, IOE private void replaceFile(File source, File target) throws IOException { try (InputStream in = new BufferedInputStream( - Files.newInputStream(source.toPath())); OutputStream os = new BufferedOutputStream( - Files.newOutputStream(target.toPath()))) { + new FileInputStream(source)); OutputStream os = new BufferedOutputStream( + new FileOutputStream(target))) { int b; while ((b = in.read()) != -1) { os.write(b); From 405044dbbc510fbaa5a19b65db98fc79565b46c8 Mon Sep 17 00:00:00 2001 From: Albumen Kevin Date: Sun, 5 Jan 2025 18:52:13 +0800 Subject: [PATCH 3/7] Fix style --- .../grpc/util/AdvancedTlsX509TrustManagerTest.java | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/util/src/test/java/io/grpc/util/AdvancedTlsX509TrustManagerTest.java b/util/src/test/java/io/grpc/util/AdvancedTlsX509TrustManagerTest.java index 658a8c3db1d..8ecc91795e3 100644 --- a/util/src/test/java/io/grpc/util/AdvancedTlsX509TrustManagerTest.java +++ b/util/src/test/java/io/grpc/util/AdvancedTlsX509TrustManagerTest.java @@ -37,7 +37,6 @@ import java.io.InputStream; import java.io.OutputStream; import java.net.Socket; -import java.nio.file.Files; import java.security.GeneralSecurityException; import java.security.cert.CertificateException; import java.security.cert.X509Certificate; @@ -185,14 +184,19 @@ public void updateTrustCredentials_rotate() throws GeneralSecurityException, IOE } private void replaceFile(File source, File target) throws IOException { - try (InputStream in = new BufferedInputStream( - new FileInputStream(source)); OutputStream os = new BufferedOutputStream( - new FileOutputStream(target))) { + FileInputStream in = new FileInputStream(source); + FileOutputStream out = new FileOutputStream(target); + InputStream is = new BufferedInputStream(in); + OutputStream os = new BufferedOutputStream(out); + try { int b; - while ((b = in.read()) != -1) { + while ((b = is.read()) != -1) { os.write(b); } os.flush(); + } finally { + is.close(); + os.close(); } } From 9940ea79ab832e8f9e13fe748be1e4b5a565b42e Mon Sep 17 00:00:00 2001 From: Albumen Kevin Date: Thu, 9 Jan 2025 19:26:37 +0800 Subject: [PATCH 4/7] Replace with Guava File --- .../util/AdvancedTlsX509TrustManagerTest.java | 20 ++----------------- 1 file changed, 2 insertions(+), 18 deletions(-) diff --git a/util/src/test/java/io/grpc/util/AdvancedTlsX509TrustManagerTest.java b/util/src/test/java/io/grpc/util/AdvancedTlsX509TrustManagerTest.java index 8ecc91795e3..041b2cd7e69 100644 --- a/util/src/test/java/io/grpc/util/AdvancedTlsX509TrustManagerTest.java +++ b/util/src/test/java/io/grpc/util/AdvancedTlsX509TrustManagerTest.java @@ -24,6 +24,7 @@ import static org.mockito.Mockito.when; import com.google.common.collect.Iterables; +import com.google.common.io.Files; import io.grpc.internal.FakeClock; import io.grpc.internal.testing.TestUtils; import io.grpc.testing.TlsTesting; @@ -178,28 +179,11 @@ public void updateTrustCredentials_rotate() throws GeneralSecurityException, IOE fakeClock.forwardTime(1, TimeUnit.MINUTES); assertArrayEquals(serverCert0, trustManager.getAcceptedIssuers()); - replaceFile(serverCert1File, serverCert0File); + Files.copy(serverCert1File, serverCert0File); fakeClock.forwardTime(1, TimeUnit.MINUTES); assertArrayEquals(serverCert1, trustManager.getAcceptedIssuers()); } - private void replaceFile(File source, File target) throws IOException { - FileInputStream in = new FileInputStream(source); - FileOutputStream out = new FileOutputStream(target); - InputStream is = new BufferedInputStream(in); - OutputStream os = new BufferedOutputStream(out); - try { - int b; - while ((b = is.read()) != -1) { - os.write(b); - } - os.flush(); - } finally { - is.close(); - os.close(); - } - } - private static class TestHandler extends Handler { private final List records = new ArrayList<>(); From b098d7c61d148421795330d00f832a94fb77007a Mon Sep 17 00:00:00 2001 From: Albumen Kevin Date: Thu, 9 Jan 2025 19:33:06 +0800 Subject: [PATCH 5/7] Add lastUpdateTime to avoid read --- .../main/java/io/grpc/util/AdvancedTlsX509TrustManager.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/util/src/main/java/io/grpc/util/AdvancedTlsX509TrustManager.java b/util/src/main/java/io/grpc/util/AdvancedTlsX509TrustManager.java index 088f4caa000..b4b9b25d1de 100644 --- a/util/src/main/java/io/grpc/util/AdvancedTlsX509TrustManager.java +++ b/util/src/main/java/io/grpc/util/AdvancedTlsX509TrustManager.java @@ -265,7 +265,7 @@ public Closeable updateTrustCredentials(File trustCertFile, long period, TimeUni } final ScheduledFuture future = checkNotNull(executor, "executor").scheduleWithFixedDelay( - new LoadFilePathExecution(trustCertFile), period, period, unit); + new LoadFilePathExecution(trustCertFile, updatedTime), period, period, unit); return () -> future.cancel(false); } @@ -312,9 +312,9 @@ private class LoadFilePathExecution implements Runnable { File file; long currentTime; - public LoadFilePathExecution(File file) { + public LoadFilePathExecution(File file, long currentTime) { this.file = file; - this.currentTime = 0; + this.currentTime = currentTime; } @Override From 74d65d1ea64a0cdf941313ba95929317113adbcf Mon Sep 17 00:00:00 2001 From: Albumen Kevin Date: Thu, 9 Jan 2025 19:35:26 +0800 Subject: [PATCH 6/7] Enhance test --- .../io/grpc/util/AdvancedTlsX509TrustManagerTest.java | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/util/src/test/java/io/grpc/util/AdvancedTlsX509TrustManagerTest.java b/util/src/test/java/io/grpc/util/AdvancedTlsX509TrustManagerTest.java index 041b2cd7e69..64918ce5b8a 100644 --- a/util/src/test/java/io/grpc/util/AdvancedTlsX509TrustManagerTest.java +++ b/util/src/test/java/io/grpc/util/AdvancedTlsX509TrustManagerTest.java @@ -179,7 +179,17 @@ public void updateTrustCredentials_rotate() throws GeneralSecurityException, IOE fakeClock.forwardTime(1, TimeUnit.MINUTES); assertArrayEquals(serverCert0, trustManager.getAcceptedIssuers()); + long beforeModify = serverCert0File.lastModified(); Files.copy(serverCert1File, serverCert0File); + serverCert0File.setLastModified(beforeModify); + + // although file content changed, file modification time is not changed + fakeClock.forwardTime(1, TimeUnit.MINUTES); + assertArrayEquals(serverCert0, trustManager.getAcceptedIssuers()); + + serverCert0File.setLastModified(beforeModify + 10); + + // file modification time changed fakeClock.forwardTime(1, TimeUnit.MINUTES); assertArrayEquals(serverCert1, trustManager.getAcceptedIssuers()); } From a56e7f10fafaf287ee4eb76980851877e8afa06e Mon Sep 17 00:00:00 2001 From: Albumen Kevin Date: Thu, 9 Jan 2025 19:37:26 +0800 Subject: [PATCH 7/7] Fix import --- .../java/io/grpc/util/AdvancedTlsX509TrustManagerTest.java | 6 ------ 1 file changed, 6 deletions(-) diff --git a/util/src/test/java/io/grpc/util/AdvancedTlsX509TrustManagerTest.java b/util/src/test/java/io/grpc/util/AdvancedTlsX509TrustManagerTest.java index 64918ce5b8a..5ad0e85fa36 100644 --- a/util/src/test/java/io/grpc/util/AdvancedTlsX509TrustManagerTest.java +++ b/util/src/test/java/io/grpc/util/AdvancedTlsX509TrustManagerTest.java @@ -29,14 +29,8 @@ import io.grpc.internal.testing.TestUtils; import io.grpc.testing.TlsTesting; import io.grpc.util.AdvancedTlsX509TrustManager.Verification; -import java.io.BufferedInputStream; -import java.io.BufferedOutputStream; import java.io.File; -import java.io.FileInputStream; -import java.io.FileOutputStream; import java.io.IOException; -import java.io.InputStream; -import java.io.OutputStream; import java.net.Socket; import java.security.GeneralSecurityException; import java.security.cert.CertificateException;