ipn/ipnlocal: improve redactErr to handle more cases

This handles the case where the inner *os.PathError is wrapped in
another error type, and additionally will redact errors of type
*os.LinkError. Finally, add tests to verify that redaction works.

Signed-off-by: Andrew Dunham <andrew@du.nham.ca>
Change-Id: Ie83424ff6c85cdb29fb48b641330c495107aab7c

cmd/tailscaled/depaware.txt

@@ -407,6 +407,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de
         flag                                                         from tailscale.com/control/controlclient+
         fmt                                                          from compress/flate+
         hash                                                         from crypto+
+        hash/adler32                                                 from tailscale.com/ipn/ipnlocal
         hash/crc32                                                   from compress/gzip+
         hash/fnv                                                     from tailscale.com/wgengine/magicsock+
         hash/maphash                                                 from go4.org/mem

ipn/ipnlocal/peerapi.go

@@ -10,6 +10,7 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"hash/adler32"
 	"hash/crc32"
 	"html"
 	"io"
@@ -341,11 +342,81 @@ func (s *peerAPIServer) DeleteFile(baseName string) error {
 // accidentally logging actual filenames anywhere.
 const redacted = "redacted"
 
+type redactedErr struct {
+	msg   string
+	inner error
+}
+
+func (re *redactedErr) Error() string {
+	return re.msg
+}
+
+func (re *redactedErr) Unwrap() error {
+	return re.inner
+}
+
+func redactString(s string) string {
+	hash := adler32.Checksum([]byte(s))
+
+	var buf [len(redacted) + len(".12345678")]byte
+	b := append(buf[:0], []byte(redacted)...)
+	b = append(b, '.')
+	b = strconv.AppendUint(b, uint64(hash), 16)
+	return string(b)
+}
+
 func redactErr(err error) error {
-	if pe, ok := err.(*os.PathError); ok {
+	var redactStrings []string
-		pe.Path = redacted
+
+	var pe *os.PathError
+	if errors.As(err, &pe) {
+		// If this is the root error, then we can redact it directly.
+		if err == pe {
+			pe.Path = redactString(pe.Path)
+			return pe
+		}
+
+		// Otherwise, we have a *PathError somewhere in the error
+		// chain, and we can't redact it because something later in the
+		// chain may have cached the Error() return already (as
+		// fmt.Errorf does).
+		//
+		// Add this path to the set of paths that we will redact, below.
+		redactStrings = append(redactStrings, pe.Path)
+
+		// Also redact the Path value so that anything that calls
+		// Unwrap in the future gets the redacted value.
+		pe.Path = redactString(pe.Path)
 	}
-	return err
+
+	var le *os.LinkError
+	if errors.As(err, &le) {
+		// If this is the root error, then we can redact it directly.
+		if err == le {
+			le.New = redactString(le.New)
+			le.Old = redactString(le.Old)
+			return le
+		}
+
+		// As above
+		redactStrings = append(redactStrings, le.New, le.Old)
+		le.New = redactString(le.New)
+		le.Old = redactString(le.Old)
+	}
+
+	if len(redactStrings) == 0 {
+		return err
+	}
+
+	s := err.Error()
+	for _, toRedact := range redactStrings {
+		s = strings.Replace(s, toRedact, redactString(toRedact), -1)
+	}
+
+	// Stringify and and replace any paths that we found above, then return
+	// the error wrapped in a type that uses the newly-redacted string
+	// while also allowing Unwrap()-ing to the inner error type(s).
+	return &redactedErr{msg: s, inner: err}
 }
 
 func touchFile(path string) error {

ipn/ipnlocal/peerapi_test.go

@@ -6,6 +6,7 @@ package ipnlocal
 
 import (
 	"bytes"
+	"errors"
 	"fmt"
 	"io"
 	"io/fs"
@@ -685,3 +686,67 @@ func TestPeerAPIReplyToDNSQueries(t *testing.T) {
 		t.Errorf("unexpectedly IPv6 deny; wanted to be a DNS server")
 	}
 }
+
+func TestRedactErr(t *testing.T) {
+	testCases := []struct {
+		name string
+		err  func() error
+		want string
+	}{
+		{
+			name: "PathError",
+			err: func() error {
+				return &os.PathError{
+					Op:   "open",
+					Path: "/tmp/sensitive.txt",
+					Err:  fs.ErrNotExist,
+				}
+			},
+			want: `open redacted.41360718: file does not exist`,
+		},
+		{
+			name: "LinkError",
+			err: func() error {
+				return &os.LinkError{
+					Op:  "symlink",
+					Old: "/tmp/sensitive.txt",
+					New: "/tmp/othersensitive.txt",
+					Err: fs.ErrNotExist,
+				}
+			},
+			want: `symlink redacted.41360718 redacted.6bcf093a: file does not exist`,
+		},
+		{
+			name: "something else",
+			err:  func() error { return errors.New("i am another error type") },
+			want: `i am another error type`,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			// For debugging
+			var i int
+			for err := tc.err(); err != nil; err = errors.Unwrap(err) {
+				t.Logf("%d: %T @ %p", i, err, err)
+				i++
+			}
+
+			t.Run("Root", func(t *testing.T) {
+				got := redactErr(tc.err()).Error()
+				if got != tc.want {
+					t.Errorf("err = %q; want %q", got, tc.want)
+				}
+			})
+			t.Run("Wrapped", func(t *testing.T) {
+				wrapped := fmt.Errorf("wrapped error: %w", tc.err())
+				want := "wrapped error: " + tc.want
+
+				got := redactErr(wrapped).Error()
+				if got != want {
+					t.Errorf("err = %q; want %q", got, want)
+				}
+			})
+		})
+	}
+}