Repository navigation
chore: re-vendor bitn.lua from lua-bitn v0.6.4 - #15
Conversation
Copied verbatim from the release asset. v0.6.4 binds string.pack/unpack only after probing for the 5.3 dialect, so the byte helpers this library now routes through fall back to byte math on a Control4 controller, whose lpack raised on them (FL-20).
|
Verified the re-vendor independently. The change is correct and I could not find a defect in it. Two notes below, neither blocking. What I checked
Behaviourally, on LuaJIT 2.1 with an lpack-shaped I also checked whether the exposure predates #14, since 1. The affected-module list is narrower than what actually breaks The description names
Worth getting right in the v0.2.4 release note, because per FL-20 the decode side does not raise, it returns a position as the value, so on a controller this is wrong digests and wrong ciphertext flowing onward rather than a crash. "sha256 and blake2 are affected" reads rather differently to a consumer than "chacha20 and aes_gcm are affected". Caveat on my own measurement: my stub raises on the decode path where real lpack is silent, so the list above establishes which modules are affected, not how they fail on hardware. 2. This class stays invisible to lua-crypto CI lua-bitn#23 added an lpack test module. lua-crypto has no equivalent, and no matrix leg presents the shape: 5.1, 5.2 and LuaJIT have no The check you already ran by hand is the thing worth capturing. It is roughly twenty lines: install an lpack-shaped One thing I could not verify locally: Not blocking, and I am not approving: the merge and tag decision is yours. |
The CI matrix has no interpreter with Control4's string.pack dialect, so a bitn re-vendor that lost the probe would land green. The stub is lua-bitn's, as measured on a controller; against the v0.6.3 vendor this module fails with unsupported code '4'.
|
Both notes taken in the PR rather than as follow-ups: the body now lists the nine affected modules and how the decode side fails, and the second commit adds the |
There was a problem hiding this comment.
Approving. Both notes from my last round are implemented, and I re-derived each rather than reading the diff for intent.
Correcting my own caveat first. Last round I wrote that my stub "raises on the decode path where real lpack is silent" and flagged that as a limit on my measurement. That caveat does not apply to the stub you shipped, because it is not the one I used. test/lpack_stub.lua here is byte-identical to lua-bitn v0.6.4's test/lpack_stub.lua (2399 bytes, md5 eab0b77bbb9326a6289212233931ad4c), and it reproduces both FL-20 failure modes rather than only the raise:
string.unpack('>I4', str, offset)where the data has a leading NUL byte, which is every big-endian u32 below 2^24, returns1silently. That is the NUL-ends-the-format path, and it matches FL-20's "reads back as 1" exactly.- Any other leading byte raises, as does the entire encode side (
unsupported code '4').
So the "shape measured on the dev controller" claim holds in both directions, and the instrument came from the repo where the shape was actually measured instead of being re-typed here.
The mutation check, re-run independently. 2x2 over {v0.6.4 vendor, v0.6.3 vendor} x {the lpack module, stock sha256}, on LuaJIT 2.1, Lua 5.1 and Lua 5.5:
| vendor | lpack module |
stock sha256 |
|---|---|---|
| v0.6.4 | PASS | PASS |
| v0.6.3 | FAIL, unsupported code '4' |
PASS |
The stock column passes on both sides, so the stub is what discriminates rather than the checkout. Your unsupported code '4' reproduces on all three.
The Lua 5.5 row is worth calling out because it is stronger than the body claims. The body motivates the module with "the CI matrix has no interpreter with that dialect", which is true, but the stub overwrites a correct native string.pack on a genuine 5.3-dialect interpreter and still discriminates. So this is real coverage on all six matrix legs, not only the four that lack string.pack.
It is actually reached in CI. make test-all has no explicit target, so it falls through the test-% pattern rule to ./run_tests.sh all, and all_modules now carries lpack. Confirmed by running it rather than reading it: 16/16, with "All selftests with lpack-shaped string.pack: PASS" in the summary. crypto.selftest() iterates all 15 modules, so the lpack arm is the full suite and not a subset.
Re-verified from the first commit: vendor/bitn.lua is untouched by the second commit and still byte-identical to the v0.6.4 release asset (118674 bytes, md5 ea9208120c88dd8931157658311ac94d). CI is 11/11 green on e274912.
On the nine-module list. It matches what I measured. The decode framing is right for this library specifically, which is worth stating because it does not transfer automatically: src/ reaches the decode direction in several places (sha256.lua:133 for every message-schedule word, blake2.lua:280 and :367, chacha20.lua:104 and :112), so a regressed probe here really does mean wrong digests flowing onward rather than a crash. A consumer that only calls the encode helpers would crash instead, so the severity is a property of the call sites, not of the helper list.
One non-blocking note, about durability rather than a defect.
lua-bitn's own lpack module carries an assertion this one omits:
if require('bitn._compat').string_pack ~= nil then
print('lpack-shaped string.pack was bound as the 5.3 dialect')
os.exit(1)
endHere a probe regression is caught only indirectly, through a selftest raising. Today that is airtight: all eight fast-path call sites in the vendored bitn use digit-bearing formats (>I4, <I4, >I4I4, <I4I4), every one of which raises under the stub. But that is a property of bitn's format strings rather than of the probe contract, and the failure surfaces as unsupported code '4' from inside sha256 instead of naming the cause.
It is available in the vendored single-file build, which I checked because I assumed it would not be: the amalgam registers package.preload["bitn._compat"], so the require resolves once bitn has been loaded, and it correctly reads nil under the stub. Ordering is the only constraint, the assertion has to sit after the require rather than before it.
Your call, and not worth a round on its own.
What I did not verify: make check locally. My luacheck install is broken under Lua 5.5 and fails identically at the base commit, so that is my environment and CI's green Check covers it. No hardware run either, the end-to-end stays on the template re-vendor PR as you have it.
The merge and the v0.2.4 tag are yours.
vendor/bitn.lua← lua-bitnv0.6.4release asset, copied verbatim and verified byte-identical to a local build of the tag.v0.6.4 is v0.6.3 plus lua-bitn#23 (FL-20):
string.pack/string.unpackare bound only after a load-time probe confirms the 5.3 dialect. On a Control4 controller they are lpack, so bitn v0.6.3's byte helpers raised on the encode side and returned a position as the value on the decode side. Since #14 pointedutils.bytesat those helpers, that reaches 9 of this library's 13 modules on v0.2.3:sha256,sha512,blake2,chacha20,chacha20_poly1305,aes_gcm, and through the hasheshkdf,srpanded25519(poly1305,bignum,x25519,x448are untouched). With v0.6.4 they take the byte-math path. No other change in bitn.Second commit:
run_tests.shgains anlpackmodule, in the default set, that installs lua-bitn's lpack-shapedstring.pack/unpack(the shape measured on the dev controller) before loading anything and runscrypto.selftest(). Against the v0.6.3 vendor it fails withunsupported code '4'; the CI matrix has no interpreter with that dialect, so without it a re-vendor that regressed the probe would land green.Verified:
make check; the full selftest suite on Lua 5.1, 5.4 and LuaJIT 2.1; thelpackmodule on all three;make build. Tagging v0.2.4 on merge; the hardware end-to-end goes on the template re-vendor PR.