Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

No v6 ECC keys with legacy OIDs #234

Merged
merged 2 commits into from
Oct 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions openpgp/internal/ecc/curve_info.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import (
"github.com/ProtonMail/go-crypto/openpgp/internal/encoding"
)

const Curve25519GenName = "Curve25519"

type CurveInfo struct {
GenName string
Oid *encoding.OID
Expand Down Expand Up @@ -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(),
},
Expand All @@ -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(),
},
Expand Down
12 changes: 9 additions & 3 deletions openpgp/key_generation.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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{
Expand Down
30 changes: 29 additions & 1 deletion openpgp/packet/public_key.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
12 changes: 9 additions & 3 deletions openpgp/v2/key_generation.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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{
Expand Down
17 changes: 17 additions & 0 deletions openpgp/v2/keys_test_data.go
Original file line number Diff line number Diff line change
Expand Up @@ -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-----
`
18 changes: 18 additions & 0 deletions openpgp/v2/keys_v6_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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", "[email protected]", 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")
}
}
Loading