IPv4 to IPv6 Mapped Addresses not mapped to IPv4 when checking known proxies/networks

[SebastianC]

My scenario:

• web client goes through a load balancer
• load balancer directs to AWS/ECS host running a docker container
• docker container hosts a kestrel endpoint
• mvc controller in kestrel determines what the RemoteIPAddress is

My problem:

• When the docker container forwards the REST call to the Kestrel server, docker does an IPv4 to IPv6 conversion.
• When the Kestrel server attempts to evaluate the known proxies/networks, none of those evaluations handle the IPv4->IPv6 conversion so I always get that IP Address as the RemoteIPAddress (in my circumstance that's the converted form of the load balancer).

Proposed fix:

• In Microsoft.AspNetCore.HttpOverrides.ForwardedHeadersMiddleware function CheckKnownAddress, check if address.IsIPv4MappedToIPv6, and if it is, map it back to the IPv4 address before continuing. Pull request coming shortly.

This solution seems like it would also resolve another issue: #341 - "::ffff:127.0.0.1 not recognized as trusted local proxy"

[Tratcher]

One trouble with this is that people have been adding the IPv6 versions as a workaround. Changing it internally will break them unless you also change the KnownNetworks values for them.

[SebastianC]

I had originally considered updating another property to ForwardedHeadersOptions to indicate that this conversion should be turned "on", and default it to "off". Would that resolve your concern?

If it's a KnownProxy, I can see how adding the mapped-value would work, but i was unable to create a KnownNetwork that would be matched appropriately.

[SebastianC]

My apologies for the spam, but another option would be to check both variations:

        private bool CheckKnownAddress(IPAddress address)
        {
            if (address.IsIPv4MappedToIPv6)
            {
                var ipv4Address = address.MapToIPv4();
                if (_options.KnownProxies.Contains(ipv4Address))
                {
                    return true;
                }
                foreach (var network in _options.KnownNetworks)
                {
                    if (network.Contains(ipv4Address))
                    {
                        return true;
                    }
                }
            }
            if (_options.KnownProxies.Contains(address))
            {
                return true;
            }
            foreach (var network in _options.KnownNetworks)
            {
                if (network.Contains(address))
                {
                    return true;
                }
            }
            return false;
        }

[Tratcher]

Checking both varians is a good mitigation.

How's this for a simplification?

        private bool CheckKnownAddress(IPAddress address)
        {
            if (address.IsIPv4MappedToIPv6)
            {
                var ipv4Address = address.MapToIPv4();
                if (CheckKnownAddress(ipv4Address))
                {
                   return true;
                }
            }

            if (_options.KnownProxies.Contains(address))
            {
                return true;
            }

            foreach (var network in _options.KnownNetworks)
            {
                if (network.Contains(address))
                {
                    return true;
                }
            }
            return false;
        }

[SebastianC]

Better than mine. Will update the PR.