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

aarch64-dit: initial crate #1102

Merged
merged 4 commits into from
Sep 6, 2024
Merged

aarch64-dit: initial crate #1102

merged 4 commits into from
Sep 6, 2024

Conversation

tarcieri
Copy link
Member

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 cpufeatures to check for FEAT_DIT and, if available, enable it for the current thread, while also first querying the processor status register and restoring the previous state on Drop, which is necessary for proper nested usage of DIT.

But for now, this just wraps the barebones functionality in an unsafe API.

Adds a crate with wrappers for the Data-Independent Timing (DIT) feature
of AArch64 CPUs.
@tarcieri tarcieri marked this pull request as draft August 25, 2024 18:01
CARGO_INCREMENTAL: 0
RUSTFLAGS: "-Dwarnings"

jobs:
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@tarcieri
Copy link
Member Author

tarcieri commented Aug 25, 2024

Looks like this will need a higher MSRV:

error[E0658]: the target feature `dit` is currently unstable
  --> aarch64-dit/src/lib.rs:[15](https://github.com/RustCrypto/utils/actions/runs/10549036451/job/29223626727?pr=1102#step:5:16):18
   |
15 | #[target_feature(enable = "dit")]
   |                  ^^^^^^^^^^^^^^
   |
   = note: see issue #44839 <https://github.com/rust-lang/rust/issues/44839> for more information

Edit: looks like it was stabilized in 1.61

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");
}
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

aarch64-dit/src/lib.rs Outdated Show resolved Hide resolved
@tarcieri
Copy link
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

@tarcieri tarcieri changed the title [WIP] aarch64-dit: initial crate aarch64-dit: initial crate Sep 6, 2024
@tarcieri tarcieri marked this pull request as ready for review September 6, 2024 02:46
@tarcieri tarcieri merged commit 4be6789 into master Sep 6, 2024
93 checks passed
@tarcieri tarcieri deleted the aarch64-dit branch September 6, 2024 02:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants