[sw, otbn, crypto] Sync with the master branch - Part 2 #23825
Merged
Conversation
This file contains 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
Signed-off-by: Vladimir Rozic <[email protected]>
Fix occurrences of 'the the' which seems to be quite a common typographical error, affecting mostly documentation and source comments. Signed-off-by: Adrian Lees <[email protected]>
This commit ammends AES fixed-vs-random key dataset capture: 1. Updates the outdated description of the PRNG used for data generation 2. Acknowledges command in aes_serial capture method. This change improves the stability of measurements. 3. Returns the full 16B ciphertect of the last encryption in the batch. This is changed from only sending the last 4 bytes. This change makes it consistent with other batch measurements, making the code easier to maintain. Signed-off-by: Vladimir Rozic <[email protected]>
At the moment, a 32-bit SW LFSR is used for masking data shares sent from IBEX to crypto-blocks (AES, KMAC, SHA3). This commit adds a new context to this PRNG so that it can be used for multiple purposes. This is needed to set the stage for implementing aes-fvsr-data capture which makes use of this LFSR to determine the order of measurements. This commit also ammends all files that make use of the lfsr functions. Signed-off-by: Vladimir Rozic <[email protected]>
Signed-off-by: Vladimir Rozic <[email protected]>
Previously, the "fail" jump point for certain paths (r and s out of range in the signature) didn't actually fail! This fixes the problem. Signed-off-by: Jade Philipoom <[email protected]>
This is a more elegant way to fail than illegal instruction errors for bad signatures or public keys. Signed-off-by: Jade Philipoom <[email protected]>
As discussed offline with @vogelpi, this PR moves the arming and disarming of the SCA trigger for SHA3 before and after the process command is issued. This ensures that we only capture the SCA relevant parts for doing power analysis. Signed-off-by: Pascal Nasahl <[email protected]> (cherry picked from commit 1a91b7e)
A zero input (such as a point-at-infinity in the conversion to affine coordinates) can cause the GCD algorithm to not terminate. Signed-off-by: Jade Philipoom <[email protected]>
Previously, the modexp implementation did not correctly set the mode, and the names of certain inputs were not compatible with the R^2 internal computation. Also, the internal modexp expects the input and output buffers for RSA to be disjoint. Signed-off-by: Jade Philipoom <[email protected]>
(1) The additional seed was being ignored, and random intermediate values were getting XORed into the key instead, resulting in a different private key every run. (2) The private key was not getting moved to the correct registers before scalar multiplication, resulting in a different private key every run. Signed-off-by: Jade Philipoom <[email protected]>
Signed-off-by: Jade Philipoom <[email protected]>
Inversion is not performance-critical, so this doesn't lead to a huge speed improvement (~2000 cycles for sign overall). However, it does reduce the code size significantly (-24 instructions) and make the structure more regular, which will allow for a second optimization pass removing nops. Signed-off-by: Jade Philipoom <[email protected]>
This commit moves register assignments slightly so that we can eliminate the `nop` at the end of each loop in modular inversion. This saves only about 253 cycles for the top-level sign operation, but more crucially it shaves off 40 instructions from the code size. Signed-off-by: Jade Philipoom <[email protected]>
This has some minor speed (-1510 cycles for sign) and code size (-4 instructions) benefits. Signed-off-by: Jade Philipoom <[email protected]>
Signed-off-by: Jade Philipoom <[email protected]>
Co-authored-by: Bojan Uljarevic <[email protected]> Signed-off-by: Moritz Wettermann <[email protected]>
This PR manually cherry-picks "Add support for AES SCA measurements on chip" lowRISC#21741 from earlgrey_es_sival to master as the automatic cherry-pick failed due to a merge conflict. Signed-off-by: Pascal Nasahl <[email protected]>
Fixes the failing wycheproof ECDSA tests in lowRISC#22322. When computing the recovered x coordinate, we get a value that's reduced modulo p (the coordinate field modulus) and we then need to reduce this value modulo n (the curve order). This operation is almost always a no-op, since p and n are very close together; there's roughly a 2^-130 probability of hitting this range by chance. However, the code implementing the reduction was *always* a no-op. Essentially, by using `bn.subm` instead of `bn.addm` here, the code was conditionally *adding* the modulus n if `w19 - 0` underflowed, which it never would. Instead, we need `bn.addm`, which will conditionally *subtract* the modulus n if `w19 + 0` is greater than n. Signed-off-by: Jade Philipoom <[email protected]>
This commit fixes a bug in the SCA library where in the sca_seed_lfsr function the wrong state was written. Signed-off-by: Pascal Nasahl <[email protected]>
This commit fixes a bug in the simpleserial and uJSON AES SCA application. Inside the aes_key_mask_and_config function, two for loops are responsible for setting the key shares. The first for loop sets all key shares up to the provided key_length. The second for loop sets all unused key bits. Previously, the second for loop did not set the key shares for the unused bits correctly. For example, when key_len=16, only key shares 0...3 were correctly set, key shares 4...7 were uninitialized, e.g. contained old values from memory. However, for SCA we assume that the unused key share values are set to 0. Signed-off-by: Pascal Nasahl <[email protected]>
The input masks are now taken from the PRNG buffer stage. This means after loading the magic seed into the PRNG and having the PRNG output the all-zero vector, the buffer stage needs to be updated as well. This can be achieved by triggering via a clearing operation of the data output registers. This resolves lowRISC#22917. Signed-off-by: Pirmin Vogel <[email protected]>
This commit removes dptr_<x> variables and adapts the code to function without it. Signed-off-by: Moritz Wettermann <[email protected]>
This commit removes dptr_<x> variables and adapts the code to function without it. Signed-off-by: Moritz Wettermann <[email protected]>
This commit removes dptr_<x> variables and adapts the code to function without it. Signed-off-by: Moritz Wettermann <[email protected]>
This commit removes dptr_<x> variables and adapts the code to function without it. Signed-off-by: Moritz Wettermann <[email protected]>
This commit removes dptr_<x> variables and adapts the code to function without it. Signed-off-by: Moritz Wettermann <[email protected]>
This commit removes dptr_<x> variables and adapts the code to function without it. Signed-off-by: Moritz Wettermann <[email protected]>
This commit removes dptr_<x> variables and adapts the code to function without it. Signed-off-by: Moritz Wettermann <[email protected]>
This commit puts `la` instructions for variable address loading inside of functions, so that address loading doesn't need to be done in wrapper functions. Signed-off-by: Moritz Wettermann <[email protected]>
Signed-off-by: Moritz Wettermann <[email protected]>
sameo
requested review from
rswarbrick,
moidx and
jadephilipoom
and removed request for
a team
|
@jadephilipoom @andreaskurth @Razer6 That one will need another waiver for a |
jadephilipoom
approved these changes
Jun 27, 2024
There was a problem hiding this comment.
Re-fetched and checked this against my verification branch again, and the remaining differences are all acceptable stuff we discussed in #23492. LGTM.
|
CHANGE AUTHORIZED: hw/ip/spi_device/rtl/spid_dpram.sv This is indeed only a comment change and doesn't affect the actual RTL. |
|
CHANGE AUTHORIZED: hw/ip/spi_device/rtl/spid_dpram.sv As mentioned by @jadephilipoom only a comment changed in that RTL file, so that doesn't affect the netlist of the TO. Nonetheless, double-checking with Rivos HW team to confirm that this isn't a problem (e.g., for auditing reasons)? @neeraj-rv @Razer6 |
|
@andreaskurth LGTM. |
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 is the second part of the sw crypto sync with master, as the original PR was too large for GH to handle (see #23492 (comment)). It literally contains only the commits that got approved as part of the larger PR.
Fixes #23478
Fixes #23479