Fix : add the 172.16.0.0/12 private IP range + IPv6 support

[rducom]

• adds the 172.16.0.0/12 private IP range (RFC 1918)
• adds IPv6 support

[rducom]

Nick, you should update your AppVeyor CI ...

[CptRobby]

I like your changes to IsPrivateIP. I went to look at where it is being used though (which seems to only be the GetRemoteIP function) and saw that it looks like it'll never have an IPv6 value passed to it from there. The relevant section is the following:

// check if we were forwarded from a proxy
if (ipForwarded.HasValue())
{
    ipForwarded = IPv4Regex.Match(ipForwarded).Value;
    if (ipForwarded.HasValue() && !IsPrivateIP(ipForwarded))
        ip = ipForwarded;
}

The IPv4Regex would not match on an IPv6 address, so ipForwarded would be null and wouldn't get passed to IsPrivateIP.

I honestly feel like the Regex match isn't really needed, especially if it's going to be parsed into an IPAddress object. So I took what you wrote and refactored it to return null if it is either invalid or a private IP, otherwise it returns the ipString. This also allowed the logic of the original GetRemoteIP to be shortened to one line. Here's what I ended up with:

/// <summary>
/// returns a valid IP address only if it is not a private network IP  
/// http://en.wikipedia.org/wiki/Private_network
/// </summary>
private static string GetRemoteIP(string ipString)
{
    IPAddress ipAddress;
    if (ipString.IsNullOrEmpty() || !IPAddress.TryParse(ipString, out ipAddress)) return null;

    if (ipAddress.AddressFamily == AddressFamily.InterNetwork)
    {
        byte[] bytes = ipAddress.GetAddressBytes();
        switch (bytes[0])
        {
            case 10:
                return null;
            case 172:
                return bytes[1] < 32 && bytes[1] >= 16 ? null : ipString;
            case 192:
                return bytes[1] == 168 ? null : ipString;
            case 127:
                return bytes[1] == 0 && bytes[2] == 0 ? null : ipString;
            default:
                return ipString;
        }
    }

    if (ipAddress.AddressFamily == AddressFamily.InterNetworkV6)
    {
        // equivalent of 127.0.0.1 in IPv6
        if (ipString == "::1")
            return null;
        // The original IPv6 Site Local addresses (fec0::/10) are deprecated. Unfortunately IsIPv6SiteLocal only checks for the original deprecated version:
        if (ipAddress.IsIPv6SiteLocal)
            return null;
        // These days Unique Local Addresses (ULA) are used in place of Site Local. 
        // ULA has two variants: 
        //      fc00::/8 is not defined yet, but might be used in the future for internal-use addresses that are registered in a central place (ULA Central). 
        //      fd00::/8 is in use and does not have to registered anywhere.
        var firstWord = ipString.Split(new[] { ':' }, StringSplitOptions.RemoveEmptyEntries)[0];
        if (firstWord.Length >= 4 && firstWord.Substring(0, 2) == "fc")
            return null;
        if (firstWord.Length >= 4 && firstWord.Substring(0, 2) == "fd")
            return null;
        // Link local addresses (prefixed with fe80) are not routable
        if (firstWord == "fe80")
            return null;
        // Discard Prefix
        if (firstWord == "100")
            return null;
    }
    return ipString;
}

/// <summary>
/// retrieves the IP address of the current request -- handles proxies and private networks
/// </summary>
public static string GetRemoteIP(this NameValueCollection serverVariables)
{
    return GetRemoteIP(serverVariables["HTTP_X_FORWARDED_FOR"])
        .IsNullOrEmptyReturn(serverVariables["REMOTE_ADDR"], UnknownIP);
}

Feel free to use this however you like. :wink:

[NickCraver]

I'll try and look at this soon, but I should be straight: another overall solution is in store. I'm trying to get MiniProfiler v4 out the door as the first clean ASP.NET Core & ASP.NET (System.Web) library setup...and then StackExchange.Exceptional will follow the same pattern. Admittedly I haven't done much here because a critical component is missing from netstandard until 2.0: the global exception handler (see: https://github.com/dotnet/corefx/issues/6398).

As for IP handling - I have this done in a comprehensive way in Opserver, I'm just not sure how to best share the code at the moment (and I haven't spent much time on it). Code-based NuGet packages are in-flux and I'd hate for StackExchange.Exceptional to take a dependency for something so tiny. See IPNet, specifically: IPNet.IsPrivate here: https://github.com/opserver/Opserver/blob/overhaul/Opserver.Core/Data/IPNet.cs#L43

[rducom]

@CptRobby you'r right, regex isn't needed, I have removed this line @NickCraver I wasn't aware of IPNet, really nice code. I think it deserve to be in a nuget package. A separate project could also be an opportunity to add some xunit tests ?

[NickCraver]

@rducom yeah as much as I hate to have a package for such a tiny thing maybe it's the best answer. I'll work on getting it into a package. I just pushed a full migration over to the new .csproj system, layout, and cleanup in prep for netstandard when 2.0 is ready.

[NickCraver]

I'm going to close this out, as I've implemented the IPNet plan in the v2 branch (see #85 for discussion!) Thanks for all the help everyone!