ProtonMail/go-crypto

bitLength not correct, and ECDH privateKey load with GPG is wrong

izouxv opened this issue · 4 comments

Hi
there are two bugs in this test case
1: bitlength error, when i generate entity with EdDSA, and the subkey bitlength is wrong. its 263, not 256
2: ouput this key to one asc file, and then , import the file with "gpg --import", its wrong. public key is ok, primary key is error
i tested ouput with password, and without password, it all wrong.
ONLY WITH ECDH

main test funcation

func TestOpenPGP_ExportECDH_GPGImport(t *testing.T) {
	entity, err := openpgp.NewEntity("name", "comment", "email@email.com", config)
	if err != nil {
		t.Fatalf("err: %v", err)
	}

	bitlen, err := entity.Subkeys[0].PublicKey.BitLength()
	if err != nil {
		t.Fatalf("err: %v", err)
	}
	if bitlen != 256 {
		t.Logf("!!! ERROR bitlen: %v: \n", bitlen)
	}

	pgpg, _ := exportGPG(entity)
	t.Fatalf("!!! ERROR, gpg --import with this file will ERROR: \n%v", pgpg)
}

ENV

go version go1.13.1 darwin/amd64
gpg (GnuPG) 2.2.16
libgcrypt 1.8.4
mac10.15

openpgp_test.go.zip

Hello again @izouxv!
Thanks for the issue. I will look into this.

I confirm the results of your test function with

go version go1.13.4 linux/amd64
gpg (GnuPG) 2.2.12
Ubuntu 19.04

I also confirm

  • The correctness of gpg --import with a key generated in Go with PubKeyAlgoRSA in config.
  • The correctness of importing a PubKeyAlsoEdDSA key (generated with Go) into Go, with ReadArmoredKeyRing.
  • The correctness of importing a EdDSA key (generated with gpg) with ReadArmoredKeyRing, into Go.

For the last bullet, the key was generated with

gpg --expert --full-gen-key

With the options:

(9) ECC and ECC
(1) Curve 25519
0 = key does not expire
No password

@izouxv, thanks a lot for bringing this up.

2.

The issue is that the current PrivateKey.SerializeUnEncrypted function (private_key.go) is missing the EdDSA case and the error checking, so it didn't serialize the private key and the error went undetected.

It should be fixed now in #40, and also the ed25519 keys were stored unmasked, which is an error and makes gpg to interpret them as encrypted.

I checked that your test is passing now (i.e., the private key you're printing in Fatalf can be correctly imported into gpg).

// (first fetch #40)
// Get your test file
wget https://github.com/ProtonMail/crypto/files/3819150/openpgp_test.go.zip
unzip openpgp_test.go.zip
go test -run TestOpenPGP_ExportECDH_GPGImport
// (write output key to file goExportedEd25519.asc)
// Import into gpg
gpg --import goExportedEd25519.asc

1.

About the bitLength error, it's completely normal that the length of the subkey is 263. For instance, I checked that a key imported from gpg also has this bit length. The reason is (from RFC4880bis):

   The EdDSA algorithm defines a specific point compression format.  To
   indicate the use of this compression format and to make sure that the
   key can be represented in the Multiprecision Integer (MPI) format the
   octet string specifying the point is prefixed with the octet 0x40.
   This encoding is an extension of the encoding given in [SEC1] which
   uses 0x04 to indicate an uncompressed point.

   For example, the length of a public key for the curve Ed25519 is 263
   bit: 7 bit to represent the 0x40 prefix octet and 32 octets for the
   native value of the public key.