From ac62c8b055cc415f7d8b17edad6abcb3ade548cf Mon Sep 17 00:00:00 2001 From: Eric Anderson Date: Fri, 7 Jan 2022 12:25:39 -0800 Subject: [PATCH] Fix tests and warnings on Java 17 SelfSignedCertificate is not available on Java 17 because OpenJdkSelfSignedCertGenerator is not available. This only impacted tests. AccessController is being removed, and these locations are doing simple reflection which is unlikely to require it even when a security policy is in effect. There's other places we do reflection without the AccessController, so either no security policies care or the users can update their policies to allow it. --- .github/workflows/testing.yml | 2 +- .../io/grpc/internal/ProxyDetectorImpl.java | 2 - .../main/java/io/grpc/netty/JettyTlsUtil.java | 10 +- .../grpc/netty/ProtocolNegotiatorsTest.java | 36 ++++--- .../io/grpc/okhttp/OkHttpChannelBuilder.java | 37 ++++--- .../grpc/okhttp/OkHttpChannelBuilderTest.java | 101 ++++++++---------- .../io/grpc/okhttp/internal/Platform.java | 33 +----- .../io/grpc/xds/SharedCallCounterMapTest.java | 2 + 8 files changed, 97 insertions(+), 126 deletions(-) diff --git a/.github/workflows/testing.yml b/.github/workflows/testing.yml index eb475b2fc7..adc02dc351 100644 --- a/.github/workflows/testing.yml +++ b/.github/workflows/testing.yml @@ -17,7 +17,7 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - jre: [8, 11] + jre: [8, 11, 17] fail-fast: false # Should swap to true if we grow a large matrix steps: diff --git a/core/src/main/java/io/grpc/internal/ProxyDetectorImpl.java b/core/src/main/java/io/grpc/internal/ProxyDetectorImpl.java index c69ee25ef3..7e402bcf65 100644 --- a/core/src/main/java/io/grpc/internal/ProxyDetectorImpl.java +++ b/core/src/main/java/io/grpc/internal/ProxyDetectorImpl.java @@ -135,7 +135,6 @@ class ProxyDetectorImpl implements ProxyDetector { Level.WARNING, "failed to create URL for Authenticator: {0} {1}", new Object[] {protocol, host}); } - // TODO(spencerfang): consider using java.security.AccessController here return Authenticator.requestPasswordAuthentication( host, addr, port, protocol, prompt, scheme, url, Authenticator.RequestorType.PROXY); } @@ -144,7 +143,6 @@ class ProxyDetectorImpl implements ProxyDetector { new Supplier() { @Override public ProxySelector get() { - // TODO(spencerfang): consider using java.security.AccessController here return ProxySelector.getDefault(); } }; diff --git a/netty/src/main/java/io/grpc/netty/JettyTlsUtil.java b/netty/src/main/java/io/grpc/netty/JettyTlsUtil.java index 8652337bba..2f5e3db138 100644 --- a/netty/src/main/java/io/grpc/netty/JettyTlsUtil.java +++ b/netty/src/main/java/io/grpc/netty/JettyTlsUtil.java @@ -17,8 +17,6 @@ package io.grpc.netty; import java.lang.reflect.Method; -import java.security.AccessController; -import java.security.PrivilegedExceptionAction; import javax.net.ssl.SSLContext; import javax.net.ssl.SSLEngine; @@ -42,13 +40,7 @@ final class JettyTlsUtil { SSLContext context = SSLContext.getInstance("TLS"); context.init(null, null, null); SSLEngine engine = context.createSSLEngine(); - Method getApplicationProtocol = - AccessController.doPrivileged(new PrivilegedExceptionAction() { - @Override - public Method run() throws Exception { - return SSLEngine.class.getMethod("getApplicationProtocol"); - } - }); + Method getApplicationProtocol = SSLEngine.class.getMethod("getApplicationProtocol"); getApplicationProtocol.invoke(engine); return null; } catch (Throwable t) { diff --git a/netty/src/test/java/io/grpc/netty/ProtocolNegotiatorsTest.java b/netty/src/test/java/io/grpc/netty/ProtocolNegotiatorsTest.java index af59e765b0..1852213da5 100644 --- a/netty/src/test/java/io/grpc/netty/ProtocolNegotiatorsTest.java +++ b/netty/src/test/java/io/grpc/netty/ProtocolNegotiatorsTest.java @@ -68,6 +68,7 @@ import io.grpc.netty.ProtocolNegotiators.HostPort; import io.grpc.netty.ProtocolNegotiators.ServerTlsHandler; import io.grpc.netty.ProtocolNegotiators.WaitUntilActiveHandler; import io.grpc.testing.TlsTesting; +import io.grpc.util.CertificateUtils; import io.netty.bootstrap.Bootstrap; import io.netty.bootstrap.ServerBootstrap; import io.netty.buffer.ByteBuf; @@ -107,16 +108,13 @@ import io.netty.handler.codec.http2.Http2Settings; import io.netty.handler.proxy.ProxyConnectException; import io.netty.handler.ssl.ApplicationProtocolConfig; import io.netty.handler.ssl.SslContext; -import io.netty.handler.ssl.SslContextBuilder; import io.netty.handler.ssl.SslHandler; import io.netty.handler.ssl.SslHandshakeCompletionEvent; -import io.netty.handler.ssl.util.SelfSignedCertificate; import java.io.File; import java.io.InputStream; import java.net.InetSocketAddress; import java.net.SocketAddress; import java.security.KeyStore; -import java.security.cert.Certificate; import java.security.cert.X509Certificate; import java.util.ArrayDeque; import java.util.Arrays; @@ -478,19 +476,26 @@ public class ProtocolNegotiatorsTest { @Test public void from_tls_managers() throws Exception { - SelfSignedCertificate cert = new SelfSignedCertificate(TestUtils.TEST_SERVER_HOST); KeyStore keyStore = KeyStore.getInstance(KeyStore.getDefaultType()); keyStore.load(null); - keyStore.setKeyEntry("mykey", cert.key(), new char[0], new Certificate[] {cert.cert()}); + try (InputStream server1Chain = TlsTesting.loadCert("server1.pem"); + InputStream server1Key = TlsTesting.loadCert("server1.key")) { + X509Certificate[] chain = CertificateUtils.getX509Certificates(server1Chain); + keyStore.setKeyEntry("key", CertificateUtils.getPrivateKey(server1Key), new char[0], chain); + } KeyManagerFactory keyManagerFactory = KeyManagerFactory.getInstance(KeyManagerFactory.getDefaultAlgorithm()); keyManagerFactory.init(keyStore, new char[0]); KeyStore certStore = KeyStore.getInstance(KeyStore.getDefaultType()); certStore.load(null); - certStore.setCertificateEntry("mycert", cert.cert()); TrustManagerFactory trustManagerFactory = TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm()); + try (InputStream ca = TlsTesting.loadCert("ca.pem")) { + for (X509Certificate cert : CertificateUtils.getX509Certificates(ca)) { + certStore.setCertificateEntry(cert.getSubjectX500Principal().getName("RFC2253"), cert); + } + } trustManagerFactory.init(certStore); ServerCredentials serverCreds = TlsServerCredentials.newBuilder() @@ -504,8 +509,7 @@ public class ProtocolNegotiatorsTest { .build(); InternalChannelz.Tls tls = expectSuccessfulHandshake(channelCreds, serverCreds); assertThat(((X509Certificate) tls.remoteCert).getSubjectX500Principal().getName()) - .isEqualTo("CN=" + TestUtils.TEST_SERVER_HOST); - cert.delete(); + .isEqualTo("CN=*.test.google.com,O=Example\\, Co.,L=Chicago,ST=Illinois,C=US"); } @Test @@ -1214,11 +1218,15 @@ public class ProtocolNegotiatorsTest { @Test public void clientTlsHandler_firesNegotiation() throws Exception { - SelfSignedCertificate cert = new SelfSignedCertificate("authority"); - SslContext clientSslContext = - GrpcSslContexts.configure(SslContextBuilder.forClient().trustManager(cert.cert())).build(); - SslContext serverSslContext = - GrpcSslContexts.configure(SslContextBuilder.forServer(cert.key(), cert.cert())).build(); + SslContext clientSslContext; + try (InputStream ca = TlsTesting.loadCert("ca.pem")) { + clientSslContext = GrpcSslContexts.forClient().trustManager(ca).build(); + } + SslContext serverSslContext; + try (InputStream server1Key = TlsTesting.loadCert("server1.key"); + InputStream server1Chain = TlsTesting.loadCert("server1.pem")) { + serverSslContext = GrpcSslContexts.forServer(server1Chain, server1Key).build(); + } FakeGrpcHttp2ConnectionHandler gh = FakeGrpcHttp2ConnectionHandler.newHandler(); ClientTlsProtocolNegotiator pn = new ClientTlsProtocolNegotiator(clientSslContext, null); WriteBufferingAndExceptionHandler clientWbaeh = @@ -1404,7 +1412,7 @@ public class ProtocolNegotiatorsTest { @Override public String getAuthority() { - return "authority"; + return "foo.test.google.fr"; } } diff --git a/okhttp/src/main/java/io/grpc/okhttp/OkHttpChannelBuilder.java b/okhttp/src/main/java/io/grpc/okhttp/OkHttpChannelBuilder.java index 24e5083632..1550811034 100644 --- a/okhttp/src/main/java/io/grpc/okhttp/OkHttpChannelBuilder.java +++ b/okhttp/src/main/java/io/grpc/okhttp/OkHttpChannelBuilder.java @@ -54,6 +54,7 @@ import io.grpc.okhttp.internal.TlsVersion; import io.grpc.util.CertificateUtils; import java.io.ByteArrayInputStream; import java.io.IOException; +import java.io.InputStream; import java.net.InetSocketAddress; import java.net.SocketAddress; import java.security.GeneralSecurityException; @@ -667,21 +668,24 @@ public final class OkHttpChannelBuilder extends ForwardingChannelBuilder2 serverSocket = SettableFuture.create(); @@ -194,9 +187,12 @@ public class OkHttpChannelBuilderTest { } }).start(); - ChannelCredentials creds = TlsChannelCredentials.newBuilder() - .trustManager(cert.certificate()) + ChannelCredentials creds; + try (InputStream ca = TlsTesting.loadCert("ca.pem")) { + creds = TlsChannelCredentials.newBuilder() + .trustManager(ca) .build(); + } OkHttpChannelBuilder.SslSocketFactoryResult result = OkHttpChannelBuilder.sslSocketFactoryFrom(creds); SSLSocket socket = @@ -208,24 +204,19 @@ public class OkHttpChannelBuilderTest { @Test public void sslSocketFactoryFrom_tls_mtls() throws Exception { - SelfSignedCertificate cert = new SelfSignedCertificate(TestUtils.TEST_SERVER_HOST); - KeyStore keyStore = KeyStore.getInstance(KeyStore.getDefaultType()); - keyStore.load(null); - keyStore.setKeyEntry("mykey", cert.key(), new char[0], new Certificate[] {cert.cert()}); - KeyManagerFactory keyManagerFactory = - KeyManagerFactory.getInstance(KeyManagerFactory.getDefaultAlgorithm()); - keyManagerFactory.init(keyStore, new char[0]); + KeyManager[] keyManagers; + try (InputStream server1Chain = TlsTesting.loadCert("server1.pem"); + InputStream server1Key = TlsTesting.loadCert("server1.key")) { + keyManagers = OkHttpChannelBuilder.createKeyManager(server1Chain, server1Key); + } - KeyStore certStore = KeyStore.getInstance(KeyStore.getDefaultType()); - certStore.load(null); - certStore.setCertificateEntry("mycert", cert.cert()); - TrustManagerFactory trustManagerFactory = - TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm()); - trustManagerFactory.init(certStore); + TrustManager[] trustManagers; + try (InputStream ca = TlsTesting.loadCert("ca.pem")) { + trustManagers = OkHttpChannelBuilder.createTrustManager(ca); + } SSLContext serverContext = SSLContext.getInstance("TLS"); - serverContext.init( - keyManagerFactory.getKeyManagers(), trustManagerFactory.getTrustManagers(), null); + serverContext.init(keyManagers, trustManagers, null); final SSLServerSocket serverListenSocket = (SSLServerSocket) serverContext.getServerSocketFactory().createServerSocket(0); serverListenSocket.setNeedClientAuth(true); @@ -244,8 +235,8 @@ public class OkHttpChannelBuilderTest { }).start(); ChannelCredentials creds = TlsChannelCredentials.newBuilder() - .keyManager(keyManagerFactory.getKeyManagers()) - .trustManager(trustManagerFactory.getTrustManagers()) + .keyManager(keyManagers) + .trustManager(trustManagers) .build(); OkHttpChannelBuilder.SslSocketFactoryResult result = OkHttpChannelBuilder.sslSocketFactoryFrom(creds); @@ -253,31 +244,22 @@ public class OkHttpChannelBuilderTest { (SSLSocket) result.factory.createSocket("localhost", serverListenSocket.getLocalPort()); socket.getSession(); // Force handshake assertThat(((X500Principal) serverSocket.get().getSession().getPeerPrincipal()).getName()) - .isEqualTo("CN=" + TestUtils.TEST_SERVER_HOST); + .isEqualTo("CN=*.test.google.com,O=Example\\, Co.,L=Chicago,ST=Illinois,C=US"); socket.close(); serverSocket.get().close(); } @Test public void sslSocketFactoryFrom_tls_mtls_keyFile() throws Exception { - SelfSignedCertificate cert = new SelfSignedCertificate(TestUtils.TEST_SERVER_HOST); - KeyStore keyStore = KeyStore.getInstance(KeyStore.getDefaultType()); - keyStore.load(null); - keyStore.setKeyEntry("mykey", cert.key(), new char[0], new Certificate[] {cert.cert()}); - KeyManagerFactory keyManagerFactory = - KeyManagerFactory.getInstance(KeyManagerFactory.getDefaultAlgorithm()); - keyManagerFactory.init(keyStore, new char[0]); - - KeyStore certStore = KeyStore.getInstance(KeyStore.getDefaultType()); - certStore.load(null); - certStore.setCertificateEntry("mycert", cert.cert()); - TrustManagerFactory trustManagerFactory = - TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm()); - trustManagerFactory.init(certStore); - SSLContext serverContext = SSLContext.getInstance("TLS"); - serverContext.init( - keyManagerFactory.getKeyManagers(), trustManagerFactory.getTrustManagers(), null); + try (InputStream server1Chain = TlsTesting.loadCert("server1.pem"); + InputStream server1Key = TlsTesting.loadCert("server1.key"); + InputStream ca = TlsTesting.loadCert("ca.pem")) { + serverContext.init( + OkHttpChannelBuilder.createKeyManager(server1Chain, server1Key), + OkHttpChannelBuilder.createTrustManager(ca), + null); + } final SSLServerSocket serverListenSocket = (SSLServerSocket) serverContext.getServerSocketFactory().createServerSocket(0); serverListenSocket.setNeedClientAuth(true); @@ -295,17 +277,22 @@ public class OkHttpChannelBuilderTest { } }).start(); - ChannelCredentials creds = TlsChannelCredentials.newBuilder() - .keyManager(cert.certificate(), cert.privateKey()) - .trustManager(cert.certificate()) - .build(); + ChannelCredentials creds; + try (InputStream server1Chain = TlsTesting.loadCert("server1.pem"); + InputStream server1Key = TlsTesting.loadCert("server1.key"); + InputStream ca = TlsTesting.loadCert("ca.pem")) { + creds = TlsChannelCredentials.newBuilder() + .keyManager(server1Chain, server1Key) + .trustManager(ca) + .build(); + } OkHttpChannelBuilder.SslSocketFactoryResult result = OkHttpChannelBuilder.sslSocketFactoryFrom(creds); SSLSocket socket = (SSLSocket) result.factory.createSocket("localhost", serverListenSocket.getLocalPort()); socket.getSession(); // Force handshake assertThat(((X500Principal) serverSocket.get().getSession().getPeerPrincipal()).getName()) - .isEqualTo("CN=" + TestUtils.TEST_SERVER_HOST); + .isEqualTo("CN=*.test.google.com,O=Example\\, Co.,L=Chicago,ST=Illinois,C=US"); socket.close(); serverSocket.get().close(); } diff --git a/okhttp/third_party/okhttp/main/java/io/grpc/okhttp/internal/Platform.java b/okhttp/third_party/okhttp/main/java/io/grpc/okhttp/internal/Platform.java index 45d874e300..6ed3bc50b8 100644 --- a/okhttp/third_party/okhttp/main/java/io/grpc/okhttp/internal/Platform.java +++ b/okhttp/third_party/okhttp/main/java/io/grpc/okhttp/internal/Platform.java @@ -28,11 +28,8 @@ import java.lang.reflect.Proxy; import java.net.InetSocketAddress; import java.net.Socket; import java.net.SocketException; -import java.security.AccessController; import java.security.KeyManagementException; import java.security.NoSuchAlgorithmException; -import java.security.PrivilegedActionException; -import java.security.PrivilegedExceptionAction; import java.security.Provider; import java.security.Security; import java.util.ArrayList; @@ -218,41 +215,21 @@ public class Platform { SSLContext context = SSLContext.getInstance("TLS", sslProvider); context.init(null, null, null); SSLEngine engine = context.createSSLEngine(); - Method getEngineApplicationProtocol = - AccessController.doPrivileged( - new PrivilegedExceptionAction() { - @Override - public Method run() throws Exception { - return SSLEngine.class.getMethod("getApplicationProtocol"); - } - }); + Method getEngineApplicationProtocol = SSLEngine.class.getMethod("getApplicationProtocol"); getEngineApplicationProtocol.invoke(engine); Method setApplicationProtocols = - AccessController.doPrivileged( - new PrivilegedExceptionAction() { - @Override - public Method run() throws Exception { - return SSLParameters.class.getMethod("setApplicationProtocols", String[].class); - } - }); - Method getApplicationProtocol = - AccessController.doPrivileged( - new PrivilegedExceptionAction() { - @Override - public Method run() throws Exception { - return SSLSocket.class.getMethod("getApplicationProtocol"); - } - }); + SSLParameters.class.getMethod("setApplicationProtocols", String[].class); + Method getApplicationProtocol = SSLSocket.class.getMethod("getApplicationProtocol"); return new JdkAlpnPlatform(sslProvider, setApplicationProtocols, getApplicationProtocol); } catch (NoSuchAlgorithmException ignored) { // On older Java } catch (KeyManagementException ignored) { // On older Java - } catch (PrivilegedActionException ignored) { - // On older Java } catch (IllegalAccessException ignored) { // On older Java + } catch (NoSuchMethodException ignored) { + // On older Java } catch (InvocationTargetException ignored) { // On older Java } diff --git a/xds/src/test/java/io/grpc/xds/SharedCallCounterMapTest.java b/xds/src/test/java/io/grpc/xds/SharedCallCounterMapTest.java index 3051a02187..3b10c0c0e3 100644 --- a/xds/src/test/java/io/grpc/xds/SharedCallCounterMapTest.java +++ b/xds/src/test/java/io/grpc/xds/SharedCallCounterMapTest.java @@ -54,6 +54,7 @@ public class SharedCallCounterMapTest { final CounterReference ref = counters.get(CLUSTER).get(EDS_SERVICE_NAME); counter = null; GcFinalization.awaitDone(new FinalizationPredicate() { + @SuppressWarnings("deprecation") // Use refersTo(null) once we require Java 17+ @Override public boolean isDone() { return ref.isEnqueued(); @@ -71,6 +72,7 @@ public class SharedCallCounterMapTest { assertThat(counter.get()).isEqualTo(0); counter = null; GcFinalization.awaitDone(new FinalizationPredicate() { + @SuppressWarnings("deprecation") // Use refersTo(null) once we require Java 17+ @Override public boolean isDone() { return ref.isEnqueued();