-
Notifications
You must be signed in to change notification settings - Fork 28
device: add API for on-demand configuration of peers #47
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
base: tailscale
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,6 +8,8 @@ package device | |
| import ( | ||
| "container/list" | ||
| "errors" | ||
| "net/netip" | ||
| "slices" | ||
| "sync" | ||
| "sync/atomic" | ||
| "time" | ||
|
|
@@ -25,6 +27,12 @@ type Peer struct { | |
| rxBytes atomic.Uint64 // bytes received from peer | ||
| lastHandshakeNano atomic.Int64 // nano seconds since epoch | ||
|
|
||
| // deleteOnIdle indicates whether the peer should be deleted when idle | ||
| // because it was auto-created via a Device.PeerLookupFunc. | ||
| // | ||
| // This field should only be set once, before the peer is started. | ||
| deleteOnIdle bool | ||
|
|
||
| endpoint struct { | ||
| sync.Mutex | ||
| val conn.Endpoint | ||
|
|
@@ -44,7 +52,9 @@ type Peer struct { | |
| } | ||
|
|
||
| state struct { | ||
| sync.Mutex // protects against concurrent Start/Stop | ||
| sync.Mutex // protects against concurrent Start/Stop, and fields below | ||
|
|
||
| allowedIPs []netip.Prefix | ||
| } | ||
|
|
||
| queue struct { | ||
|
|
@@ -87,7 +97,7 @@ func (device *Device) NewPeer(pk NoisePublicKey) (*Peer, error) { | |
| // map public key | ||
| _, ok := device.peers.keyMap[pk] | ||
| if ok { | ||
| return nil, errors.New("adding existing peer") | ||
| return nil, errAddExistingPeer | ||
| } | ||
|
|
||
| // pre-compute DH | ||
|
|
@@ -113,6 +123,18 @@ func (device *Device) NewPeer(pk NoisePublicKey) (*Peer, error) { | |
| return peer, nil | ||
| } | ||
|
|
||
| // SetAllowedIPs sets the allowed IP prefixes for this peer. | ||
| func (p *Peer) SetAllowedIPs(allowedIPs []netip.Prefix) { | ||
| p.state.Lock() | ||
| defer p.state.Unlock() | ||
|
|
||
| if slices.Equal(p.state.allowedIPs, allowedIPs) { | ||
| return | ||
| } | ||
| p.device.allowedips.SetPeerPrefixes(p, allowedIPs) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the clone and comment below seem sensible, but if that implies a dependence or leads to a future dependence, SetPeerPrefixes doesn't document the same behavior that this method relies on - if we need to preserve the behavior we should document it so future changes are less likely to accidentally change the semnatics. |
||
| p.state.allowedIPs = slices.Clone(allowedIPs) // avoid retaining caller's slice | ||
| } | ||
|
|
||
| // SendBuffers sends buffers to peer. WireGuard packet data in each element of | ||
| // buffers must be preceded by MessageEncapsulatingTransportSize number of | ||
| // bytes. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,6 +8,7 @@ | |
| package device | ||
|
|
||
| import ( | ||
| "log" | ||
| "sync" | ||
| "time" | ||
| _ "unsafe" | ||
|
|
@@ -126,6 +127,13 @@ func expiredNewHandshake(peer *Peer) { | |
| func expiredZeroKeyMaterial(peer *Peer) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we should rename this "expirePeer" or similar
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. well, I was trying to keep it closer to upstream |
||
| peer.device.log.Verbosef("%s - Removing all keys, since we haven't received a new one in %d seconds", peer, int((RejectAfterTime * 3).Seconds())) | ||
| peer.ZeroAndFlushAll() | ||
| if peer.deleteOnIdle { | ||
| toRemove := peer.handshake.remoteStatic | ||
| go func() { | ||
raggi marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| peer.device.RemovePeer(toRemove) | ||
| log.Printf("expiredZeroKeyMaterial: removed idle lazy peer %x", toRemove) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. peer.device.log.Verbosef, and we could match the overall signature to the above log message or close to it, i.e. use %s of peer
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ah, right! |
||
| }() | ||
| } | ||
| } | ||
|
|
||
| func expiredPersistentKeepalive(peer *Peer) { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,6 @@ | ||
| module github.com/tailscale/wireguard-go | ||
|
|
||
| go 1.20 | ||
| go 1.25 | ||
|
|
||
| require ( | ||
| golang.org/x/crypto v0.13.0 | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're going to need to hold this with care, and it's almost tempting to suggest fixing it wireguard-go side, though it's also sort of a pre-existing issue:
If Peer A and Peer B have the same routes, then the last write wins.
If Peer B is removed, the route becomes un-routed.
A typical user expectation would more likely be that Peer A becomes the destination instead.
I believe we do some guarding of this on our side at quite a distance away from here, but it feels risky.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh, I think I can unexport this now. I think this was from when I was waffling on the API I wanted. But this can be an internal detail now.