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.

Sign up for GitHub

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

std.mem: constant time slice comparison for cryptographic primitives #1695

Closed
wants to merge 1 commit into from
Closed

std.mem: constant time slice comparison for cryptographic primitives #1695

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions std/mem.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,7 @@ test "mem.lessThan" {
}

/// Compares two slices and returns whether they are equal.
/// Use `constant_time_eql` inside cryptographic primitives
pub fn eql(comptime T: type, a: []const T, b: []const T) bool {
if (a.len != b.len) return false;
for (a) |item, index| {
Expand All @@ -254,6 +255,27 @@ pub fn eql(comptime T: type, a: []const T, b: []const T) bool {
return true;
}

/// Compares two slices and returns whether they are equal in constant time.
Copy link
Contributor

Choose a reason for hiding this comment

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

It doesn't, does it? It returns T, not bool.

If you change the return type to bool, you run into the issue that a sufficiently smart compiler may short-circuit the loop.

I suppose that, in theory, that's already possible with zig's closed world compilation model. LLVM can look at the contexts in which a function is called and specialize when it infers a boolean context. I don't think it's actually smart enough right now to pull that off but that's shaky ground to be on.

Rather than @noInlineCall, what is probably needed here is an attribute that says "DWIM, don't optimize at all." Such an attribute doesn't exist at the moment but it'd be noinline optnone instead of just noinline.

Alternatively, maybe it's possible to convince llvm that all loads are volatile and cannot be elided. However, I'm not sure how strong llvm's guarantees are here. Does any kind of alias analysis weaken it?

FWIW, libraries like openssl implement constant-time operations in asm because, even though it's a bigger maintenance hassle, it's easier to reason about the run-time behavior.

Copy link
Member

Choose a reason for hiding this comment

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

I actually think it might be worth having a compiler builtin for a constant time equality check. Trying to implement this in userland is fighting against a foundational premise that the rest of the codebase shares: that the goal of the compiler is to interpret the semantic meaning and then emit whatever machine code it can to make it go fast. Here the goal is different: make it go the same amount of time regardless of the input.

The benefit of a compiler builtin is that the compiler would understand how to do the operation at compile-time, as well as lower to whatever code is appropriate per-architecture, be it LLVM IR with optnone or assembly.

One thing I'd like to find out is whether there is a need for more constant time primitives. Looking at https://github.com/pornin/CTTK for inspiration, it seems they have API for all manner of integer operations. I wonder what operations crypto libraries depend on.

pub fn constant_time_eql(comptime T: type, a: []const T, b: []const T) T {
//TODO: https://github.com/ziglang/zig/issues/640
return @noInlineCall(noinline__constant_time_eql, T, a, b);
}

fn noinline__constant_time_eql(comptime T: type, a: []const T, b: []const T) T {
if (a.len != b.len) return 1;
var tmp: T = 0;
var count: usize = a.len - 1;
while (count != 0) : (count -= 1) {
tmp |= a[count] ^ b[count];
Copy link
Contributor

Choose a reason for hiding this comment

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

These loads may leak timing information for some instances of T when a or b are or aren't naturally aligned for T, since unaligned loads are usually much slower. The simple workaround is to only accept u8.

Copy link
Member

Choose a reason for hiding this comment

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

In this code, all members of a and b are guaranteed by the type system to be naturally aligned. Slices whose pointers are under- or over-aligned have an align annotation, e.g.: a: []align(1) const T

}
return tmp;
}

test "mem.constant_time_eql" {
assert(constant_time_eql(u8, "abc", "abc") == 0);
assert(constant_time_eql(u8, "abc", "abd") != 0);
}

pub fn len(comptime T: type, ptr: [*]const T) usize {
var count: usize = 0;
while (ptr[count] != 0) : (count += 1) {}
Expand Down