Skip to content

Fix bignum.sub underflow and selftests resetting the acceleration flag - #19

Merged
derek-miller merged 2 commits into
mainfrom
claude/bignum-sub-and-selftest-flag
Oct 18, 2026
Merged

derek-miller merged 2 commits into
mainfrom
claude/bignum-sub-and-selftest-flag

Conversation

@derek-miller

Copy link
Copy Markdown
Contributor

Two pre-existing bugs, found while reviewing the bignum OpenSSL routing work. They are split out so they stay separate from it after a squash merge.

bignum.sub returned wrong values instead of raising

sub_raw only read the subtrahend up to the minuend's width. A wider b whose low limbs happened not to borrow came back as a plausible wrong answer:

bignum.sub({5}, {0, 1})                                       -- returned 5
bignum.sub(bignum.from_hex("ffffff"), bignum.from_hex("1000000")) -- returned ffffff

sub_raw now raises when #b > #a. Both operands are canonical, so that alone proves b > a. Internal callers (mod_sub) always pass ra >= rb and were never affected. The existing test passed only by borrow luck, so the tests now cover a wider subtrahend that does not borrow, plus equal-width underflow.

Selftests switched OpenSSL acceleration off for the caller

bignum.selftest() and random.selftest() restored the acceleration flag to the CRYPTO_USE_OPENSSL environment default, not to the caller's setting. On Control4 that variable is never set. A driver that called crypto.use_openssl(true) and then crypto.selftest() was left unaccelerated, which makes SRP unusable there: pure-Lua 3072-bit modexp takes minutes.

  • openssl_wrapper.enabled() exposes the current flag. The selftests snapshot it and hand it back.
  • crypto.selftest() fails any module whose selftest leaves the flag changed, naming the module.
  • The runner's lpack pass calls crypto.selftest() with the flag flipped from the environment default. That is how the check gets exercised, and every run now covers both settings at no extra runtime.

Test plan

  • make check clean
  • ./run_tests.sh passes (16/16) on LuaJIT with no binding, and on Lua 5.4 with the real lua-openssl 0.8.5 rock, with CRYPTO_USE_OPENSSL both set and unset
  • Reverting either selftest restore to the env default makes the lpack pass fail, naming the module
  • CI

bignum.sub returned a wrong value instead of raising when the subtrahend had
more limbs than the minuend: sub_raw only read b to a's width, so a wider b
whose low limbs happened not to borrow slipped through. It now raises on a
wider subtrahend, and the tests cover that case and equal-width underflow.

The bignum and random selftests restored the acceleration flag to the
CRYPTO_USE_OPENSSL default rather than the caller's setting, so a driver that
called crypto.use_openssl(true) and then crypto.selftest() was left
unaccelerated, which on Control4 makes SRP unusable. openssl_wrapper.enabled()
exposes the flag so they can hand the caller's choice back, crypto.selftest()
fails any module that leaves it changed, and the runner's lpack pass now runs
crypto.selftest() with the flag flipped from the environment default.
svc-finitelabs[bot]
svc-finitelabs Bot previously approved these changes Oct 17, 2026

@svc-finitelabs svc-finitelabs Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both fixes are correct and the tests now pin them.

  • sub_raw width check: zero is {} and normalize strips high zero limbs, so for canonical inputs #b > #a does mean b > a. mod_sub's calls always have ra >= rb, so they can't hit it.
  • openssl_wrapper.selftest() already snapshots and restores _use_openssl, so its new use(true)/use(false) test can't trip the new check in crypto.selftest().

Checked locally at 73edb20:

  • ./run_tests.sh passes 16/16 with CRYPTO_USE_OPENSSL set and unset.
  • Putting the env-default restore back in random.selftest() or bignum.selftest() fails the run under both settings, and the message names the right module (crypto.random / crypto.bignum).
  • Removing the #b > #a guard fails "sub - rejects a wider subtrahend whose low limbs do not borrow".
  • CI is green on every matrix entry, including lua-openssl 0.8.5.

Non-blocking: bignum.sub is public, so a hand-built operand with high zero limbs (sub({5}, {3, 0})) now raises where it used to return 2. Nothing in the library makes such an operand, and the comment says canonical inputs are required, so I think raising is fine.

@svc-finitelabs svc-finitelabs Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new commit only touches comments and docs. No executable line changed in bignum.lua, openssl_wrapper.lua, random.lua or run_tests.sh. The shortened comments still say why #b > #a is safe to reject and why enabled() returns truthy rather than == true. The "host left unaccelerated" reasoning now lives only in the CLAUDE.md paragraph, and it is still accurate there. CI is green at this head.

@derek-miller
derek-miller merged commit abf35a7 into main Oct 18, 2026
12 checks passed
@derek-miller
derek-miller deleted the claude/bignum-sub-and-selftest-flag branch October 18, 2026 01:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant