Skip to content
This repository was archived by the owner on Jun 10, 2020. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
namespace Microsoft.ApplicationInsights.AspNetCore.Common
{
/// <summary>
/// These values are listed to guard against malicious injections by limiting the max size allowed in an HTTP Response.
/// These max limits are intentionally exaggerated to allow for unexpected responses, while still guarding against unreasonably large responses.
/// Example: While a 32 character response may be expected, 50 characters may be permitted while a 10,000 character response would be unreasonable and malicious.
/// </summary>
public static class InjectionGuardConstants
{
/// <summary>
/// Max length of AppId allowed in response from Breeze.
/// </summary>
public const int AppIdMaxLengeth = 50;

/// <summary>
/// Max length of incoming Request Header value allowed.
/// </summary>
public const int RequestHeaderMaxLength = 100;

/// <summary>
/// Max length of context header key.
/// </summary>
public const int ContextHeaderKeyMaxLength = 50;
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i picked 50. Not sure if there is genuine need of key longer than this.


/// <summary>
/// Max length of context header value.
/// </summary>
public const int ContextHeaderValueMaxLength = 100;
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i picked 100. Not sure if there is genuine need of a value longer than this.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SergeyKanzhelev's guidance for WebSDK was to allow up to 1Kb. I made the same change above to RequestHeaderMaxLengeth.
Consider if that is appropriate for here as well.

}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
namespace Microsoft.ApplicationInsights.AspNetCore.Common
{
using System.Diagnostics;

/// <summary>
/// Generic functions to perform common operations on a string.
/// </summary>
public static class StringUtilities
{
/// <summary>
/// Check a strings length and trim to a max length if needed.
/// </summary>
public static string EnforceMaxLength(string input, int maxLength)
{
Debug.Assert(input != null, $"{nameof(input)} must not be null");
Debug.Assert(maxLength > 0, $"{nameof(maxLength)} must be greater than 0");

if (input != null && input.Length > maxLength)
{
input = input.Substring(0, maxLength);
}

return input;
}
}
}

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
namespace Microsoft.ApplicationInsights.AspNetCore.DiagnosticListeners
{
using Microsoft.ApplicationInsights.AspNetCore.Common;
using System.Collections.Generic;
using System.Globalization;
using System.Linq;
using System.Text.RegularExpressions;

/// <summary>
/// Generic functions that can be used to get and set Http headers.
Expand All @@ -24,7 +26,8 @@ public static string GetHeaderKeyValue(IEnumerable<string> headerValues, string
string[] keyNameValueParts = keyNameValue.Trim().Split('=');
if (keyNameValueParts.Length == 2 && keyNameValueParts[0].Trim() == keyName)
{
return keyNameValueParts[1].Trim();
string value = keyNameValueParts[1].Trim();
return StringUtilities.EnforceMaxLength(value, InjectionGuardConstants.RequestHeaderMaxLength);
}
}
}
Expand Down Expand Up @@ -93,5 +96,24 @@ private static bool HeaderMatchesKey(string headerValue, string key)

return true;
}

/// <summary>
/// Http Headers only allow Printable US-ASCII characters.
/// Remove all other characters.
/// </summary>
public static string SanitizeString(string input)
{
if (string.IsNullOrWhiteSpace(input))
{
return input;
}

// US-ASCII characters (hex: 0x00 - 0x7F) (decimal: 0-127)
// ASCII Extended characters (hex: 0x80 - 0xFF) (decimal: 0-255) (NOT ALLOWED)
// Non-Printable ASCII characters are (hex: 0x00 - 0x1F) (decimal: 0-31) (NOT ALLOWED)
// Printable ASCII characters are (hex: 0x20 - 0xFF) (decimal: 32-255)
return Regex.Replace(input, @"[^\u0020-\u007F]", string.Empty);
}

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using System.Net.Http.Headers;
using System.Reflection;
using Extensibility.Implementation.Tracing;
using Microsoft.ApplicationInsights.AspNetCore.Common;
using Microsoft.ApplicationInsights.AspNetCore.Extensions;
using Microsoft.ApplicationInsights.DataContracts;
using Microsoft.ApplicationInsights.Extensibility.Implementation;
Expand Down Expand Up @@ -79,6 +80,7 @@ public void OnHttpRequestInStart(HttpContext httpContext)
}
else if (httpContext.Request.Headers.TryGetValue(RequestResponseHeaders.StandardRootIdHeader, out xmsRequestRootId))
{
xmsRequestRootId = StringUtilities.EnforceMaxLength(xmsRequestRootId, InjectionGuardConstants.RequestHeaderMaxLength);
var activity = new Activity(ActivityCreatedByHostingDiagnosticListener);
activity.SetParentId(xmsRequestRootId);
activity.Start();
Expand Down Expand Up @@ -115,6 +117,7 @@ public void OnBeginRequest(HttpContext httpContext, long timestamp)
IHeaderDictionary requestHeaders = httpContext.Request.Headers;
if (requestHeaders.TryGetValue(RequestResponseHeaders.RequestIdHeader, out requestId))
{
requestId = StringUtilities.EnforceMaxLength(requestId, InjectionGuardConstants.RequestHeaderMaxLength);
isActivityCreatedFromRequestIdHeader = true;
activity.SetParentId(requestId);

Expand All @@ -126,13 +129,16 @@ public void OnBeginRequest(HttpContext httpContext, long timestamp)
NameValueHeaderValue baggageItem;
if (NameValueHeaderValue.TryParse(item, out baggageItem))
{
var itemName = StringUtilities.EnforceMaxLength(baggageItem.Name, InjectionGuardConstants.ContextHeaderKeyMaxLength);
var itemValue = StringUtilities.EnforceMaxLength(baggageItem.Value, InjectionGuardConstants.ContextHeaderValueMaxLength);
activity.AddBaggage(baggageItem.Name, baggageItem.Value);
}
}
}
}
else if (requestHeaders.TryGetValue(RequestResponseHeaders.StandardRootIdHeader, out standardRootId))
{
standardRootId = StringUtilities.EnforceMaxLength(standardRootId, InjectionGuardConstants.RequestHeaderMaxLength);
activity.SetParentId(standardRootId);
}

Expand Down Expand Up @@ -209,6 +215,7 @@ private RequestTelemetry InitializeRequestTelemetryFromActivity(HttpContext http
}
else if (httpContext.Request.Headers.TryGetValue(RequestResponseHeaders.StandardParentIdHeader, out standardParentId))
{
standardParentId = StringUtilities.EnforceMaxLength(standardParentId, InjectionGuardConstants.RequestHeaderMaxLength);
requestTelemetry.Context.Operation.ParentId = standardParentId;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,12 @@ public void LogHostingDiagnosticListenerOnHttpRequestInStartActivityNull(string
this.WriteEvent(9, this.ApplicationName);
}

[Event(10, Message = "Failed to retrieve App ID for the current application insights resource. Endpoint returned HttpStatusCode: {0}", Level = EventLevel.Warning, Keywords = Keywords.Diagnostics)]
public void FetchAppIdFailedWithResponseCode(string exception, string appDomainName = "Incorrect")
{
this.WriteEvent(10, exception, this.ApplicationName);
}

/// <summary>
/// Keywords for the AspNetEventSource.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
{
using System;
using System.Net.Http.Headers;
using Microsoft.ApplicationInsights.AspNetCore.Common;
using Microsoft.ApplicationInsights.AspNetCore.DiagnosticListeners;
using Microsoft.ApplicationInsights.Channel;
using Microsoft.ApplicationInsights.DataContracts;
Expand Down Expand Up @@ -45,12 +46,13 @@ protected override void OnInitializeTelemetry(
HttpRequest currentRequest = platformContext.Request;
if (currentRequest?.Headers != null && string.IsNullOrEmpty(requestTelemetry.Source))
{
string headerCorrelationId = HttpHeadersUtilities.GetRequestContextKeyValue(currentRequest.Headers, RequestResponseHeaders.RequestContextSourceKey);
string headerCorrelationId = HttpHeadersUtilities.GetRequestContextKeyValue(currentRequest.Headers, RequestResponseHeaders.RequestContextSourceKey);

string appCorrelationId = null;
// If the source header is present on the incoming request, and it is an external component (not the same ikey as the one used by the current component), populate the source field.
if (!string.IsNullOrEmpty(headerCorrelationId))
{
headerCorrelationId = StringUtilities.EnforceMaxLength(headerCorrelationId, InjectionGuardConstants.AppIdMaxLengeth);
if (string.IsNullOrEmpty(requestTelemetry.Context.InstrumentationKey))
{
requestTelemetry.Source = headerCorrelationId;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
namespace Microsoft.ApplicationInsights.AspNetCore.Tests
{
using System;
using System.Globalization;
using System.Threading;
using System.Threading.Tasks;
using ApplicationInsights.Extensibility;
using DiagnosticListeners;
Expand All @@ -10,19 +12,68 @@ public class CorrelationIdLookupHelperTest
{
private const string TestInstrumentationKey = "11111111-2222-3333-4444-555555555555";

/// <summary>
/// Makes sure that the first call to get app id returns false, because it hasn't been fetched yet.
/// But the second call is able to get it from the dictionary.
/// </summary>
[Fact]
public void TryGetXComponentCorrelationIdShouldReturnAppIdWhenHit()
public void CorrelationIdLookupHelperReturnsAppIdOnSecondCall()
{
CorrelationIdLookupHelper target = new CorrelationIdLookupHelper((iKey) =>
var correlationIdLookupHelper = new CorrelationIdLookupHelper((ikey) =>
{
return Task.FromResult(string.Format(CultureInfo.InvariantCulture, "AppId for {0}", iKey));
// Pretend App Id is the same as Ikey
return Task.FromResult(ikey);
});

string actual = null;
target.TryGetXComponentCorrelationId(TestInstrumentationKey, out actual);
string expected = string.Format(CultureInfo.InvariantCulture, CorrelationIdLookupHelper.CorrelationIdFormat, "AppId for " + TestInstrumentationKey);
string instrumenationKey = Guid.NewGuid().ToString();
string cid;

Assert.Equal(expected, actual);
// First call returns false;
Assert.False(correlationIdLookupHelper.TryGetXComponentCorrelationId(instrumenationKey, out cid));

// Let's wait for the task to complete. It should be really quick (based on the test setup) but not immediate.
while (correlationIdLookupHelper.IsFetchAppInProgress(instrumenationKey))
{
Thread.Sleep(10); // wait 10 ms.
}

// Once fetch is complete, subsequent calls should return correlation id.
Assert.True(correlationIdLookupHelper.TryGetXComponentCorrelationId(instrumenationKey, out cid));
}

/// <summary>
/// Test that if an malicious value is returned, that value will be truncated.
/// </summary>
[Fact]
public void CorrelationIdLookupHelperTruncatesMaliciousValue()
{
// 50 character string.
var value = "a123456789b123546789c123456789d123456798e123456789";

// An arbitrary string that is expected to be truncated.
var malicious = "00000000000000000000000000000000000000000000000000000000000";

var cidPrefix = "cid-v1:";

var correlationIdLookupHelper = new CorrelationIdLookupHelper((ikey) =>
{
return Task.FromResult(value + malicious);
});

string instrumenationKey = Guid.NewGuid().ToString();

// first request fails because this will create the fetch task.
Assert.False(correlationIdLookupHelper.TryGetXComponentCorrelationId(instrumenationKey, out string ignore));

// Let's wait for the task to complete. It should be really quick (based on the test setup) but not immediate.
while (correlationIdLookupHelper.IsFetchAppInProgress(instrumenationKey))
{
Thread.Sleep(10); // wait 10 ms.
}

// Once fetch is complete, subsequent calls should return correlation id.
Assert.True(correlationIdLookupHelper.TryGetXComponentCorrelationId(instrumenationKey, out string cid));
Assert.Equal(cidPrefix + value, cid);
}

[Fact]
Expand Down