diff --git a/src/csharp/Grpc.Core.Api/ChannelCredentials.cs b/src/csharp/Grpc.Core.Api/ChannelCredentials.cs index 4d6ede3b3f0..d62ca5c7f0f 100644 --- a/src/csharp/Grpc.Core.Api/ChannelCredentials.cs +++ b/src/csharp/Grpc.Core.Api/ChannelCredentials.cs @@ -89,6 +89,11 @@ namespace Grpc.Core /// /// Returns true if this credential type allows being composed by CompositeCredentials. /// + /// + /// Note: No longer used. Decision on whether composition is allowed now happens in + /// . + /// Internal property left for safety because Grpc.Core has internal access to Grpc.Core.Api. + /// internal virtual bool IsComposable => false; private sealed class InsecureCredentials : ChannelCredentials @@ -118,11 +123,6 @@ namespace Grpc.Core { this.channelCredentials = GrpcPreconditions.CheckNotNull(channelCredentials); this.callCredentials = GrpcPreconditions.CheckNotNull(callCredentials); - - if (!channelCredentials.IsComposable) - { - throw new ArgumentException(string.Format("CallCredentials can't be composed with {0}. CallCredentials must be used with secure channel credentials like SslCredentials.", channelCredentials.GetType().Name)); - } } public override void InternalPopulateConfiguration(ChannelCredentialsConfiguratorBase configurator, object state) diff --git a/src/csharp/Grpc.Core.Tests/ChannelCredentialsTest.cs b/src/csharp/Grpc.Core.Tests/ChannelCredentialsTest.cs index f5d06ad1e03..c1c2cf08bef 100644 --- a/src/csharp/Grpc.Core.Tests/ChannelCredentialsTest.cs +++ b/src/csharp/Grpc.Core.Tests/ChannelCredentialsTest.cs @@ -24,18 +24,6 @@ namespace Grpc.Core.Tests { public class ChannelCredentialsTest { - [Test] - public void InsecureCredentials_IsNonComposable() - { - Assert.IsFalse(ChannelCredentials.Insecure.IsComposable); - } - - [Test] - public void SecureCredentials_IsComposable() - { - Assert.IsTrue(ChannelCredentials.SecureSsl.IsComposable); - } - [Test] public void ChannelCredentials_CreateComposite() { @@ -44,10 +32,6 @@ namespace Grpc.Core.Tests Assert.Throws(typeof(ArgumentNullException), () => ChannelCredentials.Create(null, new FakeCallCredentials())); Assert.Throws(typeof(ArgumentNullException), () => ChannelCredentials.Create(new FakeChannelCredentials(true), null)); - - // forbid composing non-composable - var ex = Assert.Throws(typeof(ArgumentException), () => ChannelCredentials.Create(new FakeChannelCredentials(false), new FakeCallCredentials())); - Assert.AreEqual("CallCredentials can't be composed with FakeChannelCredentials. CallCredentials must be used with secure channel credentials like SslCredentials.", ex.Message); } [Test] @@ -63,5 +47,16 @@ namespace Grpc.Core.Tests var nativeCreds4 = ChannelCredentials.SecureSsl.ToNativeCredentials(); Assert.AreSame(nativeCreds3, nativeCreds4); } + + [Test] + public void ChannelCredentials_MalformedSslCredentialsCanStillCreateNativeCredentials() + { + // pass malformed root pem certs, but creation of native credentials still passes, + // since the credentials are parsed lazily by the C core. + using (var nativeCreds = new SslCredentials("MALFORMED_ROOT_CERTS_THAT_WILL_THROW_WHEN_CREATING_NATIVE_CREDENTIALS").ToNativeCredentials()) + { + Assert.IsFalse(nativeCreds.IsInvalid); + } + } } } diff --git a/src/csharp/Grpc.Core.Tests/ChannelTest.cs b/src/csharp/Grpc.Core.Tests/ChannelTest.cs index 9f9dedaa5ae..f1d507d306b 100644 --- a/src/csharp/Grpc.Core.Tests/ChannelTest.cs +++ b/src/csharp/Grpc.Core.Tests/ChannelTest.cs @@ -123,5 +123,16 @@ namespace Grpc.Core.Tests // check that Channel.ShutdownAsync has run Assert.AreEqual(ChannelState.Shutdown, channel.State); } + + [Test] + public void CompositeCredentialsWithInsecureThrow() + { + var compositeCredentials = ChannelCredentials.Create( + ChannelCredentials.Insecure, + CallCredentials.FromInterceptor((context, metadata) => TaskUtils.CompletedTask)); + var ex = Assert.Throws(typeof(InvalidOperationException), () => new Channel("localhost", compositeCredentials)); + Assert.AreEqual("CallCredentials can't be composed with InsecureCredentials. " + + "CallCredentials must be used with secure channel credentials like SslCredentials.", ex.Message); + } } } diff --git a/src/csharp/Grpc.Core/Channel.cs b/src/csharp/Grpc.Core/Channel.cs index e2d69e408d8..75944a22ae2 100644 --- a/src/csharp/Grpc.Core/Channel.cs +++ b/src/csharp/Grpc.Core/Channel.cs @@ -69,20 +69,29 @@ namespace Grpc.Core EnsureUserAgentChannelOption(this.options); this.environment = GrpcEnvironment.AddRef(); - this.completionQueue = this.environment.PickCompletionQueue(); - using (var nativeChannelArgs = ChannelOptions.CreateChannelArgs(this.options.Values)) + try { - var nativeCredentials = credentials.ToNativeCredentials(); - if (nativeCredentials != null) + this.completionQueue = this.environment.PickCompletionQueue(); + using (var nativeChannelArgs = ChannelOptions.CreateChannelArgs(this.options.Values)) { - this.handle = ChannelSafeHandle.CreateSecure(nativeCredentials, target, nativeChannelArgs); - } - else - { - this.handle = ChannelSafeHandle.CreateInsecure(target, nativeChannelArgs); + var nativeCredentials = credentials.ToNativeCredentials(); + if (nativeCredentials != null) + { + this.handle = ChannelSafeHandle.CreateSecure(nativeCredentials, target, nativeChannelArgs); + } + else + { + this.handle = ChannelSafeHandle.CreateInsecure(target, nativeChannelArgs); + } } + GrpcEnvironment.RegisterChannel(this); + } + catch (Exception) + { + // Constructor can't be async. + _ = Task.Run(async () => await GrpcEnvironment.ReleaseAsync().ConfigureAwait(false)); + throw; } - GrpcEnvironment.RegisterChannel(this); } /// diff --git a/src/csharp/Grpc.Core/Internal/DefaultChannelCredentialsConfigurator.cs b/src/csharp/Grpc.Core/Internal/DefaultChannelCredentialsConfigurator.cs index c4c044cd577..f6b8bdd6200 100644 --- a/src/csharp/Grpc.Core/Internal/DefaultChannelCredentialsConfigurator.cs +++ b/src/csharp/Grpc.Core/Internal/DefaultChannelCredentialsConfigurator.cs @@ -82,7 +82,15 @@ namespace Grpc.Core.Internal { using (var callCreds = callCredentials.ToNativeCredentials()) { - var nativeComposite = ChannelCredentialsSafeHandle.CreateComposite(channelCredentials.ToNativeCredentials(), callCreds); + var nativeChannelCredentials = channelCredentials.ToNativeCredentials(); + // nativeChannelCredentials == null means insecure channel credentials were used. + if (nativeChannelCredentials == null) + { + throw new InvalidOperationException($"CallCredentials can't be composed with {channelCredentials.GetType().Name}. " + + $"CallCredentials must be used with secure channel credentials like SslCredentials."); + } + + var nativeComposite = ChannelCredentialsSafeHandle.CreateComposite(nativeChannelCredentials, callCreds); if (nativeComposite.IsInvalid) { throw new ArgumentException("Error creating native composite credentials. Likely, this is because you are trying to compose incompatible credentials.");