Commit Graph

95 Commits (7fffddce8e15bea420d10839666c6e544ef6ed1e)

Author SHA1 Message Date
Aaron Klotz b005b79236 net/dns, paths, util/winutil: change net/dns/windowsManager NRPT management to support more than 50 domains.
AFAICT this isn't documented on MSDN, but based on the issue referenced below,
NRPT rules are not working when a rule specifies > 50 domains.

This patch modifies our NRPT rule generator to split the list of domains
into chunks as necessary, and write a separate rule for each chunk.

For compatibility reasons, we continue to use the hard-coded rule ID, but
as additional rules are required, we generate new GUIDs. Those GUIDs are
stored under the Tailscale registry path so that we know which rules are ours.

I made some changes to winutils to add additional helper functions in support
of both the code and its test: I added additional registry accessors, and also
moved some token accessors from paths to util/winutil.

Fixes https://github.com/tailscale/coral/issues/63

Signed-off-by: Aaron Klotz <aaron@tailscale.com>
3 years ago
Aaron Klotz c163b2a3f1 util/winutil, util/winutil/vss: remove winrestore and vss as they are unnecessary.
I wrote this code way back at the beginning of my tenure at Tailscale when we
had concerns about needing to restore deleted machine keys from backups.

We never ended up using this functionality, and the code is now getting in the
way, so we might as well remove it.

Signed-off-by: Aaron Klotz <aaron@tailscale.com>
3 years ago
Mihai Parparita dd5548771e util/clientmetric: add gauge_ name prefix when uploading names
The prefix is a signal to tsweb to treat this as a gauge metric when
generating the Prometheus version.

Signed-off-by: Mihai Parparita <mihai@tailscale.com>
3 years ago
Maisem Ali fd99c54e10 tailcfg,all: change structs to []*dnstype.Resolver
Signed-off-by: Maisem Ali <maisem@tailscale.com>
3 years ago
Maisem Ali e409e59a54 cmd/cloner,util/codegen: refactor cloner internals to allow reuse
Also run go generate again for Copyright updates.

Signed-off-by: Maisem Ali <maisem@tailscale.com>
3 years ago
Mihai Parparita 025867fd07
util/clientmetric: switch to TestHooks struct for test-only functions (#4632)
Followup to 7966aed1e0 to pick up
review feedback that was accidentally left out.

Signed-off-by: Mihai Parparita <mihai@tailscale.com>
3 years ago
Mihai Parparita 7966aed1e0
util/clientmetric: add test hooks and ResetLastDelta function
Necessary to force flushing of client metrics more aggressively in
dev/test mode.

Signed-off-by: Mihai Parparita <mihai@tailscale.com>
3 years ago
Brad Fitzpatrick 373176ea54 util/codegen: format generated code with goimports, not gofmt
goimports is a superset of gofmt that also groups imports.
(the goimports tool also adds/removes imports as needed, but that
part is disabled here)

Change-Id: Iacf0408dfd9497f4ed3da4fa50e165359ce38498
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
3 years ago
Brad Fitzpatrick 910ae68e0b util/mak: move tailssh's mapSet into a new package for reuse elsewhere
Change-Id: Idfe95db82275fd2be6ca88f245830731a0d5aecf
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
3 years ago
Brad Fitzpatrick 53588f632d Revert "wgengine/router,util/kmod: load & log xt_mark"
This reverts commit 8d6793fd70.

Reason: breaks Android build (cgo/pthreads addition)

We can try again next cycle.

Change-Id: I5e7e1730a8bf399a8acfce546a6d22e11fb835d5
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
3 years ago
James Tucker 8d6793fd70 wgengine/router,util/kmod: load & log xt_mark
Attempt to load the xt_mark kernel module when it is not present. If the
load fails, log error information.

It may be tempting to promote this failure to an error once it has been
in use for some time, so as to avoid reaching an error with the iptables
invocation, however, there are conditions under which the two stages may
disagree - this change adds more useful breadcrumbs.

Example new output from tailscaled running under my WSL2:

```
router: ensure module xt_mark: "/usr/sbin/modprobe xt_mark" failed: exit status 1; modprobe: FATAL: Module xt_mark not found in directory /lib/modules/5.10.43.3-microsoft-standard-WSL2
```

Background:

There are two places to lookup modules, one is `/proc/modules` "old",
the other is `/sys/module/` "new".

There was query_modules(2) in linux <2.6, alas, it is gone.

In a docker container in the default configuration, you would get
/proc/modules and /sys/module/ both populated. lsmod may work file,
modprobe will fail with EPERM at `finit_module()` for an unpriviliged
container.

In a priviliged container the load may *succeed*, if some conditions are
met. This condition should be avoided, but the code landing in this
change does not attempt to avoid this scenario as it is both difficult
to detect, and has a very uncertain impact.

In an nspawn container `/proc/modules` is populated, but `/sys/module`
does not exist. Modern `lsmod` versions will fail to gather most module
information, without sysfs being populated with module information.

In WSL2 modules are likely missing, as the in-use kernel typically is
not provided by the distribution filesystem, and WSL does not mount in a
module filesystem of its own. Notably the WSL2 kernel supports iptables
marks without listing the xt_mark module in /sys/module, and
/proc/modules is empty.

On a recent kernel, we can ask the capabilities system about SYS_MODULE,
that will help to disambiguate between the non-privileged container case
and just being root. On older kernels these calls may fail.

Update #4329

Signed-off-by: James Tucker <james@tailscale.com>
3 years ago
Maisem Ali 909f40da84
util/groupmember: remove redundant code (#4298)
Now that we have 30faf968b1
this is no longer needed.

Fixes #3001

Signed-off-by: Maisem Ali <maisem@tailscale.com>
3 years ago
Josh Bleecher Snyder 32fd42430b all: use cibuild.On
Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
3 years ago
Josh Bleecher Snyder 0868329936 all: use any instead of interface{}
My favorite part of generics.

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
3 years ago
Brad Fitzpatrick 61ee72940c all: use Go 1.18's strings.Cut
More remain.

Change-Id: I6ec562cc1f687600758deae1c9d7dbd0d04004cb
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
3 years ago
Josh Bleecher Snyder 97a01b7b17 util/deephash: remove Tailscale toolchain compatibility shim
The future is now.

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
3 years ago
Josh Bleecher Snyder 463728a885 util/netconv: add package to convert between netip and netaddr types
Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
3 years ago
Aaron Klotz 82cd98609f util/winutil: migrate corp's winutil into OSS.
It makes the most sense to have all our utility functions reside in one place.
There was nothing in corp that could not reasonably live in OSS.

I also updated `StartProcessAsChild` to no longer depend on `futureexec`,
thus reducing the amount of code that needed migration. I tested this change
with `tswin` and it is working correctly.

I have a follow-up PR to remove the corresponding code from corp.

The migrated code was mostly written by @alexbrainman.
Sourced from corp revision 03e90cfcc4dd7b8bc9b25eb13a26ec3a24ae0ef9

Signed-off-by: Aaron Klotz <aaron@tailscale.com>
3 years ago
Aaron Klotz 3a74f2d2d7 cmd/tailscaled, util/winutil: add accessor functions for Windows system policies.
This patch adds new functions to be used when accessing system policies,
and revises callers to use the new functions. They first attempt the new
registry path for policies, and if that fails, attempt to fall back to the
legacy path.

We keep non-policy variants of these functions because we should be able to
retain the ability to read settings from locations that are not exposed to
sysadmins for group policy edits.

The remaining changes will be done in corp.

Updates https://github.com/tailscale/tailscale/issues/3584

Signed-off-by: Aaron Klotz <aaron@tailscale.com>
3 years ago
Brad Fitzpatrick 0626cf4183 util/winutil: fix build
It was broken on Windows:

Error: util\winutil\winutil_windows.go:15:7: regBase redeclared in this block
Error:                                       D:\a\tailscale\tailscale\util\winutil\winutil_notwindows.go:7:17: previous declaration
Error: util\winutil\winutil_windows.go:29:6: getRegString redeclared in this block
Error:                                       D:\a\tailscale\tailscale\util\winutil\winutil_notwindows.go:9:40: previous declaration
Error: util\winutil\winutil_windows.go:47:6: getRegInteger redeclared in this block
Error:                                       D:\a\tailscale\tailscale\util\winutil\winutil_notwindows.go:11:48: previous declaration
Error: util\winutil\winutil_windows.go:77:6: isSIDValidPrincipal redeclared in this block
Error:                                       D:\a\tailscale\tailscale\util\winutil\winutil_notwindows.go:13:38: previous declaration

Change-Id: Ib1ce4b647f5711547840c736b933a6c42bf09583
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
3 years ago
Aaron Klotz d7962e3bcf ipn/ipnserver, util/winutil: update workaround for os/user.LookupId failures on Windows to reject SIDs from deleted/invalid security principals.
Our current workaround made the user check too lax, thus allowing deleted
users. This patch adds a helper function to winutil that checks that the
uid's SID represents a valid Windows security principal.

Now if `lookupUserFromID` determines that the SID is invalid, we simply
propagate the error.

Updates https://github.com/tailscale/tailscale/issues/869

Signed-off-by: Aaron Klotz <aaron@tailscale.com>
3 years ago
Brad Fitzpatrick 486059589b all: gofmt -w -s (simplify) tests
And it updates the build tag style on a couple files.

Change-Id: I84478d822c8de3f84b56fa1176c99d2ea5083237
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
3 years ago
Brad Fitzpatrick 0de1b74fbb util/clientmetric: add tests omitted from earlier commit
These were supposed to be part of
3b541c833e but I guess I forgot to "git
add" them. Whoops.

Updates #3307

Change-Id: I8c768a61ec7102a01799e81dc502a22399b9e9f0
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
3 years ago
Brad Fitzpatrick 5b5f032c9a util/clientmetric: optimize memory layout for finding updates
Updates #3307

Change-Id: I2840b190583467cc3f00688b96ce3d170df46a46
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
3 years ago
Brad Fitzpatrick 3b541c833e util/clientmetric, logtail: log metric changes
Updates #3307

Change-Id: I1399ebd786f6ff7defe6e11c0eb651144c071574
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
3 years ago
Brad Fitzpatrick 57b039c51d util/clientmetrics: add new package to add metrics to the client
And annotate magicsock as a start.

And add localapi and debug handlers with the Prometheus-format
exporter.

Updates #3307

Change-Id: I47c5d535fe54424741df143d052760387248f8d3
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
3 years ago
David Anderson 0532eb30db all: replace tailcfg.DiscoKey with key.DiscoPublic.
Signed-off-by: David Anderson <danderson@tailscale.com>
3 years ago
Josh Bleecher Snyder 3fd5f4380f util/multierr: new package
github.com/go-multierror/multierror served us well.
But we need a few feature from it (implement Is),
and it's not worth maintaining a fork of such a small module.

Instead, I did a clean room implementation inspired by its API.

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
3 years ago
David Anderson a9c78910bd wgengine/wgcfg: convert to use new node key type.
Updates #3206

Signed-off-by: David Anderson <danderson@tailscale.com>
3 years ago
Aaron Klotz c6ea282b3f utils/winutil utils/winutil/vss: add utility function for extracting data from Windows System Restore Point backups.
utils/winutil/vss contains just enough COM wrapping to query the Volume Shadow Copy service for snapshots.
WalkSnapshotsForLegacyStateDir is the friendlier interface that adds awareness of our actual use case,
mapping the snapshots and locating our legacy state directory.

Updates #3011

Signed-off-by: Aaron Klotz <aaron@tailscale.com>
3 years ago
Brad Fitzpatrick 0410f1a35a util/groupmember: adjust build tags for osusergo
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
3 years ago
Brad Fitzpatrick 2a0d3f72c5 util/groupmember: fix typo of package's name in its package doc
And canonicalize a func comment while I'm here.

Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
3 years ago
Aaron Klotz 3386a86fe5 util/winutil: add GetRegInteger
This helper allows us to retrieve `DWORD` and `QWORD` values from the Tailscale key in the Windows registry.

Signed-off-by: Aaron Klotz <aaron@tailscale.com>
3 years ago
Josh Bleecher Snyder b14db5d943 util/codegen: reorder AssertStructUnchanged args
The fully qualified name of the type is thisPkg.tname,
so write the args like that too.

Suggested-by: Joe Tsai <joetsai@digital-static.net>
Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
3 years ago
Josh Bleecher Snyder 3cd85c0ca6 util/codegen: add ContainsPointers
And use it in cmd/cloner.

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
3 years ago
Josh Bleecher Snyder d8a8f70000 util/codegen: add NamedTypes
And use it in cmd/cloner.

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
3 years ago
Josh Bleecher Snyder 618376dbc0 util/codegen: add AssertStructUnchanged
Refactored out from cmd/cloner.

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
3 years ago
Josh Bleecher Snyder fb66ff7c78 util/codegen: add package
This is a package for shared utilities used in doing codegen programs.
The inaugural API is for writing gofmt'd code to a file.

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
3 years ago
David Anderson bb10443edf wgengine/wgcfg: use just the hexlified node key as the WireGuard endpoint.
The node key is all magicsock needs to find the endpoint that WireGuard
needs.

Updates #2752

Signed-off-by: David Anderson <danderson@tailscale.com>
3 years ago
David Anderson 54bc3b7d97 util/deephash: remove soon to be deleted field from wgcfg.
Signed-off-by: David Anderson <danderson@tailscale.com>
3 years ago
Joe Tsai 3f1317e3e5
util/deephash: fix TestArrayAllocs
Unfortunately this test fails on certain architectures.
The problem comes down to inconsistencies in the Go escape analysis
where specific variables are marked as escaping on certain architectures.
The variables escaping to the heap are unfortunately in crypto/sha256,
which makes it impossible to fixthis locally in deephash.

For now, fix the test by compensating for the allocations that
occur from calling sha256.digest.Sum.

See golang/go#48055

Fixes #2727

Signed-off-by: Joe Tsai <joetsai@digital-static.net>
3 years ago
slowy07 ac0353e982 fix: typo spelling grammar
Signed-off-by: slowy07 <slowy.arfy@gmail.com>
3 years ago
David Crawshaw 360223fccb types/dnstype: introduce new package for Resolver
So the type can be used in net/dns without introducing a tailcfg
dependency.

For #2596

Signed-off-by: David Crawshaw <crawshaw@tailscale.com>
3 years ago
Josh Bleecher Snyder a5da4ed981 all: gofmt with Go 1.17
This adds "//go:build" lines and tidies up existing "// +build" lines.

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
3 years ago
Joe Tsai 9d0c86b6ec
util/deephash: remove unnecessary formatting for structs and slices (#2571)
The index for every struct field or slice element and
the number of fields for the struct is unncessary.

The hashing of Go values is unambiguous because every type (except maps)
encodes in a parsable manner. So long as we know the type information,
we could theoretically decode every value (except for maps).

At a high level:
* numbers are encoded as fixed-width records according to precision.
* strings (and AppendTo output) are encoded with a fixed-width length,
followed by the contents of the buffer.
* slices are prefixed by a fixed-width length, followed by the encoding
of each value. So long as we know the type of each element, we could
theoretically decode each element.
* arrays are encoded just like slices, but elide the length
since it is determined from the Go type.
* maps are encoded first with a byte indicating whether it is a cycle.
If a cycle, it is followed by a fixed-width index for the pointer,
otherwise followed by the SHA-256 hash of its contents. The encoding of maps
is not decodeable, but a SHA-256 hash is sufficient to avoid ambiguities.
* interfaces are encoded first with a byte indicating whether it is nil.
If not nil, it is followed by a fixed-width index for the type,
and then the encoding for the underlying value. Having the type be encoded
first ensures that the value could theoretically be decoded next.
* pointers are encoded first with a byte indicating whether it is
1) nil, 2) a cycle, or 3) newly seen. If a cycle, it is followed by
a fixed-width index for the pointer. If newly seen, it is followed by
the encoding for the pointed-at value.

Removing unnecessary details speeds up hashing:

	name              old time/op    new time/op    delta
	Hash-8              76.0µs ± 1%    55.8µs ± 2%  -26.62%        (p=0.000 n=10+10)
	HashMapAcyclic-8    61.9µs ± 0%    62.0µs ± 0%     ~             (p=0.666 n=9+9)
	TailcfgNode-8       10.2µs ± 1%     7.5µs ± 1%  -26.90%         (p=0.000 n=10+9)
	HashArray-8         1.07µs ± 1%    0.70µs ± 1%  -34.67%         (p=0.000 n=10+9)

Signed-off-by: Joe Tsai <joetsai@digital-static.net>
3 years ago
Joe Tsai d8fbce7eef
util/deephash: hash uint{8,16,32,64} explicitly (#2502)
Instead of hashing the humanly formatted forms of a number,
hash the native machine bits of the integers themselves.

There is a small performance gain for this:
	name              old time/op    new time/op    delta
	Hash-8              75.7µs ± 1%    76.0µs ± 2%    ~            (p=0.315 n=10+9)
	HashMapAcyclic-8    63.1µs ± 3%    61.3µs ± 1%  -2.77%        (p=0.000 n=10+10)
	TailcfgNode-8       10.3µs ± 1%    10.2µs ± 1%  -1.48%        (p=0.000 n=10+10)
	HashArray-8         1.07µs ± 1%    1.05µs ± 1%  -1.79%        (p=0.000 n=10+10)

Signed-off-by: Joe Tsai <joetsai@digital-static.net>
3 years ago
Joe Tsai 01d4dd331d
util/deephash: simplify hasher.hashMap (#2503)
The swapping of bufio.Writer between hasher and mapHasher is subtle.
Just embed a hasher in mapHasher to avoid complexity here.

No notable change in performance:
	name              old time/op    new time/op    delta
	Hash-8              76.7µs ± 1%    77.0µs ± 1%    ~            (p=0.182 n=9+10)
	HashMapAcyclic-8    62.4µs ± 1%    62.5µs ± 1%    ~            (p=0.315 n=10+9)
	TailcfgNode-8       10.3µs ± 1%    10.3µs ± 1%  -0.62%         (p=0.004 n=10+9)
	HashArray-8         1.07µs ± 1%    1.06µs ± 1%  -0.98%          (p=0.001 n=8+9)

Signed-off-by: Joe Tsai <joetsai@digital-static.net>
3 years ago
Joe Tsai d145c594ad
util/deephash: improve cycle detection (#2470)
The previous algorithm used a map of all visited pointers.
The strength of this approach is that it quickly prunes any nodes
that we have ever visited before. The detriment of the approach
is that pruning is heavily dependent on the order that pointers
were visited. This is especially relevant for hashing a map
where map entries are visited in a non-deterministic manner,
which would cause the map hash to be non-deterministic
(which defeats the point of a hash).

This new algorithm uses a stack of all visited pointers,
similar to how github.com/google/go-cmp performs cycle detection.
When we visit a pointer, we push it onto the stack, and when
we leave a pointer, we pop it from the stack.
Before visiting a pointer, we first check whether the pointer exists
anywhere in the stack. If yes, then we prune the node.
The detriment of this approach is that we may hash a node more often
than before since we do not prune as aggressively.

The set of visited pointers up until any node is only the
path of nodes up to that node and not any other pointers
that may have been visited elsewhere. This provides us
deterministic hashing regardless of visit order.
We can now delete hashMapFallback and associated complexity,
which only exists because the previous approach was non-deterministic
in the presence of cycles.

This fixes a failure of the old algorithm where obviously different
values are treated as equal because the pruning was too aggresive.
See https://github.com/tailscale/tailscale/issues/2443#issuecomment-883653534

The new algorithm is slightly slower since it prunes less aggresively:
	name              old time/op    new time/op    delta
	Hash-8              66.1µs ± 1%    68.8µs ± 1%   +4.09%        (p=0.000 n=19+19)
	HashMapAcyclic-8    63.0µs ± 1%    62.5µs ± 1%   -0.76%        (p=0.000 n=18+19)
	TailcfgNode-8       9.79µs ± 2%    9.88µs ± 1%   +0.95%        (p=0.000 n=19+17)
	HashArray-8          643ns ± 1%     653ns ± 1%   +1.64%        (p=0.000 n=19+19)
However, a slower but more correct algorithm seems
more favorable than a faster but incorrect algorithm.

Signed-off-by: Joe Tsai <joetsai@digital-static.net>
3 years ago
Joe Tsai d666bd8533
util/deephash: disambiguate hashing of AppendTo (#2483)
Prepend size to AppendTo output.

Fixes #2443

Signed-off-by: Joe Tsai <joetsai@digital-static.net>
3 years ago
Joe Tsai 23ad028414
util/deephash: include type as part of hash for interfaces (#2476)
A Go interface may hold any number of different concrete types.
Just because two underlying values hash to the same thing
does not mean the two values are identical if they have different
concrete types. As such, include the type in the hash.
3 years ago