From 937ec342010894f7a6239883b10f6a107ff82c9f Mon Sep 17 00:00:00 2001 From: Matthew Holt Date: Sat, 31 Oct 2020 10:51:05 -0600 Subject: 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! --- modules/caddyhttp/caddyauth/basicauth.go | 36 +++++++++++++++++++++++++++++--- modules/caddyhttp/caddyauth/command.go | 6 ++---- modules/caddyhttp/caddyauth/hashes.go | 12 +++++++++++ 3 files changed, 47 insertions(+), 7 deletions(-) (limited to 'modules/caddyhttp/caddyauth') diff --git a/modules/caddyhttp/caddyauth/basicauth.go b/modules/caddyhttp/caddyauth/basicauth.go index 92e1683..d955773 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 a0e971e..f445f67 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 5a3173e..63bfe1b 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) ) -- cgit v1.2.3