| 文件 | 最后提交记录 | 最后更新时间 |
|---|---|---|
tss, crypto, keygen, resharing: complete six checks that were unrated and unfixed Six items carried in the audit ledger as neither rated nor fixed, each with a bound that could be derived rather than chosen. Grouped because four of them are the same shape: something a peer declares decides how much work this library does, and the check that would refuse it ran afterwards. BOUND BEFORE WORK, not after. Two places, identical accept sets before and after -- only the order changed, so no existing test moves. - crypto/modproof Verify tests pf.W's range before isQuadraticResidue. W arrives unbounded (NewProofFromBytes counts parts and never measures one; KGRound2Message2.ValidateBasic does not look at the proof), and big.Jacobi reduces modulo N first, so its cost tracks W's size while the comparisons do not. Measured at 38 ms against 2.5 ms for an 8 MB W, and the new test asserts size-independence rather than an absolute time -- an absolute threshold loose enough to be stable also swallows the difference. It goes red on the previous ordering. - The four DeCommit call sites whose expected length is a function of the threshold check the part count before calling, because DeCommit hashes every part it is given. Measured at 458 ns for 7 parts against 6.6 ms for 200000. These are exactly the four NonEmptyMultiBytes call sites in the tree that pass no expected length, and they cannot: the length is (t+1)*2+1 and the message layer does not know t. A test pins that, so a later reader cannot conclude from the message layer alone that the count is bounded upstream. BOUND AT THE MESSAGE LAYER where a constant does exist. DGRound1Message's ssid and session_nonce_hash had no upper bound on either curve. Both are produced as SHA512_256i(...).Bytes(), so 32 is the width a conforming sender reaches, not a number picked to look safe. The ssid's emptiness is deliberately still round 2's to refuse, so that an empty declaration is reported against its sender instead of being dropped here with no attribution. STOP AN ABORTED PARTY. BaseUpdate had no latch: a party that reported an abort kept storing messages and re-entering rounds, walking into rounds whose predecessor never filled the slots they read. Those reads are ordinary indexing of pre-allocated slices holding nil, so it faults instead of reporting -- with no error, no round and no culprit. The abort itself was correct one message earlier; what was lost is everything after it. Latched on errors out of round Update and Start, which have written state, and NOT on the rejections above them: a message that failed ValidateMessage or StoreMessage was refused before anything was written, and one bad message from one peer must not end a party. The refusal names nobody, deliberately -- the abort named whoever was responsible once already, and re-naming per delivered message lets the pump rate decide how guilty a peer looks to a host that counts. GUARD TWO MORE PROMOTED FIELDS, the shape encoding/json and a shallow copy produce. PartyID.String() answers with a marker instead of faulting: it is the method called while something is already being diagnosed, and a direct call is what took the process, since fmt recovers a panicking String. Both BuildLocalSaveDataSubset implementations attribute the fault by name, matching the named panic they already raise one line later for a roster entry they cannot resolve -- neither function has a return channel to use instead. CORRECT A FALSE COMMENT. ecdsa/keygen/round_3.go called the Paillier ModProof "the only check that proves N is a true biprime (no small factors)". Neither half holds: absence of small factors is FacProof's statement, verified unconditionally in the same handler since the NoProofFac switch was removed, and ProofMod attests Blum-integer shape because N is its only statement input. Exclusivity is not a claim a comment can support about a codebase it does not enumerate. Every fix has a negative control alongside it. Full suite green, 18 packages. Co-Authored-By: Claude Code <noreply@anthropic.com> | 1 个月前 | |
fix: reject Edwards low-order points in proof EC validation For Ed25519 (cofactor 8) on-curve membership is strictly weaker than prime-order subgroup membership: there are 8 small-order points (orders 1, 2, 4, 8) that satisfy the curve equation and would pass ECPoint.ValidateBasic, but lie outside the prime-order subgroup. A malicious party can submit such a point as a Schnorr commitment, VSS share, or MtA WC commitment to mount small-subgroup-style attacks. Add ECPoint.IsInPrimeOrderSubgroup() — computes `[curve.N] * p` and checks the result is identity. For prime-order curves (secp256k1, NIST family, cofactor 1) this is structurally guaranteed by IsOnCurve and the explicit check costs only one extra ScalarMult; for Ed25519 the check is load-bearing. Add ECPoint.ValidateInSubgroup() as the stricter sibling of ValidateBasic, used wherever an EC point arrives from attacker- controlled wire bytes: - crypto/schnorr/schnorr_proof.go: ZKProof.Verify (X, Alpha) and ZKVProof.Verify (V, R, Alpha). - crypto/vss/feldman_vss.go: every vs[j] commitment. - crypto/mta/proofs.go: ProofBobWC.Verify's X and pf.U in the with- check branch. To avoid paying the ScalarMult on prime-order curves where the check is structurally redundant, ValidateInSubgroup short-circuits via tss.HasCompositeCofactor(p.curve). tss/curve.go gains the per-curve cofactor classification (secp256k1 → false, ed25519 → true, unknown → true conservatively). Closes the explicitly-deferred 🔥 P1 item from the ZKP audit. The EightInvEight() projection in eddsa/{keygen,signing,resharing}/ round_* still runs at the protocol level, so the new check is a defense-in-depth backstop for direct API callers and for code paths not covered by EightInvEight (Schnorr Alpha, MtA pf.U). Performance cost: ~30-50µs per Ed25519 point validated; ECDSA paths are unchanged. Test in crypto/ecpoint_test.go covers a synthesized Ed25519 order-2 point (0, p-1), a prime-order point, the identity (which is rejected by ValidateBasic before the subgroup check), and a secp256k1 sanity case. Full ECDSA / EdDSA e2e keygen / resharing / signing suites still pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> | 4 个月前 | |
error: add Unwrap | 6 年前 | |
crypto, tss: guard the nil-able thing that is actually dereferenced, not its container Four sites where a nil guard protects an outer pointer (or nothing at all) while the value the code goes on to dereference is reachable through it, nil-able, and unchecked. Each was reachable from exported API and each faulted with a bare nil dereference rather than saying what was wrong. crypto/mta ProofBobWC.Bytes read parts 10 and 11 through pf.ProofBob and pf.U without establishing either; ProofBob.Bytes read ten *big.Int fields without establishing any. Both now require the type's own ValidateBasic. They panic rather than substituting a value: the return type is a fixed array with no error channel, and an empty slice in place of a missing field would emit a proof that looks serialisable and is not. crypto ECPoint X and Y dereferenced p.coords[i] with no check on the receiver or the coordinate -- and a nil coordinate is producible through the exported NewECPointNoCurveCheck, which stores whatever it is given. The existing guards live in the callers (Add, ScalarMult, Equals) and cover the outer pointer only. X/Y now panic attributably, because returning nil merely relocates the same fault to the caller's Cmp or Bytes. Where a return channel exists it is used instead: IsOnCurve answers false for a nil point, a nil curve (a direct field of interface type, nil-able even on a non-nil point) or nil coordinates; GobEncode returns an error; Equals returns false rather than panicking through X/Y. tss NewMessageWrapper checked `routing.To != nil` and then dereferenced routing.To[i]; routing.From had no check at all. Both are now tested. This panics rather than skipping or emitting a nil entry: skipping would silently shrink the recipient list, and a nil entry produces a message the receiver is guaranteed to reject with no indication why. tss IsOldCommittee / IsNewCommittee dereferenced rgParams.partyID and the PeerContext, neither of which is validated at construction -- NewParameters never inspects partyID and stores a nil context unconditionally, and the resharing constructor calls IsOldCommittee before any round runs, so BaseStart's validation is too late. PeerContext.IDs is now nil-receiver safe, which matches the convention the file already documents and fixes every reader at once; the committee test skips roster entries whose key cannot be read and answers false for a self PartyID that has none. Key comparison moved to canonical hex via the existing partyKeyID helper, which is equality on the same integers. Tests cover both directions for each site: the malformed input must produce the intended outcome, and well-formed input must behave exactly as before, so none of these can pass by refusing everything. Verified against the unguarded versions: every new assertion fails there with the predicted nil dereference, while the negative controls stay green. Co-Authored-By: Claude Code <noreply@anthropic.com> | 1 个月前 | |
fix: bind resharing peer NTilde to ModProof ECDSA resharing was the last ⚠️ P1 item in the ZKP audit. Keygen already linked peer NTilde to a Blum-integer ModProof in `6ba5e0d` (via `KGRound2Message2.nTildeModProof`, verified in ecdsa/keygen/round_3.go before NTildej is saved). Resharing didn't carry the same binding: `DGRound2Message1` shipped Paillier ModProof + NTilde + H1 + H2 + two DLN proofs, and round_4_new_step_2 saved NTildej / H1j / H2j after the Paillier and DLN checks — leaving NTilde itself only "locally plausible" rather than proven to be the safe-prime product the QR-group argument assumes. This commit mirrors the keygen pattern in resharing: - protob/ecdsa-resharing.proto: `DGRound2Message1` gains a `repeated bytes nTildeModProof = 8` field. Optional on the wire so v3-vintage peers that omit it can be accepted under `params.NoProofMod()`; v4 default mode (the production setting in this branch's e2e test, see `0f67b50` removing the resharing SetNoProofMod) requires the proof. - ecdsa/resharing/messages.go: `NewDGRound2Message1` takes the new `nTildeModProof` argument; `UnmarshalNTildeModProof` mirrors the keygen helper. - ecdsa/resharing/round_2_new_step_1.go: generate the proof alongside the Paillier ModProof, using the safe-prime factors derived from `LocalPreParams.P/Q` (same fix shape as keygen's `6ba5e0d`). - ecdsa/resharing/round_4_new_step_2.go: verify the proof in the same goroutine as the Paillier ModProof, attributing failures to the peer via `paiProofCulprits`. Honors `NoProofMod()` for v3 backward compatibility. Other regenerated `.pb.go` diffs in the commit are protoc-tooling churn from `make protob` rebuilding all protos; no semantic changes to other messages. Full e2e ecdsa/resharing and eddsa/resharing suites still pass. Closes the last ⚠️ P1 row in `_local_only/ZKP_BASIC_PROPERTY_AUDIT.md`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> | 4 个月前 | |
crypto, tss: guard the nil-able thing that is actually dereferenced, not its container Four sites where a nil guard protects an outer pointer (or nothing at all) while the value the code goes on to dereference is reachable through it, nil-able, and unchecked. Each was reachable from exported API and each faulted with a bare nil dereference rather than saying what was wrong. crypto/mta ProofBobWC.Bytes read parts 10 and 11 through pf.ProofBob and pf.U without establishing either; ProofBob.Bytes read ten *big.Int fields without establishing any. Both now require the type's own ValidateBasic. They panic rather than substituting a value: the return type is a fixed array with no error channel, and an empty slice in place of a missing field would emit a proof that looks serialisable and is not. crypto ECPoint X and Y dereferenced p.coords[i] with no check on the receiver or the coordinate -- and a nil coordinate is producible through the exported NewECPointNoCurveCheck, which stores whatever it is given. The existing guards live in the callers (Add, ScalarMult, Equals) and cover the outer pointer only. X/Y now panic attributably, because returning nil merely relocates the same fault to the caller's Cmp or Bytes. Where a return channel exists it is used instead: IsOnCurve answers false for a nil point, a nil curve (a direct field of interface type, nil-able even on a non-nil point) or nil coordinates; GobEncode returns an error; Equals returns false rather than panicking through X/Y. tss NewMessageWrapper checked `routing.To != nil` and then dereferenced routing.To[i]; routing.From had no check at all. Both are now tested. This panics rather than skipping or emitting a nil entry: skipping would silently shrink the recipient list, and a nil entry produces a message the receiver is guaranteed to reject with no indication why. tss IsOldCommittee / IsNewCommittee dereferenced rgParams.partyID and the PeerContext, neither of which is validated at construction -- NewParameters never inspects partyID and stores a nil context unconditionally, and the resharing constructor calls IsOldCommittee before any round runs, so BaseStart's validation is too late. PeerContext.IDs is now nil-receiver safe, which matches the convention the file already documents and fixes every reader at once; the committee test skips roster entries whose key cannot be read and answers false for a self PartyID that has none. Key comparison moved to canonical hex via the existing partyKeyID helper, which is equality on the same integers. Tests cover both directions for each site: the malformed input must produce the intended outcome, and well-formed input must behave exactly as before, so none of these can pass by refusing everything. Verified against the unguarded versions: every new assertion fails there with the predicted nil dereference, while the negative controls stay green. Co-Authored-By: Claude Code <noreply@anthropic.com> | 1 个月前 | |
docs(tss): document SessionNonce secrecy and single use as security requirements SetSessionNonce's godoc did mention setting a fresh nonce per execution, but framed it as housekeeping ("previous value stays behind otherwise") rather than as a security requirement, and said nothing at all about the preimage. Since only the SHA512_256 digest ever travels on the wire, that omission actively invites the reading that the preimage need not be secret -- so a caller had no way to learn it carries these obligations. Three requirements are now stated explicitly, together with the fact that the library enforces none of them: - The preimage must be kept secret. Only its digest is transmitted, and that does not make the preimage public; anyone holding it can set the same nonce on an instance of their own. - It must be unique per resharing instance, not merely per logical session. The binding's granularity is the nonce itself, so two instantiations that share one are indistinguishable to the protocol. - The library keeps no cross-instance record of consumed nonces. Reuse lets a passively captured old-committee transcript be adopted by a second, separate new-committee instance with no live old-committee party involved, deriving share material for the same public key. The round-2 binding check does not catch this: it compares the nonce, and the nonce matches. This documents the caller's contract; behaviour is unchanged. Verified: gofmt clean; go build ./... and go vet ./tss/ exit 0; go test ./tss/ ./ecdsa/resharing/ ./eddsa/resharing/ -count=1 exit 0 (3 ok, 0 FAIL). Co-Authored-By: Claude Code <noreply@anthropic.com> | 24 天前 | |
fix: reject zero-residue PartyIDs; tighten GetRandomPositiveInt Two self-review findings on top of the pre-push review pass: 1. tss/params.go assertDistinctIDsModQ now also panics when any PartyID's KeyInt() mod q is exactly 0 (previously it only caught collisions between two distinct keys with the same residue). A party with KeyInt() ≡ 0 mod q is fatal: in Shamir secret sharing f(0) is the secret, so the polynomial evaluated at that party's "x-coordinate" gives them the raw secret outright. Even if keygen blocks the bad ID via vss.Create's CheckIndexes, the signing path (PrepareForSigning in ecdsa/signing/prepare.go) would, when the quorum includes the zero-residue party as one of the j != i others, compute iota = ksc * ModInverse(ksc - ksj) = 0, then bigWj.ScalarMult(0) returns nil, then the next ScalarMult panics. Direct API consumers loading legacy LocalPartySaveData and going straight to signing previously bypassed the vss.Create check; asserting at the NewParameters layer closes that path. 2. common/random.go GetRandomPositiveInt now strictly returns a value in the open interval (0, lessThan). The previous implementation sampled from [0, lessThan), so the name's "Positive" claim was technically false and verifiers using IsInIntervalPositive (added in 3091f00 / 004f48e) on these sampler outputs had a 1/lessThan ≈ 2^-256 chance of incorrectly rejecting an honest proof. Probability was negligible but the API/comment contract was inconsistent; the loop now rejects try.Sign() == 0 alongside the existing try >= lessThan rejection. Also reject lessThan <= 1 up-front (no value exists in (0, 1)). Tests in common/random_test.go and tss/params_test.go cover the new rejection paths. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> | 4 个月前 | |
tss, crypto, keygen, resharing: complete six checks that were unrated and unfixed Six items carried in the audit ledger as neither rated nor fixed, each with a bound that could be derived rather than chosen. Grouped because four of them are the same shape: something a peer declares decides how much work this library does, and the check that would refuse it ran afterwards. BOUND BEFORE WORK, not after. Two places, identical accept sets before and after -- only the order changed, so no existing test moves. - crypto/modproof Verify tests pf.W's range before isQuadraticResidue. W arrives unbounded (NewProofFromBytes counts parts and never measures one; KGRound2Message2.ValidateBasic does not look at the proof), and big.Jacobi reduces modulo N first, so its cost tracks W's size while the comparisons do not. Measured at 38 ms against 2.5 ms for an 8 MB W, and the new test asserts size-independence rather than an absolute time -- an absolute threshold loose enough to be stable also swallows the difference. It goes red on the previous ordering. - The four DeCommit call sites whose expected length is a function of the threshold check the part count before calling, because DeCommit hashes every part it is given. Measured at 458 ns for 7 parts against 6.6 ms for 200000. These are exactly the four NonEmptyMultiBytes call sites in the tree that pass no expected length, and they cannot: the length is (t+1)*2+1 and the message layer does not know t. A test pins that, so a later reader cannot conclude from the message layer alone that the count is bounded upstream. BOUND AT THE MESSAGE LAYER where a constant does exist. DGRound1Message's ssid and session_nonce_hash had no upper bound on either curve. Both are produced as SHA512_256i(...).Bytes(), so 32 is the width a conforming sender reaches, not a number picked to look safe. The ssid's emptiness is deliberately still round 2's to refuse, so that an empty declaration is reported against its sender instead of being dropped here with no attribution. STOP AN ABORTED PARTY. BaseUpdate had no latch: a party that reported an abort kept storing messages and re-entering rounds, walking into rounds whose predecessor never filled the slots they read. Those reads are ordinary indexing of pre-allocated slices holding nil, so it faults instead of reporting -- with no error, no round and no culprit. The abort itself was correct one message earlier; what was lost is everything after it. Latched on errors out of round Update and Start, which have written state, and NOT on the rejections above them: a message that failed ValidateMessage or StoreMessage was refused before anything was written, and one bad message from one peer must not end a party. The refusal names nobody, deliberately -- the abort named whoever was responsible once already, and re-naming per delivered message lets the pump rate decide how guilty a peer looks to a host that counts. GUARD TWO MORE PROMOTED FIELDS, the shape encoding/json and a shallow copy produce. PartyID.String() answers with a marker instead of faulting: it is the method called while something is already being diagnosed, and a direct call is what took the process, since fmt recovers a panicking String. Both BuildLocalSaveDataSubset implementations attribute the fault by name, matching the named panic they already raise one line later for a roster entry they cannot resolve -- neither function has a return channel to use instead. CORRECT A FALSE COMMENT. ecdsa/keygen/round_3.go called the Paillier ModProof "the only check that proves N is a true biprime (no small factors)". Neither half holds: absence of small factors is FacProof's statement, verified unconditionally in the same handler since the NoProofFac switch was removed, and ProofMod attests Blum-integer shape because N is its only statement input. Exclusivity is not a claim a comment can support about a codebase it does not enumerate. Every fix has a negative control alongside it. Full suite green, 18 packages. Co-Authored-By: Claude Code <noreply@anthropic.com> | 1 个月前 | |
tss, crypto, keygen, resharing: complete six checks that were unrated and unfixed Six items carried in the audit ledger as neither rated nor fixed, each with a bound that could be derived rather than chosen. Grouped because four of them are the same shape: something a peer declares decides how much work this library does, and the check that would refuse it ran afterwards. BOUND BEFORE WORK, not after. Two places, identical accept sets before and after -- only the order changed, so no existing test moves. - crypto/modproof Verify tests pf.W's range before isQuadraticResidue. W arrives unbounded (NewProofFromBytes counts parts and never measures one; KGRound2Message2.ValidateBasic does not look at the proof), and big.Jacobi reduces modulo N first, so its cost tracks W's size while the comparisons do not. Measured at 38 ms against 2.5 ms for an 8 MB W, and the new test asserts size-independence rather than an absolute time -- an absolute threshold loose enough to be stable also swallows the difference. It goes red on the previous ordering. - The four DeCommit call sites whose expected length is a function of the threshold check the part count before calling, because DeCommit hashes every part it is given. Measured at 458 ns for 7 parts against 6.6 ms for 200000. These are exactly the four NonEmptyMultiBytes call sites in the tree that pass no expected length, and they cannot: the length is (t+1)*2+1 and the message layer does not know t. A test pins that, so a later reader cannot conclude from the message layer alone that the count is bounded upstream. BOUND AT THE MESSAGE LAYER where a constant does exist. DGRound1Message's ssid and session_nonce_hash had no upper bound on either curve. Both are produced as SHA512_256i(...).Bytes(), so 32 is the width a conforming sender reaches, not a number picked to look safe. The ssid's emptiness is deliberately still round 2's to refuse, so that an empty declaration is reported against its sender instead of being dropped here with no attribution. STOP AN ABORTED PARTY. BaseUpdate had no latch: a party that reported an abort kept storing messages and re-entering rounds, walking into rounds whose predecessor never filled the slots they read. Those reads are ordinary indexing of pre-allocated slices holding nil, so it faults instead of reporting -- with no error, no round and no culprit. The abort itself was correct one message earlier; what was lost is everything after it. Latched on errors out of round Update and Start, which have written state, and NOT on the rejections above them: a message that failed ValidateMessage or StoreMessage was refused before anything was written, and one bad message from one peer must not end a party. The refusal names nobody, deliberately -- the abort named whoever was responsible once already, and re-naming per delivered message lets the pump rate decide how guilty a peer looks to a host that counts. GUARD TWO MORE PROMOTED FIELDS, the shape encoding/json and a shallow copy produce. PartyID.String() answers with a marker instead of faulting: it is the method called while something is already being diagnosed, and a direct call is what took the process, since fmt recovers a panicking String. Both BuildLocalSaveDataSubset implementations attribute the fault by name, matching the named panic they already raise one line later for a roster entry they cannot resolve -- neither function has a return channel to use instead. CORRECT A FALSE COMMENT. ecdsa/keygen/round_3.go called the Paillier ModProof "the only check that proves N is a true biprime (no small factors)". Neither half holds: absence of small factors is FacProof's statement, verified unconditionally in the same handler since the NoProofFac switch was removed, and ProofMod attests Blum-integer shape because N is its only statement input. Exclusivity is not a claim a comment can support about a codebase it does not enumerate. Every fix has a negative control alongside it. Full suite green, 18 packages. Co-Authored-By: Claude Code <noreply@anthropic.com> | 1 个月前 | |
tss: guard SortedPartyIDs.Keys too, rather than fixing only its two neighbours The previous commit fixed the two sites that spelled the check as `id.KeyInt() == nil`. Keys() has the same defect and was missed by that pass precisely because it has no check at all to grep for: it goes straight to `ids[i] = pid.KeyInt()`, so a nil embedded pointer faults there. It is reachable without SortPartyIDs having screened anything: SortedPartyIDs is an exported slice type and NewPeerContext accepts one as-is, so callers can and do build the slice directly. It panics rather than substituting a value, because there is no safe substitute. The slice is positional, so a placeholder would have to be a number, and 0 is the one value that must never appear -- a party whose key is 0 mod q would receive the Shamir secret itself as its share. An attributable panic is the only honest outcome, and it matches what SortPartyIDs does. Tests cover both directions: the malformed entry must produce Keys's own error, and well-formed input must still return the keys, so the change cannot pass by panicking unconditionally. Verified against the unguarded version: the first assertion fails there. Co-Authored-By: Claude Code <noreply@anthropic.com> | 1 个月前 | |
crypto, tss: guard the nil-able thing that is actually dereferenced, not its container Four sites where a nil guard protects an outer pointer (or nothing at all) while the value the code goes on to dereference is reachable through it, nil-able, and unchecked. Each was reachable from exported API and each faulted with a bare nil dereference rather than saying what was wrong. crypto/mta ProofBobWC.Bytes read parts 10 and 11 through pf.ProofBob and pf.U without establishing either; ProofBob.Bytes read ten *big.Int fields without establishing any. Both now require the type's own ValidateBasic. They panic rather than substituting a value: the return type is a fixed array with no error channel, and an empty slice in place of a missing field would emit a proof that looks serialisable and is not. crypto ECPoint X and Y dereferenced p.coords[i] with no check on the receiver or the coordinate -- and a nil coordinate is producible through the exported NewECPointNoCurveCheck, which stores whatever it is given. The existing guards live in the callers (Add, ScalarMult, Equals) and cover the outer pointer only. X/Y now panic attributably, because returning nil merely relocates the same fault to the caller's Cmp or Bytes. Where a return channel exists it is used instead: IsOnCurve answers false for a nil point, a nil curve (a direct field of interface type, nil-able even on a non-nil point) or nil coordinates; GobEncode returns an error; Equals returns false rather than panicking through X/Y. tss NewMessageWrapper checked `routing.To != nil` and then dereferenced routing.To[i]; routing.From had no check at all. Both are now tested. This panics rather than skipping or emitting a nil entry: skipping would silently shrink the recipient list, and a nil entry produces a message the receiver is guaranteed to reject with no indication why. tss IsOldCommittee / IsNewCommittee dereferenced rgParams.partyID and the PeerContext, neither of which is validated at construction -- NewParameters never inspects partyID and stores a nil context unconditionally, and the resharing constructor calls IsOldCommittee before any round runs, so BaseStart's validation is too late. PeerContext.IDs is now nil-receiver safe, which matches the convention the file already documents and fixes every reader at once; the committee test skips roster entries whose key cannot be read and answers false for a self PartyID that has none. Key comparison moved to canonical hex via the existing partyKeyID helper, which is equality on the same integers. Tests cover both directions for each site: the malformed input must produce the intended outcome, and well-formed input must behave exactly as before, so none of these can pass by refusing everything. Verified against the unguarded versions: every new assertion fails there with the predicted nil dereference, while the negative controls stay green. Co-Authored-By: Claude Code <noreply@anthropic.com> | 1 个月前 | |
tss, crypto, keygen, resharing: complete six checks that were unrated and unfixed Six items carried in the audit ledger as neither rated nor fixed, each with a bound that could be derived rather than chosen. Grouped because four of them are the same shape: something a peer declares decides how much work this library does, and the check that would refuse it ran afterwards. BOUND BEFORE WORK, not after. Two places, identical accept sets before and after -- only the order changed, so no existing test moves. - crypto/modproof Verify tests pf.W's range before isQuadraticResidue. W arrives unbounded (NewProofFromBytes counts parts and never measures one; KGRound2Message2.ValidateBasic does not look at the proof), and big.Jacobi reduces modulo N first, so its cost tracks W's size while the comparisons do not. Measured at 38 ms against 2.5 ms for an 8 MB W, and the new test asserts size-independence rather than an absolute time -- an absolute threshold loose enough to be stable also swallows the difference. It goes red on the previous ordering. - The four DeCommit call sites whose expected length is a function of the threshold check the part count before calling, because DeCommit hashes every part it is given. Measured at 458 ns for 7 parts against 6.6 ms for 200000. These are exactly the four NonEmptyMultiBytes call sites in the tree that pass no expected length, and they cannot: the length is (t+1)*2+1 and the message layer does not know t. A test pins that, so a later reader cannot conclude from the message layer alone that the count is bounded upstream. BOUND AT THE MESSAGE LAYER where a constant does exist. DGRound1Message's ssid and session_nonce_hash had no upper bound on either curve. Both are produced as SHA512_256i(...).Bytes(), so 32 is the width a conforming sender reaches, not a number picked to look safe. The ssid's emptiness is deliberately still round 2's to refuse, so that an empty declaration is reported against its sender instead of being dropped here with no attribution. STOP AN ABORTED PARTY. BaseUpdate had no latch: a party that reported an abort kept storing messages and re-entering rounds, walking into rounds whose predecessor never filled the slots they read. Those reads are ordinary indexing of pre-allocated slices holding nil, so it faults instead of reporting -- with no error, no round and no culprit. The abort itself was correct one message earlier; what was lost is everything after it. Latched on errors out of round Update and Start, which have written state, and NOT on the rejections above them: a message that failed ValidateMessage or StoreMessage was refused before anything was written, and one bad message from one peer must not end a party. The refusal names nobody, deliberately -- the abort named whoever was responsible once already, and re-naming per delivered message lets the pump rate decide how guilty a peer looks to a host that counts. GUARD TWO MORE PROMOTED FIELDS, the shape encoding/json and a shallow copy produce. PartyID.String() answers with a marker instead of faulting: it is the method called while something is already being diagnosed, and a direct call is what took the process, since fmt recovers a panicking String. Both BuildLocalSaveDataSubset implementations attribute the fault by name, matching the named panic they already raise one line later for a roster entry they cannot resolve -- neither function has a return channel to use instead. CORRECT A FALSE COMMENT. ecdsa/keygen/round_3.go called the Paillier ModProof "the only check that proves N is a true biprime (no small factors)". Neither half holds: absence of small factors is FacProof's statement, verified unconditionally in the same handler since the NoProofFac switch was removed, and ProofMod attests Blum-integer shape because N is its only statement input. Exclusivity is not a claim a comment can support about a codebase it does not enumerate. Every fix has a negative control alongside it. Full suite green, 18 packages. Co-Authored-By: Claude Code <noreply@anthropic.com> | 1 个月前 | |
tss: guard the embedded *Parameters, not only the PartyID inside it ReSharingParameters embeds *Parameters, so OldParties, OldPartyCount and the two committee predicates all read promoted fields and methods. Nothing sets that pointer for a value the caller did not build with NewReSharingParameters: ReSharingParameters{} and json.Unmarshal("{}") both leave it nil, and every promoted read then faults. That includes IsOldCommittee and IsNewCommittee. The previous pass guarded them against a PartyID whose key cannot be read, which is a different nil-able thing one level in; the pointer the promotion itself dereferences was still unguarded, so the panic they were meant to remove was still there for the zero value. Each reader now answers the way the package already answers for an undescribed committee: no roster, as PeerContext.IDs does for a nil context, and a count of zero. Values built by the constructor behave exactly as before, nil PeerContexts included. Co-Authored-By: Claude Code <noreply@anthropic.com> | 1 个月前 | |
Add copyright (#58) * add copyright comments * add .idea files for copyright | 6 年前 | |
tss: reject a malformed sender PartyID instead of faulting on it PartyID embeds *MessageWrapper_PartyID, so pid.Key is a promoted field: reading it also dereferences the embedded pointer. ValidateBasic tested only the outer pointer, so a PartyID whose outer pointer is non-nil and whose embedded pointer is nil faulted inside the very predicate meant to reject it. That shape is not exotic -- encoding/json produces it from {"index":n}, and so does a partial copy. ValidateBasic now tests the embedded pointer before the promoted read. ParseWireMessage had the same ordering one step earlier: it assigned from.MessageWrapper_PartyID before any validation ran. It now returns an error naming the missing quantity. This is a caller-contract fix, not a wire-facing one. The sender is not carried on the wire -- the only production proto.Unmarshal targets the inner Any, and two different senders produce byte-identical wire bytes -- so no byte sequence reaching the library produces either shape. They come from host code. ParseWireMessage's accept set narrows by exactly the embedded-nil shapes; every sender with a non-nil embedded pointer still parses, and ValidateBasic's truth table is otherwise unchanged, a non-nil zero-length key included. Wire format unchanged and go test ./... is green. Scope: this does not make tss nil-safe and does not enforce the caller contract generally. The outbound path is untouched -- NewMessageWrapper still faults on a nil sender -- and From.Index is not bound to From.Key anywhere in this library. Co-Authored-By: Claude Code <noreply@anthropic.com> | 1 个月前 |
| 文件 | 最后提交记录 | 最后更新时间 |
|---|---|---|
| 1 个月前 | ||
| 4 个月前 | ||
| 6 年前 | ||
| 1 个月前 | ||
| 4 个月前 | ||
| 1 个月前 | ||
| 24 天前 | ||
| 4 个月前 | ||
| 1 个月前 | ||
| 1 个月前 | ||
| 1 个月前 | ||
| 1 个月前 | ||
| 1 个月前 | ||
| 1 个月前 | ||
| 6 年前 | ||
| 1 个月前 |