From f6ad483b4efe49419338a7ee23fffef48739acb8 Mon Sep 17 00:00:00 2001 From: Lukas Burkhalter <10532077+lubux@users.noreply.github.com> Date: Tue, 1 Oct 2024 09:37:11 +0200 Subject: [PATCH] No v6 ECC keys with legacy OIDs (#234) This PR addresses a compliance issue with RFC 9580, which mandates: "Implementations MUST NOT accept or generate version 6 key material using the deprecated OIDs." Currently, the implementation allows generation of v6 keys with deprecated OIDs, which violates the specification. This MR resolves the issue by introducing an error during key generation and parsing when deprecated OIDs are detected. --- openpgp/internal/ecc/curve_info.go | 6 ++++-- openpgp/key_generation.go | 12 +++++++++--- openpgp/packet/public_key.go | 30 +++++++++++++++++++++++++++++- openpgp/v2/key_generation.go | 12 +++++++++--- openpgp/v2/keys_test_data.go | 17 +++++++++++++++++ openpgp/v2/keys_v6_test.go | 18 ++++++++++++++++++ 6 files changed, 86 insertions(+), 9 deletions(-) diff --git a/openpgp/internal/ecc/curve_info.go b/openpgp/internal/ecc/curve_info.go index 97f891ffc..0da2d0d85 100644 --- a/openpgp/internal/ecc/curve_info.go +++ b/openpgp/internal/ecc/curve_info.go @@ -10,6 +10,8 @@ import ( "github.com/ProtonMail/go-crypto/openpgp/internal/encoding" ) +const Curve25519GenName = "Curve25519" + type CurveInfo struct { GenName string Oid *encoding.OID @@ -43,7 +45,7 @@ var Curves = []CurveInfo{ }, { // Curve25519 - GenName: "Curve25519", + GenName: Curve25519GenName, Oid: encoding.NewOID([]byte{0x2B, 0x06, 0x01, 0x04, 0x01, 0x97, 0x55, 0x01, 0x05, 0x01}), Curve: NewCurve25519(), }, @@ -55,7 +57,7 @@ var Curves = []CurveInfo{ }, { // Ed25519 - GenName: "Curve25519", + GenName: Curve25519GenName, Oid: encoding.NewOID([]byte{0x2B, 0x06, 0x01, 0x04, 0x01, 0xDA, 0x47, 0x0F, 0x01}), Curve: NewEd25519(), }, diff --git a/openpgp/key_generation.go b/openpgp/key_generation.go index a40e45bee..c9502c25f 100644 --- a/openpgp/key_generation.go +++ b/openpgp/key_generation.go @@ -41,7 +41,9 @@ func NewEntity(name, comment, email string, config *packet.Config) (*Entity, err } primary := packet.NewSignerPrivateKey(creationTime, primaryPrivRaw) if config.V6() { - primary.UpgradeToV6() + if err := primary.UpgradeToV6(); err != nil { + return nil, err + } } e := &Entity{ @@ -187,7 +189,9 @@ func (e *Entity) AddSigningSubkey(config *packet.Config) error { sub := packet.NewSignerPrivateKey(creationTime, subPrivRaw) sub.IsSubkey = true if config.V6() { - sub.UpgradeToV6() + if err := sub.UpgradeToV6(); err != nil { + return err + } } subkey := Subkey{ @@ -232,7 +236,9 @@ func (e *Entity) addEncryptionSubkey(config *packet.Config, creationTime time.Ti sub := packet.NewDecrypterPrivateKey(creationTime, subPrivRaw) sub.IsSubkey = true if config.V6() { - sub.UpgradeToV6() + if err := sub.UpgradeToV6(); err != nil { + return err + } } subkey := Subkey{ diff --git a/openpgp/packet/public_key.go b/openpgp/packet/public_key.go index f279789dc..f8da781bb 100644 --- a/openpgp/packet/public_key.go +++ b/openpgp/packet/public_key.go @@ -63,9 +63,10 @@ func (pk *PublicKey) UpgradeToV5() { // UpgradeToV6 updates the version of the key to v6, and updates all necessary // fields. -func (pk *PublicKey) UpgradeToV6() { +func (pk *PublicKey) UpgradeToV6() error { pk.Version = 6 pk.setFingerprintAndKeyId() + return pk.checkV6Compatibility() } // signingKey provides a convenient abstraction over signature verification @@ -313,6 +314,23 @@ func (pk *PublicKey) setFingerprintAndKeyId() { } } +func (pk *PublicKey) checkV6Compatibility() error { + // Implementations MUST NOT accept or generate version 6 key material using the deprecated OIDs. + switch pk.PubKeyAlgo { + case PubKeyAlgoECDH: + curveInfo := ecc.FindByOid(pk.oid) + if curveInfo == nil { + return errors.UnsupportedError(fmt.Sprintf("unknown oid: %x", pk.oid)) + } + if curveInfo.GenName == ecc.Curve25519GenName { + return errors.StructuralError("cannot generate v6 key with deprecated OID: Curve25519Legacy") + } + case PubKeyAlgoEdDSA: + return errors.StructuralError("cannot generate v6 key with deprecated algorithm: EdDSALegacy") + } + return nil +} + // parseRSA parses RSA public key material from the given Reader. See RFC 4880, // section 5.5.2. func (pk *PublicKey) parseRSA(r io.Reader) (err error) { @@ -437,6 +455,11 @@ func (pk *PublicKey) parseECDH(r io.Reader) (err error) { return errors.UnsupportedError(fmt.Sprintf("unknown oid: %x", pk.oid)) } + if pk.Version == 6 && curveInfo.GenName == ecc.Curve25519GenName { + // Implementations MUST NOT accept or generate version 6 key material using the deprecated OIDs. + return errors.StructuralError("cannot read v6 key with deprecated OID: Curve25519Legacy") + } + pk.p = new(encoding.MPI) if _, err = pk.p.ReadFrom(r); err != nil { return @@ -474,6 +497,11 @@ func (pk *PublicKey) parseECDH(r io.Reader) (err error) { } func (pk *PublicKey) parseEdDSA(r io.Reader) (err error) { + if pk.Version == 6 { + // Implementations MUST NOT accept or generate version 6 key material using the deprecated OIDs. + return errors.StructuralError("cannot generate v6 key with deprecated algorithm: EdDSALegacy") + } + pk.oid = new(encoding.OID) if _, err = pk.oid.ReadFrom(r); err != nil { return diff --git a/openpgp/v2/key_generation.go b/openpgp/v2/key_generation.go index 5537d4f84..1617d48b4 100644 --- a/openpgp/v2/key_generation.go +++ b/openpgp/v2/key_generation.go @@ -83,7 +83,9 @@ func newEntity(uid *userIdData, config *packet.Config) (*Entity, error) { } primary := packet.NewSignerPrivateKey(creationTime, primaryPrivRaw) if config.V6() { - primary.UpgradeToV6() + if err := primary.UpgradeToV6(); err != nil { + return nil, err + } } keyProperties := selectKeyProperties(creationTime, config, primary) @@ -259,7 +261,9 @@ func (e *Entity) AddSigningSubkey(config *packet.Config) error { sub.IsSubkey = true // Every subkey for a v6 primary key MUST be a v6 subkey. if e.PrimaryKey.Version == 6 { - sub.UpgradeToV6() + if err := sub.UpgradeToV6(); err != nil { + return err + } } subkey := Subkey{ @@ -308,7 +312,9 @@ func (e *Entity) addEncryptionSubkey(config *packet.Config, creationTime time.Ti sub.IsSubkey = true // Every subkey for a v6 primary key MUST be a v6 subkey. if e.PrimaryKey.Version == 6 { - sub.UpgradeToV6() + if err := sub.UpgradeToV6(); err != nil { + return err + } } subkey := Subkey{ diff --git a/openpgp/v2/keys_test_data.go b/openpgp/v2/keys_test_data.go index 4e0fed110..14e0b36c3 100644 --- a/openpgp/v2/keys_test_data.go +++ b/openpgp/v2/keys_test_data.go @@ -536,3 +536,20 @@ VppQxdtxPvAA/34snHBX7Twnip1nMt7P4e2hDiw/hwQ7oqioOvc6jMkP =Z8YJ -----END PGP PRIVATE KEY BLOCK----- ` + +const LegacyECCKeyV6 = `-----BEGIN PGP PRIVATE KEY BLOCK----- + +xVoGZtnEsRYAAAAtCSsGAQQB2kcPAQEHQJ8u3Y3xhq7vDoYgyIjhniUavG9lhqqn +eoQLj08i1UyCAAEAnZMljJWA74axuVNrL6uhyBLGCwypSeWCXBRqKzR4yjzCswYf +FgoAAABBBQJm2cSxAhsDAh4JCAsJCAcKDQwLBRUKCQgLAhYCIiEG6/BxiEOhwOXw +t2eAShAq9G0keONgsN0KdQSyvNnuD3YAAAAAFEcgEheAMaYvmoAP4UKwK0hOPtFC +wnuf7v+O9tpko93/9GQA9Am6sr0XTsa8RDRc49lm5k2WYwGx5yk2JgsFLX26RJAA +/j0NorcLtTCV/uejDw26ZsWg2kF69ovLed4wF26iCaAHx18GZtnEsRIAAAAyCisG +AQQBl1UBBQEBB0DavdPV3M/mAO5/IkYVdHu006uVFO3eIuZ2ffJPoe5KGQMBCAcA +AP9V2OXFg/QewXMGMNol6S1DUbKhGuyKEK0hrmzUz224qMKZBhgWCAAAACwFAmbZ +xLECGwwiIQbr8HGIQ6HA5fC3Z4BKECr0bSR442Cw3Qp1BLK82e4PdgAAAAoJEOvw +cYhDocDl37EQls/BpUt572AJX8ZBqbsycgD/QfCjSRcPHelHFHkaAHMzXscDBaUS +jiCm2KiM+33wOYYA/2Hu4xmHg7wN2prRcB4+2qkUIdEzMyFs5TYcOU7Hst0N +=pm0Y +-----END PGP PRIVATE KEY BLOCK----- +` diff --git a/openpgp/v2/keys_v6_test.go b/openpgp/v2/keys_v6_test.go index c40c1ba84..66220a8ad 100644 --- a/openpgp/v2/keys_v6_test.go +++ b/openpgp/v2/keys_v6_test.go @@ -224,3 +224,21 @@ func TestKeyGenerationHighSecurityLevel(t *testing.T) { } } + +func TestKeyGenLegacyECC(t *testing.T) { + c := &packet.Config{ + V6Keys: true, + Algorithm: packet.PubKeyAlgoEdDSA, + } + _, err := NewEntity("V6 Key Owner", "V6 Key", "v6@pm.me", c) + if err == nil { + t.Fatal("should not be able to generate v6 keys with legacy OIDs") + } +} + +func TestReadLegacyECC(t *testing.T) { + _, err := ReadArmoredKeyRing(strings.NewReader(LegacyECCKeyV6)) + if err == nil { + t.Fatal("should not be able to read v6 legacy ECC key") + } +}