firmware: validate antiklepto signatures

api/firmware/btc.go

@@ -465,6 +465,11 @@ func (device *Device) nonAtomicBTCSign(
 				if !next.HasSignature {
 					return nil, errp.New("unexpected response; expected signature")
 				}
+				if !inputIsSchnorr && !performAntiklepto {
+					if err := validateCompactECDSASignature(next.Signature); err != nil {
+						return nil, err
+					}
+				}
 				signatures[inputIndex] = next.Signature
 			}
 
@@ -711,12 +716,11 @@ func (device *Device) nonAtomicBTCSignMessage(
 			return nil, errp.New("unexpected response")
 		}
 		signature = signResponse.SignMessage.Signature
-		err = antikleptoVerify(
+		if err := antikleptoVerifyRecoverable(
 			hostNonce,
 			signerCommitment.AntikleptoSignerCommitment.Commitment,
-			signature[:64],
-		)
-		if err != nil {
+			signature,
+		); err != nil {
 			return nil, err
 		}
 	} else {
@@ -725,6 +729,9 @@ func (device *Device) nonAtomicBTCSignMessage(
 			return nil, errp.New("unexpected response")
 		}
 		signature = signResponse.SignMessage.Signature
+		if err := validateRecoverableECDSASignature(signature); err != nil {
+			return nil, err
+		}
 	}
 
 	sig, recID := signature[:64], signature[64]

api/firmware/eth.go

@@ -133,10 +133,10 @@ func (device *Device) nonAtomicHandleSignerNonceCommitment(
 		return nil, errp.New("unexpected response")
 	}
 	signature := signResponse.Sign.Signature
-	err = antikleptoVerify(
+	err = antikleptoVerifyRecoverable(
 		hostNonce,
 		signerCommitment.AntikleptoSignerCommitment.Commitment,
-		signature[:64],
+		signature,
 	)
 	if err != nil {
 		return nil, err
@@ -290,7 +290,11 @@ func (device *Device) nonAtomicETHSign(
 	if !ok {
 		return nil, errp.New("unexpected response")
 	}
-	return signResponse.Sign.Signature, nil
+	signature := signResponse.Sign.Signature
+	if err := validateRecoverableECDSASignature(signature); err != nil {
+		return nil, err
+	}
+	return signature, nil
 }
 
 // ETHSignEIP1559 signs an ethereum EIP1559 transaction. It returns a 65 byte
@@ -463,6 +467,9 @@ func (device *Device) nonAtomicETHSignMessage(
 		return nil, errp.New("unexpected response")
 	}
 	signature := signResponse.Sign.Signature
+	if err := validateRecoverableECDSASignature(signature); err != nil {
+		return nil, err
+	}
 	// 27 is the magic constant to add to the recoverable ID to denote an uncompressed pubkey.
 	signature[64] += 27
 
@@ -830,6 +837,9 @@ func (device *Device) nonAtomicETHSignTypedMessage(
 		return nil, errp.New("unexpected response")
 	}
 	signature := signResponse.Sign.Signature
+	if err := validateRecoverableECDSASignature(signature); err != nil {
+		return nil, err
+	}
 	// 27 is the magic constant to add to the recoverable ID to denote an uncompressed pubkey.
 	signature[64] += 27
 	return signature, nil

api/firmware/secp256k1.go

@@ -24,10 +24,65 @@ func antikleptoHostCommit(hostNonce []byte) []byte {
 	return taggedSha256([]byte("s2c/ecdsa/data"), hostNonce)
 }
 
+const (
+	compactECDSASignatureLen     = 64
+	recoverableECDSASignatureLen = compactECDSASignatureLen + 1
+)
+
+func validateCompactECDSASignature(signature []byte) error {
+	if len(signature) != compactECDSASignatureLen {
+		return errp.New("compact ECDSA signature must be 64 bytes")
+	}
+
+	var r, s btcec.ModNScalar
+	if r.SetByteSlice(signature[:32]) || r.IsZero() ||
+		s.SetByteSlice(signature[32:]) || s.IsZero() {
+		return errp.New("invalid compact ECDSA signature scalar")
+	}
+	if s.IsOverHalfOrder() {
+		return errp.New("invalid compact ECDSA signature: S is not in low-S form")
+	}
+
+	return nil
+}
+
+func validateRecoverableECDSASignature(signature []byte) error {
+	if len(signature) != recoverableECDSASignatureLen {
+		return errp.New("recoverable ECDSA signature must be 65 bytes")
+	}
+	if err := validateCompactECDSASignature(signature[:compactECDSASignatureLen]); err != nil {
+		return err
+	}
+	if signature[compactECDSASignatureLen] > 3 {
+		return errp.New("invalid recoverable ECDSA signature: recovery ID must be between 0 and 3")
+	}
+	return nil
+}
+
 // antikleptoVerify verifies that hostNonce was used to tweak the nonce during signature
 // generation according to k' = k + H(clientCommitment, hostNonce) by checking that
 // k'*G = signerCommitment + H(signerCommitment, hostNonce)*G.
 func antikleptoVerify(hostNonce, signerCommitment, signature []byte) error {
+	if err := validateCompactECDSASignature(signature); err != nil {
+		return err
+	}
+	return antikleptoVerifyNonce(hostNonce, signerCommitment, signature)
+}
+
+func antikleptoVerifyRecoverable(
+	hostNonce, signerCommitment, signature []byte,
+) error {
+	if err := validateRecoverableECDSASignature(signature); err != nil {
+		return err
+	}
+	return antikleptoVerifyNonce(
+		hostNonce,
+		signerCommitment,
+		signature[:compactECDSASignatureLen],
+	)
+}
+
+func antikleptoVerifyNonce(hostNonce, signerCommitment, signature []byte) error {
 	signerCommitmentPubkey, err := btcec.ParsePubKey(signerCommitment)
 	if err != nil {
 		return errp.WithStack(err)

api/firmware/secp256k1_test.go

@@ -52,6 +52,65 @@ func TestAntikleptoVerify(t *testing.T) {
 	}
 }
 
+func TestAntikleptoVerifyRejectsInvalidSignatures(t *testing.T) {
+	hostNonce := unhex("8b4c26aa2695a34bdbc34235f6c91be14b93037a063b13f7c814101359561092")
+	signerCommitment := unhex("0236ff92fe02c08d0d04851e0ce1516104085215f05a178307de60ea53e207f971")
+	valid := unhex("7fd66b48ffea2fe048869880bbb3a1819e262af14980e8885df1e5765750cb8f47e01eca356377870356d54853573a955076228e5044cd3dd3a049abe70d5585")
+	require.NoError(t, antikleptoVerify(hostNonce, signerCommitment, valid))
+
+	highS := unhex("7fd66b48ffea2fe048869880bbb3a1819e262af14980e8885df1e5765750cb8fb81fe135ca9c8878fca92ab7aca8c5696a38ba585f03d2fdec3214e0e928ebbc")
+	require.ErrorContains(
+		t,
+		antikleptoVerify(hostNonce, signerCommitment, highS),
+		"S is not in low-S form",
+	)
+
+	order := make([]byte, 32)
+	btcec.S256().N.FillBytes(order)
+	replaceScalar := func(offset int, scalar []byte) []byte {
+		signature := append([]byte(nil), valid...)
+		copy(signature[offset:offset+32], scalar)
+		return signature
+	}
+
+	for _, signature := range [][]byte{
+		valid[:compactECDSASignatureLen-1],
+		append(append([]byte(nil), valid...), 0),
+		replaceScalar(0, make([]byte, 32)),
+		replaceScalar(0, order),
+		replaceScalar(32, make([]byte, 32)),
+		replaceScalar(32, order),
+	} {
+		require.Error(t, antikleptoVerify(hostNonce, signerCommitment, signature))
+	}
+}
+
+func TestAntikleptoVerifyRecoverable(t *testing.T) {
+	hostNonce := unhex("8b4c26aa2695a34bdbc34235f6c91be14b93037a063b13f7c814101359561092")
+	signerCommitment := unhex("0236ff92fe02c08d0d04851e0ce1516104085215f05a178307de60ea53e207f971")
+	signature := append(
+		unhex("7fd66b48ffea2fe048869880bbb3a1819e262af14980e8885df1e5765750cb8f47e01eca356377870356d54853573a955076228e5044cd3dd3a049abe70d5585"),
+		0,
+	)
+	require.NoError(t, antikleptoVerifyRecoverable(hostNonce, signerCommitment, signature))
+
+	require.ErrorContains(
+		t,
+		antikleptoVerifyRecoverable(hostNonce, signerCommitment, signature[:64]),
+		"must be 65 bytes",
+	)
+
+	signature[compactECDSASignatureLen] = 3
+	require.NoError(t, antikleptoVerifyRecoverable(hostNonce, signerCommitment, signature))
+
+	signature[compactECDSASignatureLen] = 4
+	require.ErrorContains(
+		t,
+		antikleptoVerifyRecoverable(hostNonce, signerCommitment, signature),
+		"recovery ID",
+	)
+}
+
 func TestDLEQVerify(t *testing.T) {
 	// secret key sk=077eb75a52eca24cdedf058c92f1ca8b9d4841771fd6baa3d27885fb5b49fba2
 	// is the secret key in pubKey=sk*G and otherPubKey=sk*otherBase.