From a7c4274d23005d94014835b635b48ba532a3ae6f Mon Sep 17 00:00:00 2001 From: Filippo Valsorda Date: Sun, 17 May 2020 13:47:13 -0400 Subject: [PATCH] internal/age: remove EncryptWithArmor and armor support in Decrypt The caller can take care of the armor. For consistency move the responsibility to close the armor to the caller, and make the stream Writer not propagate Close. This also will also allow us to spin the armor implementation out into its won package that imports format, without getting an import loop from format.Parse magically invoking armor decoding. Less magic in the API, more magic in the CLI. --- cmd/age/age.go | 18 +++++++++++++++++- internal/age/age.go | 13 ------------- internal/format/format.go | 7 ------- internal/stream/stream.go | 18 ++++++++---------- internal/stream/stream_test.go | 19 +------------------ 5 files changed, 26 insertions(+), 49 deletions(-) diff --git a/cmd/age/age.go b/cmd/age/age.go index a604bb6..8159e2e 100644 --- a/cmd/age/age.go +++ b/cmd/age/age.go @@ -7,6 +7,7 @@ package main import ( + "bufio" "bytes" "flag" "fmt" @@ -16,6 +17,7 @@ import ( "strings" "filippo.io/age/internal/age" + "filippo.io/age/internal/format" "golang.org/x/crypto/ssh/terminal" ) @@ -208,7 +210,13 @@ func encryptPass(pass string, in io.Reader, out io.Writer, armor bool) { func encrypt(recipients []age.Recipient, in io.Reader, out io.Writer, armor bool) { ageEncrypt := age.Encrypt if armor { - ageEncrypt = age.EncryptWithArmor + a := format.ArmoredWriter(out) + defer func() { + if err := a.Close(); err != nil { + logFatalf("Error: %v", err) + } + }() + out = a } w, err := ageEncrypt(out, recipients...) if err != nil { @@ -239,6 +247,14 @@ func decrypt(keys []string, in io.Reader, out io.Writer) { identities = append(identities, ids...) } + rr := bufio.NewReader(in) + armorHeader := "-----BEGIN AGE ENCRYPTED FILE-----" + if start, _ := rr.Peek(len(armorHeader)); string(start) == armorHeader { + in = format.ArmoredReader(rr) + } else { + in = rr + } + r, err := age.Decrypt(in, identities...) if err != nil { logFatalf("Error: %v", err) diff --git a/internal/age/age.go b/internal/age/age.go index 11e1167..6ce04ec 100644 --- a/internal/age/age.go +++ b/internal/age/age.go @@ -36,19 +36,6 @@ type Recipient interface { } func Encrypt(dst io.Writer, recipients ...Recipient) (io.WriteCloser, error) { - // stream.Writer takes a WriteCloser, and will propagate Close calls (so - // that the ArmoredWriter will get closed), but we don't want to expose - // that behavior to our caller. - dstCloser := format.NopCloser(dst) - return encrypt(dstCloser, recipients...) -} - -func EncryptWithArmor(dst io.Writer, recipients ...Recipient) (io.WriteCloser, error) { - dstCloser := format.ArmoredWriter(dst) - return encrypt(dstCloser, recipients...) -} - -func encrypt(dst io.WriteCloser, recipients ...Recipient) (io.WriteCloser, error) { if len(recipients) == 0 { return nil, errors.New("no recipients specified") } diff --git a/internal/format/format.go b/internal/format/format.go index d20bfbf..7c717c6 100644 --- a/internal/format/format.go +++ b/internal/format/format.go @@ -112,13 +112,6 @@ func Parse(input io.Reader) (*Header, io.Reader, error) { h := &Header{} rr := bufio.NewReader(input) - // TODO: find a way to communicate to the caller that the file was armored, - // as they might not appreciate the malleability. - if start, _ := rr.Peek(len(armorPreamble)); string(start) == armorPreamble { - input = ArmoredReader(rr) - rr = bufio.NewReader(input) - } - line, err := rr.ReadString('\n') if err != nil { return nil, nil, errorf("failed to read intro: %v", err) diff --git a/internal/stream/stream.go b/internal/stream/stream.go index f3db36c..96a0660 100644 --- a/internal/stream/stream.go +++ b/internal/stream/stream.go @@ -132,14 +132,14 @@ func setLastChunkFlag(nonce *[chacha20poly1305.NonceSize]byte) { type Writer struct { a cipher.AEAD - dst io.WriteCloser + dst io.Writer unwritten []byte // backed by buf buf [encChunkSize]byte nonce [chacha20poly1305.NonceSize]byte err error } -func NewWriter(key []byte, dst io.WriteCloser) (*Writer, error) { +func NewWriter(key []byte, dst io.Writer) (*Writer, error) { aead, err := chacha20poly1305.New(key) if err != nil { return nil, err @@ -178,21 +178,19 @@ func (w *Writer) Write(p []byte) (n int, err error) { return total, nil } -// Close will flush the last chunk and call the underlying -// WriteCloser's Close method. +// Close flushes the last chunk. It does not close the underlying Writer. func (w *Writer) Close() error { if w.err != nil { return w.err } - err := w.flushChunk(lastChunk) - if err != nil { - w.err = err - return err + w.err = w.flushChunk(lastChunk) + if w.err != nil { + return w.err } - w.err = errors.New("stream.Writer is already closed") - return w.dst.Close() + w.err = errors.New("stream.Writer is already closed") + return nil } const ( diff --git a/internal/stream/stream_test.go b/internal/stream/stream_test.go index 9ca2d47..57c99eb 100644 --- a/internal/stream/stream_test.go +++ b/internal/stream/stream_test.go @@ -10,10 +10,8 @@ import ( "bytes" "crypto/rand" "fmt" - "io" "testing" - "filippo.io/age/internal/format" "filippo.io/age/internal/stream" "golang.org/x/crypto/chacha20poly1305" ) @@ -40,19 +38,7 @@ func testRoundTrip(t *testing.T, stepSize, length int) { t.Fatal(err) } - var closed bool - bufCloser := struct { - io.Writer - format.CloserFunc - }{ - Writer: buf, - CloserFunc: func() error { - closed = true - return nil - }, - } - - w, err := stream.NewWriter(key, bufCloser) + w, err := stream.NewWriter(key, buf) if err != nil { t.Fatal(err) } @@ -84,9 +70,6 @@ func testRoundTrip(t *testing.T, stepSize, length int) { if err := w.Close(); err != nil { t.Error("Close returned an error:", err) } - if !closed { - t.Error("(*stream.Writer).Close didn't close the underlying WriteCloser") - } t.Logf("buffer size: %d", buf.Len())