Adapt existing ristretto225 implementation to the CIRCL Group interfaces()

[chris-wood]

This change wraps @bwesterb's ristretto225 implementation. I'm opening it up as a draft PR to start, since there are some questions I'd like to work through regarding the Order() function (see #214). Once we agree on the general design, I'll add tests along the lines of group_test.go.

cc @claucece, @wbl

[bwesterb]

I'm happy to make changes to go-ristretto to accommodate the spec.

[chris-wood]

I think it's a new API. Derive is basically FromUniformBytes, but this is a wrapper around that interface.

[bwesterb]

Could you be a bit more specific what functionality you need?

[bwesterb]

I could expose a more efficient double in go-ristretto, if desired.

[chris-wood]

Yes, please. :-)

[chris-wood]

bwesterb/go-ristretto#21

[bwesterb]

Done.

[chris-wood]

Promoting for full review after adding test vectors. I'll investigate the build failures in a bit.

[chris-wood]

Hmm, the "verifying code" stage is failing in CI, due to a new external library:

# github.com/stretchr/testify/assert
Error: ../../../go/pkg/mod/github.com/stretchr/testify@v1.7.0/assert/assertions.go:1704:5: undefined: errors.Is
Error: ../../../go/pkg/mod/github.com/stretchr/testify@v1.7.0/assert/assertions.go:1727:6: undefined: errors.Is
Error: ../../../go/pkg/mod/github.com/stretchr/testify@v1.7.0/assert/assertions.go:1750:5: undefined: errors.As
Error: ../../../go/pkg/mod/github.com/stretchr/testify@v1.7.0/assert/assertions.go:1767:7: undefined: errors.Unwrap
Error: ../../../go/pkg/mod/github.com/stretchr/testify@v1.7.0/assert/assertions.go:1771:7: undefined: errors.Unwrap

@armfazh, what does this stage do?

[armfazh]

also add ristretto to the batch of tests.

func TestGroup(t *testing.T) {
	for _, g := range []group.Group{
		group.P256,
		group.P384,
		group.P521,
		group.Ristretto255,

[chris-wood]

@armfazh done -- good catch! I had to fix the identity serialization test. The change makes me wonder if Marshal() should ever return an encoded element whose byte length is less than the expected size. (For example, 32 zeroes instead of 1 zero.) I can see arguments for either or, but here I went with the former.

[armfazh]

@armfazh done -- good catch! I had to fix the identity serialization test. The change makes me wonder if Marshal() should ever return an encoded element whose byte length is less than the expected size. (For example, 32 zeroes instead of 1 zero.) I can see arguments for either or, but here I went with the former.

I think it is ok to check for all-zeros, but how many of them?

There should be some field in Group that tells you the encoding size. This works well with groups that have fixed-size encodings, but for NIST groups we have variable length encoding, say 1 byte, n+1 bytes, or 2n+1 bytes.
-- btw, a similar discussion in the hpke list.

[chris-wood]

I think it is ok to check for all-zeros, but how many of them?

My take: the number of zeroes should match the size of an encoded element. (That means the existing group implementations should change, since the identity encoding is []byte{0x00}.)

Can we address this in a future issue?

[armfazh]

My take: the number of zeroes should match the size of an encoded element. (That means the existing group implementations should change, since the identity encoding is []byte{0x00}.)

1-byte encoding for identity follows the SEC specification.

Can we address this in a future issue?

sure, this is not a blocker for this pr.

[claucece]

LGTM.

[chris-wood]

@armfazh what additional changes would you like to see?