Skip to content
This repository has been archived by the owner on Feb 22, 2024. It is now read-only.

The sign method panics with go1.19 #15

Closed
KirillovDenis opened this issue Aug 8, 2022 · 1 comment · Fixed by #16
Closed

The sign method panics with go1.19 #15

KirillovDenis opened this issue Aug 8, 2022 · 1 comment · Fixed by #16
Labels
bug Something isn't working

Comments

@KirillovDenis
Copy link

KirillovDenis commented Aug 8, 2022

With go1.19 this

return marshalXY(x, y), nil
panics now: https://tip.golang.org/doc/go1.19#minor_library_changes

This affects neofs-api-go https://github.com/nspcc-dev/neofs-api-go/blob/882c4ab76c53d9eb8c41ca9155b25dcd99836946/go.mod#L6 and then neofs-sdk-go, neofs-http-gw and so on.

Code to reproduce:

pk, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
require.NoError(t, err)

msg := []byte{1, 2, 3, 4}

_, err = crypto.Sign(pk, msg)
require.NoError(t, err)

Panic message:

panic: crypto/elliptic: attempted operation on invalid point [recovered]
	panic: crypto/elliptic: attempted operation on invalid point

goroutine 6 [running]:
testing.tRunner.func1.2({0xacc2e0, 0xcf7450})
	/home/denis/go/go1.19/src/testing/testing.go:1396 +0x24e
testing.tRunner.func1()
	/home/denis/go/go1.19/src/testing/testing.go:1399 +0x39f
panic({0xacc2e0, 0xcf7450})
	/home/denis/go/go1.19/src/runtime/panic.go:884 +0x212
crypto/elliptic.panicIfNotOnCurve({0xd05fa8?, 0x12245b0?}, 0x80?, 0x7?)
	/home/denis/go/go1.19/src/crypto/elliptic/elliptic.go:182 +0xa5
crypto/elliptic.Marshal({0xd05fa8, 0x12245b0}, 0xc0001d9a40?, 0xc00008feb8?)
	/home/denis/go/go1.19/src/crypto/elliptic/elliptic.go:75 +0x31
github.com/nspcc-dev/neofs-crypto.marshalXY(...)
	/home/denis/go/pkg/mod/github.com/nspcc-dev/[email protected]/ecdsa.go:44
github.com/nspcc-dev/neofs-crypto.Sign(0xd02318?, {0xc00008ff4c?, 0x0?, 0x0?})
	/home/denis/go/pkg/mod/github.com/nspcc-dev/[email protected]/ecdsa.go:217 +0xd8
@roman-khimov
Copy link
Member

This Sign() is very strange to me:

  1. It outputs 65-byte slice, while normally signatures are 64-byte, that is they don't have a prefix.
  2. The output of ecdsa.Sign() (https://pkg.go.dev/crypto/ecdsa#Sign) is not X and Y, but R and S. No wonder they're not on the curve.

Consider serializing the result like this: https://github.com/nspcc-dev/neo-go/blob/41613cd631e662dab1791adc306f53faddb17b89/pkg/crypto/keys/private_key.go#L155-L163

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants