Ipv6 overlay #1176
Ipv6 overlay #1176
Conversation
| RawNetwork Ip = 2; | ||
| repeated RawNetwork Subnets = 3; |
There was a problem hiding this comment.
For a compatible version, I think we can just deprecate the old versions of these fields and make the new ones be optional. Then we could have the code prefer the new fields but fall back to the old ones if missing.
There was a problem hiding this comment.
I like this approach as well
There was a problem hiding this comment.
Current goals are to get the innards of ipv6 working, figure backwards/forwards compatibility will take the most time so I am ignoring it for now.
| } | ||
|
|
||
| vpnIp = vpnIp.Unmap() | ||
| vpnIp := remoteCert.Details.Ip.Addr() |
There was a problem hiding this comment.
If Ip is somehow blank/invalid, do we need a check for that here to mirror the old way?
| } | ||
|
|
||
| //TODO: whats a reasonable number of extension headers to attempt to parse? | ||
| //https://www.ietf.org/archive/id/draft-ietf-6man-eh-limits-00.html |
There was a problem hiding this comment.
If I'm reading this correctly, the last bullet of 3.2.2 says we can set a limit of 104 bytes for the "maximum length of the IPv6 header chain". Perhaps that's the way to go, until we observe problems?
There was a problem hiding this comment.
Cisco has a nice paper here that explains what they do with extension headers: https://www.cisco.com/en/US/technologies/tk648/tk872/technologies_white_paper0900aecd8054d37d.html
There was a problem hiding this comment.
https://datatracker.ietf.org/doc/html/rfc7872#page-3 This RFC measured the drop rate of IPv6 packets with any extension headers circa 2016, with dismal results. It seems that attempting to deal with them at all is considered noble.
| return nil | ||
|
|
||
| case layers.IPProtocolIPv6Fragment: | ||
| //TODO: can we determine the protocol? |
There was a problem hiding this comment.
Kind of? Subsequent fragment packets don't contain their TCP/UDP headers. We can keep a list of all the fragment Identification Numbers with their protocols and port numbers, but that could get out of hand quickly.
There was a problem hiding this comment.
Because ipv6 only allows the host to fragment, support for fragments at all could be less critical.
Another ipv6-specific problem is because of extension headers, I think it's possible for port numbers to be missing from the first packet of a fragmented message :(
| for i := 0; i < 24; i++ { | ||
| switch proto { | ||
| case layers.IPProtocolICMPv6: | ||
| //TODO: we need a new protocol in config language "icmpv6" |
There was a problem hiding this comment.
We may also want to think about dropping NDP and perhaps also RS/RA traffic, but allowing things like ping and error messages. NDP in particular should never occur on a nebula network (I think?)
| @@ -35,6 +35,12 @@ message RawNebulaCertificateDetails { | |||
| Curve curve = 100; | |||
| } | |||
|
|
|||
| message RawNetwork { | |||
There was a problem hiding this comment.
Because submessages are LEN types, it could make sense to use a string here, which would let us use netip.Addr.MarshalBinary.
I'm gonna ignore bits for now because it needs to be encoded somehow anyway
A submessage:
[LEN+id] [length] [ [hi id][hi varint] [lo id][lo varint] ] =
min 1 + 1 + 2(1+1) = 6, max 1 + 1 + 2(1+10) = 24
A bytes for address:
[LEN+id] [length] [16-byte address] =
1 + 1 + 16 = 18
A submessage can make sense if we think hi/lo of the v6 address are likely to be "small numbers", but if they're not, we could end up wasting bytes compared to a string-ier representation.
There was a problem hiding this comment.
I dont anticipate this to be the final types, fixed64 Hi and uint64 Lo is probably correct though. I agree avoiding the submessage is ideal as well. Avoiding protobuf would also be pretty cool :D
| uint32 counter = 3; | ||
| } | ||
|
|
||
| message Ip { |
There was a problem hiding this comment.
same comment as above, "what if we used bytes?"
There was a problem hiding this comment.
I dislike the unbounded nature of bytes.
As mentioned in the other comment, I had noodled on something along these lines
message NebulaMetaDetails {
fixed64 VpnIpHi = 1;
uint64 VpnIpLo = 2;
...
}
Stuff to noodle on at least
|
Does this mean ipv6 support is not being added anymore? |
|
Don't worry, It is still being actively developed. This was simply an early proof-of-concept PR that was used to get some feedback from other team members. |
This is a very early proof of concept. It is 100% not backward compatible with released nebula versions in its current form
Moving over to https://github.com/nbrownus/nebula/tree/ipv6-overlay-final for the time being
Closes #6