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

crypto: set Data Independent Timing flag on arm64 #49702

Open
FiloSottile opened this issue Nov 21, 2021 · 4 comments
Open

crypto: set Data Independent Timing flag on arm64 #49702

FiloSottile opened this issue Nov 21, 2021 · 4 comments
Assignees
Labels
arch-arm64 NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. Security
Milestone

Comments

@FiloSottile
Copy link
Contributor

Someone pointed me to this terrifying piece of arm64 documentation.

https://developer.arm.com/documentation/ddi0595/2021-06/AArch64-Registers/DIT--Data-Independent-Timing

It's a flag that ensures that "the timing of every load and store instruction is insensitive to the value of the data being loaded or stored" and "for certain data processing instructions, the instruction takes a time which is independent of: the values of the data supplied in any of its registers".

The nightmare fuel implication is that with this flag unset, instructions like MUL and LOAD might take different time based on the value they operate on. Due to the potential for timing attacks, this is not acceptable in any cryptographic code.

We should probably put a call-and-defer at each public crypto entry point that on arm64 detects the feature and set/unsets the flag. Setting it for the whole program might have an unacceptable performance cost, even if marginal.

I'll think about requesting a freeze exception, depending on the implementation details.

@FiloSottile FiloSottile added Security NeedsFix The path to resolution is known, but the work has not been done. arch-arm64 labels Nov 21, 2021
@FiloSottile FiloSottile added this to the Go1.18 milestone Nov 21, 2021
@FiloSottile FiloSottile self-assigned this Nov 21, 2021
@erifan
Copy link

erifan commented Nov 22, 2021

DIT is introduced since arm v8.4, are there any considerations for the safety of versions before 8.4?

We should probably put a call-and-defer at each public crypto entry point that on arm64 detects the feature and set/unsets the flag.

We may need to consider this situation. The function is preempted before it unsets the flag. Then the goroutine may be switched to run on other m, and other goroutines may also be switched to run on this m. The setting of the flag for different m is inconsistent.

@ianlancetaylor
Copy link
Contributor

@FiloSottile @golang/security This is in the 1.18 milestone; time to move to 1.19? Thanks.

@FiloSottile FiloSottile modified the milestones: Go1.18, Backlog Mar 2, 2022
@FiloSottile FiloSottile added NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. and removed NeedsFix The path to resolution is known, but the work has not been done. labels Mar 2, 2022
@FiloSottile
Copy link
Contributor Author

Linux is discussing how to handle this and the new Intel equivalent. https://lore.kernel.org/lkml/Ywzr2d52ixYXUDWR@zx2c4.com/T/#mfcba14511c69461bd8921fef758baae120d090dc

@AGSaidi
Copy link

AGSaidi commented Sep 9, 2022

https://developer.arm.com/documentation/ka005181/latest

The intention of FEAT_DIT is to provide an architectural guarantee that an architecturally defined set of data processioning instructions would be data value timing invariant.

Arm is not aware of any Arm processor implementations from before the introduction of FEAT_DIT that exhibit data value timing variation for the instructions covered by FEAT_DIT, but this behavior was not required by the architecture.

...
Some Arm processor implementations which implement FEAT_DIT (such as the Neoverse-V1) do not exhibit data value timing variation for the instructions covered by FEAT_DIT, irrespective of the PSTATE.DIT setting. This being said, Arm strongly recommends that software transitions to adopting the correct usage of the architectural feature.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arch-arm64 NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. Security
Projects
None yet
Development

No branches or pull requests

4 participants