diff --git a/examples/config.yml b/examples/config.yml index 1310613c..593491b9 100644 --- a/examples/config.yml +++ b/examples/config.yml @@ -245,7 +245,7 @@ tun: # For Linux: a single `%d` anywhere in the name is treated as a template and replaced with the # lowest number that yields an unused device name (e.g. `nebula%d` becomes `nebula0`, then `nebula1`, and so on, `neb%dprod` becomes `neb0prod`). # Only on Linux: `nebula%d` is the default if tun.dev is unset. - # The resulting name must be shorter than the kernel limit of 16 characters. + # The name, both before and after %d substitution, must be shorter than the kernel limit of 16 characters. # For macOS: if set, must be in the form `utun[0-9]+`. # For NetBSD: Required to be set, must be in the form `tun[0-9]+` dev: nebula1 diff --git a/overlay/tun_linux.go b/overlay/tun_linux.go index 69b47abe..09802312 100644 --- a/overlay/tun_linux.go +++ b/overlay/tun_linux.go @@ -251,10 +251,14 @@ func newTunFromFd(c *config.C, l *slog.Logger, deviceFd int, vpnNetworks []netip } func newTun(c *config.C, l *slog.Logger, vpnNetworks []netip.Prefix, multiqueue bool) (*tun, error) { - // Resolve (and validate) the device name up front so a bad tun.dev fails - // fast, before we open /dev/net/tun or leak a file descriptor. - tunName, err := findNextTunName(c.GetString("tun.dev", "nebula%d")) - if err != nil { + // Validate the device name up front so a bad tun.dev fails fast, before we + // open /dev/net/tun or leak a file descriptor. A single %d in the name is + // substituted by the kernel during TUNSETIFF (dev_alloc_name) with the + // lowest number that yields an unused device name. Resolving the template + // in the kernel keeps the pick-a-name/create-the-device pair atomic, so + // concurrent callers can never race each other to the same name. + tunName := c.GetString("tun.dev", "nebula%d") + if err := validateTunName(tunName); err != nil { return nil, err } @@ -318,55 +322,14 @@ func validateTunName(tunName string) error { if tunName == "%d" { return errors.New("please don't name your tun device '%d'") } - // The shortest name a template can produce replaces %d with a single digit; - // if even that is not shorter than IFNAMSIZ the template can never yield a - // usable name. - if len(tunName)-len("%d")+len("0") >= unix.IFNAMSIZ { - return fmt.Errorf("tun.dev template %q would result in a name that is not shorter than the maximum device name length of %d", tunName, unix.IFNAMSIZ) + // The kernel substitutes the %d itself and requires the template, like a + // literal name, to be NUL-terminated within IFNAMSIZ bytes. + if len(tunName) >= unix.IFNAMSIZ { + return fmt.Errorf("tun.dev template %q is not shorter than the maximum device name length of %d", tunName, unix.IFNAMSIZ) } return nil } -// findNextTunName resolves a tun.dev value into a concrete device name. A value -// without a "%d" is returned unchanged; a "%d" placeholder (anywhere in the -// name) has the lowest unused integer substituted in based on the devices -// currently present. -func findNextTunName(tunName string) (string, error) { - if err := validateTunName(tunName); err != nil { - return "", err - } - if !strings.Contains(tunName, "%d") { - return tunName, nil - } - - links, err := netlink.LinkList() - if err != nil { - return "", err - } - used := make(map[string]struct{}, len(links)) - for _, link := range links { - used[link.Attrs().Name] = struct{}{} - } - return nextTunName(tunName, used) -} - -// nextTunName substitutes the lowest unused integer into a template's "%d" -// placeholder, skipping any name present in used. tunName is assumed to have -// already passed validateTunName (exactly one "%d", room for a digit). It errors -// only if every candidate that is shorter than IFNAMSIZ is already taken. -func nextTunName(tunName string, used map[string]struct{}) (string, error) { - prefix, suffix, _ := strings.Cut(tunName, "%d") - for i := 0; ; i++ { - candidateName := fmt.Sprintf("%s%d%s", prefix, i, suffix) - if len(candidateName) >= unix.IFNAMSIZ { - return "", fmt.Errorf("all device names matching template %q shorter than the maximum length of %d are already in use", tunName, unix.IFNAMSIZ) - } - if _, taken := used[candidateName]; !taken { - return candidateName, nil - } - } -} - // newTunGeneric does all the stuff common to different tun initialization paths. It will close your files on error. func newTunGeneric(c *config.C, l *slog.Logger, fd int, vpnNetworks []netip.Prefix) (*tun, error) { tfd, err := newTunFd(fd) diff --git a/overlay/tun_linux_test.go b/overlay/tun_linux_test.go index bead0b08..12c52408 100644 --- a/overlay/tun_linux_test.go +++ b/overlay/tun_linux_test.go @@ -38,14 +38,6 @@ func TestTunAdvMSS(t *testing.T) { } } -func nameSet(names ...string) map[string]struct{} { - used := make(map[string]struct{}, len(names)) - for _, n := range names { - used[n] = struct{}{} - } - return used -} - func TestValidateTunName(t *testing.T) { // A device name must be shorter than IFNAMSIZ (i.e. IFNAMSIZ-1 chars max). maxLenName := strings.Repeat("a", unix.IFNAMSIZ-1) @@ -61,11 +53,12 @@ func TestValidateTunName(t *testing.T) { {"trailing template is fine", "nebula%d", false}, {"mid-string template is fine", "neb%dprod", false}, {"leading template is fine", "%dnebula", false}, - {"template at the max static length is fine", strings.Repeat("a", unix.IFNAMSIZ-2) + "%d", false}, + {"template at the max length is fine", strings.Repeat("a", unix.IFNAMSIZ-3) + "%d", false}, + {"template at IFNAMSIZ is rejected", strings.Repeat("a", unix.IFNAMSIZ-2) + "%d", true}, {"bare %d is rejected", "%d", true}, {"multiple %d is rejected", "neb%d%dprod", true}, - {"template with no room for a digit is rejected", strings.Repeat("a", unix.IFNAMSIZ-1) + "%d", true}, - {"mid-string template with no room for a digit is rejected", "neb%d" + strings.Repeat("a", unix.IFNAMSIZ-3), true}, + {"over-long template is rejected", strings.Repeat("a", unix.IFNAMSIZ-1) + "%d", true}, + {"over-long mid-string template is rejected", "neb%d" + strings.Repeat("a", unix.IFNAMSIZ-3), true}, } for _, tt := range tests { @@ -80,48 +73,3 @@ func TestValidateTunName(t *testing.T) { }) } } - -func TestNextTunName(t *testing.T) { - // A prefix long enough that only single-digit suffixes (0-9) fit within - // IFNAMSIZ, so marking all ten used exercises running out of names. - longPrefix := strings.Repeat("a", unix.IFNAMSIZ-2) - longUsed := make([]string, 0, 10) - for i := 0; i < 10; i++ { - longUsed = append(longUsed, longPrefix+string(rune('0'+i))) - } - - tests := []struct { - name string - tmpl string - used map[string]struct{} - want string - wantErr bool - }{ - {"nothing used picks zero", "nebula%d", nil, "nebula0", false}, - {"skips taken names", "nebula%d", nameSet("nebula0", "nebula1"), "nebula2", false}, - {"picks the lowest free index", "nebula%d", nameSet("nebula0", "nebula2"), "nebula1", false}, - {"ignores unrelated names", "nebula%d", nameSet("eth0", "tun5"), "nebula0", false}, - {"mid-string placeholder picks zero", "neb%dprod", nil, "neb0prod", false}, - {"mid-string placeholder skips taken", "neb%dprod", nameSet("neb0prod", "neb1prod"), "neb2prod", false}, - {"leading placeholder picks zero", "%dnebula", nameSet("tun0"), "0nebula", false}, - {"runs out of names within IFNAMSIZ", longPrefix + "%d", nameSet(longUsed...), "", true}, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got, err := nextTunName(tt.tmpl, tt.used) - if tt.wantErr { - if err == nil { - t.Fatalf("expected an error, got name %q", got) - } - return - } - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if got != tt.want { - t.Errorf("got %q, want %q", got, tt.want) - } - }) - } -}