Remove max length restrictions on secret keys

dougbu · dougbu · commit 30897e047f63 · 2018-04-13T09:49:40.000-07:00
- #245 nits: - let VS remove BOMs
diff --git a/src/Microsoft.AspNetCore.WebHooks.Receivers.Dropbox/DropboxConstants.cs b/src/Microsoft.AspNetCore.WebHooks.Receivers.Dropbox/DropboxConstants.cs
@@ -1,4 +1,4 @@
-﻿// Copyright (c) .NET Foundation. All rights reserved.
+// Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 namespace Microsoft.AspNetCore.WebHooks
@@ -30,12 +30,6 @@ public static class DropboxConstants
         /// </summary>
         public static int SecretKeyMinLength => 15;
 
-        /// <summary>
-        /// Gets the maximum length of the secret key configured for this receiver. Used to confirm the secret key is
-        /// property configured before responding to an HTTP GET request.
-        /// </summary>
-        public static int SecretKeyMaxLength => 128;
-
         /// <summary>
         /// Gets the name of the HTTP header that contains the (hex-encoded) signature of the request.
         /// </summary>
diff --git a/src/Microsoft.AspNetCore.WebHooks.Receivers.Dropbox/Filters/DropboxVerifySignatureFilter.cs b/src/Microsoft.AspNetCore.WebHooks.Receivers.Dropbox/Filters/DropboxVerifySignatureFilter.cs
@@ -82,11 +82,7 @@ public async Task OnResourceExecutionAsync(ResourceExecutingContext context, Res
                 }
 
                 // 3. Get the configured secret key.
-                var secretKey = GetSecretKey(
-                    ReceiverName,
-                    context.RouteData,
-                    DropboxConstants.SecretKeyMinLength,
-                    DropboxConstants.SecretKeyMaxLength);
+                var secretKey = GetSecretKey(ReceiverName, context.RouteData, DropboxConstants.SecretKeyMinLength);
                 if (secretKey == null)
                 {
                     context.Result = new NotFoundResult();
diff --git a/src/Microsoft.AspNetCore.WebHooks.Receivers.Dropbox/Metadata/DropboxMetadata.cs b/src/Microsoft.AspNetCore.WebHooks.Receivers.Dropbox/Metadata/DropboxMetadata.cs
@@ -53,9 +53,6 @@ public DropboxMetadata(DropboxVerifySignatureFilter verifySignatureFilter)
         /// <inheritdoc />
         public int SecretKeyMinLength => DropboxConstants.SecretKeyMinLength;
 
-        /// <inheritdoc />
-        public int SecretKeyMaxLength => DropboxConstants.SecretKeyMaxLength;
-
         // IWebHookFilterMetadata...
 
         /// <inheritdoc />
diff --git a/src/Microsoft.AspNetCore.WebHooks.Receivers.GitHub/Filters/GitHubVerifySignatureFilter.cs b/src/Microsoft.AspNetCore.WebHooks.Receivers.GitHub/Filters/GitHubVerifySignatureFilter.cs
@@ -119,11 +119,7 @@ public async Task OnResourceExecutionAsync(ResourceExecutingContext context, Res
                 }
 
                 // 3. Get the configured secret key.
-                var secretKey = GetSecretKey(
-                    ReceiverName,
-                    context.RouteData,
-                    GitHubConstants.SecretKeyMinLength,
-                    GitHubConstants.SecretKeyMaxLength);
+                var secretKey = GetSecretKey(ReceiverName, context.RouteData, GitHubConstants.SecretKeyMinLength);
                 if (secretKey == null)
                 {
                     context.Result = new NotFoundResult();
diff --git a/src/Microsoft.AspNetCore.WebHooks.Receivers.GitHub/GitHubConstants.cs b/src/Microsoft.AspNetCore.WebHooks.Receivers.GitHub/GitHubConstants.cs
@@ -1,4 +1,4 @@
-﻿// Copyright (c) .NET Foundation. All rights reserved.
+// Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 namespace Microsoft.AspNetCore.WebHooks
@@ -28,11 +28,6 @@ public static class GitHubConstants
         /// </summary>
         public static int SecretKeyMinLength => 16;
 
-        /// <summary>
-        /// Gets the maximum length of the secret key configured for this receiver.
-        /// </summary>
-        public static int SecretKeyMaxLength => 128;
-
         /// <summary>
         /// Gets the key of the hex-encoded signature in the <see cref="SignatureHeaderName"/> value.
         /// </summary>
diff --git a/src/Microsoft.AspNetCore.WebHooks.Receivers.MailChimp/Metadata/MailChimpMetadata.cs b/src/Microsoft.AspNetCore.WebHooks.Receivers.MailChimp/Metadata/MailChimpMetadata.cs
@@ -44,8 +44,5 @@ public MailChimpMetadata()
 
         /// <inheritdoc />
         public int SecretKeyMinLength => WebHookConstants.CodeParameterMinLength;
-
-        /// <inheritdoc />
-        public int SecretKeyMaxLength => WebHookConstants.CodeParameterMaxLength;
     }
 }
diff --git a/src/Microsoft.AspNetCore.WebHooks.Receivers.Pusher/Filters/PusherVerifySignatureFilter.cs b/src/Microsoft.AspNetCore.WebHooks.Receivers.Pusher/Filters/PusherVerifySignatureFilter.cs
@@ -102,9 +102,7 @@ public async Task OnResourceExecutionAsync(ResourceExecutingContext context, Res
                 }
 
                 var secretKey = secretKeys[applicationKey];
-                if (secretKey == null ||
-                    secretKey.Length < PusherConstants.SecretKeyMinLength ||
-                    secretKey.Length > PusherConstants.SecretKeyMaxLength)
+                if (secretKey == null || secretKey.Length < PusherConstants.SecretKeyMinLength)
                 {
                     Logger.LogWarning(
                         0,
diff --git a/src/Microsoft.AspNetCore.WebHooks.Receivers.Pusher/PusherConstants.cs b/src/Microsoft.AspNetCore.WebHooks.Receivers.Pusher/PusherConstants.cs
@@ -1,4 +1,4 @@
-﻿// Copyright (c) .NET Foundation. All rights reserved.
+// Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 namespace Microsoft.AspNetCore.WebHooks
@@ -48,11 +48,6 @@ public static class PusherConstants
         /// </summary>
         public static int SecretKeyMinLength => 8;
 
-        /// <summary>
-        /// Gets the maximum length of the secret key configured for this receiver.
-        /// </summary>
-        public static int SecretKeyMaxLength => 128;
-
         /// <summary>
         /// Gets the name of the HTTP header containing the key into the configured lookup table. Named entry in the
         /// table contains the secret key the sender used when generating the <see cref="SignatureHeaderName"/> value.
diff --git a/src/Microsoft.AspNetCore.WebHooks.Receivers.Salesforce/Filters/SalesforceVerifyOrganizationIdFilter.cs b/src/Microsoft.AspNetCore.WebHooks.Receivers.Salesforce/Filters/SalesforceVerifyOrganizationIdFilter.cs
@@ -128,11 +128,7 @@ public async Task OnResourceExecutionAsync(ResourceExecutingContext context, Res
             }
 
             var routeData = context.RouteData;
-            var secret = GetSecretKey(
-                ReceiverName,
-                routeData,
-                SalesforceConstants.SecretKeyMinLength,
-                SalesforceConstants.SecretKeyMaxLength);
+            var secret = GetSecretKey(ReceiverName, routeData, SalesforceConstants.SecretKeyMinLength);
 
             var organizationId = GetShortOrganizationId(organizationIds[0]);
             var secretKey = GetShortOrganizationId(secret);
diff --git a/src/Microsoft.AspNetCore.WebHooks.Receivers.Salesforce/SalesforceConstants.cs b/src/Microsoft.AspNetCore.WebHooks.Receivers.Salesforce/SalesforceConstants.cs
@@ -1,4 +1,4 @@
-﻿// Copyright (c) .NET Foundation. All rights reserved.
+// Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 namespace Microsoft.AspNetCore.WebHooks
@@ -30,10 +30,5 @@ public static class SalesforceConstants
         /// Gets the minimum length of the secret key configured for this receiver.
         /// </summary>
         public static int SecretKeyMinLength => 15;
-
-        /// <summary>
-        /// Gets the maximum length of the secret key configured for this receiver.
-        /// </summary>
-        public static int SecretKeyMaxLength => 18;
     }
 }
diff --git a/src/Microsoft.AspNetCore.WebHooks.Receivers.Slack/Filters/SlackVerifyTokenFilter.cs b/src/Microsoft.AspNetCore.WebHooks.Receivers.Slack/Filters/SlackVerifyTokenFilter.cs
@@ -113,12 +113,7 @@ public async Task OnResourceExecutionAsync(ResourceExecutingContext context, Res
             }
 
             var routeData = context.RouteData;
-            var secretKey = GetSecretKey(
-                ReceiverName,
-                routeData,
-                SlackConstants.SecretKeyMinLength,
-                SlackConstants.SecretKeyMaxLength);
-
+            var secretKey = GetSecretKey(ReceiverName, routeData, SlackConstants.SecretKeyMinLength);
             if (!SecretEqual(token, secretKey))
             {
                 Logger.LogWarning(
diff --git a/src/Microsoft.AspNetCore.WebHooks.Receivers.Slack/SlackConstants.cs b/src/Microsoft.AspNetCore.WebHooks.Receivers.Slack/SlackConstants.cs
@@ -1,4 +1,4 @@
-﻿// Copyright (c) .NET Foundation. All rights reserved.
+// Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 namespace Microsoft.AspNetCore.WebHooks
@@ -29,11 +29,6 @@ public static class SlackConstants
         /// </summary>
         public static int SecretKeyMinLength => 16;
 
-        /// <summary>
-        /// Gets the maximum length of the secret key configured for this receiver.
-        /// </summary>
-        public static int SecretKeyMaxLength => 128;
-
         /// <summary>
         /// Gets the name of the <see cref="AspNetCore.Routing.RouteValueDictionary"/> entry containing the subtext
         /// value for the current request. This entry contains the portion of the <see cref="TextRequestFieldName"/>
diff --git a/src/Microsoft.AspNetCore.WebHooks.Receivers.Stripe/Filters/StripeVerifySignatureFilter.cs b/src/Microsoft.AspNetCore.WebHooks.Receivers.Stripe/Filters/StripeVerifySignatureFilter.cs
@@ -98,11 +98,7 @@ public async Task OnResourceExecutionAsync(ResourceExecutingContext context, Res
             var signatures = GetSignatures(header);
 
             // 4. Get the configured secret key.
-            var secretKey = GetSecretKey(
-                ReceiverName,
-                context.RouteData,
-                StripeConstants.SecretKeyMinLength,
-                StripeConstants.SecretKeyMaxLength);
+            var secretKey = GetSecretKey(ReceiverName, context.RouteData, StripeConstants.SecretKeyMinLength);
             if (secretKey == null)
             {
                 context.Result = new NotFoundResult();
diff --git a/src/Microsoft.AspNetCore.WebHooks.Receivers.Stripe/StripeConstants.cs b/src/Microsoft.AspNetCore.WebHooks.Receivers.Stripe/StripeConstants.cs
@@ -83,11 +83,6 @@ public static class StripeConstants
         /// </summary>
         public static int SecretKeyMinLength => 16;
 
-        /// <summary>
-        /// Gets the maximum length of the secret key configured for this receiver.
-        /// </summary>
-        public static int SecretKeyMaxLength => 128;
-
         /// <summary>
         /// Gets the name of the HTTP header that contains the timestamp and signature of the request. See
         /// <see href="https://stripe.com/docs/webhooks#replay-attacks"/> and
diff --git a/src/Microsoft.AspNetCore.WebHooks.Receivers.Trello/Filters/TrelloVerifySignatureFilter.cs b/src/Microsoft.AspNetCore.WebHooks.Receivers.Trello/Filters/TrelloVerifySignatureFilter.cs
@@ -83,11 +83,7 @@ public async Task OnResourceExecutionAsync(ResourceExecutingContext context, Res
                 }
 
                 // 3. Get the configured secret key.
-                var secretKey = GetSecretKey(
-                    ReceiverName,
-                    context.RouteData,
-                    TrelloConstants.SecretKeyMinLength,
-                    TrelloConstants.SecretKeyMaxLength);
+                var secretKey = GetSecretKey(ReceiverName, context.RouteData, TrelloConstants.SecretKeyMinLength);
                 if (secretKey == null)
                 {
                     context.Result = new NotFoundResult();
diff --git a/src/Microsoft.AspNetCore.WebHooks.Receivers.Trello/Metadata/TrelloMetadata.cs b/src/Microsoft.AspNetCore.WebHooks.Receivers.Trello/Metadata/TrelloMetadata.cs
@@ -53,9 +53,6 @@ public TrelloMetadata(TrelloVerifySignatureFilter verifySignatureFilter)
         /// <inheritdoc />
         public int SecretKeyMinLength => TrelloConstants.SecretKeyMinLength;
 
-        /// <inheritdoc />
-        public int SecretKeyMaxLength => TrelloConstants.SecretKeyMaxLength;
-
         // IWebHookFilterMetadata...
 
         /// <inheritdoc />
diff --git a/src/Microsoft.AspNetCore.WebHooks.Receivers.Trello/TrelloConstants.cs b/src/Microsoft.AspNetCore.WebHooks.Receivers.Trello/TrelloConstants.cs
@@ -1,4 +1,4 @@
-﻿// Copyright (c) .NET Foundation. All rights reserved.
+// Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 namespace Microsoft.AspNetCore.WebHooks
@@ -24,11 +24,6 @@ public static class TrelloConstants
         /// </summary>
         public static int SecretKeyMinLength => 32;
 
-        /// <summary>
-        /// Gets the maximum length of the secret key configured for this receiver.
-        /// </summary>
-        public static int SecretKeyMaxLength => 128;
-
         /// <summary>
         /// Gets the name of the HTTP header that contains the (hex-encoded) signature of the request.
         /// </summary>
diff --git a/src/Microsoft.AspNetCore.WebHooks.Receivers/Filters/WebHookGetHeadRequestFilter.cs b/src/Microsoft.AspNetCore.WebHooks.Receivers/Filters/WebHookGetHeadRequestFilter.cs
@@ -156,8 +156,7 @@ public void OnResourceExecuting(ResourceExecutingContext context)
                 var secretKey = GetSecretKey(
                     receiverName,
                     routeData,
-                    getHeadRequestMetadata.SecretKeyMinLength,
-                    getHeadRequestMetadata.SecretKeyMaxLength);
+                    getHeadRequestMetadata.SecretKeyMinLength);
                 if (secretKey == null)
                 {
                     // Have already logged about this case.
diff --git a/src/Microsoft.AspNetCore.WebHooks.Receivers/Filters/WebHookSecurityFilter.cs b/src/Microsoft.AspNetCore.WebHooks.Receivers/Filters/WebHookSecurityFilter.cs
@@ -182,15 +182,10 @@ protected virtual IActionResult EnsureSecureConnection(string receiverName, Http
         /// <see cref="WebHookSecurityFilter"/> subclass to support multiple senders with individual configurations.
         /// </param>
         /// <param name="minLength">The minimum length of the key value.</param>
-        /// <param name="maxLength">The maximum length of the key value.</param>
         /// <returns>
         /// The configured WebHook secret key. <see langword="null"/> if the configuration value does not exist.
         /// </returns>
-        protected virtual string GetSecretKey(
-            string sectionKey,
-            RouteData routeData,
-            int minLength,
-            int maxLength)
+        protected virtual string GetSecretKey(string sectionKey, RouteData routeData, int minLength)
         {
             if (sectionKey == null)
             {
@@ -220,7 +215,7 @@ protected virtual string GetSecretKey(
                 return null;
             }
 
-            if (secret.Length < minLength || secret.Length > maxLength)
+            if (secret.Length < minLength)
             {
                 // Secrete key found but it does not meet the length requirements.
                 routeData.TryGetWebHookReceiverId(out var id);
@@ -229,8 +224,7 @@ protected virtual string GetSecretKey(
                     Resources.Security_BadSecret,
                     sectionKey,
                     id,
-                    minLength,
-                    maxLength);
+                    minLength);
                 throw new InvalidOperationException(message);
             }
 
diff --git a/src/Microsoft.AspNetCore.WebHooks.Receivers/Filters/WebHookVerifyCodeFilter.cs b/src/Microsoft.AspNetCore.WebHooks.Receivers/Filters/WebHookVerifyCodeFilter.cs
@@ -187,11 +187,7 @@ protected virtual IActionResult EnsureValidCode(
                 return noCode;
             }
 
-            var secretKey = GetSecretKey(
-                receiverName,
-                routeData,
-                WebHookConstants.CodeParameterMinLength,
-                WebHookConstants.CodeParameterMaxLength);
+            var secretKey = GetSecretKey(receiverName, routeData, WebHookConstants.CodeParameterMinLength);
             if (secretKey == null)
             {
                 return new NotFoundResult();
diff --git a/src/Microsoft.AspNetCore.WebHooks.Receivers/Metadata/IWebHookGetHeadRequestMetadata.cs b/src/Microsoft.AspNetCore.WebHooks.Receivers/Metadata/IWebHookGetHeadRequestMetadata.cs
@@ -49,11 +49,5 @@ public interface IWebHookGetHeadRequestMetadata : IWebHookMetadata, IWebHookRece
         /// property configured before responding to an HTTP GET or HEAD request.
         /// </summary>
         int SecretKeyMinLength { get; }
-
-        /// <summary>
-        /// Gets the maximum length of the secret key configured for this receiver. Used to confirm the secret key is
-        /// property configured before responding to an HTTP GET or HEAD request.
-        /// </summary>
-        int SecretKeyMaxLength { get; }
     }
 }
diff --git a/src/Microsoft.AspNetCore.WebHooks.Receivers/Properties/Resources.Designer.cs b/src/Microsoft.AspNetCore.WebHooks.Receivers/Properties/Resources.Designer.cs
diff --git a/src/Microsoft.AspNetCore.WebHooks.Receivers/Properties/Resources.resx b/src/Microsoft.AspNetCore.WebHooks.Receivers/Properties/Resources.resx
@@ -154,7 +154,7 @@
     <value>'{0}' and '{1}' were applied to the same action. '{2}' must not be combined with another attribute that provides a route template.</value>
   </data>
   <data name="Security_BadSecret" xml:space="preserve">
-    <value>Could not find a valid configuration for the '{0}' WebHook receiver, instance '{1}'. The value must be between {2} and {3} characters long.</value>
+    <value>Could not find a valid configuration for the '{0}' WebHook receiver, instance '{1}'. The value must be at least {2} characters long.</value>
   </data>
   <data name="Security_NoHttps" xml:space="preserve">
     <value>The '{0}' WebHook receiver requires {1} in order to be secure. Register a WebHook URI of type '{2}'.</value>
diff --git a/src/Microsoft.AspNetCore.WebHooks.Receivers/WebHookConstants.cs b/src/Microsoft.AspNetCore.WebHooks.Receivers/WebHookConstants.cs
@@ -18,13 +18,6 @@ public static class WebHookConstants
         /// <seealso cref="Metadata.IWebHookVerifyCodeMetadata"/>
         public static int CodeParameterMinLength => 32;
 
-        /// <summary>
-        /// Gets the maximum length of the <see cref="CodeQueryParameterName"/> query parameter value and secret key
-        /// for receivers using such a query parameter.
-        /// </summary>
-        /// <seealso cref="Metadata.IWebHookVerifyCodeMetadata"/>
-        public static int CodeParameterMaxLength => 128;
-
         /// <summary>
         /// Gets the name of a query parameter containing the secret key used to verify some WebHook receivers'
         /// requests.

Original file line number	Diff line number	Diff line change
`@@ -82,11 +82,7 @@ public async Task OnResourceExecutionAsync(ResourceExecutingContext context, Res`
`82`	`82`	`}`
`83`	`83`
`84`	`84`	`// 3. Get the configured secret key.`
`85`		`- var secretKey = GetSecretKey(`
`86`		`- ReceiverName,`
`87`		`- context.RouteData,`
`88`		`- DropboxConstants.SecretKeyMinLength,`
`89`		`- DropboxConstants.SecretKeyMaxLength);`
	`85`	`+ var secretKey = GetSecretKey(ReceiverName, context.RouteData, DropboxConstants.SecretKeyMinLength);`
`90`	`86`	`if (secretKey == null)`
`91`	`87`	`{`
`92`	`88`	`context.Result = new NotFoundResult();`
Original file line number	Diff line number	Diff line change
`@@ -119,11 +119,7 @@ public async Task OnResourceExecutionAsync(ResourceExecutingContext context, Res`
`119`	`119`	`}`
`120`	`120`
`121`	`121`	`// 3. Get the configured secret key.`
`122`		`- var secretKey = GetSecretKey(`
`123`		`- ReceiverName,`
`124`		`- context.RouteData,`
`125`		`- GitHubConstants.SecretKeyMinLength,`
`126`		`- GitHubConstants.SecretKeyMaxLength);`
	`122`	`+ var secretKey = GetSecretKey(ReceiverName, context.RouteData, GitHubConstants.SecretKeyMinLength);`
`127`	`123`	`if (secretKey == null)`
`128`	`124`	`{`
`129`	`125`	`context.Result = new NotFoundResult();`
Original file line number	Diff line number	Diff line change
`@@ -44,8 +44,5 @@ public MailChimpMetadata()`
`44`	`44`
`45`	`45`	`/// <inheritdoc />`
`46`	`46`	`public int SecretKeyMinLength => WebHookConstants.CodeParameterMinLength;`
`47`		`-`
`48`		`- /// <inheritdoc />`
`49`		`- public int SecretKeyMaxLength => WebHookConstants.CodeParameterMaxLength;`
`50`	`47`	`}`
`51`	`48`	`}`
Original file line number	Diff line number	Diff line change
`@@ -102,9 +102,7 @@ public async Task OnResourceExecutionAsync(ResourceExecutingContext context, Res`
`102`	`102`	`}`
`103`	`103`
`104`	`104`	`var secretKey = secretKeys[applicationKey];`
`105`		`- if (secretKey == null \|\|`
`106`		`- secretKey.Length < PusherConstants.SecretKeyMinLength \|\|`
`107`		`- secretKey.Length > PusherConstants.SecretKeyMaxLength)`
	`105`	`+ if (secretKey == null \|\| secretKey.Length < PusherConstants.SecretKeyMinLength)`
`108`	`106`	`{`
`109`	`107`	`Logger.LogWarning(`
`110`	`108`	`0,`