client/core: use sha256 sighash in comms

This creates message signatures using a message hash (sha256) instead of
the message itself, which was truncated.

Verification of server signatures in incoming messages recognizes both
signatures.  When v0 is purged (V0PURGE), this should be removed.

Also insert our pubkey back into register response.

client/core/account_test.go

@@ -11,6 +11,8 @@ import (
 
 	"decred.org/dcrdex/client/db"
 	"decred.org/dcrdex/dex/order"
+	"decred.org/dcrdex/server/account"
+	"github.com/decred/dcrd/dcrec/secp256k1/v4"
 )
 
 func TestAccountExport(t *testing.T) {
@@ -237,11 +239,12 @@ func TestAccountExportAccountProofError(t *testing.T) {
 }
 
 func buildTestAccount(host string) Account {
+	privKey, _ := secp256k1.GeneratePrivateKey()
 	return Account{
 		Host:          host,
-		AccountID:     tDexAccountID.String(),
+		AccountID:     account.NewID(privKey.PubKey().SerializeCompressed()).String(), // can be anything though
+		PrivKey:       hex.EncodeToString(privKey.Serialize()),
 		DEXPubKey:     hex.EncodeToString(tDexKey.SerializeCompressed()),
-		PrivKey:       hex.EncodeToString(tDexPriv.Serialize()),
 		Cert:          hex.EncodeToString([]byte{}),
 		FeeCoin:       hex.EncodeToString([]byte("somecoin")),
 		FeeProofSig:   hex.EncodeToString(tFeeProofSig),

client/core/core.go

@@ -6,6 +6,7 @@ package core
 import (
 	"bytes"
 	"context"
+	"crypto/sha256"
 	"encoding/binary"
 	"encoding/csv"
 	"encoding/hex"
@@ -2974,6 +2975,9 @@ func (c *Core) register(dc *dexConnection, assetID uint32) (regRes *msgjson.Regi
 		if !bytes.Equal(dc.acct.dexPubKey.SerializeCompressed(), regRes.DEXPubKey) {
 			return nil, false, false, fmt.Errorf("different pubkeys reported by dex in 'config' and 'register' responses")
 		}
+		// Insert our pubkey back into the register result since it is excluded
+		// from the JSON serialization.
+		regRes.ClientPubKey = acctPubKey
 		// Check the DEX server's signature.
 		msg := regRes.Serialize()
 		err = checkSigS256(msg, regRes.DEXPubKey, regRes.Sig)
@@ -6548,17 +6552,27 @@ func checkSigS256(msg, pkBytes, sigBytes []byte) error {
 	if err != nil {
 		return fmt.Errorf("error decoding secp256k1 Signature from bytes: %w", err)
 	}
-	if !signature.Verify(msg, pubKey) {
-		return fmt.Errorf("secp256k1 signature verification failed")
+	hash := sha256.Sum256(msg)
+	if !signature.Verify(hash[:], pubKey) {
+		// This might be a legacy (buggy) server that signed the truncated
+		// message itself. (V0PURGE!)
+		if !signature.Verify(msg, pubKey) {
+			return fmt.Errorf("secp256k1 signature verification failed")
+		}
 	}
 	return nil
 }
 
+func signMsg(privKey *secp256k1.PrivateKey, msg []byte) []byte {
+	// NOTE: legacy servers will not accept this signature.
+	hash := sha256.Sum256(msg)
+	return ecdsa.Sign(privKey, hash[:]).Serialize()
+}
+
 // sign signs the msgjson.Signable with the provided private key.
 func sign(privKey *secp256k1.PrivateKey, payload msgjson.Signable) {
 	sigMsg := payload.Serialize()
-	sig := ecdsa.Sign(privKey, sigMsg) // should we be signing the *hash* of the payload?
-	payload.SetSig(sig.Serialize())
+	payload.SetSig(signMsg(privKey, sigMsg))
 }
 
 // stampAndSign time stamps the msgjson.Stampable, and signs it with the given

client/core/core_test.go

@@ -38,7 +38,6 @@ import (
 	serverdex "decred.org/dcrdex/server/dex"
 	"github.com/decred/dcrd/crypto/blake256"
 	"github.com/decred/dcrd/dcrec/secp256k1/v4"
-	"github.com/decred/dcrd/dcrec/secp256k1/v4/ecdsa"
 	"golang.org/x/text/language"
 	"golang.org/x/text/message"
 )
@@ -98,7 +97,6 @@ var (
 	}
 	tDexPriv            *secp256k1.PrivateKey
 	tDexKey             *secp256k1.PublicKey
-	tDexAccountID       account.AccountID
 	tPW                        = []byte("dexpw")
 	wPW                        = []byte("walletpw")
 	tDexHost                   = "somedex.tld:7232"
@@ -143,9 +141,9 @@ func makeAcker(serializer func(msg *msgjson.Message) msgjson.Signable) func(msg
 	return func(msg *msgjson.Message, f msgFunc) error {
 		signable := serializer(msg)
 		sigMsg := signable.Serialize()
-		sig := ecdsa.Sign(tDexPriv, sigMsg)
+		sig := signMsg(tDexPriv, sigMsg)
 		ack := &msgjson.Acknowledgement{
-			Sig: sig.Serialize(),
+			Sig: sig,
 		}
 		resp, _ := msgjson.NewResponse(msg.ID, ack, nil)
 		f(resp)
@@ -202,7 +200,7 @@ func tNewAccount(crypter *tCrypter) *dexAccount {
 		encKey:    encKey,
 		dexPubKey: tDexKey,
 		privKey:   privKey,
-		id:        tDexAccountID,
+		id:        account.NewID(privKey.PubKey().SerializeCompressed()),
 		feeCoin:   []byte("somecoin"),
 	}
 }
@@ -1104,6 +1102,13 @@ func (rig *testRig) queueConfig() {
 
 func (rig *testRig) queueRegister(regRes *msgjson.RegisterResult) {
 	rig.ws.queueResponse(msgjson.RegisterRoute, func(msg *msgjson.Message, f msgFunc) error {
+		// The response includes data from the request. Patch the pre-defined
+		// response with the client's pubkey from the request, and then sign the
+		// response, which needs the client pubkey for serialization.
+		reg := new(msgjson.Register)
+		msg.Unmarshal(reg)
+		regRes.ClientPubKey = reg.PubKey
+		sign(tDexPriv, regRes)
 		resp, _ := msgjson.NewResponse(msg.ID, regRes, nil)
 		f(resp)
 		return nil
@@ -1115,9 +1120,8 @@ func (rig *testRig) queueNotifyFee() {
 		req := new(msgjson.NotifyFee)
 		json.Unmarshal(msg.Payload, req)
 		sigMsg := req.Serialize()
-		sig := ecdsa.Sign(tDexPriv, sigMsg)
-		// Shouldn't Sig be dex.Bytes?
-		result := &msgjson.Acknowledgement{Sig: sig.Serialize()}
+		sig := signMsg(tDexPriv, sigMsg)
+		result := &msgjson.Acknowledgement{Sig: sig}
 		resp, _ := msgjson.NewResponse(msg.ID, result, nil)
 		f(resp)
 		return nil
@@ -1171,7 +1175,6 @@ func TestMain(m *testing.M) {
 	tCtx, shutdown = context.WithCancel(context.Background())
 	tDexPriv, _ = secp256k1.GeneratePrivateKey()
 	tDexKey = tDexPriv.PubKey()
-	tDexAccountID = account.NewID(tDexPriv.PubKey().SerializeCompressed())
 
 	doIt := func() int {
 		// Not counted as coverage, must test Archiver constructor explicitly.
@@ -1639,14 +1642,17 @@ func TestRegister(t *testing.T) {
 	// an error (no dupes). Initial state is to return an error.
 	rig.db.acctErr = tErr
 
+	// (*Core).Register does setupCryptoV2 to make the dc.acct.privKey etc., so
+	// we don't know the ClientPubKey here. It must be set in the request
+	// handler configured by queueRegister.
+
 	regRes := &msgjson.RegisterResult{
-		DEXPubKey:    rig.acct.dexPubKey.SerializeCompressed(),
-		ClientPubKey: dex.Bytes{0x1}, // part of the serialization, but not the response
-		Address:      "someaddr",
-		Fee:          tFee,
-		Time:         encode.UnixMilliU(time.Now()),
+		DEXPubKey: rig.acct.dexPubKey.SerializeCompressed(),
+		// ClientPubKey is set in the handler, where the sig is made
+		Address: "someaddr",
+		Fee:     tFee,
+		Time:    encode.UnixMilliU(time.Now()),
 	}
-	sign(tDexPriv, regRes)
 
 	var wg sync.WaitGroup
 	defer wg.Wait() // don't allow fail after TestRegister return
@@ -1859,7 +1865,15 @@ func TestRegister(t *testing.T) {
 	goodSig := regRes.Sig
 	regRes.Sig = []byte("badsig")
 	queueConfigAndConnectUnknownAcct()
-	rig.queueRegister(regRes)
+	rig.ws.queueResponse(msgjson.RegisterRoute, func(msg *msgjson.Message, f msgFunc) error {
+		reg := new(msgjson.Register)
+		msg.Unmarshal(reg)
+		regRes.ClientPubKey = reg.PubKey
+		// NOT re-signing it, just keep badsig
+		resp, _ := msgjson.NewResponse(msg.ID, regRes, nil)
+		f(resp)
+		return nil
+	})
 	run()
 	if !errorHasCode(err, signatureErr) {
 		t.Fatalf("wrong error for bad signature on register response: %v", err)
@@ -2920,6 +2934,7 @@ func TestRefundReserves(t *testing.T) {
 	lo.Force = order.ImmediateTiF
 	loid = lo.ID()
 	msgMatch.OrderID = loid[:]
+	sign(tDexPriv, msgMatch)
 
 	test("partial immediate TiF limit order", reserves/3, func() {
 		matchReq, _ := msgjson.NewRequest(1, msgjson.MatchRoute, []*msgjson.Match{msgMatch})
@@ -2931,14 +2946,17 @@ func TestRefundReserves(t *testing.T) {
 	lo.Force = order.StandingTiF
 	loid = lo.ID()
 	msgMatch.OrderID = loid[:]
+	sign(tDexPriv, msgMatch)
 
 	addMatch := func(side order.MatchSide, status order.MatchStatus, qty uint64) order.MatchID {
+		t.Helper()
 		msgMatch.Side = uint8(side)
 		m := *msgMatch
 		var mid order.MatchID
 		copy(mid[:], encode.RandomBytes(32))
 		m.MatchID = mid[:]
 		m.Quantity = qty
+		sign(tDexPriv, &m)
 		matchReq, _ := msgjson.NewRequest(1, msgjson.MatchRoute, []*msgjson.Match{&m})
 		if err := handleMatchRoute(tCore, rig.dc, matchReq); err != nil {
 			t.Fatalf("handleMatchRoute error: %v", err)
@@ -2979,6 +2997,7 @@ func TestRefundReserves(t *testing.T) {
 	moid := mo.ID()
 	dbOrder.Order = mo
 	msgMatch.OrderID = moid[:]
+	sign(tDexPriv, msgMatch)
 
 	tracker = newTrackedTrade(dbOrder, preImg, dc, mkt.EpochLen, rig.core.lockTimeTaker, rig.core.lockTimeMaker,
 		rig.db, rig.queue, walletSet, nil, rig.core.notify, rig.core.formatDetails, nil, 0, reserves)
@@ -3166,6 +3185,7 @@ func TestRedemptionReserves(t *testing.T) {
 	lo.Force = order.ImmediateTiF
 	loid = lo.ID()
 	msgMatch.OrderID = loid[:]
+	sign(tDexPriv, msgMatch)
 
 	test("partially filled immediate TiF limit order", reserves/3, func() {
 		matchReq, _ := msgjson.NewRequest(1, msgjson.MatchRoute, []*msgjson.Match{msgMatch})
@@ -3182,6 +3202,7 @@ func TestRedemptionReserves(t *testing.T) {
 	moid := mo.ID()
 	dbOrder.Order = mo
 	msgMatch.OrderID = moid[:]
+	sign(tDexPriv, msgMatch)
 
 	tracker = newTrackedTrade(dbOrder, preImg, dc, mkt.EpochLen, rig.core.lockTimeTaker, rig.core.lockTimeMaker,
 		rig.db, rig.queue, walletSet, nil, rig.core.notify, rig.core.formatDetails, nil, reserves, 0)
@@ -3205,6 +3226,7 @@ func TestRedemptionReserves(t *testing.T) {
 		copy(mid[:], encode.RandomBytes(32))
 		m.MatchID = mid[:]
 		m.Quantity = qty
+		sign(tDexPriv, &m)
 		matchReq, _ := msgjson.NewRequest(1, msgjson.MatchRoute, []*msgjson.Match{&m})
 		if err := handleMatchRoute(tCore, rig.dc, matchReq); err != nil {
 			t.Fatalf("handleMatchRoute error: %v", err)
@@ -4180,6 +4202,7 @@ func TestTradeTracking(t *testing.T) {
 	// Make sure that a fee rate higher than our recorded MaxFeeRate results in
 	// an error.
 	msgMatch.FeeRateBase = tMaxFeeRate + 1
+	sign(tDexPriv, msgMatch)
 	msg, _ := msgjson.NewRequest(1, msgjson.MatchRoute, []*msgjson.Match{msgMatch})
 	err = handleMatchRoute(tCore, rig.dc, msg)
 	if err == nil || !strings.Contains(err.Error(), "is > MaxFeeRate") {
@@ -4188,6 +4211,7 @@ func TestTradeTracking(t *testing.T) {
 
 	// Restore fee rate.
 	msgMatch.FeeRateBase = tMaxFeeRate
+	sign(tDexPriv, msgMatch)
 	msg, _ = msgjson.NewRequest(1, msgjson.MatchRoute, []*msgjson.Match{msgMatch})
 
 	// Handle new match as maker with a queued invalid DEX init ack.
@@ -4623,7 +4647,7 @@ func TestTradeTracking(t *testing.T) {
 	tracker.cancel = nil
 	rig.ws.queueResponse(msgjson.InitRoute, initAcker)
 	msgMatch.Side = uint8(order.Maker)
-	sign(tDexPriv, msgMatch)
+	sign(tDexPriv, msgMatch) // Side is not in the serialization but whatever
 	msg, _ = msgjson.NewRequest(1, msgjson.MatchRoute, []*msgjson.Match{msgMatch})
 
 	tracker.metaData.Status = order.OrderStatusEpoch

client/core/types.go

@@ -22,7 +22,6 @@ import (
 	"decred.org/dcrdex/dex/order"
 	"decred.org/dcrdex/server/account"
 	"github.com/decred/dcrd/dcrec/secp256k1/v4"
-	"github.com/decred/dcrd/dcrec/secp256k1/v4/ecdsa"
 	"github.com/decred/dcrd/hdkeychain/v3"
 )
 
@@ -784,8 +783,7 @@ func (a *dexAccount) sign(msg []byte) ([]byte, error) {
 	if a.privKey == nil {
 		return nil, fmt.Errorf("account locked")
 	}
-	sig := ecdsa.Sign(a.privKey, msg)
-	return sig.Serialize(), nil
+	return signMsg(a.privKey, msg), nil
 }
 
 // checkSig checks the signature against the message and the DEX pubkey.