From c45ccfd2151faac82b20dd26764fe082b803e3a4 Mon Sep 17 00:00:00 2001 From: Filippo Valsorda Date: Sat, 29 Aug 2026 16:15:09 +0200 Subject: [PATCH] age,internal/format: avoid echoing private keys in errors Reported by Joe Doyle of Trail of Bits. --- age_test.go | 54 +++++++++++++++++++++++ internal/format/format.go | 26 +++++++++++- internal/format/format_test.go | 78 ++++++++++++++++++++++++++++++++++ parse.go | 8 +++- 4 files changed, 162 insertions(+), 4 deletions(-) diff --git a/age_test.go b/age_test.go index 22e1579..9c17a8c 100644 --- a/age_test.go +++ b/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 } diff --git a/internal/format/format.go b/internal/format/format.go index b274653..af81d9a 100644 --- a/internal/format/format.go +++ b/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} diff --git a/internal/format/format_test.go b/internal/format/format_test.go index 74344db..8f8a010 100644 --- a/internal/format/format_test.go +++ b/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) + } + }) + } +} diff --git a/parse.go b/parse.go index 99c15d4..a261665 100644 --- a/parse.go +++ b/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") } }