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

math/bits: Add, Mul and Sub are not constant time #31229

Closed
FiloSottile opened this issue Apr 3, 2019 · 11 comments
Closed

math/bits: Add, Mul and Sub are not constant time #31229

FiloSottile opened this issue Apr 3, 2019 · 11 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@FiloSottile
Copy link
Contributor

These functions were added in #24813 to be intrinsified for high performance code like cryptography.

Cryptography code also needs to be constant time most of the time, so it would be good for the Go fallbacks to try to be constant time. (I realize that a smart compiler can rewrite them.)

The fallbacks are going to be slow anyway, so I don’t think there’s a big performance concern.

@FiloSottile FiloSottile added the NeedsFix The path to resolution is known, but the work has not been done. label Apr 3, 2019
@FiloSottile FiloSottile added this to the Go1.13 milestone Apr 3, 2019
@randall77
Copy link
Contributor

Mul looks to be constant time already.

@TuomLarsen
Copy link

Constant time operations are only useful for crypto and the extra slowness is undesirable in every other domain. Just saying...

@randall77
Copy link
Contributor

This issue is only about the Go fallbacks when intrinsics are not available for the target architecture.
The intrinsics are already constant time.

If your architecture doesn't have intrinsics, your code will already be extra slow. The fix in that case is to implement the intrinsics.

@FiloSottile
Copy link
Contributor Author

The goal is to have fast and safe intrinsics, it's why these functions were added. In the meantime, I'd rather have slow fallbacks than unsafe ones, though.

@TuomLarsen
Copy link

Not every architecture has add with carry, for example. If by "intrinsics" you mean e.g. ADC then it may very well happen that a non-constant time "fallback" will be faster than constant time implementation. What I try to say is that promising constant time fallbacks creates pressure for native implementations which except for crypto is not desirable, IMO.

@smasher164
Copy link
Member

The branchless, constant-time versions of Add and Sub from Hacker's Delight were originally proposed, but it was faster to compute the sum normally, and check a condition to compute the carryOut and borrowOut.

I have the constant-time version here that I can send in as a CL:
https://github.com/smasher164/extprec/blob/master/extprec.go

@FiloSottile
Copy link
Contributor Author

@smasher164 please do, yeah.

@gopherbot
Copy link

Change https://golang.org/cl/170758 mentions this issue: math/bits: make Add and Sub constant time

@rsc
Copy link
Contributor

rsc commented Apr 10, 2019

Discussion on #31267 about whether to change these.

@rsc
Copy link
Contributor

rsc commented Apr 10, 2019

Also, if someone wants to double-check the math/bits Add64 benchmark, that would be helpful. If it is literally never executing the 'carryOut = true' case, then it's a bad benchmark. It should do a mix of operations.

@FiloSottile
Copy link
Contributor Author

Looks like this is now a duplicate of #31267.

@golang golang locked and limited conversation to collaborators Apr 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

6 participants