Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[cryptotest] //sw/device/tests/crypto/cryptotest:ecdh_kat #22210

Closed
1 task
moidx opened this issue Mar 23, 2024 · 5 comments
Closed
1 task

[cryptotest] //sw/device/tests/crypto/cryptotest:ecdh_kat #22210

moidx opened this issue Mar 23, 2024 · 5 comments
Assignees
@moidx @vogelpi
Labels
Component:TestTriage SW:cryptolib Crypto library
Milestone
Earlgrey-PROD.M4

Comments

@moidx
Copy link
Contributor

moidx commented Mar 23, 2024

Hierarchy of regression failure

Block level

Failure Description

[2024-03-23T04:43:43.099Z INFO  harness] Test counter: 332
[2024-03-23T04:43:43.099Z INFO  harness] vendor: wycheproof, test case: 332
[2024-03-23T04:43:43.099Z INFO  opentitanlib::test_utils::rpc] Sending: "Ecdh"
[2024-03-23T04:43:43.106Z INFO  opentitanlib::test_utils::rpc] Sending: "P256"
[2024-03-23T04:43:43.113Z INFO  opentitanlib::test_utils::rpc] Sending: {"d0":[177,101,82,146,221,80,170,112,109,135,24,140,227,103,29,83,28,124,200,238,162,37,236,242,2,68,226,233,184,229,201,55],"d0_len":32,"d1":[220,130,34,162,241,90,174,121,212,243,54,91,204,28,2,31,106,190,33,173,131,42,112,5,27,172,105,135,150,191,128,70],"d1_len":32}
[2024-03-23T04:43:43.410Z INFO  opentitanlib::test_utils::rpc] Sending: {"coordinate":[0],"coordinate_len":1}
[2024-03-23T04:43:43.450Z INFO  opentitanlib::test_utils::rpc] Sending: {"coordinate":[0],"coordinate_len":1}
[2024-03-23T04:43:43.491Z  console]E00002 ecdh.c:123] Unexpected status value returned from otcrypto_ecdh: 0x80006d89
[2024-03-23T04:43:43.498Z  console]RESP_ERR:{"Internal":["ECD",124]} CRC:1516886847

253:Finished test test_ecdh: Err(CRC didn't match received json body.

Steps to Reproduce

  • GitHub Revision: 6d3cb9a earlgrey_es_sival
  • Bazel: //sw/device/tests/crypto/cryptotest:ecdh_kat_silicon_owner_prodc_rom_ext

Should be reproducible with sival sku as well.

Tests with similar or related failures

  • //sw/device/tests/crypto/cryptotest:ecdh_kat
@moidx moidx added this to the Earlgrey-PROD.M4 milestone Mar 23, 2024
@moidx
Copy link
Contributor Author

moidx commented Mar 23, 2024

CC @milesdai @jadephilipoom

@andreaskurth
Copy link
Contributor

@moidx: Could be a problem in OTBN or with the entropy complex (not sure if entropy is required)

@andreaskurth
Copy link
Contributor

Likely related to #22209.

@vogelpi
Copy link
Contributor

vogelpi commented May 9, 2024 •
edited
Loading

After including the keymgr fix (see #22819 (comment)), the test now passes on silicon for sival:

  • //sw/device/tests/crypto/cryptotest:ecdh_kat_silicon_owner_sival_rom_ext

See the attached log: ecdh_kat.txt Thanks @nasahlpa !

We couldn't test prodc though. @moidx, do you mind checking prodc again?

vogelpi added a commit to vogelpi/opentitan that referenced this issue May 17, 2024
@vogelpi
[cryptolib] Also poll CSRNG command ready bit for additional data
50a5da8 
Previously, the command register ready bit was only polled for the
additional data when writing commands to the SW register of EDN.

When writing commands to CSRNG instead, not polling the command ready
bit could cause CSRNG to drop additional data provided by software while
handling a hardware command. Once the hardware command was handled, the
software instance would win arbitration and CSRNG would wait for
software to provide the dropped additional data, thereby stalling the
entire entropy complex. This would happen for example if a HW instance
request would collide with a software request. Since the software
command FIFO is only 2 entries deep, it can only be guaranteed to absorb
2 words if the software interface doesn't win arbitration immediately.

This is related to lowRISC#22209 and lowRISC#22210.

Signed-off-by: Pirmin Vogel <[email protected]>
@vogelpi vogelpi mentioned this issue May 17, 2024
Merged
vogelpi added a commit to vogelpi/opentitan that referenced this issue May 17, 2024
@vogelpi
[cryptolib] Fix CSRNG driver for using the SW command interface of CSRNG
dd140ef 
Previously, the main SM state was only checked for the first command
word but not for the additional data. Not checking the main SM state
before writing the additional data could cause CSRNG to drop additional
data provided by software while handling a hardware command. Once the
hardware command was handled, the software instance would win
arbitration and CSRNG would wait for software to provide the dropped
additional data, thereby stalling the entire entropy complex. This
would happen for example if a HW instance request would collide with a
software request. Since the software command FIFO is only 2 entries
deep, it can only be guaranteed to absorb 2 words if the software
interface doesn't win arbitration immediately.

This commit contains the ES / CSRNG v.1.0.0 specific version of
lowRISC#23166. In addition, it adds some more comments to
properly document the underlying hardware issue directly in the
cryptolib implementation.

This is related to lowRISC#22209 and lowRISC#22210.

Signed-off-by: Pirmin Vogel <[email protected]>
@vogelpi vogelpi mentioned this issue May 17, 2024
Merged
@vogelpi vogelpi self-assigned this May 17, 2024
vogelpi added a commit that referenced this issue May 17, 2024
@vogelpi
[cryptolib] Fix CSRNG driver for using the SW command interface of CSRNG
a9068ae 
Previously, the main SM state was only checked for the first command
word but not for the additional data. Not checking the main SM state
before writing the additional data could cause CSRNG to drop additional
data provided by software while handling a hardware command. Once the
hardware command was handled, the software instance would win
arbitration and CSRNG would wait for software to provide the dropped
additional data, thereby stalling the entire entropy complex. This
would happen for example if a HW instance request would collide with a
software request. Since the software command FIFO is only 2 entries
deep, it can only be guaranteed to absorb 2 words if the software
interface doesn't win arbitration immediately.

This commit contains the ES / CSRNG v.1.0.0 specific version of
#23166. In addition, it adds some more comments to
properly document the underlying hardware issue directly in the
cryptolib implementation.

This is related to #22209 and #22210.

Signed-off-by: Pirmin Vogel <[email protected]>
vogelpi added a commit that referenced this issue May 17, 2024
@vogelpi
[cryptolib] Also poll CSRNG command ready bit for additional data
f1eb1e0 
Previously, the command register ready bit was only polled for the
additional data when writing commands to the SW register of EDN.

When writing commands to CSRNG instead, not polling the command ready
bit could cause CSRNG to drop additional data provided by software while
handling a hardware command. Once the hardware command was handled, the
software instance would win arbitration and CSRNG would wait for
software to provide the dropped additional data, thereby stalling the
entire entropy complex. This would happen for example if a HW instance
request would collide with a software request. Since the software
command FIFO is only 2 entries deep, it can only be guaranteed to absorb
2 words if the software interface doesn't win arbitration immediately.

This is related to #22209 and #22210.

Signed-off-by: Pirmin Vogel <[email protected]>
@vogelpi
Copy link
Contributor

vogelpi commented May 17, 2024

The two relevant PRs for this issue (for ES and PROD) have both been merged. I've extensively tested this on FPGA (using an fpga_cw310_sival_rom_ext environment).

@vogelpi vogelpi closed this as completed May 17, 2024
AlexJones0 pushed a commit to AlexJones0/opentitan that referenced this issue Jul 8, 2024
@vogelpi @AlexJones0
[cryptolib] Also poll CSRNG command ready bit for additional data
0fc0178 
Previously, the command register ready bit was only polled for the
additional data when writing commands to the SW register of EDN.

When writing commands to CSRNG instead, not polling the command ready
bit could cause CSRNG to drop additional data provided by software while
handling a hardware command. Once the hardware command was handled, the
software instance would win arbitration and CSRNG would wait for
software to provide the dropped additional data, thereby stalling the
entire entropy complex. This would happen for example if a HW instance
request would collide with a software request. Since the software
command FIFO is only 2 entries deep, it can only be guaranteed to absorb
2 words if the software interface doesn't win arbitration immediately.

This is related to lowRISC#22209 and lowRISC#22210.

Signed-off-by: Pirmin Vogel <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@moidx moidx

@vogelpi vogelpi

Labels
Component:TestTriage SW:cryptolib Crypto library
Projects
None yet
Milestone
Earlgrey-PROD.M4
Development

No branches or pull requests
3 participants
@andreaskurth @moidx @vogelpi