ECIES with ECDSA keys from the Go standard library?

[sietseringers]

Since #28946, in particular commit ab49f228ad6f37ba78be66b34aa5fee740245f57, 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]

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]

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

edit: https://github.com/ethereum/go-ethereum/pull/28946

[MariusVanDerWijden]

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]

Right, thanks.

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

[MariusVanDerWijden]

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