-
Notifications
You must be signed in to change notification settings - Fork 0
Add test for encryption and VSS #3
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
base: test-kyber-library-versions
Are you sure you want to change the base?
Conversation
| func TestEncryptedDeal_Serialization(t *testing.T) { | ||
| nbV3 := 3 | ||
| nbV4 := 7 | ||
| nbVerifiers := nbV3 + nbV4 | ||
| threshold := pedersenv3.MinimumT(nbVerifiers) | ||
|
|
||
| // Generate all the public/secret keys | ||
| _, publicKeys := genPubSecKeys(nbV3, nbV4) | ||
|
|
||
| var dealer VersionlessDealer | ||
|
|
||
| // Create the dealer | ||
| dealer, _, err := genDealerV3(publicKeys, threshold) | ||
| require.NoError(t, err) | ||
|
|
||
| // 1. dispatch deal | ||
| deals, err := dealer.(*DealerV3).EncryptedDeals() | ||
|
|
||
| // Encode the deals to bytes | ||
| dealsBytes := make([][]byte, len(deals)) | ||
| for i, deal := range deals { | ||
| dealBytes, err := protobuf.Encode(deal) | ||
| require.NoError(t, err) | ||
| dealsBytes[i] = dealBytes | ||
| } | ||
|
|
||
| // Decode the deals to V3 | ||
| for i, deal := range dealsBytes { | ||
| dealV3 := &pedersenv3.EncryptedDeal{} | ||
| err := protobuf.Decode(deal, dealV3) | ||
| require.NoError(t, err) | ||
| dealOg := deals[i] | ||
| require.Equal(t, dealOg.DHKey, dealV3.DHKey) | ||
| require.Equal(t, dealOg.Nonce, dealV3.Nonce) | ||
| require.Equal(t, dealOg.Cipher, dealV3.Cipher) | ||
| require.Equal(t, dealOg.Signature, dealV3.Signature) | ||
| } | ||
|
|
||
| // Decode the deals to V4 | ||
| for i, deal := range dealsBytes { | ||
| dealV4 := &pedersenv4.EncryptedDeal{} | ||
| err := protobuf.Decode(deal, dealV4) | ||
| require.NoError(t, err) | ||
| dealOg := deals[i] | ||
| require.Equal(t, dealOg.DHKey, dealV4.DHKey) | ||
| require.Equal(t, dealOg.Cipher, dealV4.Cipher) | ||
| require.Equal(t, dealOg.Signature, dealV4.Signature) | ||
| } | ||
|
|
||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test does not pass because EncryptedDeal struct is not compatible between version 3 and 4 of Kyber:
Version 3:
type EncryptedDeal struct {
DHKey []byte
Signature []byte
Nonce []byte
Cipher []byte
}
Version 4:
type EncryptedDeal struct {
DHKey []byte
Signature []byte
Cipher []byte
}
Thus, when decoding from V3 to V4, the Nonce from V3 is interpreted as the cipher in V4. Conversly, the Cipher in V4 is interpreted as the Nonce in V3 when encoding/decoding the other way around
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One simple fix from V3 to V4 is to re-order Nonce and Cipher in the V3 struct
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But, there also is a more subtil difference which is that the PriSharestruct in V3 contains an index field int and in V4 uint32. This is a problem because the way Kyber's protobuf is designed, all int field are encoded using zigzag (i.e. PriShare field is encoded, then it's index, say 3, is encoded as
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can't really modify V3, not without breaking the API promise. How come the nonce is gone?
Worst case we can have a placeholder array that is explicitly ignored.
Regarding the int / uint32 this is a known issue that we need to address. I have a few ideas, let's talk about it either this Friday or next week.
Add cross-versions test:
encrypt_test.govss_test.go