search

fthomas / refined

Refinement types for Scala
MIT License
1.71k stars 154 forks source link

PrivateNetwork type does not match the expected IP addresses #702

Open steinybot opened 4 years ago

steinybot commented 4 years ago

Was there a particular reason behind the RFCs that were chosen in https://github.com/fthomas/refined/pull/356?

I found that PrivateNetwork did not match what I expected it to.

The addresses in rfc5737 seem like they should never be routed to:

  1. Operational Implications

    Addresses within the TEST-NET-1, TEST-NET-2, and TEST-NET-3 blocks SHOULD NOT appear on the public Internet and are used without any coordination with IANA or an Internet registry [RFC2050]. Network operators SHOULD add these address blocks to the list of non- routeable address spaces, and if packet filters are deployed, then this address block SHOULD be added to packet filters.

    These blocks are not for local use, and the filters may be used in both local and public contexts.

There is also a bug in Rfc1918ClassBPrivateSpec. It should start at 172.16.0.0 not 172.15.0.0. https://tools.ietf.org/html/rfc1918#section-3

I'm by no means an expert but it seems like adding types for rfc6890 would be a better way to go.

Without digging into each RFC I suspect what most people would want is any address which is listed in rfc6890 as a valid destination IP address and maybe the loopback addresses.

Destination - A boolean value indicating whether an address from the allocated special-purpose address block is valid when used as the destination address of an IP datagram that transits two devices.

Thoughts?

improved-broccoli commented 4 years ago

IP adresses in Rfc5737 are reserved for documentation usage. Rfc6890 list address blocks for different purposes, except public routing, like benchmarking, documentation, private routing, ...

IMHO, address blocks not involved in private or public networks, if they come to be managed by this library, should be isolated in a separated object, something like SpecificPurposesIpAddressBlocks.

But the only use case I see is to exclude those specific-purpose IPs, you can already do that by combining both public and private types. So I don't think it worth the effort just for the completeness.