security: address findings from full security audit#2
Merged
Conversation
Implements fixes for the High-severity and clear-cut Medium-severity
defects surfaced by the security audit across dklstss, eddsa/eddsatss,
frosttss, and shared crypto primitives. Each fix lands with a regression
test exercising the previously-vulnerable path.
High severity
-------------
H-1 crypto/ot/otext/extend.go: ExtSender.Extend branched on `deltaBit`
(the long-term Δ secret) when XOR-ing msg.U[j] into the PRG
expansion. Replace the if-branch with a byte-level CT XOR mask
(`expansion[b] ^ (msg.U[j][b] & mask)`). Without this fix, a
side-channel observer who can drive repeated Extend calls recovers
Δ bit by bit, which in DKLs23 is equivalent to threshold-key
extraction.
H-2 eddsatss/signing.go, eddsa/signing/round_1.go, frosttss/signing.go:
per-party EdDSA / FROST signing nonces (ri, di, ei) had their
base-point scalar multiplications routed through Go's standard
`crypto.ScalarBaseMult`, which is not constant-time on the Edwards
curve. Route through the new `crypto.CTScalarBaseMultEd25519`
helper backed by `edwards25519.GeScalarMultBase` (CT fixed-base
table). Nonce leakage via timing is the textbook hidden-number
attack against EdDSA / FROST.
Medium severity (selected)
--------------------------
* crypto/ot/otext/transpose.go: drop the `if b == 0 { continue }` fast
path; process every byte and bit uniformly. The shortcut was an
observable data-dependent branch on the q matrix (which carries Δ
through `expansion ⊕ U`).
* crypto/schnorr/schnorr_proof.go: `ZKProof.ValidateBasic` now calls
`Alpha.ValidateBasic()`. Previously a ZKProof with non-nil Alpha but
nil coordinates would panic inside Verify on `Alpha.X()`.
* crypto/ctmul/ctmul.go: `ScalarMultWithRand` on RNG failure now
panics instead of silently downgrading to the non-CT primitive — a
silent downgrade defeated the entire CT guarantee.
* dklstss/lagrange.go: new `hashToScalar` helper applies SEC 1 §4.1.3
leftmost-q.BitLen()-bits truncation before mod q. Callsites in
signing.go, signing_checked.go, presign.go, signing_party.go updated.
Previously, a 64-byte digest (SHA-512) signed via dklstss produced
a signature that did not verify under crypto/ecdsa.Verify.
* dklstss/refresh_party.go: `evalCommitmentSumZeroConst` now propagates
ECPoint.Add errors instead of silently dropping terms (previous
`if err == nil { acc = sum }` swallowed protocol-violation evidence).
* dklstss/serialize.go: `Load` now cross-checks ChainCode against
deriveChainCode(ECDSAPub) and ECDSAPub against Σ λ_j · BigXj[j]. A
key tampered on disk (e.g. ECDSAPub swapped) is rejected.
* dklstss/setup.go: replace `byte(i), byte(j)` truncation in OT-pair
sids with a 9-byte (i:uint32-BE, j:uint32-BE, dir:byte) encoding.
The truncated form would have collided across pairs for n > 256.
* dklstss/keygen_party.go, refresh_party.go, resharing_party.go:
reject non-canonical (>= q) round-1 shares to close an echo-bypass
channel (the VSS Verify path silently accepts share + k·q).
* dklstss/signing_party.go, presign_party.go: enforce
`validateSortedSubset` so a caller passing an unsorted subset
cannot end up in a state where parties absorb the HD tweak at
different slots (producing a non-verifying signature).
Low severity
------------
* common/random.go: `GetRandomPositiveInt` actually rejects zero
(matches its name).
* crypto/ot/ole/mul.go: `bitsLE` uses unconditional shift-OR instead
of an if-branch on `v.Bit(i)`.
Deferred (require larger design changes)
----------------------------------------
* H-3 (`dklstss.Key.Save` AEAD-at-rest) and `ssidNonce` in eddsa/eddsatss
randomization (would require a coin-flip round so all parties agree
on the nonce — see explanatory comments left in keygen.go / signing.go).
Other Low-severity items (variable-length big.Int sid encodings; non-CT
big.Int.Mul in z_i / t computations; per-channel multi-writer guards)
are tracked but not addressed in this patch.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements fixes for the High-severity and clear-cut Medium-severity defects surfaced by the security audit across dklstss, eddsa/eddsatss, frosttss, and shared crypto primitives. Each fix lands with a regression test exercising the previously-vulnerable path.
High severity
H-1 crypto/ot/otext/extend.go: ExtSender.Extend branched on
deltaBit(the long-term Δ secret) when XOR-ing msg.U[j] into the PRG
expansion. Replace the if-branch with a byte-level CT XOR mask
(
expansion[b] ^ (msg.U[j][b] & mask)). Without this fix, aside-channel observer who can drive repeated Extend calls recovers
Δ bit by bit, which in DKLs23 is equivalent to threshold-key
extraction.
H-2 eddsatss/signing.go, eddsa/signing/round_1.go, frosttss/signing.go:
per-party EdDSA / FROST signing nonces (ri, di, ei) had their
base-point scalar multiplications routed through Go's standard
crypto.ScalarBaseMult, which is not constant-time on the Edwardscurve. Route through the new
crypto.CTScalarBaseMultEd25519helper backed by
edwards25519.GeScalarMultBase(CT fixed-basetable). Nonce leakage via timing is the textbook hidden-number
attack against EdDSA / FROST.
Medium severity (selected)
crypto/ot/otext/transpose.go: drop the
if b == 0 { continue }fast path; process every byte and bit uniformly. The shortcut was an observable data-dependent branch on the q matrix (which carries Δ throughexpansion ⊕ U).crypto/schnorr/schnorr_proof.go:
ZKProof.ValidateBasicnow callsAlpha.ValidateBasic(). Previously a ZKProof with non-nil Alpha but nil coordinates would panic inside Verify onAlpha.X().crypto/ctmul/ctmul.go:
ScalarMultWithRandon RNG failure now panics instead of silently downgrading to the non-CT primitive — a silent downgrade defeated the entire CT guarantee.dklstss/lagrange.go: new
hashToScalarhelper applies SEC 1 §4.1.3 leftmost-q.BitLen()-bits truncation before mod q. Callsites in signing.go, signing_checked.go, presign.go, signing_party.go updated. Previously, a 64-byte digest (SHA-512) signed via dklstss produced a signature that did not verify under crypto/ecdsa.Verify.dklstss/refresh_party.go:
evalCommitmentSumZeroConstnow propagates ECPoint.Add errors instead of silently dropping terms (previousif err == nil { acc = sum }swallowed protocol-violation evidence).dklstss/serialize.go:
Loadnow cross-checks ChainCode against deriveChainCode(ECDSAPub) and ECDSAPub against Σ λ_j · BigXj[j]. A key tampered on disk (e.g. ECDSAPub swapped) is rejected.dklstss/setup.go: replace
byte(i), byte(j)truncation in OT-pair sids with a 9-byte (i:uint32-BE, j:uint32-BE, dir:byte) encoding. The truncated form would have collided across pairs for n > 256.dklstss/keygen_party.go, refresh_party.go, resharing_party.go: reject non-canonical (>= q) round-1 shares to close an echo-bypass channel (the VSS Verify path silently accepts share + k·q).
dklstss/signing_party.go, presign_party.go: enforce
validateSortedSubsetso a caller passing an unsorted subset cannot end up in a state where parties absorb the HD tweak at different slots (producing a non-verifying signature).Low severity
common/random.go:
GetRandomPositiveIntactually rejects zero (matches its name).crypto/ot/ole/mul.go:
bitsLEuses unconditional shift-OR instead of an if-branch onv.Bit(i).Deferred (require larger design changes)
dklstss.Key.SaveAEAD-at-rest) andssidNoncein eddsa/eddsatss randomization (would require a coin-flip round so all parties agree on the nonce — see explanatory comments left in keygen.go / signing.go). Other Low-severity items (variable-length big.Int sid encodings; non-CT big.Int.Mul in z_i / t computations; per-channel multi-writer guards) are tracked but not addressed in this patch.