Endpoint URI validation

Author: rcameronm

src/main/java/emissary/grpc/GrpcRoutingPlace.java

@@ -9,6 +9,8 @@
 
 import com.google.common.util.concurrent.ListenableFuture;
 import com.google.protobuf.Message;
+import io.grpc.ChannelCredentials;
+import io.grpc.InsecureChannelCredentials;
 import io.grpc.ManagedChannel;
 import io.grpc.Status;
 import io.grpc.StatusRuntimeException;
@@ -113,9 +115,11 @@ private void configureGrpc() {
 
         RetryHandler retryHandler = new RetryHandler(configG, this.getPlaceName(), this::retryOnException);
         String channelManagerName = configG.findStringEntry(CHANNEL_MANAGER_CLASS_NAME, PooledChannelManager.class.getName());
+        ChannelCredentials channelCredentials = getChannelCredentials(configG);
 
         for (String id : targetIds) {
-            ChannelManager channelManager = ChannelManager.ofSubClass(channelManagerName, hosts.get(id), ports.get(id), configG);
+            ChannelManager channelManager = ChannelManager.ofSubClass(
+                    channelManagerName, hosts.get(id), ports.get(id), configG, channelCredentials);
             GrpcInvoker grpcInvoker = new GrpcInvoker(channelManager, retryHandler);
             invokerTable.put(id, grpcInvoker);
         }
@@ -130,6 +134,18 @@ protected Map<String, Integer> getPortNumberConfigs() {
                 .collect(Collectors.toMap(Map.Entry::getKey, entry -> Integer.parseInt(entry.getValue())));
     }
 
+    /**
+     * Creates a new {@link ChannelCredentials} object with all security and encryption configurations necessary for
+     * establishing a gRPC connection. Base class returns {@link InsecureChannelCredentials}, which asserts that no client
+     * identity, authentication, or encryption is to be used. Subclasses should override this method as necessary.
+     *
+     * @param configG any necessary configurations for creating the credentials object
+     * @return gRPC channel credentials
+     */
+    protected ChannelCredentials getChannelCredentials(Configurator configG) {
+        return InsecureChannelCredentials.create();
+    }
+
     /**
      * Determines if the {@link RetryHandler} should try again when an exception is thrown during gRPC invocation. Default
      * behavior attempts retries for any Exception with a {@link Status.Code} in {@link #RETRY_GRPC_CODES}. Subclasses may

src/main/java/emissary/grpc/channel/ChannelManager.java

@@ -4,7 +4,6 @@
 
 import io.grpc.ChannelCredentials;
 import io.grpc.Grpc;
-import io.grpc.InsecureChannelCredentials;
 import io.grpc.ManagedChannel;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -34,6 +33,10 @@ public abstract class ChannelManager implements AutoCloseable {
     public static final String MAX_INBOUND_MESSAGE_BYTE_SIZE = GRPC_CHANNEL_PREFIX + "MAX_INBOUND_MESSAGE_BYTE_SIZE";
     public static final String MAX_INBOUND_METADATA_BYTE_SIZE = GRPC_CHANNEL_PREFIX + "MAX_INBOUND_METADATA_BYTE_SIZE";
 
+    protected static final String LOCALHOST = "localhost";
+    protected static final String DNS_PREFIX = "dns:///";
+    protected static final int MAX_PORT_NUMBER = 0xFFFF;
+
     protected final Logger logger;
 
     protected final String host;
@@ -54,16 +57,17 @@ public abstract class ChannelManager implements AutoCloseable {
      * @param host gRPC service hostname or DNS target
      * @param port gRPC service port
      * @param configG configuration provider for channel parameters
+     * @param credentials transport-level credentials for the gRPC server
      * @see ChannelManager
      * @see <a href="https://docs.microsoft.com/en-us/aspnet/core/grpc/performance?view=aspnetcore-5.0">Source</a> for
      *      default gRPC configurations.
      */
-    protected ChannelManager(String host, int port, Configurator configG) {
+    protected ChannelManager(String host, int port, Configurator configG, ChannelCredentials credentials) {
         this.logger = LoggerFactory.getLogger(this.getClass().getName());
 
-        this.host = host;
-        this.port = port;
-        this.target = host + ":" + port; // target may be a host or dns service
+        this.host = validateHostName(host);
+        this.port = validatePortNumber(port);
+        this.target = createTarget(host, port);
 
         // How often (in milliseconds) to send pings when the connection is idle
         this.keepAliveMillis = configG.findLongEntry(KEEP_ALIVE_MILLIS, 60000L);
@@ -84,32 +88,42 @@ protected ChannelManager(String host, int port, Configurator configG) {
         this.maxInboundMessageByteSize = configG.findIntEntry(MAX_INBOUND_MESSAGE_BYTE_SIZE, 4 << 20); // 4 MiB
         this.maxInboundMetadataByteSize = configG.findIntEntry(MAX_INBOUND_METADATA_BYTE_SIZE, 8 << 10); // 8 KiB
 
-        this.channelCredentials = createChannelCredentials(configG);
+        this.channelCredentials = credentials;
     }
 
-    /**
-     * Creates a new {@link ChannelCredentials} object with all security and encryption configurations necessary for
-     * establishing a gRPC connection. Base class returns {@link InsecureChannelCredentials}, which asserts that no client
-     * identity, authentication, or encryption is to be used. Subclasses should override this method as necessary.
-     *
-     * @param configG any necessary configurations for creating the credentials object
-     * @return gRPC channel credentials
-     */
-    protected ChannelCredentials createChannelCredentials(Configurator configG) {
-        return InsecureChannelCredentials.create();
+    protected String validateHostName(String host) {
+        if (host.equals(LOCALHOST) || host.startsWith(DNS_PREFIX)) {
+            return host;
+        }
+        throw new IllegalArgumentException(String.format("Expected DNS URI prefix \"dns:///\" but got \"%s\"", host));
+    }
+
+    protected int validatePortNumber(int port) {
+        if (port > 0 && port <= MAX_PORT_NUMBER) {
+            return port;
+        }
+        throw new IllegalArgumentException(String.format("Port \"%d\" is outside valid range [1, %d]", port, MAX_PORT_NUMBER));
+    }
+
+    protected String createTarget(String host, int port) {
+        return host + ":" + port;
     }
 
-    public static ChannelManager ofSubClass(String className, String host, int port, Configurator configG) {
+    public static ChannelManager ofSubClass(
+            String className, String host, int port, Configurator configG, ChannelCredentials credentials) {
         try {
-            return ofSubClass(Class.forName(className).asSubclass(ChannelManager.class), host, port, configG);
+            return ofSubClass(Class.forName(className).asSubclass(ChannelManager.class), host, port, configG, credentials);
         } catch (ClassNotFoundException e) {
             throw new IllegalArgumentException("Cannot find class: " + className, e);
         }
     }
 
-    public static ChannelManager ofSubClass(Class<? extends ChannelManager> clz, String host, int port, Configurator configG) {
+    public static ChannelManager ofSubClass(
+            Class<? extends ChannelManager> clz, String host, int port, Configurator configG, ChannelCredentials credentials) {
         try {
-            return clz.getDeclaredConstructor(String.class, int.class, Configurator.class).newInstance(host, port, configG);
+            return clz
+                    .getDeclaredConstructor(String.class, int.class, Configurator.class, ChannelCredentials.class)
+                    .newInstance(host, port, configG, credentials);
         } catch (ReflectiveOperationException e) {
             throw new IllegalStateException("Unable to instantiate new ChannelManager: " + e.getMessage(), e);
         }

src/main/java/emissary/grpc/channel/PooledChannelManager.java

@@ -3,6 +3,7 @@
 import emissary.config.Configurator;
 import emissary.grpc.exceptions.GrpcExceptionUtils;
 
+import io.grpc.ChannelCredentials;
 import io.grpc.ManagedChannel;
 import org.apache.commons.pool2.ObjectPool;
 import org.apache.commons.pool2.PoolUtils;
@@ -58,8 +59,8 @@ public class PooledChannelManager extends ChannelManager implements PooledObject
      * @see ChannelManager
      * @see PooledChannelManager
      */
-    public PooledChannelManager(String host, int port, Configurator configG) {
-        super(host, port, configG);
+    public PooledChannelManager(String host, int port, Configurator configG, ChannelCredentials credentials) {
+        super(host, port, configG, credentials);
 
         GenericObjectPoolConfig<ManagedChannel> poolConfig = new GenericObjectPoolConfig<>();
 

src/main/java/emissary/grpc/channel/SingletonChannelManager.java

@@ -2,6 +2,7 @@
 
 import emissary.config.Configurator;
 
+import io.grpc.ChannelCredentials;
 import io.grpc.ManagedChannel;
 
 /**
@@ -19,8 +20,8 @@ public class SingletonChannelManager extends ChannelManager {
      * @param configG configuration provider for channel parameters
      * @see ChannelManager
      */
-    public SingletonChannelManager(String host, int port, Configurator configG) {
-        super(host, port, configG);
+    public SingletonChannelManager(String host, int port, Configurator configG, ChannelCredentials credentials) {
+        super(host, port, configG, credentials);
         channel = create();
     }
 

src/test/java/emissary/grpc/channel/ChannelManagerTest.java

@@ -0,0 +1,72 @@
+package emissary.grpc.channel;
+
+import emissary.config.ServiceConfigGuide;
+import emissary.test.core.junit5.UnitTest;
+
+import io.grpc.InsecureChannelCredentials;
+import io.grpc.ManagedChannel;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
+
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+class ChannelManagerTest extends UnitTest {
+    @ParameterizedTest
+    @ValueSource(strings = {"localhost", "dns:///foo.bar"})
+    void testGoodHostName(String host) {
+        Runnable invocation = () -> new TestChannelManager(host, 1).close();
+        assertDoesNotThrow(invocation::run);
+    }
+
+    @ParameterizedTest
+    @ValueSource(strings = {"dns:foo.bar", "dns:/foo.bar", "dns://foo.bar", "http:///foo.bar", "https:///foo.bar", "foo.bar"})
+    void testBadHostName(String host) {
+        Runnable invocation = () -> new TestChannelManager(host, 1).close();
+        IllegalArgumentException e = assertThrows(IllegalArgumentException.class, invocation::run);
+        assertEquals(String.format("Expected DNS URI prefix \"dns:///\" but got \"%s\"", host), e.getMessage());
+    }
+
+    @ParameterizedTest
+    @ValueSource(ints = {1, 8000, 8001, 8080, 65535})
+    void testGoodPortNumber(int port) {
+        Runnable invocation = () -> new TestChannelManager("dns:///foo.bar", port).close();
+        assertDoesNotThrow(invocation::run);
+    }
+
+    @ParameterizedTest
+    @ValueSource(ints = {-1, 0, 65536})
+    void testBadPortNumber(int port) {
+        Runnable invocation = () -> new TestChannelManager("dns:///foo.bar", port).close();
+        IllegalArgumentException e = assertThrows(IllegalArgumentException.class, invocation::run);
+        assertEquals(String.format("Port \"%d\" is outside valid range [1, 65535]", port), e.getMessage());
+    }
+
+
+    private static class TestChannelManager extends ChannelManager {
+        public TestChannelManager(String host, int port) {
+            super(host, port, new ServiceConfigGuide(), InsecureChannelCredentials.create());
+        }
+
+        @Override
+        public ManagedChannel acquire() {
+            return null;
+        }
+
+        @Override
+        public void release(ManagedChannel channel) {
+            /* No-op */
+        }
+
+        @Override
+        public void shutdown(ManagedChannel channel) {
+            /* No-op */
+        }
+
+        @Override
+        public void close() {
+            /* No-op */
+        }
+    }
+}

src/test/java/emissary/grpc/channel/PooledChannelManagerTest.java

@@ -6,6 +6,7 @@
 import emissary.grpc.channel.PooledChannelManager.PoolRetrievalOrdering;
 import emissary.test.core.junit5.UnitTest;
 
+import io.grpc.InsecureChannelCredentials;
 import io.grpc.ManagedChannel;
 import org.junit.jupiter.api.Test;
 
@@ -22,7 +23,7 @@
 class PooledChannelManagerTest extends UnitTest {
 
     private static PooledChannelManager newChannelManager(Configurator configT) {
-        return new PooledChannelManager("localhost", 1234, configT);
+        return new PooledChannelManager("localhost", 1234, configT, InsecureChannelCredentials.create());
     }
 
     private static Configurator buildConfigs(ConfigEntry... configEntries) {

src/test/java/emissary/grpc/channel/SingletonChannelManagerTest.java

@@ -4,6 +4,7 @@
 import emissary.config.ServiceConfigGuide;
 import emissary.test.core.junit5.UnitTest;
 
+import io.grpc.InsecureChannelCredentials;
 import io.grpc.ManagedChannel;
 import org.junit.jupiter.api.Test;
 
@@ -12,7 +13,7 @@
 
 class SingletonChannelManagerTest extends UnitTest {
     private static SingletonChannelManager newChannelManager(Configurator configT) {
-        return new SingletonChannelManager("localhost", 1234, configT);
+        return new SingletonChannelManager("localhost", 1234, configT, InsecureChannelCredentials.create());
     }
 
     @Test