From ab7b035cd64ae528b2e7cfebf4a8283a320fd395 Mon Sep 17 00:00:00 2001 From: benma's agent Date: Wed, 5 Aug 2026 23:59:56 +0000 Subject: [PATCH] firmware: validate antiklepto signatures Use shared compact and recoverable ECDSA validators for direct signing responses and from inside Anti-Klepto verification. Reject invalid scalars, high-S encodings, and recovery IDs outside the secp256k1 range. Accept recovery IDs 0..3 for all recoverable ECDSA signatures. Add regression coverage for malformed signatures. --- api/firmware/btc.go | 15 ++++++--- api/firmware/eth.go | 16 +++++++-- api/firmware/secp256k1.go | 55 +++++++++++++++++++++++++++++++ api/firmware/secp256k1_test.go | 59 ++++++++++++++++++++++++++++++++++ 4 files changed, 138 insertions(+), 7 deletions(-) diff --git a/api/firmware/btc.go b/api/firmware/btc.go index 729492a..0e985ed 100644 --- a/api/firmware/btc.go +++ b/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] diff --git a/api/firmware/eth.go b/api/firmware/eth.go index d8e1563..a103662 100644 --- a/api/firmware/eth.go +++ b/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 diff --git a/api/firmware/secp256k1.go b/api/firmware/secp256k1.go index a5e204a..d871270 100644 --- a/api/firmware/secp256k1.go +++ b/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) diff --git a/api/firmware/secp256k1_test.go b/api/firmware/secp256k1_test.go index bc8606a..28eb1b0 100644 --- a/api/firmware/secp256k1_test.go +++ b/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.