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.

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

[sival/aes] Introduce aes_force_prng_reseed_test and aes_prng_reseed_test #24574

Merged
merged 1 commit into from
Nov 21, 2024

Conversation

github-gcontributor
Copy link
Contributor

@github-gcontributor github-gcontributor commented Sep 14, 2024

[sival/aes] Add aes_prng_reseed,aes_prng_force_reseed

This commit introduces the AES PRNG reseed test (chip_sw_aes_prng_reseed)
and (chip_sw_aes_force_prng_reseed) and AES FORCE PRNG reseed, to handle the
reseed process by disabling entrpy complex and triggering PRNG reseed
accordingly to test the actual behavior of the AES module's PRNG reseeding in
ECB mode.

- Updated the test plan descriptions in `chip_aes_testplan.hjson` to match
  the implemented tests. Added missing test feature.
- Resoloved latest code suggestions provided in PR #24574 and updated
  code with suggestions from other thread (using TRY and remove .c suffix
  in bazel tag).
- Single commit for a cleaner history.
- Resolve multiple commits.

See: hw/top_earlgrey/data/ip/chip_aes_testplan.hjson
Closes Issue #24857
Closes Issue #24899
PR #24574

@github-gcontributor github-gcontributor requested a review from a team as a code owner September 14, 2024 02:15
@github-gcontributor github-gcontributor requested review from moidx and removed request for a team September 14, 2024 02:15
@github-gcontributor
Copy link
Contributor Author

@moidx ,

Updated with feedback and as per format/style.
https://github.com/github-gcontributor/opentitan/blob/vt_checkins/sw/device/tests/aes_force_prng_reseed_test.c
Please review.

sw/device/tests/aes_force_prng_reseed_test.c Outdated Show resolved Hide resolved
sw/device/tests/aes_force_prng_reseed_test.c Outdated Show resolved Hide resolved
sw/device/tests/aes_force_prng_reseed_test.c Outdated Show resolved Hide resolved
sw/device/tests/aes_force_prng_reseed_test.c Outdated Show resolved Hide resolved
sw/device/tests/aes_force_prng_reseed_test.c Outdated Show resolved Hide resolved
sw/device/tests/aes_force_prng_reseed_test.c Outdated Show resolved Hide resolved
@moidx
Copy link
Contributor

moidx commented Sep 15, 2024

Hi @github-gcontributor, Can you also do the following:

  1. Add Bazel target to sw/device/tests/BUILD
  2. Update the commit title and text to describe the changes being introduced in this PR.
  3. Update the PR title and description to follow the commit details.
  4. You will also have to setup your Name and Email in gitgub to be able to sign your commit. This is the reason one of the CI checks is failing. This is needed to meet the CLA guidelines.

@github-gcontributor github-gcontributor force-pushed the vt_checkins branch 2 times, most recently from 0e063c2 to 21a2971 Compare September 16, 2024 23:04
@github-gcontributor github-gcontributor changed the title Initial Checkins Updated the code with the review feedback Sep 16, 2024
@github-gcontributor
Copy link
Contributor Author

@moidx,
Thanks for the review.
Made changes to the code as per the comments.
i.e. The Test checks for AES Stall with Disabled Entropy Complex (and not just edn)
Added BUILD file with target

Please provide feedback on the changes..

@github-gcontributor
Copy link
Contributor Author

github-gcontributor commented Sep 20, 2024

Hi @github-gcontributor, Can you also do the following:

  1. Add Bazel target to sw/device/tests/BUILD
  2. Update the commit title and text to describe the changes being introduced in this PR.
  3. Update the PR title and description to follow the commit details.
  4. You will also have to setup your Name and Email in gitgub to be able to sign your commit. This is the reason one of the CI checks is failing. This is needed to meet the CLA guidelines.

Regarding point 4,
I tried making the changes as per the instruction. Using my corporate id, does not allow me to proceed checkin as it would expose the privacy. As a second step, I added "[email protected]" as mentioned in my github steps to protect privacy. However it still asking showing up issue in quick lint check.

Still issue is persistent:
"""
remote: error: GH007: Your push would publish a private email address.
remote: You can make your email public or disable this protection by visiting:
remote: http://github.com/settings/emails
To github.com:github-gcontributor/opentitan.git
! [remote rejected] vt_checkins -> vt_checkins (push declined due to email privacy restrictions)
error: failed to push some refs to 'github.com:github-gcontributor/opentitan.git'
"""
I added my personal id.
With non corporate id, I am disabling the "Keep my email addresses private" option now. Looks like it is allowing me to chekcin now. Waiting for the quick lint

@github-gcontributor github-gcontributor force-pushed the vt_checkins branch 3 times, most recently from b62a3ec to 50fd1cf Compare September 20, 2024 19:46
@a-will
Copy link
Contributor

a-will commented Sep 20, 2024

  1. You will also have to setup your Name and Email in gitgub to be able to sign your commit. This is the reason one of the CI checks is failing. This is needed to meet the CLA guidelines.

Regarding point 4, I tried making the changes as per the instruction. Using my corporate id, does not allow me to proceed checkin as it would expose the privacy. As a second step, I added "[email protected]" as mentioned in my github steps to protect privacy. However it still asking showing up issue in quick lint check.

Still issue is persistent: """ remote: error: GH007: Your push would publish a private email address. remote: You can make your email public or disable this protection by visiting: remote: http://github.com/settings/emails To github.com:github-gcontributor/opentitan.git ! [remote rejected] vt_checkins -> vt_checkins (push declined due to email privacy restrictions) error: failed to push some refs to 'github.com:github-gcontributor/opentitan.git' """ I added my personal id. With non corporate id, I am disabling the "Keep my email addresses private" option now. Looks like it is allowing me to chekcin now. Waiting for the quick lint

You are contributing to a public repo with a CLA from your company. The corporate email must be exposed in commits, else the contribution cannot be accepted. You must uncheck "Block command line pushes that expose my email" (or else get someone else from your organization to submit the change).

Some people choose to have separate accounts for corporate work, and this enabled them to keep this safeguard for their personal account. I'm not sure if there is a way to make it more granular (like by repo or something).

@github-gcontributor
Copy link
Contributor Author

github-gcontributor commented Sep 20, 2024

  1. You will also have to setup your Name and Email in gitgub to be able to sign your commit. This is the reason one of the CI checks is failing. This is needed to meet the CLA guidelines.

Regarding point 4, I tried making the changes as per the instruction. Using my corporate id, does not allow me to proceed checkin as it would expose the privacy. As a second step, I added "[email protected]" as mentioned in my github steps to protect privacy. However it still asking showing up issue in quick lint check.
Still issue is persistent: """ remote: error: GH007: Your push would publish a private email address. remote: You can make your email public or disable this protection by visiting: remote: http://github.com/settings/emails To github.com:github-gcontributor/opentitan.git ! [remote rejected] vt_checkins -> vt_checkins (push declined due to email privacy restrictions) error: failed to push some refs to 'github.com:github-gcontributor/opentitan.git' """ I added my personal id. With non corporate id, I am disabling the "Keep my email addresses private" option now. Looks like it is allowing me to chekcin now. Waiting for the quick lint

You are contributing to a public repo with a CLA from your company. The corporate email must be exposed in commits, else the contribution cannot be accepted. You must uncheck "Block command line pushes that expose my email" (or else get someone else from your organization to submit the change).

Some people choose to have separate accounts for corporate work, and this enabled them to keep this safeguard for their personal account. I'm not sure if there is a way to make it more granular (like by repo or something).

Sure, in that case let me change my id to corporate email.
After disabling "Keep my email addresses private" I am able to checkin with my corporate id. Waiting for quick lint now...

@pamaury
Copy link
Contributor

pamaury commented Oct 1, 2024

Hi @github-gcontributor, could you please edit the title of the pull request to something that reflects its content? This makes it easier for other contributors to know what the PR is about.

@github-gcontributor github-gcontributor changed the title Updated the code with the review feedback Test cases aes_force_prng_reseed_test and aes_prng_reseed_test Oct 1, 2024
@github-gcontributor github-gcontributor changed the title Test cases aes_force_prng_reseed_test and aes_prng_reseed_test Please review test cases aes_force_prng_reseed_test and aes_prng_reseed_test Oct 1, 2024
@github-gcontributor
Copy link
Contributor Author

Hi @github-gcontributor, could you please edit the title of the pull request to something that reflects its content? This makes it easier for other contributors to know what the PR is about.

Title updated

@moidx moidx changed the title Please review test cases aes_force_prng_reseed_test and aes_prng_reseed_test [sival] Introduce aes_force_prng_reseed_test and aes_prng_reseed_test Oct 2, 2024
@moidx moidx requested a review from nasahlpa October 2, 2024 16:09
github-gcontributor added a commit to github-gcontributor/opentitan that referenced this pull request Nov 15, 2024
This commit introduces the AES PRNG reseed test (chip_sw_aes_prng_reseed)
and (chip_sw_aes_force_prng_reseed) and AES FORCE PRNG reseed, to handle the
reseed process by disabling entrpy complex and triggering PRNG reseed
accordingly to test the actual behavior of the AES module's PRNG reseeding in
ECB mode.

- Updated the test plan descriptions in `chip_aes_testplan.hjson` to match
  the implemented tests.
- Resolved code suggestions provided in PR lowRISC#24574.
- Single commit for a cleaner history.

See: hw/top_earlgrey/data/ip/chip_aes_testplan.hjson
Closes Issue lowRISC#24857
Closes Issue lowRISC#24899
PR lowRISC#24574

Signed-off-by: Varunkumar Trivedi <[email protected]>
github-gcontributor added a commit to github-gcontributor/opentitan that referenced this pull request Nov 15, 2024
This commit introduces the AES PRNG reseed test (chip_sw_aes_prng_reseed)
and (chip_sw_aes_force_prng_reseed) and AES FORCE PRNG reseed, to handle the
reseed process by disabling entrpy complex and triggering PRNG reseed
accordingly to test the actual behavior of the AES module's PRNG reseeding in
ECB mode.

- Updated the test plan descriptions in `chip_aes_testplan.hjson` to match
  the implemented tests. Added missing test feature.
- Resoloved latest code suggestions provided in PR lowRISC#24574 and updated
  code with suggestions from other thread (using TRY and remove .c suffix
  in bazel tag).
- Single commit for a cleaner history.
- Resolve multiple commits

See: hw/top_earlgrey/data/ip/chip_aes_testplan.hjson
Closes Issue lowRISC#24857
Closes Issue lowRISC#24899
PR lowRISC#24574

Signed-off-by: Varunkumar Trivedi <[email protected]>
github-gcontributor added a commit to github-gcontributor/opentitan that referenced this pull request Nov 15, 2024
This commit introduces the AES PRNG reseed test (chip_sw_aes_prng_reseed)
and (chip_sw_aes_force_prng_reseed) and AES FORCE PRNG reseed, to handle the
reseed process by disabling entrpy complex and triggering PRNG reseed
accordingly to test the actual behavior of the AES module's PRNG reseeding in
ECB mode.

- Updated the test plan descriptions in `chip_aes_testplan.hjson` to match
  the implemented tests.
- Resolved code suggestions provided in PR lowRISC#24574.
- Single commit for a cleaner history.

See: hw/top_earlgrey/data/ip/chip_aes_testplan.hjson
Closes Issue lowRISC#24857
Closes Issue lowRISC#24899
PR lowRISC#24574

Signed-off-by: Varunkumar Trivedi <[email protected]>
github-gcontributor added a commit to github-gcontributor/opentitan that referenced this pull request Nov 15, 2024
This commit introduces the AES PRNG reseed test (chip_sw_aes_prng_reseed)
and (chip_sw_aes_force_prng_reseed) and AES FORCE PRNG reseed, to handle the
reseed process by disabling entrpy complex and triggering PRNG reseed
accordingly to test the actual behavior of the AES module's PRNG reseeding in
ECB mode.

- Updated the test plan descriptions in `chip_aes_testplan.hjson` to match
  the implemented tests. Added missing test feature.
- Resoloved latest code suggestions provided in PR lowRISC#24574 and updated
  code with suggestions from other thread (using TRY and remove .c suffix
  in bazel tag).
- Single commit for a cleaner history.
- Resolve multiple commits.

See: hw/top_earlgrey/data/ip/chip_aes_testplan.hjson
Closes Issue lowRISC#24857
Closes Issue lowRISC#24899
PR lowRISC#24574

Signed-off-by: Varunkumar Trivedi <[email protected]>

[sival/aes] Update PRNG reseed tests to reflect actual behavior

This commit introduces the AES PRNG reseed test (chip_sw_aes_prng_reseed)
and (chip_sw_aes_force_prng_reseed) and AES FORCE PRNG reseed, to handle the
reseed process by disabling entrpy complex and triggering PRNG reseed
accordingly to test the actual behavior of the AES module's PRNG reseeding in
ECB mode.

- Updated the test plan descriptions in `chip_aes_testplan.hjson` to match
  the implemented tests.
- Resolved code suggestions provided in PR lowRISC#24574.
- Single commit for a cleaner history.

See: hw/top_earlgrey/data/ip/chip_aes_testplan.hjson
Closes Issue lowRISC#24857
Closes Issue lowRISC#24899
PR lowRISC#24574

Signed-off-by: Varunkumar Trivedi <[email protected]>
This commit introduces the AES PRNG reseed test (chip_sw_aes_prng_reseed)
and (chip_sw_aes_force_prng_reseed) and AES FORCE PRNG reseed, to handle the
reseed process by disabling entrpy complex and triggering PRNG reseed
accordingly to test the actual behavior of the AES module's PRNG reseeding in
ECB mode.

- Updated the test plan descriptions in `chip_aes_testplan.hjson` to match
  the implemented tests. Added missing test feature.
- Resoloved latest code suggestions provided in PR lowRISC#24574 and updated
  code with suggestions from other thread (using TRY and remove .c suffix
  in bazel tag).
- Single commit for a cleaner history.
- Resolve multiple commits.

See: hw/top_earlgrey/data/ip/chip_aes_testplan.hjson
Closes Issue lowRISC#24857
Closes Issue lowRISC#24899
PR lowRISC#24574

Signed-off-by: Varunkumar Trivedi <[email protected]>
@engdoreis engdoreis added the CherryPick:earlgrey_1.0.0 This PR should be cherry-picked to earlgrey_1.0.0 label Nov 21, 2024
@engdoreis engdoreis merged commit 036c58a into lowRISC:master Nov 21, 2024
45 checks passed
Copy link

Backport failed for earlgrey_1.0.0, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin earlgrey_1.0.0
git worktree add -d .worktree/backport-24574-to-earlgrey_1.0.0 origin/earlgrey_1.0.0
cd .worktree/backport-24574-to-earlgrey_1.0.0
git switch --create backport-24574-to-earlgrey_1.0.0
git cherry-pick -x 06976d752d5ac1baa848ade96133f38cf9d6b29e

@github-actions github-actions bot added the Manually CherryPick This PR should be manually cherry picked. label Nov 21, 2024
@engdoreis
Copy link
Contributor

@github-gcontributor could you please handle this cherry-pick?

@github-gcontributor github-gcontributor added CherryPick:earlgrey_1.0.0 This PR should be cherry-picked to earlgrey_1.0.0 and removed CherryPick:earlgrey_1.0.0 This PR should be cherry-picked to earlgrey_1.0.0 Manually CherryPick This PR should be manually cherry picked. labels Nov 23, 2024
Copy link

Backport failed for earlgrey_1.0.0, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin earlgrey_1.0.0
git worktree add -d .worktree/backport-24574-to-earlgrey_1.0.0 origin/earlgrey_1.0.0
cd .worktree/backport-24574-to-earlgrey_1.0.0
git switch --create backport-24574-to-earlgrey_1.0.0
git cherry-pick -x 06976d752d5ac1baa848ade96133f38cf9d6b29e

@github-actions github-actions bot added the Manually CherryPick This PR should be manually cherry picked. label Nov 23, 2024
github-gcontributor added a commit to github-gcontributor/opentitan that referenced this pull request Nov 23, 2024
This commit introduces the AES PRNG reseed test (chip_sw_aes_prng_reseed)
and (chip_sw_aes_force_prng_reseed) and AES FORCE PRNG reseed, to handle the
reseed process by disabling entrpy complex and triggering PRNG reseed
accordingly to test the actual behavior of the AES module's PRNG reseeding in
ECB mode.

- Updated the test plan descriptions in `chip_aes_testplan.hjson` to match
  the implemented tests. Added missing test feature.
- Resoloved latest code suggestions provided in PR lowRISC#24574 and updated
  code with suggestions from other thread (using TRY and remove .c suffix
  in bazel tag).
- Single commit for a cleaner history.
- Resolve multiple commits.

See: hw/top_earlgrey/data/ip/chip_aes_testplan.hjson
Closes Issue lowRISC#24857
Closes Issue lowRISC#24899
PR lowRISC#24574

Signed-off-by: Varunkumar Trivedi <[email protected]>
(cherry picked from commit 06976d7)
@github-gcontributor
Copy link
Contributor Author

HI @engdoreis,
the backport is complete:

git push origin backport-24574-to-earlgrey_1.0.0
Enumerating objects: 23, done.
Counting objects: 100% (23/23), done.
Delta compression using up to 12 threads
Compressing objects: 100% (13/13), done.
Writing objects: 100% (13/13), 6.37 KiB | 724.00 KiB/s, done.
Total 13 (delta 11), reused 0 (delta 0), pack-reused 0 (from 0)
remote: Resolving deltas: 100% (11/11), completed with 10 local objects.
remote:
remote: Create a pull request for 'backport-24574-to-earlgrey_1.0.0' on GitHub by visiting:
remote: https://github.com/github-gcontributor/opentitan/pull/new/backport-24574-to-earlgrey_1.0.0
remote:
To github.com:github-gcontributor/opentitan.git

  • [new branch] backport-24574-to-earlgrey_1.0.0 -> backport-24574-to-earlgrey_1.0.0

Kindly verify and proceed for the next step or let me know if anything from my end.
Thanks.

@engdoreis
Copy link
Contributor

@github-gcontributor can you please create and link the backport PR here?

@github-gcontributor
Copy link
Contributor Author

@github-gcontributor can you please create and link the backport PR here?

Crearted backport-24574-to-earlgrey_1.0.0
Please suggest the next steps

@engdoreis
Copy link
Contributor

@github-gcontributor I can see that you created a branch on your fork named backport-24574-to-earlgrey_1.0.0. Now you have to create a pull request from that branch to earlgrey_1.0.0 and add the link of this PR on the description.

Please see this example #25399

@github-gcontributor
Copy link
Contributor Author

@github-gcontributor I can see that you created a branch on your fork named backport-24574-to-earlgrey_1.0.0. Now you have to create a pull request from that branch to earlgrey_1.0.0 and add the link of this PR on the description.

Please see this example #25399

#25441

github-gcontributor added a commit to github-gcontributor/opentitan that referenced this pull request Dec 4, 2024
This commit introduces the AES PRNG reseed test (chip_sw_aes_prng_reseed)
and (chip_sw_aes_force_prng_reseed) and AES FORCE PRNG reseed, to handle the
reseed process by disabling entrpy complex and triggering PRNG reseed
accordingly to test the actual behavior of the AES module's PRNG reseeding in
ECB mode.

- Updated the test plan descriptions in `chip_aes_testplan.hjson` to match
  the implemented tests. Added missing test feature.
- Resoloved latest code suggestions provided in PR lowRISC#24574 and updated
  code with suggestions from other thread (using TRY and remove .c suffix
  in bazel tag).
- Single commit for a cleaner history.
- Resolve multiple commits.
- Manualy resolve conflict

See: hw/top_earlgrey/data/ip/chip_aes_testplan.hjson
Closes Issue lowRISC#24857
Closes Issue lowRISC#24899
PR lowRISC#24574

Signed-off-by: Varunkumar Trivedi <[email protected]>
(cherry picked from commit 06976d7)
engdoreis pushed a commit that referenced this pull request Dec 10, 2024
This commit introduces the AES PRNG reseed test (chip_sw_aes_prng_reseed)
and (chip_sw_aes_force_prng_reseed) and AES FORCE PRNG reseed, to handle the
reseed process by disabling entrpy complex and triggering PRNG reseed
accordingly to test the actual behavior of the AES module's PRNG reseeding in
ECB mode.

- Updated the test plan descriptions in `chip_aes_testplan.hjson` to match
  the implemented tests. Added missing test feature.
- Resoloved latest code suggestions provided in PR #24574 and updated
  code with suggestions from other thread (using TRY and remove .c suffix
  in bazel tag).
- Single commit for a cleaner history.
- Resolve multiple commits.
- Manualy resolve conflict

See: hw/top_earlgrey/data/ip/chip_aes_testplan.hjson
Closes Issue #24857
Closes Issue #24899
PR #24574

Signed-off-by: Varunkumar Trivedi <[email protected]>
(cherry picked from commit 06976d7)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CherryPick:earlgrey_1.0.0 This PR should be cherry-picked to earlgrey_1.0.0 Manually CherryPick This PR should be manually cherry picked.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants