diff --git a/src/Private/CanonicalRequest.cs b/src/Private/CanonicalRequest.cs index 913dab81..9f5df646 100644 --- a/src/Private/CanonicalRequest.cs +++ b/src/Private/CanonicalRequest.cs @@ -30,6 +30,24 @@ public static class CanonicalRequest /// public static string HeaderValueSeparator { get; set; } = ", "; + // Including most headers from + // https://github.com/smithy-lang/smithy-typescript/blob/430021abf44f8a4d6c24de2dfa25709bf91a92c8/packages/signature-v4/src/constants.ts#L19-L35 + private static HashSet UnsignableHeaders = new(StringComparer.OrdinalIgnoreCase) { + "authorization", + "connection", + "expect", + "keep-alive", + "proxy-authenticate", + "proxy-authorization", + "proxy-connection", + "range", + "te", + "trailer", + "transfer-encoding", + "upgrade", + HeaderKeys.XAmznTraceIdHeader, + }; + /// /// The first value is the canonical request, the second value is the signed headers. /// @@ -91,8 +109,10 @@ public static (string, string) Build( builder.Append($"{string.Join("&", parameters)}\n"); // Add the canonical headers, followed by a newline character. The canonical headers - // consist of a list of all the HTTP headers that you are including with the signed - // request. + // consist of a list of HTTP headers that you are including with the signed request. + // + // Some headers are unsignable, i.e. signatures including them are always deemed invalid. + // They are excluded from the canonical headers (but will be kept in the HTTP request). // // To create the canonical headers list, convert all header names to lowercase and // remove leading spaces and trailing spaces. Convert sequential spaces in the header @@ -108,7 +128,7 @@ public static (string, string) Build( // PLEASE NOTE: Microsoft has chosen to separate the header values with ", ", not "," // as defined by the Canonical Request algorithm. // - Append a new line ('\n'). - var sortedHeaders = SortHeaders(request.Headers, defaultHeaders); + var sortedHeaders = PruneAndSortHeaders(request.Headers, defaultHeaders); foreach (var header in sortedHeaders) { @@ -187,7 +207,7 @@ public static SortedList> SortQueryParameters(string query) return sortedQueryParameters; } - public static SortedDictionary> SortHeaders( + public static SortedDictionary> PruneAndSortHeaders( HttpRequestHeaders headers, IEnumerable>> defaultHeaders) { @@ -202,6 +222,11 @@ void AddHeader(KeyValuePair> header) { var headerName = FormatHeaderName(header.Key); + if (UnsignableHeaders.Contains(headerName)) + { + return; + } + // Create header if it doesn't already exist if (!sortedHeaders.TryGetValue(headerName, out var headerValues)) { diff --git a/test/Unit/Private/CanonicalRequestGivenDotnetShould.cs b/test/Unit/Private/CanonicalRequestGivenDotnetShould.cs index c192e428..09bb997d 100644 --- a/test/Unit/Private/CanonicalRequestGivenDotnetShould.cs +++ b/test/Unit/Private/CanonicalRequestGivenDotnetShould.cs @@ -33,7 +33,7 @@ public void RespectDefaultHeader() defaultHeaders.Add("some-header-name", "some-header-value"); // Act - var actual = CanonicalRequest.SortHeaders(headers, defaultHeaders); + var actual = CanonicalRequest.PruneAndSortHeaders(headers, defaultHeaders); // Assert actual["some-header-name"].ShouldBe(new[] { "some-header-value" }); @@ -50,7 +50,7 @@ public void IgnoreDuplicateDefaultHeader() defaultHeaders.Add("some-header-name", "some-ignored-header-value"); // Act - var actual = CanonicalRequest.SortHeaders(headers, defaultHeaders); + var actual = CanonicalRequest.PruneAndSortHeaders(headers, defaultHeaders); // Assert actual["some-header-name"].ShouldBe(new[] { "some-header-value" }); diff --git a/test/Unit/Private/CanonicalRequestGivenMonoShould.cs b/test/Unit/Private/CanonicalRequestGivenMonoShould.cs index 0fbb15ee..8ff3f882 100644 --- a/test/Unit/Private/CanonicalRequestGivenMonoShould.cs +++ b/test/Unit/Private/CanonicalRequestGivenMonoShould.cs @@ -33,7 +33,7 @@ public void RespectDefaultHeader() defaultHeaders.Add("some-header-name", "some-header-value"); // Act - var actual = CanonicalRequest.SortHeaders(headers, defaultHeaders); + var actual = CanonicalRequest.PruneAndSortHeaders(headers, defaultHeaders); // Assert actual["some-header-name"].ShouldBe(new[] { "some-header-value" }); @@ -50,7 +50,7 @@ public void RespectDuplicateDefaultHeader() defaultHeaders.Add("some-header-name", "some-other-header-value"); // Act - var actual = CanonicalRequest.SortHeaders(headers, defaultHeaders); + var actual = CanonicalRequest.PruneAndSortHeaders(headers, defaultHeaders); // Assert actual["some-header-name"].ShouldBe(new[] { "some-header-value", "some-other-header-value" }); diff --git a/test/Unit/Private/CanonicalRequestShould.cs b/test/Unit/Private/CanonicalRequestShould.cs index cf6912ba..5041b5e5 100644 --- a/test/Unit/Private/CanonicalRequestShould.cs +++ b/test/Unit/Private/CanonicalRequestShould.cs @@ -109,7 +109,7 @@ public void TransformHeaderNamesToLowercase(string headerName, string expected) headers.Add(headerName, "some header value"); // Act - var actual = CanonicalRequest.SortHeaders(headers, null); + var actual = CanonicalRequest.PruneAndSortHeaders(headers, null); // Assert actual.Keys.ShouldBe(new[] { expected }); @@ -133,7 +133,7 @@ public void SortHeaderNames(string[] headerNames, string[] expected) } // Act - var actual = CanonicalRequest.SortHeaders(headers, null); + var actual = CanonicalRequest.PruneAndSortHeaders(headers, null); // Assert actual.Keys.ShouldBe(expected); @@ -156,7 +156,7 @@ public void TrimHeaderValue(string headerValue, string expected) headers.Add("some-header-name", headerValue); // Act - var actual = CanonicalRequest.SortHeaders(headers, null); + var actual = CanonicalRequest.PruneAndSortHeaders(headers, null); // Assert actual["some-header-name"].ShouldBe(new[] { expected }); @@ -176,12 +176,32 @@ public void RemoveSequentialSpacesInHeaderValue(string headerValue, string expec headers.Add("some-header-name", headerValue); // Act - var actual = CanonicalRequest.SortHeaders(headers, null); + var actual = CanonicalRequest.PruneAndSortHeaders(headers, null); // Assert actual["some-header-name"].ShouldBe(new[] { expected }); } + [Theory] + [InlineData("Connection", "keep-alive")] + [InlineData("Expect", "100-continue")] + [InlineData("Keep-Alive", "timeout=5")] + [InlineData("Proxy-Authenticate", "Basic")] + [InlineData("Range", "bytes=500-999")] + [InlineData("TE", "gzip")] + public void RemoveUnsignableHeaders(string headerName, string headerValue) + { + // Arrange + var headers = new HttpRequestMessage().Headers; + headers.Add(headerName, headerValue); + + // Act + var actual = CanonicalRequest.PruneAndSortHeaders(headers, null); + + // Assert + actual.ShouldNotContainKey(headerName.ToLowerInvariant()); + } + [Theory] [InlineData("?a=1&b=2&c=3", new[] { "a", "b", "c" })] [InlineData("?a=1&c=3&b=2", new[] { "a", "b", "c" })]