From 2caaec3d8b7318d29bf6e996a7319c4d3b466cd8 Mon Sep 17 00:00:00 2001 From: Alberto Bertogli Date: Wed, 26 Apr 2017 10:26:54 +0100 Subject: [PATCH] userdb: Use a constant-time byte comparison in PasswordMatches PasswordMatches calculates the proposed derived key, and then compares it with the actual derived key. That comparison is done using bytes.Equal, which is not in constant time. In theory, users with knowledge of the salt could use timing to extract information about the actual derived key. In practice, the salt is not being exposed to users, and the caller of PasswordMatches will add a delay to password checks, so it should not be easy to exploit via chasquid. But just to be safe and more future-proof, this patch changes the comparison to be in constant time. --- internal/userdb/userdb.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/internal/userdb/userdb.go b/internal/userdb/userdb.go index 182cc72..83f9942 100644 --- a/internal/userdb/userdb.go +++ b/internal/userdb/userdb.go @@ -33,8 +33,8 @@ package userdb //go:generate protoc --go_out=. userdb.proto import ( - "bytes" "crypto/rand" + "crypto/subtle" "errors" "fmt" "sync" @@ -210,5 +210,7 @@ func (s *Scrypt) PasswordMatches(plain string) bool { panic(fmt.Sprintf("scrypt failed: %v", err)) } - return bytes.Equal(dk, []byte(s.Encrypted)) + // This comparison should be high enough up the stack that it doesn't + // matter, but do it in constant time just in case. + return subtle.ConstantTimeCompare(dk, []byte(s.Encrypted)) == 1 }