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

capsules: Add support for AES GCM #3092

Merged
merged 4 commits into from
Jun 7, 2023
Merged

Conversation

alistair23
Copy link
Contributor

@alistair23 alistair23 commented Jul 18, 2022

Pull Request Overview

This PR adds a new aes_gcm capsule. This capsule exposes AES-GCM support if the hardware doesn't actually have it.

We implement AES GCM using other supported AES operations and with a software implementation of ghash.

This is similar to the existing virtual_aes_ccm capsule, except without virtulisation.

AES-GCM is commonly used in TLS. It isn't commonly used in embedded systems, but it's still a good example I think.

Testing Strategy

Running the OpenTitan tests

TODO or Help Wanted

This PR adds an external dependency!

This PR adds a dependency on the ghash crate as part of Rust-crypto.

We are seeing more and more requirements for cryptographic operations in Tock, which means we are also seeing more and more software implementations of them.

Tock already has a SHA256 and CRC32 software implementation. It's not unreasonable that RSA becomes more common with the cryptographic app IDs as well.

I wanted to start the discussion about allowing Rust-crypto crates as capsules. Re-implementing a crypto library ourselves seem very risky, so if we do want software implementations Rust-crypto would be the way to go. I'm not convinced one way or the other, I just wanted to get some feedback on how people feel.

The Rust-crypto crates are well respected, have been audited and match the Tock software license

Documentation Updated

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

Formatting

  • Ran make prepush.

@github-actions github-actions bot added the WG-OpenTitan In the purview of the OpenTitan working group. label Jul 18, 2022
@phil-levis phil-levis self-assigned this Jul 29, 2022
static mut AES: Option<
&virtual_aes_gcm::VirtualAES128GCM<
'static,
virtual_aes_ccm::VirtualAES128CCM<'static, earlgrey::aes::Aes<'static>>,
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is GCM building on top of CCM? They are different algorithms.

Copy link
Contributor

Choose a reason for hiding this comment

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

@alistair23 Can you explain your intended relationship between CCM and GCM?

@phil-levis
Copy link
Contributor

Tock already has a SHA256 and CRC32 software implementation. It's not unreasonable that RSA becomes more common with the cryptographic app IDs as well.

I wanted to start the discussion about allowing Rust-crypto crates as capsules. Re-implementing a crypto library ourselves seem very risky, so if we do want software implementations Rust-crypto would be the way to go. I'm not convinced one way or the other, I just wanted to get some feedback on how people feel.

This has been discussed in the core call. The general consensus is that Tock should not have any cipher implementations used for confidentiality. E.g., there should not be Tock-specific implementations of AES or RSA. However, having simple software implementations of integrity primitives (e.g., hashes and CRCs) is OK. The expectation is that these are included to allow software testing without introducing dependencies.

The point you bring up, though, that some Tock boards are going to need software implementations of encryption/decryption and we should pull in crates to do so, is a good one. I will put it on the core call agenda.

@alistair23
Copy link
Contributor Author

This has been discussed in the core call. The general consensus is that Tock should not have any cipher implementations used for confidentiality. E.g., there should not be Tock-specific implementations of AES or RSA. However, having simple software implementations of integrity primitives (e.g., hashes and CRCs) is OK. The expectation is that these are included to allow software testing without introducing dependencies.

This PR only adds a software hash implementation. Does that mean you agree this should be added? Or do you mean Tock is ok with writing it's own implementations?

The point you bring up, though, that some Tock boards are going to need software implementations of encryption/decryption and we should pull in crates to do so, is a good one. I will put it on the core call agenda.

I was thinking a bit more about this.

Once we have verified AppIDs I could see a future where the default research loading mechanism is to try loading signed apps, with an allow fallback. This way we wouldn't break any users (if it's not signed we still load the app) but also provides a lot more test coverage on signed app loading. We can then update the tools to handle this as transparently as possible.

As well as that, there are lots of good Tock use cases that need crypto. LoRaWAN and other wireless communication stacks all use crypto by default.

These would only work if we had a default software crypto implementation though. So I think it's something worth considering

@phil-levis
Copy link
Contributor

This PR only adds a software hash implementation. Does that mean you agree this should be added? Or do you mean Tock is ok with writing it's own implementations?

It does not mean that I agree it should be added. It means that it's something we need to discuss to explore all of the issues in play. Tock (so far) is OK writing its own software implementations of integrity primitives, such as hashes, checksums, and CRCs. However, that doesn't mean that it's the preferred thing to do.

I was thinking a bit more about this.

Once we have verified AppIDs I could see a future where the default research loading mechanism is to try loading signed apps, with an allow fallback. This way we wouldn't break any users (if it's not signed we still load the app) but also provides a lot more test coverage on signed app loading. We can then update the tools to handle this as transparently as possible.

This is already supported in the API. But the policies are more complex than this; there's a question of what signing keys are allowed. The current API requires the checker know something about the key, due to the exponent being implicit. We might make the exponent explicit, depending on requirements.

As well as that, there are lots of good Tock use cases that need crypto. LoRaWAN and other wireless communication stacks all use crypto by default.

True. Most chips (LoRa being an exception) have cryptographic support, though. The issue here is introducing external dependencies into the kernel for narrow use cases. We've resisted external dependencies for a long time, for simplicity. That doesn't mean the kernel won't ever use them, but for now it doesn't.

@phil-levis
Copy link
Contributor

phil-levis commented Aug 6, 2022

Notes from the OT call:

A virtual_aes_gcm capsule should virtualize an AES-GCM implementation for multiple clients. The current capsule should decompose the composition of AES-GCM from AES-CCM and virtualization into two separate capsules. The first is a translation that turns an AES implementation (and perhaps a GCM implementation) into AES-GCM. The second virtualizes AES-GCM.

The fact that the current AES-CCM virtualizer isn't decomposed this way and combines translation and virtualization shouldn't be taken as a reference design. The current code started with an early, 5-year-old AES-CCM implementation and extended the capsule to add virtualization.

@phil-levis
Copy link
Contributor

@alistair23 Any progress on this?

@alistair23
Copy link
Contributor Author

I'm still waiting for an Ack or Nack about adding external crypto crates

@phil-levis
Copy link
Contributor

@alistair23 I will bring it up on Friday just to be sure, but I am very confident that Tock will allow external (high quality, vetted) implementations of cryptographic primitives.

@alistair23
Copy link
Contributor Author

Ok, updated to remove the virtuliser part from this.

Once this is merged we can then look at adding a single AES virtuliser (while also removing the virtulisation from virtual_aes_ccm.rs)

@alistair23 alistair23 marked this pull request as ready for review November 2, 2022 08:55
@alistair23 alistair23 mentioned this pull request Nov 3, 2022
2 tasks
@alistair23
Copy link
Contributor Author

Any more comments? I can rebase this if it will be accepted

bors bot added a commit that referenced this pull request May 12, 2023
3312: doc: Add ExternalDependencies.md r=hudson-ayers a=alistair23

### Pull Request Overview

This adds a document that describes some of the requirements of adding external crates.

Once this is accepted the idea is then to add an external crate with #3092

### Testing Strategy

### TODO or Help Wanted

### Documentation Updated

- [X] Updated the relevant files in `/docs`, or no updates are required.

### Formatting

- [X] Ran `make prepush`.


Co-authored-by: Alistair Francis <alistair.francis@wdc.com>
Co-authored-by: Brad Campbell <bradjc5@gmail.com>
Co-authored-by: Leon Schuermann <leon@is.currently.online>
Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
@alistair23 alistair23 force-pushed the alistair/aes-gcm branch 2 times, most recently from 5f3b018 to 0264fbc Compare May 15, 2023 06:52
@alistair23
Copy link
Contributor Author

I have rebased this now that the ExternalDependencies.md doc has been merged

phil-levis
phil-levis previously approved these changes May 17, 2023
Copy link
Contributor

@phil-levis phil-levis left a comment

Choose a reason for hiding this comment

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

Please add a comment explaining the reason behind the CCM/GCM composition (decorator pattern for API, to enable both), as well as a README with the cargo tree from the new crate. Then it's good to go.

Initial commit of a GCM capsule. This implements AES GCM
support using an underlying AES CTR engine and the external GHash crate.

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
@alistair23
Copy link
Contributor Author

Done!

@bradjc
Copy link
Contributor

bradjc commented Jun 7, 2023

bors r+

@bors
Copy link
Contributor

bors bot commented Jun 7, 2023

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@alistair23 alistair23 closed this Jun 7, 2023
@bors bors bot merged commit f5bd104 into tock:master Jun 7, 2023
15 checks passed
@alistair23 alistair23 deleted the alistair/aes-gcm branch June 7, 2023 23:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
WG-OpenTitan In the purview of the OpenTitan working group.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants