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

Add support for AES GCM #341

Open
ZuluForce opened this issue Jan 6, 2020 · 3 comments
Open

Add support for AES GCM #341

ZuluForce opened this issue Jan 6, 2020 · 3 comments
Labels
enhancement New feature or request good first issue Good for newcomers Help Wanted Extra attention is needed

Comments

@ZuluForce
Copy link
Contributor

The title says it all. GCM is the only mode for AES in TLS 1.3 and even with TLS 1.2 the most commonly negotiated cipher suites are GCM.

I did see https://pagure.io/jss/issue/2 so apologies if you don't want two issues for it. The two methods of tracking issues don't appear to track one another and GitHub seems more active.

Besides the reasons I listed above, we would also like to use AES-NI on MacOS. Presently the NSS library will not use AES-NI for CBC mode on MacOS. It does for GCM and in general GCM is faster since it's an AEAD cipher mode.

@cipherboy cipherboy added enhancement New feature or request good first issue Good for newcomers Help Wanted Extra attention is needed labels Jan 6, 2020
@cipherboy
Copy link
Member

cipherboy commented Jan 6, 2020

When we complete #150 / #196, does the need for this go away? We're looking at the other ticket (number 1 on pagure) as higher priority than GCM currently. But SSLEngine is higher priority than both again.

This is nice to have, but I'd imagine (with #150 / #196) the need for this mostly disappears (outside of the keywrap use case).

Regardless, we have other AES modes already in NSS and JSS. It should be fairly easy to wire up GCM. Feel free to take a stab if you're interested and need this soon.

Edit: Also, have you raised the AES-NI bug to Mozilla? I'd appreciate a link to the ticket if so. It surprises me that they'd do it this way.

@ZuluForce
Copy link
Contributor Author

The SSLEngine implementation would remove our biggest need which is AES as used with TLS. There are some use cases I have for AES that's not part of TLS. I fully understand this sort of usage is a low priority for the maintainers.

As for the AES-NI, my colleague is going to ask on their mailing list. The assembly for the AES-NI as used in CBC mode was created about 11 years ago and only modified a bit since. The errors we get when trying to include the AES-NI assembly on MacOS is this:

intel-aes.s:27:2: error: unknown directive
 .type intel_aes_encrypt_init_128,@function
 ^
intel-aes.s:58:2: error: unknown directive
 .size intel_aes_encrypt_init_128, .-intel_aes_encrypt_init_128
 ^

Some small modifications will make it work but for our purposes we are not choosing to patch the NSS source.

Thanks for the speedy reply.

@cipherboy
Copy link
Member

Just in case anyone wants to pick this up in the future...

There's an existing parameter spec we should use.

The hard part is PK11Cipher only understands basic (IV-only) cipher block modes. We don't support (currently) IV + Tag length, as required by GCM. So we'd have to weave that in. Perhaps its better to switch to a generic cipher parameter like I've added in #306 (parameter spec access via NativeEnclosure). This refactoring should probably be a separate PR.

Additionally, I don't think we actually expose any cipher block modes currently. I think we only expose them as KeyWrapAlgorithms, which isn't part of the provider interface either. These both should probably be added as separate PRs.

Other than that, adding it via the existing structures should be fairly easy. Ideally GCM support would both be as a Cipher and as a KeyWrapAlgorithm.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers Help Wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants