Skip to content

Commit

Permalink
caddyauth: Prevent user enumeration by timing
Browse files Browse the repository at this point in the history
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!
  • Loading branch information
mholt committed Oct 31, 2020
1 parent 966d5e6 commit 937ec34
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 7 deletions.
36 changes: 33 additions & 3 deletions modules/caddyhttp/caddyauth/basicauth.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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.
Expand Down
6 changes: 2 additions & 4 deletions modules/caddyhttp/caddyauth/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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)
}
Expand Down
12 changes: 12 additions & 0 deletions modules/caddyhttp/caddyauth/hashes.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
}
Expand All @@ -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)
)

0 comments on commit 937ec34

Please sign in to comment.