diff --git a/CHANGELOG.md b/CHANGELOG.md index 9c7800a..a0a79ed 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,14 +5,20 @@ 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`. This change impacts : - `func (sk *SessionKey) DecryptAndVerify(...)` - `func (msg *PlainMessageReader) VerifySignature(...)` - `func (keyRing *KeyRing) Decrypt(...)` +- Improved error messages for failures in password protected message decryption ### Added - Helper to access the SignatureVerificationError explicitly when decrypting streams in mobile apps: diff --git a/README.md b/README.md index 54f5e86..04b71b1 100644 --- a/README.md +++ b/README.md @@ -373,23 +373,4 @@ newPGPSplitMessage, err := pgpMessage.SeparateKeyAndData() ``` ### 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. +Keys are now checked on import and the explicit check via `Key#Check()` is deprecated and no longer necessary. \ No newline at end of file diff --git a/constants/armor.go b/constants/armor.go index 1f33704..7196b46 100644 --- a/constants/armor.go +++ b/constants/armor.go @@ -3,7 +3,7 @@ package constants // Constants for armored data. const ( - ArmorHeaderVersion = "GopenPGP 2.2.0" + ArmorHeaderVersion = "GopenPGP 2.2.1" ArmorHeaderComment = "https://gopenpgp.org" PGPMessageHeader = "PGP MESSAGE" PGPSignatureHeader = "PGP SIGNATURE" diff --git a/constants/version.go b/constants/version.go index 0df2485..26c0693 100644 --- a/constants/version.go +++ b/constants/version.go @@ -1,3 +1,3 @@ package constants -const Version = "2.2.0" +const Version = "2.2.1" diff --git a/crypto/key.go b/crypto/key.go index 38a9e8c..ced0fc3 100644 --- a/crypto/key.go +++ b/crypto/key.go @@ -318,37 +318,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..1130e7f 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) @@ -425,3 +415,14 @@ func TestKeyCapabilities(t *testing.T) { assert.True(t, publicKey.CanVerify()) assert.True(t, publicKey.CanEncrypt()) } + +func TestUnlockMismatchingKey(t *testing.T) { + privateKey, err := NewKeyFromArmored(readTestFile("key_mismatching_eddsa_key", false)) + if err != nil { + t.Fatal("Expected no error while unarmoring private key, got:", err) + } + + if _, err = privateKey.Unlock([]byte("123")); err == nil { + t.Fatalf("Mismatching private key was not detected") + } +} diff --git a/crypto/testdata/key_mismatching_eddsa_key b/crypto/testdata/key_mismatching_eddsa_key new file mode 100644 index 0000000..e931d2a --- /dev/null +++ b/crypto/testdata/key_mismatching_eddsa_key @@ -0,0 +1,18 @@ +-----BEGIN PGP PRIVATE KEY BLOCK----- +Version: OpenPGP.js v4.10.4 +Comment: https://openpgpjs.org + +xYYEX6FzhhYJKwYBBAHaRw8BAQdA0Yeb41B5BRI25Dq0z7oRCcImRQfT+WcJ +HLaHpd6baGT+CQMI/6uWP8YxbIIAJDKhBYD+D8yTJW6JBtnOISKzPSbTNxOP +aQEQz1YPMhZhTBIWZFNXo614n3C7Ak1Q73bv8zsJq0EmRJPNkFXmY1S/B0Lm +GM0SQm9iIDxpbmZvQGJvYi5jb20+wngEEBYKACAFAl+hc4YGCwkHCAMCBBUI +CgIEFgIBAAIZAQIbAwIeAQAKCRBeNKXxFWm5mDjwAQC6vYacL9/oBZ3Ev3DR +l9a//92L8hYrx3Le3pRZhJmDqAEAoz5fPGxQEXdgzI14i7ZdNsRRzyGQh3nL +jnGeUJx7aQnHiwRfoXOGEgorBgEEAZdVAQUBAQdA2yQoAO2pqLJe48Wazz6+ +PSxyh5EXGQZ9yy/ZO2y8Rl8DAQgH/gkDCJE4VlUHrTStAIYCzED5f7BXphR0 +jqbMnwTxXxFsI7H9kUykmOPjzYayTLkp/Pw5OMzxjwVKD6+zO4YKtd9EuhEH +pD32k72LbNPYE/j3p77CYQQYFggACQUCX6FzhgIbDAAKCRBeNKXxFWm5mJa3 +AQDZA/yx5SotHacjZmJir/ly7aPdPUv4krqx86BHbl/2HAD/YSPzjDxBYUDA +kWK2+FnnoMBGL6PIKIHlxONRrDVknwQ= +=IsER +-----END PGP PRIVATE KEY BLOCK----- \ No newline at end of file