sodium: incorporate the libsodium 1.0.21/1.0.22 hardenings (three items, not one)#167
Merged
Merged
Conversation
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.
Clears the
pending incorporationitem insodium/NOTES.md— and corrects the review that recorded it, which found one relevant change when there were three.The earlier review read the upstream changelog. This one diffed the trees file-by-file. A summary is not a diff, and two of the three were missed.
What changed inside the vendored slice
ACQUIRE_FENCEin all three AEAD decrypt pathscrypto_verify_nconstant-time hardening (verify.c)volatile optblocker_u16through the final reduction. This is the function that compares the Poly1305 tag — it defends constant-time code against the compiler making it branchy, i.e. against reintroducing a timing side channel. Arguably the most security-relevant of the three.optblocker_u64treatment. Upstream did not change donna64, and the changed line here is confirmed inside thepoly1305_state_internal__sse2section — so only the SSE2 copy moved.SODIUM_VERSION_STRING1.0.20 → 1.0.22.HAVE_GCC_MEMORY_FENCESandHAVE_INLINE_ASMare derived from the compiler's own macros, the same way this amalgamation already derivesHAVE_TI_MODEand the SIMD intrinsics. On MSVCACQUIRE_FENCEfalls back to(void) 0— precisely upstream's behaviour when autoconf finds no fence primitive.Verification, including proof the tests can fail
These are defence-in-depth: none should change ciphertext, tags or interop. So the test is a differential one, not a hand-copied vector.
crypto_verify_*contract. No observable behaviour changed.>> 3for>> 2in the optblocker reduction) made the harness reject valid ciphertext and exit 1 at the first iteration.bin/testsuite passes.The documentation defect underneath
NOTES.mdtold the reader to re-runtest_crypto_aead_vectorsas the primary validation. That test did not exist in this repo — it lives in yojimbo, and these notes had been copied across (they still said "so yojimbo has no external libsodium dependency"). A documented validation path that cannot be followed is the same failure as no validation at all.So this PR adds
test_crypto_aead_vectorstonetcode.c, registered first innetcode_test(): golden ciphertext for both AEADs, the tamper-rejection path, and thecrypto_verify_*contract. Proven to fail — flipping one byte of the golden turns it red, and reverting turns it green.It is declared
staticdeliberately: yojimbo vendorsnetcode.cand compiles it withNETCODE_ENABLE_TESTS=1while defining its own non-statictest_crypto_aead_vectorsintest.cpp. Internal linkage keeps the two from colliding on the next re-vendor.Deliberately not applied
The remaining 1.0.20 → 1.0.22 deltas inside the slice are cosmetic and are now enumerated in NOTES.md rather than left unknown:
/* LCOV_EXCL_LINE */coverage annotations,defined _AIX→defined(_AIX), and an unuseduint32_t t32removal. None affects generated code.Follow-up
yojimbo vendors
sodium/andnetcode.cfrom here, and its copy is currently blob-identical. Once this lands, yojimbo needs a re-vendor to pick up the hardenings — base libraries first, green, then yojimbo.