From 73edb2047d9e15bd0aab3d3791cdf3218b405ba2 Mon Sep 17 00:00:00 2001 From: Derek Miller <1340081+derek-miller@users.noreply.github.com> Date: Sat, 17 Oct 2026 18:29:03 -0500 Subject: [PATCH 1/2] Fix bignum.sub underflow and selftests resetting the acceleration flag 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. --- CLAUDE.md | 8 ++++++++ run_tests.sh | 8 ++++++-- src/crypto/bignum.lua | 27 +++++++++++++++++++++++++-- src/crypto/init.lua | 6 ++++++ src/crypto/openssl_wrapper.lua | 20 ++++++++++++++++++++ src/crypto/random.lua | 8 +++++--- 6 files changed, 70 insertions(+), 7 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 485c997..9a5d07d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -327,6 +327,14 @@ vectors). The runner invokes `require("crypto.").selftest()` per module: make test-matrix # across Lua 5.1–5.4 + LuaJIT ``` +`crypto.selftest()` fails any module whose selftest leaves the acceleration +flag changed, because a driver that enables acceleration and then runs it would +otherwise be left unaccelerated, which on Control4 makes SRP unusable. A +selftest that toggles the flag snapshots `openssl_wrapper.enabled()` first and +hands that back, not the `CRYPTO_USE_OPENSSL` default. The runner's last pass +(lpack) calls `crypto.selftest()` with the flag flipped from the environment +default, so every run covers both settings. + ## Building `amalg` produces two single-file distributions from `src/crypto/init.lua`: diff --git a/run_tests.sh b/run_tests.sh index fd7554a..b2b3d59 100755 --- a/run_tests.sh +++ b/run_tests.sh @@ -148,10 +148,14 @@ run_selftest "Ed25519" "ed25519" "crypto.ed25519" run_selftest "OpenSSL gating" "openssl_wrapper" "crypto.openssl_wrapper" # Control4's LuaJIT has string.pack/unpack as lpack, a different dialect: every -# selftest again with that shape installed before bitn loads. +# selftest again with that shape installed before bitn loads, through +# crypto.selftest() as a host would call it, and with the acceleration flag on +# the opposite setting from the passes above. run_test "All selftests with lpack-shaped string.pack" "lpack" " dofile('$script_dir/test/lpack_stub.lua') - if not require('crypto').selftest() then + local crypto = require('crypto') + crypto.use_openssl(not crypto.openssl_wrapper.enabled()) + if not crypto.selftest() then os.exit(1) end " diff --git a/src/crypto/bignum.lua b/src/crypto/bignum.lua index d48a37c..4c51ae9 100644 --- a/src/crypto/bignum.lua +++ b/src/crypto/bignum.lua @@ -177,6 +177,12 @@ end --- @param b BigNum Subtrahend (must not exceed a) --- @return BigNum difference local function sub_raw(a, b) + -- The loop below reads only a's width of b, so a wider b would otherwise + -- slip through whenever its low limbs happen not to borrow. Both operands + -- are canonical, so a wider b is a larger b. + if #b > #a then + error("bignum: subtraction would be negative") + end local r = {} local borrow = 0 local na = #a @@ -1204,6 +1210,7 @@ function bignum.selftest() -- Snapshot the OpenSSL state so the accelerator tests cannot leak. local saved_loaded = package.loaded["openssl"] local saved_preload = package.preload["openssl"] + local saved_use = openssl_wrapper.enabled() --- Build a stand-in lua-openssl binding. Real lua-openssl is not installed --- here, so this validates the *routing* and the bytes-in/hex-out conversion, @@ -1437,7 +1444,21 @@ function bignum.selftest() { name = "sub - rejects a negative result", test = function() + -- Equal widths too: only the final borrow can catch those. return pcall(bignum.sub, Y, X) == false + and pcall(bignum.sub, from_number(5), from_number(7)) == false + and pcall(bignum.sub, Y, bignum.add(Y, bignum.one())) == false + end, + }, + { + -- Neither subtrahend's low limbs borrow against the minuend, so only the + -- width check can see that the result would be negative. + name = "sub - rejects a wider subtrahend whose low limbs do not borrow", + test = function() + return pcall(bignum.sub, from_number(5), from_hex("1000000")) == false + and pcall(bignum.sub, from_hex("ffffff"), from_hex("1000000")) == false + and pcall(bignum.sub, X, bignum.add(X, from_hex("1" .. string_rep("0", 66)))) == false + and to_hex(bignum.sub(from_hex("1000000"), from_hex("ffffff"))) == "1" end, }, { @@ -1756,10 +1777,12 @@ function bignum.selftest() end end - -- Restore the module-load default so later tests in this process are unaffected. + -- Restore the caller's state, acceleration choice included, so later tests + -- in this process are unaffected and a host that called + -- crypto.use_openssl(true) is not left unaccelerated. package.loaded["openssl"] = saved_loaded package.preload["openssl"] = saved_preload - openssl_wrapper.use(os.getenv("CRYPTO_USE_OPENSSL") == "1" or os.getenv("CRYPTO_USE_OPENSSL") == "true") + openssl_wrapper.use(saved_use) print(string_format("\nBignum result: %d/%d tests passed\n", passed, #tests)) return passed == #tests diff --git a/src/crypto/init.lua b/src/crypto/init.lua index b7ad931..c3d00b8 100644 --- a/src/crypto/init.lua +++ b/src/crypto/init.lua @@ -126,9 +126,15 @@ function crypto.selftest() for _, name in ipairs(modules) do local mod = crypto[name] if type(mod.selftest) == "function" then + local accelerated = openssl_wrapper.enabled() if mod.selftest() == false then ok = false end + -- A host that enabled acceleration and then ran this must still have it. + if openssl_wrapper.enabled() ~= accelerated then + print("crypto." .. name .. ".selftest() did not restore the OpenSSL acceleration flag") + ok = false + end end end return ok diff --git a/src/crypto/openssl_wrapper.lua b/src/crypto/openssl_wrapper.lua index a43501f..4d861e5 100644 --- a/src/crypto/openssl_wrapper.lua +++ b/src/crypto/openssl_wrapper.lua @@ -172,6 +172,16 @@ function openssl_wrapper.use(use) _openssl_unavailable = false end +--- Whether OpenSSL acceleration is currently requested, as last set by `use`. +--- Code that toggles the flag temporarily, such as a selftest, reads this first +--- so it can hand the caller's choice back rather than the environment default. +--- @return boolean enabled +function openssl_wrapper.enabled() + -- Truthy, not `== true`: `get` treats any truthy flag as enabled, and a + -- selftest restoring this answer must not switch acceleration off. + return _use_openssl and true or false +end + --- Parse semantic version string into comparable components --- @param version_str string Version string like "0.9.1" or "1.0.0-rc1" --- @return number major Major version number @@ -493,6 +503,16 @@ function openssl_wrapper.selftest() return openssl_wrapper.get(OpenSSLFeature.AAD) == nil end, }, + { + name = "enabled reports what use last set", + test = function() + openssl_wrapper.use(true) + local on = openssl_wrapper.enabled() + openssl_wrapper.use(false) + local off = openssl_wrapper.enabled() + return on == true and off == false + end, + }, { name = "absent binding falls back gracefully", test = function() diff --git a/src/crypto/random.lua b/src/crypto/random.lua index 081b3c2..67f387e 100644 --- a/src/crypto/random.lua +++ b/src/crypto/random.lua @@ -218,6 +218,7 @@ function random.selftest() local saved_path = random._urandom_path local saved_preload = package.preload["openssl"] local saved_loaded = package.loaded["openssl"] + local saved_use = openssl_wrapper.enabled() --- Force both built-in sources to be unavailable. local function starve() @@ -233,13 +234,14 @@ function random.selftest() random._urandom_path = "/nonexistent/crypto-random-selftest" end - --- Restore real detection, including the wrapper's initial opt-in state, so - --- running this inside `crypto.selftest()` cannot disturb a later module. + --- Restore real detection, including the caller's acceleration choice, so + --- running this inside `crypto.selftest()` cannot disturb a later module or + --- leave a host that called `crypto.use_openssl(true)` unaccelerated. local function unstarve() random.reset() package.loaded["openssl"] = saved_loaded package.preload["openssl"] = saved_preload - openssl_wrapper.use(os.getenv("CRYPTO_USE_OPENSSL") == "1" or os.getenv("CRYPTO_USE_OPENSSL") == "true") + openssl_wrapper.use(saved_use) random._urandom_path = saved_path end From da6661752210d73795c968ded7a39df151c8539a Mon Sep 17 00:00:00 2001 From: Derek Miller <1340081+derek-miller@users.noreply.github.com> Date: Sat, 17 Oct 2026 19:41:14 -0500 Subject: [PATCH 2/2] Trim comments and docs to what a maintainer needs --- CLAUDE.md | 11 +++++------ run_tests.sh | 5 ++--- src/crypto/bignum.lua | 10 ++-------- src/crypto/openssl_wrapper.lua | 7 ++----- src/crypto/random.lua | 3 +-- 5 files changed, 12 insertions(+), 24 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 9a5d07d..7c07e14 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -328,12 +328,11 @@ make test-matrix # across Lua 5.1–5.4 + LuaJIT ``` `crypto.selftest()` fails any module whose selftest leaves the acceleration -flag changed, because a driver that enables acceleration and then runs it would -otherwise be left unaccelerated, which on Control4 makes SRP unusable. A -selftest that toggles the flag snapshots `openssl_wrapper.enabled()` first and -hands that back, not the `CRYPTO_USE_OPENSSL` default. The runner's last pass -(lpack) calls `crypto.selftest()` with the flag flipped from the environment -default, so every run covers both settings. +flag changed: a driver that enabled acceleration and then ran it would be left +unaccelerated, which on Control4 makes SRP unusable. A selftest that toggles the +flag restores what `openssl_wrapper.enabled()` returned, not the +`CRYPTO_USE_OPENSSL` default. The runner's lpack pass flips the flag, so every +run covers both settings. ## Building diff --git a/run_tests.sh b/run_tests.sh index b2b3d59..88160b5 100755 --- a/run_tests.sh +++ b/run_tests.sh @@ -148,9 +148,8 @@ run_selftest "Ed25519" "ed25519" "crypto.ed25519" run_selftest "OpenSSL gating" "openssl_wrapper" "crypto.openssl_wrapper" # Control4's LuaJIT has string.pack/unpack as lpack, a different dialect: every -# selftest again with that shape installed before bitn loads, through -# crypto.selftest() as a host would call it, and with the acceleration flag on -# the opposite setting from the passes above. +# selftest again with that shape installed before bitn loads, and with the +# acceleration flag flipped so every run covers both settings. run_test "All selftests with lpack-shaped string.pack" "lpack" " dofile('$script_dir/test/lpack_stub.lua') local crypto = require('crypto') diff --git a/src/crypto/bignum.lua b/src/crypto/bignum.lua index 4c51ae9..dd98911 100644 --- a/src/crypto/bignum.lua +++ b/src/crypto/bignum.lua @@ -177,9 +177,7 @@ end --- @param b BigNum Subtrahend (must not exceed a) --- @return BigNum difference local function sub_raw(a, b) - -- The loop below reads only a's width of b, so a wider b would otherwise - -- slip through whenever its low limbs happen not to borrow. Both operands - -- are canonical, so a wider b is a larger b. + -- The loop reads only #a limbs of b, and with canonical operands a wider b is a larger b. if #b > #a then error("bignum: subtraction would be negative") end @@ -1451,8 +1449,6 @@ function bignum.selftest() end, }, { - -- Neither subtrahend's low limbs borrow against the minuend, so only the - -- width check can see that the result would be negative. name = "sub - rejects a wider subtrahend whose low limbs do not borrow", test = function() return pcall(bignum.sub, from_number(5), from_hex("1000000")) == false @@ -1777,9 +1773,7 @@ function bignum.selftest() end end - -- Restore the caller's state, acceleration choice included, so later tests - -- in this process are unaffected and a host that called - -- crypto.use_openssl(true) is not left unaccelerated. + -- Restore the caller's state, acceleration choice included, so later tests in this process are unaffected. package.loaded["openssl"] = saved_loaded package.preload["openssl"] = saved_preload openssl_wrapper.use(saved_use) diff --git a/src/crypto/openssl_wrapper.lua b/src/crypto/openssl_wrapper.lua index 4d861e5..a3b8b78 100644 --- a/src/crypto/openssl_wrapper.lua +++ b/src/crypto/openssl_wrapper.lua @@ -172,13 +172,10 @@ function openssl_wrapper.use(use) _openssl_unavailable = false end ---- Whether OpenSSL acceleration is currently requested, as last set by `use`. ---- Code that toggles the flag temporarily, such as a selftest, reads this first ---- so it can hand the caller's choice back rather than the environment default. +--- Whether acceleration is requested, as last set by `use`. Snapshot it before toggling the flag temporarily. --- @return boolean enabled function openssl_wrapper.enabled() - -- Truthy, not `== true`: `get` treats any truthy flag as enabled, and a - -- selftest restoring this answer must not switch acceleration off. + -- Truthy like `get`, not `== true`, or restoring this answer could switch acceleration off. return _use_openssl and true or false end diff --git a/src/crypto/random.lua b/src/crypto/random.lua index 67f387e..5f0ad0d 100644 --- a/src/crypto/random.lua +++ b/src/crypto/random.lua @@ -235,8 +235,7 @@ function random.selftest() end --- Restore real detection, including the caller's acceleration choice, so - --- running this inside `crypto.selftest()` cannot disturb a later module or - --- leave a host that called `crypto.use_openssl(true)` unaccelerated. + --- running this inside `crypto.selftest()` cannot disturb a later module. local function unstarve() random.reset() package.loaded["openssl"] = saved_loaded