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

Zkt: explicitely consider timing dependences across several instructions #136

Closed
ronan-lashermes opened this issue Oct 25, 2021 · 2 comments

Comments

@ronan-lashermes
Copy link

ronan-lashermes commented Oct 25, 2021

The Concern

The Public Review raised the concern that
Data dependent optimizations are possible at the level of a block of instructions, but the Zkt extension does not mention what is authorized in this case; or if it is out of scope.
Made-up example:

mul a3, a2, a1
mul a6, a5, a4
add a7, a3, a6

At runtime, the processor observes that a5 = a2, and that a3, a6 are overwritten just after. It can replace the instructions with

add a3, a1, a4
mul a7, a3, a5

The instructions are compliant with Zkt, but timing leakage still occurs.
This particular optimization may not exist, but similar data dependent optimizations are often performed in OoO cores.

The recommendation

Explicitly state what is in scope of the extension concerning blocks of instructions. If in scope, decide what is authorized.

@ben-marshall
Copy link
Member

Hi @ronan-lashermes

Good catch, the spec could certainly be clearer here. The intent with Zkt is that it only the core functionality of the instructions is in scope. Everything outside of that from branch target buffers to caches to the kinds of fusion / substitution and OoO optimisations you describe here are out of scope. I figure we can add some text to the spec which makes it clear the example you describe is not considered.

@mjosaarinen - Have I got that right?

Cheers,
Ben

ben-marshall added a commit that referenced this issue Oct 27, 2021
- See #136 for context.

 On branch dev/next-release
 Your branch is up-to-date with 'origin/dev/next-release'.

 Changes to be committed:
	modified:   doc/scalar/riscv-crypto-scalar-zkt.adoc
ben-marshall added a commit that referenced this issue Oct 27, 2021
- See #136 for context.

 On branch dev/next-release
 Your branch is up-to-date with 'origin/dev/next-release'.

 Changes to be committed:
	modified:   doc/scalar/riscv-crypto-scalar-zkt.adoc
ben-marshall added a commit that referenced this issue Oct 29, 2021
Proposed clarifying words for Zkt addressing #136
@ben-marshall
Copy link
Member

Closed with the merge of #137

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

No branches or pull requests

2 participants