In the earlier http2 package migration (1d93bdce20, #17394) I had
removed Direct.Close's tracking of the connPool, thinking it wasn't
necessary.
Some tests (in another repo) are strict and like it to tear down the
world and wait, to check for leaked goroutines. And they caught this
letting some goroutines idle past Close, even if they'd eventually
close down on their own.
This restores the connPool accounting and the aggressife close.
Updates #17305
Updates #17394
Change-Id: I5fed283a179ff7c3e2be104836bbe58b05130cc7
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
A recent change (009d702adf) introduced a deadlock where the
/machine/update-health network request to report the client's health
status update to the control plane was moved to being synchronous
within the eventbus's pump machinery.
I started to instead make the health reporting be async, but then we
realized in the three years since we added that, it's barely been used
and doesn't pay for itself, for how many HTTP requests it makes.
Instead, delete it all and replace it with a c2n handler, which
provides much more helpful information.
Fixestailscale/corp#32952
Change-Id: I9e8a5458269ebfdda1c752d7bbb8af2780d71b04
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Saves 262 KB so far. I'm sure I missed some places, but shotizam says
these were the low hanging fruit.
Updates #12614
Change-Id: Ia31c01b454f627e6d0470229aae4e19d615e45e3
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Maybe it matters? At least globally across all nodes?
Fixes#17343
Change-Id: I3f61758ea37de527e16602ec1a6e453d913b3195
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Saves 352 KB, removing one of our two HTTP/2 implementations linked
into the binary.
Fixes#17305
Updates #15015
Change-Id: I53a04b1f2687dca73c8541949465038b69aa6ade
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
A following change will split out the controlclient.NoiseClient type
out, away from the rest of the controlclient package which is
relatively dependency heavy.
A question was where to move it, and whether to make a new (a fifth!)
package in the ts2021 dependency chain.
@creachadair and I brainstormed and decided to merge
internal/noiseconn and controlclient.NoiseClient into one package,
with names ts2021.Conn and ts2021.Client.
For ease of reviewing the subsequent PR, this is the first step that
just renames the internal/noiseconn package to control/ts2021.
Updates #17305
Change-Id: Ib5ea162dc1d336c1d805bdd9548d1702dd6e1468
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
depaware was merging golang.org/x/foo and std's
vendor/golang.org/x/foo packages (which could both be in the binary!),
leading to confusing output, especially when I was working on
eliminating duplicate packages imported under different names.
This makes the depaware output longer and grosser, but doesn't hide
reality from us.
Updates #17305
Change-Id: I21cc3418014e127f6c1a81caf4e84213ce84ab57
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
It never launched and I've lost hope of it launching and it's in my
way now, so I guess it's time to say goodbye.
Updates tailscale/corp#4383
Updates #17305
Change-Id: I2eb551d49f2fb062979cc307f284df4b3dfa5956
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
This permits other programs (in other repos) to conditionally
import ipn/store/awsstore and/or ipn/store/kubestore and have them
register themselves, rather than feature/condregister doing it.
Updates tailscale/corp#32922
Change-Id: I2936229ce37fd2acf9be5bf5254d4a262d090ec1
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Saves 442 KB. Lock it with a new min test.
Updates #12614
Change-Id: Ia7bf6f797b6cbf08ea65419ade2f359d390f8e91
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
I'm trying to remove the "regexp" and "regexp/syntax" packages from
our minimal builds. But tsweb pulls in regexp (via net/http/pprof etc)
and util/eventbus was importing the tsweb for no reason.
Updates #12614
Change-Id: Ifa8c371ece348f1dbf80d6b251381f3ed39d5fbd
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Even with ts_omit_drive, the drive package is currently still imported
for some types. So it should be light. But it was depending on the
"regexp" packge, which I'd like to remove from our minimal builds.
Updates #12614
Change-Id: I5bf85d8eb15a739793723b1da11c370d3fcd2f32
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Saves 139 KB.
Also Synology support, which I saw had its own large-ish proxy parsing
support on Linux, but support for proxies without Synology proxy
support is reasonable, so I pulled that out as its own thing.
Updates #12614
Change-Id: I22de285a3def7be77fdcf23e2bec7c83c9655593
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
In Dec 2021 in d3d503d997 I had grand plans to make exit node DNS
cheaper by using HTTP/2 over PeerAPI, at least on some platforms. I
only did server-side support though and never made it to the client.
In the ~4 years since, some things have happened:
* Go 1.24 got support for http.Protocols (https://pkg.go.dev/net/http#Protocols)
and doing UnencryptedHTTP2 ("HTTP2 with prior knowledge")
* The old h2c upgrade mechanism was deprecated; see https://github.com/golang/go/issues/63565
and https://github.com/golang/go/issues/67816
* Go plans to deprecate x/net/http2 and move everything to the standard library.
So this drops our use of the x/net/http2/h2c package and instead
enables h2c (on all platforms now) using the standard library.
This does mean we lose the deprecated h2c Upgrade support, but that's
fine.
If/when we do the h2c client support for ExitDNS, we'll have to probe
the peer to see whether it supports it. Or have it reply with a header
saying that future requests can us h2c. (It's tempting to use capver,
but maybe people will disable that support anyway, so we should
discover it at runtime instead.)
Also do the same in the sessionrecording package.
Updates #17305
Change-Id: If323f5ef32486effb18ed836888aa05c0efb701e
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Saves 328 KB (2.5%) off the minimal binary.
For IoT devices that don't need MagicDNS (e.g. they don't make
outbound connections), this provides a knob to disable all the DNS
functionality.
Rather than a massive refactor today, this uses constant false values
as a deadcode sledgehammer, guided by shotizam to find the largest DNS
functions which survived deadcode.
A future refactor could make it so that the net/dns/resolver and
publicdns packages don't even show up in the import graph (along with
their imports) but really it's already pretty good looking with just
these consts, so it's not at the top of my list to refactor it more
soon.
Also do the same in a few places with the ACME (cert) functionality,
as I saw those while searching for DNS stuff.
Updates #12614
Change-Id: I8e459f595c2fde68ca16503ff61c8ab339871f97
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
When I added dependency support to featuretag, I broke the handling of
the non-omit build tags (as used by the "box" support for bundling the
CLI into tailscaled). That then affected depaware. The
depaware-minbox.txt this whole time recently has not included the CLI.
So fix that, and also add a new depaware variant that's only the
daemon, without the CLI.
Updates #12614
Updates #17139
Change-Id: I4a4591942aa8c66ad8e3242052e3d9baa42902ca
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Otherwise they're uselessly imported by tsnet applications, even
though they do nothing. tsnet applications wanting to use these
already had to explicitly import them and use kubestore.New or
awsstore.New and assign those to their tsnet.Server.Store fields.
Updates #12614
Change-Id: I358e3923686ddf43a85e6923c3828ba2198991d4
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
So wgengine/router is just the docs + entrypoint + types, and then
underscore importing wgengine/router/osrouter registers the constructors
with the wgengine/router package.
Then tsnet can not pull those in.
Updates #17313
Change-Id: If313226f6987d709ea9193c8f16a909326ceefe7
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Removes 434 KB from the minimal Linux binary, or ~3%.
Primarily this comes from not linking in the zstd encoding code.
Fixes#17323
Change-Id: I0a90de307dfa1ad7422db7aa8b1b46c782bfaaf7
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
As of the earlier 85febda86d, our new preferred zstd API of choice
is zstdframe.
Updates #cleanup
Updates tailscale/corp#18514
Change-Id: I5a6164d3162bf2513c3673b6d1e34cfae84cb104
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
It has nothing to do with logtail and is confusing named like that.
Updates #cleanup
Updates #17323
Change-Id: Idd34587ba186a2416725f72ffc4c5778b0b9db4a
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Now cmd/derper doesn't depend on iptables, nftables, and netlink code :)
But this is really just a cleanup step I noticed on the way to making
tsnet applications able to not link all the OS router code which they
don't use.
Updates #17313
Change-Id: Ic7b4e04e3a9639fd198e9dbeb0f7bae22a4a47a9
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
The dnstype package is used by tailcfg, which tries to be light and
leafy. But it brings in dnstype. So dnstype shouldn't bring in
x/net/dns/dnsmessage.
Updates #12614
Change-Id: I043637a7ce7fed097e648001f13ca1927a781def
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
I noticed this while modularizing clientupdate. With this in first,
moving clientupdate to be modular removes a bunch more stuff from
the minimal build + tsnet.
Updates #17115
Change-Id: I44bd055fca65808633fd3a848b0bbc09b00ad4fa
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
As part of making Tailscale's gvisor dependency optional for small builds,
this was one of the last places left that depended on gvisor. Just copy
the couple functions were were using.
Updates #17283
Change-Id: Id2bc07ba12039afe4c8a3f0b68f4d76d1863bbfe
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Baby steps. This permits building without much of gvisor, but not all of it.
Updates #17283
Change-Id: I8433146e259918cc901fe86b4ea29be22075b32c
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
This only saves ~32KB in the minimal linux/amd64 binary, but it's a
step towards permitting not depending on gvisor for small builds.
Updates #17283
Change-Id: Iae8da5e9465127de354dbcaf25e794a6832d891b
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
And yay: tsnet (and thus k8s-operator etc) no longer depends on
portlist! And LocalBackend is smaller.
Removes 50 KB from the minimal binary.
Updates #12614
Change-Id: Iee04057053dc39305303e8bd1d9599db8368d926
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
I think this was originally a brain-o in 9380e2dfc6. It's
disabling the port _poller_, listing what open ports (i.e. services)
are open, not PMP/PCP/UPnP port mapping.
While there, drop in some more testenv.AssertInTest() in a few places.
Updates #cleanup
Change-Id: Ia6f755ad3544f855883b8a7bdcfc066e8649547b
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
This exports a number of things from the derp (generic + client) package
to be used by the new derpserver package, as now used by cmd/derper.
And then enough other misc changes to lock in that cmd/tailscaled can
be configured to not bring in tailscale.com/client/local. (The webclient
in particular, even when disabled, was bringing it in, so that's now fixed)
Fixes#17257
Change-Id: I88b6c7958643fb54f386dd900bddf73d2d4d96d5
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Help out the linker's dead code elimination.
Updates #12614
Change-Id: I6c13cb44d3250bf1e3a01ad393c637da4613affb
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
This doesn't yet fully pull it out into a feature/captiveportal package.
This is the usual first step, moving the code to its own files within
the same packages.
Updates #17254
Change-Id: Idfaec839debf7c96f51ca6520ce36ccf2f8eec92
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
We never implemented the peercred package on OpenBSD (and I just tried
again and failed), but we've always documented that the creds pointer
can be nil for operating systems where we can't map the unix socket
back to its UID. On those platforms, we set the default unix socket
permissions such that only the admin can open it anyway and we don't
have a read-only vs read-write distinction. OpenBSD was always in that
camp, where any access to Tailscale's unix socket meant full access.
But during some refactoring, we broke OpenBSD in that we started
assuming during one logging path (during login) that Creds was non-nil
when looking up an ipnauth.Actor's username, which wasn't relevant (it
was called from a function "maybeUsernameOf" anyway, which threw away
errors).
Verified on an OpenBSD VM. We don't have any OpenBSD integration tests yet.
Fixes#17209
Updates #17221
Change-Id: I473c5903dfaa645694bcc75e7f5d484f3dd6044d
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
controlhttp has the responsibility of dialing a set of candidate control
endpoints in a way that minimizes user facing latency. If one control
endpoint is unavailable we promptly dial another, racing across the
dimensions of: IPv6, IPv4, port 80, and port 443, over multiple server
endpoints.
In the case that the top priority endpoint was not available, the prior
implementation would hang waiting for other results, so as to try to
return the highest priority successful connection to the rest of the
client code. This hang would take too long with a large dialplan and
sufficient client to endpoint latency as to cause the server to timeout
the connection due to inactivity in the intermediate state.
Instead of trying to prioritize non-ideal candidate connections, the
first successful connection is now used unconditionally, improving user
facing latency and avoiding any delays that would encroach on the
server-side timeout.
The tests are converted to memnet and synctest, running on all
platforms.
Fixes#8442Fixestailscale/corp#32534
Co-authored-by: James Tucker <james@tailscale.com>
Change-Id: I4eb57f046d8b40403220e40eb67a31c41adb3a38
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Signed-off-by: James Tucker <james@tailscale.com>
The controlhttp dialer with a ControlDialPlan IPv6 entry was hitting a
case where the dnscache Resolver was returning an netip.Addr zero
value, where it should've been returning the IPv6 address.
We then tried to dial "invalid IP:80", which would immediately fail,
at least locally.
Mostly this was causing spammy logs when debugging other stuff.
Updates tailscale/corp#32534
Change-Id: If8b9a20f10c1a6aa8a662c324151d987fe9bd2f8
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
tsnet apps in particular never use the Linux DNS OSManagers, so they don't need
DBus, etc. I started to pull that all out into separate features so tsnet doesn't
need to bring in DBus, but hit this first.
Here you can see that tsnet (and the k8s-operator) no longer pulls in inotify.
Updates #17206
Change-Id: I7af0f391f60c5e7dbeed7a080346f83262346591
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
And another case of the same typo in a comment elsewhere.
Updates #cleanup
Change-Id: Iaa9d865a1cf83318d4a30263c691451b5d708c9c
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
It doesn't really pull its weight: it adds 577 KB to the binary and
is rarely useful.
Also, we now have static IPs and other connectivity paths coming
soon enough.
Updates #5853
Updates #1278
Updates tailscale/corp#32168
Change-Id: If336fed00a9c9ae9745419e6d81f7de6da6f7275
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
I'd started to do this in the earlier ts_omit_server PR but
decided to split it into this separate PR.
Updates #17128
Change-Id: Ief8823a78d1f7bbb79e64a5cab30a7d0a5d6ff4b
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Remove the need for the caller to hold on to and call an unregister
function. Both two callers (one real, one test) already have a context
they can use. Use context.AfterFunc instead. There are no observable
side effects from scheduling too late if the goroutine doesn't run sync.
Updates #17148
Change-Id: Ie697dae0e797494fa8ef27fbafa193bfe5ceb307
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
When you say --features=foo,bar, that was supposed to mean
to only show features "foo" and "bar" in the table.
But it was also being used as the set of all features that are
omittable, which was wrong, leading to misleading numbers
when --features was non-empty.
Updates #12614
Change-Id: Idad2fa67fb49c39454032e84a3dede967890fdf5
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
This renames the package+symbols in the earlier 17ffa80138 to be
in their own package ("buildfeatures") and start with the word "Has"
like "if buildfeatures.HasFoo {".
Updates #12614
Change-Id: I510e5f65993e5b76a0e163e3aa4543755213cbf6
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
So code (in upcoming PRs) can test for the build tags with consts and
get dead code elimination from the compiler+linker.
Updates #12614
Change-Id: If6160453ffd01b798f09894141e7631a93385941
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
As of this commit (per the issue), the Taildrive code remains where it
was, but in new files that are protected by the new ts_omit_drive
build tag. Future commits will move it.
Updates #17058
Change-Id: Idf0a51db59e41ae8da6ea2b11d238aefc48b219e
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Its doc said its signature matched a std signature, but it used
Tailscale-specific types.
Nowadays it's the caller (func control) that curries the logf/netmon
and returns the std-matching signature.
Updates #cleanup (while answering a question on Slack)
Change-Id: Ic99de41fc6a1c720575a7f33c564d0bcfd9a2c30
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
This adds a file that's not compiled by default that exists just to
make it easier to do binary size checks, probing what a binary would
be like if it included reflect methods (as used by html/template, etc).
As an example, once tailscaled uses reflect.Type.MethodByName(non-const-string) anywhere,
the build jumps up by 14.5 MB:
$ GOOS=linux GOARCH=amd64 ./tool/go build -tags=ts_include_cli,ts_omit_webclient,ts_omit_systray,ts_omit_debugeventbus -o before ./cmd/tailscaled
$ GOOS=linux GOARCH=amd64 ./tool/go build -tags=ts_include_cli,ts_omit_webclient,ts_omit_systray,ts_omit_debugeventbus,ts_debug_forcereflect -o after ./cmd/tailscaled
$ ls -l before after
-rwxr-xr-x@ 1 bradfitz staff 41011861 Sep 9 07:28 before
-rwxr-xr-x@ 1 bradfitz staff 55610948 Sep 9 07:29 after
This is particularly pronounced with large deps like the AWS SDK. If you compare using ts_omit_aws:
-rwxr-xr-x@ 1 bradfitz staff 38284771 Sep 9 07:40 no-aws-no-reflect
-rwxr-xr-x@ 1 bradfitz staff 45546491 Sep 9 07:41 no-aws-with-reflect
That means adding AWS to a non-reflect binary adds 2.7 MB but adding
AWS to a reflect binary adds 10 MB.
Updates #17063
Updates #12614
Change-Id: I18e9b77c9cf33565ce5bba65ac5584fa9433f7fb
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
I probably could've deflaked this without synctest, but might as well use
it now that Go 1.25 has it.
Fixes#15348
Change-Id: I81c9253fcb7eada079f3e943ab5f1e29ba8e8e31
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
We should never use the real syspolicy implementation in tests by
default. (the machine's configuration shouldn't affect tests)
You either specify a test policy, or you get a no-op one.
Updates #16998
Change-Id: I3350d392aad11573a5ad7caab919bb3bbaecb225
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
There's a TODO to delete all of handler.go, but part of it's
still used in another repo.
But this deletes some.
Updates #17022
Change-Id: Ic5a8a5a694ca258440307436731cd92b45ee2d21
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Now that we have policytest and the policyclient.Client interface, we
can de-global-ify many of the tests, letting them run concurrently
with each other, and just removing global variable complexity.
This does ~half of the LocalBackend ones.
Updates #16998
Change-Id: Iece754e1ef4e49744ccd967fa83629d0dca6f66a
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
This is step 4 of making syspolicy a build-time feature.
This adds a policyclient.Get() accessor to return the correct
implementation to use: either the real one, or the no-op one. (A third
type, a static one for testing, also exists, so in general a
policyclient.Client should be plumbed around and not always fetched
via policyclient.Get whenever possible, especially if tests need to use
alternate syspolicy)
Updates #16998
Updates #12614
Change-Id: Iaf19670744a596d5918acfa744f5db4564272978
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
