sign stack usage: compute z incrementally #821
Closed
mkannwischer wants to merge 2 commits intomainfrom
Closed
Conversation
01d6afc to
691c8ca
Compare
0a3ebf4 to
57fd4a8
Compare
mkannwischer
added a commit
that referenced
this pull request
Jan 3, 2026
Alternative to #822 that I hope to be less controversial. Currently the constant time tests for verification rely on the signature being declassified at the end of verification. This is not ideal. This commit moves this declassification to the constant-time test instead. As suggested in #822 (review), there is more work left to clean up the story around declassifications. This PR is a first step towards cleaning up that story to unblock #825 and #821, but there is more work left. Signed-off-by: Matthias J. Kannwischer <matthias@kannwischer.eu>
57fd4a8 to
27e553a
Compare
mkannwischer
added a commit
that referenced
this pull request
Jan 3, 2026
Alternative to #822 that I hope to be less controversial. Currently the constant time tests for verification rely on the signature being declassified at the end of verification. This is not ideal. This commit moves this declassification to the constant-time test instead. As suggested in #822 (review), there is more work left to clean up the story around declassifications. This PR is a first step towards cleaning up that story to unblock #825 and #821, but there is more work left. Signed-off-by: Matthias J. Kannwischer <matthias@kannwischer.eu>
381cfef to
b9e4017
Compare
This commit reduces the stack usage of signing by computing z = y + s1*cp incrementally (one polynomial at a time) allowing to eliminate the polyvecl z (at to cost of a single poly z). De-facto this saves L-1 KB irrespective of MLD_CONFIG_REDUCE_RAM. Practically, the same buffer was used early in the function too. Here we instead introduce a new polyvecl buffer tmp, but that can be placed in a union together with w1. Unfortuantely, with the current struct workaround for diffblue/cbmc#8813, this results in an increase in stack space by L KB. This gets eliminated when MLD_CONFIG_REDUCE_RAM is set. Hoisted out from #791 Signed-off-by: Matthias J. Kannwischer <matthias@kannwischer.eu>
For some reason the previous (unrelated) commit caused the verify_pre_hash_internal proof to fail due to extra functions in USE_FUNCTION_CONTRACTS. This commit removes the extra functions. Signed-off-by: Matthias J. Kannwischer <matthias@kannwischer.eu>
b9e4017 to
b3632bb
Compare
Contributor
|
I prefer #825 hoisting the computation of |
Contributor
Author
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.
This commit reduces the stack usage of signing by computing z = y + s1*cp
incrementally (one polynomial at a time) allowing to eliminate the polyvecl
z (at to cost of a single poly z).
De-facto this saves L-1 KB irrespective of MLD_CONFIG_REDUCE_RAM.
Practically, the same buffer was used early in the function too. Here we
instead introduce a new polyvecl buffer tmp, but that can be placed in a union
together with w1.
Unfortuantely, with the current struct workaround for
diffblue/cbmc#8813, this results in an increase in
stack space by L KB.
This gets eliminated when MLD_CONFIG_REDUCE_RAM is set.
crypto_sign_signature_internalstack usage #791