From 547096db3423a60e5cb78045b99724a7e1ac8c66 Mon Sep 17 00:00:00 2001 From: Aron Wussler Date: Mon, 20 Mar 2023 13:43:19 +0100 Subject: [PATCH] Convert all valid subkeys when issuing a forwarding key --- openpgp/forwarding.go | 168 ++++++++++++++++++++++++-------- openpgp/forwarding_test.go | 69 ++++++++++--- openpgp/packet/encrypted_key.go | 4 +- 3 files changed, 184 insertions(+), 57 deletions(-) diff --git a/openpgp/forwarding.go b/openpgp/forwarding.go index 0e76e56bb..3e447782d 100644 --- a/openpgp/forwarding.go +++ b/openpgp/forwarding.go @@ -11,66 +11,154 @@ import ( "github.com/ProtonMail/go-crypto/openpgp/packet" ) -func (e *Entity) NewForwardingEntity(name, comment, email string, config *packet.Config) (forwardeeKey *Entity, proxyParam []byte, err error) { - encryptionSubKey, ok := e.EncryptionKey(config.Now()) - if !ok { - return nil, nil, errors.InvalidArgumentError("no valid encryption key found") - } - - if encryptionSubKey.PublicKey.Version != 4 { - return nil, nil, errors.InvalidArgumentError("unsupported encryption subkey version") - } +// ForwardingInstance represents a single forwarding instance (mapping IDs to a Proxy Param) +type ForwardingInstance struct { + ForwarderKeyId uint64 + ForwardeeKeyId uint64 + ProxyParameter []byte +} - if encryptionSubKey.PrivateKey.PubKeyAlgo != packet.PubKeyAlgoECDH { - return nil, nil, errors.InvalidArgumentError("encryption subkey is not algorithm 18 (ECDH)") +// NewForwardingEntity generates a new forwardee key and derives the proxy parameters from the entity e. +// If strict, it will return an error if encryption-capable non-revoked subkeys with a wrong algorithm are found, +// instead of ignoring them +func (e *Entity) NewForwardingEntity( + name, comment, email string, config *packet.Config, strict bool, +) ( + forwardeeKey *Entity, instances []ForwardingInstance, err error, +) { + if e.PrimaryKey.Version != 4 { + return nil, nil, errors.InvalidArgumentError("unsupported key version") } - ecdhKey, ok := encryptionSubKey.PrivateKey.PrivateKey.(*ecdh.PrivateKey) - if !ok { - return nil, nil, errors.InvalidArgumentError("encryption subkey is not type ECDH") + now := config.Now() + i := e.PrimaryIdentity() + if e.PrimaryKey.KeyExpired(i.SelfSignature, now) || // primary key has expired + i.SelfSignature == nil || // user ID has no self-signature + i.SelfSignature.SigExpired(now) || // user ID self-signature has expired + e.Revoked(now) || // primary key has been revoked + i.Revoked(now) { // user ID has been revoked + return nil, nil, errors.InvalidArgumentError("primary key is expired") } + // Generate a new Primary key for the forwardee config.Algorithm = packet.PubKeyAlgoEdDSA config.Curve = packet.Curve25519 + keyLifetimeSecs := config.KeyLifetime() - forwardeeKey, err = NewEntity(name, comment, email, config) + forwardeePrimaryPrivRaw, err := newSigner(config) if err != nil { return nil, nil, err } - forwardeeEcdhKey, ok := forwardeeKey.Subkeys[0].PrivateKey.PrivateKey.(*ecdh.PrivateKey) - if !ok { - return nil, nil, goerrors.New("wrong forwarding sub key generation") + primary := packet.NewSignerPrivateKey(now, forwardeePrimaryPrivRaw) + + forwardeeKey = &Entity{ + PrimaryKey: &primary.PublicKey, + PrivateKey: primary, + Identities: make(map[string]*Identity), + Subkeys: []Subkey{}, } - proxyParam, err = ecdh.DeriveProxyParam(ecdhKey, forwardeeEcdhKey) + err = forwardeeKey.addUserId(name, comment, email, config, now, keyLifetimeSecs) if err != nil { return nil, nil, err } - kdf := ecdh.KDF{ - Version: ecdh.KDFVersionForwarding, - Hash: ecdhKey.KDF.Hash, - Cipher: ecdhKey.KDF.Cipher, - ReplacementFingerprint: encryptionSubKey.PublicKey.Fingerprint, + // Init empty instances + instances = []ForwardingInstance{} + + // Handle all forwarder subkeys + for _, forwarderSubKey := range e.Subkeys { + // Filter flags + if !forwarderSubKey.Sig.FlagsValid || forwarderSubKey.Sig.FlagCertify || forwarderSubKey.Sig.FlagSign || + forwarderSubKey.Sig.FlagAuthenticate || forwarderSubKey.Sig.FlagGroupKey { + continue + } + + // Filter expiration & revokal + if forwarderSubKey.PublicKey.KeyExpired(forwarderSubKey.Sig, now) || + forwarderSubKey.Sig.SigExpired(now) || + forwarderSubKey.Revoked(now) { + continue + } + + if forwarderSubKey.PublicKey.PubKeyAlgo != packet.PubKeyAlgoECDH { + if strict { + return nil, nil, errors.InvalidArgumentError("encryption subkey is not algorithm 18 (ECDH)") + } else { + continue + } + } + + forwarderEcdhKey, ok := forwarderSubKey.PrivateKey.PrivateKey.(*ecdh.PrivateKey) + if !ok { + return nil, nil, errors.InvalidArgumentError("malformed key") + } + + err = forwardeeKey.addEncryptionSubkey(config, now, 0) + if err != nil { + return nil, nil, err + } + + forwardeeSubKey := forwardeeKey.Subkeys[len(forwardeeKey.Subkeys) - 1] + + forwardeeEcdhKey, ok := forwardeeSubKey.PrivateKey.PrivateKey.(*ecdh.PrivateKey) + if !ok { + return nil, nil, goerrors.New("wrong forwarding sub key generation") + } + + instance := ForwardingInstance{ + ForwarderKeyId: forwarderSubKey.PublicKey.KeyId, + } + + instance.ProxyParameter, err = ecdh.DeriveProxyParam(forwarderEcdhKey, forwardeeEcdhKey) + if err != nil { + return nil, nil, err + } + + kdf := ecdh.KDF{ + Version: ecdh.KDFVersionForwarding, + Hash: forwarderEcdhKey.KDF.Hash, + Cipher: forwarderEcdhKey.KDF.Cipher, + } + + // If deriving a forwarding key from a forwarding key + if forwarderSubKey.Sig.FlagForward { + if forwarderEcdhKey.KDF.Version != ecdh.KDFVersionForwarding { + return nil, nil, goerrors.New("malformed forwarder key") + } + kdf.ReplacementFingerprint = forwarderEcdhKey.KDF.ReplacementFingerprint + } else { + kdf.ReplacementFingerprint = forwarderSubKey.PublicKey.Fingerprint + } + + err = forwardeeSubKey.PublicKey.ReplaceKDF(kdf) + if err != nil { + return nil, nil, err + } + + // Set ID after changing the KDF + instance.ForwardeeKeyId = forwardeeSubKey.PublicKey.KeyId + + // 0x04 - This key may be used to encrypt communications. + forwardeeSubKey.Sig.FlagEncryptCommunications = false + + // 0x08 - This key may be used to encrypt storage. + forwardeeSubKey.Sig.FlagEncryptStorage = false + + // 0x10 - The private component of this key may have been split by a secret-sharing mechanism. + forwardeeSubKey.Sig.FlagSplitKey = true + + // 0x40 - This key may be used for forwarded communications. + forwardeeSubKey.Sig.FlagForward = true + + // Append each valid instance to the list + instances = append(instances, instance) } - err = forwardeeKey.Subkeys[0].PublicKey.ReplaceKDF(kdf) - if err != nil { - return nil, nil, err + if len(instances) == 0 { + return nil, nil, errors.InvalidArgumentError("no valid subkey found") } - // 0x04 - This key may be used to encrypt communications. - forwardeeKey.Subkeys[0].Sig.FlagEncryptCommunications = false - - // 0x08 - This key may be used to encrypt storage. - forwardeeKey.Subkeys[0].Sig.FlagEncryptStorage = false - - // 0x10 - The private component of this key may have been split by a secret-sharing mechanism. - forwardeeKey.Subkeys[0].Sig.FlagSplitKey = true - - // 0x40 - This key may be used for forwarded communications. - forwardeeKey.Subkeys[0].Sig.FlagForward = true - - return forwardeeKey, proxyParam, nil + return forwardeeKey, instances, nil } diff --git a/openpgp/forwarding_test.go b/openpgp/forwarding_test.go index 1307b1c7f..e32e9d517 100644 --- a/openpgp/forwarding_test.go +++ b/openpgp/forwarding_test.go @@ -78,11 +78,23 @@ func TestForwardingFull(t *testing.T) { t.Fatal(err) } - charlesEntity, proxyParam, err := bobEntity.NewForwardingEntity("charles", "", "charles@proton.me", keyConfig) + charlesEntity, instances, err := bobEntity.NewForwardingEntity("charles", "", "charles@proton.me", keyConfig, true) if err != nil { t.Fatal(err) } + if len(instances) != 1 { + t.Fatalf("invalid number of instances, expected 1 got %d", len(instances)) + } + + if instances[0].ForwarderKeyId != bobEntity.Subkeys[0].PublicKey.KeyId { + t.Fatalf("invalid forwarder key ID, expected: %x, got: %x", bobEntity.Subkeys[0].PublicKey.KeyId, instances[0].ForwarderKeyId) + } + + if instances[0].ForwardeeKeyId != charlesEntity.Subkeys[0].PublicKey.KeyId { + t.Fatalf("invalid forwardee key ID, expected: %x, got: %x", charlesEntity.Subkeys[0].PublicKey.KeyId, instances[0].ForwardeeKeyId) + } + // Encrypt message buf := bytes.NewBuffer(nil) w, err := Encrypt(buf, []*Entity{bobEntity}, nil, nil, nil) @@ -114,6 +126,43 @@ func TestForwardingFull(t *testing.T) { } // Forward message + + transformed := transformTestMessage(t, encrypted, instances[0]) + + // Decrypt forwarded message for Charles + m, err = ReadMessage(bytes.NewBuffer(transformed), EntityList([]*Entity{charlesEntity}), nil /* no prompt */, nil) + if err != nil { + t.Fatal(err) + } + + dec, err = ioutil.ReadAll(m.decrypted) + + if bytes.Compare(dec, plaintext) != 0 { + t.Fatal("forwarded decrypted does not match original") + } + + // Setup further forwarding + danielEntity, secondForwardInstances, err := charlesEntity.NewForwardingEntity("Daniel", "", "daniel@proton.me", keyConfig, true) + if err != nil { + t.Fatal(err) + } + + secondTransformed := transformTestMessage(t, transformed, secondForwardInstances[0]) + + // Decrypt forwarded message for Charles + m, err = ReadMessage(bytes.NewBuffer(secondTransformed), EntityList([]*Entity{danielEntity}), nil /* no prompt */, nil) + if err != nil { + t.Fatal(err) + } + + dec, err = ioutil.ReadAll(m.decrypted) + + if bytes.Compare(dec, plaintext) != 0 { + t.Fatal("forwarded decrypted does not match original") + } +} + +func transformTestMessage(t *testing.T, encrypted []byte, instance ForwardingInstance) []byte { bytesReader := bytes.NewReader(encrypted) packets := packet.NewReader(bytesReader) splitPoint := int64(0) @@ -131,9 +180,9 @@ Loop: switch p := p.(type) { case *packet.EncryptedKey: err = p.ProxyTransform( - proxyParam, - charlesEntity.Subkeys[0].PublicKey.KeyId, - bobEntity.Subkeys[0].PublicKey.KeyId, + instance.ProxyParameter, + instance.ForwarderKeyId, + instance.ForwardeeKeyId, ) if err != nil { t.Fatalf("error transforming PKESK: %s", err) @@ -152,15 +201,5 @@ Loop: transformed := transformedEncryptedKey.Bytes() transformed = append(transformed, encrypted[splitPoint:]...) - // Decrypt forwarded message for Charles - m, err = ReadMessage(bytes.NewBuffer(transformed), EntityList([]*Entity{charlesEntity}), nil /* no prompt */, nil) - if err != nil { - t.Fatal(err) - } - - dec, err = ioutil.ReadAll(m.decrypted) - - if bytes.Compare(dec, plaintext) != 0 { - t.Fatal("forwarded decrypted does not match original") - } + return transformed } diff --git a/openpgp/packet/encrypted_key.go b/openpgp/packet/encrypted_key.go index 7d99f59f6..ad2cd748b 100644 --- a/openpgp/packet/encrypted_key.go +++ b/openpgp/packet/encrypted_key.go @@ -338,12 +338,12 @@ func serializeEncryptedKeyAEAD(w io.Writer, rand io.Reader, header [10]byte, pub return err } -func (e *EncryptedKey) ProxyTransform(proxyParam []byte, forwardeeKeyId, forwardingKeyId uint64) error { +func (e *EncryptedKey) ProxyTransform(proxyParam []byte, forwarderKeyId, forwardeeKeyId uint64) error { if e.Algo != PubKeyAlgoECDH { return errors.InvalidArgumentError("invalid PKESK") } - if e.KeyId != 0 && e.KeyId != forwardingKeyId { + if e.KeyId != 0 && e.KeyId != forwarderKeyId { return errors.InvalidArgumentError("invalid key id in PKESK") }