taildrop: lazily perform full deletion scan after first taildrop use (#10137)

Simply reading the taildrop directory can pop up security dialogs
on platforms like macOS. Avoid this by only performing garbage collection
of partial and deleted files after the first received taildrop file,
which would have prompted the security dialog window.

Updates tailscale/corp#14772

Signed-off-by: Joe Tsai <joetsai@digital-static.net>
This commit is contained in:
Joe Tsai 2023-11-13 10:20:28 -08:00 committed by GitHub
parent e848736927
commit 975c5f7684
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 54 additions and 7 deletions

View File

@ -3728,6 +3728,7 @@ func (b *LocalBackend) initPeerAPIListener() {
taildrop: taildrop.ManagerOptions{ taildrop: taildrop.ManagerOptions{
Logf: b.logf, Logf: b.logf,
Clock: tstime.DefaultClock{Clock: b.clock}, Clock: tstime.DefaultClock{Clock: b.clock},
State: b.store,
Dir: fileRoot, Dir: fileRoot,
DirectFileMode: b.directFileRoot != "", DirectFileMode: b.directFileRoot != "",
AvoidFinalRename: !b.directFileDoFinalRename, AvoidFinalRename: !b.directFileDoFinalRename,

View File

@ -53,6 +53,11 @@
// CurrentProfileStateKey is the key under which we store the current // CurrentProfileStateKey is the key under which we store the current
// profile. // profile.
CurrentProfileStateKey = StateKey("_current-profile") CurrentProfileStateKey = StateKey("_current-profile")
// TaildropReceivedKey is the key to indicate whether any taildrop file
// has ever been received (even if partially).
// Any non-empty value indicates that at least one file has been received.
TaildropReceivedKey = StateKey("_taildrop-received")
) )
// CurrentProfileID returns the StateKey that stores the // CurrentProfileID returns the StateKey that stores the

View File

@ -13,6 +13,7 @@
"sync" "sync"
"time" "time"
"tailscale.com/ipn"
"tailscale.com/syncs" "tailscale.com/syncs"
"tailscale.com/tstime" "tailscale.com/tstime"
"tailscale.com/types/logger" "tailscale.com/types/logger"
@ -52,13 +53,24 @@ func (d *fileDeleter) Init(m *Manager, eventHook func(string)) {
d.dir = m.opts.Dir d.dir = m.opts.Dir
d.event = eventHook d.event = eventHook
// From a cold-start, load the list of partial and deleted files.
d.byName = make(map[string]*list.Element) d.byName = make(map[string]*list.Element)
d.emptySignal = make(chan struct{}) d.emptySignal = make(chan struct{})
d.shutdownCtx, d.shutdown = context.WithCancel(context.Background()) d.shutdownCtx, d.shutdown = context.WithCancel(context.Background())
// From a cold-start, load the list of partial and deleted files.
//
// Only run this if we have ever received at least one file
// to avoid ever touching the taildrop directory on systems (e.g., MacOS)
// that pop up a security dialog window upon first access.
if m.opts.State == nil {
return
}
if b, _ := m.opts.State.ReadState(ipn.TaildropReceivedKey); len(b) == 0 {
return
}
d.group.Go(func() { d.group.Go(func() {
d.event("start init") d.event("start full-scan")
defer d.event("end init") defer d.event("end full-scan")
rangeDir(d.dir, func(de fs.DirEntry) bool { rangeDir(d.dir, func(de fs.DirEntry) bool {
switch { switch {
case d.shutdownCtx.Err() != nil: case d.shutdownCtx.Err() != nil:

View File

@ -11,6 +11,8 @@
"time" "time"
"github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp"
"tailscale.com/ipn"
"tailscale.com/ipn/store/mem"
"tailscale.com/tstest" "tailscale.com/tstest"
"tailscale.com/tstime" "tailscale.com/tstime"
"tailscale.com/util/must" "tailscale.com/util/must"
@ -72,6 +74,8 @@ func TestDeleter(t *testing.T) {
m.opts.Logf = t.Logf m.opts.Logf = t.Logf
m.opts.Clock = tstime.DefaultClock{Clock: clock} m.opts.Clock = tstime.DefaultClock{Clock: clock}
m.opts.Dir = dir m.opts.Dir = dir
m.opts.State = must.Get(mem.New(nil, ""))
must.Do(m.opts.State.WriteState(ipn.TaildropReceivedKey, []byte{1}))
fd.Init(&m, eventHook) fd.Init(&m, eventHook)
defer fd.Shutdown() defer fd.Shutdown()
insert := func(name string) { insert := func(name string) {
@ -85,8 +89,8 @@ func TestDeleter(t *testing.T) {
fd.Remove(name) fd.Remove(name)
} }
checkEvents("start init") checkEvents("start full-scan")
checkEvents("end init", "start waitAndDelete") checkEvents("end full-scan", "start waitAndDelete")
checkDirectory("foo.partial", "bar.partial", "buzz.deleted") checkDirectory("foo.partial", "bar.partial", "buzz.deleted")
advance(deleteDelay / 2) advance(deleteDelay / 2)
@ -134,3 +138,15 @@ func TestDeleter(t *testing.T) {
remove("wuzz.partial") remove("wuzz.partial")
checkEvents("end waitAndDelete") checkEvents("end waitAndDelete")
} }
// Test that the asynchronous full scan of the taildrop directory does not occur
// on a cold start if taildrop has never received any files.
func TestDeleterInitWithoutTaildrop(t *testing.T) {
var m Manager
var fd fileDeleter
m.opts.Logf = t.Logf
m.opts.Dir = t.TempDir()
m.opts.State = must.Get(mem.New(nil, ""))
fd.Init(&m, func(event string) { t.Errorf("unexpected event: %v", event) })
fd.Shutdown()
}

View File

@ -13,6 +13,7 @@
"time" "time"
"tailscale.com/envknob" "tailscale.com/envknob"
"tailscale.com/ipn"
"tailscale.com/tstime" "tailscale.com/tstime"
"tailscale.com/version/distro" "tailscale.com/version/distro"
) )
@ -136,6 +137,17 @@ func (m *Manager) PutFile(id ClientID, baseName string, r io.Reader, offset, len
}() }()
inFile.w = f inFile.w = f
// Record that we have started to receive at least one file.
// This is used by the deleter upon a cold-start to scan the directory
// for any files that need to be deleted.
if m.opts.State != nil {
if b, _ := m.opts.State.ReadState(ipn.TaildropReceivedKey); len(b) == 0 {
if err := m.opts.State.WriteState(ipn.TaildropReceivedKey, []byte{1}); err != nil {
m.opts.Logf("WriteState error: %v", err) // non-fatal error
}
}
}
// A positive offset implies that we are resuming an existing file. // A positive offset implies that we are resuming an existing file.
// Seek to the appropriate offset and truncate the file. // Seek to the appropriate offset and truncate the file.
if offset != 0 { if offset != 0 {

View File

@ -67,8 +67,9 @@ func (id ClientID) partialSuffix() string {
// ManagerOptions are options to configure the [Manager]. // ManagerOptions are options to configure the [Manager].
type ManagerOptions struct { type ManagerOptions struct {
Logf logger.Logf Logf logger.Logf // may be nil
Clock tstime.DefaultClock Clock tstime.DefaultClock // may be nil
State ipn.StateStore // may be nil
// Dir is the directory to store received files. // Dir is the directory to store received files.
// This main either be the final location for the files // This main either be the final location for the files