Complete fix of 5.4 Reshare Denial-of-Service via Predicable Instance IDs

pkgs/initiator/initiator.go

@@ -19,14 +19,14 @@ import (
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/herumi/bls-eth-go-binary/bls"
 	"github.com/imroc/req/v3"
+	"go.uber.org/zap"
 
 	spec "github.com/ssvlabs/dkg-spec"
 	spec_crypto "github.com/ssvlabs/dkg-spec/crypto"
 	"github.com/ssvlabs/ssv-dkg/pkgs/consts"
 	"github.com/ssvlabs/ssv-dkg/pkgs/crypto"
 	"github.com/ssvlabs/ssv-dkg/pkgs/utils"
 	"github.com/ssvlabs/ssv-dkg/pkgs/wire"
-	"go.uber.org/zap"
 )
 
 type VerifyMessageSignatureFunc func(pub *rsa.PublicKey, msg, sig []byte) error
@@ -165,7 +165,7 @@ func ValidatedOperatorData(ids []uint64, operators wire.OperatorsCLI) ([]*spec.O
 }
 
 // messageFlowHandling main steps of DKG at initiator
-func (c *Initiator) initMessageFlowHandling(init *spec.Init, id [24]byte, operators []*spec.Operator) ([][]byte, error) {
+func (c *Initiator) initMessageFlowHandling(init *spec.Init, id, instanceID [24]byte, operators []*spec.Operator) ([][]byte, error) {
 	c.Logger.Info("phase 1: sending init message to operators")
 	exchangeMsgs, errs, err := c.SendInitMsg(id, init, operators)
 	if err != nil {
@@ -175,33 +175,33 @@ func (c *Initiator) initMessageFlowHandling(init *spec.Init, id [24]byte, operat
 	if err := checkThreshold(exchangeMsgs, errs, operators, operators, len(operators)); err != nil {
 		return nil, err
 	}
-	err = verifyMessageSignatures(id, exchangeMsgs, c.VerifyMessageSignature)
+	err = verifyMessageSignatures(instanceID, exchangeMsgs, c.VerifyMessageSignature)
 	if err != nil {
 		return nil, err
 	}
 	c.Logger.Info("phase 1: ✅ verified operator init responses signatures")
 	c.Logger.Info("phase 2: ➡️ sending operator data (exchange messages) required for dkg")
-	kyberMsgs, errs, err := c.SendExchangeMsgs(id, exchangeMsgs, operators)
+	kyberMsgs, errs, err := c.SendExchangeMsgs(instanceID, exchangeMsgs, operators)
 	if err != nil {
 		return nil, err
 	}
 	if err := checkThreshold(kyberMsgs, errs, operators, operators, len(operators)); err != nil {
 		return nil, err
 	}
-	err = verifyMessageSignatures(id, kyberMsgs, c.VerifyMessageSignature)
+	err = verifyMessageSignatures(instanceID, kyberMsgs, c.VerifyMessageSignature)
 	if err != nil {
 		return nil, err
 	}
 	c.Logger.Info("phase 2: ✅ verified operator responses (deal messages) signatures")
 	c.Logger.Info("phase 3: ➡️ sending deal dkg data to all operators")
-	dkgResult, errs, err := c.SendKyberMsgs(id, kyberMsgs, operators)
+	dkgResult, errs, err := c.SendKyberMsgs(instanceID, kyberMsgs, operators)
 	if err != nil {
 		return nil, err
 	}
 	if err := checkThreshold(dkgResult, errs, operators, operators, len(operators)); err != nil {
 		return nil, err
 	}
-	err = verifyMessageSignatures(id, dkgResult, c.VerifyMessageSignature)
+	err = verifyMessageSignatures(instanceID, dkgResult, c.VerifyMessageSignature)
 	if err != nil {
 		return nil, err
 	}
@@ -215,9 +215,13 @@ func (c *Initiator) initMessageFlowHandling(init *spec.Init, id [24]byte, operat
 
 func (c *Initiator) ResignMessageFlowHandling(signedResign *wire.SignedResign, id [24]byte, operators []*spec.Operator) ([][][]byte, error) {
 	// reqIDtracker is used to track if all ceremony are in the responses in the expected order
+	pub, err := spec_crypto.EncodeRSAPublicKey(&c.PrivateKey.PublicKey)
+	if err != nil {
+		return nil, err
+	}
 	reqIDs := make([][24]byte, 0)
 	for _, msg := range signedResign.Messages {
-		msgID, err := utils.GetReqIDfromMsg(msg, id)
+		msgID, err := utils.GetInstanceIDfromMsg(msg, id, pub)
 		if err != nil {
 			return nil, err
 		}
@@ -269,10 +273,14 @@ func (c *Initiator) ReshareMessageFlowHandling(id [24]byte, signedReshare *wire.
 	if err != nil {
 		return nil, err
 	}
+	pub, err := spec_crypto.EncodeRSAPublicKey(&c.PrivateKey.PublicKey)
+	if err != nil {
+		return nil, err
+	}
 	// reqIDtracker is used to track if all ceremony are in the responses in the expected order
 	reqIDs := make([][24]byte, 0)
 	for _, msg := range signedReshare.Messages {
-		msgID, err := utils.GetReqIDfromMsg(msg, id)
+		msgID, err := utils.GetInstanceIDfromMsg(msg, id, pub)
 		if err != nil {
 			return nil, err
 		}
@@ -383,20 +391,32 @@ func (c *Initiator) StartDKG(id [24]byte, withdraw []byte, ids []uint64, network
 		zap.Uint64("nonce", init.Nonce),
 		zap.Any("operator IDs", ids))
 	c.Logger = c.Logger.With(instanceIDField)
-	dkgResultsBytes, err := c.initMessageFlowHandling(init, id, ops)
+	pub, err := spec_crypto.EncodeRSAPublicKey(&c.PrivateKey.PublicKey)
 	if err != nil {
 		return nil, nil, nil, err
 	}
-	return c.CreateCeremonyResults(dkgResultsBytes, id, init.Operators, init.WithdrawalCredentials, nil, init.Fork, init.Owner, init.Nonce, phase0.Gwei(init.Amount))
+	instanceID, err := utils.GetInstanceIDfromMsg(init, id, pub)
+	if err != nil {
+		return nil, nil, nil, err
+	}
+	dkgResultsBytes, err := c.initMessageFlowHandling(init, id, instanceID, ops)
+	if err != nil {
+		return nil, nil, nil, err
+	}
+	return c.CreateCeremonyResults(dkgResultsBytes, instanceID, init.Operators, init.WithdrawalCredentials, nil, init.Fork, init.Owner, init.Nonce, phase0.Gwei(init.Amount))
 }
 
 func (c *Initiator) StartResigning(id [24]byte, signedResign *wire.SignedResign) ([]*wire.DepositDataCLI, []*wire.KeySharesCLI, [][]*wire.SignedProof, error) {
 	if len(signedResign.Messages) == 0 {
 		return nil, nil, nil, errors.New("no resign messages")
 	}
+	pub, err := spec_crypto.EncodeRSAPublicKey(&c.PrivateKey.PublicKey)
+	if err != nil {
+		return nil, nil, nil, err
+	}
 	resignIDMap := make(map[[24]byte]*spec.Resign)
 	for _, msg := range signedResign.Messages {
-		msgID, err := utils.GetReqIDfromMsg(msg, id)
+		msgID, err := utils.GetInstanceIDfromMsg(msg, id, pub)
 		if err != nil {
 			return nil, nil, nil, err
 		}
@@ -511,9 +531,13 @@ func (c *Initiator) StartResharing(id [24]byte, signedReshare *wire.SignedReshar
 	if len(signedReshare.Messages) == 0 {
 		return nil, nil, nil, errors.New("no reshare messages")
 	}
+	pub, err := spec_crypto.EncodeRSAPublicKey(&c.PrivateKey.PublicKey)
+	if err != nil {
+		return nil, nil, nil, err
+	}
 	reshareIDMap := make(map[[24]byte]*spec.Reshare)
 	for _, msg := range signedReshare.Messages {
-		msgID, err := utils.GetReqIDfromMsg(msg, id)
+		msgID, err := utils.GetInstanceIDfromMsg(msg, id, pub)
 		if err != nil {
 			return nil, nil, nil, err
 		}

pkgs/initiator/initiator_test.go

@@ -9,6 +9,7 @@ import (
 	"testing"
 
 	"github.com/attestantio/go-eth2-client/spec/phase0"
+	e2m_core "github.com/bloxapp/eth2-key-manager/core"
 	"github.com/bloxapp/ssv/logging"
 	kyber_bls12381 "github.com/drand/kyber-bls12381"
 	kyber_dkg "github.com/drand/kyber/share/dkg"
@@ -18,7 +19,6 @@ import (
 	"github.com/stretchr/testify/require"
 	"go.uber.org/zap"
 
-	e2m_core "github.com/bloxapp/eth2-key-manager/core"
 	spec "github.com/ssvlabs/dkg-spec"
 	spec_crypto "github.com/ssvlabs/dkg-spec/crypto"
 	"github.com/ssvlabs/dkg-spec/testing/stubs"
@@ -468,7 +468,11 @@ func TestDKGFailWithOperatorsMisbehave(t *testing.T) {
 
 		exchangeMsgs, _, err := intr.SendInitMsg(id, init, ops)
 		require.NoError(t, err)
-		kyberMsgs, _, err := intr.SendExchangeMsgs(id, exchangeMsgs, ops)
+		pub, err := spec_crypto.EncodeRSAPublicKey(&intr.PrivateKey.PublicKey)
+		require.NoError(t, err)
+		instanceID, err := utils.GetInstanceIDfromMsg(init, id, pub)
+		require.NoError(t, err)
+		kyberMsgs, _, err := intr.SendExchangeMsgs(instanceID, exchangeMsgs, ops)
 		require.NoError(t, err)
 
 		tsp := &wire.SignedTransport{}
@@ -508,7 +512,7 @@ func TestDKGFailWithOperatorsMisbehave(t *testing.T) {
 
 		trsp := &wire.Transport{
 			Type:       wire.KyberMessageType,
-			Identifier: id,
+			Identifier: instanceID,
 			Data:       byts,
 			Version:    intr.Version,
 		}
@@ -519,19 +523,19 @@ func TestDKGFailWithOperatorsMisbehave(t *testing.T) {
 		sign, err := srv1.Srv.State.Sign(bts)
 		require.NoError(t, err)
 
-		pub, err := spec_crypto.EncodeRSAPublicKey(&srv1.Srv.State.PrivateKey.PublicKey)
+		pubOp, err := spec_crypto.EncodeRSAPublicKey(&srv1.Srv.State.PrivateKey.PublicKey)
 		require.NoError(t, err)
 
 		signed := &wire.SignedTransport{
 			Message:   trsp,
-			Signer:    pub,
+			Signer:    pubOp,
 			Signature: sign,
 		}
 		final, err := signed.MarshalSSZ()
 		kyberMsgs[srv1.ID] = final
 		require.NoError(t, err)
 
-		dkgResult, errs, err := intr.SendKyberMsgs(id, kyberMsgs, ops)
+		dkgResult, errs, err := intr.SendKyberMsgs(instanceID, kyberMsgs, ops)
 		require.NoError(t, err)
 
 		for _, err := range errs {
@@ -542,7 +546,7 @@ func TestDKGFailWithOperatorsMisbehave(t *testing.T) {
 			finalResults = append(finalResults, res)
 		}
 
-		_, _, _, err = intr.CreateCeremonyResults(finalResults, id, init.Operators, init.WithdrawalCredentials, nil, init.Fork, init.Owner, init.Nonce, phase0.Gwei(init.Amount))
+		_, _, _, err = intr.CreateCeremonyResults(finalResults, instanceID, init.Operators, init.WithdrawalCredentials, nil, init.Fork, init.Owner, init.Nonce, phase0.Gwei(init.Amount))
 		require.ErrorContains(t, err, "protocol failed with response complaints")
 	})
 

pkgs/operator/instances_manager.go

@@ -107,16 +107,20 @@ func (s *Switch) InitInstance(reqID [24]byte, initMsg *wire.Transport, initiator
 		return nil, fmt.Errorf("init: initiator signature isn't valid: %w", err)
 	}
 	s.Logger.Info("✅ init message signature is successfully verified", zap.String("from initiator", fmt.Sprintf("%x", initiatorPubKey.N.Bytes())))
-	if err := s.validateInstances(reqID); err != nil {
+	instanceID, err := utils.GetInstanceIDfromMsg(init, reqID, initiatorPub)
+	if err != nil {
 		return nil, err
 	}
-	inst, resp, err := s.CreateInstance(reqID, init.Operators, init, initiatorPubKey)
+	if err := s.validateInstances(instanceID); err != nil {
+		return nil, err
+	}
+	inst, resp, err := s.CreateInstance(instanceID, init.Operators, init, initiatorPubKey)
 	if err != nil {
 		return nil, fmt.Errorf("init: failed to create instance: %w", err)
 	}
 	s.Mtx.Lock()
-	s.Instances[reqID] = inst
-	s.InstanceInitTime[reqID] = time.Now()
+	s.Instances[instanceID] = inst
+	s.InstanceInitTime[instanceID] = time.Now()
 	s.Mtx.Unlock()
 	return resp, nil
 }
@@ -286,7 +290,11 @@ func (s *Switch) validateInstances(reqID InstanceID) error {
 }
 
 func (s *Switch) runInstance(reqID [24]byte, instance interface{}, allOps []*spec.Operator, initiatorPubKey *rsa.PublicKey, operationType string) ([]byte, error) {
-	instanceID, err := utils.GetReqIDfromMsg(instance, reqID)
+	pub, err := spec_crypto.EncodeRSAPublicKey(initiatorPubKey)
+	if err != nil {
+		return nil, err
+	}
+	instanceID, err := utils.GetInstanceIDfromMsg(instance, reqID, pub)
 	if err != nil {
 		return nil, err
 	}

pkgs/operator/state_test.go

@@ -240,13 +240,15 @@ func TestSwitch_cleanInstances(t *testing.T) {
 	require.NoError(t, err)
 	sig, err := spec_crypto.SignRSA(priv, tsssz)
 	require.NoError(t, err)
+	instanceID, err := utils.GetInstanceIDfromMsg(init, reqID, encPubKey)
+	require.NoError(t, err)
 	resp, err := swtch.InitInstance(reqID, initMessage, encPubKey, sig)
 	require.NoError(t, err)
 	require.NotNil(t, resp)
 	require.Equal(t, swtch.cleanInstances(), 0)
 
 	require.Len(t, swtch.Instances, 1)
-	swtch.InstanceInitTime[reqID] = time.Now().Add(-time.Minute * 6)
+	swtch.InstanceInitTime[instanceID] = time.Now().Add(-time.Minute * 6)
 
 	require.Equal(t, swtch.cleanInstances(), 1)
 	require.Len(t, swtch.Instances, 0)

pkgs/utils/utils.go

@@ -264,15 +264,16 @@ func GetMessageHash(msg interface{}) ([32]byte, error) {
 	return hash, nil
 }
 
-func GetReqIDfromMsg(instance interface{}, id [24]byte) ([24]byte, error) {
+func GetInstanceIDfromMsg(instance interface{}, id [24]byte, initiatorPub []byte) ([24]byte, error) {
 	// make a unique ID for each reshare using the instance hash
 	reqID := [24]byte{}
 	instanceHash, err := GetMessageHash(instance)
 	if err != nil {
 		return reqID, fmt.Errorf("failed to get reqID: %w", err)
 	}
-	copy(reqID[:12], id[:12])
-	copy(reqID[12:24], instanceHash[:12])
+	copy(reqID[:8], eth_crypto.Keccak256(initiatorPub)[:8])
+	copy(reqID[8:16], instanceHash[:8])
+	copy(reqID[16:24], id[:8])
 	return reqID, nil
 }