dogtagpki / jss

Network Security Services for Java is a Java interface to NSS
https://dogtagpki.github.io/jss
19 stars 30 forks source link

Add support for AES GCM #341

Open ZuluForce opened 4 years ago

ZuluForce commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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.