aes-gcm: Clarify CPU feature detection. #2106
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
briansmith
force-pushed
the
b/aes-features-refactor-1
branch
from
48c8fe8
to
e2a4800
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2106 +/- ##
==========================================
+ Coverage 97.07% 97.12% +0.05%
==========================================
Files 144 151 +7
Lines 20124 20101 -23
Branches 456 447 -9
==========================================
- Hits 19536 19524 -12
+ Misses 492 482 -10
+ Partials 96 95 -1 ☔ View full report in Codecov by Sentry. |
briansmith
force-pushed
the
b/aes-features-refactor-1
branch
5 times, most recently
from
5f52f1d
to
63f67a7
Compare
Although every key has been represented with the same types `aes::AES_KEY` and `gcm::HTable` regardless of which implementation is used, in reality those types are polymorphic in ways that aren't captured by the type system currently. Thus, the `set_encrypt_key!` function must be matched with the corresponding `encrypt_block!` and/or `ctr32_encrypt_blocks!` function. Previously, we did CPU feature detection for each function call and assumed that CPU feature detection is idempotent. Now, we do CPU feature detection during key construction and make the lesser assumption that at least those same CPU features are available as long as the key exists. This is a step towards making further improvements in CPU-feature-based dispatching. This makes code coverage reporting a little clearer. For example, it became clearer that the x86 VPAES implementation wasn't being tested in CI; this will be rectified in another commit. One side-effect of this change is that GCM keys (and thus AES-GCM keys) are now much smaller on targets that don't support any assembly implementation, as they now just store a single `U128` instead of a whole `HTable`. Another nice effect is that the dead ctr32_encrypt_blocks implementation for aarch64 is no longer needed. ``` git difftool HEAD^1:src/aead/aes.rs src/aead/aes/bs.rs git difftool HEAD^1:src/aead/aes.rs src/aead/aes/vp.rs ```
briansmith
force-pushed
the
b/aes-features-refactor-1
branch
from
63f67a7
to
9ce7475
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Although every key has been represented with the same types
aes::AES_KEYandgcm::HTableregardless of which implementation is used, in reality those types are polymorphic in ways that aren't captured by the type system currently. Thus, theset_encrypt_key!function must be matched with the correspondingencrypt_block!and/orctr32_encrypt_blocks!function. Previously, we did CPU feature detection for each function call and assumed that CPU feature detection is idempotent. Now, we do CPU feature detection during key construction and make the lesser assumption that at least those same CPU features are available as long as the key exists.This is a step towards making further improvements in CPU-feature-based dispatching.
One side-effect of this change is that GCM keys (and thus AES-GCM keys) are now much smaller on targets that don't support any assembly implementation, as they now just store a single
U128instead of a wholeHTable.