falahati / WindowsFirewallHelper

A class library to manage the Windows Firewall as well as adding your program to the Windows Firewall Exception list.
MIT License
274 stars 72 forks source link

Incorrect StartAddress and EndAddress properties when using small subnet masks #46

Closed hpshelton closed 3 years ago

hpshelton commented 3 years ago

When parsing CIDR address notation to generate a NetworkAddress type, the Address and SubnetMask properties store the intended values. However, the logic for StartAddress and EndAddress attempt to exclude the lowest (original network) IP and the highest (broadcast) IP. This works for large subnet masks, but is improperly executed when using a small mask to request a small number of IP addresses per this sample program:

var addresses = new string[] {
    "1.1.1.1/8",
    "1.1.1.1/16",
    "1.1.1.1/30",
    "1.1.1.1/31",
    "1.1.1.1/32"
};

foreach(string a in addresses)
{
    NetworkAddress na = NetworkAddress.Parse(a);
    Console.WriteLine($"CIDR: {a}\nAddress: {na.Address}\nSubnetMask: {na.SubnetMask}\nStartAddress: {na.StartAddress}\nEndAddress: {na.EndAddress}\n");
}
CIDR: 1.1.1.1/8
Address: 1.1.1.1
SubnetMask: 255.0.0.0
StartAddress: 1.0.0.1
EndAddress: 1.255.255.254

CIDR: 1.1.1.1/16
Address: 1.1.1.1
SubnetMask: 255.255.0.0
StartAddress: 1.1.0.1
EndAddress: 1.1.255.254

CIDR: 1.1.1.1/30
Address: 1.1.1.1
SubnetMask: 255.255.255.252
StartAddress: 1.1.1.1
EndAddress: 1.1.1.2

CIDR: 1.1.1.1/31
Address: 1.1.1.1
SubnetMask: 255.255.255.254
StartAddress: 1.1.1.1
EndAddress: 1.1.1.0

CIDR: 1.1.1.1/32
Address: 1.1.1.1
SubnetMask: 255.255.255.255
StartAddress: 1.1.1.1
EndAddress: 1.1.1.1

Note that StartAddress and EndAddress are reversed for the /31 case.

At a high level, these results are counter-intuitive to begin with as when requesting a /30, one would expect 4 IP addresses when only 2 are provided. When requesting a /31, intuitively 2 IP addresses would be provided. Seemingly returning the full range without truncation would be a better behavior than having both '/31' and '/30' returning 2 IP addresses.

hpshelton commented 3 years ago

StartAddress works as intended when commenting out the final mask addressBytes[addressBytes.Length - 1] |= 0x01 and EndAddress works as intended when commenting out addressBytes[addressBytes.Length - 1] &= 0xFE.

hpshelton commented 3 years ago

https://docs.netgate.com/pfsense/en/latest/network/cidr.html has helpful documentation about support for /31 and asserts that systems supporting RFC 3021 should return two useable host addresses.

Subnet Mask | CIDR Prefix | Total IP Addresses | Usable IP Addresses -- | -- | -- | -- 255.255.255.255 | /32 | 1 | 1 255.255.255.254 | /31 | 2 | 2* 255.255.255.252 | /30 | 4 | 2 255.255.255.248 | /29 | 8 | 6
falahati commented 3 years ago

thanks for reporting this, fixed and pushed to the repo. will be published in the next release which will be in a couple of hours