summaryrefslogtreecommitdiff
path: root/modules/caddyhttp/caddyauth
diff options
context:
space:
mode:
authorMatthew Holt <mholt@users.noreply.github.com>2020-10-31 10:51:05 -0600
committerMatthew Holt <mholt@users.noreply.github.com>2020-10-31 10:51:05 -0600
commit937ec342010894f7a6239883b10f6a107ff82c9f (patch)
treee3dc861f6ac4e2675091b5c5b95d2ad3ffb96acb /modules/caddyhttp/caddyauth
parent966d5e6b42fc6da3da8bd39dd6ceceb8f1da3999 (diff)
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!
Diffstat (limited to 'modules/caddyhttp/caddyauth')
-rw-r--r--modules/caddyhttp/caddyauth/basicauth.go36
-rw-r--r--modules/caddyhttp/caddyauth/command.go6
-rw-r--r--modules/caddyhttp/caddyauth/hashes.go12
3 files changed, 47 insertions, 7 deletions
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)
)