Skip to content

Conversation

wysiwys
Copy link
Contributor

@wysiwys wysiwys commented Sep 30, 2025

Rebased onto main from aes-gcm:


[AES-GCM] Cleanup to prepare the new libcrux-aesgcm crate for release

  • Addresses the points in [AES-GCM] Cleanup to prepare crate for release #1166
  • Reorganize module structure, so that the exported Key, Nonce, Tag type aliases can be found in a module that corresponds to each implementation
  • Rename crate libcrux_aesgcm -> libcrux-aesgcm (for consistency with other crates in this repository)
  • Apply clippy lints
  • Update tests, benchmarks and examples to use libcrux_traits-based public APIs (the ones defined in typed_owned and typed_refs)
  • Move libcrux_traits API implementation into separate module
    • E.g. libcrux_aesgcm::aes_gcm_128::neon::{Key, Nonce, Tag}

One other point that was part of release prep/cleanup for this crate:

  • Deciding which API will be the main public API for this crate
  • Currently, this crate exposes the strongly-typed, key-centric APIs defined in libcrux_traits::aead as the main public APIs. Usage examples for these APIs are provided in the crate documentation and in the examples/ directory.

Closes #1166

@wysiwys wysiwys self-assigned this Sep 30, 2025
@wysiwys wysiwys changed the title Wysiwys/aes gcm cleanup [AES-GCM] Cleanup to prepare crate for release Sep 30, 2025
@wysiwys wysiwys force-pushed the wysiwys/aes-gcm-cleanup branch from 03b27a5 to f8c6bb9 Compare September 30, 2025 09:59
Copy link
Member

@franziskuskiefer franziskuskiefer left a comment

Choose a reason for hiding this comment

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

Looks like there's an AesGcm128 (and 256) in the top level as well as in the aes_gcm_128 module. That's a little odd? Looks like we have multiple APIs exposed here for the same thing.

@wysiwys wysiwys marked this pull request as ready for review September 30, 2025 16:11
@wysiwys wysiwys requested review from a team as code owners September 30, 2025 16:11
@wysiwys wysiwys requested review from jschneider-bensch and removed request for a team September 30, 2025 16:11
@wysiwys wysiwys changed the base branch from aes-gcm to main October 1, 2025 11:22
@wysiwys wysiwys changed the base branch from main to aes-gcm October 1, 2025 11:23
@wysiwys wysiwys force-pushed the wysiwys/aes-gcm-cleanup branch from 5e07eb8 to 4cd7a74 Compare October 1, 2025 11:39
@wysiwys wysiwys changed the base branch from aes-gcm to main October 1, 2025 11:39
Copy link
Collaborator

@jschneider-bensch jschneider-bensch left a comment

Choose a reason for hiding this comment

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

I think this looks pretty good!
Just one check should be added from the initial discussion.

@github-actions github-actions bot dismissed stale reviews from franziskuskiefer and jschneider-bensch October 1, 2025 14:09

Review re-requested

aad: &[u8],
plaintext: &[u8],
) -> Result<(), EncryptError> {
if plaintext.len() / crate::aes::AES_BLOCK_LEN >= (u32::MAX - 1) as usize {
Copy link
Member

Choose a reason for hiding this comment

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

There's lots of code duplication here. That's not great. We should move all checks into the common functions instead.
These implementations shouldn't do much more than passing things along to the actual functions, picking the right one, and converting arguments where necessary.

pub(crate) use aesgcm;

/// Helper module for implementing platform-specific modules
macro_rules! platform_mod {
Copy link
Member

Choose a reason for hiding this comment

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

Do we really need all these types? The types aren't different for different platforms.

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.

[AES-GCM] Cleanup to prepare crate for release
4 participants