Deprecate Key#Check()

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

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.

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
 }
 

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) {
+func TestFailCheckIntegrity25519(t *testing.T) {
-	isVerified, err := keyTestRSA.Check()
+	failCheckIntegrity(t, "x25519", 0)
-	if err != nil {
-		t.Fatal("Expected no error while checking correct passphrase, got:", err)
-	}
-
-	assert.Exactly(t, true, isVerified)
 }
 
-func TestFailCheckIntegrity(t *testing.T) {
+func TestFailCheckIntegrityRSA(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,
+	failCheckIntegrity(t, "rsa", 2048)
-	// 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)