From 0fa220e4d7574d61ad01cf3a8c95b6fa2ca700f5 Mon Sep 17 00:00:00 2001 From: Filippo Valsorda Date: Mon, 4 Jan 2021 16:56:22 +0100 Subject: [PATCH] age: remove IdentityMatcher It was completely useless: the same checks in Match could be implemented in Unwrap, returning an early ErrIncorrectIdentity. Not sure why I added it. It felt clever at the time. --- age.go | 33 +++---------------------- agessh/encrypted_keys.go | 53 +++++++++++++++++----------------------- 2 files changed, 27 insertions(+), 59 deletions(-) diff --git a/age.go b/age.go index 52d4e36..7c90f42 100644 --- a/age.go +++ b/age.go @@ -57,18 +57,6 @@ type Identity interface { Unwrap(block *Stanza) (fileKey []byte, err error) } -// IdentityMatcher can be optionally implemented by an Identity that can -// communicate whether it can decrypt a recipient stanza without decrypting it. -// -// If an Identity implements IdentityMatcher, its Unwrap method will only be -// invoked on blocks for which Match returned nil. Match must return -// ErrIncorrectIdentity for recipient blocks that don't match the identity, any -// other error might be considered fatal. -type IdentityMatcher interface { - Identity - Match(block *Stanza) error -} - var ErrIncorrectIdentity = errors.New("incorrect identity for recipient block") // A Recipient is a public key or other value that can encrypt an opaque file @@ -161,24 +149,11 @@ RecipientsLoop: return nil, errors.New("an scrypt recipient must be the only one") } for _, i := range identities { - if i, ok := i.(IdentityMatcher); ok { - err := i.Match((*Stanza)(r)) - if err != nil { - if err == ErrIncorrectIdentity { - continue - } - return nil, err - } - } - fileKey, err = i.Unwrap((*Stanza)(r)) + if errors.Is(err, ErrIncorrectIdentity) { + continue + } if err != nil { - if err == ErrIncorrectIdentity { - // TODO: we should collect these errors and return them as an - // []error type with an Error method. That will require turning - // ErrIncorrectIdentity into an interface or wrapper error. - continue - } return nil, err } @@ -186,7 +161,7 @@ RecipientsLoop: } } if fileKey == nil { - return nil, errors.New("no identity matched a recipient") + return nil, errors.New("no identity matched any of the recipients") } if mac, err := headerMAC(fileKey, hdr); err != nil { diff --git a/agessh/encrypted_keys.go b/agessh/encrypted_keys.go index 48454a4..78757c4 100644 --- a/agessh/encrypted_keys.go +++ b/agessh/encrypted_keys.go @@ -15,14 +15,13 @@ import ( "golang.org/x/crypto/ssh" ) -// EncryptedSSHIdentity is an age.IdentityMatcher implementation based on a -// passphrase encrypted SSH private key. +// EncryptedSSHIdentity is an age.Identity implementation based on a passphrase +// encrypted SSH private key. // -// It provides public key based matching and deferred decryption so the -// passphrase is only requested if necessary. If the application knows it will -// unconditionally have to decrypt the private key, it would be simpler to use -// ssh.ParseRawPrivateKeyWithPassphrase directly and pass the result to -// NewEd25519Identity or NewRSAIdentity. +// It requests the passphrase only if the public key matches a recipient stanza. +// If the application knows it will always have to decrypt the private key, it +// would be simpler to use ssh.ParseRawPrivateKeyWithPassphrase directly and +// pass the result to NewEd25519Identity or NewRSAIdentity. type EncryptedSSHIdentity struct { pubKey ssh.PublicKey pemBytes []byte @@ -35,7 +34,7 @@ type EncryptedSSHIdentity struct { // // pubKey must be the public key associated with the encrypted private key, and // it must have type "ssh-ed25519" or "ssh-rsa". For OpenSSH encrypted files it -// can be extracted from an ssh.PassphraseMissingError, otherwise in can often +// can be extracted from an ssh.PassphraseMissingError, otherwise it can often // be found in ".pub" files. // // pemBytes must be a valid input to ssh.ParseRawPrivateKeyWithPassphrase. @@ -54,16 +53,26 @@ func NewEncryptedSSHIdentity(pubKey ssh.PublicKey, pemBytes []byte, passphrase f }, nil } -var _ age.IdentityMatcher = &EncryptedSSHIdentity{} +var _ age.Identity = &EncryptedSSHIdentity{} -// Unwrap implements age.Identity. If the private key is still encrypted, it -// will request the passphrase. The decrypted private key will be cached after -// the first successful invocation. +// Unwrap implements age.Identity. If the private key is still encrypted, and +// the block matches the public key, it will request the passphrase. The +// decrypted private key will be cached after the first successful invocation. func (i *EncryptedSSHIdentity) Unwrap(block *age.Stanza) (fileKey []byte, err error) { if i.decrypted != nil { return i.decrypted.Unwrap(block) } + if block.Type != i.pubKey.Type() { + return nil, age.ErrIncorrectIdentity + } + if len(block.Args) < 1 { + return nil, fmt.Errorf("invalid %v recipient block", i.pubKey.Type()) + } + if block.Args[0] != sshFingerprint(i.pubKey) { + return nil, age.ErrIncorrectIdentity + } + passphrase, err := i.passphrase() if err != nil { return nil, fmt.Errorf("failed to obtain passphrase: %v", err) @@ -77,12 +86,12 @@ func (i *EncryptedSSHIdentity) Unwrap(block *age.Stanza) (fileKey []byte, err er case *ed25519.PrivateKey: i.decrypted, err = NewEd25519Identity(*k) if i.pubKey.Type() != ssh.KeyAlgoED25519 { - return nil, fmt.Errorf("mismatched SSH key type: got %q, expected %q", ssh.KeyAlgoED25519, i.pubKey.Type()) + return nil, fmt.Errorf("mismatched private (%s) and public (%s) SSH key types", ssh.KeyAlgoED25519, i.pubKey.Type()) } case *rsa.PrivateKey: i.decrypted, err = NewRSAIdentity(k) if i.pubKey.Type() != ssh.KeyAlgoRSA { - return nil, fmt.Errorf("mismatched SSH key type: got %q, expected %q", ssh.KeyAlgoRSA, i.pubKey.Type()) + return nil, fmt.Errorf("mismatched private (%s) and public (%s) SSH key types", ssh.KeyAlgoRSA, i.pubKey.Type()) } default: return nil, fmt.Errorf("unexpected SSH key type: %T", k) @@ -93,19 +102,3 @@ func (i *EncryptedSSHIdentity) Unwrap(block *age.Stanza) (fileKey []byte, err er return i.decrypted.Unwrap(block) } - -// Match implements age.IdentityMatcher without decrypting the private key, to -// ensure the passphrase is only obtained if necessary. -func (i *EncryptedSSHIdentity) Match(block *age.Stanza) error { - if block.Type != i.pubKey.Type() { - return age.ErrIncorrectIdentity - } - if len(block.Args) < 1 { - return fmt.Errorf("invalid %v recipient block", i.pubKey.Type()) - } - - if block.Args[0] != sshFingerprint(i.pubKey) { - return age.ErrIncorrectIdentity - } - return nil -}