Make exception serialization settings used by Remoting secure by def…

…ault (#398)

- Deprecate the `ExceptionDeserializationTechnique` property of the
`FabricTransportRemotingSettings` and change its default value from
`Fallback` to `Default`.
- Deprecate the `ExceptionSerializationTechnique` property of the
`FabricTransportListenerSettings` and change its default value from
`BinaryFormatter` to `Default`.

properties/service_fabric_common.props

@@ -24,7 +24,7 @@
     <!-- TODO: Versions numbers are changed here manually for now, Integrate this with GitVersion. -->
     <MajorVersion>8</MajorVersion>
     <MinorVersion>0</MinorVersion>
-    <BuildVersion>12</BuildVersion>
+    <BuildVersion>13</BuildVersion>
     <Revision>0</Revision>
 
   </PropertyGroup>

src/Microsoft.ServiceFabric.Services.Remoting/FabricTransport/FabricTransportRemotingSettings.cs

@@ -35,7 +35,6 @@ public FabricTransportRemotingSettings()
             this.headerBufferSize = Constants.DefaultHeaderBufferSize;
             this.headerMaxBufferCount = Constants.DefaultHeaderMaxBufferCount;
             this.useWrappedMessage = false;
-            this.ExceptionDeserializationTechnique = ExceptionDeserialization.Fallback;
         }
 
         internal FabricTransportRemotingSettings(FabricTransportSettings fabricTransportSettings)
@@ -47,6 +46,7 @@ internal FabricTransportRemotingSettings(FabricTransportSettings fabricTransport
         /// <summary>
         /// Exception Deserialization option to use(applies to V2 Remoting only).
         /// </summary>
+        [Obsolete(DeprecationMessage.RemotingV1)]
         public enum ExceptionDeserialization
         {
             /// <summary>
@@ -64,6 +64,7 @@ public enum ExceptionDeserialization
         /// <summary>
         /// Gets or sets the exception deserialization techinique to use.
         /// </summary>
+        [Obsolete(DeprecationMessage.RemotingV1)]
         public ExceptionDeserialization ExceptionDeserializationTechnique { get; set; }
 
         /// <summary>
@@ -280,6 +281,7 @@ internal static FabricTransportRemotingSettings GetDefault(string sectionName =
             transportSettings = FabricTransportSettings.GetDefault(sectionName);
             var settings = new FabricTransportRemotingSettings(transportSettings);
 
+#pragma warning disable 618
             AppTrace.TraceSource.WriteInfo(
               Tracetype,
               "MaxMessageSize: {0}, MaxConcurrentCalls: {1}, MaxQueueSize: {2}, OperationTimeoutInSeconds: {3}, KeepAliveTimeoutInSeconds : {4}, SecurityCredentials {5}, HeaderBufferSize {6}, HeaderBufferCount {7}, ExceptionSerializationTechinique {8}",
@@ -293,6 +295,7 @@ internal static FabricTransportRemotingSettings GetDefault(string sectionName =
               settings.HeaderMaxBufferCount,
               settings.ExceptionDeserializationTechnique);
             return settings;
+#pragma warning restore 618
         }
 
         internal FabricTransportSettings GetInternalSettings()

src/Microsoft.ServiceFabric.Services.Remoting/FabricTransport/Runtime/FabricTransportRemotingListenerSettings.cs

@@ -36,7 +36,6 @@ public FabricTransportRemotingListenerSettings()
             this.headerMaxBufferCount = Constants.DefaultHeaderMaxBufferCount;
             this.useWrappedMessage = false;
             this.remotingExceptionDepth = DefaultRemotingExceptionDepth;
-            this.ExceptionSerializationTechnique = ExceptionSerialization.BinaryFormatter;
         }
 
         private FabricTransportRemotingListenerSettings(FabricTransportListenerSettings listenerSettings)
@@ -48,6 +47,7 @@ private FabricTransportRemotingListenerSettings(FabricTransportListenerSettings
         /// <summary>
         /// Exception serialization option to use(applicable only to V2 Remoting).
         /// </summary>
+        [Obsolete(DeprecationMessage.RemotingV1)]
         public enum ExceptionSerialization
         {
             /// <summary>
@@ -57,7 +57,7 @@ public enum ExceptionSerialization
 
             /// <summary>
             /// Uses binary formatter to serialize exception details in service remoting message.
-            /// To be used in compat scenarios. This option will be deprecated in future.
+            /// To be used in compat scenarios.
             /// </summary>
             BinaryFormatter,
         }
@@ -66,6 +66,7 @@ public enum ExceptionSerialization
         /// Gets or sets the exception serialization technique.
         /// </summary>
         /// <remarks>Applies only to V2 Remoting.</remarks>
+        [Obsolete(DeprecationMessage.RemotingV1)]
         public ExceptionSerialization ExceptionSerializationTechnique { get; set; }
 
         /// <summary>
@@ -298,6 +299,7 @@ internal static FabricTransportRemotingListenerSettings GetDefault(
 
             var settings = new FabricTransportRemotingListenerSettings(listenerinternalSettings);
 
+#pragma warning disable 618
             AppTrace.TraceSource.WriteInfo(
                 Tracetype,
                 "MaxMessageSize: {0} , MaxConcurrentCalls: {1} , MaxQueueSize: {2} , OperationTimeoutInSeconds: {3} KeepAliveTimeoutInSeconds : {4} , SecurityCredentials {5} , HeaderBufferSize {6}," + "HeaderBufferCount {7} , ExceptionSerializationTechinique {8} , RemotingExceptionDepth {9}",
@@ -311,6 +313,7 @@ internal static FabricTransportRemotingListenerSettings GetDefault(
                 settings.HeaderMaxBufferCount,
                 settings.ExceptionSerializationTechnique.ToString(),
                 settings.RemotingExceptionDepth);
+#pragma warning restore 618
 
             return settings;
         }

src/Microsoft.ServiceFabric.Services.Remoting/RemotingClientVersion.cs

@@ -14,9 +14,9 @@ public enum RemotingClientVersion
     {
 #if !DotNetCoreClr
         /// <summary>
-        /// This is selected to create V1 Client. V1 is an old(soon to be deprecated) Remoting Stack.
+        /// This is selected to create V1 Client. V1 is a deprecated Remoting Stack.
         /// </summary>
-        [Obsolete("This is an old version of remoting stack and will be removed in future versions.")]
+        [Obsolete(DeprecationMessage.RemotingV1)]
         V1 = 1,
 
 #endif

src/Microsoft.ServiceFabric.Services.Remoting/RemotingListenerVersion.cs

@@ -3,10 +3,10 @@
 // Licensed under the MIT License (MIT). See License.txt in the repo root for license information.
 // ------------------------------------------------------------
 
+using System;
+
 namespace Microsoft.ServiceFabric.Services.Remoting
 {
-    using System;
-
     /// <summary>
     /// Determines the remoting stack for server/listener when using remoting provider attribuite to determine the remoting client.
     /// </summary>
@@ -15,9 +15,9 @@ public enum RemotingListenerVersion
     {
 #if !DotNetCoreClr
         /// <summary>
-        /// This is selected to create V1 Listener.V1 is an old (soon to be deprecated) Remoting Stack.
+        /// This is selected to create V1 Listener.V1 is a deprecated Remoting Stack.
         /// </summary>
-        [Obsolete("This is an old version of remoting stack and will be removed in future versions.")]
+        [Obsolete(DeprecationMessage.RemotingV1)]
         V1 = 1,
 
 #endif

src/Microsoft.ServiceFabric.Services.Remoting/V2/Client/ExceptionConversionHandler.cs

@@ -114,16 +114,18 @@ public RemoteException2 DeserializeRemoteException2(byte[] buffer)
             }
             catch (Exception e)
             {
-                if (this.remotingSettings.ExceptionDeserializationTechnique == FabricTransportRemotingSettings.ExceptionDeserialization.Default)
+#pragma warning disable 618
+                if (this.remotingSettings.ExceptionDeserializationTechnique == FabricTransportRemotingSettings.ExceptionDeserialization.Fallback)
                 {
-                    ServiceTrace.Source.WriteWarning(
+                    ServiceTrace.Source.WriteInfo(
                        TraceEventType,
                        "Failed to deserialize stream to RemoteException2: Reason - {0}",
                        e);
                 }
                 else
+#pragma warning restore 618
                 {
-                    ServiceTrace.Source.WriteInfo(
+                    ServiceTrace.Source.WriteWarning(
                        TraceEventType,
                        "Failed to deserialize stream to RemoteException2: Reason - {0}",
                        e);
@@ -163,6 +165,7 @@ public async Task DeserializeRemoteExceptionAndThrowAsync(Stream stream)
                         SR.ErrorDeserializationFailure,
                         dcsE.ToString()));
 
+#pragma warning disable 618
                 if (this.remotingSettings.ExceptionDeserializationTechnique == FabricTransportRemotingSettings.ExceptionDeserialization.Fallback)
                 {
                     using (var tSteam = new MemoryStream(buffer))
@@ -184,6 +187,7 @@ public async Task DeserializeRemoteExceptionAndThrowAsync(Stream stream)
                         }
                     }
                 }
+#pragma warning restore 618
             }
 
             var requestId = LogContext.GetRequestIdOrDefault();

src/Microsoft.ServiceFabric.Services.Remoting/V2/FabricTransport/Runtime/FabricTransportServiceRemotingListener.cs

@@ -140,10 +140,12 @@ internal FabricTransportServiceRemotingListener(
                 this.transportMessageHandler,
                 new FabricTransportRemotingConnectionHandler());
 
+#pragma warning disable 618
             ServiceTelemetry.FabricTransportServiceRemotingV2Event(
                 serviceContext,
                 !remotingSettings.SecurityCredentials.CredentialType.Equals(CredentialType.None),
                 remotingSettings.ExceptionSerializationTechnique.ToString());
+#pragma warning restore 618
         }
 
         /// <summary>

src/Microsoft.ServiceFabric.Services.Remoting/V2/Runtime/ExceptionConversionHandler.cs

@@ -124,17 +124,14 @@ public List<ArraySegment<byte>> SerializeRemoteException(RemoteException2 remote
 
         public List<ArraySegment<byte>> SerializeRemoteException(Exception exception)
         {
-            if (this.listenerSettings.ExceptionSerializationTechnique == FabricTransportRemotingListenerSettings.ExceptionSerialization.Default)
-            {
-                var svcEx = this.ToServiceException(exception);
-                var remoteEx = this.ToRemoteException(svcEx);
-
-                return this.SerializeRemoteException(remoteEx);
-            }
-            else
-            {
+#pragma warning disable 618
+            if (this.listenerSettings.ExceptionSerializationTechnique == FabricTransportRemotingListenerSettings.ExceptionSerialization.BinaryFormatter)
                 return RemoteException.FromException(exception).Data;
-            }
+#pragma warning restore 618
+
+            ServiceException svcEx = this.ToServiceException(exception);
+            RemoteException2 remoteEx = this.ToRemoteException(svcEx);
+            return this.SerializeRemoteException(remoteEx);
         }
 
         public class DefaultExceptionConvertor : IExceptionConvertor

test/unittests/Microsoft.ServiceFabric.Actors.Tests/ExceptionConvertors/FabricActorExceptionTest.cs

@@ -28,11 +28,8 @@ private static List<Services.Remoting.V2.Runtime.IExceptionConvertor> runtimeCon
             };
 
         private static Services.Remoting.V2.Runtime.ExceptionConversionHandler runtimeHandler
-            = new Services.Remoting.V2.Runtime.ExceptionConversionHandler(runtimeConvertors, new FabricTransportRemotingListenerSettings()
-            {
-                RemotingExceptionDepth = 2,
-                ExceptionSerializationTechnique = FabricTransportRemotingListenerSettings.ExceptionSerialization.Default,
-            });
+            = new Services.Remoting.V2.Runtime.ExceptionConversionHandler(runtimeConvertors, 
+                new FabricTransportRemotingListenerSettings { RemotingExceptionDepth = 2 });
 
         private static List<Services.Remoting.V2.Client.IExceptionConvertor> clientConvertors
             = new List<Services.Remoting.V2.Client.IExceptionConvertor>()
@@ -43,10 +40,7 @@ private static List<Services.Remoting.V2.Client.IExceptionConvertor> clientConve
         private static Services.Remoting.V2.Client.ExceptionConversionHandler clientHandler
             = new Services.Remoting.V2.Client.ExceptionConversionHandler(
                 clientConvertors,
-                new FabricTransportRemotingSettings()
-                {
-                    ExceptionDeserializationTechnique = FabricTransportRemotingSettings.ExceptionDeserialization.Default,
-                });
+                new FabricTransportRemotingSettings());
 
         private static List<FabricException> fabricExceptions = new List<FabricException>()
         {

test/unittests/Microsoft.ServiceFabric.Services.Remoting.Tests/V2/ExceptionConvertors/CompatScenarioTest.cs

@@ -17,6 +17,7 @@ namespace Microsoft.ServiceFabric.Services.Remoting.Tests.V2.ExceptionConvertors
     /// <summary>
     /// Compat scenario tests.
     /// </summary>
+    [Obsolete(DeprecationMessage.RemotingV1)]
     public class CompatScenarioTest
     {
         private static List<Remoting.V2.Runtime.IExceptionConvertor> runtimeConvertors
@@ -28,7 +29,8 @@ private static List<Remoting.V2.Runtime.IExceptionConvertor> runtimeConvertors
            };
 
         private static Remoting.V2.Runtime.ExceptionConversionHandler runtimeHandler
-            = new Remoting.V2.Runtime.ExceptionConversionHandler(runtimeConvertors, FabricTransportRemotingListenerSettings.GetDefault());
+            = new Remoting.V2.Runtime.ExceptionConversionHandler(runtimeConvertors,
+                new FabricTransportRemotingListenerSettings { ExceptionSerializationTechnique = FabricTransportRemotingListenerSettings.ExceptionSerialization.BinaryFormatter });
 
         private static List<Remoting.V2.Client.IExceptionConvertor> clientConvertors
             = new List<Remoting.V2.Client.IExceptionConvertor>()
@@ -40,7 +42,7 @@ private static List<Remoting.V2.Client.IExceptionConvertor> clientConvertors
         private static Remoting.V2.Client.ExceptionConversionHandler clientHandler
             = new Remoting.V2.Client.ExceptionConversionHandler(
                 clientConvertors,
-                FabricTransportRemotingSettings.GetDefault());
+                new FabricTransportRemotingSettings { ExceptionDeserializationTechnique = FabricTransportRemotingSettings.ExceptionDeserialization.Fallback });
 
         /// <summary>
         /// Old client and new service test.

test/unittests/Microsoft.ServiceFabric.Services.Remoting.Tests/V2/ExceptionConvertors/CustomConvertorTest.cs

@@ -24,22 +24,15 @@ private static Remoting.V2.Runtime.ExceptionConversionHandler runtimeHandler
                 {
                     new CustomConvertorRuntime(),
                 },
-                new FabricTransportRemotingListenerSettings()
-                {
-                    RemotingExceptionDepth = 3,
-                    ExceptionSerializationTechnique = FabricTransportRemotingListenerSettings.ExceptionSerialization.Default,
-                });
+                new FabricTransportRemotingListenerSettings { RemotingExceptionDepth = 3 });
 
         private static Remoting.V2.Client.ExceptionConversionHandler clientHandler
             = new Remoting.V2.Client.ExceptionConversionHandler(
                 new List<Remoting.V2.Client.IExceptionConvertor>()
                 {
                     new CustomConvertorClient(),
                 },
-                new FabricTransport.FabricTransportRemotingSettings()
-                {
-                    ExceptionDeserializationTechnique = FabricTransport.FabricTransportRemotingSettings.ExceptionDeserialization.Default,
-                });
+                new FabricTransport.FabricTransportRemotingSettings());
 
         /// <summary>
         /// Custom types test.

test/unittests/Microsoft.ServiceFabric.Services.Remoting.Tests/V2/ExceptionConvertors/FabricExceptionConvertorTest.cs

@@ -28,11 +28,8 @@ private static List<Remoting.V2.Runtime.IExceptionConvertor> runtimeConvertors
             };
 
         private static Remoting.V2.Runtime.ExceptionConversionHandler runtimeHandler
-            = new Remoting.V2.Runtime.ExceptionConversionHandler(runtimeConvertors, new FabricTransportRemotingListenerSettings()
-            {
-                RemotingExceptionDepth = 2,
-                ExceptionSerializationTechnique = FabricTransportRemotingListenerSettings.ExceptionSerialization.Default,
-            });
+            = new Remoting.V2.Runtime.ExceptionConversionHandler(runtimeConvertors, 
+                new FabricTransportRemotingListenerSettings { RemotingExceptionDepth = 2 });
 
         private static List<Remoting.V2.Client.IExceptionConvertor> clientConvertors
             = new List<Remoting.V2.Client.IExceptionConvertor>()
@@ -44,10 +41,7 @@ private static List<Remoting.V2.Client.IExceptionConvertor> clientConvertors
         private static Remoting.V2.Client.ExceptionConversionHandler clientHandler
             = new Remoting.V2.Client.ExceptionConversionHandler(
                 clientConvertors,
-                new FabricTransport.FabricTransportRemotingSettings()
-                {
-                    ExceptionDeserializationTechnique = FabricTransport.FabricTransportRemotingSettings.ExceptionDeserialization.Default,
-                });
+                new FabricTransport.FabricTransportRemotingSettings());
 
         private static List<FabricException> fabricExceptions = new List<FabricException>()
         {

test/unittests/Microsoft.ServiceFabric.Services.Remoting.Tests/V2/ExceptionConvertors/SystemExceptionConvertorTest.cs

@@ -35,11 +35,8 @@ private static List<Remoting.V2.Runtime.IExceptionConvertor> runtimeConvertors
            };
 
         private static Remoting.V2.Runtime.ExceptionConversionHandler runtimeHandler
-            = new Remoting.V2.Runtime.ExceptionConversionHandler(runtimeConvertors, new FabricTransportRemotingListenerSettings()
-            {
-                RemotingExceptionDepth = 3,
-                ExceptionSerializationTechnique = FabricTransportRemotingListenerSettings.ExceptionSerialization.Default,
-            });
+            = new Remoting.V2.Runtime.ExceptionConversionHandler(runtimeConvertors, 
+                new FabricTransportRemotingListenerSettings { RemotingExceptionDepth = 3 });
 
         private static List<Remoting.V2.Client.IExceptionConvertor> clientConvertors
             = new List<Remoting.V2.Client.IExceptionConvertor>()
@@ -51,10 +48,7 @@ private static List<Remoting.V2.Client.IExceptionConvertor> clientConvertors
         private static Remoting.V2.Client.ExceptionConversionHandler clientHandler
             = new Remoting.V2.Client.ExceptionConversionHandler(
                 clientConvertors,
-                new FabricTransport.FabricTransportRemotingSettings()
-                {
-                    ExceptionDeserializationTechnique = FabricTransport.FabricTransportRemotingSettings.ExceptionDeserialization.Default,
-                });
+                new FabricTransport.FabricTransportRemotingSettings());
 
         private static List<SystemException> systemExceptions = new List<SystemException>()
         {