Skip to content

[AppSec] Send ip addresses headers to the backend #1764

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
anna-git merged 16 commits into from
Oct 13, 2021
Merged

[AppSec] Send ip addresses headers to the backend #1764

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
206fa08
Share a function to send all headers between core / framework
anna-git Sep 10, 2021
81a0490
socket ip address in remote ip property for net framework
anna-git Sep 10, 2021
15428b8
Add environment value
anna-git Sep 10, 2021
519706f
ip extractor algorithm
anna-git Oct 7, 2021
2749803
send correct ip address> local resolution, send extra headers
anna-git Oct 7, 2021
0de0160
unit tests
anna-git Oct 5, 2021
5ab9e25
adding unit test to sln
anna-git Oct 5, 2021
9cd93c0
add unit tests to sln
anna-git Oct 5, 2021
70badbc
remove some changes in sln
anna-git Oct 5, 2021
5474803
Fixing sln
anna-git Oct 7, 2021
87447df
Changes after Roberts comments
anna-git Oct 7, 2021
fa82ff3
Don't send null values to backend #1698
anna-git Oct 8, 2021
4fda7cc
Actor should contain the resolved ip , Request should contain the pee…
anna-git Oct 10, 2021
e461244
Add unit tests, remove port and send more headers
anna-git Oct 11, 2021
0866b4d
x-client-ip instead of client-ip
anna-git Oct 11, 2021
3419144
Dont assign port if no ip, fixes to comments
anna-git Oct 13, 2021
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
1 change: 1 addition & 0 deletions Datadog.Trace.Security.slnf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
"tracer\\src\\Datadog.Trace\\Datadog.Trace.csproj",
"tracer\\test\\Datadog.Trace.ClrProfiler.IntegrationTests\\Datadog.Trace.ClrProfiler.IntegrationTests.csproj",
"tracer\\test\\Datadog.Trace.Security.IntegrationTests\\Datadog.Trace.Security.IntegrationTests.csproj",
"tracer\\test\\Datadog.Trace.Security.Unit.Tests\\Datadog.Trace.Security.Unit.Tests.csproj",
"tracer\\test\\Datadog.Trace.TestHelpers\\Datadog.Trace.TestHelpers.csproj",
"tracer\\test\\test-applications\\security\\Samples.AspNetCore2\\Samples.AspNetCore2.csproj",
"tracer\\test\\test-applications\\security\\Samples.AspNetCore5\\Samples.AspNetCore5.csproj",
Expand Down
16 changes: 16 additions & 0 deletions Datadog.Trace.sln
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -395,6 +395,8 @@ Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Samples.AspNetMvc5", "trace
EndProject
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Datadog.Trace.Security.IntegrationTests", "tracer\test\Datadog.Trace.Security.IntegrationTests\Datadog.Trace.Security.IntegrationTests.csproj", "{7C66569C-1174-49AF-8DA7-8B216685C1D4}"
EndProject
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Datadog.Trace.Security.Unit.Tests", "tracer\test\Datadog.Trace.Security.Unit.Tests\Datadog.Trace.Security.Unit.Tests.csproj", "{EE45C020-5EC5-4722-9E35-BC5CC62D2722}"
EndProject
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Samples.AspNetCore2", "tracer\test\test-applications\security\Samples.AspNetCore2\Samples.AspNetCore2.csproj", "{8A73100E-F2C3-44D3-A5B8-49B5BFDF1B52}"
EndProject
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Datadog.Monitoring.Distribution", "tracer\src\Datadog.Monitoring.Distribution\Datadog.Monitoring.Distribution.csproj", "{CF364E70-F5B5-4D44-B29E-2165525D3A69}"
Expand Down Expand Up @@ -442,6 +444,7 @@ Global
tracer\test\test-applications\Samples.Shared\Samples.Shared.projitems*{b6a98887-4a47-4c19-9c6f-d833e24f4b1c}*SharedItemsImports = 4
tracer\test\test-applications\Samples.Shared\Samples.Shared.projitems*{bbb60b0f-bf01-4499-936a-4a299a9acfd4}*SharedItemsImports = 4
tracer\test\test-applications\Samples.Shared\Samples.Shared.projitems*{c41b289a-a344-49df-a1bd-081753f9a286}*SharedItemsImports = 5
tracer\test\test-applications\Samples.Shared\Samples.Shared.projitems*{f5582f54-e911-4258-b419-5e894d338c5b}*SharedItemsImports = 4
EndGlobalSection
GlobalSection(SolutionConfigurationPlatforms) = preSolution
Debug|Any CPU = Debug|Any CPU
Expand Down Expand Up @@ -1675,6 +1678,18 @@ Global
{7C66569C-1174-49AF-8DA7-8B216685C1D4}.Release|x64.Build.0 = Release|x64
{7C66569C-1174-49AF-8DA7-8B216685C1D4}.Release|x86.ActiveCfg = Release|x86
{7C66569C-1174-49AF-8DA7-8B216685C1D4}.Release|x86.Build.0 = Release|x86
{EE45C020-5EC5-4722-9E35-BC5CC62D2722}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
{EE45C020-5EC5-4722-9E35-BC5CC62D2722}.Debug|Any CPU.Build.0 = Debug|Any CPU
{EE45C020-5EC5-4722-9E35-BC5CC62D2722}.Debug|x64.ActiveCfg = Debug|Any CPU
{EE45C020-5EC5-4722-9E35-BC5CC62D2722}.Debug|x64.Build.0 = Debug|Any CPU
{EE45C020-5EC5-4722-9E35-BC5CC62D2722}.Debug|x86.ActiveCfg = Debug|Any CPU
{EE45C020-5EC5-4722-9E35-BC5CC62D2722}.Debug|x86.Build.0 = Debug|Any CPU
{EE45C020-5EC5-4722-9E35-BC5CC62D2722}.Release|Any CPU.ActiveCfg = Release|Any CPU
{EE45C020-5EC5-4722-9E35-BC5CC62D2722}.Release|Any CPU.Build.0 = Release|Any CPU
{EE45C020-5EC5-4722-9E35-BC5CC62D2722}.Release|x64.ActiveCfg = Release|Any CPU
{EE45C020-5EC5-4722-9E35-BC5CC62D2722}.Release|x64.Build.0 = Release|Any CPU
{EE45C020-5EC5-4722-9E35-BC5CC62D2722}.Release|x86.ActiveCfg = Release|Any CPU
{EE45C020-5EC5-4722-9E35-BC5CC62D2722}.Release|x86.Build.0 = Release|Any CPU
{8A73100E-F2C3-44D3-A5B8-49B5BFDF1B52}.Debug|Any CPU.ActiveCfg = Debug|x64
{8A73100E-F2C3-44D3-A5B8-49B5BFDF1B52}.Debug|Any CPU.Build.0 = Debug|x64
{8A73100E-F2C3-44D3-A5B8-49B5BFDF1B52}.Debug|x64.ActiveCfg = Debug|x64
Expand Down Expand Up @@ -1952,6 +1967,7 @@ Global
{AAFFA51D-1357-4560-97DC-43AD039442E6} = {0972AD57-B16B-494F-AE0A-091DD6F3B42B}
{F5582F54-E911-4258-B419-5E894D338C5B} = {AAFFA51D-1357-4560-97DC-43AD039442E6}
{7C66569C-1174-49AF-8DA7-8B216685C1D4} = {8CEC2042-F11C-49F5-A674-2355793B600A}
{EE45C020-5EC5-4722-9E35-BC5CC62D2722} = {8CEC2042-F11C-49F5-A674-2355793B600A}
{8A73100E-F2C3-44D3-A5B8-49B5BFDF1B52} = {0972AD57-B16B-494F-AE0A-091DD6F3B42B}
{CF364E70-F5B5-4D44-B29E-2165525D3A69} = {9E5F0022-0A50-40BF-AC6A-C3078585ECAB}
{463A6FB2-1ABE-4B92-A470-97134D0BBC7E} = {BAF8F246-3645-42AD-B1D0-0F7EAFBAB34A}
Expand Down
3 changes: 3 additions & 0 deletions tracer/src/Datadog.Trace/AppSec/AddressesConstants.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@
// This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2017 Datadog, Inc.
// </copyright>

using System.Collections.Generic;
using System.Linq;

namespace Datadog.Trace.AppSec
{
internal static class AddressesConstants
Expand Down
12 changes: 9 additions & 3 deletions tracer/src/Datadog.Trace/AppSec/EventModel/Attack.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@
// </copyright>

using System;
using System.Collections.Generic;
using System.Net;
using Datadog.Trace.AppSec.Transports.Http;
using Datadog.Trace.Vendors.Newtonsoft.Json;

namespace Datadog.Trace.AppSec.EventModel
Expand All @@ -23,16 +25,20 @@ internal class Attack : AppSecEvent
[JsonProperty("context")]
public Context Context { get; set; }

public static Attack From(Waf.ReturnTypes.Managed.Return result, Trace.Span span, Transport.ITransport transport)
public static Attack From(Waf.ReturnTypes.Managed.Return result, Trace.Span span, Transport.ITransport transport, string customIpHeader, IEnumerable<string> extraHeaders)
{
var ruleMatch = result.ResultData.Filter[0];
var request = transport.Request();
var headersIpAndPort = RequestHeadersHelper.ExtractHeadersIpAndPort(transport.GetHeader, customIpHeader, extraHeaders, transport.IsSecureConnection, new IpInfo(request.RemoteIp, request.RemotePort));
request.Headers = headersIpAndPort.HeadersToSend;

var frameworkDescription = FrameworkDescription.Instance;
var attack = new Attack
{
EventId = Guid.NewGuid().ToString(),
Context = new Context()
{
Actor = new Actor { Ip = new Ip { Address = headersIpAndPort.IpInfo.IpAddress } },
Host = new Host
{
OsType = frameworkDescription.OSPlatform,
Expand All @@ -43,7 +49,7 @@ public static Attack From(Waf.ReturnTypes.Managed.Return result, Trace.Span span
Request = request,
Response = transport.Response(result.Blocked)
},
Actor = new Actor { Ip = new Ip { Address = request.RemoteIp } },
Service = new Service { Environment = CorrelationIdentifier.Env },

@zacharycmontoya zacharycmontoya Oct 12, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

FYI CorrelationIdentifier.Env will return an empty string as a fallback case. Will that input cause any issues?

@anna-git anna-git Oct 13, 2021

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.

It's supposed to have the content of the DD_ENV environment variable. We actually have some system tests failing because of it, I'm not sure if it's going to be empty sometimes even if DD_ENV is set?

@zacharycmontoya zacharycmontoya Oct 13, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If DD_ENV is set, then this should receive that value. But if DD_ENV isn't set then I expect an empty string

@anna-git anna-git Oct 13, 2021

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.

thanks!
so I think that's what we're expecting :)

Tracer = new Tracer
{
RuntimeType = frameworkDescription.Name,
Expand All @@ -66,7 +72,7 @@ public static Attack From(Waf.ReturnTypes.Managed.Return result, Trace.Span span
{
attack.Context.Span = new Span { Id = span.SpanId };
attack.Context.Trace = new Span { Id = span.TraceId };
attack.Context.Service = new Service { Name = span.ServiceName };
attack.Context.Service.Name = span.ServiceName;
}

return attack;
Expand Down
8 changes: 6 additions & 2 deletions tracer/src/Datadog.Trace/AppSec/EventModel/Request.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
// </copyright>

using System;
using System.Collections.Generic;
using Datadog.Trace.Vendors.Newtonsoft.Json;

namespace Datadog.Trace.AppSec.EventModel
Expand All @@ -17,7 +18,7 @@ internal class Request
public string Method { get; set; }

[JsonProperty("url")]
public Uri Url { get; set; }
public string Url { get; set; }

[JsonProperty("host")]
public string Host { get; set; }
Expand All @@ -35,6 +36,9 @@ internal class Request
public string RemoteIp { get; set; }

[JsonProperty("remote_port")]
public long RemotePort { get; set; }
public int RemotePort { get; set; }

[JsonProperty("headers")]
public IDictionary<string, string> Headers { get; set; }
}
}
2 changes: 1 addition & 1 deletion tracer/src/Datadog.Trace/AppSec/Security.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ private void Report(ITransport transport, Span span, Waf.ReturnTypes.Managed.Ret

transport.OnCompleted(() =>
{
var attack = Attack.From(result, span, transport);
var attack = Attack.From(result, span, transport, _settings.CustomIpHeader, _settings.ExtraHeaders);
_agentWriter.AddEvent(attack);
});
}
Expand Down
10 changes: 10 additions & 0 deletions tracer/src/Datadog.Trace/AppSec/SecuritySettings.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,20 @@ public SecuritySettings(IConfigurationSource source)
Enabled = source?.GetBool(ConfigurationKeys.AppSecEnabled) ?? false;
BlockingEnabled = source?.GetBool(ConfigurationKeys.AppSecBlockingEnabled) ?? false;
Rules = source?.GetString(ConfigurationKeys.AppSecRules);
CustomIpHeader = source?.GetString(ConfigurationKeys.AppSecCustomIpHeader);
var extraHeaders = source?.GetString(ConfigurationKeys.AppSecExtraHeaders);
ExtraHeaders = !string.IsNullOrEmpty(extraHeaders) ? extraHeaders.Split(',') : new string[0];
}

public bool Enabled { get; set; }

public string CustomIpHeader { get; }

/// <summary>
/// Gets keys indicating the optional custom appsec headers the user wants to send.
/// </summary>
public IReadOnlyList<string> ExtraHeaders { get; }

public bool BlockingEnabled { get; }

public string Rules { get; }
Expand Down
28 changes: 28 additions & 0 deletions tracer/src/Datadog.Trace/AppSec/Transports/Http/ExtractedHeadersAndIpInfos.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
// <copyright file="ExtractedHeadersAndIpInfos.cs" company="Datadog">
// Unless explicitly stated otherwise all files in this repository are licensed under the Apache 2 License.
// This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2017 Datadog, Inc.
// </copyright>

using System.Collections.Generic;

namespace Datadog.Trace.AppSec.Transports.Http
{
internal class ExtractedHeadersAndIpInfos
{
public ExtractedHeadersAndIpInfos(IDictionary<string, string> headersToSend, string address, int port)
{
HeadersToSend = headersToSend;
IpInfo = new IpInfo(address, port);
}

public ExtractedHeadersAndIpInfos(IDictionary<string, string> headersToSend, IpInfo ipInfo)
{
HeadersToSend = headersToSend;
IpInfo = ipInfo;
}

public IDictionary<string, string> HeadersToSend { get; }

public IpInfo IpInfo { get; }
}
}
10 changes: 8 additions & 2 deletions tracer/src/Datadog.Trace/AppSec/Transports/Http/HttpTransport.Core.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
using System.Threading.Tasks;
using Datadog.Trace.AppSec.EventModel;
using Datadog.Trace.AppSec.Waf;
using Datadog.Trace.Util.Http;
using Microsoft.AspNetCore.Http;

namespace Datadog.Trace.AppSec.Transport.Http
Expand All @@ -18,20 +19,25 @@ internal class HttpTransport : ITransport

public HttpTransport(HttpContext context) => this.context = context;

public bool IsSecureConnection => context.Request.IsHttps;

public Func<string, string> GetHeader => key => context.Request.Headers[key];

public Request Request()
{
var request = new Request
{
Method = context.Request.Method,
Path = context.Request.Path,
Scheme = context.Request.Scheme,
RemoteIp = context.Connection.RemoteIpAddress.ToString()
Url = context.Request.GetUrl()
};

if (context.Request.Host.HasValue)
{
request.Host = context.Request.Host.ToString();
request.Port = context.Request.Host.Port.GetValueOrDefault();
request.RemoteIp = context.Connection.RemoteIpAddress.ToString();

@zacharycmontoya zacharycmontoya Oct 12, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is there a reason that this code doesn't make a call to IpExtractor.ExtractAddressAndPort, like in HttpTransport.Framework?

@anna-git anna-git Oct 13, 2021 •
edited
Loading

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.

These properties should contain the peer ip address and port so in core we have them out of the box, but in framework, afaik it seems it's the remote_addr server variable which UserHostAddress points to, so I assumed the port could be part of it in some cases.

request.Port = context.Connection.RemotePort;
}

return request;
Expand Down
41 changes: 22 additions & 19 deletions tracer/src/Datadog.Trace/AppSec/Transports/Http/HttpTransport.Framework.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,24 +5,24 @@

#if NETFRAMEWORK
using System;
using System.Threading.Tasks;
using System.Collections.Generic;
using System.Web;
using Datadog.Trace.AppSec.EventModel;
using Datadog.Trace.AppSec.Transports.Http;
using Datadog.Trace.AppSec.Waf;
using Datadog.Trace.Logging;

namespace Datadog.Trace.AppSec.Transport.Http
{
internal class HttpTransport : ITransport
{
private const string WafKey = "waf";
private static readonly IDatadogLogger Log = DatadogLogging.GetLoggerFor<HttpTransport>();
private readonly System.Web.HttpContext context;
private readonly HttpContext context;

public HttpTransport(HttpContext context)
{
this.context = context;
}
public HttpTransport(HttpContext context) => this.context = context;

public bool IsSecureConnection => context.Request.IsSecureConnection;

public Func<string, string> GetHeader => key => context.Request.Headers[key];

public void AddRequestScope(Guid guid)
{
Expand All @@ -38,19 +38,22 @@ public void Block()
context.ApplicationInstance.CompleteRequest();
}

public IContext GetAdditiveContext()
{
return context.Items[WafKey] as IContext;
}
public IContext GetAdditiveContext() => context.Items[WafKey] as IContext;

public Request Request() => new()
public Request Request()
{
Url = context.Request.Url,
Method = context.Request.HttpMethod,
Scheme = context.Request.Url.Scheme,
RemoteIp = context.Request.ServerVariables["HTTP_X_FORWARDED_FOR"] ?? context.Request.ServerVariables["REMOTE_ADDR"],
Host = context.Request.UserHostAddress,
};
var extracted = IpExtractor.ExtractAddressAndPort(context.Request.UserHostAddress, context.Request.IsSecureConnection);
var request = new Request()
{
Url = context.Request.Url.ToString(),
Method = context.Request.HttpMethod,
Scheme = context.Request.Url.Scheme,
Host = context.Request.UserHostName,
RemoteIp = extracted.IpAddress,
RemotePort = extracted.Port
};
return request;
}

public Response Response(bool blocked) => new()
{
Expand Down
Loading