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

[sca, kmac] Add new capture script for KMAC #229

Merged
merged 2 commits into from
Dec 1, 2023

Conversation

m-temp
Copy link
Collaborator

@m-temp m-temp commented Nov 27, 2023

This PR addresses #217

As this was a major rework, it would be good if s.o. could test if the TVLA still works.

There are three open points:

  1. FIXMEs in the config file: I'm not sure how to translate the cycles vs. samples
  2. In the non-bach fvsr-mode, the internal PRNG is used not AES. This is the same behavior as in the old-script. However we should adjust this eventually.
  3. The keyword "ciphertext" is used throughout the script. We should change this to mac or tag eventually.

@m-temp m-temp force-pushed the kmac-capture branch 2 times, most recently from 29652b1 to 7708152 Compare November 27, 2023 15:27
@nasahlpa
Copy link
Member

Thanks @m-temp, I will have a closer look tomorrow.

Regarding the samples vs. cycles. You either can specify whether you want to configure the Husky scope in samples or cycles. If both are provided, the samples are taken.
In the AES script, we configured the scope in cycles, the cycle converter then converts the cycles into samples (which is needed by Husky).

So if you know how many cycles a KMAC run takes, you probably want to configure the num_cycles parameter and remove the num_samples.

@nasahlpa
Copy link
Member

nasahlpa commented Nov 29, 2023

Thanks for changing the configuration file! Capture trace looks IMHO good. Could you please try to run TVLA?

Do you think capturing KMAC traces should be added to CI?

@vrozic
Copy link
Contributor

vrozic commented Nov 30, 2023

Thanks @m-temp . Great work

I've tested all three capture types locally with my setup and they all work.

TVLA works well with one caveat. At the moment TVLA figures of general test results contain meta-data that are read from the project file. This reports an error when we use the new data-base format. After slightly modifying the tvla.py I got this figure:
kmac_fixed_vs_random

The results look as expected.
Since the TVLA problem is unrelated, I will solve it in a separate PR. I am tracking this in issue #72

@vrozic
Copy link
Contributor

vrozic commented Nov 30, 2023

I see that for AES captures, we're using aes_sca_cw310.yaml
Maybe we can rename capture_kmac_cw310.yaml to kmac_sca_cw310.yaml

Copy link
Contributor

@vrozic vrozic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my view this is ready to merge, modulo two small comments.
We should open an issue to remember to add this capture to CI later on.

This commit adds the new capture script to reflect the recent changes
in the repository.

Signed-off-by: Michael Tempelmeier <[email protected]>
@m-temp
Copy link
Collaborator Author

m-temp commented Nov 30, 2023

I see that for AES captures, we're using aes_sca_cw310.yaml Maybe we can rename capture_kmac_cw310.yaml to kmac_sca_cw310.yaml

done

@m-temp m-temp merged commit e74a916 into lowRISC:master Dec 1, 2023
5 checks passed
@m-temp m-temp deleted the kmac-capture branch December 1, 2023 16:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants