net/dns: ensure /etc/resolv.conf is world-readable even with a umask

Previously, if we had a umask set (e.g. 0027) that prevented creating a
world-readable file, /etc/resolv.conf would be created without the o+r
bit and thus other users may be unable to resolve DNS.

Since a umask only applies to file creation, chmod the file after
creation and before renaming it to ensure that it has the appropriate
permissions.

Updates #12609

Signed-off-by: Andrew Dunham <[email protected]>
Change-Id: I2a05d64f4f3a8ee8683a70be17a7da0e70933137

net/dns/direct.go

@@ -276,6 +276,14 @@ func (m *directManager) rename(old, new string) error {
 		return fmt.Errorf("writing to %q in rename of %q: %w", new, old, err)
 	}
 
+	// Explicitly set the permissions on the new file. This ensures that
+	// if we have a umask set which prevents creating world-readable files,
+	// the file will still have the correct permissions once it's renamed
+	// into place. See #12609.
+	if err := m.fs.Chmod(new, 0644); err != nil {
+		return fmt.Errorf("chmod %q in rename of %q: %w", new, old, err)
+	}
+
 	if err := m.fs.Remove(old); err != nil {
 		err2 := m.fs.Truncate(old)
 		if err2 != nil {
@@ -467,6 +475,14 @@ func (m *directManager) atomicWriteFile(fs wholeFileFS, filename string, data []
 	if err := fs.WriteFile(tmpName, data, perm); err != nil {
 		return fmt.Errorf("atomicWriteFile: %w", err)
 	}
+	// Explicitly set the permissions on the temporary file before renaming
+	// it. This ensures that if we have a umask set which prevents creating
+	// world-readable files, the file will still have the correct
+	// permissions once it's renamed into place. See #12609.
+	if err := fs.Chmod(tmpName, perm); err != nil {
+		return fmt.Errorf("atomicWriteFile: Chmod: %w", err)
+	}
+
 	return m.rename(tmpName, filename)
 }
 
@@ -475,10 +491,11 @@ func (m *directManager) atomicWriteFile(fs wholeFileFS, filename string, data []
 //
 // All name parameters are absolute paths.
 type wholeFileFS interface {
-	Stat(name string) (isRegular bool, err error)
-	Rename(oldName, newName string) error
-	Remove(name string) error
+	Chmod(name string, mode os.FileMode) error
 	ReadFile(name string) ([]byte, error)
+	Remove(name string) error
+	Rename(oldName, newName string) error
+	Stat(name string) (isRegular bool, err error)
 	Truncate(name string) error
 	WriteFile(name string, contents []byte, perm os.FileMode) error
 }
@@ -502,6 +519,10 @@ func (fs directFS) Stat(name string) (isRegular bool, err error) {
 	return fi.Mode().IsRegular(), nil
 }
 
+func (fs directFS) Chmod(name string, mode os.FileMode) error {
+	return os.Chmod(fs.path(name), mode)
+}
+
 func (fs directFS) Rename(oldName, newName string) error {
 	return os.Rename(fs.path(oldName), fs.path(newName))
 }

net/dns/direct_unix_test.go

@@ -0,0 +1,43 @@
+// Copyright (c) Tailscale Inc & AUTHORS
+// SPDX-License-Identifier: BSD-3-Clause
+
+//go:build unix
+
+package dns
+
+import (
+	"context"
+	"os"
+	"path/filepath"
+	"syscall"
+	"testing"
+)
+
+func TestWriteFileUmask(t *testing.T) {
+	// Set a umask that disallows world-readable files for the duration of
+	// this test.
+	oldUmask := syscall.Umask(0027)
+	defer syscall.Umask(oldUmask)
+
+	tmp := t.TempDir()
+	fs := directFS{prefix: tmp}
+
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+
+	m := directManager{logf: t.Logf, fs: fs, ctx: ctx, ctxClose: cancel}
+
+	const perms = 0644
+	if err := m.atomicWriteFile(fs, "resolv.conf", []byte("nameserver 8.8.8.8\n"), perms); err != nil {
+		t.Fatal(err)
+	}
+
+	// Ensure that the created file has the world-readable bit set.
+	fi, err := os.Stat(filepath.Join(tmp, "resolv.conf"))
+	if err != nil {
+		t.Fatal(err)
+	}
+	if got := fi.Mode().Perm(); got != perms {
+		t.Fatalf("file mode: got 0o%o, want 0o%o", got, perms)
+	}
+}

net/dns/manager_linux_test.go

@@ -313,8 +313,9 @@ func (m memFS) Stat(name string) (isRegular bool, err error) {
 	return false, nil
 }
 
-func (m memFS) Rename(oldName, newName string) error { panic("TODO") }
-func (m memFS) Remove(name string) error             { panic("TODO") }
+func (m memFS) Chmod(name string, mode os.FileMode) error { panic("TODO") }
+func (m memFS) Rename(oldName, newName string) error      { panic("TODO") }
+func (m memFS) Remove(name string) error                  { panic("TODO") }
 func (m memFS) ReadFile(name string) ([]byte, error) {
 	v, ok := m[name]
 	if !ok {

net/dns/wsl_windows.go

@@ -159,6 +159,10 @@ func (fs wslFS) Stat(name string) (isRegular bool, err error) {
 	return true, nil
 }
 
+func (fs wslFS) Chmod(name string, perm os.FileMode) error {
+	return wslRun(fs.cmd("chmod", "--", fmt.Sprintf("%04o", perm), name))
+}
+
 func (fs wslFS) Rename(oldName, newName string) error {
 	return wslRun(fs.cmd("mv", "--", oldName, newName))
 }