Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cert v2 + tun changes for Linux #1224

Merged
merged 17 commits into from
Sep 21, 2024

Conversation

JackDoanRivian
Copy link
Contributor

No description provided.

overlay/tun_linux.go Outdated Show resolved Hide resolved
cert/cert_v2.go Outdated Show resolved Hide resolved
overlay/tun_linux.go Outdated Show resolved Hide resolved
@@ -52,11 +52,13 @@ func newWaterTun(c *config.C, l *logrus.Logger, cidr netip.Prefix, _ bool) (*wat

func (t *waterTun) Activate() error {
var err error
//TODO multi-ip support
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a very ugly hack

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can probably safely remove the fallback windows tap driver support now.

@@ -1170,6 +1170,9 @@ func (lhh *LightHouseHandler) handleHostUpdateNotification(n *NebulaMeta, vpnAdd
useVersion = 2
}

//todo hosts with only v2 certs cannot provide their ipv6 addr when contacting the lighthouse via v4?
//todo why do we care about the vpnip in the packet? We know where it came from, right?
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its a great question and I have it in my internals notes as well as ditching the V4AddrPort in v2 protocol stuff.

@@ -1170,6 +1170,9 @@ func (lhh *LightHouseHandler) handleHostUpdateNotification(n *NebulaMeta, vpnAdd
useVersion = 2
}

//todo hosts with only v2 certs cannot provide their ipv6 addr when contacting the lighthouse via v4?
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They should default to protocol v2, are you seeing something I'm missing?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

protocol v2 is working, this is actually a multi-IP issue in a funny hat.

If you have a LH at 10.0.0.1 and you contact it via 10.0.0.2 (who is also fc00:02), because we try to use the IP in the message instead of the hostmap, the LH will never learn the underlay IP for fc00:02, unless you also contact it via overlay-ipv6.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unless of course, I have something backwards

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah gotcha. Yeah this and the hostmap don't understand follow on addresses yet. We need a final loop on the updates to point all subsequent addresses to the primary address. I have some of this code staged but its not ready just yet.

cert/cert_v2.go Outdated Show resolved Hide resolved
if err != nil {
return fmt.Errorf("failed to get tun device link: %s", err)
// Run the interface
ifrf.Flags = ifrf.Flags | unix.IFF_UP | unix.IFF_RUNNING
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's likely entirely academic but we had waited until everything else was done before RUNNING the device to avoid packet loss from not yet fully configured devices

Copy link
Collaborator

@nbrownus nbrownus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Happy to merge this into the branch now if you are ready

@JackDoanRivian
Copy link
Contributor Author

Ship it!

@nbrownus nbrownus merged commit 28cd257 into slackhq:cert-v2 Sep 21, 2024
9 checks passed
nbrownus pushed a commit that referenced this pull request Oct 8, 2024
nbrownus pushed a commit that referenced this pull request Oct 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants