8000 nRF52 AES Improvements by bradjc · Pull Request #3448 · tock/tock · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

nRF52 AES Improvements #3448

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

Merged
merged 7 commits into from
Jun 28, 2023
Merged

nRF52 AES Improvements #3448

merged 7 commits into from
Jun 28, 2023

Conversation

bradjc
Copy link
Contributor
@bradjc bradjc commented May 24, 2023

Pull Request Overview

This pull request:

  • Updates the nRF52 AES driver to support ECB and CBC, as well as in-place operations for ECB, CBC, and CTR.
  • Adds an AES component.
  • Adds the userspace AES driver to the nRF52840dk board.
  • Adds the AES tests to the nRF52840dk board.
  • Updates the AES tests to run all configurations (encryption first, both diff buffers and in place, then decryption).

Note, the nRF52 ECB module does not support decryption, so the ECB and CBC decryption tests fail.

This pull request is me getting familiar with the AES support on the nRF52 in hopes of using it for the Tock Tutorial.

Testing Strategy

Running the kernel AES tests.

TODO or Help Wanted

When I run the libtock-c/aes test app, I get:

[TEST] AES CTR Example Test
Loading in the key...
   done
Loading in the IV buf...
   done
Loading in the first half of the source buf...
   done
Setting up the first half of the dest buf...
   done
Setting up the callback...
   done
Starting the initial run...
   done
Loading in second half of the source buf...
   done
Running AES twice...
   done
Finish AES operation...
   done
Encrypted text: '���␒�␇	ѕ�6␐��ΓSY␕R'
Loading in the decryption source buf...
   done
Setting up the decryption dest buf...
   done
Running decryption...
   done
Finish AES operation...
   done
Original text: 'A language empowering everyone to build reliable and efficient software.'
[TEST] AES CCM Example Test
Loading in the key...
   done
Loading in the nonce buf...
   done
Loading in the source buf...
tock$    done
Setting up the dest buf...
   done
Setting up the callback...
   done
Starting the initial run...
   done
Finish AES operation...
   done
Comparison failure at 0
Expected value: 0x4a
 But got value: 0x2

So the AES CTR works, but the CCM does not. I did not change any of the CCM code. Do we know if that test is supposed to work? I believe the userland test app uses a different size IV than the in-kernel CCM test.

Nevermind, since we don't have GCM we cannot use the userspace driver.

Documentation Updated

  • Updated the relevant files in /docs, or no updates are required.

Formatting

  • Ran make prepush.

@github-actions github-actions bot added component nrf Change pertains to the nRF5x family of MCUs. labels May 24, 2023
Comment on lines +752 to +822
// test::aes_test::run_aes128_ctr(&base_peripherals.ecb);
// test::aes_test::run_aes128_cbc(&base_peripherals.ecb);
// test::aes_test::run_aes128_ecb(&base_peripherals.ecb);
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not use the unit test framework to run these instead of commenting them out?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's the right thing to do but not something I'm going to take on right now.

@github-actions github-actions bot added the WG-OpenTitan In the purview of the OpenTitan working group. label Jun 2, 2023
@bradjc
Copy link
Contributor Author
bradjc commented Jun 28, 2023

Rebased. Ready to go again.

Copy link
Member
@ppannuto ppannuto left a comment

Choose a reason for hiding this comment

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

It's unfortunate that the use of a static mut buffer for DMA also means those array accesses also end up in the unsafe block, but I don't think things are any meaningfully worse than before. Added some explainer comments for the unsafe's

@ppannuto ppannuto enabled auto-merge June 28, 2023 19:23
@ppannuto ppannuto added this pull request to the merge queue Jun 28, 2023
Merged via the queue into master with commit fa7e3e3 Jun 28, 2023
@ppannuto ppannuto deleted the nrf52-aes branch June 29, 2023 11:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component nrf Change pertains to the nRF5x family of MCUs. WG-OpenTitan In the purview of the OpenTitan working group.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants
0