fix(avs): prevent BLS12-381 replay attacks

precompiles/avs/IAVSManager.sol

@@ -195,14 +195,12 @@ interface IAVSManager {
     /// @param sender The external address for calling this method.
     /// @param avsAddress The address of AVS.
     /// @param pubKey the public keys of the operator
-    /// @param pubkeyRegistrationSignature the public keys of the operator
-    /// @param pubkeyRegistrationMessageHash the public keys of the operator
+    /// @param pubKeyRegistrationSignature the bls signature of the operator
     function registerBLSPublicKey(
         address sender,
         address avsAddress,
         bytes calldata pubKey,
-        bytes calldata pubkeyRegistrationSignature,
-        bytes calldata pubkeyRegistrationMessageHash
+        bytes calldata pubKeyRegistrationSignature
     ) external returns (bool success);
 
     /// @dev operatorSubmitTask ,  this function enables a operator submit a task result.

precompiles/avs/abi.json

@@ -1021,12 +1021,7 @@
 			},
 			{
 				"internalType": "bytes",
-				"name": "pubkeyRegistrationSignature",
-				"type": "bytes"
-			},
-			{
-				"internalType": "bytes",
-				"name": "pubkeyRegistrationMessageHash",
+				"name": "pubKeyRegistrationSignature",
 				"type": "bytes"
 			}
 		],

precompiles/avs/tx.go

@@ -314,13 +314,7 @@ func (p Precompile) RegisterBLSPublicKey(
 	if !ok {
 		return nil, fmt.Errorf(exocmn.ErrContractInputParamOrType, 3, "[]byte", pubKeyRegistrationSignature)
 	}
-	blsParams.PubkeyRegistrationSignature = pubKeyRegistrationSignature
-
-	pubKeyRegistrationMessageHash, ok := args[4].([]byte)
-	if !ok {
-		return nil, fmt.Errorf(exocmn.ErrContractInputParamOrType, 4, "[]byte", pubKeyRegistrationMessageHash)
-	}
-	blsParams.PubkeyRegistrationMessageHash = pubKeyRegistrationMessageHash
+	blsParams.PubKeyRegistrationSignature = pubKeyRegistrationSignature
 
 	err := p.avsKeeper.RegisterBLSPublicKey(ctx, blsParams)
 	if err != nil {

x/avs/keeper/avs_test.go

@@ -1,16 +1,20 @@
 package keeper_test
 
 import (
+	"fmt"
 	"math/big"
 	"strings"
 	"time"
 
+	testutiltx "github.com/ExocoreNetwork/exocore/testutil/tx"
+	"github.com/ethereum/go-ethereum/crypto"
+	"github.com/prysmaticlabs/prysm/v4/crypto/bls/blst"
+
 	"cosmossdk.io/math"
 	assetstypes "github.com/ExocoreNetwork/exocore/x/assets/types"
 	"github.com/ethereum/go-ethereum/common"
 
 	"github.com/ExocoreNetwork/exocore/x/avs/types"
-	avstypes "github.com/ExocoreNetwork/exocore/x/avs/types"
 	delegationtypes "github.com/ExocoreNetwork/exocore/x/delegation/types"
 	epochstypes "github.com/ExocoreNetwork/exocore/x/epochs/types"
 	operatorTypes "github.com/ExocoreNetwork/exocore/x/operator/types"
@@ -78,7 +82,7 @@ func (suite *AVSTestSuite) TestUpdateAVSInfo_Register() {
 	avsParams := &types.AVSRegisterOrDeregisterParams{
 		AvsName:               avsName,
 		AvsAddress:            common.HexToAddress(avsAddres),
-		Action:                avstypes.RegisterAction,
+		Action:                types.RegisterAction,
 		RewardContractAddress: common.HexToAddress(rewardAddress),
 		AvsOwnerAddresses:     avsOwnerAddress,
 		AssetIDs:              assetIDs,
@@ -110,7 +114,7 @@ func (suite *AVSTestSuite) TestUpdateAVSInfo_DeRegister() {
 	avsParams := &types.AVSRegisterOrDeregisterParams{
 		AvsName:              avsName,
 		AvsAddress:           common.HexToAddress(avsAddress),
-		Action:               avstypes.DeRegisterAction,
+		Action:               types.DeRegisterAction,
 		AvsOwnerAddresses:    avsOwnerAddress,
 		AssetIDs:             assetIDs,
 		MinSelfDelegation:    uint64(10),
@@ -123,10 +127,11 @@ func (suite *AVSTestSuite) TestUpdateAVSInfo_DeRegister() {
 	suite.Error(err)
 	suite.Contains(err.Error(), types.ErrUnregisterNonExistent.Error())
 
-	avsParams.Action = avstypes.RegisterAction
+	avsParams.Action = types.RegisterAction
 	err = suite.App.AVSManagerKeeper.UpdateAVSInfo(suite.Ctx, avsParams)
 	suite.NoError(err)
 	info, err := suite.App.AVSManagerKeeper.GetAVSInfo(suite.Ctx, avsAddress)
+	suite.NoError(err)
 	suite.Equal(strings.ToLower(avsAddress), info.GetInfo().AvsAddress)
 
 	epoch, _ := suite.App.EpochsKeeper.GetEpochInfo(suite.Ctx, epochstypes.DayEpochID)
@@ -138,22 +143,24 @@ func (suite *AVSTestSuite) TestUpdateAVSInfo_DeRegister() {
 		suite.Equal(epoch.CurrentEpoch, epochEnd+1)
 	}
 
-	avsParams.Action = avstypes.DeRegisterAction
+	avsParams.Action = types.DeRegisterAction
 	avsParams.CallerAddress, err = sdk.AccAddressFromBech32("exo13h6xg79g82e2g2vhjwg7j4r2z2hlncelwutkjr")
+	suite.NoError(err)
 	err = suite.App.AVSManagerKeeper.UpdateAVSInfo(suite.Ctx, avsParams)
 	suite.NoError(err)
 	info, err = suite.App.AVSManagerKeeper.GetAVSInfo(suite.Ctx, avsAddress)
 	suite.Error(err)
 	suite.Contains(err.Error(), types.ErrNoKeyInTheStore.Error())
+	suite.Nil(info)
 }
 
 func (suite *AVSTestSuite) TestUpdateAVSInfoWithOperator_Register() {
 	avsAddress := suite.avsAddress
 	operatorAddress := sdk.AccAddress(utiltx.GenerateAddress().Bytes())
 
-	operatorParams := &avstypes.OperatorOptParams{
+	operatorParams := &types.OperatorOptParams{
 		AvsAddress:      avsAddress,
-		Action:          avstypes.RegisterAction,
+		Action:          types.RegisterAction,
 		OperatorAddress: operatorAddress,
 	}
 	//  operator Not Exist
@@ -195,3 +202,201 @@ func (suite *AVSTestSuite) TestAddressSwitch() {
 	commonAddress := common.Address(accAddress)
 	suite.Equal(common.HexToAddress("0x8dF46478a83Ab2a429979391E9546A12AfF9E33f"), commonAddress)
 }
+
+func (suite *AVSTestSuite) TestRegisterBLSPublicKey() {
+	type testCase struct {
+		name          string
+		setupParams   func() *types.BlsParams
+		errorContains string
+	}
+
+	testCases := []testCase{
+		{
+			name: "successful registration",
+			setupParams: func() *types.BlsParams {
+				privateKey, err := blst.RandKey()
+				suite.NoError(err)
+				operatorAddress := sdk.AccAddress(utiltx.GenerateAddress().Bytes())
+				msg := fmt.Sprintf(types.BLSMessageToSign, types.ChainIDWithoutRevision(suite.Ctx.ChainID()), operatorAddress.String())
+				hashedMsg := crypto.Keccak256Hash([]byte(msg))
+				sig := privateKey.Sign(hashedMsg.Bytes())
+
+				return &types.BlsParams{
+					OperatorAddress:             operatorAddress,
+					AvsAddress:                  testutiltx.GenerateAddress(),
+					PubKey:                      privateKey.PublicKey().Marshal(),
+					PubKeyRegistrationSignature: sig.Marshal(),
+				}
+			},
+		},
+		{
+			name: "reuse BLS key - same operator + avs",
+			setupParams: func() *types.BlsParams {
+				privateKey, err := blst.RandKey()
+				suite.NoError(err)
+				operatorAddress := sdk.AccAddress(utiltx.GenerateAddress().Bytes())
+				msg := fmt.Sprintf(types.BLSMessageToSign, types.ChainIDWithoutRevision(suite.Ctx.ChainID()), operatorAddress.String())
+				hashedMsg := crypto.Keccak256Hash([]byte(msg))
+				sig := privateKey.Sign(hashedMsg.Bytes())
+
+				params := types.BlsParams{
+					OperatorAddress:             operatorAddress,
+					AvsAddress:                  testutiltx.GenerateAddress(),
+					PubKey:                      privateKey.PublicKey().Marshal(),
+					PubKeyRegistrationSignature: sig.Marshal(),
+				}
+				err = suite.App.AVSManagerKeeper.RegisterBLSPublicKey(suite.Ctx, &params)
+				suite.NoError(err)
+				return &params
+			},
+			errorContains: types.ErrAlreadyExists.Error() + "a key has already been set for this operator and avs",
+		},
+		{
+			name: "reuse BLS key - different operator + same avs",
+			setupParams: func() *types.BlsParams {
+				privateKey, err := blst.RandKey()
+				suite.NoError(err)
+				operatorAddress := sdk.AccAddress(utiltx.GenerateAddress().Bytes())
+				msg := fmt.Sprintf(types.BLSMessageToSign, types.ChainIDWithoutRevision(suite.Ctx.ChainID()), operatorAddress.String())
+				hashedMsg := crypto.Keccak256Hash([]byte(msg))
+				sig := privateKey.Sign(hashedMsg.Bytes())
+
+				params := types.BlsParams{
+					OperatorAddress:             operatorAddress,
+					AvsAddress:                  testutiltx.GenerateAddress(),
+					PubKey:                      privateKey.PublicKey().Marshal(),
+					PubKeyRegistrationSignature: sig.Marshal(),
+				}
+				err = suite.App.AVSManagerKeeper.RegisterBLSPublicKey(suite.Ctx, &params)
+				suite.NoError(err)
+
+				anotherOperatorAddress := sdk.AccAddress(utiltx.GenerateAddress().Bytes())
+				msg = fmt.Sprintf(types.BLSMessageToSign, types.ChainIDWithoutRevision(suite.Ctx.ChainID()), anotherOperatorAddress.String())
+				hashedMsg = crypto.Keccak256Hash([]byte(msg))
+				sig = privateKey.Sign(hashedMsg.Bytes())
+
+				return &types.BlsParams{
+					OperatorAddress:             anotherOperatorAddress,
+					AvsAddress:                  params.AvsAddress,
+					PubKey:                      privateKey.PublicKey().Marshal(),
+					PubKeyRegistrationSignature: sig.Marshal(),
+				}
+			},
+			errorContains: types.ErrAlreadyExists.Error() + "this BLS key is already in use",
+		},
+		{
+			name: "wrong chain ID",
+			setupParams: func() *types.BlsParams {
+				privateKey, err := blst.RandKey()
+				suite.NoError(err)
+				operatorAddress := sdk.AccAddress(utiltx.GenerateAddress().Bytes())
+				msg := fmt.Sprintf(types.BLSMessageToSign, "onemorechain_211", operatorAddress.String())
+				hashedMsg := crypto.Keccak256Hash([]byte(msg))
+				sig := privateKey.Sign(hashedMsg.Bytes())
+
+				return &types.BlsParams{
+					OperatorAddress:             operatorAddress,
+					AvsAddress:                  testutiltx.GenerateAddress(),
+					PubKey:                      privateKey.PublicKey().Marshal(),
+					PubKeyRegistrationSignature: sig.Marshal(),
+				}
+			},
+			errorContains: types.ErrSigNotMatchPubKey.Error(),
+		},
+		{
+			name: "wrong operator address in signature",
+			setupParams: func() *types.BlsParams {
+				privateKey, err := blst.RandKey()
+				suite.NoError(err)
+				operatorAddress := sdk.AccAddress(utiltx.GenerateAddress().Bytes())
+				anotherOperatorAddress := sdk.AccAddress(utiltx.GenerateAddress().Bytes())
+				msg := fmt.Sprintf(types.BLSMessageToSign, types.ChainIDWithoutRevision(suite.Ctx.ChainID()), anotherOperatorAddress.String())
+				hashedMsg := crypto.Keccak256Hash([]byte(msg))
+				sig := privateKey.Sign(hashedMsg.Bytes())
+
+				return &types.BlsParams{
+					OperatorAddress:             operatorAddress,
+					AvsAddress:                  testutiltx.GenerateAddress(),
+					PubKey:                      privateKey.PublicKey().Marshal(),
+					PubKeyRegistrationSignature: sig.Marshal(),
+				}
+			},
+			errorContains: types.ErrSigNotMatchPubKey.Error(),
+		},
+		{
+			name: "mismatched BLS key",
+			setupParams: func() *types.BlsParams {
+				privateKey, err := blst.RandKey()
+				suite.NoError(err)
+				anotherPrivateKey, err := blst.RandKey()
+				suite.NoError(err)
+				operatorAddress := sdk.AccAddress(utiltx.GenerateAddress().Bytes())
+				msg := fmt.Sprintf(types.BLSMessageToSign, types.ChainIDWithoutRevision(suite.Ctx.ChainID()), operatorAddress.String())
+				hashedMsg := crypto.Keccak256Hash([]byte(msg))
+				sig := privateKey.Sign(hashedMsg.Bytes())
+
+				return &types.BlsParams{
+					OperatorAddress:             operatorAddress,
+					AvsAddress:                  testutiltx.GenerateAddress(),
+					PubKey:                      anotherPrivateKey.PublicKey().Marshal(),
+					PubKeyRegistrationSignature: sig.Marshal(),
+				}
+			},
+			errorContains: types.ErrSigNotMatchPubKey.Error(),
+		},
+		{
+			name: "invalid public key format",
+			setupParams: func() *types.BlsParams {
+				operatorAddress := sdk.AccAddress(utiltx.GenerateAddress().Bytes())
+				return &types.BlsParams{
+					OperatorAddress: operatorAddress,
+					AvsAddress:      testutiltx.GenerateAddress(),
+					PubKey:          []byte("invalid"),
+				}
+			},
+			errorContains: types.ErrSigNotMatchPubKey.Error(),
+		},
+		{
+			name: "invalid signature format",
+			setupParams: func() *types.BlsParams {
+				privateKey, err := blst.RandKey()
+				suite.NoError(err)
+				operatorAddress := sdk.AccAddress(utiltx.GenerateAddress().Bytes())
+				return &types.BlsParams{
+					OperatorAddress:             operatorAddress,
+					AvsAddress:                  testutiltx.GenerateAddress(),
+					PubKey:                      privateKey.PublicKey().Marshal(),
+					PubKeyRegistrationSignature: []byte("invalid"),
+				}
+			},
+			errorContains: types.ErrSigNotMatchPubKey.Error(),
+		},
+		{
+			name: "empty operator address",
+			setupParams: func() *types.BlsParams {
+				privateKey, err := blst.RandKey()
+				suite.NoError(err)
+				return &types.BlsParams{
+					AvsAddress:                  testutiltx.GenerateAddress(),
+					PubKey:                      privateKey.PublicKey().Marshal(),
+					PubKeyRegistrationSignature: []byte{},
+				}
+			},
+			errorContains: types.ErrSigNotMatchPubKey.Error(),
+		},
+	}
+
+	for _, tc := range testCases {
+		suite.Run(tc.name, func() {
+			params := tc.setupParams()
+			err := suite.App.AVSManagerKeeper.RegisterBLSPublicKey(suite.Ctx, params)
+
+			if tc.errorContains != "" {
+				suite.Error(err)
+				suite.Contains(err.Error(), tc.errorContains)
+			} else {
+				suite.NoError(err)
+			}
+		})
+	}
+}

x/avs/keeper/keeper.go

@@ -288,28 +288,43 @@ func (k Keeper) CreateAVSTask(ctx sdk.Context, params *types.TaskInfoParams) (ui
 
 func (k Keeper) RegisterBLSPublicKey(ctx sdk.Context, params *types.BlsParams) error {
 	// check bls signature to prevent rogue key attacks
-	sig := params.PubkeyRegistrationSignature
-	msgHash := params.PubkeyRegistrationMessageHash
+	// use a templated message to
+	// (1) validate that this signature is intended solely for RegisterBLSPublicKey
+	// (2) prevent replay attacks by including the chain-id and operator-address
+	// note that the operator address is bech32 encoded and thus already lowercase
+	msg := fmt.Sprintf(types.BLSMessageToSign, types.ChainIDWithoutRevision(ctx.ChainID()), params.OperatorAddress.String())
+	hashedMsg := crypto.Keccak256Hash([]byte(msg))
+
+	sig := params.PubKeyRegistrationSignature
 	pubKey, _ := bls.PublicKeyFromBytes(params.PubKey)
-	valid, err := blst.VerifySignature(sig, [32]byte(msgHash), pubKey)
+	valid, err := blst.VerifySignature(sig, hashedMsg, pubKey)
 	if err != nil || !valid {
-		return errorsmod.Wrap(types.ErrSigNotMatchPubKey, fmt.Sprintf("the operator is :%s", params.OperatorAddress))
+		return errorsmod.Wrap(types.ErrSigNotMatchPubKey, fmt.Sprintf("the operator is %s", params.OperatorAddress))
 	}
+	// check that a key has not already been set for this operator and avs
 	if k.IsExistPubKeyForAVS(ctx, params.OperatorAddress.String(), params.AvsAddress.String()) {
-		return errorsmod.Wrap(types.ErrAlreadyExists, fmt.Sprintf("the operator is :%s", params.OperatorAddress))
+		return errorsmod.Wrap(
+			types.ErrAlreadyExists,
+			"a key has already been set for this operator and avs",
+		)
 	}
-	bls := &types.BlsPubKeyInfo{
+	blsInfo := &types.BlsPubKeyInfo{
 		AvsAddress:      strings.ToLower(params.AvsAddress.String()),
-		OperatorAddress: strings.ToLower(params.OperatorAddress.String()),
+		OperatorAddress: params.OperatorAddress.String(),
 		PubKey:          params.PubKey,
 	}
-	// check a bls key can only be used once.
-	// if operator are using multiple servers for different AVSs .
-	// In case one server is compromised, signing can continue as expected on the AVSs for which there has been no compromise.
-	if k.IsExistPubKey(ctx, bls) {
-		return errorsmod.Wrap(types.ErrAlreadyExists, fmt.Sprintf("the bls key is already exists:%s", bls.PubKey))
+	// verify that the BLS key is not already in use by
+	// (1) even the same operator on a different AVS
+	// (2) different operators on the same AVS
+	// this design decision is made to ensure that the same BLS key
+	// is not reused across different AVSs by the same operator
+	if k.IsExistPubKey(ctx, blsInfo) {
+		return errorsmod.Wrap(
+			types.ErrAlreadyExists,
+			"this BLS key is already in use",
+		)
 	}
-	return k.SetOperatorPubKey(ctx, bls)
+	return k.SetOperatorPubKey(ctx, blsInfo)
 }
 
 func (k Keeper) OperatorOptAction(ctx sdk.Context, params *types.OperatorOptParams) error {
@@ -411,7 +426,7 @@ func (k Keeper) GetAVSEpochInfo(ctx sdk.Context, addr string) (*epochstypes.Epoc
 	}
 	avsInfo := avsInfoResp.Info
 	// Epoch information must be available because it is checked when setting AVS information.
-	// Therefore, we don’t need to check it here.
+	// Therefore, we don't need to check it here.
 	epochInfo, _ := k.epochsKeeper.GetEpochInfo(ctx, avsInfo.EpochIdentifier)
 	return &epochInfo, nil
 }