age,internal/format: avoid echoing private keys in errors

Reported by Joe Doyle of Trail of Bits.

age_test.go

@@ -18,6 +18,7 @@ import (
 	"testing"
 
 	"filippo.io/age"
+	"filippo.io/age/internal/bech32"
 	"filippo.io/age/plugin"
 )
 
@@ -260,6 +261,59 @@ AGE-SECRET-KEY--1D6K0SGAX3NU66R4GYFZY0UQWCLM3UUSF3CXLW4KXZM342WQSJ82QKU59Q`},
 	}
 }
 
+func TestParseErrorsDoNotIncludeLine(t *testing.T) {
+	x25519, err := age.GenerateX25519Identity()
+	if err != nil {
+		t.Fatal(err)
+	}
+	hybrid, err := age.GenerateHybridIdentity()
+	if err != nil {
+		t.Fatal(err)
+	}
+	// Re-encode the hybrid identity as a plugin identity.
+	_, seed, err := bech32.Decode(hybrid.String())
+	if err != nil {
+		t.Fatal(err)
+	}
+	pluginIdentity, err := bech32.Encode("AGE-PLUGIN-PQ-", seed)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	tests := []struct {
+		name  string
+		line  string
+		parse func(io.Reader) error
+	}{
+		{"identities/plugin", pluginIdentity, func(r io.Reader) error {
+			_, err := age.ParseIdentities(r)
+			return err
+		}},
+		{"recipients/x25519", x25519.String(), func(r io.Reader) error {
+			_, err := age.ParseRecipients(r)
+			return err
+		}},
+		{"recipients/hybrid", hybrid.String(), func(r io.Reader) error {
+			_, err := age.ParseRecipients(r)
+			return err
+		}},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			err := tt.parse(strings.NewReader("# a comment\n" + tt.line + "\n"))
+			if err == nil {
+				t.Fatal("expected an error for an unrecognized line, got nil")
+			}
+			if strings.Contains(err.Error(), tt.line) {
+				t.Errorf("error includes the private key from the file: %v", err)
+			}
+			if !strings.Contains(err.Error(), "line 2") {
+				t.Errorf("error doesn't say which line failed: %v", err)
+			}
+		})
+	}
+}
+
 type testRecipient struct {
 	labels []string
 }

internal/format/format.go

@@ -268,6 +268,19 @@ func errorf(format string, a ...any) error {
 	return &ParseError{fmt.Errorf(format, a...)}
 }
 
+// describeIntro returns a quoted description of a bad intro line, or an empty
+// string if the line contains recognizable private key material.
+func describeIntro(line string) string {
+	for _, prefix := range []string{"AGE-SECRET-KEY-", "AGE-PLUGIN-"} {
+		if strings.HasPrefix(line, prefix) {
+			return ""
+		}
+	}
+	// Preserve enough context to diagnose a mangled intro without echoing an
+	// arbitrarily long first line.
+	return fmt.Sprintf("%q", line[:min(len(line), len(intro))])
+}
+
 // Parse returns the header and a Reader that begins at the start of the
 // payload.
 func Parse(input io.Reader) (*Header, io.Reader, error) {
@@ -277,7 +290,13 @@ func Parse(input io.Reader) (*Header, io.Reader, error) {
 
 	line, err := hr.ReadString('\n')
 	if err == io.EOF {
-		return nil, nil, errorf("file is empty")
+		if len(line) == 0 {
+			return nil, nil, errorf("file is empty")
+		}
+		if description := describeIntro(line); description != "" {
+			return nil, nil, errorf("unexpected EOF reading intro: %s", description)
+		}
+		return nil, nil, errorf("unexpected EOF reading intro, expected %q", intro)
 	} else if err != nil {
 		// headerReader errors are already ParseErrors; don't nest the prefix.
 		if _, ok := err.(*ParseError); ok {
@@ -286,7 +305,10 @@ func Parse(input io.Reader) (*Header, io.Reader, error) {
 		return nil, nil, errorf("failed to read intro: %w", err)
 	}
 	if line != intro {
-		return nil, nil, errorf("unexpected intro: %q", line)
+		if description := describeIntro(line); description != "" {
+			return nil, nil, errorf("unexpected intro: %s", description)
+		}
+		return nil, nil, errorf("unexpected intro, expected %q", intro)
 	}
 
 	sr := &StanzaReader{r: hr}

internal/format/format_test.go

@@ -184,3 +184,81 @@ func FuzzMalleability(f *testing.F) {
 		}
 	})
 }
+
+const secret = "AGE-SECRET-KEY-1NOTAREALKEYNOTAREALKEYNOTAREALKEYNOTAREALKEYNOTAREALKEYNOTA"
+
+const pluginSecret = "AGE-PLUGIN-PQ-1NOTAREALKEYNOTAREALKEYNOTAREALKEYNOTAREALKEYNOTAREALKEYNOTA"
+
+func TestParseIntroErrorIsNotSecret(t *testing.T) {
+	for _, test := range []struct {
+		name   string
+		input  string
+		secret bool
+	}{
+		{"identity", secret + "\n", true},
+		{"identity, no newline", secret, true},
+		{"plugin identity", pluginSecret + "\n", true},
+		{"plugin identity, no newline", pluginSecret, true},
+		{"truncated intro", "age-encryption.org/v1", false},
+		{"binary", "\x00\x01\x02", false},
+	} {
+		t.Run(test.name, func(t *testing.T) {
+			_, _, err := format.Parse(strings.NewReader(test.input))
+			if err == nil {
+				t.Fatal("expected an error")
+			}
+			if strings.Contains(err.Error(), "file is empty") {
+				t.Errorf("error says the file is empty, but it is %d bytes: %v",
+					len(test.input), err)
+			}
+			// Check substrings, not just the key prefix.
+			if test.secret {
+				key := strings.TrimSuffix(test.input, "\n")
+				for i := 0; i+17 <= len(key); i++ {
+					if strings.Contains(err.Error(), key[i:i+17]) {
+						t.Errorf("error includes %q, a run of the first line, "+
+							"which is a private key: %v", key[i:i+17], err)
+						break
+					}
+				}
+			}
+		})
+	}
+
+	// Preserve the error for empty files (#416).
+	t.Run("empty", func(t *testing.T) {
+		_, _, err := format.Parse(strings.NewReader(""))
+		if err == nil {
+			t.Fatal("expected an error")
+		}
+		if !strings.Contains(err.Error(), "file is empty") {
+			t.Errorf("expected an empty file error, got: %v", err)
+		}
+	})
+}
+
+func TestParseIntroErrorShowsMangling(t *testing.T) {
+	for _, test := range []struct {
+		name  string
+		input string
+		want  string
+	}{
+		{"crlf", "age-encryption.org/v1\r\n", `\r`},
+		{"utf8 bom", "\xef\xbb\xbfage-encryption.org/v1\n", `\ufeff`},
+		{"utf16be", "\x00a\x00g\x00e\x00-\x00e\x00n\x00c\x00r\x00y\x00p\x00", `\x00a\x00g\x00e`},
+		{"trailing space", "age-encryption.org/v1 \n", `v1 `},
+		{"wrong version", "age-encryption.org/v2\n", "v2"},
+		{"leading blank line", "\nage-encryption.org/v1\n", `"\n"`},
+	} {
+		t.Run(test.name, func(t *testing.T) {
+			_, _, err := format.Parse(strings.NewReader(test.input))
+			if err == nil {
+				t.Fatal("expected an error")
+			}
+			if !strings.Contains(err.Error(), test.want) {
+				t.Errorf("error does not show the mangling: got %v, want it to contain %q",
+					err, test.want)
+			}
+		})
+	}
+}

parse.go

@@ -57,7 +57,9 @@ func parseIdentity(arg string) (Identity, error) {
 	case strings.HasPrefix(arg, "AGE-SECRET-KEY-PQ-1"):
 		return ParseHybridIdentity(arg)
 	default:
-		return nil, fmt.Errorf("unknown identity type: %q", arg)
+		// Don't include arg in the error: it may contain private key material,
+		// and callers print these errors.
+		return nil, fmt.Errorf("unknown identity type")
 	}
 }
 
@@ -107,6 +109,8 @@ func parseRecipient(arg string) (Recipient, error) {
 	case strings.HasPrefix(arg, "age1"):
 		return ParseX25519Recipient(arg)
 	default:
-		return nil, fmt.Errorf("unknown recipient type: %q", arg)
+		// It might be a private key from an identities file accidentally used
+		// as a recipients file, so don't include arg in the error.
+		return nil, fmt.Errorf("unknown recipient type")
 	}
 }