Merge pull request #2658 from jtattermusch/csharp_explicit_insecure

Make insecure channel/server creation explicit in C#

src/csharp/Grpc.Core.Tests/ClientServerTest.cs

@@ -79,9 +79,9 @@ namespace Grpc.Core.Tests
         {
             server = new Server();
             server.AddServiceDefinition(ServiceDefinition);
-            int port = server.AddListeningPort(Host, Server.PickUnusedPort);
+            int port = server.AddPort(Host, Server.PickUnusedPort, ServerCredentials.Insecure);
             server.Start();
-            channel = new Channel(Host, port);
+            channel = new Channel(Host, port, Credentials.Insecure);
         }
 
         [TearDown]

src/csharp/Grpc.Core.Tests/ServerTest.cs

@@ -45,7 +45,7 @@ namespace Grpc.Core.Tests
         public void StartAndShutdownServer()
         {
             Server server = new Server();
-            server.AddListeningPort("localhost", Server.PickUnusedPort);
+            server.AddPort("localhost", Server.PickUnusedPort, ServerCredentials.Insecure);
             server.Start();
             server.ShutdownAsync().Wait();
             GrpcEnvironment.Shutdown();

src/csharp/Grpc.Core.Tests/TimeoutsTest.cs

@@ -72,9 +72,9 @@ namespace Grpc.Core.Tests
         {
             server = new Server();
             server.AddServiceDefinition(ServiceDefinition);
-            int port = server.AddListeningPort(Host, Server.PickUnusedPort);
+            int port = server.AddPort(Host, Server.PickUnusedPort, ServerCredentials.Insecure);
             server.Start();
-            channel = new Channel(Host, port);
+            channel = new Channel(Host, port, Credentials.Insecure);
 
             stringFromServerHandlerTcs = new TaskCompletionSource<string>();
         }

src/csharp/Grpc.Core/Channel.cs

@@ -56,26 +56,24 @@ namespace Grpc.Core
         /// Port will default to 80 for an unsecure channel and to 443 a secure channel.
         /// </summary>
         /// <param name="host">The DNS name of IP address of the host.</param>
-        /// <param name="credentials">Optional credentials to create a secure channel.</param>
+        /// <param name="credentials">Credentials to secure the channel.</param>
         /// <param name="options">Channel options.</param>
-        public Channel(string host, Credentials credentials = null, IEnumerable<ChannelOption> options = null)
+        public Channel(string host, Credentials credentials, IEnumerable<ChannelOption> options = null)
         {
             this.environment = GrpcEnvironment.GetInstance();
             this.options = options != null ? new List<ChannelOption>(options) : new List<ChannelOption>();
 
             EnsureUserAgentChannelOption(this.options);
+            using (CredentialsSafeHandle nativeCredentials = credentials.ToNativeCredentials())
             using (ChannelArgsSafeHandle nativeChannelArgs = ChannelOptions.CreateChannelArgs(this.options))
             {
-                if (credentials != null)
+                if (nativeCredentials != null)
                 {
-                    using (CredentialsSafeHandle nativeCredentials = credentials.ToNativeCredentials())
-                    {
-                        this.handle = ChannelSafeHandle.CreateSecure(nativeCredentials, host, nativeChannelArgs);
-                    }
+                    this.handle = ChannelSafeHandle.CreateSecure(nativeCredentials, host, nativeChannelArgs);
                 }
                 else
                 {
-                    this.handle = ChannelSafeHandle.Create(host, nativeChannelArgs);
+                    this.handle = ChannelSafeHandle.CreateInsecure(host, nativeChannelArgs);
                 }
             }
             this.target = GetOverridenTarget(host, this.options);
@@ -86,9 +84,9 @@ namespace Grpc.Core
         /// </summary>
         /// <param name="host">DNS name or IP address</param>
         /// <param name="port">the port</param>
-        /// <param name="credentials">Optional credentials to create a secure channel.</param>
+        /// <param name="credentials">Credentials to secure the channel.</param>
         /// <param name="options">Channel options.</param>
-        public Channel(string host, int port, Credentials credentials = null, IEnumerable<ChannelOption> options = null) :
+        public Channel(string host, int port, Credentials credentials, IEnumerable<ChannelOption> options = null) :
             this(string.Format("{0}:{1}", host, port), credentials, options)
         {
         }

src/csharp/Grpc.Core/Credentials.cs

@@ -41,17 +41,40 @@ namespace Grpc.Core
     /// </summary>
     public abstract class Credentials
     {
+        static readonly Credentials InsecureInstance = new InsecureCredentialsImpl();
+
+        /// <summary>
+        /// Returns instance of credential that provides no security and 
+        /// will result in creating an unsecure channel with no encryption whatsoever.
+        /// </summary>
+        public static Credentials Insecure
+        {
+            get
+            {
+                return InsecureInstance;
+            }
+        }
+
         /// <summary>
-        /// Creates native object for the credentials.
+        /// Creates native object for the credentials. May return null if insecure channel
+        /// should be created.
         /// </summary>
         /// <returns>The native credentials.</returns>
         internal abstract CredentialsSafeHandle ToNativeCredentials();
+
+        private sealed class InsecureCredentialsImpl : Credentials
+        {
+            internal override CredentialsSafeHandle ToNativeCredentials()
+            {
+                return null;
+            }
+        }
     }
 
     /// <summary>
     /// Client-side SSL credentials.
     /// </summary>
-    public class SslCredentials : Credentials
+    public sealed class SslCredentials : Credentials
     {
         readonly string rootCertificates;
         readonly KeyCertificatePair keyCertificatePair;

src/csharp/Grpc.Core/Internal/ChannelSafeHandle.cs

@@ -41,7 +41,7 @@ namespace Grpc.Core.Internal
     internal class ChannelSafeHandle : SafeHandleZeroIsInvalid
     {
         [DllImport("grpc_csharp_ext.dll")]
-        static extern ChannelSafeHandle grpcsharp_channel_create(string target, ChannelArgsSafeHandle channelArgs);
+        static extern ChannelSafeHandle grpcsharp_insecure_channel_create(string target, ChannelArgsSafeHandle channelArgs);
 
         [DllImport("grpc_csharp_ext.dll")]
         static extern ChannelSafeHandle grpcsharp_secure_channel_create(CredentialsSafeHandle credentials, string target, ChannelArgsSafeHandle channelArgs);
@@ -56,9 +56,9 @@ namespace Grpc.Core.Internal
         {
         }
 
-        public static ChannelSafeHandle Create(string target, ChannelArgsSafeHandle channelArgs)
+        public static ChannelSafeHandle CreateInsecure(string target, ChannelArgsSafeHandle channelArgs)
         {
-            return grpcsharp_channel_create(target, channelArgs);
+            return grpcsharp_insecure_channel_create(target, channelArgs);
         }
 
         public static ChannelSafeHandle CreateSecure(CredentialsSafeHandle credentials, string target, ChannelArgsSafeHandle channelArgs)

src/csharp/Grpc.Core/Internal/CredentialsSafeHandle.cs

@@ -50,6 +50,13 @@ namespace Grpc.Core.Internal
         {
         }
 
+        public static CredentialsSafeHandle CreateNullCredentials()
+        {
+            var creds = new CredentialsSafeHandle();
+            creds.SetHandle(IntPtr.Zero);
+            return creds;
+        }
+
         public static CredentialsSafeHandle CreateSslCredentials(string pemRootCerts, KeyCertificatePair keyCertPair)
         {
             if (keyCertPair != null)

src/csharp/Grpc.Core/Internal/ServerSafeHandle.cs

@@ -48,7 +48,7 @@ namespace Grpc.Core.Internal
         static extern ServerSafeHandle grpcsharp_server_create(CompletionQueueSafeHandle cq, ChannelArgsSafeHandle args);
 
         [DllImport("grpc_csharp_ext.dll")]
-        static extern int grpcsharp_server_add_http2_port(ServerSafeHandle server, string addr);
+        static extern int grpcsharp_server_add_insecure_http2_port(ServerSafeHandle server, string addr);
 
         [DllImport("grpc_csharp_ext.dll")]
         static extern int grpcsharp_server_add_secure_http2_port(ServerSafeHandle server, string addr, ServerCredentialsSafeHandle creds);
@@ -77,12 +77,12 @@ namespace Grpc.Core.Internal
             return grpcsharp_server_create(cq, args);
         }
 
-        public int AddListeningPort(string addr)
+        public int AddInsecurePort(string addr)
         {
-            return grpcsharp_server_add_http2_port(this, addr);
+            return grpcsharp_server_add_insecure_http2_port(this, addr);
         }
 
-        public int AddListeningPort(string addr, ServerCredentialsSafeHandle credentials)
+        public int AddSecurePort(string addr, ServerCredentialsSafeHandle credentials)
         {
             return grpcsharp_server_add_secure_http2_port(this, addr, credentials);
         }

src/csharp/Grpc.Core/Server.cs

@@ -98,28 +98,31 @@ namespace Grpc.Core
         }
 
         /// <summary>
-        /// Add a non-secure port on which server should listen.
+        /// Add a port on which server should listen.
         /// Only call this before Start().
         /// </summary>
         /// <returns>The port on which server will be listening.</returns>
         /// <param name="host">the host</param>
         /// <param name="port">the port. If zero, an unused port is chosen automatically.</param>
-        public int AddListeningPort(string host, int port)
+        public int AddPort(string host, int port, ServerCredentials credentials)
         {
-            return AddListeningPortInternal(host, port, null);
-        }
-
-        /// <summary>
-        /// Add a non-secure port on which server should listen.
-        /// Only call this before Start().
-        /// </summary>
-        /// <returns>The port on which server will be listening.</returns>
-        /// <param name="host">the host</param>
-        /// <param name="port">the port. If zero, an unused port is chosen automatically.</param>
-        public int AddListeningPort(string host, int port, ServerCredentials credentials)
-        {
-            Preconditions.CheckNotNull(credentials);
-            return AddListeningPortInternal(host, port, credentials);
+            lock (myLock)
+            {
+                Preconditions.CheckNotNull(credentials);
+                Preconditions.CheckState(!startRequested);
+                var address = string.Format("{0}:{1}", host, port);
+                using (var nativeCredentials = credentials.ToNativeCredentials())
+                {
+                    if (nativeCredentials != null)
+                    {
+                        return handle.AddSecurePort(address, nativeCredentials);
+                    }
+                    else
+                    {
+                        return handle.AddInsecurePort(address);
+                    }
+                }
+            }
         }
 
         /// <summary>
@@ -186,26 +189,6 @@ namespace Grpc.Core
             handle.Dispose();
         }
 
-        private int AddListeningPortInternal(string host, int port, ServerCredentials credentials)
-        {
-            lock (myLock)
-            {
-                Preconditions.CheckState(!startRequested);    
-                var address = string.Format("{0}:{1}", host, port);
-                if (credentials != null)
-                {
-                    using (var nativeCredentials = credentials.ToNativeCredentials())
-                    {
-                        return handle.AddListeningPort(address, nativeCredentials);
-                    }
-                }
-                else
-                {
-                    return handle.AddListeningPort(address);    
-                }
-            }
-        }
-
         /// <summary>
         /// Allows one new RPC call to be received by server.
         /// </summary>

src/csharp/Grpc.Core/ServerCredentials.cs

@@ -44,11 +44,33 @@ namespace Grpc.Core
     /// </summary>
     public abstract class ServerCredentials
     {
+        static readonly ServerCredentials InsecureInstance = new InsecureServerCredentialsImpl();
+
+        /// <summary>
+        /// Returns instance of credential that provides no security and 
+        /// will result in creating an unsecure server port with no encryption whatsoever.
+        /// </summary>
+        public static ServerCredentials Insecure
+        {
+            get
+            {
+                return InsecureInstance;
+            }
+        }
+
         /// <summary>
         /// Creates native object for the credentials.
         /// </summary>
         /// <returns>The native credentials.</returns>
         internal abstract ServerCredentialsSafeHandle ToNativeCredentials();
+
+        private sealed class InsecureServerCredentialsImpl : ServerCredentials
+        {
+            internal override ServerCredentialsSafeHandle ToNativeCredentials()
+            {
+                return null;
+            }
+        }
     }
 
     /// <summary>

src/csharp/Grpc.Examples.MathClient/MathClient.cs

@@ -39,7 +39,7 @@ namespace math
     {
         public static void Main(string[] args)
         {
-            using (Channel channel = new Channel("127.0.0.1", 23456))
+            using (Channel channel = new Channel("127.0.0.1", 23456, Credentials.Insecure))
             {
                 Math.IMathClient client = new Math.MathClient(channel);
                 MathExamples.DivExample(client);

src/csharp/Grpc.Examples.MathServer/MathServer.cs

@@ -44,7 +44,7 @@ namespace math
 
             Server server = new Server();
             server.AddServiceDefinition(Math.BindService(new MathServiceImpl()));
-            int port = server.AddListeningPort(host, 23456);
+            int port = server.AddPort(host, 23456, ServerCredentials.Insecure);
             server.Start();
 
             Console.WriteLine("MathServer listening on port " + port);

src/csharp/Grpc.Examples.Tests/MathClientServerTests.cs

@@ -56,9 +56,9 @@ namespace math.Tests
         {
             server = new Server();
             server.AddServiceDefinition(Math.BindService(new MathServiceImpl()));
-            int port = server.AddListeningPort(host, Server.PickUnusedPort);
+            int port = server.AddPort(host, Server.PickUnusedPort, ServerCredentials.Insecure);
             server.Start();
-            channel = new Channel(host, port);
+            channel = new Channel(host, port, Credentials.Insecure);
             client = Math.NewClient(channel);
 
             // TODO(jtattermusch): get rid of the custom header here once we have dedicated tests

src/csharp/Grpc.HealthCheck.Tests/HealthClientServerTest.cs

@@ -59,9 +59,9 @@ namespace Grpc.HealthCheck.Tests
 
             server = new Server();
             server.AddServiceDefinition(Grpc.Health.V1Alpha.Health.BindService(serviceImpl));
-            int port = server.AddListeningPort(Host, Server.PickUnusedPort);
+            int port = server.AddPort(Host, Server.PickUnusedPort, ServerCredentials.Insecure);
             server.Start();
-            channel = new Channel(Host, port);
+            channel = new Channel(Host, port, Credentials.Insecure);
 
             client = Grpc.Health.V1Alpha.Health.NewClient(channel);
         }

src/csharp/Grpc.IntegrationTesting/InteropClientServerTest.cs

@@ -57,7 +57,7 @@ namespace Grpc.IntegrationTesting
         {
             server = new Server();
             server.AddServiceDefinition(TestService.BindService(new TestServiceImpl()));
-            int port = server.AddListeningPort(host, Server.PickUnusedPort, TestCredentials.CreateTestServerCredentials());
+            int port = server.AddPort(host, Server.PickUnusedPort, TestCredentials.CreateTestServerCredentials());
             server.Start();
 
             var options = new List<ChannelOption>

src/csharp/Grpc.IntegrationTesting/InteropServer.cs

@@ -95,11 +95,11 @@ namespace Grpc.IntegrationTesting
             int port = options.port.Value;
             if (options.useTls)
             {
-                server.AddListeningPort(host, port, TestCredentials.CreateTestServerCredentials());
+                server.AddPort(host, port, TestCredentials.CreateTestServerCredentials());
             }
             else
             {
-                server.AddListeningPort(host, options.port.Value);
+                server.AddPort(host, options.port.Value, ServerCredentials.Insecure);
             }
             Console.WriteLine("Running server on " + string.Format("{0}:{1}", host, port));
             server.Start();

src/csharp/Grpc.IntegrationTesting/SslCredentialsTest.cs

@@ -67,7 +67,7 @@ namespace Grpc.IntegrationTesting
 
             server = new Server();
             server.AddServiceDefinition(TestService.BindService(new TestServiceImpl()));
-            int port = server.AddListeningPort(host, Server.PickUnusedPort, serverCredentials);
+            int port = server.AddPort(host, Server.PickUnusedPort, serverCredentials);
             server.Start();
 
             var options = new List<ChannelOption>

src/csharp/ext/grpc_csharp_ext.c

@@ -367,7 +367,7 @@ grpcsharp_completion_queue_pluck(grpc_completion_queue *cq, void *tag) {
 /* Channel */
 
 GPR_EXPORT grpc_channel *GPR_CALLTYPE
-grpcsharp_channel_create(const char *target, const grpc_channel_args *args) {
+grpcsharp_insecure_channel_create(const char *target, const grpc_channel_args *args) {
   return grpc_channel_create(target, args);
 }
 
@@ -717,7 +717,7 @@ grpcsharp_server_create(grpc_completion_queue *cq,
 }
 
 GPR_EXPORT gpr_int32 GPR_CALLTYPE
-grpcsharp_server_add_http2_port(grpc_server *server, const char *addr) {
+grpcsharp_server_add_insecure_http2_port(grpc_server *server, const char *addr) {
   return grpc_server_add_http2_port(server, addr);
 }