Conversation
Adds a crate with wrappers for the Data-Independent Timing (DIT) feature of AArch64 CPUs.
tarcieri
commented
Aug 25, 2024
| CARGO_INCREMENTAL: 0 | ||
| RUSTFLAGS: "-Dwarnings" | ||
|
|
||
| jobs: |
Member
Author
There was a problem hiding this comment.
FWIW I tried to see if it would work under cross in Linux, but unfortunately not:
---- tests::get stdout ----
thread 'tests::get' panicked at aarch64-dit/src/lib.rs:46:13:
DIT is not available on this CPU
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
Member
Author
|
Looks like this will need a higher MSRV: Edit: looks like it was stabilized in 1.61 |
tarcieri
commented
Aug 25, 2024
Comment on lines
+22
to
+35
| /// Enable DIT for the current thread. | ||
| #[target_feature(enable = "dit")] | ||
| pub unsafe fn set_dit_enabled() { | ||
| asm!("msr DIT, #1"); | ||
| } | ||
|
|
||
| /// Restore DIT state depending on the enabled bit. | ||
| #[target_feature(enable = "dit")] | ||
| pub unsafe fn restore_dit(enabled: bool) { | ||
| if !enabled { | ||
| // Disable DIT | ||
| asm!("msr DIT, #0"); | ||
| } | ||
| } |
Member
Author
There was a problem hiding this comment.
These could be combined into a single function like:
Suggested change
| /// Enable DIT for the current thread. | |
| #[target_feature(enable = "dit")] | |
| pub unsafe fn set_dit_enabled() { | |
| asm!("msr DIT, #1"); | |
| } | |
| /// Restore DIT state depending on the enabled bit. | |
| #[target_feature(enable = "dit")] | |
| pub unsafe fn restore_dit(enabled: bool) { | |
| if !enabled { | |
| // Disable DIT | |
| asm!("msr DIT, #0"); | |
| } | |
| } | |
| /// Enable DIT for the current thread. | |
| #[target_feature(enable = "dit")] | |
| pub unsafe fn set_dit_enabled(enabled: bool) { | |
| if enabled { | |
| asm!("msr DIT, #1"); | |
| } else { | |
| asm!("msr DIT, #0"); | |
| } | |
| } |
...however the current version does avoid duplicated msr calls in the event DIT is already enabled.
newpavlov
approved these changes
Aug 25, 2024
Member
Author
|
At some point we might consider having an ISA-independent crate for this sort of instruction pattern, provided we can actually build a portable abstraction over it: https://www.intel.com/content/www/us/en/developer/articles/technical/software-security-guidance/best-practices/data-operand-independent-timing-isa-guidance.html |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Adds a crate with wrappers for the Data-Independent Timing (DIT) feature of AArch64 CPUs.
The implementation is largely a translation of Apple's guide of how to write wrappers for enabling/disabling DIT: https://developer.apple.com/documentation/xcode/writing-arm64-code-for-apple-platforms#Enable-DIT-for-constant-time-cryptographic-operations
It would be nice to wrap that all up into an RAII guard which can first use
cpufeaturesto check forFEAT_DITand, if available, enable it for the current thread, while also first querying the processor status register and restoring the previous state onDrop, which is necessary for proper nested usage of DIT.But for now, this just wraps the barebones functionality in an
unsafeAPI.