fix(crypto): bind g1 identity to g2 in bls verification

internal/client/repository/codec/codec_test.go

@@ -0,0 +1,84 @@
+package codec
+
+import (
+	"context"
+	"math/big"
+	"testing"
+
+	"github.com/ethereum/go-ethereum/common"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	symbiotic "github.com/symbioticfi/relay/symbiotic/entity"
+)
+
+func TestValidatorSetHeaderRoundTrip_PreservesCommitterOrder(t *testing.T) {
+	t.Parallel()
+
+	const slotDuration = uint64(100)
+	keyTag := symbiotic.KeyTag(15)
+
+	publicKey1 := []byte("committer1_key")
+	publicKey2 := []byte("committer2_key")
+	publicKey3 := []byte("committer3_key")
+
+	valset := symbiotic.ValidatorSet{
+		RequiredKeyTag:   keyTag,
+		CaptureTimestamp: 1000,
+		Validators: symbiotic.Validators{
+			{
+				Operator:    common.HexToAddress("0x1111111111111111111111111111111111111111"),
+				VotingPower: symbiotic.ToVotingPower(big.NewInt(100)),
+				IsActive:    true,
+				Keys: []symbiotic.ValidatorKey{
+					{Tag: keyTag, Payload: publicKey1},
+				},
+			},
+			{
+				Operator:    common.HexToAddress("0x2222222222222222222222222222222222222222"),
+				VotingPower: symbiotic.ToVotingPower(big.NewInt(200)),
+				IsActive:    true,
+				Keys: []symbiotic.ValidatorKey{
+					{Tag: keyTag, Payload: publicKey2},
+				},
+			},
+			{
+				Operator:    common.HexToAddress("0x3333333333333333333333333333333333333333"),
+				VotingPower: symbiotic.ToVotingPower(big.NewInt(150)),
+				IsActive:    true,
+				Keys: []symbiotic.ValidatorKey{
+					{Tag: keyTag, Payload: publicKey3},
+				},
+			},
+		},
+		CommitterIndices: []uint32{2, 0, 1},
+	}
+
+	require.True(
+		t,
+		valset.IsActiveCommitter(context.Background(), slotDuration, 1050, 0, publicKey3),
+		"sanity check: validator 2 should own the first slot before storage round-trip",
+	)
+
+	headerBytes, err := ValidatorSetHeaderToBytes(valset)
+	require.NoError(t, err)
+
+	_, committerIndices, err := ExtractAdditionalInfoFromHeaderData(headerBytes)
+	require.NoError(t, err)
+
+	assert.Equal(
+		t,
+		valset.CommitterIndices,
+		committerIndices,
+		"header round-trip should preserve committer order because slot rotation depends on it",
+	)
+
+	roundTripped := valset
+	roundTripped.CommitterIndices = committerIndices
+
+	assert.True(
+		t,
+		roundTripped.IsActiveCommitter(context.Background(), slotDuration, 1050, 0, publicKey3),
+		"the same validator should still own the first slot after header round-trip",
+	)
+}

internal/usecase/entity-processor/entity_processor_key_binding_test.go

@@ -0,0 +1,85 @@
+package entity_processor
+
+import (
+	"math/big"
+	"testing"
+
+	"github.com/consensys/gnark-crypto/ecc/bn254"
+	"github.com/stretchr/testify/require"
+
+	symbiotic "github.com/symbioticfi/relay/symbiotic/entity"
+	"github.com/symbioticfi/relay/symbiotic/usecase/crypto"
+	"github.com/symbioticfi/relay/symbiotic/usecase/crypto/blsBn254"
+)
+
+// Validators are looked up by the on-chain G1 key, so a gossiped signature that
+// keeps a victim's G1 but carries an attacker's G2 must not verify. Otherwise
+// any peer could sign under any validator's identity, and the per-index dedup
+// would then censor that validator's genuine signature.
+func TestEntityProcessor_ProcessSignature_RejectsUnboundG2(t *testing.T) {
+	t.Parallel()
+
+	for name, newRepo := range backends() {
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
+
+			repo := newRepo(t)
+			epoch := symbiotic.Epoch(700)
+			req := randomSignatureRequest(t, epoch)
+
+			_, privateKeys := setupValidatorSetHeader(t, repo, epoch, big.NewInt(1000))
+
+			processor, err := NewEntityProcessor(Config{
+				Repo:                     repo,
+				Aggregator:               createMockAggregator(t),
+				AggProofSignal:           createMockAggProofSignal(t),
+				SignatureProcessedSignal: createMockSignatureProcessedSignal(t),
+				Metrics:                  doNothingMetrics{},
+			})
+			require.NoError(t, err)
+
+			// The attacker knows only the victim's public G1 identity.
+			victimPub := privateKeys[0][req.KeyTag].PublicKey()
+			victimG1Compressed := victimPub.Raw()[:32]
+
+			messageHash, err := crypto.HashMessage(req.KeyTag.Type(), req.Message)
+			require.NoError(t, err)
+
+			// Sign with an arbitrary scalar and publish the matching G2.
+			attackerScalar := big.NewInt(0xDEADBEEF)
+			_, _, _, g2Gen := bn254.Generators()
+			var attackerG2 bn254.G2Affine
+			attackerG2.ScalarMultiplication(&g2Gen, attackerScalar)
+
+			g1Hash, err := blsBn254.HashToG1(messageHash)
+			require.NoError(t, err)
+			var sig bn254.G1Affine
+			sig.ScalarMultiplication(g1Hash, attackerScalar)
+
+			attackerG2Compressed := attackerG2.Bytes()
+			forgedRaw := append(append([]byte{}, victimG1Compressed...), attackerG2Compressed[:]...)
+			forgedPub, err := crypto.NewPublicKey(symbiotic.KeyTypeBlsBn254, forgedRaw)
+			require.NoError(t, err)
+			require.Equal(t, victimPub.OnChain(), forgedPub.OnChain(),
+				"forgery resolves to the victim's on-chain identity")
+
+			forged := symbiotic.Signature{
+				KeyTag:      req.KeyTag,
+				Epoch:       epoch,
+				MessageHash: messageHash,
+				Signature:   sig.Marshal(),
+				PublicKey:   forgedPub,
+			}
+
+			require.Error(t, processor.ProcessSignature(t.Context(), forged, false),
+				"forged signature must not be accepted under the victim's identity")
+
+			_, err = repo.GetSignatureMap(t.Context(), forged.RequestID())
+			require.Error(t, err, "nothing recorded for the victim")
+
+			// The victim's genuine signature is still accepted afterwards.
+			genuine := signatureExtendedForRequest(t, privateKeys[0][req.KeyTag], req)
+			require.NoError(t, processor.ProcessSignature(t.Context(), genuine, false))
+		})
+	}
+}

symbiotic/usecase/crypto/bls12381/key.go

@@ -113,12 +113,28 @@ func (k *PublicKey) VerifyWithHash(msgHash MessageHash, sig Signature) error {
 		return errors.Errorf("bls12381: failed to set big into G1: %w", err)
 	}
 
-	_, _, _, g2Gen := bls12381.Generators()
-	var negSig bls12381.G1Affine
-	negSig.Neg(&g1Sig)
+	// A zero key would make the keypair binding below vacuous.
+	if k.g1PubKey.IsInfinity() {
+		return errors.Errorf("bls12381: zero public key")
+	}
+
+	_, _, g1Gen, g2Gen := bls12381.Generators()
+	var negG2Gen bls12381.G2Affine
+	negG2Gen.Neg(&g2Gen)
+
+	// Folds two equations under a Fiat-Shamir challenge: the signature check
+	// e(H(m), G2) == e(sig, g2Gen), and the keypair binding
+	// e(G1, g2Gen) == e(g1Gen, G2). Without the binding, a G2 taken from an
+	// untrusted message is never tied to the G1 a validator is identified by,
+	// so anyone could sign under someone else's identity.
+	alpha := keypairChallenge(&g1Sig, &k.g1PubKey, &k.g2PubKey, g1Hash)
 
-	g1P := [2]bls12381.G1Affine{*g1Hash, negSig}
-	g1Q := [2]bls12381.G2Affine{k.g2PubKey, g2Gen}
+	var sigTerm, msgTerm bls12381.G1Affine
+	sigTerm.ScalarMultiplication(&k.g1PubKey, alpha).Add(&sigTerm, &g1Sig)
+	msgTerm.ScalarMultiplication(&g1Gen, alpha).Add(&msgTerm, g1Hash)
+
+	g1P := [2]bls12381.G1Affine{sigTerm, msgTerm}
+	g1Q := [2]bls12381.G2Affine{negG2Gen, k.g2PubKey}
 
 	ok, err := bls12381.PairingCheck(g1P[:], g1Q[:])
 	if err != nil {
@@ -130,6 +146,18 @@ func (k *PublicKey) VerifyWithHash(msgHash MessageHash, sig Signature) error {
 	return nil
 }
 
+// keypairChallenge commits to every point so the two folded equations cannot be
+// made to cancel each other out.
+func keypairChallenge(sig, g1PubKey *bls12381.G1Affine, g2PubKey *bls12381.G2Affine, msg *bls12381.G1Affine) *big.Int {
+	sigBytes := sig.Marshal()
+	g1Bytes := g1PubKey.Marshal()
+	g2Bytes := g2PubKey.Marshal()
+	msgBytes := msg.Marshal()
+
+	h := crypto.Keccak256(sigBytes, g1Bytes, g2Bytes, msgBytes)
+	return new(big.Int).Mod(new(big.Int).SetBytes(h), fr.Modulus())
+}
+
 // OnChain might be one way operation, meaning that it's impossible to reconstruct PublicKey from compact
 func (k *PublicKey) OnChain() CompactPublicKey {
 	// DEV: g1PubKey Marshalled is 96 bytes in total, x and y each 48bytes

symbiotic/usecase/crypto/bls12381/key_test.go

@@ -5,6 +5,7 @@ import (
 	"math/big"
 	"testing"
 
+	bls12381 "github.com/consensys/gnark-crypto/ecc/bls12-381"
 	"github.com/stretchr/testify/require"
 )
 
@@ -116,6 +117,36 @@ func TestFromRaw(t *testing.T) {
 	require.EqualError(t, err, "bls12381: failed to unmarshal G1 pubkey: short buffer")
 }
 
+// A key is a (G1, G2) pair sharing one secret scalar, but validators are
+// identified by G1 alone. Verification must therefore prove the supplied G2
+// belongs to that same G1, or anyone can pair a victim's G1 with their own G2
+// and sign under the victim's identity.
+func TestBLSKeysRejectUnboundG2(t *testing.T) {
+	victim, err := GenerateKey()
+	require.NoError(t, err)
+	victimPub, ok := victim.PublicKey().(*PublicKey)
+	require.True(t, ok)
+
+	msg := randData(t)
+	msgHash := HashMessage(msg)
+
+	attackerScalar := big.NewInt(0xDEADBEEF)
+	_, _, _, g2Gen := bls12381.Generators()
+	var attackerG2 bls12381.G2Affine
+	attackerG2.ScalarMultiplication(&g2Gen, attackerScalar)
+
+	g1Hash, err := HashToG1(msgHash)
+	require.NoError(t, err)
+	var sig bls12381.G1Affine
+	sig.ScalarMultiplication(g1Hash, attackerScalar)
+
+	forged := NewPublicKey(victimPub.g1PubKey, attackerG2)
+	require.Equal(t, victimPub.OnChain(), forged.OnChain(), "forgery keeps the victim's identity")
+
+	require.Error(t, forged.VerifyWithHash(msgHash, sig.Marshal()))
+	require.Error(t, forged.Verify(msg, sig.Marshal()))
+}
+
 func randData(t *testing.T) []byte {
 	t.Helper()
 	data := make([]byte, 32)