diff --git a/.github/workflows/DevelopmentDPC.yml b/.github/workflows/DevelopmentDPC.yml index 26190ec..3f70424 100644 --- a/.github/workflows/DevelopmentDPC.yml +++ b/.github/workflows/DevelopmentDPC.yml @@ -7,9 +7,11 @@ name: Development DPC on: push: - branches: [ "Development" ] + branches-ignore: + - main pull_request: - branches: [ "Development" ] + branches-ignore: + - main jobs: build: diff --git a/DPCLibrary/Models/IPv4Address.cs b/DPCLibrary/Models/IPv4Address.cs index 82a23ff..6d4713f 100644 --- a/DPCLibrary/Models/IPv4Address.cs +++ b/DPCLibrary/Models/IPv4Address.cs @@ -40,7 +40,7 @@ public void LoadFromString(string address) throw new ArgumentException("IPAddress must not be null"); } - if (!Validate.IPv4(address)) + if (!Validate.IPv4Address(address)) { throw new ArgumentException("IPAddress must be a valid IPv4 Address"); } diff --git a/DPCLibrary/Models/IPv6Address.cs b/DPCLibrary/Models/IPv6Address.cs index 5fce82a..48a40d8 100644 --- a/DPCLibrary/Models/IPv6Address.cs +++ b/DPCLibrary/Models/IPv6Address.cs @@ -81,7 +81,7 @@ public void LoadFromString(string address) throw new ArgumentException("IPAddress must not be null"); } - if (!Validate.IPv6(address)) + if (!Validate.IPv6Address(address)) { throw new ArgumentException("IPAddress must be a valid IPv6 Address"); } diff --git a/DPCLibrary/Models/Route.cs b/DPCLibrary/Models/Route.cs index ca58ed8..5d2d982 100644 --- a/DPCLibrary/Models/Route.cs +++ b/DPCLibrary/Models/Route.cs @@ -81,12 +81,12 @@ public Route(XElement routeNode) Prefix = Convert.ToInt32(routeNode.XPathSelectElement("PrefixSize")?.Value, CultureInfo.InvariantCulture); } - if (Validate.IPv4(tempAddress)) + if (Validate.IPv4Address(tempAddress)) { Address = new IPv4Address(); Address.LoadFromString(tempAddress); } - else if (Validate.IPv6(tempAddress)) + else if (Validate.IPv6Address(tempAddress)) { Address = new IPv6Address(); Address.LoadFromString(tempAddress); diff --git a/DPCLibrary/Utils/IPUtils.cs b/DPCLibrary/Utils/IPUtils.cs index 0a5e2c5..0a83ff1 100644 --- a/DPCLibrary/Utils/IPUtils.cs +++ b/DPCLibrary/Utils/IPUtils.cs @@ -46,19 +46,19 @@ public static int GetIPCIDRSuffix(string address) } string[] splitAddress = address.Split('/'); - if (splitAddress.Length == 1 && Validate.IPv4(address)) + if (splitAddress.Length == 1 && Validate.IPv4EndpointAddress(address)) { //If there is no CIDR then assume it is a single IPv4 hence /32 return 32; } - if (splitAddress.Length == 1 && Validate.IPv6(address)) + if (splitAddress.Length == 1 && Validate.IPv6EndpointAddress(address)) { //If there is no CIDR then assume it is a single IPv6 hence /128 return 128; } else if (splitAddress.Length == 2) { - if (Validate.IPv4(splitAddress[1])) + if (Validate.IPv4Address(splitAddress[1])) { //Mask may be in the format 255.255.0.0 return GetCIDRFromNetMask(splitAddress[1]); diff --git a/DPCLibrary/Utils/VPNProfileCreator.cs b/DPCLibrary/Utils/VPNProfileCreator.cs index 11ce4d4..c372dd3 100644 --- a/DPCLibrary/Utils/VPNProfileCreator.cs +++ b/DPCLibrary/Utils/VPNProfileCreator.cs @@ -1838,7 +1838,7 @@ private static List GetOffice365ExcludeRoutes() { if (ipList.Contains(item)) continue; //Don't add IPv6 addresses as currently windows won't connect the profile if a client doesn't have an IPv6 address and there are IPv6 routes in the Route Table - if (Validate.IPv4(item) || Validate.IPv4CIDR(item)) + if (Validate.IPv4EndpointAddress(item) || Validate.IPv4CIDR(item)) { ipList.Add(item); } @@ -1887,7 +1887,7 @@ private static string GetDNSRoutes(ref Dictionary resolvedIPList } //Don't add IPv6 addresses as IPv6 Exclusions added to a machine without an IPv6 address breaks the tunnel completely //if (Validate.IPv4(item) || Validate.IPv6(item)) - if (Validate.IPv4(item.Key)) + if (Validate.IPv4EndpointAddress(item.Key)) { resolvedIPList.Add(item.Key, item.Value); } diff --git a/DPCLibrary/Utils/Validate.cs b/DPCLibrary/Utils/Validate.cs index 8bf3396..af097db 100644 --- a/DPCLibrary/Utils/Validate.cs +++ b/DPCLibrary/Utils/Validate.cs @@ -1,64 +1,40 @@ using System.IO; -using System.Text.RegularExpressions; +using System.Linq; using System.Net; using System.Net.Sockets; +using System.Text.RegularExpressions; namespace DPCLibrary.Utils { public static class Validate { + /// + /// Validates if the address is either a valid IPv4 or IPv6 endpoint address, or a valid IPv4 or IPv6 CIDR address + /// + /// The address to validate public static bool IPv4OrIPv6OrCIDR(string address) { - if (string.IsNullOrWhiteSpace(address)) - { - return false; - } - //If address is either ipv4 or ipv6 in either IP or CIDR format allow it - if (IPv4OrCIDR(address) || IPv6OrCIDR(address)) - { - return true; - } - else - { - return false; - } + return ValidateIPInternal(address, AddressFamily.InterNetwork, true, false) || + ValidateIPInternal(address, AddressFamily.InterNetworkV6, true, false); } + /// + /// Validates if the address is either a valid IPv6 endpoint address, or a valid IPv6 CIDR address + /// + /// The address to validate public static bool IPv6OrCIDR(string address) { - if (string.IsNullOrWhiteSpace(address)) - { - return false; - } - - string[] SplitCIDR = address.Split('/'); - - if (SplitCIDR.Length == 1) - { - return IPv6(address); - } - else - { - return IPv6CIDR(address); - } + return ValidateIPInternal(address, AddressFamily.InterNetworkV6, true, false); } + /// + /// Validates if the address is either a valid IPv4 endpoint address, or a valid IPv4 CIDR address + /// + /// + /// public static bool IPv4OrCIDR(string address) { - if (string.IsNullOrWhiteSpace(address)) - { - return false; - } - - string[] SplitCIDR = address.Split('/'); - if (SplitCIDR.Length == 1) - { - return IPv4(address); - } - else - { - return IPv4CIDR(address); - } + return ValidateIPInternal(address, AddressFamily.InterNetwork, true, false); } //Ensure that comments don't have --> which could really mess up the XML as it will cancel the comment and allow arbitrary XML injection @@ -85,7 +61,7 @@ public static bool IPAddressCommaList(string IPList) //Invalidate the entire record if there is 1 invalid IP foreach (string ip in ipSplitList) { - if (!IPv4(ip.Trim())) + if (!IPv4EndpointAddress(ip.Trim())) { return false; } @@ -94,125 +70,181 @@ public static bool IPAddressCommaList(string IPList) return true; } - public static bool IPv6(string address) + /// + /// Validates if the address is a valid IPv6 endpoint address + /// + /// + /// + public static bool IPv6EndpointAddress(string address) + { + return ValidateIPInternal(address, AddressFamily.InterNetworkV6, false, false); + } + + /// + /// Validates if the address is a valid IPv6 endpoint address or :: (unspecified address) + /// + /// + /// + public static bool IPv6Address(string address) + { + return ValidateIPInternal(address, AddressFamily.InterNetworkV6, false, true); + } + + /// + /// Validates if the address is a valid IPv4 endpoint address + /// + /// + /// + public static bool IPv4EndpointAddress(string address) + { + return ValidateIPInternal(address, AddressFamily.InterNetwork, false, false); + } + + /// + /// Validates if the address is a valid IPv4 endpoint address or 0.0.0.0 + /// + /// + /// + public static bool IPv4Address(string address) + { + return ValidateIPInternal(address, AddressFamily.InterNetwork, false, true); + } + + /// + /// Validates if the address is a valid IP address of the specified family, with options for CIDR and unspecified address + /// + /// The string representation of the address to validate + /// The expected address family (IPv4 or IPv6) + /// Specifies if CIDR notation is allowed + /// Specifies if the unspecified address (0.0.0.0 or ::) is allowed + /// + private static bool ValidateIPInternal(string address, AddressFamily addressFamily, bool allowCidr, bool allowUnspecifiedAddress) { if (string.IsNullOrWhiteSpace(address)) { return false; } - if (address == "::") + IPAddress anyAddress; + + if (addressFamily == AddressFamily.InterNetwork) { - return false; //reject simple IPv6 Zero + anyAddress = IPAddress.Any; } - - //Find and reject other ways of making IPv6 Zero - Match zeroResult = Regex.Match(address, "^s*(((0{1,4}:){7}(0{1,4}|:))|((0{1,4}:){6}|(:))|((0{1,4}:){5}(((:0]{1,4}){1,2})|:))|((0{1,4}:){4}(((:0{1,4}){1,3})|((:0{1,4})?:)|:))|((0{1,4}:){3}(((:0{1,4}){1,4})|((:0{1,4}){0,2}:)|:))|((0{1,4}:){2}(((:0{1,4}){1,5})|((:0{1,4}){0,3}:)|:))|((0{1,4}:){1}(((:0{1,4}){1,6})|((:0{1,4}){0,4}:)|:))|(:(((:0{1,4}){1,7})|((:0{1,4}){0,5}:)|:)))(%.+)?s*"); - if (zeroResult.Value == address) + else if (addressFamily == AddressFamily.InterNetworkV6) { - return false; + anyAddress = IPAddress.IPv6Any; } - - //Check that the IPAddress class accepts the value - if (!IPAddress.TryParse(address, out IPAddress potentialv6)) + else { return false; } - //Checks that the address is actually considered IPv6 and not IPv4 etc - return potentialv6.AddressFamily == AddressFamily.InterNetworkV6; - } + string[] addressParts = address.Split('/'); + bool isCidr = addressParts.Length == 2; + int prefix = -1; - public static bool IPv4(string address) - { - if (string.IsNullOrWhiteSpace(address)) + if (isCidr) { - return false; - } + if (!allowCidr) + { + return false; + } - if (address == "0.0.0.0") - { - return false; //reject all 0s as its only valid as a CIDR - } + if (!int.TryParse(addressParts[1], out prefix)) + { + // Unable to turn second part into int + return false; + } - Match result = Regex.Match(address, "\\b(25[0-5]|2[0-4]\\d|1\\d\\d|[1-9]?\\d)(\\.(25[0-5]|2[0-4]\\d|1\\d\\d|[1-9]?\\d)){3}\\b"); - return result.Value == address; - } + if (addressFamily == AddressFamily.InterNetwork && (prefix < 0 || prefix > 32)) + { + // Not a valid IPv4 CIDR prefix + return false; + } - public static bool IPv6CIDR(string address) - { - if (string.IsNullOrWhiteSpace(address)) - { - return false; + if (addressFamily == AddressFamily.InterNetworkV6 && (prefix < 0 || prefix > 128)) + { + // Not a valid IPv6 CIDR prefix + return false; + } } - string[] SplitCIDR = address.Split('/'); - if (SplitCIDR.Length != 2) + if (!IPAddress.TryParse(addressParts[0], out IPAddress ipAddress)) { + // Unable to turn first part into IP return false; } - if (!IPv6(SplitCIDR[0])) + if (ipAddress.AddressFamily != addressFamily) { - //Front part isn't a valid address + // Not the correct address family return false; } - if (!int.TryParse(SplitCIDR[1], out int CIDRVal)) + if (anyAddress.Equals(ipAddress) && !(allowUnspecifiedAddress || (isCidr && allowCidr))) { - //Unable to turn second part into int + // The unspecified address is only valid if allowAnyAddress is true or if it's a CIDR and allowCidr is true return false; } - if (CIDRVal > 128) + if (prefix == 0 && !anyAddress.Equals(ipAddress)) { + // A CIDR of 0 is only valid if the IP is the unspecified address return false; } - if (CIDRVal <= 0) + if (ipAddress.AddressFamily == AddressFamily.InterNetwork) { - return false; + // v4 specific validations + + if (address?.Count(c => c == '.') != 3) + { + return false; // There's a few unit tests for `10.0.0` which are valid shorthand IPs, but adding this check to ensure there are 4 octets for consistency with previous behaviour + } } return true; } - public static bool IPv4CIDR(string address) + /// + /// Validates if the address is a valid IPv6 CIDR address + /// + /// + /// + public static bool IPv6CIDR(string address) { if (string.IsNullOrWhiteSpace(address)) { return false; } - string[] SplitCIDR = address.Split('/'); - if (SplitCIDR.Length != 2) + if (!address.Contains("/")) { return false; } - if (!IPv4(SplitCIDR[0])) - { - //Front part isn't a valid address - return false; - } - - if (!int.TryParse(SplitCIDR[1], out int CIDRVal)) - { - //Unable to turn second part into int - return false; - } + return ValidateIPInternal(address, AddressFamily.InterNetworkV6, true, true); + } - if (CIDRVal > 32) + /// + /// Validates if the address is a valid IPv4 CIDR address + /// + /// + /// + public static bool IPv4CIDR(string address) + { + if (string.IsNullOrWhiteSpace(address)) { return false; } - if (CIDRVal <= 0) + if (!address.Contains("/")) { return false; } - return true; + return ValidateIPInternal(address, AddressFamily.InterNetwork, true, true); } public static string Thumbprint(string potentialThumbprint) @@ -386,8 +418,8 @@ public static bool PortList(string list) foreach (string PortElement in PortElements) { string element = PortElement.Trim(); - string[] portRange = element.Split(new char[]{ '-' }, 2); - foreach(string portRangeElement in portRange) + string[] portRange = element.Split(new char[] { '-' }, 2); + foreach (string portRangeElement in portRange) { string rangeElement = portRangeElement.Trim(); if (string.IsNullOrEmpty(rangeElement)) @@ -444,7 +476,7 @@ public static bool PackageId(string id) } Match result = Regex.Match(id, "^[a-zA-Z0-9.-]+[_][a-zA-Z0-9]+"); - if(result.Value == id) + if (result.Value == id) { return true; } @@ -461,4 +493,4 @@ public static bool PackageId(string id) } } } -} \ No newline at end of file +} diff --git a/DPCLibraryTests/ValidateTests.cs b/DPCLibraryTests/ValidateTests.cs index 076de47..ad20d54 100644 --- a/DPCLibraryTests/ValidateTests.cs +++ b/DPCLibraryTests/ValidateTests.cs @@ -29,9 +29,10 @@ public class ValidateTests [DataRow("1.1.1.1/-1")] [DataRow("1.1.1.1/33")] [DataRow("1.1.1.1/bob")] - [DataRow("2001:0db9::1/64")] - [DataRow("2001:0db9::1")] [DataRow("192.168..1/32")] + [DataRow("192.168.1.1/0")] + [DataRow("192.168.1.1/35")] + [DataRow("0.0.0.0")] public void InvalidIpv4ORCIDR(string IPv4Address) { bool result = Validate.IPv4OrCIDR(IPv4Address); @@ -50,6 +51,8 @@ public void InvalidIpv4ORCIDR(string IPv4Address) [DataRow("192.168.0.0/16")] [DataRow("192.168.1.1/32")] [DataRow("20.1.2.3/32")] + [DataRow("0.0.0.0/1")] + [DataRow("0.0.0.0/0")] public void ValidIpv4ORCIDR(string IPv4Address) { bool result = Validate.IPv4OrCIDR(IPv4Address); @@ -80,9 +83,9 @@ public void ValidIpv4ORCIDR(string IPv4Address) [DataRow("2001:0db9::1")] [DataRow("0.0.0.0/0")] [DataRow("0.0.0.0")] - public void InvalidIpv4(string IPv4Address) + public void InvalidIpv4EndpointAddress(string IPv4Address) { - bool result = Validate.IPv4(IPv4Address); + bool result = Validate.IPv4EndpointAddress(IPv4Address); Assert.IsFalse(result); } @@ -92,9 +95,22 @@ public void InvalidIpv4(string IPv4Address) [DataRow("192.168.5.4")] [DataRow("255.255.255.255")] [DataRow("20.1.2.3")] - public void ValidIpv4(string IPv4Address) + public void ValidIpv4EndpointAddress(string IPv4Address) { - bool result = Validate.IPv4(IPv4Address); + bool result = Validate.IPv4EndpointAddress(IPv4Address); + Assert.IsTrue(result); + } + + [DataTestMethod] + [DataRow("10.0.0.0")] + [DataRow("172.16.35.3")] + [DataRow("192.168.5.4")] + [DataRow("255.255.255.255")] + [DataRow("20.1.2.3")] + [DataRow("0.0.0.0")] + public void ValidIpv4Address(string IPv4Address) + { + bool result = Validate.IPv4Address(IPv4Address); Assert.IsTrue(result); } @@ -118,6 +134,7 @@ public void ValidIpv6CIDR(string IPv6Address) [DataRow("10.32.99.0/24")] [DataRow("192.168.0.0/16")] [DataRow("192.168.1.1/32")] + [DataRow("2001::1/129")] public void InvalidIpv6CIDR(string IPv6Address) { bool result = Validate.IPv6CIDR(IPv6Address); @@ -150,23 +167,16 @@ public void InvalidIpv6CIDR(string IPv6Address) [DataRow("255.255.255.255")] [DataRow("20.1.2.3")] [DataRow("0.0.0.0")] - [DataRow("10.0.0.0")] - [DataRow("172.16.35.3")] - [DataRow("192.168.5.4")] - [DataRow("255.255.255.255")] - [DataRow("20.1.2.3")] [DataRow("172.16.0.0/12")] - [DataRow("10.32.99.0/24")] [DataRow("192.168.0.0/16")] [DataRow("192.168.1.1/32")] [DataRow("20.1.2.3/32")] [DataRow("00:00:00:00:00:00:00:00")] - [DataRow("00:00:00:00:00:00:00:00/8")] [DataRow("0:0:0:0:0:0:0:0")] [DataRow("0:00:000:0000:0000:000:00:0")] - [DataRow("0:00:000:0000:0000:000:00:0/0")] [DataRow("0::0")] - [DataRow("::/0")] + [DataRow("2001:0db9::1/0")] + [DataRow("2001::1/129")] public void InvalidIpv6ORCIDR(string IPv4Address) { bool result = Validate.IPv6OrCIDR(IPv4Address); @@ -180,9 +190,12 @@ public void InvalidIpv6ORCIDR(string IPv4Address) [DataRow("2001:0db9::1/64")] [DataRow("2001:0db9::1/128")] [DataRow("2001:0db9:c9::/128")] - public void ValidIpv6ORCIDR(string IPv4Address) + [DataRow("::/0")] + [DataRow("0:00:000:0000:0000:000:00:0/0")] + [DataRow("00:00:00:00:00:00:00:00/8")] + public void ValidIpv6ORCIDR(string IPv6Address) { - bool result = Validate.IPv6OrCIDR(IPv4Address); + bool result = Validate.IPv6OrCIDR(IPv6Address); Assert.IsTrue(result); } @@ -203,6 +216,8 @@ public void ValidIpv6ORCIDR(string IPv4Address) [DataRow("192.168.0.0/16")] [DataRow("192.168.1.1/32")] [DataRow("20.1.2.3/32")] + [DataRow("0.0.0.0/1")] + [DataRow("0.0.0.0/0")] public void ValidIpv4ORIpv6ORCIDR(string IPv4Address) { bool result = Validate.IPv4OrIPv6OrCIDR(IPv4Address); @@ -244,9 +259,10 @@ public void ValidIpv4ORIpv6ORCIDR(string IPv4Address) [DataRow("0:00:000:0000:0000:000:00:0/0")] [DataRow("0::0")] [DataRow("::/0")] - public void InvalidIpv6(string IPv6Address) + [DataRow("2001:0db9::1/129")] + public void InvalidIpv6EndpointAddress(string IPv6Address) { - bool result = Validate.IPv6(IPv6Address); + bool result = Validate.IPv6EndpointAddress(IPv6Address); Assert.IsFalse(result); } @@ -256,12 +272,26 @@ public void InvalidIpv6(string IPv6Address) [DataRow("1:2:3:4:5:6:7:8")] [DataRow("1111:2222:3333:4444:5555:6666:7777:8888")] [DataRow("2001:0db9::ac11:c9")] - public void ValidIpv6(string IPv6Address) + public void ValidIpv6EndpointAddress(string IPv6Address) { - bool result = Validate.IPv6(IPv6Address); + bool result = Validate.IPv6EndpointAddress(IPv6Address); Assert.IsTrue(result); } + [DataTestMethod] + [DataRow("2001:0db9::1")] + [DataRow("2001::1")] + [DataRow("1:2:3:4:5:6:7:8")] + [DataRow("1111:2222:3333:4444:5555:6666:7777:8888")] + [DataRow("2001:0db9::ac11:c9")] + [DataRow("::")] + public void ValidIpv6Address(string IPv6Address) + { + bool result = Validate.IPv6Address(IPv6Address); + Assert.IsTrue(result); + } + + [DataTestMethod] [DataRow("")] //Empty [DataRow(null)] //null @@ -424,6 +454,8 @@ public void InvalidIpv4CIDR(string IPv4Address) [DataRow("192.168.0.0/16")] [DataRow("192.168.1.1/32")] [DataRow("20.1.2.3/32")] + [DataRow("0.0.0.0/1")] + [DataRow("0.0.0.0/0")] public void ValidIpv4CIDR(string IPv4Address) { bool result = Validate.IPv4CIDR(IPv4Address); diff --git a/ServiceIntergrationTests/UserProfileTests.cs b/ServiceIntergrationTests/UserProfileTests.cs index 9fd8cd1..cdde26e 100644 --- a/ServiceIntergrationTests/UserProfileTests.cs +++ b/ServiceIntergrationTests/UserProfileTests.cs @@ -1938,7 +1938,7 @@ public void BasicUserProfileWithIPv6Routes(ProfileType profileType) [DataTestMethod] [DataRow(ProfileType.User)] [DataRow(ProfileType.UserBackup)] - public void BasicUserProfileWithSillyRoutes(ProfileType profileType) + public void BasicUserProfileWithDefaultRoutes(ProfileType profileType) { string profileName = TestContext.TestName; @@ -1964,7 +1964,7 @@ public void BasicUserProfileWithSillyRoutes(ProfileType profileType) TestContext.WriteLine(profile.GetValidationFailures()); TestContext.WriteLine(profile.GetValidationWarnings()); Assert.IsFalse(profile.ValidateFailed()); - Assert.IsTrue(profile.ValidateWarnings()); + Assert.IsFalse(profile.ValidateWarnings()); sharedData.AddProfileUpdate(profile.GetProfileUpdate());