Improve security of AutomaticRedirection options for HttpClientHandler/WinHttpHandler

As per the latest security review, this issue captures the following tasks in order to remove insecure automatic redirection of HTTPS -> HTTP in HttpClientHandler/WinHttpHandler. This will also match the current NETCore/NETNative behavior as well.

In general, the security guidance is to remove insecure patterns from new code/platforms. This helps the entire ecosystem become more secure while balancing the needs of app-compat.

1. Change the WinHttpHandler API surface to remove the AutomaticRedirectionPolicy enum 3-state value.

2. Change the name/type of the 'AutomaticRedirectionPolicy' property to 'bool AutomaticRedirection'. This matches the current Framework Design guidelines of fully spelling out words and the word pattern of the 'AutomaticDecompression' property.

3. The default for the AutomaticRedirection property will be true. This value means that all redirects will be followed automatically EXCEPT for https:// to http://. This behavior is different from .NET desktop where https:// to http:// redirects are also followed.

[tfs-changeset: 1498199]

Author: davidsh

src/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpHandler.cs

@@ -34,17 +34,6 @@ public enum WindowsProxyUsePolicy
         UseCustomProxy = 3 // Use the custom proxy specified in the Proxy property.
     }
 
-#if HTTP_DLL
-    internal enum AutomaticRedirectionPolicy
-#else
-    public enum AutomaticRedirectionPolicy
-#endif
-    {
-        Never = 0,
-        DisallowHttpsToHttp = 1,
-        Always = 2
-    }
-
 #if HTTP_DLL
     internal enum CookieUsePolicy
 #else
@@ -118,7 +107,7 @@ public class WinHttpHandler : HttpMessageHandler
         private object _lockObject = new object();
         private bool _doManualDecompressionCheck = false;
         private WinInetProxyHelper _proxyHelper = null;
-        private AutomaticRedirectionPolicy _automaticRedirectionPolicy = AutomaticRedirectionPolicy.DisallowHttpsToHttp;
+        private bool _automaticRedirection = true;
         private int _maxAutomaticRedirections = 50;
         private DecompressionMethods _automaticDecompression = DecompressionMethods.Deflate | DecompressionMethods.GZip;
         private CookieUsePolicy _cookieUsePolicy = CookieUsePolicy.UseInternalCookieStoreOnly;
@@ -162,24 +151,17 @@ public WinHttpHandler()
         }
 
         #region Properties
-        public AutomaticRedirectionPolicy AutomaticRedirectionPolicy
+        public bool AutomaticRedirection
         {
             get
             {
-                return _automaticRedirectionPolicy;
+                return _automaticRedirection;
             }
 
             set
             {
-                if (value != AutomaticRedirectionPolicy.Never
-                    && value != AutomaticRedirectionPolicy.DisallowHttpsToHttp
-                    && value != AutomaticRedirectionPolicy.Always)
-                {
-                    throw new ArgumentOutOfRangeException("value");
-                }
-
                 CheckDisposedOrStarted();
-                _automaticRedirectionPolicy = value;
+                _automaticRedirection = value;
             }
         }
 
@@ -1604,7 +1586,7 @@ private void SetRequestHandleRedirectionOptions(SafeWinHttpHandle requestHandle)
         {
             uint optionData = 0;
 
-            if (_automaticRedirectionPolicy != AutomaticRedirectionPolicy.Never)
+            if (_automaticRedirection)
             {
                 optionData = (uint)_maxAutomaticRedirections;
                 SetWinHttpOption(
@@ -1613,7 +1595,9 @@ private void SetRequestHandleRedirectionOptions(SafeWinHttpHandle requestHandle)
                     ref optionData);
             }
 
-            optionData = (uint)_automaticRedirectionPolicy;
+            optionData = _automaticRedirection ? 
+                Interop.WinHttp.WINHTTP_OPTION_REDIRECT_POLICY_DISALLOW_HTTPS_TO_HTTP :
+                Interop.WinHttp.WINHTTP_OPTION_REDIRECT_POLICY_NEVER;
             SetWinHttpOption(requestHandle, Interop.WinHttp.WINHTTP_OPTION_REDIRECT_POLICY, ref optionData);
         }
 

src/System.Net.Http.WinHttpHandler/tests/WinHttpHandlerTest.cs

@@ -34,49 +34,47 @@ public void Dispose()
         }
 
         [Fact]
-        public void AutomaticRedirectionPolicy_SetUsingInvalidEnum_ThrowsArgumentOutOfRangeException()
+        public void AutomaticRedirection_CtorAndGet_DefaultValueIsTrue()
         {
             var handler = new WinHttpHandler();
-
-            Assert.Throws<ArgumentOutOfRangeException>(
-                () => { handler.AutomaticRedirectionPolicy = (AutomaticRedirectionPolicy)100; });
+            
+            Assert.True(handler.AutomaticRedirection);
         }
 
         [Fact]
-        public void AutomaticRedirectionPolicy_SetAlways_NoExceptionThrown()
+        public void AutomaticRedirection_SetFalseAndGet_ValueIsFalse()
         {
             var handler = new WinHttpHandler();
-
-            handler.AutomaticRedirectionPolicy = AutomaticRedirectionPolicy.Always;
+            handler.AutomaticRedirection = false;
+            
+            Assert.False(handler.AutomaticRedirection);
         }
 
         [Fact]
-        public void AutomaticRedirectionPolicy_SetDisallowHttpsToHttp_NoExceptionThrown()
+        public void AutomaticRedirection_SetTrue_ExpectedWinHttpHandleSettings()
         {
             var handler = new WinHttpHandler();
 
-            handler.AutomaticRedirectionPolicy = AutomaticRedirectionPolicy.DisallowHttpsToHttp;
-        }
-
-        [Fact]
-        public void AutomaticRedirectionPolicy_SetNever_NoExceptionThrown()
-        {
-            var handler = new WinHttpHandler();
+            SendRequestHelper(
+                handler,
+                delegate { handler.AutomaticRedirection = true; });
 
-            handler.AutomaticRedirectionPolicy = AutomaticRedirectionPolicy.Never;
+            Assert.Equal(
+                Interop.WinHttp.WINHTTP_OPTION_REDIRECT_POLICY_DISALLOW_HTTPS_TO_HTTP,
+                APICallHistory.WinHttpOptionRedirectPolicy);
         }
 
         [Fact]
-        public void AutomaticRedirectionPolicy_SetDisallowHttpsToHttp_ExpectedWinHttpHandleSettings()
+        public void AutomaticRedirection_SetFalse_ExpectedWinHttpHandleSettings()
         {
             var handler = new WinHttpHandler();
 
             SendRequestHelper(
                 handler,
-                delegate { handler.AutomaticRedirectionPolicy = AutomaticRedirectionPolicy.DisallowHttpsToHttp; });
+                delegate { handler.AutomaticRedirection = false; });
 
             Assert.Equal(
-                Interop.WinHttp.WINHTTP_OPTION_REDIRECT_POLICY_DISALLOW_HTTPS_TO_HTTP,
+                Interop.WinHttp.WINHTTP_OPTION_REDIRECT_POLICY_NEVER,
                 APICallHistory.WinHttpOptionRedirectPolicy);
         }
 

src/System.Net.Http/src/System/Net/Http/HttpClientHandler.Windows.cs

@@ -103,15 +103,11 @@ public bool AllowAutoRedirect
         {
             get
             {
-                return (_winHttpHandler.AutomaticRedirectionPolicy != AutomaticRedirectionPolicy.Never);
+                return _winHttpHandler.AutomaticRedirection;
             }
             set
             {
-                // The existing .NET Desktop HttpClientHandler based on the HWR stack allows HTTPS -> HTTP redirection by default.
-                // But, we're changing behavior to be more secure for ProjectK.
-                _winHttpHandler.AutomaticRedirectionPolicy = value ? 
-                    AutomaticRedirectionPolicy.DisallowHttpsToHttp : 
-                    AutomaticRedirectionPolicy.Never;
+                _winHttpHandler.AutomaticRedirection = value;
             }
         }