Page MenuHomeFreeBSD

ossl: Add AES-GCM implementation for ARMv7 NEON
Needs ReviewPublic

Authored by kd on Nov 17 2022, 2:51 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Jun 14, 1:47 PM
Unknown Object (File)
Fri, Jun 14, 9:10 AM
Unknown Object (File)
Fri, Jun 14, 3:16 AM
Unknown Object (File)
May 8 2024, 6:35 PM
Unknown Object (File)
May 7 2024, 2:50 PM
Unknown Object (File)
May 6 2024, 4:08 PM
Unknown Object (File)
May 6 2024, 8:00 AM
Unknown Object (File)
Apr 29 2024, 12:39 AM
Subscribers

Details

Summary

Add GCM implementation based on OSSL source code.
It uses dedicated openssl NEON assembly functions: gcm_init_neon, gcm_ghash_neon, gcm_gmult_neon.
Works with 12 byte IV and 16 byte tag only.
Tested on armada388-clearfog with NISTs vector set.

Diff Detail

Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

sys/crypto/openssl/ossl.c
160

style(9) wants to keep this blank line

sys/crypto/openssl/ossl_aes.c
61

Given the amount of modifications needed for GCM vs CBC compared to the duplication, I think it would be cleaner to have a separate osal_aes_gcm and leave osal_aes_cbc as-is. It's also worth it I think given the point is accelerated crypto to avoid at least some branches that way by using dedicated functions for the different modes.

113

This will not work with stream ciphers like GCM where payloads are not a multiple of the block size.

164
167
168

Tag mismatches should fail with EBADMSG not EINVAL

Please remove the file mode change to tests/sys/opencrypto/runtests.sh.

sys/crypto/openssl/ossl.h
70

I'm not sure if we need the Htable to be this big.
It's main use case is to precompute different values of H, so that later we can apply calculate ghash on multiple blocks at the same time.
From what I can see gcm_init_neon uses only a single value of H, so uint128_t Htable[1]; should be sufficient.
With a smaller context we might not need to increase the size kernel stack. (D37441)

Fix typo
Separate function for ossl_aes_cbc and ossl_aes_gcm
Revert tests/sys/opencrypto/runtests.sh
Change return EINVAL to EBADMSG
Decrease Htable size to single 128bit value

TODO: fix to work with stream ciphers

The biggest use case of AES-GCM is in the ipsec stack. Actually, that layer should be responsible for providing valid data to ossl. It is expected for the ossl function to fail if lengths are not aligned to block size (similar to what AES CBC does) as only the upper layers have the knowledge what algorithm has to be used for padding, if any. The AES-GCM RFC does not define any standard padding mechanism as this is the "stream encryption", but RFCs which make use of AES-GCM might have.

@mkoz_semihalf.com please ensure if all code paths have implemented the len % blocksize check and return EINVAL in that case.

In D37421#854943, @wma wrote:

The biggest use case of AES-GCM is in the ipsec stack. Actually, that layer should be responsible for providing valid data to ossl. It is expected for the ossl function to fail if lengths are not aligned to block size (similar to what AES CBC does) as only the upper layers have the knowledge what algorithm has to be used for padding, if any. The AES-GCM RFC does not define any standard padding mechanism as this is the "stream encryption", but RFCs which make use of AES-GCM might have.

@mkoz_semihalf.com please ensure if all code paths have implemented the len % blocksize check and return EINVAL in that case.

Huh? Rejecting that would be utterly wrong. AES-GCM as a stream cipher permits payloads that are not a multiple of a block size. tools/tools/crypto/cryptocheck explicitly checks for that for stream ciphers like AES-CTR and AES-GCM in its tests with -z as well. There is no padding for IPsec when using a stream cipher.

kd updated this revision to Diff 114096.
kd edited reviewers, added: mkoz_semihalf.com; removed: kd.
kd marked 5 inline comments as done.
kd edited the summary of this revision. (Show Details)
In D37421#855702, @jhb wrote:

Huh? Rejecting that would be utterly wrong. AES-GCM as a stream cipher permits payloads that are not a multiple of a block size. tools/tools/crypto/cryptocheck explicitly checks for that for stream ciphers like AES-CTR and AES-GCM in its tests with -z as well. There is no padding for IPsec when using a stream cipher.

I've fixed that, please take a look.
I've also added support for 192 and 256bit keys.

With those changed the NIST test suite(tests/sys/opencrypto) seems to work just fine.