dedis / cothority

Scalable collective authority
Other
424 stars 106 forks source link

kyberJS: point.data() might not handle 0 embedded bytes correctly #2276

Open jeffallen opened 4 years ago

jeffallen commented 4 years ago

In the evoting app, I tried decoding and counting a ballot which had been encrypted as follows:

            secret := cothority.Suite.Scalar().Pick(random.New())
            public := cothority.Suite.Point().Mul(secret, nil)
            K, C := lib.Encrypt(public, nil)
            b := &lib.Ballot{
                User:  req.Election.Users[0],
                Alpha: K,
                Beta:  C,
            }

The second argument to lib.Encrypt is a []byte. So I asked to embed 0 bytes into a point and then transform the point via ElGamal encryption.

When I decoded the ballot later, I got: iteration 1 invalid ballot: Error: invalid embedded data length. I would have expected instead to not get an exception from point.data(), but instead to get zero bytes back, and then it would be up to my ballot decoder to notice that zero bytes is not a valid ballot.

ineiti commented 4 years ago

OK, I wrote a small test:

        data := []byte{}
    p := (&edwards25519.SuiteEd25519{}).Point()
    p.Embed(data, random.New())
    d, err := p.Data()
    if err != nil{
        log.Fatal(err)
    }
    log.Printf("%+v", d)
    log.Print(p.MarshalBinary())

This works. But if data := nil, then it fails - sometimes, as it depends what the beginning of the point looks like...

Looking at the implementation in edwards25519, it seems that it's a choice of returning a random point if data := nil - which doesn't make sense to me. @bford @Daeinar ?

So: what should p.Embed do with a nil data? If it would return an error, we could return it there. Unfortunately it only returns a Point. My order of preference:

  1. panic if a nil data is entered
  2. embed a size of 255 for nil and interpret this as a nil to be returned by Data
  3. treat a nil data the same as []byte{} and embed a 0-length data slice
  4. return a nil Point to indicate an error

But perhaps you can think of other solutions?