Revert "Adding support for TLS connections for gRPC clients"
This reverts commit 47a3aa697049bf53e8dd4cb76cf9ca262762a175.
Change-Id: I457594c3f423f276b9c49dbbaf068284a36ba9f1
diff --git a/protocols/grpc/BUILD b/protocols/grpc/BUILD
index 2debf84..26e178c 100644
--- a/protocols/grpc/BUILD
+++ b/protocols/grpc/BUILD
@@ -18,11 +18,18 @@
"@io_opencensus_opencensus_api//jar",
"@io_opencensus_opencensus_contrib_grpc_metrics//jar",
"@com_google_code_gson_gson//jar",
- # Adding Netty http and http2 codecs
- # The rest of Netty is loaded in the onos-thirdparty-base feature
- # See tools/package/features/BUILD for more details
+ # Lazily adding all netty-related packages.
+ # Some of them might not be necessary.
+ "@io_netty_netty//jar",
+ "@io_netty_netty_buffer//jar",
+ "@io_netty_netty_codec//jar",
"@io_netty_netty_codec_http//jar",
"@io_netty_netty_codec_http2//jar",
+ "@io_netty_netty_common//jar",
+ "@io_netty_netty_handler//jar",
+ "@io_netty_netty_transport//jar",
+ "@io_netty_netty_transport_native_epoll//jar",
+ "@io_netty_netty_resolver//jar",
]
onos_app(
diff --git a/protocols/grpc/ctl/BUILD b/protocols/grpc/ctl/BUILD
index 475a90e..ac0703d 100644
--- a/protocols/grpc/ctl/BUILD
+++ b/protocols/grpc/ctl/BUILD
@@ -3,7 +3,6 @@
"//protocols/grpc/proto:onos-protocols-grpc-proto",
"@io_grpc_grpc_java//core",
"@io_grpc_grpc_java//netty",
- "@io_netty_netty_handler//jar",
]
osgi_jar(
diff --git a/protocols/grpc/ctl/src/main/java/org/onosproject/grpc/ctl/AbstractGrpcClientController.java b/protocols/grpc/ctl/src/main/java/org/onosproject/grpc/ctl/AbstractGrpcClientController.java
index db29284..375ff32 100644
--- a/protocols/grpc/ctl/src/main/java/org/onosproject/grpc/ctl/AbstractGrpcClientController.java
+++ b/protocols/grpc/ctl/src/main/java/org/onosproject/grpc/ctl/AbstractGrpcClientController.java
@@ -19,11 +19,8 @@
import com.google.common.collect.Maps;
import com.google.common.util.concurrent.Striped;
import io.grpc.ManagedChannel;
-import io.grpc.netty.GrpcSslContexts;
+import io.grpc.ManagedChannelBuilder;
import io.grpc.netty.NettyChannelBuilder;
-import io.netty.handler.ssl.NotSslRecordException;
-import io.netty.handler.ssl.SslContext;
-import io.netty.handler.ssl.util.InsecureTrustManagerFactory;
import org.onosproject.event.AbstractListenerManager;
import org.onosproject.event.Event;
import org.onosproject.event.EventListener;
@@ -39,14 +36,12 @@
import org.osgi.service.component.annotations.ReferenceCardinality;
import org.slf4j.Logger;
-import javax.net.ssl.SSLException;
import java.io.IOException;
import java.util.Map;
import java.util.concurrent.locks.Lock;
import java.util.function.Supplier;
import static com.google.common.base.Preconditions.checkNotNull;
-import static com.google.common.base.Preconditions.checkState;
import static org.slf4j.LoggerFactory.getLogger;
/**
@@ -96,15 +91,10 @@
@Override
public boolean createClient(K clientKey) {
checkNotNull(clientKey);
- /*
- FIXME we might want to move "useTls" and "fallback" to properties of the netcfg and clientKey
- For now, we will first try to connect with TLS (accepting any cert), then fall back to
- plaintext for every device
- */
- return withDeviceLock(() -> doCreateClient(clientKey, true, true), clientKey.deviceId());
+ return withDeviceLock(() -> doCreateClient(clientKey), clientKey.deviceId());
}
- private boolean doCreateClient(K clientKey, boolean useTls, boolean fallbackToPlainText) {
+ private boolean doCreateClient(K clientKey) {
DeviceId deviceId = clientKey.deviceId();
String serverAddr = clientKey.serverAddr();
int serverPort = clientKey.serverPort();
@@ -122,57 +112,20 @@
doRemoveClient(deviceId);
}
}
- log.info("Creating client for {} (server={}:{})...", deviceId, serverAddr, serverPort);
-
- SslContext sslContext = null;
- if (useTls) {
- try {
- // Accept any server certificate; this is insecure and should not be used in production
- sslContext = GrpcSslContexts.forClient().trustManager(InsecureTrustManagerFactory.INSTANCE).build();
- } catch (SSLException e) {
- log.error("Failed to build SSL Context", e);
- return false;
- }
- }
-
+ log.info("Creating client for {} (server={}:{})...",
+ deviceId, serverAddr, serverPort);
GrpcChannelId channelId = GrpcChannelId.of(clientKey.deviceId(), clientKey.toString());
- NettyChannelBuilder channelBuilder = NettyChannelBuilder
+ ManagedChannelBuilder channelBuilder = NettyChannelBuilder
.forAddress(serverAddr, serverPort)
- .maxInboundMessageSize(DEFAULT_MAX_INBOUND_MSG_SIZE * MEGABYTES);
- if (sslContext != null) {
- log.debug("Using SSL for gRPC connection to {}", deviceId);
- channelBuilder
- .sslContext(sslContext)
- .useTransportSecurity();
- } else {
- checkState(!useTls,
- "Not authorized to use plaintext for gRPC connection to {}", deviceId);
- log.debug("Using plaintext TCP for gRPC connection to {}", deviceId);
- channelBuilder.usePlaintext();
- }
+ .maxInboundMessageSize(DEFAULT_MAX_INBOUND_MSG_SIZE * MEGABYTES)
+ .usePlaintext();
ManagedChannel channel;
try {
channel = grpcChannelController.connectChannel(channelId, channelBuilder);
} catch (IOException e) {
- for (Throwable cause = e; cause != null; cause = cause.getCause()) {
- if (useTls && cause instanceof NotSslRecordException) {
- // Likely root cause is that server is using plaintext
- log.info("Failed to connect to server (device={}) using TLS", deviceId);
- log.debug("TLS connection exception", e);
- if (fallbackToPlainText) {
- log.info("Falling back to plaintext for connection to {}", deviceId);
- return doCreateClient(clientKey, false, false);
- }
- }
- if (!useTls && "Connection reset by peer".equals(cause.getMessage())) {
- // Not a great signal, but could indicate the server is expected a TLS connection
- log.error("Failed to connect to server (device={}) using plaintext TCP; is the server using TLS?",
- deviceId);
- break;
- }
- }
- log.warn("Unable to connect to gRPC server for {}", deviceId, e);
+ log.warn("Unable to connect to gRPC server of {}: {}",
+ clientKey.deviceId(), e.getMessage());
return false;
}