From ee6475a44dab0ecdbb672e1af0a1c2631fb67696 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Sat, 27 Feb 2021 21:42:34 -0800 Subject: [PATCH] wgengine: unify NewUserspaceEngine, NewUserspaceEngineAdvanced Also rename EngineConfig to Config to avoid wgengine.EngineConfig stutter. Signed-off-by: Brad Fitzpatrick --- Makefile | 5 +- cmd/tailscaled/tailscaled.go | 5 +- cmd/tailscaled/tailscaled_windows.go | 5 +- wgengine/userspace.go | 144 +++++++++++++-------------- 4 files changed, 84 insertions(+), 75 deletions(-) diff --git a/Makefile b/Makefile index 3665f05d1..dee8743ca 100644 --- a/Makefile +++ b/Makefile @@ -12,7 +12,10 @@ depaware: go run github.com/tailscale/depaware --check tailscale.com/cmd/tailscaled go run github.com/tailscale/depaware --check tailscale.com/cmd/tailscale -check: staticcheck vet depaware +buildwindows: + GOOS=windows GOARCH=amd64 go install tailscale.com/cmd/tailscale tailscale.com/cmd/tailscaled + +check: staticcheck vet depaware buildwindows staticcheck: go run honnef.co/go/tools/cmd/staticcheck -- $$(go list ./... | grep -v tempfork) diff --git a/cmd/tailscaled/tailscaled.go b/cmd/tailscaled/tailscaled.go index 4ff26fbb8..c4eb331c8 100644 --- a/cmd/tailscaled/tailscaled.go +++ b/cmd/tailscaled/tailscaled.go @@ -198,7 +198,10 @@ func run() error { } e, err = wgengine.NewFakeUserspaceEngine(logf, 0, impl) } else { - e, err = wgengine.NewUserspaceEngine(logf, args.tunname, args.port) + e, err = wgengine.NewUserspaceEngine(logf, wgengine.Config{ + TUNName: args.tunname, + ListenPort: args.port, + }) } if err != nil { logf("wgengine.New: %v", err) diff --git a/cmd/tailscaled/tailscaled_windows.go b/cmd/tailscaled/tailscaled_windows.go index 4de9a50e6..342f0c237 100644 --- a/cmd/tailscaled/tailscaled_windows.go +++ b/cmd/tailscaled/tailscaled_windows.go @@ -114,7 +114,10 @@ func startIPNServer(ctx context.Context, logid string) error { var err error getEngine := func() (wgengine.Engine, error) { - eng, err := wgengine.NewUserspaceEngine(logf, "Tailscale", 41641) + eng, err := wgengine.NewUserspaceEngine(logf, wgengine.Config{ + TUNName: "Tailscale", + ListenPort: 41641, + }) if err != nil { return nil, err } diff --git a/wgengine/userspace.go b/wgengine/userspace.go index e7f8dc034..c6d1d9394 100644 --- a/wgengine/userspace.go +++ b/wgengine/userspace.go @@ -85,17 +85,18 @@ const ( ) type userspaceEngine struct { - logf logger.Logf - wgLogger *wglog.Logger //a wireguard-go logging wrapper - reqCh chan struct{} - waitCh chan struct{} // chan is closed when first Close call completes; contrast with closing bool - timeNow func() time.Time - tundev *tstun.TUN - wgdev *device.Device - router router.Router - resolver *tsdns.Resolver - magicConn *magicsock.Conn - linkMon *monitor.Mon + logf logger.Logf + wgLogger *wglog.Logger //a wireguard-go logging wrapper + reqCh chan struct{} + waitCh chan struct{} // chan is closed when first Close call completes; contrast with closing bool + timeNow func() time.Time + tundev *tstun.TUN + wgdev *device.Device + router router.Router + resolver *tsdns.Resolver + magicConn *magicsock.Conn + linkMon *monitor.Mon + linkMonUnregister func() // unsubscribes from changes; used regardless of linkMonOwned testMaybeReconfigHook func() // for tests; if non-nil, fires if maybeReconfigWireguardLocked called @@ -133,15 +134,24 @@ type userspaceEngine struct { // router.Router. type RouterGen func(logf logger.Logf, wgdev *device.Device, tundev tun.Device) (router.Router, error) -type EngineConfig struct { - // Logf is the logging function used by the engine. - Logf logger.Logf - // TUN is the tun device used by the engine. +// Config is the engine configuration. +type Config struct { + // TUN is the TUN device used by the engine. + // Exactly one of either TUN or TUNName must be specified. TUN tun.Device + + // TUNName is the TUN device to create. + // Exactly one of either TUN or TUNName must be specified. + TUNName string + // RouterGen is the function used to instantiate the router. + // If nil, wgengine/router.New is used. RouterGen RouterGen + // ListenPort is the port on which the engine will listen. + // If zero, a port is automatically selected. ListenPort uint16 + // Fake determines whether this engine is running in fake mode, // which disables such features as DNS configuration and unrestricted ICMP Echo responses. Fake bool @@ -156,7 +166,7 @@ type EngineConfig struct { } // FakeImpl is a fake or alternate version of Engine that can be started. See -// EngineConfig.FakeImplFactory for details. +// Config.FakeImplFactory for details. type FakeImpl interface { Start() error } @@ -166,78 +176,54 @@ type FakeImplFactory func(logger.Logf, *tstun.TUN, Engine, *magicsock.Conn) (Fak func NewFakeUserspaceEngine(logf logger.Logf, listenPort uint16, impl FakeImplFactory) (Engine, error) { logf("Starting userspace wireguard engine (with fake TUN device)") - conf := EngineConfig{ - Logf: logf, + return NewUserspaceEngine(logf, Config{ TUN: tstun.NewFakeTUN(), RouterGen: router.NewFake, ListenPort: listenPort, Fake: true, FakeImplFactory: impl, - } - return NewUserspaceEngineAdvanced(conf) + }) } // NewUserspaceEngine creates the named tun device and returns a // Tailscale Engine running on it. -func NewUserspaceEngine(logf logger.Logf, tunName string, listenPort uint16) (Engine, error) { - if tunName == "" { - return nil, fmt.Errorf("--tun name must not be blank") - } - - logf("Starting userspace wireguard engine with tun device %q", tunName) - - tun, err := tun.CreateTUN(tunName, minimalMTU) - if err != nil { - diagnoseTUNFailure(tunName, logf) - logf("CreateTUN: %v", err) - return nil, err - } - logf("CreateTUN ok.") - - if err := waitInterfaceUp(tun, 90*time.Second, logf); err != nil { - return nil, err - } - - conf := EngineConfig{ - Logf: logf, - TUN: tun, - RouterGen: router.New, - ListenPort: listenPort, - } +func NewUserspaceEngine(logf logger.Logf, conf Config) (Engine, error) { + if conf.TUN != nil && conf.TUNName != "" { + return nil, errors.New("TUN and TUNName are mutually exclusive") + } + if conf.TUN == nil && conf.TUNName == "" { + return nil, errors.New("either TUN or TUNName are required") + } + tunDev := conf.TUN + var err error + if tunName := conf.TUNName; tunName != "" { + logf("Starting userspace wireguard engine with tun device %q", tunName) + tunDev, err = tun.CreateTUN(tunName, minimalMTU) + if err != nil { + diagnoseTUNFailure(tunName, logf) + logf("CreateTUN: %v", err) + return nil, err + } + logf("CreateTUN ok.") - e, err := NewUserspaceEngineAdvanced(conf) - if err != nil { - return nil, err + if err := waitInterfaceUp(tunDev, 90*time.Second, logf); err != nil { + return nil, err + } } - return e, err -} - -type closeOnErrorPool []func() -func (p *closeOnErrorPool) add(c io.Closer) { *p = append(*p, func() { c.Close() }) } -func (p *closeOnErrorPool) addFunc(fn func()) { *p = append(*p, fn) } -func (p closeOnErrorPool) closeAllIfError(errp *error) { - if *errp != nil { - for _, closeFn := range p { - closeFn() - } + if conf.RouterGen == nil { + conf.RouterGen = router.New } -} -// NewUserspaceEngineAdvanced is like NewUserspaceEngine -// but provides control over all config fields. -func NewUserspaceEngineAdvanced(conf EngineConfig) (Engine, error) { - return newUserspaceEngineAdvanced(conf) + return newUserspaceEngine(logf, tunDev, conf) } -func newUserspaceEngineAdvanced(conf EngineConfig) (_ Engine, reterr error) { - logf := conf.Logf - +func newUserspaceEngine(logf logger.Logf, rawTUNDev tun.Device, conf Config) (_ Engine, reterr error) { var closePool closeOnErrorPool defer closePool.closeAllIfError(&reterr) - tunDev := tstun.WrapTUN(logf, conf.TUN) - closePool.add(tunDev) + tsTUNDev := tstun.WrapTUN(logf, rawTUNDev) + closePool.add(tsTUNDev) rconf := tsdns.ResolverConfig{ Logf: logf, @@ -248,7 +234,7 @@ func newUserspaceEngineAdvanced(conf EngineConfig) (_ Engine, reterr error) { logf: logf, reqCh: make(chan struct{}, 1), waitCh: make(chan struct{}), - tundev: tunDev, + tundev: tsTUNDev, resolver: tsdns.NewResolver(rconf), pingers: make(map[wgkey.Key]*pinger), } @@ -261,12 +247,13 @@ func newUserspaceEngineAdvanced(conf EngineConfig) (_ Engine, reterr error) { return nil, err } closePool.add(mon) - unregisterMonWatch := mon.RegisterChangeCallback(func() { + e.linkMon = mon + unregisterMonWatch := e.linkMon.RegisterChangeCallback(func() { e.LinkChange(false) tshttpproxy.InvalidateCache() }) closePool.addFunc(unregisterMonWatch) - e.linkMon = mon + e.linkMonUnregister = unregisterMonWatch endpointsFn := func(endpoints []string) { e.mu.Lock() @@ -1249,6 +1236,7 @@ func (e *userspaceEngine) Close() { e.wgdev.IpcSetOperation(r) e.resolver.Close() e.magicConn.Close() + e.linkMonUnregister() e.linkMon.Close() e.router.Close() e.wgdev.Close() @@ -1475,3 +1463,15 @@ func diagnoseLinuxTUNFailure(tunName string, logf logger.Logf) { } } } + +type closeOnErrorPool []func() + +func (p *closeOnErrorPool) add(c io.Closer) { *p = append(*p, func() { c.Close() }) } +func (p *closeOnErrorPool) addFunc(fn func()) { *p = append(*p, fn) } +func (p closeOnErrorPool) closeAllIfError(errp *error) { + if *errp != nil { + for _, closeFn := range p { + closeFn() + } + } +}