ProtonMail/go-crypto

file that encrypted by go-openPGP can not be decrypted by GPG (Low probability

izouxv opened this issue · 8 comments

BUG info

file that encrypted by go-openPGP can not be decrypted by GPG.
file that encrypted by go-openPGP can  be decrypted by go-openPGP.
file that encrypted by PGP can  be decrypted by go-openPGP.

go (encrypt )      =>    gpg ( decrypt )  ERR
gpg (encrypt )    =>  go (decrypt )    OK
go ( encrypt)      =>   go (decrypt )    OK

this test data package have a test file that can reproduce the err

part of code


func encryptFile(t *testing.T, sourcefile, targetfile string, entity *openpgp.Entity) {
	in, err := os.Open(sourcefile)
	if err != nil {
		t.Fatal(err)
	}
	defer in.Close()
	w, err := encryptWithEntitle(t, entity, in)
	if err != nil {
		t.Fatal(err)
	}
	// defer w.Close()
	ww, err := os.Create(targetfile)
	if err != nil {
		t.Fatal(err)
	}
	defer ww.Close()
	len, err := io.Copy(ww, w)
	log.Printf("stream encrypted, len: %v", len)
	if err != nil {
		t.Fatal(err)
	}
}
func encryptWithEntitle(t *testing.T, entity *openpgp.Entity, data io.Reader) (io.Reader, error) {
	entityList := openpgp.EntityList{entity}
	buf := new(bytes.Buffer)
	wc, err := openpgp.Encrypt(buf, entityList, nil, nil, config)
	if err != nil {
		t.Errorf("encrypt err: %v", err)
		return nil, err
	}
	writelen, err := io.Copy(wc, data)
	defer wc.Close()
	t.Logf("encryptStream, len: %v, err: %v", writelen, err)
	return buf, nil
}

func TestOpenPGP_GoEncrypt_GpgDecrypt(t *testing.T) {
	entile, prikey2, err := createPgpKeyAndFile(t)
	if err != nil {
		t.Fatalf("create pgp key err: %v", err)
	}
	importKeyToKeyChain(t, fullPath(kAscName), prikey2)
	t.Log("encrypt with go openpgpg")
	encryptFile(t, fullPath(kOriginName), fullPath(kGpgName), entile)

	t.Log("decrypt with gpg")
	Run(kTestDir, "gpg", []string{"--output", kTestNewName, "--recipient", kEmail, "--decrypt", kGpgName})

	t.Log("file md5 caculating...")
	testNewNameMd5 := MD5(fullPath(kTestNewName))
	originNameMd5 := MD5(fullPath(kOriginName))

	if testNewNameMd5 != originNameMd5 {
		t.Fatalf("origin file md5 not equal the new file md5")
	}
}

gpg command line

gpg --output origin_new.png --recipient email@email.com --decrypt test.png.gpg

env information

gpg (GnuPG) 2.2.16
libgcrypt 1.8.4
mac os 10.15
go version go1.13.1 darwin/amd64

full test code:

[testdata.zip](https://github.com/ProtonMail/crypto/files/3782874/testdata.zip)

Thanks for the bug report.
I confirm the same results, with

gpg (GnuPG) 2.2.12
libgcrypt 1.8.4
Ubuntu 19.04
go version go1.12.9 linux/amd64

Hello, small update on this.
I witnessed correct results of the following with RSA3072, GPG-generated keys, armored input/outputs, and dummy .txt textfile)

EncryptGPG   -> DecryptGo    OK
EncryptGo    -> DecryptGo    OK (also covered in end_to_end_test, see refactor pr #37)
EncryptGo    -> DecryptGPG   OK

This doesn't rule out anything, but it suggests that the problem may arise when using Go-generated keys, unarmored encrypted files, etc. I'm looking into it.

@izouxv,
Could you please elaborate on the "Low Probability" claim? Was your test passing with other files?

Identified the probable cause of the problem: The original binary file uses DOS (^M or \r\n) linebreaks. However, after encrypt/decrypt it gets translated to unix: Inspect origin.png versus test-gpg-new.txt with vi.
Deleting all ^M occurrences in the binary makes the tests to pass, however, a classical dos to unix conversion is not enough (see e.g. dos2unix), since some of the conflicting ^M do not occur at the end of lines (in origin.png, there is one right here ^@^@^@^MIHDR^@, for instance).
I'll come back with a fix soon, thanks for letting us know, cheers.

i think media file, like png, mp4 is more likely to happen
Yes, This error will also occur with other files, especially media files, eg jpg, mp4. i am trying to encryptGO and decrypt pgp a movie, and this error be occured. and, i tested many txt file , all txt file passed the test case

Archive.zip
txt file passed
png, jpe file error

Thank you.

When encrypting a binary, you need to include the IsBinary hint into the Encrypt call. So instead of

wc, err := openpgp.Encrypt(buf, entityList, nil, nil, config)

you need

hints := &openpgp.FileHints{
	IsBinary: true,
}
wc, err := openpgp.Encrypt(buf, entityList, nil, hints, config)

Your test passes with this (and also some other similar tests I wrote for the occasion).

thank you

No problem, feel free to report back if you have any other issue 👍