From 9d4b2ae7ac0e2913839a545e16504e1ba0391124 Mon Sep 17 00:00:00 2001 From: Filippo Valsorda Date: Sun, 6 Jun 2021 13:42:16 +0200 Subject: [PATCH] age: move the scrypt lone recipient check out of Decrypt The important one is the decryption side one, because when a user types a password they expect it to both decrypt and authenticate the file. Moved that one out of Decrypt and into ScryptIdentity, now that Identities get all the stanzas. special_cases-- This also opens the door to other Identity implementations that do allow multiple scrypt recipients, if someone really wants that. The CLI will never allow it, but an explicit choice by an API consumer feels like something we shouldn't interfere with. Moreover, this also allows alternative Identity implementations that use different recipient types to replicate the behavior if they have the same authentication semantics. The encryption side one is only a courtesy, to stop API users from making files that won't decrypt. Unfortunately, that one needs to stay as a special case in Encrypt, as the Recipient can't see around itself. However, changed it to a type assertion, so custom recipients can generate multiple scrypt recipient stanzas, if they really want. --- README.md | 2 -- age.go | 21 +++++++------- cmd/age/age_test.go | 28 ++++++++++++------- cmd/age/encrypted_keys.go | 5 ++++ cmd/age/testdata/fail_bad_hmac.age | 5 ++++ .../{ed25519.age => good_ed25519.age} | 0 .../{ed25519_key.txt => good_ed25519_key.txt} | 0 ...9_key.txt.pub => good_ed25519_key.txt.pub} | 0 ...body.age => good_empty_recipient_body.age} | 0 cmd/age/testdata/{rsa.age => good_rsa.age} | 0 .../{rsa_key.txt => good_rsa_key.txt} | 0 .../{rsa_key.txt.pub => good_rsa_key.txt.pub} | 0 ..._10.age => good_scrypt_work_factor_10.age} | 0 cmd/age/testdata/good_simple.age | 6 ++++ scrypt.go | 5 ++++ 15 files changed, 49 insertions(+), 23 deletions(-) create mode 100644 cmd/age/testdata/fail_bad_hmac.age rename cmd/age/testdata/{ed25519.age => good_ed25519.age} (100%) rename cmd/age/testdata/{ed25519_key.txt => good_ed25519_key.txt} (100%) rename cmd/age/testdata/{ed25519_key.txt.pub => good_ed25519_key.txt.pub} (100%) rename cmd/age/testdata/{empty_recipient_body.age => good_empty_recipient_body.age} (100%) rename cmd/age/testdata/{rsa.age => good_rsa.age} (100%) rename cmd/age/testdata/{rsa_key.txt => good_rsa_key.txt} (100%) rename cmd/age/testdata/{rsa_key.txt.pub => good_rsa_key.txt.pub} (100%) rename cmd/age/testdata/{scrypt_work_factor_10.age => good_scrypt_work_factor_10.age} (100%) create mode 100644 cmd/age/testdata/good_simple.age diff --git a/README.md b/README.md index 0d47eb9..b2152e1 100644 --- a/README.md +++ b/README.md @@ -105,9 +105,7 @@ $ age-keygen | age -p > key.age Public key: age1yhm4gctwfmrpz87tdslm550wrx6m79y9f2hdzt0lndjnehwj0ukqrjpyx5 Enter passphrase (leave empty to autogenerate a secure one): Using the autogenerated passphrase "hip-roast-boring-snake-mention-east-wasp-honey-input-actress". - $ age -r age1yhm4gctwfmrpz87tdslm550wrx6m79y9f2hdzt0lndjnehwj0ukqrjpyx5 secrets.txt > secrets.txt.age - $ age -d -i key.age secrets.txt.age > secrets.txt Enter passphrase for identity file "key.age": ``` diff --git a/age.go b/age.go index 0d32d2a..bd2055b 100644 --- a/age.go +++ b/age.go @@ -103,6 +103,16 @@ func Encrypt(dst io.Writer, recipients ...Recipient) (io.WriteCloser, error) { return nil, errors.New("no recipients specified") } + // As a best effort, prevent an API user from generating a file that the + // ScryptIdentity will refuse to decrypt. This check can't unfortunately be + // implemented as part of the Recipient interface, so it lives as a special + // case in Encrypt. + for _, r := range recipients { + if _, ok := r.(*ScryptRecipient); ok && len(recipients) != 1 { + return nil, errors.New("an ScryptRecipient must be the only one for the file") + } + } + fileKey := make([]byte, fileKeySize) if _, err := rand.Read(fileKey); err != nil { return nil, err @@ -118,11 +128,6 @@ func Encrypt(dst io.Writer, recipients ...Recipient) (io.WriteCloser, error) { hdr.Recipients = append(hdr.Recipients, (*format.Stanza)(s)) } } - for _, s := range hdr.Recipients { - if s.Type == "scrypt" && len(hdr.Recipients) != 1 { - return nil, errors.New("an scrypt recipient must be the only one") - } - } if mac, err := headerMAC(fileKey, hdr); err != nil { return nil, fmt.Errorf("failed to compute header MAC: %v", err) } else { @@ -169,12 +174,6 @@ func Decrypt(src io.Reader, identities ...Identity) (io.Reader, error) { return nil, fmt.Errorf("failed to read header: %v", err) } - for _, r := range hdr.Recipients { - if r.Type == "scrypt" && len(hdr.Recipients) != 1 { - return nil, errors.New("an scrypt recipient must be the only one") - } - } - stanzas := make([]*Stanza, 0, len(hdr.Recipients)) for _, s := range hdr.Recipients { stanzas = append(stanzas, (*Stanza)(s)) diff --git a/cmd/age/age_test.go b/cmd/age/age_test.go index 7e07464..296055f 100644 --- a/cmd/age/age_test.go +++ b/cmd/age/age_test.go @@ -18,24 +18,30 @@ import ( ) func TestVectors(t *testing.T) { - defaultIDs, err := parseIdentitiesFile("testdata/default_key.txt") + var defaultIDs []age.Identity + + password, err := ioutil.ReadFile("testdata/default_password.txt") if err != nil { t.Fatal(err) } - password, err := ioutil.ReadFile("testdata/default_password.txt") - if err == nil { - p := strings.TrimSpace(string(password)) - i, err := age.NewScryptIdentity(p) - if err != nil { - t.Fatal(err) - } - defaultIDs = append(defaultIDs, i) + p := strings.TrimSpace(string(password)) + i, err := age.NewScryptIdentity(p) + if err != nil { + t.Fatal(err) } + defaultIDs = append(defaultIDs, i) + + ids, err := parseIdentitiesFile("testdata/default_key.txt") + if err != nil { + t.Fatal(err) + } + defaultIDs = append(defaultIDs, ids...) files, _ := filepath.Glob("testdata/*.age") for _, f := range files { _, name := filepath.Split(f) name = strings.TrimSuffix(name, ".age") + expectPass := strings.HasPrefix(name, "good_") expectFailure := strings.HasPrefix(name, "fail_") expectNoMatch := strings.HasPrefix(name, "nomatch_") t.Run(name, func(t *testing.T) { @@ -73,7 +79,7 @@ func TestVectors(t *testing.T) { if e := new(age.NoIdentityMatchError); !errors.As(err, &e) { t.Errorf("expected ErrIncorrectIdentity, got %v", err) } - } else { + } else if expectPass { if err != nil { t.Fatal(err) } @@ -82,6 +88,8 @@ func TestVectors(t *testing.T) { t.Fatal(err) } t.Logf("%s", out) + } else { + t.Fatal("invalid test vector") } }) } diff --git a/cmd/age/encrypted_keys.go b/cmd/age/encrypted_keys.go index 8e95df9..b3a2e51 100644 --- a/cmd/age/encrypted_keys.go +++ b/cmd/age/encrypted_keys.go @@ -24,6 +24,11 @@ type LazyScryptIdentity struct { var _ age.Identity = &LazyScryptIdentity{} func (i *LazyScryptIdentity) Unwrap(stanzas []*age.Stanza) (fileKey []byte, err error) { + for _, s := range stanzas { + if s.Type == "scrypt" && len(stanzas) != 1 { + return nil, errors.New("an scrypt recipient must be the only one") + } + } if len(stanzas) != 1 || stanzas[0].Type != "scrypt" { return nil, age.ErrIncorrectIdentity } diff --git a/cmd/age/testdata/fail_bad_hmac.age b/cmd/age/testdata/fail_bad_hmac.age new file mode 100644 index 0000000..b524055 --- /dev/null +++ b/cmd/age/testdata/fail_bad_hmac.age @@ -0,0 +1,5 @@ +age-encryption.org/v1 +-> X25519 i6JOY3uvMdBuEybYbTp3ECFsOPEY/A3lJY1l0Qv2NC4 +cD7VpfIOchU6ZjAccEjlPCNSOdJvVkxZPSf+7XS1YhY +--- 1111111111111111111111111111111111111111111 +�-\�P9��0�hń��Tt�|:٘�#&R�r� �� diff --git a/cmd/age/testdata/ed25519.age b/cmd/age/testdata/good_ed25519.age similarity index 100% rename from cmd/age/testdata/ed25519.age rename to cmd/age/testdata/good_ed25519.age diff --git a/cmd/age/testdata/ed25519_key.txt b/cmd/age/testdata/good_ed25519_key.txt similarity index 100% rename from cmd/age/testdata/ed25519_key.txt rename to cmd/age/testdata/good_ed25519_key.txt diff --git a/cmd/age/testdata/ed25519_key.txt.pub b/cmd/age/testdata/good_ed25519_key.txt.pub similarity index 100% rename from cmd/age/testdata/ed25519_key.txt.pub rename to cmd/age/testdata/good_ed25519_key.txt.pub diff --git a/cmd/age/testdata/empty_recipient_body.age b/cmd/age/testdata/good_empty_recipient_body.age similarity index 100% rename from cmd/age/testdata/empty_recipient_body.age rename to cmd/age/testdata/good_empty_recipient_body.age diff --git a/cmd/age/testdata/rsa.age b/cmd/age/testdata/good_rsa.age similarity index 100% rename from cmd/age/testdata/rsa.age rename to cmd/age/testdata/good_rsa.age diff --git a/cmd/age/testdata/rsa_key.txt b/cmd/age/testdata/good_rsa_key.txt similarity index 100% rename from cmd/age/testdata/rsa_key.txt rename to cmd/age/testdata/good_rsa_key.txt diff --git a/cmd/age/testdata/rsa_key.txt.pub b/cmd/age/testdata/good_rsa_key.txt.pub similarity index 100% rename from cmd/age/testdata/rsa_key.txt.pub rename to cmd/age/testdata/good_rsa_key.txt.pub diff --git a/cmd/age/testdata/scrypt_work_factor_10.age b/cmd/age/testdata/good_scrypt_work_factor_10.age similarity index 100% rename from cmd/age/testdata/scrypt_work_factor_10.age rename to cmd/age/testdata/good_scrypt_work_factor_10.age diff --git a/cmd/age/testdata/good_simple.age b/cmd/age/testdata/good_simple.age new file mode 100644 index 0000000..5f7ab75 --- /dev/null +++ b/cmd/age/testdata/good_simple.age @@ -0,0 +1,6 @@ +age-encryption.org/v1 +-> X25519 kx2RzHNfNuts0I131KwMCyYclZzKCGMzPUaMkH9J4z4 +9qEzjtIF4NsLFnxv8EEtCwOQiXj5WHl+HWaDKNeAk+4 +--- N+7l3M/ofCyzZVlPJ33CTHH8AddF0itK70QV+IIvXXA +] +zAIǏL +H%ѥ \ No newline at end of file diff --git a/scrypt.go b/scrypt.go index 6986c57..183735e 100644 --- a/scrypt.go +++ b/scrypt.go @@ -122,6 +122,11 @@ func (i *ScryptIdentity) SetMaxWorkFactor(logN int) { } func (i *ScryptIdentity) Unwrap(stanzas []*Stanza) ([]byte, error) { + for _, s := range stanzas { + if s.Type == "scrypt" && len(stanzas) != 1 { + return nil, errors.New("an scrypt recipient must be the only one") + } + } return multiUnwrap(i.unwrap, stanzas) }