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

KMAC support for Python scripts #54

Merged
merged 1 commit into from
Sep 17, 2021
Merged

Conversation

m-temp
Copy link
Collaborator

@m-temp m-temp commented Sep 7, 2021

This is WIP to add KMAC (and cshake) support to the SCA-repo.
Unfortunately, there is no (native) support for kmac and cshake (mainly because of the customization string in SP800-185) in python. There is a feature request for hashlib, but to the best of my knowledge there is no SP800-185 python module available. Therefore I used the standard (other optimizations would be possible) C-implementation from the official eXtended Keccak Code Package which uses the CC0 and a BSD 3-clause license. Is this OK?

The provided demo works for the testvektors and are compatible to cw's ktp-generator.

As I'm waiting for a cw310 board I cannot integrate the code into the ot-sca-framework atm.
However this pull request is intended to further discuss the implementation process. Help on how to integrate the cffi, ffi/lib stuff into a package is welcome. Apologies, if I missed things in the integration flow. Please bear with me, I'm new ;)

@vogelpi vogelpi self-requested a review September 7, 2021 14:32
Copy link
Collaborator

@vogelpi vogelpi left a comment

Choose a reason for hiding this comment

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

Thanks @m-temp for your first PR to ot-sca! ;-)

At the moment, it's not fully clear to me what code is from XKCP and what code you contributed yourself, i.e., where I should focus with my review. I suggest that you try to use the OpenTitan vendor tool for including the XKCP sources and then things become more obvious for me and I can do a second pass.

Please just let me know if things are not clear or you need some help.

cw/cw305/kmac_cw_demo.py Outdated Show resolved Hide resolved
cw/cw305/pyXKCP/KeccakP-1600-SnP.h Outdated Show resolved Hide resolved
Copy link
Collaborator Author

@m-temp m-temp left a comment

Choose a reason for hiding this comment

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

I reworked the code as suggested. However, I need to place this single config file in the vendor directory, or patch all files to use a different location.
What's the best way to do it?
I tried to use the patch_dir config option, however it seems like the patch_dir can only be used for "real" patches, but not for creating files. CRITICAL: In hjson at cw/cw305/vendor/xkcp_xkcp.vendor.hjson, Has patch_dir, but no mapping item uses it.

As this file is needed by my build script I could dynamically create it during compile time. This would be similar to the upstream project where this file is created in the makefile.

Copy link
Collaborator

@vogelpi vogelpi left a comment

Choose a reason for hiding this comment

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

Thanks for updating this PR and using the vendor tool @m-temp . I've left some comments around formatting and linting + some other stuff. But this is definitely going in the right direction.

One general comment: Since this is a PR, it is completely fine to fixup/squash commits and do force push. Ultimately, this PR should have one or two commits.

cw/cw305/vendor/xkcp_xkcp/.gitignore Outdated Show resolved Hide resolved
cw/cw305/pyXKCP/README.md Outdated Show resolved Hide resolved
cw/cw305/pyXKCP/README.md Outdated Show resolved Hide resolved
cw/cw305/pyXKCP/.gitignore Outdated Show resolved Hide resolved
cw/cw305/kmac_cw_demo.py Outdated Show resolved Hide resolved
cw/cw305/pyXKCP/pyXKCP_extention_build.py Outdated Show resolved Hide resolved
cw/cw305/pyXKCP/pyXKCP_extention_build.py Outdated Show resolved Hide resolved
cw/cw305/pyXKCP/__init__.py Outdated Show resolved Hide resolved
cw/cw305/pyXKCP/pyXKCP.py Outdated Show resolved Hide resolved
cw/cw305/kmac_cw_demo.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@vogelpi vogelpi left a comment

Choose a reason for hiding this comment

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

Thanks @m-temp for addressing the feedback. I had to read up a bit on the __init__.py topic and I think calling the build function inside that is how this should be done. Sorry for raising this point in the first place. I also forgot to answer your question on the handling of the config file in the vendor dir. The solution you came up with looks okay to me, too.

Other than a couple of nits in the readme, this looks good to me, thanks for your work on this!

cw/cw305/pyXKCP/README.md Outdated Show resolved Hide resolved
cw/cw305/pyXKCP/README.md Outdated Show resolved Hide resolved
cw/cw305/pyXKCP/README.md Outdated Show resolved Hide resolved
cw/cw305/pyXKCP/README.md Outdated Show resolved Hide resolved
cw/cw305/pyXKCP/pyxkcp_build.py Outdated Show resolved Hide resolved
Most of the source and header files of the XKCP are released to the public
domain and associated to the CC0 deed. The exception is brg_endian.h which
is copyrighted by Brian Gladman and comes with a BSD 3-clause license;

Signed-off-by: Michael Tempelmeier <michael.tempelmeier@gi-de.com>
@m-temp m-temp changed the title [WIP] Kmac support KMAC support for Python scripts Sep 14, 2021
@m-temp m-temp marked this pull request as ready for review September 14, 2021 12:49
@vogelpi
Copy link
Collaborator

vogelpi commented Sep 17, 2021

This looks good, I've successfully tested this and integrated it into the capture (will file a PR shortly). I am merging this. Congrats @m-temp to your first PR :-)

@vogelpi vogelpi merged commit 77d894a into lowRISC:master Sep 17, 2021
@m-temp m-temp deleted the kmac-demo branch November 2, 2021 08:49
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.

None yet

2 participants