From 5f6ab836dea4341844f1e1e5a1e1d45c7c6621c1 Mon Sep 17 00:00:00 2001 From: VooDooS Date: Thu, 11 Apr 2019 15:35:06 +0200 Subject: [PATCH 1/5] Extend Microsoft.Net.Http.Headers.HeaderNames --- MediaBrowser.Common/Net/MoreHeaderNames.cs | 83 ++++++++++++++++++++++ 1 file changed, 83 insertions(+) create mode 100644 MediaBrowser.Common/Net/MoreHeaderNames.cs diff --git a/MediaBrowser.Common/Net/MoreHeaderNames.cs b/MediaBrowser.Common/Net/MoreHeaderNames.cs new file mode 100644 index 0000000000..669646db1e --- /dev/null +++ b/MediaBrowser.Common/Net/MoreHeaderNames.cs @@ -0,0 +1,83 @@ +using HN = Microsoft.Net.Http.Headers.HeaderNames; + +namespace MediaBrowser.Common.Net +{ + public static class MoreHeaderNames + { + // Other Headers + public const string XForwardedFor = "X-Forwarded-For"; + public const string XForwardedPort = "X-Forwarded-Port"; + public const string XForwardedProto = "X-Forwarded-Proto"; + public const string XRealIP = "X-Real-IP"; + + // Headers from Microsoft.Net.Http.Headers.HeaderNames + public const string Accept = HN.Accept; + public const string AcceptCharset = HN.AcceptCharset; + public const string AcceptEncoding = HN.AcceptEncoding; + public const string AcceptLanguage = HN.AcceptLanguage; + public const string AcceptRanges = HN.AcceptRanges; + public const string AccessControlAllowCredentials = HN.AccessControlAllowCredentials; + public const string AccessControlAllowHeaders = HN.AccessControlAllowHeaders; + public const string AccessControlAllowMethods = HN.AccessControlAllowMethods; + public const string AccessControlAllowOrigin = HN.AccessControlAllowOrigin; + public const string AccessControlExposeHeaders = HN.AccessControlExposeHeaders; + public const string AccessControlMaxAge = HN.AccessControlMaxAge; + public const string AccessControlRequestHeaders = HN.AccessControlRequestHeaders; + public const string AccessControlRequestMethod = HN.AccessControlRequestMethod; + public const string Age = HN.Age; + public const string Allow = HN.Allow; + public const string Authority = HN.Authority; + public const string Authorization = HN.Authorization; + public const string CacheControl = HN.CacheControl; + public const string Connection = HN.Connection; + public const string ContentDisposition = HN.ContentDisposition; + public const string ContentEncoding = HN.ContentEncoding; + public const string ContentLanguage = HN.ContentLanguage; + public const string ContentLength = HN.ContentLength; + public const string ContentLocation = HN.ContentLocation; + public const string ContentMD5 = HN.ContentMD5; + public const string ContentRange = HN.ContentRange; + public const string ContentSecurityPolicy = HN.ContentSecurityPolicy; + public const string ContentSecurityPolicyReportOnly = HN.ContentSecurityPolicyReportOnly; + public const string ContentType = HN.ContentType; + public const string Cookie = HN.Cookie; + public const string Date = HN.Date; + public const string ETag = HN.ETag; + public const string Expires = HN.Expires; + public const string Expect = HN.Expect; + public const string From = HN.From; + public const string Host = HN.Host; + public const string IfMatch = HN.IfMatch; + public const string IfModifiedSince = HN.IfModifiedSince; + public const string IfNoneMatch = HN.IfNoneMatch; + public const string IfRange = HN.IfRange; + public const string IfUnmodifiedSince = HN.IfUnmodifiedSince; + public const string LastModified = HN.LastModified; + public const string Location = HN.Location; + public const string MaxForwards = HN.MaxForwards; + public const string Method = HN.Method; + public const string Origin = HN.Origin; + public const string Path = HN.Path; + public const string Pragma = HN.Pragma; + public const string ProxyAuthenticate = HN.ProxyAuthenticate; + public const string ProxyAuthorization = HN.ProxyAuthorization; + public const string Range = HN.Range; + public const string Referer = HN.Referer; + public const string RetryAfter = HN.RetryAfter; + public const string Scheme = HN.Scheme; + public const string Server = HN.Server; + public const string SetCookie = HN.SetCookie; + public const string Status = HN.Status; + public const string StrictTransportSecurity = HN.StrictTransportSecurity; + public const string TE = HN.TE; + public const string Trailer = HN.Trailer; + public const string TransferEncoding = HN.TransferEncoding; + public const string Upgrade = HN.Upgrade; + public const string UserAgent = HN.UserAgent; + public const string Vary = HN.Vary; + public const string Via = HN.Via; + public const string Warning = HN.Warning; + public const string WebSocketSubProtocols = HN.WebSocketSubProtocols; + public const string WWWAuthenticate = HN.WWWAuthenticate; + } +} \ No newline at end of file From a6e1b23eb04d620e241e4babbcb6ee0ffbf156a9 Mon Sep 17 00:00:00 2001 From: VooDooS Date: Thu, 11 Apr 2019 16:58:28 +0200 Subject: [PATCH 2/5] Simplify headers use in WSS --- .../SocketSharp/WebSocketSharpRequest.cs | 16 +++++---------- MediaBrowser.Model/Services/IHttpRequest.cs | 20 ------------------- 2 files changed, 5 insertions(+), 31 deletions(-) diff --git a/Emby.Server.Implementations/SocketSharp/WebSocketSharpRequest.cs b/Emby.Server.Implementations/SocketSharp/WebSocketSharpRequest.cs index 792615a0f6..957371df62 100644 --- a/Emby.Server.Implementations/SocketSharp/WebSocketSharpRequest.cs +++ b/Emby.Server.Implementations/SocketSharp/WebSocketSharpRequest.cs @@ -10,6 +10,7 @@ using Microsoft.AspNetCore.Http.Extensions; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Primitives; using Microsoft.Net.Http.Headers; +using HeaderNames = MediaBrowser.Common.Net.MoreHeaderNames; using IHttpFile = MediaBrowser.Model.Services.IHttpFile; using IHttpRequest = MediaBrowser.Model.Services.IHttpRequest; using IResponse = MediaBrowser.Model.Services.IResponse; @@ -38,16 +39,9 @@ namespace Emby.Server.Implementations.SocketSharp public string RawUrl => request.GetEncodedPathAndQuery(); public string AbsoluteUri => request.GetDisplayUrl().TrimEnd('/'); + // Header[name] returns "" when undefined - public string XForwardedFor - => StringValues.IsNullOrEmpty(request.Headers["X-Forwarded-For"]) ? null : request.Headers["X-Forwarded-For"].ToString(); - - public int? XForwardedPort - => StringValues.IsNullOrEmpty(request.Headers["X-Forwarded-Port"]) ? (int?)null : int.Parse(request.Headers["X-Forwarded-Port"], CultureInfo.InvariantCulture); - - public string XForwardedProtocol => StringValues.IsNullOrEmpty(request.Headers["X-Forwarded-Proto"]) ? null : request.Headers["X-Forwarded-Proto"].ToString(); - - public string XRealIp => StringValues.IsNullOrEmpty(request.Headers["X-Real-IP"]) ? null : request.Headers["X-Real-IP"].ToString(); + private string GetHeader(string name) => request.Headers[name].ToString(); private string remoteIp; public string RemoteIp @@ -59,13 +53,13 @@ namespace Emby.Server.Implementations.SocketSharp return remoteIp; } - var temp = CheckBadChars(XForwardedFor.AsSpan()); + var temp = CheckBadChars(GetHeader(HeaderNames.XForwardedFor).AsSpan()); if (temp.Length != 0) { return remoteIp = temp.ToString(); } - temp = CheckBadChars(XRealIp.AsSpan()); + temp = CheckBadChars(GetHeader(HeaderNames.XRealIP).AsSpan()); if (temp.Length != 0) { return remoteIp = NormalizeIp(temp).ToString(); diff --git a/MediaBrowser.Model/Services/IHttpRequest.cs b/MediaBrowser.Model/Services/IHttpRequest.cs index 50c6076f30..daf91488f0 100644 --- a/MediaBrowser.Model/Services/IHttpRequest.cs +++ b/MediaBrowser.Model/Services/IHttpRequest.cs @@ -7,26 +7,6 @@ namespace MediaBrowser.Model.Services /// string HttpMethod { get; } - /// - /// The IP Address of the X-Forwarded-For header, null if null or empty - /// - string XForwardedFor { get; } - - /// - /// The Port number of the X-Forwarded-Port header, null if null or empty - /// - int? XForwardedPort { get; } - - /// - /// The http or https scheme of the X-Forwarded-Proto header, null if null or empty - /// - string XForwardedProtocol { get; } - - /// - /// The value of the X-Real-IP header, null if null or empty - /// - string XRealIp { get; } - /// /// The value of the Accept HTTP Request Header /// From 56d1050bac3a56249acd7f3b3615f796683e0783 Mon Sep 17 00:00:00 2001 From: VooDooS Date: Thu, 11 Apr 2019 17:13:40 +0200 Subject: [PATCH 3/5] Replace custom ip "normalization" by methods from `IPAddress` --- .../SocketSharp/WebSocketSharpRequest.cs | 21 +++++++++---------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/Emby.Server.Implementations/SocketSharp/WebSocketSharpRequest.cs b/Emby.Server.Implementations/SocketSharp/WebSocketSharpRequest.cs index 957371df62..d153a85a32 100644 --- a/Emby.Server.Implementations/SocketSharp/WebSocketSharpRequest.cs +++ b/Emby.Server.Implementations/SocketSharp/WebSocketSharpRequest.cs @@ -62,10 +62,10 @@ namespace Emby.Server.Implementations.SocketSharp temp = CheckBadChars(GetHeader(HeaderNames.XRealIP).AsSpan()); if (temp.Length != 0) { - return remoteIp = NormalizeIp(temp).ToString(); + return remoteIp = NormalizeIp(temp.ToString()).ToString(); } - return remoteIp = NormalizeIp(request.HttpContext.Connection.RemoteIpAddress.ToString().AsSpan()).ToString(); + return remoteIp = NormalizeIp(request.HttpContext.Connection.RemoteIpAddress).ToString(); } } @@ -137,22 +137,21 @@ namespace Emby.Server.Implementations.SocketSharp return name; } - private ReadOnlySpan NormalizeIp(ReadOnlySpan ip) + private IPAddress NormalizeIp(IPAddress ip) { - if (ip.Length != 0 && !ip.IsWhiteSpace()) + if (ip.IsIPv4MappedToIPv6) { - // Handle ipv4 mapped to ipv6 - const string srch = "::ffff:"; - var index = ip.IndexOf(srch.AsSpan(), StringComparison.OrdinalIgnoreCase); - if (index == 0) - { - ip = ip.Slice(srch.Length); - } + return ip.MapToIPv4(); } return ip; } + private IPAddress NormalizeIp(string sip) + { + return NormalizeIp(IPAddress.Parse(sip)); + } + public string[] AcceptTypes => request.Headers.GetCommaSeparatedValues(HeaderNames.Accept); private Dictionary items; From bb807554e2c33f066402898a3ff79913865d50e3 Mon Sep 17 00:00:00 2001 From: VooDooS Date: Thu, 11 Apr 2019 17:17:48 +0200 Subject: [PATCH 4/5] Replace CRLF injection mitigation by use of .NET ip parsing --- .../SocketSharp/WebSocketSharpRequest.cs | 93 ++----------------- 1 file changed, 10 insertions(+), 83 deletions(-) diff --git a/Emby.Server.Implementations/SocketSharp/WebSocketSharpRequest.cs b/Emby.Server.Implementations/SocketSharp/WebSocketSharpRequest.cs index d153a85a32..38a860a511 100644 --- a/Emby.Server.Implementations/SocketSharp/WebSocketSharpRequest.cs +++ b/Emby.Server.Implementations/SocketSharp/WebSocketSharpRequest.cs @@ -53,91 +53,23 @@ namespace Emby.Server.Implementations.SocketSharp return remoteIp; } - var temp = CheckBadChars(GetHeader(HeaderNames.XForwardedFor).AsSpan()); - if (temp.Length != 0) - { - return remoteIp = temp.ToString(); - } - - temp = CheckBadChars(GetHeader(HeaderNames.XRealIP).AsSpan()); - if (temp.Length != 0) - { - return remoteIp = NormalizeIp(temp.ToString()).ToString(); - } - - return remoteIp = NormalizeIp(request.HttpContext.Connection.RemoteIpAddress).ToString(); - } - } - - private static readonly char[] HttpTrimCharacters = new char[] { (char)0x09, (char)0xA, (char)0xB, (char)0xC, (char)0xD, (char)0x20 }; - - // CheckBadChars - throws on invalid chars to be not found in header name/value - internal static ReadOnlySpan CheckBadChars(ReadOnlySpan name) - { - if (name.Length == 0) - { - return name; - } + IPAddress ip; - // VALUE check - // Trim spaces from both ends - name = name.Trim(HttpTrimCharacters); - - // First, check for correctly formed multi-line value - // Second, check for absence of CTL characters - int crlf = 0; - for (int i = 0; i < name.Length; ++i) - { - char c = (char)(0x000000ff & (uint)name[i]); - switch (crlf) + // "Real" remote ip might be in X-Forwarded-For of X-Real-Ip + // (if the server is behind a reverse proxy for example) + if (!IPAddress.TryParse(GetHeader(HeaderNames.XForwardedFor), out ip)) { - case 0: - if (c == '\r') - { - crlf = 1; - } - else if (c == '\n') - { - // Technically this is bad HTTP. But it would be a breaking change to throw here. - // Is there an exploit? - crlf = 2; - } - else if (c == 127 || (c < ' ' && c != '\t')) - { - throw new ArgumentException("net_WebHeaderInvalidControlChars", nameof(name)); - } - - break; - - case 1: - if (c == '\n') - { - crlf = 2; - break; - } - - throw new ArgumentException("net_WebHeaderInvalidCRLFChars", nameof(name)); - - case 2: - if (c == ' ' || c == '\t') - { - crlf = 0; - break; - } - - throw new ArgumentException("net_WebHeaderInvalidCRLFChars", nameof(name)); + if (!IPAddress.TryParse(GetHeader(HeaderNames.XRealIP), out ip)) + { + ip = request.HttpContext.Connection.RemoteIpAddress; + } } - } - if (crlf != 0) - { - throw new ArgumentException("net_WebHeaderInvalidCRLFChars", nameof(name)); + return remoteIp = NormalizeIp(ip).ToString(); } - - return name; } - private IPAddress NormalizeIp(IPAddress ip) + private static IPAddress NormalizeIp(IPAddress ip) { if (ip.IsIPv4MappedToIPv6) { @@ -147,11 +79,6 @@ namespace Emby.Server.Implementations.SocketSharp return ip; } - private IPAddress NormalizeIp(string sip) - { - return NormalizeIp(IPAddress.Parse(sip)); - } - public string[] AcceptTypes => request.Headers.GetCommaSeparatedValues(HeaderNames.Accept); private Dictionary items; From ba12d96d23a53ce16a1da1b2fcf68a301050b858 Mon Sep 17 00:00:00 2001 From: VooDooS Date: Fri, 12 Apr 2019 15:25:18 +0200 Subject: [PATCH 5/5] Removed wrapping of HeaderNames fields --- .../SocketSharp/WebSocketSharpRequest.cs | 6 +- MediaBrowser.Common/Net/CustomHeaderNames.cs | 11 +++ MediaBrowser.Common/Net/MoreHeaderNames.cs | 83 ------------------- 3 files changed, 14 insertions(+), 86 deletions(-) create mode 100644 MediaBrowser.Common/Net/CustomHeaderNames.cs delete mode 100644 MediaBrowser.Common/Net/MoreHeaderNames.cs diff --git a/Emby.Server.Implementations/SocketSharp/WebSocketSharpRequest.cs b/Emby.Server.Implementations/SocketSharp/WebSocketSharpRequest.cs index 38a860a511..00465b63eb 100644 --- a/Emby.Server.Implementations/SocketSharp/WebSocketSharpRequest.cs +++ b/Emby.Server.Implementations/SocketSharp/WebSocketSharpRequest.cs @@ -4,13 +4,13 @@ using System.Globalization; using System.IO; using System.Net; using System.Text; +using MediaBrowser.Common.Net; using MediaBrowser.Model.Services; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Http.Extensions; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Primitives; using Microsoft.Net.Http.Headers; -using HeaderNames = MediaBrowser.Common.Net.MoreHeaderNames; using IHttpFile = MediaBrowser.Model.Services.IHttpFile; using IHttpRequest = MediaBrowser.Model.Services.IHttpRequest; using IResponse = MediaBrowser.Model.Services.IResponse; @@ -57,9 +57,9 @@ namespace Emby.Server.Implementations.SocketSharp // "Real" remote ip might be in X-Forwarded-For of X-Real-Ip // (if the server is behind a reverse proxy for example) - if (!IPAddress.TryParse(GetHeader(HeaderNames.XForwardedFor), out ip)) + if (!IPAddress.TryParse(GetHeader(CustomHeaderNames.XForwardedFor), out ip)) { - if (!IPAddress.TryParse(GetHeader(HeaderNames.XRealIP), out ip)) + if (!IPAddress.TryParse(GetHeader(CustomHeaderNames.XRealIP), out ip)) { ip = request.HttpContext.Connection.RemoteIpAddress; } diff --git a/MediaBrowser.Common/Net/CustomHeaderNames.cs b/MediaBrowser.Common/Net/CustomHeaderNames.cs new file mode 100644 index 0000000000..ff148dc801 --- /dev/null +++ b/MediaBrowser.Common/Net/CustomHeaderNames.cs @@ -0,0 +1,11 @@ +namespace MediaBrowser.Common.Net +{ + public static class CustomHeaderNames + { + // Other Headers + public const string XForwardedFor = "X-Forwarded-For"; + public const string XForwardedPort = "X-Forwarded-Port"; + public const string XForwardedProto = "X-Forwarded-Proto"; + public const string XRealIP = "X-Real-IP"; + } +} \ No newline at end of file diff --git a/MediaBrowser.Common/Net/MoreHeaderNames.cs b/MediaBrowser.Common/Net/MoreHeaderNames.cs deleted file mode 100644 index 669646db1e..0000000000 --- a/MediaBrowser.Common/Net/MoreHeaderNames.cs +++ /dev/null @@ -1,83 +0,0 @@ -using HN = Microsoft.Net.Http.Headers.HeaderNames; - -namespace MediaBrowser.Common.Net -{ - public static class MoreHeaderNames - { - // Other Headers - public const string XForwardedFor = "X-Forwarded-For"; - public const string XForwardedPort = "X-Forwarded-Port"; - public const string XForwardedProto = "X-Forwarded-Proto"; - public const string XRealIP = "X-Real-IP"; - - // Headers from Microsoft.Net.Http.Headers.HeaderNames - public const string Accept = HN.Accept; - public const string AcceptCharset = HN.AcceptCharset; - public const string AcceptEncoding = HN.AcceptEncoding; - public const string AcceptLanguage = HN.AcceptLanguage; - public const string AcceptRanges = HN.AcceptRanges; - public const string AccessControlAllowCredentials = HN.AccessControlAllowCredentials; - public const string AccessControlAllowHeaders = HN.AccessControlAllowHeaders; - public const string AccessControlAllowMethods = HN.AccessControlAllowMethods; - public const string AccessControlAllowOrigin = HN.AccessControlAllowOrigin; - public const string AccessControlExposeHeaders = HN.AccessControlExposeHeaders; - public const string AccessControlMaxAge = HN.AccessControlMaxAge; - public const string AccessControlRequestHeaders = HN.AccessControlRequestHeaders; - public const string AccessControlRequestMethod = HN.AccessControlRequestMethod; - public const string Age = HN.Age; - public const string Allow = HN.Allow; - public const string Authority = HN.Authority; - public const string Authorization = HN.Authorization; - public const string CacheControl = HN.CacheControl; - public const string Connection = HN.Connection; - public const string ContentDisposition = HN.ContentDisposition; - public const string ContentEncoding = HN.ContentEncoding; - public const string ContentLanguage = HN.ContentLanguage; - public const string ContentLength = HN.ContentLength; - public const string ContentLocation = HN.ContentLocation; - public const string ContentMD5 = HN.ContentMD5; - public const string ContentRange = HN.ContentRange; - public const string ContentSecurityPolicy = HN.ContentSecurityPolicy; - public const string ContentSecurityPolicyReportOnly = HN.ContentSecurityPolicyReportOnly; - public const string ContentType = HN.ContentType; - public const string Cookie = HN.Cookie; - public const string Date = HN.Date; - public const string ETag = HN.ETag; - public const string Expires = HN.Expires; - public const string Expect = HN.Expect; - public const string From = HN.From; - public const string Host = HN.Host; - public const string IfMatch = HN.IfMatch; - public const string IfModifiedSince = HN.IfModifiedSince; - public const string IfNoneMatch = HN.IfNoneMatch; - public const string IfRange = HN.IfRange; - public const string IfUnmodifiedSince = HN.IfUnmodifiedSince; - public const string LastModified = HN.LastModified; - public const string Location = HN.Location; - public const string MaxForwards = HN.MaxForwards; - public const string Method = HN.Method; - public const string Origin = HN.Origin; - public const string Path = HN.Path; - public const string Pragma = HN.Pragma; - public const string ProxyAuthenticate = HN.ProxyAuthenticate; - public const string ProxyAuthorization = HN.ProxyAuthorization; - public const string Range = HN.Range; - public const string Referer = HN.Referer; - public const string RetryAfter = HN.RetryAfter; - public const string Scheme = HN.Scheme; - public const string Server = HN.Server; - public const string SetCookie = HN.SetCookie; - public const string Status = HN.Status; - public const string StrictTransportSecurity = HN.StrictTransportSecurity; - public const string TE = HN.TE; - public const string Trailer = HN.Trailer; - public const string TransferEncoding = HN.TransferEncoding; - public const string Upgrade = HN.Upgrade; - public const string UserAgent = HN.UserAgent; - public const string Vary = HN.Vary; - public const string Via = HN.Via; - public const string Warning = HN.Warning; - public const string WebSocketSubProtocols = HN.WebSocketSubProtocols; - public const string WWWAuthenticate = HN.WWWAuthenticate; - } -} \ No newline at end of file