Merge branch 'master' into master

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: 

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.

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"

constants/version.go

@@ -1,3 +1,3 @@
 package constants
 
-const Version = "2.2.0"
+const Version = "2.2.1"

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
 }
 

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)
+func TestFailCheckIntegrity25519(t *testing.T) {
+	failCheckIntegrity(t, "x25519", 0)
 }
 
-	assert.Exactly(t, true, isVerified)
+func TestFailCheckIntegrityRSA(t *testing.T) {
+	failCheckIntegrity(t, "rsa", 2048)
 }
 
-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 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")
+	}
+}

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-----