caddyauth: Prevent user enumeration by timing

Always follow the code path of hashing and comparing a plaintext
password even if the account is not found by the given username; this
ensures that similar CPU cycles are spent for both valid and invalid
usernames.

Thanks to @tylerlm for helping and looking into this!

3 files changed, 47 insertions, 7 deletions

diff --git a/modules/caddyhttp/caddyauth/basicauth.go b/modules/caddyhttp/caddyauth/basicauth.go
index 92e1683ad..d95577305 100644
--- a/modules/caddyhttp/caddyauth/basicauth.go
+++ b/modules/caddyhttp/caddyauth/basicauth.go
@@ -52,11 +52,19 @@ type HTTPBasicAuth struct {
 	// memory for a longer time (this should not be a problem
 	// as long as your machine is not compromised, at which point
 	// all bets are off, since basicauth necessitates plaintext
-	// passwords being received over the wire anyway).
+	// passwords being received over the wire anyway). Note that
+	// a cache hit does not mean it is a valid password.
 	HashCache *Cache `json:"hash_cache,omitempty"`
 
 	Accounts map[string]Account `json:"-"`
 	Hash     Comparer           `json:"-"`
+
+	// fakePassword is used when a given user is not found,
+	// so that timing side-channels can be mitigated: it gives
+	// us something to hash and compare even if the user does
+	// not exist, which should have similar timing as a user
+	// account that does exist.
+	fakePassword []byte
 }
 
 // CaddyModule returns the Caddy module information.
@@ -84,6 +92,14 @@ func (hba *HTTPBasicAuth) Provision(ctx caddy.Context) error {
 		return fmt.Errorf("hash is required")
 	}
 
+	// if supported, generate a fake password we can compare against if needed
+	if hasher, ok := hba.Hash.(Hasher); ok {
+		hba.fakePassword, err = hasher.Hash([]byte("antitiming"), []byte("fakesalt"))
+		if err != nil {
+			return fmt.Errorf("generating anti-timing password hash: %v", err)
+		}
+	}
+
 	repl := caddy.NewReplacer()
 
 	// load account list
@@ -132,8 +148,12 @@ func (hba HTTPBasicAuth) Authenticate(w http.ResponseWriter, req *http.Request)
 	}
 
 	account, accountExists := hba.Accounts[username]
-	// don't return early if account does not exist; we want
-	// to try to avoid side-channels that leak existence
+	if !accountExists {
+		// don't return early if account does not exist; we want
+		// to try to avoid side-channels that leak existence, so
+		// we use a fake password to simulate realistic CPU cycles
+		account.password = hba.fakePassword
+	}
 
 	same, err := hba.correctPassword(account, []byte(plaintextPasswordStr))
 	if err != nil {
@@ -249,6 +269,16 @@ type Comparer interface {
 	Compare(hashedPassword, plaintextPassword, salt []byte) (bool, error)
 }
 
+// Hasher is a type that can generate a secure hash
+// given a plaintext and optional salt (for algorithms
+// that require a salt). Hashing modules which implement
+// this interface can be used with the hash-password
+// subcommand as well as benefitting from anti-timing
+// features.
+type Hasher interface {
+	Hash(plaintext, salt []byte) ([]byte, error)
+}
+
 // Account contains a username, password, and salt (if applicable).
 type Account struct {
 	// A user's username.
diff --git a/modules/caddyhttp/caddyauth/command.go b/modules/caddyhttp/caddyauth/command.go
index a0e971ed5..f445f67d6 100644
--- a/modules/caddyhttp/caddyauth/command.go
+++ b/modules/caddyhttp/caddyauth/command.go
@@ -25,8 +25,6 @@ import (
 
 	"github.com/caddyserver/caddy/v2"
 	caddycmd "github.com/caddyserver/caddy/v2/cmd"
-	"golang.org/x/crypto/bcrypt"
-	"golang.org/x/crypto/scrypt"
 	"golang.org/x/crypto/ssh/terminal"
 )
 
@@ -116,11 +114,11 @@ func cmdHashPassword(fs caddycmd.Flags) (int, error) {
 	var hash []byte
 	switch algorithm {
 	case "bcrypt":
-		hash, err = bcrypt.GenerateFromPassword(plaintext, 14)
+		hash, err = BcryptHash{}.Hash(plaintext, nil)
 	case "scrypt":
 		def := ScryptHash{}
 		def.SetDefaults()
-		hash, err = scrypt.Key(plaintext, salt, def.N, def.R, def.P, def.KeyLength)
+		hash, err = def.Hash(plaintext, salt)
 	default:
 		return caddy.ExitCodeFailedStartup, fmt.Errorf("unrecognized hash algorithm: %s", algorithm)
 	}
diff --git a/modules/caddyhttp/caddyauth/hashes.go b/modules/caddyhttp/caddyauth/hashes.go
index 5a3173eb4..63bfe1be4 100644
--- a/modules/caddyhttp/caddyauth/hashes.go
+++ b/modules/caddyhttp/caddyauth/hashes.go
@@ -50,6 +50,11 @@ func (BcryptHash) Compare(hashed, plaintext, _ []byte) (bool, error) {
 	return true, nil
 }
 
+// Hash hashes plaintext using a random salt.
+func (BcryptHash) Hash(plaintext, _ []byte) ([]byte, error) {
+	return bcrypt.GenerateFromPassword(plaintext, 14)
+}
+
 // ScryptHash implements the scrypt KDF as a hash.
 type ScryptHash struct {
 	// scrypt's N parameter. If unset or 0, a safe default is used.
@@ -113,6 +118,11 @@ func (s ScryptHash) Compare(hashed, plaintext, salt []byte) (bool, error) {
 	return false, nil
 }
 
+// Hash hashes plaintext using the given salt.
+func (s ScryptHash) Hash(plaintext, salt []byte) ([]byte, error) {
+	return scrypt.Key(plaintext, salt, s.N, s.R, s.P, s.KeyLength)
+}
+
 func hashesMatch(pwdHash1, pwdHash2 []byte) bool {
 	return subtle.ConstantTimeCompare(pwdHash1, pwdHash2) == 1
 }
@@ -121,5 +131,7 @@ func hashesMatch(pwdHash1, pwdHash2 []byte) bool {
 var (
 	_ Comparer          = (*BcryptHash)(nil)
 	_ Comparer          = (*ScryptHash)(nil)
+	_ Hasher            = (*BcryptHash)(nil)
+	_ Hasher            = (*ScryptHash)(nil)
 	_ caddy.Provisioner = (*ScryptHash)(nil)
 )