git » chasquid » commit 2caaec3

userdb: Use a constant-time byte comparison in PasswordMatches

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.

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
 }