perf(keys): use binary GCD in the batch-GCD shared-prime pass - #20
Merged
Merged
Conversation
The cross-file batch-GCD check runs a pairwise BigUint::gcd over every RSA modulus in the tree. The O(n^2) loop is capped and expected; the cost was per-gcd. BigUint::gcd was Euclidean (a % b), and operator% goes through the bit-by-bit long-division divmod, which iterates over every bit of the dividend, allocates fresh vectors per bit, and computes a quotient it then discards. A single 2048-bit gcd cost ~37ms, so a rootfs shipping a full CA certificate store (hundreds of RSA CA certs) turned the pass into a multi-minute hang. Replace it with Stein's binary GCD (shifts and subtraction, no division), keeping divmod off the hot path. Same O(bits^2) worst case, but without the per-bit allocation churn: the --keys pass on such a tree drops from 6+ minutes to ~5s, and the remaining time is the now-cheap O(n^2) loop. Correctness is unchanged: the bigint gcd KATs and the shared-prime recovery fixtures still pass, and the detector still recovers real shared factors. Closes #19
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.
Summary
Fixes the
--keyspass hanging for minutes on a firmware rootfs that ships afull CA certificate store. Closes #19.
The cross-file batch-GCD shared-prime check runs a pairwise
BigUint::gcdoverevery RSA modulus in the tree. The O(n^2) loop is capped (256 keys) and
expected; the cost was per-gcd.
BigUint::gcdwas Euclidean (a % b), andoperator%routes through the bit-by-bit long-divisiondivmod, which iteratesover every bit of the dividend, allocates fresh vectors per bit, and computes a
quotient it then discards. A single 2048-bit gcd cost ~37 ms, so a standard
/etc/ssl/certsbundle (hundreds of RSA CA certs) produced enough moduli toturn the pass into a multi-minute hang.
Change
Replace the Euclidean
BigUint::gcdwith Stein's binary GCD (shifts andsubtraction only, no division), keeping
divmodoff the hot path.This is a constant-factor fix, not an asymptotic one — both forms are
O(bits^2) worst case. The win is removing the per-bit allocation churn that made
each gcd pathologically slow. The O(n^2) pairwise loop and its key-count / bit
caps are unchanged, so the DoS bound on a hostile tree is unaffected.
Effect
The
--keyspass on a rootfs with a full CA store drops from 6+ minutes to~5s; the remaining time is the now-cheap O(n^2) loop. Quadratic shape before the
fix, for reference (whole
--keyspass over N certs):Testing
Fermat / batch-GCD recovery KATs.
tests/run.shintegration suite green, including the shared-prime recoveryfixture — the detector still recovers real shared factors.
only the expected weak-modulus findings, with no spurious shared-prime hits.
Follow-up (not in this PR)
If the O(n^2) loop ever needs to scale to very large key counts, a product-tree
batch-GCD gives near-linear big-integer work. That is a larger change and not
needed to fix this hang.