I was going to write a test for this using the tstest/integration test
stuff, but the testcontrol implementation isn't quite there yet (it
always registers nodes and doesn't provide AuthURLs). So, manually
tested for now.
Fixes#1843
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Per discussion, we want to have only one test assertion library,
and we want to start by exploring quicktest.
This was a mostly mechanical translation.
I think we could make this nicer by defining a few helper
closures at the beginning of the test. Later.
Signed-off-by: Josh Bleecher Snyder <josharian@gmail.com>
This fixes#1833 in two ways:
* stop setting NoSNAT on non-Linux. It only matters on Linux and the flag
is hidden on non-Linux, but the code was still setting it. Because of
that, the new pref-reverting safety checks were failing when it was
changing.
* Ignore the two Linux-only prefs changing on non-Linux.
Fixes#1833
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
There's no need to warn that it was not provided on the command line
after doing a sequence of up; logout; up --args. If you're asking for
tailscale to be up, you always mean that you prefer LoggedOut to become
false.
Fixes#1828
Signed-off-by: Avery Pennarun <apenwarr@tailscale.com>
Prior to wireguard-go using printf-style logging,
all wireguard-go logging occurred using format string "%s".
We fixed that but continued to use %s when we rewrote
peer identifiers into Tailscale style.
This commit removes that %sl, which makes rate limiting work correctly.
As a happy side-benefit, it should generate less garbage.
Instead of replacing all wireguard-go peer identifiers
that might occur anywhere in a fully formatted log string,
assume that they only come from args.
Check all args for things that look like *device.Peers
and replace them with appropriately reformatted strings.
There is a variety of ways that this could go wrong
(unusual format verbs or modifiers, peer identifiers
occurring as part of a larger printed object, future API changes),
but none of them occur now, are likely to be added,
or would be hard to work around if they did.
Signed-off-by: Josh Bleecher Snyder <josharian@gmail.com>
The "stop phrases" we use all occur in wireguard-go in the format string.
We can avoid doing a bunch of fmt.Sprintf work when they appear.
Signed-off-by: Josh Bleecher Snyder <josharian@gmail.com>
This removes the NewLocalBackendWithClientGen constructor added in
b4d04a065f and instead adds
LocalBackend.SetControlClientGetterForTesting, mirroring
LocalBackend.SetHTTPTestClient. NewLocalBackendWithClientGen was
weird in being exported but taking an unexported type. This was noted
during code review:
https://github.com/tailscale/tailscale/pull/1818#discussion_r623155669
which ended in:
"I'll leave it for y'all to clean up if you find some way to do it elegantly."
This is more idiomatic.
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Without this, macOS would fail to display its menu state correctly if you
started it while !WantRunning. It relies on the netmap in order to show
the logged-in username.
Signed-off-by: Avery Pennarun <apenwarr@tailscale.com>
There was logic that would make a "down" tailscale backend (ie.
!WantRunning) refuse to do any network activity. Unfortunately, this
makes the macOS and iOS UI unable to render correctly if they start
while !WantRunning.
Now that we have Prefs.LoggedOut, use that instead. So `tailscale down`
will still allow the controlclient to connect its authroutine, but
pause the maproutine. `tailscale logout` will entirely stop all
activity.
This new behaviour is not obviously correct; it's a bit annoying that
`tailsale down` doesn't terminate all activity like you might expect.
Maybe we should redesign the UI code to render differently when
disconnected, and then revert this change.
Signed-off-by: Avery Pennarun <apenwarr@tailscale.com>
EditPrefs should be just a wrapper around the action of changing prefs,
but someone had added a side effect of calling Login() sometimes. The
side effect happened *after* running the state machine, which would
sometimes result in us going into NeedsLogin immediately before calling
cc.Login().
This manifested as the macOS app not being able to Connect if you
launched it with LoggedOut=false and WantRunning=false. Trying to
Connect() would sent us to the NeedsLogin state instead.
Signed-off-by: Avery Pennarun <apenwarr@tailscale.com>
- Switch to our own simpler token bucket, since x/time/rate is missing
necessary stuff (can't provide your own time func; can't check the
current bucket contents) and it's overkill anyway.
- Add tests that actually include advancing time.
- Don't remove the rate limit on a message until there's enough room to
print at least two more of them. When we do, we'll also print how
many we dropped, as a contextual reminder that some were previously
lost. (This is more like how the Linux kernel does it.)
- Reformat the [RATE LIMITED] messages to be shorter, and to not
corrupt original message. Instead, we print the message, then print
its format string.
- Use %q instead of \"%s\", for more accurate parsing later, if the
format string contained quotes.
Fixes#1772
Signed-off-by: Avery Pennarun <apenwarr@tailscale.com>
A very long unit test that verifies the way the controlclient and
ipn.Backend interact.
This is a giant sequential test of the state machine. The test passes,
but only because it's asserting all the wrong behaviour. I marked all
the behaviour I think is wrong with BUG comments, and several
additional test opportunities with TODO.
Note: the new test supercedes TestStartsInNeedsLoginState, which was
checking for incorrect behaviour (although the new test still checks
for the same incorrect behaviour) and assumed .Start() would converge
before returning, which it happens to do, but only for this very
specific case, for the current implementation. You're supposed to wait
for the notifications.
Updates: tailscale/corp#1660
Signed-off-by: Avery Pennarun <apenwarr@tailscale.com>
With this change, shared node names resolve correctly on split DNS-supporting
operating systems.
Fixestailscale/corp#1706
Signed-off-by: David Anderson <danderson@tailscale.com>
Only minimal tailscale + tailscaled for now.
And a super minimal in-memory logcatcher.
No control ... yet.
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Pointer receivers used with MarshalJSON are code rakes.
https://github.com/golang/go/issues/22967https://github.com/dominikh/go-tools/issues/911
I just stepped on one, and it hurt. Turn it over.
While we're here, optimize the code a bit.
name old time/op new time/op delta
MarshalJSON-8 184ns ± 0% 44ns ± 0% -76.03% (p=0.000 n=20+19)
name old alloc/op new alloc/op delta
MarshalJSON-8 184B ± 0% 80B ± 0% -56.52% (p=0.000 n=20+20)
name old allocs/op new allocs/op delta
MarshalJSON-8 4.00 ± 0% 1.00 ± 0% -75.00% (p=0.000 n=20+20)
Signed-off-by: Josh Bleecher Snyder <josharian@gmail.com>
For historical reasons, we ended up with two near-duplicate
copies of curve25519 key types, one in the wireguard-go module
(wgcfg) and one in the tailscale module (types/wgkey).
Then we moved wgcfg to the tailscale module.
We can now remove the wgcfg key type in favor of wgkey.
Signed-off-by: Josh Bleecher Snyder <josharian@gmail.com>
One of the consequences of the bind refactoring in 6f23087175
is that attempting to bind an IPv6 socket will always
result in c.pconn6.pconn being non-nil.
If the bind fails, it'll be set to a placeholder packet conn
that blocks forever.
As a result, we can always run ReceiveIPv6 and health check it.
This removes IPv4/IPv6 asymmetry and also will allow health checks
to detect any IPv6 receive func failures.
Signed-off-by: Josh Bleecher Snyder <josharian@gmail.com>
