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

ECIES with ECDSA keys from the Go standard library? #29744

Open
sietseringers opened this issue May 9, 2024 · 5 comments
Open

ECIES with ECDSA keys from the Go standard library? #29744

sietseringers opened this issue May 9, 2024 · 5 comments
Assignees

Comments

@sietseringers
Copy link

Since #28946, in particular commit ab49f22, the Encrypt() and Decrypt() functions in the crypto/ecies require the public keys to implement the crypto.EllipticCurve interface, otherwise they return ecies.ErrInvalidCurve. Consequentially, since this change the ecies package no longer accepts ECDSA keys as returned by the Go standard library, e.g. generated with ecdsa.GenerateKey(elliptic.P256(), rand.Reader), since those do not implement crypto.EllipticCurve. This used to work fine in versions v1.13.x, as shown by the following test, which works in v1.13.x and fails in v1.14.x:

func TestECDSAKeys(t *testing.T) {
	privkey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
	if err != nil {
		t.Fatal(err)
	}

	ecies_privkey := ImportECDSA(privkey)
	message := []byte("Hello, world.")
	ct, err := Encrypt(rand.Reader, &ecies_privkey.PublicKey, message, nil, nil)
	if err != nil {
		t.Fatal(err)
	}

	pt, err := ecies_privkey.Decrypt(ct, nil, nil)
	if err != nil {
		t.Fatal(err)
	}

	if !bytes.Equal(pt, message) {
		t.Fatal("ecies: plaintext doesn't match message")
	}
}

At the same time, in the ecies package the functions ExportECDSA(), ImportECDSA() and ImportECDSAPublic() for importing ecdsa.PublicKey and ecdsa.PrivateKey instances still exist. Those sort of suggest that using ordinary ECDSA keys (i.e. P256 keys from the Go standard library) should work, as well as these ECIES parameters being set up for the P256 curve from the standard library.

Should using ECDSA keys from the Go standard library work? In other words, is it a bug that the above test fails?

@MariusVanDerWijden
Copy link
Member

Are you compiling both tests with the same go version ?

Tagged this as triage, as we should have a discussion about ecies imo. We should in my opinion make it very clear that our ecies implementation should not be used or imported by 3rd parties, since we don't have the capabilities to properly maintain it atm

@MariusVanDerWijden
Copy link
Member

I remember there was a change recently that required us to explicitly set the curve in the marshalling and unmarshalling operations

edit: #28946

@MariusVanDerWijden
Copy link
Member

MariusVanDerWijden commented May 10, 2024

After more digging:
Changing the curve used in your test from elliptic.P256() to our own crypto.S256() made the test pass

Its because elliptic.P256() does not implement crypto.EllipticCurve because of the missing Marshall and Unmarshall functions.

If you want to use elliptic.P256() with our ecies library, you need to wrap it with marshalling and unmarshalling functions

@sietseringers
Copy link
Author

Right, thanks.

But then, why does this line exlicitly reference elliptic.P256()?

@MariusVanDerWijden
Copy link
Member

It used to work out of the box, but unfortunately due to the changes in go 1.22, it doesn't work without marshalling anymore

@fjl fjl self-assigned this May 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants