From 25c09e604da1fa8a7fe91752544f8a5dd05958d2 Mon Sep 17 00:00:00 2001 From: Lukas Burkhalter Date: Wed, 20 Nov 2024 11:42:39 +0100 Subject: [PATCH 1/4] feat(v2): Add flag to allow decryption with signing keys --- openpgp/packet/config.go | 12 ++++++++++++ openpgp/v2/keys.go | 4 ++-- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/openpgp/packet/config.go b/openpgp/packet/config.go index fb21e6d1..8bf8e6e5 100644 --- a/openpgp/packet/config.go +++ b/openpgp/packet/config.go @@ -139,6 +139,11 @@ type Config struct { // might be no other way than to tolerate the missing MDC. Setting this flag, allows this // mode of operation. It should be considered a measure of last resort. InsecureAllowUnauthenticatedMessages bool + // InsecureAllowDecryptionWithSigningKeys allows decryption with keys marked as signing keys in the v2 API. + // This setting is potentially insecure, but it is needed as some libraries + // ignored key flags when selecting a key for encryption. + // Not relevant for the v1 API, as all keys were allowed in decryption. + InsecureAllowDecryptionWithSigningKeys bool // KnownNotations is a map of Notation Data names to bools, which controls // the notation names that are allowed to be present in critical Notation Data // signature subpackets. @@ -291,6 +296,13 @@ func (c *Config) AllowUnauthenticatedMessages() bool { return c.InsecureAllowUnauthenticatedMessages } +func (c *Config) AllowDecryptionWithSigningKeys() bool { + if c == nil { + return false + } + return c.InsecureAllowDecryptionWithSigningKeys +} + func (c *Config) KnownNotation(notationName string) bool { if c == nil { return false diff --git a/openpgp/v2/keys.go b/openpgp/v2/keys.go index 9a5226d5..93082dd4 100644 --- a/openpgp/v2/keys.go +++ b/openpgp/v2/keys.go @@ -164,12 +164,12 @@ func (e *Entity) DecryptionKeys(id uint64, date time.Time, config *packet.Config for _, subkey := range e.Subkeys { subkeySelfSig, err := subkey.LatestValidBindingSignature(date, config) if err == nil && - isValidEncryptionKey(subkeySelfSig, subkey.PublicKey.PubKeyAlgo) && + (config.AllowDecryptionWithSigningKeys() || isValidEncryptionKey(subkeySelfSig, subkey.PublicKey.PubKeyAlgo)) && (id == 0 || subkey.PublicKey.KeyId == id) { keys = append(keys, Key{subkey.Primary, primarySelfSignature, subkey.PublicKey, subkey.PrivateKey, subkeySelfSig}) } } - if isValidEncryptionKey(primarySelfSignature, e.PrimaryKey.PubKeyAlgo) { + if config.AllowDecryptionWithSigningKeys() || isValidEncryptionKey(primarySelfSignature, e.PrimaryKey.PubKeyAlgo) { keys = append(keys, Key{e, primarySelfSignature, e.PrimaryKey, e.PrivateKey, primarySelfSignature}) } return From 788cec1bb8453214d4f92db376a563d7f52a26d1 Mon Sep 17 00:00:00 2001 From: Lukas Burkhalter Date: Wed, 20 Nov 2024 14:01:37 +0100 Subject: [PATCH 2/4] test: Add tests for decryption with singing key --- openpgp/v2/read_test.go | 38 +++++++++++++++++++++++ openpgp/v2/read_write_test_data.go | 48 ++++++++++++++++++++++++++++++ 2 files changed, 86 insertions(+) diff --git a/openpgp/v2/read_test.go b/openpgp/v2/read_test.go index 24eec2f4..4de095f0 100644 --- a/openpgp/v2/read_test.go +++ b/openpgp/v2/read_test.go @@ -1023,3 +1023,41 @@ func testMalformedMessage(t *testing.T, keyring EntityList, message string) { return } } + +func TestReadMessageWithSignOnly(t *testing.T) { + config := packet.Config{ + InsecureAllowDecryptionWithSigningKeys: true, + } + key, err := ReadArmoredKeyRing(strings.NewReader(rsaSignOnly)) + if err != nil { + t.Error(err) + return + } + // Success + msgReader, err := armor.Decode(strings.NewReader(armoredMessageRsaSignOnly)) + if err != nil { + t.Error(err) + return + } + md, err := ReadMessage(msgReader.Body, key, nil, &config) + if err != nil { + t.Error(err) + return + } + _, err = io.ReadAll(md.UnverifiedBody) + if err != nil { + t.Error(err) + return + } + + // Fail + msgReader, err = armor.Decode(strings.NewReader(armoredMessageRsaSignOnly)) + if err != nil { + t.Error(err) + return + } + md, err = ReadMessage(msgReader.Body, key, nil, nil) + if err == nil { + t.Fatal("Should not decrypt") + } +} diff --git a/openpgp/v2/read_write_test_data.go b/openpgp/v2/read_write_test_data.go index 2f0efc22..9322b949 100644 --- a/openpgp/v2/read_write_test_data.go +++ b/openpgp/v2/read_write_test_data.go @@ -740,3 +740,51 @@ NVniEke6hM3CNBXYPAMhQBMWhCulcoz+0lxi8L34rMN+Dsbma96psdUrn7uLaB91 xqAY9Bwizt4FWgXuLm1a4+So4V9j1TRCXd12Uc2l2RNmgDE= =miES -----END PGP PRIVATE KEY BLOCK-----` + +const rsaSignOnly = `-----BEGIN PGP PRIVATE KEY BLOCK----- + +xcLYBF9Gl+MBCACc09O3gjyO0B1ledGxGFSUpPmhhJzkxKoY1WDX8VlASCHz +bAA/BytgYBXHTe7N+N3yJ6uiN3DIQ2j5uGWk/h5jyIOsRuzQxJ40n8AdK/71 +SGDCG1X5l1h9vmVTJxkQ3pcOxqRg55EEuJWKN1v7B1hIPxhaM5hgH/7s+PNn +lQddckQJqYkpm9Hy6EI7f9oHrOtWJWZoCHkWZVld7+9ZVPi34ex5ofYOuvNL +AIKZCc7lAiUiDJYQ+hIJRoYwLYhjIshpYoHgNeG4snlupNO32BiwDbHFDjeu +eoBLQ0rxZV7B664ceCmIl+VRht9G20hfGoTjAiop5tyrN1ZeL4EaI+aTABEB +AAEAB/oCKTQnftvHwrkBVlyzSN6tfXylF2551Q3n4CZGg3efI/9PCa9wF58+ +WApqmgsUqcNbVnDfl2T58ow05FLMxnFFNnHJq8ltfnXl+gG6c7iy94p79SQE +AGCOL7xNassXrDAQZhqWkCdiLK3b6r9F8Y3URb/AYbWH2BkFkS0oWQDav+Tw +lABt5vG2L5QtnShdqi8CCitcHGEKHocPHp0yAQlp3oAMq09YubgrzESDJ7Pe +l93cT35NlyimAZ6IYk/gumX0/6spqcw7205KfG6P84WlMp3WmE0IUWtiOp+7 +rjMjDki0WeVKtuLbHBhOwKvxcILWz+0vQf3uu6aXOKQ3JlsVBADHoXa6QjrT +RmKD9ch65Pkd+EZiKhe+pqqIArVj4QsVBEnaggR59SD8uXhtpyBnvOp3xpof +Vut3SKWl/jmH7vKansFbHOo8xLUyVctu7lCL2/v85FcRJxfPK00MBic+z/vf +mWOAY1VBoi5I9qi6o8vVHA5BJ/xw2uV9VpxfiLG0vwQAyRxHmoZl/OxaZUsm +J9eDYV9xyYumkTCYvHPk9X+ehS+XeYh24z1q9a/1jEnSR3A5XE67UCLaspiA ++Px7nSU1+ftJ9bC2bnRR0Upop+3UkPeCBVp4tYAhsNnPXhSWC0gCgeGU7EmW +DechFg29LId35LXKgmXls9u5yDy2w978Hy0D/jbKZaxNMMwlx/XCFCoBEcXS +DBzg7GHNXdillJqy215j46lfVqOCB3IiffNKjHua2l6fQc0BoiWIZnElMnIa +faEBBSpOVqKhktDFacHa5xChjqXZVyw68qc0I36xkCfcwvYCpNKKkXv90r8A +tRI6gpBLeMJvkL3VkmKd6AZymxFxRGjNEkJvYiA8aW5mb0Bib2IuY29tPsLA +jQQQAQgAIAUCX0aX4wYLCQcIAwIEFQgKAgQWAgEAAhkBAhsDAh4BACEJEAr9 +x5ZY6oZmFiEEm+B7p+lshgEOwGGZCv3HlljqhmaUWgf/efmGSpOKIGQ3Kh32 +HUqn/4ARvUmqMtZz4xUA9P3GAPY8XwJf00jSQlAo4//3aA1eEOJFHCr2qzCk +/4gIoZEScTTZp4itfL/Fer3UX+bV/VeTNgZGi+MRylSDQxLRQNpRgu+FmRAi +E6fr8D8GMvEcGb0jTRgWGj1EVtfOHfDg+EyPrtw+Z8u/bErUJ+Fnxz+KOGSN +SBQVAOflUYFoQhUNgZiq1s8WFD55sfes3UdBwsmHquDtYGo9dvWLJXxTEF8q +QCyKHYdk25ShIlNpRUqOH3CHqY/38z7QeV7INwtZaQvoES08RlD6ZMtczYLj +BZou86lozq7ISvRg1RSIWZ0ZRA== +=A9Ts +-----END PGP PRIVATE KEY BLOCK----- +` + +const armoredMessageRsaSignOnly = `-----BEGIN PGP MESSAGE----- + +wcBMAwr9x5ZY6oZmAQf+Lxghg4keIFpEq8a65gFkIfW+chHTDPlfI8xnx6U9 +HdsICX3Oye5V0ToCVKkEWDxfN1yCfXiYalSNo7ScRZKR7C+j02/pC+FfR6AJ +2cvdFoGIrLaXdjXXc/oXbsCCZA4C1DhQqpdORo2qGF0Q6Sm8659B0CfOgYSL +fBfKQ5VJngUT5JG8Uek3YuXBufPNhzdmXLHyB2Y2CwKkldi2vo4YNAukDhrR +2TojxdNoouhnMm+gloCE1n8huY1vw5F78/uiHen0tmHQ0dxtfk8cc1burgl/ +zUdJ3Sg6Eu+OC2ae5II63iB5fG+lCwZtfuepWnePDv8RDKNHCVP/LoBNpGOZ +U9I6AUkZWdcsueib9ghKDDy+HbUbf2kCJWUnuyeOCKqQifDb8bsLmdQY4Wb6 +EBeLgD8oZHVsH3NLjPakPw== +=STqy +-----END PGP MESSAGE-----` From 5ad654034e96c3df4f3ac8cb85b869bdd7a7a87c Mon Sep 17 00:00:00 2001 From: Lukas Burkhalter Date: Wed, 20 Nov 2024 13:48:21 +0100 Subject: [PATCH 3/4] feat(v2): Flag to verify siganture with keys in the future --- openpgp/packet/config.go | 16 ++++++++++++++++ openpgp/v2/keys.go | 8 ++++++++ openpgp/v2/read.go | 7 +++++-- openpgp/v2/subkeys.go | 8 ++++++++ openpgp/v2/user.go | 3 +++ 5 files changed, 40 insertions(+), 2 deletions(-) diff --git a/openpgp/packet/config.go b/openpgp/packet/config.go index 8bf8e6e5..7cb7ba51 100644 --- a/openpgp/packet/config.go +++ b/openpgp/packet/config.go @@ -144,6 +144,15 @@ type Config struct { // ignored key flags when selecting a key for encryption. // Not relevant for the v1 API, as all keys were allowed in decryption. InsecureAllowDecryptionWithSigningKeys bool + // InsecureAllowVerificationWithReformattedKeys enables signature verification + // for scenarios where the signing key is newer than the message being verified (v2 API). + // This situation can occur if a key was reformatted, resulting in its self-signatures + // having timestamps in the future relative to the message signature, which would + // typically render the key invalid at the time of signing. + // Enabling this setting bypasses the timestamp validation, allowing such messages + // to be verified successfully. + // Note: This option is not applicable to the v1 API. + InsecureAllowVerificationWithReformattedKeys bool // KnownNotations is a map of Notation Data names to bools, which controls // the notation names that are allowed to be present in critical Notation Data // signature subpackets. @@ -296,6 +305,13 @@ func (c *Config) AllowUnauthenticatedMessages() bool { return c.InsecureAllowUnauthenticatedMessages } +func (c *Config) AllowVerificationWithReformattedKeys() bool { + if c == nil { + return false + } + return c.InsecureAllowVerificationWithReformattedKeys +} + func (c *Config) AllowDecryptionWithSigningKeys() bool { if c == nil { return false diff --git a/openpgp/v2/keys.go b/openpgp/v2/keys.go index 93082dd4..d6e9afbb 100644 --- a/openpgp/v2/keys.go +++ b/openpgp/v2/keys.go @@ -671,6 +671,9 @@ func (e *Entity) SignIdentity(identity string, signer *Entity, config *packet.Co func (e *Entity) LatestValidDirectSignature(date time.Time, config *packet.Config) (selectedSig *packet.Signature, err error) { for sigIdx := len(e.DirectSignatures) - 1; sigIdx >= 0; sigIdx-- { sig := e.DirectSignatures[sigIdx] + if config.AllowVerificationWithReformattedKeys() && date.Unix() < sig.Packet.CreationTime.Unix() { + date = sig.Packet.CreationTime + } if (date.IsZero() || date.Unix() >= sig.Packet.CreationTime.Unix()) && (selectedSig == nil || selectedSig.CreationTime.Unix() < sig.Packet.CreationTime.Unix()) { if sig.Valid == nil { @@ -718,6 +721,11 @@ func (e *Entity) VerifyPrimaryKey(date time.Time, config *packet.Config) (*packe if err != nil { return nil, goerrors.New("no valid self signature found") } + + if config.AllowVerificationWithReformattedKeys() && primarySelfSignature.CreationTime.Unix() > date.Unix() { + date = primarySelfSignature.CreationTime + } + // check for key revocation signatures if e.Revoked(date) { return nil, errors.ErrKeyRevoked diff --git a/openpgp/v2/read.go b/openpgp/v2/read.go index 5ab9aff5..3c1a507b 100644 --- a/openpgp/v2/read.go +++ b/openpgp/v2/read.go @@ -577,7 +577,6 @@ func (scr *signatureCheckReader) Read(buf []byte) (int, error) { // Verify and retrieve signing key at signature creation time key, err := candidate.SignedByEntity.signingKeyByIdUsage(sig.CreationTime, candidate.IssuerKeyId, packet.KeyFlagSign, scr.config) if err != nil { - candidate.SignatureError = err continue } else { candidate.SignedBy = &key @@ -783,7 +782,11 @@ func checkMessageSignatureDetails(verifiedKey *Key, signature *packet.Signature, if sig == signature { time = config.Now() } else { - time = signature.CreationTime + if config.AllowVerificationWithReformattedKeys() && signature.CreationTime.Unix() < sig.CreationTime.Unix() { + time = sig.CreationTime + } else { + time = signature.CreationTime + } } if err := checkSignatureDetails(pk, sig, time, config); err != nil { errs = append(errs, err) diff --git a/openpgp/v2/subkeys.go b/openpgp/v2/subkeys.go index 9dc70899..7ef31761 100644 --- a/openpgp/v2/subkeys.go +++ b/openpgp/v2/subkeys.go @@ -109,6 +109,11 @@ func (s *Subkey) Verify(date time.Time, config *packet.Config) (selfSig *packet. if err != nil { return nil, err } + + if config.AllowVerificationWithReformattedKeys() && selfSig.CreationTime.Unix() > date.Unix() { + date = selfSig.CreationTime + } + if s.Revoked(selfSig, date) { return nil, errors.ErrKeyRevoked } @@ -182,6 +187,9 @@ func (s *Subkey) Revoke(reason packet.ReasonForRevocation, reasonText string, co func (s *Subkey) LatestValidBindingSignature(date time.Time, config *packet.Config) (selectedSig *packet.Signature, err error) { for sigIdx := len(s.Bindings) - 1; sigIdx >= 0; sigIdx-- { sig := s.Bindings[sigIdx] + if config.AllowVerificationWithReformattedKeys() && date.Unix() < sig.Packet.CreationTime.Unix() { + date = sig.Packet.CreationTime + } if (date.IsZero() || date.Unix() >= sig.Packet.CreationTime.Unix()) && (selectedSig == nil || selectedSig.CreationTime.Unix() < sig.Packet.CreationTime.Unix()) { if sig.Valid == nil { diff --git a/openpgp/v2/user.go b/openpgp/v2/user.go index 3da03bd7..bd1d4078 100644 --- a/openpgp/v2/user.go +++ b/openpgp/v2/user.go @@ -200,6 +200,9 @@ func (ident *Identity) SignIdentity(signer *Entity, config *packet.Config) error func (i *Identity) LatestValidSelfCertification(date time.Time, config *packet.Config) (selectedSig *packet.Signature, err error) { for sigIdx := len(i.SelfCertifications) - 1; sigIdx >= 0; sigIdx-- { sig := i.SelfCertifications[sigIdx] + if config.AllowVerificationWithReformattedKeys() && date.Unix() < sig.Packet.CreationTime.Unix() { + date = sig.Packet.CreationTime + } if (date.IsZero() || date.Unix() >= sig.Packet.CreationTime.Unix()) && // SelfCertification must be older than date (selectedSig == nil || selectedSig.CreationTime.Unix() < sig.Packet.CreationTime.Unix()) { // Newer ones are preferred if sig.Valid == nil { From 37881fe1fb05bc1a5f8653f60ea4f31bc6ddc744 Mon Sep 17 00:00:00 2001 From: Lukas Burkhalter Date: Wed, 20 Nov 2024 14:08:28 +0100 Subject: [PATCH 4/4] test: Add test to verify signature with future key --- openpgp/v2/read_test.go | 57 ++++++++++++++++++++++++++++++ openpgp/v2/read_write_test_data.go | 25 +++++++++++++ 2 files changed, 82 insertions(+) diff --git a/openpgp/v2/read_test.go b/openpgp/v2/read_test.go index 4de095f0..5a81f6b4 100644 --- a/openpgp/v2/read_test.go +++ b/openpgp/v2/read_test.go @@ -1024,6 +1024,63 @@ func testMalformedMessage(t *testing.T, keyring EntityList, message string) { } } +func TestReadMessageWithReformattedKey(t *testing.T) { + config := packet.Config{ + InsecureAllowVerificationWithReformattedKeys: true, + } + key, err := ReadArmoredKeyRing(strings.NewReader(armoredReformattedKey)) + if err != nil { + t.Error(err) + return + } + + msgReader, err := armor.Decode(strings.NewReader(armoredMessageWithReformattedKey)) + if err != nil { + t.Error(err) + return + } + md, err := ReadMessage(msgReader.Body, key, nil, &config) + if err != nil { + t.Error(err) + return + } + _, err = io.ReadAll(md.UnverifiedBody) + if err != nil { + t.Error(err) + return + } + if !md.IsVerified { + t.Errorf("not verified despite all data read") + } + if md.SignatureError != nil { + t.Error("expected no signature error, got:", md.SignatureError) + } + + msgReader, err = armor.Decode(strings.NewReader(armoredMessageWithReformattedKey)) + if err != nil { + t.Error(err) + return + } + + // Fail + md, err = ReadMessage(msgReader.Body, key, nil, nil) + if err != nil { + t.Error(err) + return + } + _, err = io.ReadAll(md.UnverifiedBody) + if err != nil { + t.Error(err) + return + } + if !md.IsVerified { + t.Errorf("not verified despite all data read") + } + if md.SignatureError == nil { + t.Fatal("signature verification must fail") + } +} + func TestReadMessageWithSignOnly(t *testing.T) { config := packet.Config{ InsecureAllowDecryptionWithSigningKeys: true, diff --git a/openpgp/v2/read_write_test_data.go b/openpgp/v2/read_write_test_data.go index 9322b949..df18f3d3 100644 --- a/openpgp/v2/read_write_test_data.go +++ b/openpgp/v2/read_write_test_data.go @@ -741,6 +741,31 @@ xqAY9Bwizt4FWgXuLm1a4+So4V9j1TRCXd12Uc2l2RNmgDE= =miES -----END PGP PRIVATE KEY BLOCK-----` +const armoredReformattedKey = `-----BEGIN PGP PRIVATE KEY BLOCK----- + +xVgEYWmlshYJKwYBBAHaRw8BAQdAAxpFNPiHxz9q4HBzWqveHdP/knjwlgv8 +pEQCMHDpIZIAAP9WFlwHDuVlvNb7CyoikwmG01nmdMDe9wXQRWA5vasWKA+g +zSV0ZXN0QHJlZm9ybWF0LmNvbSA8dGVzdEByZWZvcm1hdC5jb20+wowEEBYK +AB0FAmFppjQECwkHCAMVCAoEFgACAQIZAQIbAwIeAQAhCRAOZNKOg+/XQxYh +BGqP/hIaYCSJsZ4TrQ5k0o6D79dD+c8BAIXdh2hrC+l49WPN/KZF+ZzvWCWa +W5n+ozbp/sOGXvODAP4oGEj0RUDDA33b6x7fhQysBZxdrrnHxP9AXEdOTQC3 +CsddBGFppbISCisGAQQBl1UBBQEBB0Cjy8Z2K7rl6J6AK1lCfVozmyLE0Gbv +1cspce6oCF6oCwMBCAcAAP9OL5V80EaYm2ic19aM+NtTj4UNOqKqIt10AaH9 +SlcdMBDgwngEGBYIAAkFAmFppjQCGwwAIQkQDmTSjoPv10MWIQRqj/4SGmAk +ibGeE60OZNKOg+/XQx/EAQCM0UYrObp60YbOCxu07Dm6XjCVylbOcsaxCnE7 +2eMU4AD+OkgajZgbqSIdAR1ud76FW+W+3xlDi/SMFdU7D49SbQI= +=ASQu +-----END PGP PRIVATE KEY BLOCK-----` + +const armoredMessageWithReformattedKey = `-----BEGIN PGP MESSAGE----- + +xA0DAQoWDmTSjoPv10MByw91AGFpplFwbGFpbnRleHTCdQQBFgoABgUCYWml +sgAhCRAOZNKOg+/XQxYhBGqP/hIaYCSJsZ4TrQ5k0o6D79dDDWwBAKUnRWXj +P3HTW521iD/DngK54mYS3feQzhDokhkYjO3UAP0ZlsYShKaJvXh+JgvR5BPP +gjVcn04JVVlxqgVnMqeVBw== +=eyO7 +-----END PGP MESSAGE-----` + const rsaSignOnly = `-----BEGIN PGP PRIVATE KEY BLOCK----- xcLYBF9Gl+MBCACc09O3gjyO0B1ledGxGFSUpPmhhJzkxKoY1WDX8VlASCHz