diff --git a/CHANGELOG.md b/CHANGELOG.md index 37ab976..a0a79ed 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,13 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## Unreleased +### Security +- All keys are now checked on parsing from the underlying library + +### Deprecated +- `(key *Key) Check() (bool, error)` is now deprecated, all keys are now checked upon import from x/crypto + ## [2.2.1] 2021-07-27 ### Changed - Changed the returned `SignatureVerificationError.Status` when trying to verify a message with no embedded signature. It used to return `constants.SIGNATURE_NO_VERIFIER` and now returns `constants.SIGNATURE_NOT_SIGNED`. diff --git a/README.md b/README.md index 54f5e86..8238358 100644 --- a/README.md +++ b/README.md @@ -371,25 +371,3 @@ newPGPSplitMessage, err := pgpMessage.SeparateKeyAndData() // Key Packet is in newPGPSplitMessage.GetBinaryKeyPacket() // Data Packet is in newPGPSplitMessage.GetBinaryDataPacket() ``` - -### Checking keys -In order to check that the primary key is valid the `Key#Check` function can be used. -This operation is as of 2.0.0 fairly expensive, as it requires a signature operation. -It will be improved in the future versions, and possibly expanded to the subkeys, that are -for now assumed to be correct thanks to the binding signature. -```go -const privkey = `-----BEGIN PGP PRIVATE KEY BLOCK----- -... ------END PGP PRIVATE KEY BLOCK-----` // Encrypted private key -const passphrase = []byte("LongSecret") // Private key passphrase - -privateKeyObj, err := crypto.NewKeyFromArmored(privkey) -unlockedKeyObj = privateKeyObj.Unlock(passphrase) - -isVerified, _ := unlockedKeyObj.Check(); -if !isVerified { - // Handle broken keys -} -``` -This function runs on unlocked private keys, and it will return an error if called with public keys -or locked keys. diff --git a/crypto/key.go b/crypto/key.go index 49af11e..25af16e 100644 --- a/crypto/key.go +++ b/crypto/key.go @@ -306,37 +306,8 @@ func (key *Key) IsUnlocked() (bool, error) { // Check verifies if the public keys match the private key parameters by // signing and verifying. +// Deprecated: all keys are now checked on parsing. func (key *Key) Check() (bool, error) { - var err error - testSign := bytes.Repeat([]byte{0x01}, 64) - testReader := bytes.NewReader(testSign) - - if !key.IsPrivate() { - return false, errors.New("gopenpgp: can check only private key") - } - - unlocked, err := key.IsUnlocked() - if err != nil { - return false, err - } - - if !unlocked { - return false, errors.New("gopenpgp: key is not unlocked") - } - - var signBuf bytes.Buffer - - if err = openpgp.DetachSign(&signBuf, key.entity, testReader, nil); err != nil { - return false, errors.New("gopenpgp: unable to sign with key") - } - - testReader = bytes.NewReader(testSign) - signer, err := openpgp.CheckDetachedSignature(openpgp.EntityList{key.entity}, testReader, &signBuf, nil) - - if signer == nil || err != nil { - return false, nil - } - return true, nil } diff --git a/crypto/key_test.go b/crypto/key_test.go index 9eb35f5..e2c2808 100644 --- a/crypto/key_test.go +++ b/crypto/key_test.go @@ -241,33 +241,23 @@ func TestGenerateKeyWithPrimes(t *testing.T) { assert.Exactly(t, prime2, pk.Primes[1].Bytes()) } -func TestCheckIntegrity(t *testing.T) { - isVerified, err := keyTestRSA.Check() - if err != nil { - t.Fatal("Expected no error while checking correct passphrase, got:", err) - } - - assert.Exactly(t, true, isVerified) +func TestFailCheckIntegrity25519(t *testing.T) { + failCheckIntegrity(t, "x25519", 0) } -func TestFailCheckIntegrity(t *testing.T) { - // This test is done with ECC because in an RSA key we would need to replace the primes, but maintaining the moduli, - // that is a private struct element. - k1, _ := GenerateKey(keyTestName, keyTestDomain, "x25519", 256) - k2, _ := GenerateKey(keyTestName, keyTestDomain, "x25519", 256) +func TestFailCheckIntegrityRSA(t *testing.T) { + failCheckIntegrity(t, "rsa", 2048) +} + +func failCheckIntegrity(t *testing.T, keyType string, bits int) { + k1, _ := GenerateKey(keyTestName, keyTestDomain, keyType, bits) + k2, _ := GenerateKey(keyTestName, keyTestDomain, keyType, bits) k1.entity.PrivateKey.PrivateKey = k2.entity.PrivateKey.PrivateKey // Swap private keys - isVerified, err := k1.Check() - if err != nil { - t.Fatal("Expected no error while checking key, got:", err) - } - - assert.Exactly(t, false, isVerified) - serialized, err := k1.Serialize() if err != nil { - t.Fatal("Expected no error while serializing keyring kr3, got:", err) + t.Fatal("Expected no error while serializing keyring, got:", err) } _, err = NewKey(serialized)