From 15df6e2cf71bd7457d6a7e1c15754030dc6a9304 Mon Sep 17 00:00:00 2001 From: Filippo Valsorda Date: Sun, 3 Jan 2021 20:57:09 +0100 Subject: [PATCH] internal/format: require the last line of stanzas to be short We are going to reuse the stanza format for IPC in the plugin protocol, but in that context we need stanzas to be self-closing. Currently they almost are, but if the body is 0 modulo 48, there is no way to know if the stanza is over after the last line. Now, all stanzas have to end with a short line, even if empty. No ciphertexts generated by age in the past are affected, but 3% of the ciphertexts generated by rage will now stop working. They are still supported by rage going forward. If it turns out to be a common issue, we can add an exception. --- cmd/age/testdata/empty_recipient_body.age | 11 ++--- cmd/age/testdata/empty_recipient_body_key.txt | 8 +--- internal/format/format.go | 24 +++++------ internal/format/format_test.go | 41 +++++++++++++++++++ 4 files changed, 60 insertions(+), 24 deletions(-) create mode 100644 internal/format/format_test.go diff --git a/cmd/age/testdata/empty_recipient_body.age b/cmd/age/testdata/empty_recipient_body.age index 55769e9..d101cf6 100644 --- a/cmd/age/testdata/empty_recipient_body.age +++ b/cmd/age/testdata/empty_recipient_body.age @@ -1,6 +1,7 @@ age-encryption.org/v1 --> ssh-ed25519 o1Hudg SZISkI5Qn8YgUBmTKG/Zp/QpFjXWvAivzvB+hOcN5W8 -dYfwGWYvCwpSU5EXIC1XqfXdsBvCi3kMypdqCVShrpk --> joint-oil-hw ---- gC/27VAgqOEzAQMKHvBjih7sJ1oDKht+HNdguTIbjt8 -fëtAeµÖ¨&8{Ëéðνcat—íΘœ¯šË·}«=šC†Ÿu \ No newline at end of file +-> X25519 alRneDshIh43nwyD5+fhuTD5TReSn88f2us4hzZPyzU +pGduNK5MUhnuzMxW0qbZnC2k7mRzz69bbJpKQrRc7uc +-> A7)h-grease !,_ + +--- 5bA0uXjBxI6wuI5SseCRgD5/G8LkSVISRe/hnrQMb9s +¦Ã 1‘·¬Žä6_RÙäÚ…›…U<ï1ús®ÿ?`Ö+–’$§HÖW‚v?w8ZW \ No newline at end of file diff --git a/cmd/age/testdata/empty_recipient_body_key.txt b/cmd/age/testdata/empty_recipient_body_key.txt index a91b78c..3366a0f 100644 --- a/cmd/age/testdata/empty_recipient_body_key.txt +++ b/cmd/age/testdata/empty_recipient_body_key.txt @@ -1,7 +1 @@ ------BEGIN OPENSSH PRIVATE KEY----- -b3BlbnNzaC1rZXktdjEAAAAABG5vbmUAAAAEbm9uZQAAAAAAAAABAAAAMwAAAAtzc2gtZW -QyNTUxOQAAACAwKgrb/LkvtI887QylSoUh5xUlKr1fb37euR6et5jHowAAAJgxqUx+MalM -fgAAAAtzc2gtZWQyNTUxOQAAACAwKgrb/LkvtI887QylSoUh5xUlKr1fb37euR6et5jHow -AAAEC7gKj74YIwaM1BT2tnODjfeZJvo8lcazvL6Uljv3+nIDAqCtv8uS+0jzztDKVKhSHn -FSUqvV9vft65Hp63mMejAAAADnJ1bm5lckBmdi1hejMyAQIDBAUGBw== ------END OPENSSH PRIVATE KEY----- +AGE-SECRET-KEY-1TRYTV7PQS5XPUYSTAQZCD7DQCWC7Q77YJD7UVFJRMW4J82Q6930QS70MRX diff --git a/internal/format/format.go b/internal/format/format.go index d84ba2b..8f79b61 100644 --- a/internal/format/format.go +++ b/internal/format/format.go @@ -47,7 +47,8 @@ const BytesPerLine = ColumnsPerLine / 4 * 3 // NewlineWriter returns a Writer that writes to dst, inserting an LF character // every ColumnsPerLine bytes. It does not insert a newline neither at the -// beginning nor at the end of the stream. +// beginning nor at the end of the stream, but it ensures the last line is +// shorter than ColumnsPerLine, which means it might be empty. func NewlineWriter(dst io.Writer) io.Writer { return &newlineWriter{dst: dst} } @@ -63,17 +64,16 @@ func (w *newlineWriter) Write(p []byte) (int, error) { panic("age: internal error: non-empty newlineWriter.buf") } for len(p) > 0 { - remainingInLine := ColumnsPerLine - (w.written % ColumnsPerLine) - if remainingInLine == ColumnsPerLine && w.written != 0 { - w.buf.Write([]byte("\n")) - } - toWrite := remainingInLine + toWrite := ColumnsPerLine - (w.written % ColumnsPerLine) if toWrite > len(p) { toWrite = len(p) } n, _ := w.buf.Write(p[:toWrite]) w.written += n p = p[n:] + if w.written%ColumnsPerLine == 0 { + w.buf.Write([]byte("\n")) + } } if _, err := w.buf.WriteTo(w.dst); err != nil { // We always return n = 0 on error because it's hard to work back to the @@ -101,9 +101,6 @@ func (r *Stanza) Marshal(w io.Writer) error { if _, err := io.WriteString(w, "\n"); err != nil { return err } - if len(r.Body) == 0 { - return nil - } ww := base64.NewEncoder(b64, NewlineWriter(w)) if _, err := ww.Write(r.Body); err != nil { return err @@ -169,6 +166,9 @@ func Parse(input io.Reader) (*Header, io.Reader, error) { } if bytes.HasPrefix(line, footerPrefix) { + if r != nil { + return nil, nil, errorf("malformed body line %q: reached footer without previous stanza being closed\nNote: this might be a file encrypted with an old beta version of rage. Use rage to decrypt it.", line) + } prefix, args := splitArgs(line) if prefix != string(footerPrefix) || len(args) != 1 { return nil, nil, errorf("malformed closing line: %q", line) @@ -180,6 +180,9 @@ func Parse(input io.Reader) (*Header, io.Reader, error) { break } else if bytes.HasPrefix(line, recipientPrefix) { + if r != nil { + return nil, nil, errorf("malformed body line %q: new stanza started without previous stanza being closed\nNote: this might be a file encrypted with an old beta version of rage. Use rage to decrypt it.", line) + } r = &Stanza{} prefix, args := splitArgs(line) if prefix != string(recipientPrefix) || len(args) < 1 { @@ -202,9 +205,6 @@ func Parse(input io.Reader) (*Header, io.Reader, error) { if len(b) > BytesPerLine { return nil, nil, errorf("malformed body line %q: too long", line) } - if len(b) == 0 { - return nil, nil, errorf("malformed body line %q: line is empty", line) - } r.Body = append(r.Body, b...) if len(b) < BytesPerLine { // Only the last line of a body can be short. diff --git a/internal/format/format_test.go b/internal/format/format_test.go new file mode 100644 index 0000000..6a612ce --- /dev/null +++ b/internal/format/format_test.go @@ -0,0 +1,41 @@ +// Copyright 2021 Google LLC +// +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file or at +// https://developers.google.com/open-source/licenses/bsd + +package format_test + +import ( + "bytes" + "testing" + + "filippo.io/age/internal/format" +) + +func TestStanzaMarshal(t *testing.T) { + s := &format.Stanza{ + Type: "test", + Args: []string{"1", "2", "3"}, + Body: nil, // empty + } + buf := &bytes.Buffer{} + s.Marshal(buf) + if exp := "-> test 1 2 3\n\n"; buf.String() != exp { + t.Errorf("wrong empty stanza encoding: expected %q, got %q", exp, buf.String()) + } + + buf.Reset() + s.Body = []byte("AAA") + s.Marshal(buf) + if exp := "-> test 1 2 3\nQUFB\n"; buf.String() != exp { + t.Errorf("wrong normal stanza encoding: expected %q, got %q", exp, buf.String()) + } + + buf.Reset() + s.Body = bytes.Repeat([]byte("A"), format.BytesPerLine) + s.Marshal(buf) + if exp := "-> test 1 2 3\nQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFB\n\n"; buf.String() != exp { + t.Errorf("wrong 64 columns stanza encoding: expected %q, got %q", exp, buf.String()) + } +}