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

Initial support for RSA in ESP32-H2 #526

Merged
merged 6 commits into from
May 15, 2023

Conversation

SergioGasquez
Copy link
Member

Adds initial support for RSA and the rsa example.

I've used the rsa/esp32cX.rs which I think should work for ESP32-H2 but not sure if we should update the name of it.

Tested the rsa example and here is the output:

it took 348770 cycles for hw modular exponentiation
it took 19064457 cycles for sw modular exponentiation
modular exponentiation done
it took 4395 cycles for hw modular multiplication
it took 19947 cycles for sw modular multiplication
modular multiplication done
it took 3992 cycles for hw multiplication
it took 13476 cycles for sw multiplication
multiplication done

Copy link
Member

@MabezDev MabezDev left a comment

Choose a reason for hiding this comment

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

I don't really have an opinions on renaming the file, I guess it doesn't matter too much to me.

Could you add an entry to the CHANGELOG.md please?

@SergioGasquez SergioGasquez changed the title Initial support for RSA Initial support for RSA in ESP32-H2 May 11, 2023
@SergioGasquez
Copy link
Member Author

Could you add an entry to the CHANGELOG.md please?

Updated! Not sure how the workflow for updating it should be. Open a PR and then update the CHANGELOG, so we can add the PR number to it?

@bjoernQ
Copy link
Contributor

bjoernQ commented May 11, 2023

Could you add an entry to the CHANGELOG.md please?

Updated! Not sure how the workflow for updating it should be. Open a PR and then update the CHANGELOG, so we can add the PR number to it?

Probably, yes! In theory you can also guess the next PR number if no one else submits a PR at the same moment in time

@jessebraham
Copy link
Member

Could you add an entry to the CHANGELOG.md please?

Updated! Not sure how the workflow for updating it should be. Open a PR and then update the CHANGELOG, so we can add the PR number to it?

Two options I see are:

  1. Push an additional commit once the PR is open
  2. Just add the text to the CHANGELOG, any missing PR numbers can be updated prior to release.

Copy link
Member

@jessebraham jessebraham left a comment

Choose a reason for hiding this comment

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

Conflicts need resolving but LGTM otherwise, thanks!

@SergioGasquez
Copy link
Member Author

Just noticed there was some duplicated coded in the rsa examples of H2 and C6, just removed it in 8ee8a68

Copy link
Member

@MabezDev MabezDev left a comment

Choose a reason for hiding this comment

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

LGTM!

@MabezDev MabezDev merged commit 70e4539 into esp-rs:main May 15, 2023
@SergioGasquez SergioGasquez deleted the feature/esp32h2-rsa branch May 15, 2023 08:44
SergioGasquez added a commit to SergioGasquez/esp-hal that referenced this pull request Jun 9, 2023
* feat: ✨ Initial support for RSA

* docs: 📝 Update docstring

* docs: 📝 Update changelog

* fix: 🔥 Remove duplicated code
i404788 pushed a commit to i404788/esp-hal that referenced this pull request Jul 22, 2023
* feat: ✨ Initial support for RSA

* docs: 📝 Update docstring

* docs: 📝 Update changelog

* fix: 🔥 Remove duplicated code
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.

4 participants