Merge pull request #271 from ProtonMail/feat/improve-errors-key-selection

v2 API: Improve error messages for encryption key selection

This PR enhances error messages by providing detailed explanations 
when key selection for encryption fails in the v2 API.

Author: lubux

openpgp/errors/errors.go

@@ -6,6 +6,7 @@
 package errors // import "github.com/ProtonMail/go-crypto/openpgp/errors"
 
 import (
+	"fmt"
 	"strconv"
 )
 
@@ -178,3 +179,22 @@ type ErrMalformedMessage string
 func (dke ErrMalformedMessage) Error() string {
 	return "openpgp: malformed message " + string(dke)
 }
+
+// ErrEncryptionKeySelection is returned if encryption key selection fails (v2 API).
+type ErrEncryptionKeySelection struct {
+	PrimaryKeyId      string
+	PrimaryKeyErr     error
+	EncSelectionKeyId *string
+	EncSelectionErr   error
+}
+
+func (eks ErrEncryptionKeySelection) Error() string {
+	prefix := fmt.Sprintf("openpgp: key selection for primary key %s:", eks.PrimaryKeyId)
+	if eks.PrimaryKeyErr != nil {
+		return fmt.Sprintf("%s invalid primary key: %s", prefix, eks.PrimaryKeyErr)
+	}
+	if eks.EncSelectionKeyId != nil {
+		return fmt.Sprintf("%s invalid encryption key %s: %s", prefix, *eks.EncSelectionKeyId, eks.EncSelectionErr)
+	}
+	return fmt.Sprintf("%s no encryption key: %s", prefix, eks.EncSelectionErr)
+}

openpgp/v2/keys.go

@@ -98,27 +98,62 @@ func shouldPreferIdentity(existingId, potentialNewId *packet.Signature) bool {
 // EncryptionKey returns the best candidate Key for encrypting a message to the
 // given Entity.
 func (e *Entity) EncryptionKey(now time.Time, config *packet.Config) (Key, bool) {
+	encryptionKey, err := e.EncryptionKeyWithError(now, config)
+	return encryptionKey, err == nil
+}
+
+// EncryptionKeyWithError returns the best candidate Key for encrypting a message to the
+// given Entity.
+// Provides an error if the function fails to find an encryption key.
+func (e *Entity) EncryptionKeyWithError(now time.Time, config *packet.Config) (Key, error) {
 	// The primary key has to be valid at time now
 	primarySelfSignature, err := e.VerifyPrimaryKey(now, config)
 	if err != nil { // primary key is not valid
-		return Key{}, false
+		return Key{}, errors.ErrEncryptionKeySelection{
+			PrimaryKeyId:  e.PrimaryKey.KeyIdString(),
+			PrimaryKeyErr: err,
+		}
 	}
 
-	if checkKeyRequirements(e.PrimaryKey, config) != nil {
+	if err := checkKeyRequirements(e.PrimaryKey, config); err != nil {
 		// The primary key produces weak signatures
-		return Key{}, false
+		return Key{}, errors.ErrEncryptionKeySelection{
+			PrimaryKeyId:  e.PrimaryKey.KeyIdString(),
+			PrimaryKeyErr: err,
+		}
 	}
 
 	// Iterate the keys to find the newest, unexpired one
+	var latestSelectionError *errors.ErrEncryptionKeySelection
 	candidateSubkey := -1
 	var maxTime time.Time
 	var selectedSubkeySelfSig *packet.Signature
 	for i, subkey := range e.Subkeys {
+		subkeyErr := func(encSelectionErr error) *errors.ErrEncryptionKeySelection {
+			subkeyKeyId := subkey.PublicKey.KeyIdString()
+			return &errors.ErrEncryptionKeySelection{
+				PrimaryKeyId:      e.PrimaryKey.KeyIdString(),
+				EncSelectionKeyId: &subkeyKeyId,
+				EncSelectionErr:   encSelectionErr,
+			}
+
+		}
+		// Verify the subkey signature.
 		subkeySelfSig, err := subkey.Verify(now, config) // subkey has to be valid at time now
-		if err == nil &&
-			isValidEncryptionKey(subkeySelfSig, subkey.PublicKey.PubKeyAlgo, config) &&
-			checkKeyRequirements(subkey.PublicKey, config) == nil &&
-			(maxTime.IsZero() || subkeySelfSig.CreationTime.Unix() >= maxTime.Unix()) {
+		if err != nil {
+			latestSelectionError = subkeyErr(err)
+			continue
+		}
+		// Check the algorithm and key flags.
+		if !isValidEncryptionKey(subkeySelfSig, subkey.PublicKey.PubKeyAlgo, config) {
+			continue
+		}
+		// Check if the key fulfils the requirements
+		if err := checkKeyRequirements(subkey.PublicKey, config); err != nil {
+			latestSelectionError = subkeyErr(err)
+			continue
+		}
+		if maxTime.IsZero() || subkeySelfSig.CreationTime.Unix() >= maxTime.Unix() {
 			candidateSubkey = i
 			selectedSubkeySelfSig = subkeySelfSig
 			maxTime = subkeySelfSig.CreationTime
@@ -133,7 +168,7 @@ func (e *Entity) EncryptionKey(now time.Time, config *packet.Config) (Key, bool)
 			PublicKey:            subkey.PublicKey,
 			PrivateKey:           subkey.PrivateKey,
 			SelfSignature:        selectedSubkeySelfSig,
-		}, true
+		}, nil
 	}
 
 	// If we don't have any subkeys for encryption and the primary key
@@ -145,10 +180,17 @@ func (e *Entity) EncryptionKey(now time.Time, config *packet.Config) (Key, bool)
 			PublicKey:            e.PrimaryKey,
 			PrivateKey:           e.PrivateKey,
 			SelfSignature:        primarySelfSignature,
-		}, true
+		}, nil
+	}
+
+	if latestSelectionError == nil {
+		latestSelectionError = &errors.ErrEncryptionKeySelection{
+			PrimaryKeyId:    e.PrimaryKey.KeyIdString(),
+			EncSelectionErr: goerrors.New("no encryption-capable key found (no key flags or invalid algorithm)"),
+		}
 	}
 
-	return Key{}, false
+	return Key{}, latestSelectionError
 }
 
 // DecryptionKeys returns all keys that are available for decryption, matching the keyID when given

openpgp/v2/keys_test.go

@@ -2075,3 +2075,37 @@ func TestAllowAllKeyFlagsWhenMissing(t *testing.T) {
 		t.Error("isValidCertificationKey must be true when InsecureAllowAllKeyFlagsWhenMissing is true")
 	}
 }
+
+func TestEncryptionKeyError(t *testing.T) {
+	// Make a master key.
+	entity, err := NewEntity("Golang Gopher", "Test Key", "no-reply@golang.com", nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	_, err = entity.EncryptionKeyWithError(time.Unix(1405544146, 0), nil)
+	if err == nil {
+		t.Fatal("should fail")
+	}
+	if !strings.Contains(err.Error(), "no valid self signature found") {
+		t.Fatal("wrong error")
+	}
+
+	entity.Subkeys[0].PublicKey.Version = 20
+	_, err = entity.EncryptionKeyWithError(time.Now(), nil)
+	if err == nil {
+		t.Fatal("should fail")
+	}
+	if !strings.Contains(err.Error(), "no valid binding signature found for subkey") {
+		t.Fatal("wrong error")
+	}
+
+	entity.Subkeys = nil
+	_, err = entity.EncryptionKeyWithError(time.Now(), nil)
+	if err == nil {
+		t.Fatal("should fail")
+	}
+	if !strings.Contains(err.Error(), "no encryption-capable key found (no key flags or invalid algorithm)") {
+		t.Fatal("wrong error")
+	}
+}

openpgp/v2/write.go

@@ -611,10 +611,8 @@ func encrypt(
 		timeForEncryptionKey = *params.EncryptionTime
 	}
 	for i, recipient := range append(to, toHidden...) {
-		var ok bool
-		encryptKeys[i], ok = recipient.EncryptionKey(timeForEncryptionKey, config)
-		if !ok {
-			return nil, errors.InvalidArgumentError("cannot encrypt a message to key id " + strconv.FormatUint(to[i].PrimaryKey.KeyId, 16) + " because it has no valid encryption keys")
+		if encryptKeys[i], err = recipient.EncryptionKeyWithError(timeForEncryptionKey, config); err != nil {
+			return nil, err
 		}
 
 		primarySelfSignature, _ := recipient.PrimarySelfSignature(timeForEncryptionKey, config)