From 6849f60895efa8206ba8517cc7a5daf782036dc2 Mon Sep 17 00:00:00 2001
From: Vibhu Mohindra <vibs29@hotmail.com>
Date: Tue, 5 Jul 2022 01:11:32 +0000
Subject: [PATCH] security fix: any participant could disable DKG

A deal with a bogus signature would still get stored (before producing
an error). That would prevent a properly signed deal coming later from
being stored. This fixes the bug.

The test was also written in a flawed way. It measured two failure
situations ("wrong index" and "wrong deal") after a success situation. But
written that far down, those measurements could well end up measuring
failures due to a duplicate deal instead. This changeset also patches
the test flaw.
---
 share/dkg/rabin/dkg.go      |  4 ++--
 share/dkg/rabin/dkg_test.go | 28 ++++++++++++++--------------
 2 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/share/dkg/rabin/dkg.go b/share/dkg/rabin/dkg.go
index 376d1e1a4..969044b1e 100644
--- a/share/dkg/rabin/dkg.go
+++ b/share/dkg/rabin/dkg.go
@@ -282,7 +282,6 @@ func (d *DistKeyGenerator) ProcessDeal(dd *Deal) (*Response, error) {
 		return nil, err
 	}
 
-	d.verifiers[dd.Index] = ver
 	resp, err := ver.ProcessEncryptedDeal(dd.Deal)
 	if err != nil {
 		return nil, err
@@ -290,8 +289,9 @@ func (d *DistKeyGenerator) ProcessDeal(dd *Deal) (*Response, error) {
 
 	// Set StatusApproval for the verifier that represents the participant
 	// that distibuted the Deal
-	d.verifiers[dd.Index].UnsafeSetResponseDKG(dd.Index, true)
+	ver.UnsafeSetResponseDKG(dd.Index, true)
 
+	d.verifiers[dd.Index] = ver
 	return &Response{
 		Index:    dd.Index,
 		Response: resp,
diff --git a/share/dkg/rabin/dkg_test.go b/share/dkg/rabin/dkg_test.go
index a90e0a03a..2d4a29b9c 100644
--- a/share/dkg/rabin/dkg_test.go
+++ b/share/dkg/rabin/dkg_test.go
@@ -89,20 +89,6 @@ func TestDKGProcessDeal(t *testing.T) {
 	assert.Error(t, err)
 	rec.participants = goodP
 
-	// good deal
-	resp, err = rec.ProcessDeal(deal)
-	assert.NotNil(t, resp)
-	assert.Equal(t, true, resp.Response.Approved)
-	assert.Nil(t, err)
-	_, ok := rec.verifiers[deal.Index]
-	require.True(t, ok)
-	assert.Equal(t, uint32(0), resp.Index)
-
-	// duplicate
-	resp, err = rec.ProcessDeal(deal)
-	assert.Nil(t, resp)
-	assert.Error(t, err)
-
 	// wrong index
 	goodIdx := deal.Index
 	deal.Index = uint32(nbParticipants + 1)
@@ -119,6 +105,20 @@ func TestDKGProcessDeal(t *testing.T) {
 	assert.Error(t, err)
 	deal.Deal.Signature = goodSig
 
+	// good deal
+	resp, err = rec.ProcessDeal(deal)
+	assert.NotNil(t, resp)
+	assert.Equal(t, true, resp.Response.Approved)
+	assert.Nil(t, err)
+	_, ok := rec.verifiers[deal.Index]
+	require.True(t, ok)
+	assert.Equal(t, uint32(0), resp.Index)
+
+	// duplicate
+	resp, err = rec.ProcessDeal(deal)
+	assert.Nil(t, resp)
+	assert.Error(t, err)
+
 }
 
 func TestDKGProcessResponse(t *testing.T) {