From 1cc7572445789c703bb9456224e3f768083c4f65 Mon Sep 17 00:00:00 2001 From: Shadowghost Date: Wed, 15 Feb 2023 11:31:47 +0100 Subject: [PATCH] Apply review suggestions --- Jellyfin.Networking/Manager/NetworkManager.cs | 372 +++++++++--------- MediaBrowser.Common/Net/IPData.cs | 14 +- 2 files changed, 206 insertions(+), 180 deletions(-) diff --git a/Jellyfin.Networking/Manager/NetworkManager.cs b/Jellyfin.Networking/Manager/NetworkManager.cs index f4bd4e7662..ca9e9274f3 100644 --- a/Jellyfin.Networking/Manager/NetworkManager.cs +++ b/Jellyfin.Networking/Manager/NetworkManager.cs @@ -263,6 +263,7 @@ namespace Jellyfin.Networking.Manager _logger.LogError(ex, "Error obtaining interfaces."); } + // If no interfaces are found, fallback to loopback interfaces. if (_interfaces.Count == 0) { _logger.LogWarning("No interface information available. Using loopback interface(s)."); @@ -278,8 +279,8 @@ namespace Jellyfin.Networking.Manager } } - _logger.LogDebug("Discovered {0} interfaces.", _interfaces.Count); - _logger.LogDebug("Interfaces addresses: {0}", _interfaces.OrderByDescending(s => s.AddressFamily == AddressFamily.InterNetwork).Select(s => s.Address.ToString())); + _logger.LogDebug("Discovered {NumberOfInterfaces} interfaces.", _interfaces.Count); + _logger.LogDebug("Interfaces addresses: {Addresses}", _interfaces.OrderByDescending(s => s.AddressFamily == AddressFamily.InterNetwork).Select(s => s.Address.ToString())); } } @@ -293,14 +294,21 @@ namespace Jellyfin.Networking.Manager _logger.LogDebug("Refreshing LAN information."); // Get configuration options - string[] subnets = config.LocalNetworkSubnets; + var subnets = config.LocalNetworkSubnets; - _ = NetworkExtensions.TryParseToSubnets(subnets, out _lanSubnets, false); - _ = NetworkExtensions.TryParseToSubnets(subnets, out _excludedSubnets, true); + if (!NetworkExtensions.TryParseToSubnets(subnets, out _lanSubnets, false)) + { + _lanSubnets.Clear(); + } + + if (!NetworkExtensions.TryParseToSubnets(subnets, out _excludedSubnets, true)) + { + _excludedSubnets.Clear(); + } + // If no LAN addresses are specified, all private subnets and Loopback are deemed to be the LAN if (_lanSubnets.Count == 0) { - // If no LAN addresses are specified, all private subnets and Loopback are deemed to be the LAN _logger.LogDebug("Using LAN interface addresses as user provided no LAN details."); if (IsIpv6Enabled) @@ -334,15 +342,15 @@ namespace Jellyfin.Networking.Manager { // Respect explicit bind addresses var localNetworkAddresses = config.LocalNetworkAddresses; - if (localNetworkAddresses.Length > 0 && !string.IsNullOrWhiteSpace(localNetworkAddresses.First())) + if (localNetworkAddresses.Length > 0 && !string.IsNullOrWhiteSpace(localNetworkAddresses[0])) { var bindAddresses = localNetworkAddresses.Select(p => NetworkExtensions.TryParseToSubnet(p, out var network) ? network.Prefix : (_interfaces.Where(x => x.Name.Equals(p, StringComparison.OrdinalIgnoreCase)) .Select(x => x.Address) .FirstOrDefault() ?? IPAddress.None)) - .ToList(); - bindAddresses.RemoveAll(x => x == IPAddress.None); + .Where(x => x != IPAddress.None) + .ToHashSet(); _interfaces = _interfaces.Where(x => bindAddresses.Contains(x.Address)).ToList(); if (bindAddresses.Contains(IPAddress.Loopback)) @@ -401,11 +409,15 @@ namespace Jellyfin.Networking.Manager if (remoteIPFilter.Any() && !string.IsNullOrWhiteSpace(remoteIPFilter.First())) { // Parse all IPs with netmask to a subnet - _ = NetworkExtensions.TryParseToSubnets(remoteIPFilter.Where(x => x.Contains('/', StringComparison.OrdinalIgnoreCase)).ToArray(), out _remoteAddressFilter, false); + var remoteFilteredSubnets = remoteIPFilter.Where(x => x.Contains('/', StringComparison.OrdinalIgnoreCase)).ToArray(); + if (!NetworkExtensions.TryParseToSubnets(remoteFilteredSubnets, out _remoteAddressFilter, false)) + { + _remoteAddressFilter.Clear(); + } // Parse everything else as an IP and construct subnet with a single IP - var ips = remoteIPFilter.Where(x => !x.Contains('/', StringComparison.OrdinalIgnoreCase)); - foreach (var ip in ips) + var remoteFilteredIPs = remoteIPFilter.Where(x => !x.Contains('/', StringComparison.OrdinalIgnoreCase)); + foreach (var ip in remoteFilteredIPs) { if (IPAddress.TryParse(ip, out var ipp)) { @@ -436,45 +448,44 @@ namespace Jellyfin.Networking.Manager if (parts.Length != 2) { _logger.LogError("Unable to parse bind override: {Entry}", entry); + return; } - else + + var replacement = parts[1].Trim(); + var identifier = parts[0]; + if (string.Equals(identifier, "all", StringComparison.OrdinalIgnoreCase)) { - var replacement = parts[1].Trim(); - var identifier = parts[0]; - if (string.Equals(identifier, "all", StringComparison.OrdinalIgnoreCase)) - { - _publishedServerUrls[new IPData(IPAddress.Broadcast, null)] = replacement; - } - else if (string.Equals(identifier, "external", StringComparison.OrdinalIgnoreCase)) - { - _publishedServerUrls[new IPData(IPAddress.Any, new IPNetwork(IPAddress.Any, 0))] = replacement; - _publishedServerUrls[new IPData(IPAddress.IPv6Any, new IPNetwork(IPAddress.IPv6Any, 0))] = replacement; - } - else if (string.Equals(identifier, "internal", StringComparison.OrdinalIgnoreCase)) - { - foreach (var lan in _lanSubnets) - { - var lanPrefix = lan.Prefix; - _publishedServerUrls[new IPData(lanPrefix, new IPNetwork(lanPrefix, lan.PrefixLength))] = replacement; - } - } - else if (NetworkExtensions.TryParseToSubnet(identifier, out var result) && result != null) - { - var data = new IPData(result.Prefix, result); - _publishedServerUrls[data] = replacement; - } - else if (TryParseInterface(identifier, out var ifaces)) + _publishedServerUrls[new IPData(IPAddress.Broadcast, null)] = replacement; + } + else if (string.Equals(identifier, "external", StringComparison.OrdinalIgnoreCase)) + { + _publishedServerUrls[new IPData(IPAddress.Any, new IPNetwork(IPAddress.Any, 0))] = replacement; + _publishedServerUrls[new IPData(IPAddress.IPv6Any, new IPNetwork(IPAddress.IPv6Any, 0))] = replacement; + } + else if (string.Equals(identifier, "internal", StringComparison.OrdinalIgnoreCase)) + { + foreach (var lan in _lanSubnets) { - foreach (var iface in ifaces) - { - _publishedServerUrls[iface] = replacement; - } + var lanPrefix = lan.Prefix; + _publishedServerUrls[new IPData(lanPrefix, new IPNetwork(lanPrefix, lan.PrefixLength))] = replacement; } - else + } + else if (NetworkExtensions.TryParseToSubnet(identifier, out var result) && result != null) + { + var data = new IPData(result.Prefix, result); + _publishedServerUrls[data] = replacement; + } + else if (TryParseInterface(identifier, out var ifaces)) + { + foreach (var iface in ifaces) { - _logger.LogError("Unable to parse bind override: {Entry}", entry); + _publishedServerUrls[iface] = replacement; } } + else + { + _logger.LogError("Unable to parse bind override: {Entry}", entry); + } } } } @@ -556,31 +567,28 @@ namespace Jellyfin.Networking.Manager public bool TryParseInterface(string intf, out List result) { result = new List(); - if (string.IsNullOrEmpty(intf)) + if (string.IsNullOrEmpty(intf) || _interfaces is null) { return false; } - if (_interfaces != null) + // Match all interfaces starting with names starting with token + var matchedInterfaces = _interfaces.Where(s => s.Name.Equals(intf, StringComparison.OrdinalIgnoreCase)).OrderBy(x => x.Index).ToList(); + if (matchedInterfaces.Count > 0) { - // Match all interfaces starting with names starting with token - var matchedInterfaces = _interfaces.Where(s => s.Name.Equals(intf, StringComparison.OrdinalIgnoreCase)).OrderBy(x => x.Index).ToList(); - if (matchedInterfaces.Any()) - { - _logger.LogInformation("Interface {Token} used in settings. Using its interface addresses.", intf); + _logger.LogInformation("Interface {Token} used in settings. Using its interface addresses.", intf); - // Use interface IP instead of name - foreach (var iface in matchedInterfaces) + // Use interface IP instead of name + foreach (var iface in matchedInterfaces) + { + if ((IsIpv4Enabled && iface.Address.AddressFamily == AddressFamily.InterNetwork) + || (IsIpv6Enabled && iface.Address.AddressFamily == AddressFamily.InterNetworkV6)) { - if ((IsIpv4Enabled && iface.Address.AddressFamily == AddressFamily.InterNetwork) - || (IsIpv6Enabled && iface.Address.AddressFamily == AddressFamily.InterNetworkV6)) - { - result.Add(iface); - } + result.Add(iface); } - - return true; } + + return true; } return false; @@ -626,6 +634,11 @@ namespace Jellyfin.Networking.Manager /// public IReadOnlyList GetLoopbacks() { + if (!(IsIpv4Enabled && IsIpv4Enabled)) + { + return Array.Empty(); + } + var loopbackNetworks = new List(); if (IsIpv4Enabled) { @@ -643,62 +656,64 @@ namespace Jellyfin.Networking.Manager /// public IReadOnlyList GetAllBindInterfaces(bool individualInterfaces = false) { - if (_interfaces.Count == 0) + if (_interfaces.Count != 0) { - // No bind address and no exclusions, so listen on all interfaces. - var result = new List(); - - if (individualInterfaces) - { - foreach (var iface in _interfaces) - { - result.Add(iface); - } + return _interfaces; + } - return result; - } + // No bind address and no exclusions, so listen on all interfaces. + var result = new List(); - if (IsIpv4Enabled && IsIpv6Enabled) - { - // Kestrel source code shows it uses Sockets.DualMode - so this also covers IPAddress.Any by default - result.Add(new IPData(IPAddress.IPv6Any, new IPNetwork(IPAddress.IPv6Any, 0))); - } - else if (IsIpv4Enabled) + if (individualInterfaces) + { + foreach (var iface in _interfaces) { - result.Add(new IPData(IPAddress.Any, new IPNetwork(IPAddress.Any, 0))); + result.Add(iface); } - else if (IsIpv6Enabled) + + return result; + } + + if (IsIpv4Enabled && IsIpv6Enabled) + { + // Kestrel source code shows it uses Sockets.DualMode - so this also covers IPAddress.Any by default + result.Add(new IPData(IPAddress.IPv6Any, new IPNetwork(IPAddress.IPv6Any, 0))); + } + else if (IsIpv4Enabled) + { + result.Add(new IPData(IPAddress.Any, new IPNetwork(IPAddress.Any, 0))); + } + else if (IsIpv6Enabled) + { + // Cannot use IPv6Any as Kestrel will bind to IPv4 addresses too. + foreach (var iface in _interfaces) { - // Cannot use IPv6Any as Kestrel will bind to IPv4 addresses too. - foreach (var iface in _interfaces) + if (iface.AddressFamily == AddressFamily.InterNetworkV6) { - if (iface.AddressFamily == AddressFamily.InterNetworkV6) - { - result.Add(iface); - } + result.Add(iface); } } - - return result; } - return _interfaces; + return result; } /// public string GetBindInterface(string source, out int? port) { - _ = NetworkExtensions.TryParseHost(source, out var address, IsIpv4Enabled, IsIpv6Enabled); - var result = GetBindAddress(address.FirstOrDefault(), out port); + if (!NetworkExtensions.TryParseHost(source, out var addresses, IsIpv4Enabled, IsIpv6Enabled)) + { + addresses = Array.Empty(); + } + + var result = GetBindAddress(addresses.FirstOrDefault(), out port); return result; } /// public string GetBindInterface(HttpRequest source, out int? port) { - string result; - _ = NetworkExtensions.TryParseHost(source.Host.Host, out var addresses, IsIpv4Enabled, IsIpv6Enabled); - result = GetBindAddress(addresses.FirstOrDefault(), out port); + var result = GetBindInterface(source.Host.Host, out port); port ??= source.Host.Port; return result; @@ -749,36 +764,36 @@ namespace Jellyfin.Networking.Manager .ThenBy(x => x.Index) .ToList(); - if (availableInterfaces.Any()) + if (availableInterfaces.Count > 0) { - if (source != null) + if (source is null) + { + result = NetworkExtensions.FormatIpString(availableInterfaces.First().Address); + _logger.LogDebug("{Source}: Using first internal interface as bind address: {Result}", source, result); + return result; + } + + foreach (var intf in availableInterfaces) { - foreach (var intf in availableInterfaces) + if (intf.Address.Equals(source)) { - if (intf.Address.Equals(source)) - { - result = NetworkExtensions.FormatIpString(intf.Address); - _logger.LogDebug("{Source}: Found matching interface to use as bind address: {Result}", source, result); - return result; - } + result = NetworkExtensions.FormatIpString(intf.Address); + _logger.LogDebug("{Source}: Found matching interface to use as bind address: {Result}", source, result); + return result; } + } - // Does the request originate in one of the interface subnets? - // (For systems with multiple internal network cards, and multiple subnets) - foreach (var intf in availableInterfaces) + // Does the request originate in one of the interface subnets? + // (For systems with multiple internal network cards, and multiple subnets) + foreach (var intf in availableInterfaces) + { + if (intf.Subnet.Contains(source)) { - if (intf.Subnet.Contains(source)) - { - result = NetworkExtensions.FormatIpString(intf.Address); - _logger.LogDebug("{Source}: Found internal interface with matching subnet, using it as bind address: {Result}", source, result); - return result; - } + result = NetworkExtensions.FormatIpString(intf.Address); + _logger.LogDebug("{Source}: Found internal interface with matching subnet, using it as bind address: {Result}", source, result); + return result; } } - - result = NetworkExtensions.FormatIpString(availableInterfaces.First().Address); - _logger.LogDebug("{Source}: Using first internal interface as bind address: {Result}", source, result); - return result; } // There isn't any others, so we'll use the loopback. @@ -810,6 +825,11 @@ namespace Jellyfin.Networking.Manager foreach (var ept in addresses) { match |= IPAddress.IsLoopback(ept) || (_lanSubnets.Any(x => x.Contains(ept)) && !_excludedSubnets.Any(x => x.Contains(ept))); + + if (match) + { + break; + } } return match; @@ -824,15 +844,15 @@ namespace Jellyfin.Networking.Manager ArgumentNullException.ThrowIfNull(address); // See conversation at https://github.com/jellyfin/jellyfin/pull/3515. - if (TrustAllIpv6Interfaces && address.AddressFamily == AddressFamily.InterNetworkV6) + if ((TrustAllIpv6Interfaces && address.AddressFamily == AddressFamily.InterNetworkV6) + || address.Equals(IPAddress.Loopback) + || address.Equals(IPAddress.IPv6Loopback)) { return true; } // As private addresses can be redefined by Configuration.LocalNetworkAddresses - var match = CheckIfLanAndNotExcluded(address); - - return address.Equals(IPAddress.Loopback) || address.Equals(IPAddress.IPv6Loopback) || match; + return CheckIfLanAndNotExcluded(address); } private bool CheckIfLanAndNotExcluded(IPAddress address) @@ -865,20 +885,16 @@ namespace Jellyfin.Networking.Manager int? port = null; var validPublishedServerUrls = _publishedServerUrls.Where(x => x.Key.Address.Equals(IPAddress.Any) - || x.Key.Address.Equals(IPAddress.IPv6Any) - || x.Key.Subnet.Contains(source)) - .GroupBy(x => x.Key) - .Select(x => x.First()) - .OrderBy(x => x.Key.Address.Equals(IPAddress.Any) - || x.Key.Address.Equals(IPAddress.IPv6Any)) - .ToList(); + || x.Key.Address.Equals(IPAddress.IPv6Any) + || x.Key.Subnet.Contains(source)) + .DistinctBy(x => x.Key) + .OrderBy(x => x.Key.Address.Equals(IPAddress.Any) + || x.Key.Address.Equals(IPAddress.IPv6Any)) + .ToList(); // Check for user override. foreach (var data in validPublishedServerUrls) { - // Get address interface. - var intf = _interfaces.OrderBy(x => x.Index).FirstOrDefault(x => data.Key.Subnet.Contains(x.Address)); - if (isInExternalSubnet && (data.Key.Address.Equals(IPAddress.Any) || data.Key.Address.Equals(IPAddress.IPv6Any))) { // External. @@ -886,6 +902,9 @@ namespace Jellyfin.Networking.Manager break; } + // Get address interface. + var intf = _interfaces.OrderBy(x => x.Index).FirstOrDefault(x => data.Key.Subnet.Contains(x.Address)); + if (intf?.Address != null) { // Match IP address. @@ -896,7 +915,7 @@ namespace Jellyfin.Networking.Manager if (string.IsNullOrEmpty(bindPreference)) { - _logger.LogDebug("{Source}: No matching bind address override found.", source); + _logger.LogDebug("{Source}: No matching bind address override found", source); return false; } @@ -941,40 +960,22 @@ namespace Jellyfin.Networking.Manager count = 0; } - if (count > 0) + if (count == 0) + { + return false; + } + + IPAddress? bindAddress = null; + if (isInExternalSubnet) { - IPAddress? bindAddress = null; var externalInterfaces = _interfaces.Where(x => !IsInLocalNetwork(x.Address)) .OrderBy(x => x.Index) .ToList(); - - if (isInExternalSubnet) - { - if (externalInterfaces.Any()) - { - // Check to see if any of the external bind interfaces are in the same subnet as the source. - // If none exists, this will select the first external interface if there is one. - bindAddress = externalInterfaces - .OrderByDescending(x => x.Subnet.Contains(source)) - .ThenBy(x => x.Index) - .Select(x => x.Address) - .FirstOrDefault(); - - if (bindAddress != null) - { - result = NetworkExtensions.FormatIpString(bindAddress); - _logger.LogDebug("{Source}: External request received, matching external bind address found: {Result}", source, result); - return true; - } - } - - _logger.LogWarning("{Source}: External request received, no matching external bind address found, trying internal addresses.", source); - } - else + if (externalInterfaces.Count > 0) { - // Check to see if any of the internal bind interfaces are in the same subnet as the source. - // If none exists, this will select the first internal interface if there is one. - bindAddress = _interfaces.Where(x => IsInLocalNetwork(x.Address)) + // Check to see if any of the external bind interfaces are in the same subnet as the source. + // If none exists, this will select the first external interface if there is one. + bindAddress = externalInterfaces .OrderByDescending(x => x.Subnet.Contains(source)) .ThenBy(x => x.Index) .Select(x => x.Address) @@ -983,10 +984,29 @@ namespace Jellyfin.Networking.Manager if (bindAddress != null) { result = NetworkExtensions.FormatIpString(bindAddress); - _logger.LogDebug("{Source}: Internal request received, matching internal bind address found: {Result}", source, result); + _logger.LogDebug("{Source}: External request received, matching external bind address found: {Result}", source, result); return true; } } + + _logger.LogWarning("{Source}: External request received, no matching external bind address found, trying internal addresses.", source); + } + else + { + // Check to see if any of the internal bind interfaces are in the same subnet as the source. + // If none exists, this will select the first internal interface if there is one. + bindAddress = _interfaces.Where(x => IsInLocalNetwork(x.Address)) + .OrderByDescending(x => x.Subnet.Contains(source)) + .ThenBy(x => x.Index) + .Select(x => x.Address) + .FirstOrDefault(); + + if (bindAddress != null) + { + result = NetworkExtensions.FormatIpString(bindAddress); + _logger.LogDebug("{Source}: Internal request received, matching internal bind address found: {Result}", source, result); + return true; + } } return false; @@ -1000,16 +1020,21 @@ namespace Jellyfin.Networking.Manager /// true if a match is found, false otherwise. private bool MatchesExternalInterface(IPAddress source, out string result) { - result = string.Empty; - // Get the first WAN interface address that isn't a loopback. - var extResult = _interfaces.Where(p => !IsInLocalNetwork(p.Address)).OrderBy(x => x.Index); + // Get the first external interface address that isn't a loopback. + var extResult = _interfaces.Where(p => !IsInLocalNetwork(p.Address)).OrderBy(x => x.Index).ToArray(); + + // No external interface found + if (extResult.Length == 0) + { + result = string.Empty; + _logger.LogWarning("{Source}: External request received, but no external interface found. Need to route through internal network.", source); + return false; + } - IPAddress? hasResult = null; // Does the request originate in one of the interface subnets? - // (For systems with multiple internal network cards, and multiple subnets) + // (For systems with multiple network cards and/or multiple subnets) foreach (var intf in extResult) { - hasResult ??= intf.Address; if (!IsInLocalNetwork(intf.Address) && intf.Subnet.Contains(source)) { result = NetworkExtensions.FormatIpString(intf.Address); @@ -1018,15 +1043,10 @@ namespace Jellyfin.Networking.Manager } } - if (hasResult != null) - { - result = NetworkExtensions.FormatIpString(hasResult); - _logger.LogDebug("{Source}: Using first external interface as bind address: {Result}", source, result); - return true; - } - - _logger.LogWarning("{Source}: External request received, but no external interface found. Need to route through internal network.", source); - return false; + // Fallback to first external interface. + result = NetworkExtensions.FormatIpString(extResult.First().Address); + _logger.LogDebug("{Source}: Using first external interface as bind address: {Result}", source, result); + return true; } } } diff --git a/MediaBrowser.Common/Net/IPData.cs b/MediaBrowser.Common/Net/IPData.cs index 384efe8f68..05842632c8 100644 --- a/MediaBrowser.Common/Net/IPData.cs +++ b/MediaBrowser.Common/Net/IPData.cs @@ -59,10 +59,16 @@ namespace MediaBrowser.Common.Net { get { - return Address.Equals(IPAddress.None) - ? (Subnet.Prefix.AddressFamily.Equals(IPAddress.None) - ? AddressFamily.Unspecified : Subnet.Prefix.AddressFamily) - : Address.AddressFamily; + if (Address.Equals(IPAddress.None)) + { + return Subnet.Prefix.AddressFamily.Equals(IPAddress.None) + ? AddressFamily.Unspecified + : Subnet.Prefix.AddressFamily; + } + else + { + return Address.AddressFamily; + } } } }