Repository navigation
Fix bignum.sub underflow and selftests resetting the acceleration flag - #19
Conversation
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.
There was a problem hiding this comment.
Both fixes are correct and the tests now pin them.
sub_rawwidth check: zero is{}andnormalizestrips high zero limbs, so for canonical inputs#b > #adoes meanb > a.mod_sub's calls always havera >= rb, so they can't hit it.openssl_wrapper.selftest()already snapshots and restores_use_openssl, so its newuse(true)/use(false)test can't trip the new check incrypto.selftest().
Checked locally at 73edb20:
./run_tests.shpasses 16/16 withCRYPTO_USE_OPENSSLset and unset.- Putting the env-default restore back in
random.selftest()orbignum.selftest()fails the run under both settings, and the message names the right module (crypto.random/crypto.bignum). - Removing the
#b > #aguard 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.
There was a problem hiding this comment.
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.
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.subreturned wrong values instead of raisingsub_rawonly read the subtrahend up to the minuend's width. A widerbwhose low limbs happened not to borrow came back as a plausible wrong answer:sub_rawnow raises when#b > #a. Both operands are canonical, so that alone provesb > a. Internal callers (mod_sub) always passra >= rband 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()andrandom.selftest()restored the acceleration flag to theCRYPTO_USE_OPENSSLenvironment default, not to the caller's setting. On Control4 that variable is never set. A driver that calledcrypto.use_openssl(true)and thencrypto.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.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 checkclean./run_tests.shpasses (16/16) on LuaJIT with no binding, and on Lua 5.4 with the real lua-openssl 0.8.5 rock, withCRYPTO_USE_OPENSSLboth set and unset