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

Add mem.secureEql #3553

Closed
wants to merge 1 commit into from
Closed

Conversation

lukechampine
Copy link
Contributor

Constant-time comparisons prevent timing attacks in sensitive crypto code. For example, Go's poly1305 implementation uses a constant-time routine to verify MACs. Now Zig users can do the same with mem.secureEql. (But since this is easy to forget, it may be prudent to add a verify function to Zig's Poly1305 struct, and elsewhere.)

Currently this function only works on integer types. I imagine it could be made to work with other types as well, but I'm very new to Zig so I didn't attempt to do so.

@andrewrk
Copy link
Member

This is unsound. See #1776 for an explanation and attempt to address this.

In summary the compiler is allowed to figure out that this is a memory comparison operation and replace the implementation with something that is not constant time.

Until #1776 is closed, the use case of constant time operations must be done with a third party library, inline assembly, or a heavy handed approach such as delaying the result until a fixed amount of time has transpired.

@andrewrk andrewrk closed this Oct 29, 2019
@lukechampine lukechampine deleted the secure-eql branch October 29, 2019 04:52
@lukechampine
Copy link
Contributor Author

my bad, should have searched first. Looking forward to seeing how Zig solves this!

@daurnimator daurnimator added the standard library This issue involves writing Zig code for the standard library. label Oct 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
standard library This issue involves writing Zig code for the standard library.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants