ipn/ipnlocal: only show Taildrive peers to which ACLs grant us access

This improves convenience and security.

* Convenience - no need to see nodes that can't share anything with you.
* Security - malicious nodes can't expose shares to peers that aren't
             allowed to access their shares.

Updates tailscale/corp#19432

Signed-off-by: Percy Wegmann <percy@tailscale.com>
This commit is contained in:
Percy Wegmann 2024-04-23 16:11:04 -05:00 committed by Percy Wegmann
parent 5d4b4ffc3c
commit 955ad12489
3 changed files with 37 additions and 25 deletions

View File

@ -39,7 +39,7 @@ func ParsePermissions(rawGrants [][]byte) (Permissions, error) {
var g grant var g grant
err := json.Unmarshal(rawGrant, &g) err := json.Unmarshal(rawGrant, &g)
if err != nil { if err != nil {
return nil, fmt.Errorf("unmarshal raw grants: %v", err) return nil, fmt.Errorf("unmarshal raw grants %s: %v", rawGrant, err)
} }
for _, share := range g.Shares { for _, share := range g.Shares {
existingPermission := permissions[share] existingPermission := permissions[share]

View File

@ -4,10 +4,10 @@
package ipnlocal package ipnlocal
import ( import (
"cmp"
"fmt" "fmt"
"os" "os"
"slices" "slices"
"strings"
"tailscale.com/drive" "tailscale.com/drive"
"tailscale.com/ipn" "tailscale.com/ipn"
@ -318,40 +318,47 @@ func (b *LocalBackend) updateDrivePeersLocked(nm *netmap.NetworkMap) {
func (b *LocalBackend) driveRemotesFromPeers(nm *netmap.NetworkMap) []*drive.Remote { func (b *LocalBackend) driveRemotesFromPeers(nm *netmap.NetworkMap) []*drive.Remote {
driveRemotes := make([]*drive.Remote, 0, len(nm.Peers)) driveRemotes := make([]*drive.Remote, 0, len(nm.Peers))
for _, p := range nm.Peers { for _, p := range nm.Peers {
// Exclude mullvad exit nodes from list of Taildrive peers
// TODO(oxtoacart) - once we have a better mechanism for finding only accessible sharers
// (see below) we can remove this logic.
if strings.HasSuffix(p.Name(), ".mullvad.ts.net.") {
continue
}
peerID := p.ID() peerID := p.ID()
url := fmt.Sprintf("%s/%s", peerAPIBase(nm, p), taildrivePrefix[1:]) url := fmt.Sprintf("%s/%s", peerAPIBase(nm, p), taildrivePrefix[1:])
driveRemotes = append(driveRemotes, &drive.Remote{ driveRemotes = append(driveRemotes, &drive.Remote{
Name: p.DisplayName(false), Name: p.DisplayName(false),
URL: url, URL: url,
Available: func() bool { Available: func() bool {
// TODO(oxtoacart): need to figure out a performant and reliable way to only // Peers are available to Taildrive if:
// show the peers that have shares to which we have access // - They are online
// This will require work on the control server to transmit the inverse // - They are allowed to share at least one folder with us
// of the "tailscale.com/cap/drive" capability.
// For now, at least limit it only to nodes that are online.
// Note, we have to iterate the latest netmap because the peer we got from the first iteration may not be it
b.mu.Lock() b.mu.Lock()
latestNetMap := b.netMap latestNetMap := b.netMap
b.mu.Unlock() b.mu.Unlock()
for _, candidate := range latestNetMap.Peers { idx, found := slices.BinarySearchFunc(latestNetMap.Peers, peerID, func(candidate tailcfg.NodeView, id tailcfg.NodeID) int {
if candidate.ID() == peerID { return cmp.Compare(candidate.ID(), id)
online := candidate.Online() })
if !found {
return false
}
peer := latestNetMap.Peers[idx]
// Exclude offline peers.
// TODO(oxtoacart): for some reason, this correctly // TODO(oxtoacart): for some reason, this correctly
// catches when a node goes from offline to online, // catches when a node goes from offline to online,
// but not the other way around... // but not the other way around...
return online != nil && *online online := peer.Online()
if online == nil || !*online {
return false
}
// Check that the peer is allowed to share with us.
addresses := peer.Addresses()
for i := range addresses.Len() {
addr := addresses.At(i)
capsMap := b.PeerCaps(addr.Addr())
if capsMap.HasCapability(tailcfg.PeerCapabilityTaildriveSharer) {
return true
} }
} }
// peer not found, must not be available
return false return false
}, },
}) })

View File

@ -131,7 +131,8 @@
// - 88: 2024-03-05: Client understands NodeAttrSuggestExitNode // - 88: 2024-03-05: Client understands NodeAttrSuggestExitNode
// - 89: 2024-03-23: Client no longer respects deleted PeerChange.Capabilities (use CapMap) // - 89: 2024-03-23: Client no longer respects deleted PeerChange.Capabilities (use CapMap)
// - 90: 2024-04-03: Client understands PeerCapabilityTaildrive. // - 90: 2024-04-03: Client understands PeerCapabilityTaildrive.
const CurrentCapabilityVersion CapabilityVersion = 90 // - 91: 2024-04-24: Client understands PeerCapabilityTaildriveSharer.
const CurrentCapabilityVersion CapabilityVersion = 91
type StableID string type StableID string
@ -1357,8 +1358,12 @@ type CapGrant struct {
// PeerCapabilityWebUI grants the ability for a peer to edit features from the // PeerCapabilityWebUI grants the ability for a peer to edit features from the
// device Web UI. // device Web UI.
PeerCapabilityWebUI PeerCapability = "tailscale.com/cap/webui" PeerCapabilityWebUI PeerCapability = "tailscale.com/cap/webui"
// PeerCapabilityTaildrive grants the ability for a peer to access Taildrive shares. // PeerCapabilityTaildrive grants the ability for a peer to access Taildrive
// shares.
PeerCapabilityTaildrive PeerCapability = "tailscale.com/cap/drive" PeerCapabilityTaildrive PeerCapability = "tailscale.com/cap/drive"
// PeerCapabilityTaildriveSharer indicates that a peer has the ability to
// share folders with us.
PeerCapabilityTaildriveSharer PeerCapability = "tailscale.com/cap/drive-sharer"
) )
// NodeCapMap is a map of capabilities to their optional values. It is valid for // NodeCapMap is a map of capabilities to their optional values. It is valid for