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

Include a target IP in handshake stage0, for fast rejection #673

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

brad-defined
Copy link
Collaborator

Nebula Stage0 handshakes carry the initiator's cert identity, but include no hint as to which peer identity they're attempting to connect to.
When connecting to a peer that shares the same CA, the peer will successfully complete the tunnel generation, and respond with the Handshake Stage1 packet, which includes the peer's certificate.

Only during the initiator's processing of this Stage1 handshake does the initiator discover that it's talking to the wrong peer.

This PR adds a TargetIp hint to the handshake stage 0 message. Old Nebula clients should ignore this new optional field, and new Nebula clients detect when a peer didn't include the field - so it's backwards and forwards compatible.

When the TargetIp is included, handshake_ix stage1 is updated to drop any handshake requests whose TargetIp doesn't match the current Nebula vpn IP.

I believe this handshake behavior will more smoothly handle error cases in which a Nebula client receives some stale info from a lighthouse, or where Nebula clients appear in LANs with overlapping LAN IP's.

I'm not quite sure how to test it, as the test would need to verify that a peer doesn't send a handshake packet upon receiving a handshake for an incorrect target ip. Maybe a counter for this event can be included and verified in an e2e test?

@jasikpark
Copy link
Collaborator

jasikpark commented Jun 27, 2022

Does this leak info in the case of getting an incorrect host? i.e. an attacker controlled host? I suppose overlay IP address isn't really protected info...

  • I suppose QUIC does the same..

@johnmaguire
Copy link
Collaborator

@jasikpark yes I believe so, somewhat mitigated by #441

@jasikpark
Copy link
Collaborator

cool, thx for the link; i'll have to read up on what PSK is and what it affords 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants