From 938c39cd46463189f801e2936d8d18db9d54f3d9 Mon Sep 17 00:00:00 2001 From: Francis Lavoie Date: Thu, 21 Apr 2022 22:13:20 -0400 Subject: [PATCH 1/3] caddyauth: Speed up basicauth provisioning, precalculate fake password --- modules/caddyhttp/caddyauth/basicauth.go | 6 ++++-- modules/caddyhttp/caddyauth/hashes.go | 16 ++++++++++++++++ 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/modules/caddyhttp/caddyauth/basicauth.go b/modules/caddyhttp/caddyauth/basicauth.go index 33be70dff8b..b38e54e42cc 100644 --- a/modules/caddyhttp/caddyauth/basicauth.go +++ b/modules/caddyhttp/caddyauth/basicauth.go @@ -94,7 +94,7 @@ func (hba *HTTPBasicAuth) Provision(ctx caddy.Context) error { // 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")) + hba.fakePassword = hasher.FakeHash() if err != nil { return fmt.Errorf("generating anti-timing password hash: %v", err) } @@ -271,9 +271,11 @@ type Comparer interface { // 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. +// features. A hasher also returns a fake hash which +// can be used for timing side-channel mitigation. type Hasher interface { Hash(plaintext, salt []byte) ([]byte, error) + FakeHash() []byte } // Account contains a username, password, and salt (if applicable). diff --git a/modules/caddyhttp/caddyauth/hashes.go b/modules/caddyhttp/caddyauth/hashes.go index 63bfe1be456..ad9e3363631 100644 --- a/modules/caddyhttp/caddyauth/hashes.go +++ b/modules/caddyhttp/caddyauth/hashes.go @@ -16,6 +16,7 @@ package caddyauth import ( "crypto/subtle" + "encoding/base64" "github.com/caddyserver/caddy/v2" "golang.org/x/crypto/bcrypt" @@ -55,6 +56,13 @@ func (BcryptHash) Hash(plaintext, _ []byte) ([]byte, error) { return bcrypt.GenerateFromPassword(plaintext, 14) } +// FakeHash returns a fake hash. +func (BcryptHash) FakeHash() []byte { + // hashed with the following command: + // caddy hash-password --plaintext "antitiming" --algorithm "bcrypt" + return []byte("$2a$14$X3ulqf/iGxnf1k6oMZ.RZeJUoqI9PX2PM4rS5lkIKJXduLGXGPrt6") +} + // ScryptHash implements the scrypt KDF as a hash. type ScryptHash struct { // scrypt's N parameter. If unset or 0, a safe default is used. @@ -123,6 +131,14 @@ func (s ScryptHash) Hash(plaintext, salt []byte) ([]byte, error) { return scrypt.Key(plaintext, salt, s.N, s.R, s.P, s.KeyLength) } +// FakeHash returns a fake hash. +func (ScryptHash) FakeHash() []byte { + // hashed with the following command: + // caddy hash-password --plaintext "antitiming" --salt "fakesalt" --algorithm "scrypt" + bytes, _ := base64.StdEncoding.DecodeString("kFbjiVemlwK/ZS0tS6/UQqEDeaNMigyCs48KEsGUse8=") + return bytes +} + func hashesMatch(pwdHash1, pwdHash2 []byte) bool { return subtle.ConstantTimeCompare(pwdHash1, pwdHash2) == 1 } From d4c86831bed0c02f7163ff45124179741d164491 Mon Sep 17 00:00:00 2001 From: Francis Lavoie Date: Thu, 21 Apr 2022 23:11:43 -0400 Subject: [PATCH 2/3] Deprecate scrypt, allow using decoded bcrypt hashes --- modules/caddyhttp/caddyauth/basicauth.go | 15 ++++++++++++--- modules/caddyhttp/caddyauth/command.go | 11 +++++++---- modules/caddyhttp/caddyauth/hashes.go | 5 ++++- 3 files changed, 23 insertions(+), 8 deletions(-) diff --git a/modules/caddyhttp/caddyauth/basicauth.go b/modules/caddyhttp/caddyauth/basicauth.go index b38e54e42cc..83a82d9570b 100644 --- a/modules/caddyhttp/caddyauth/basicauth.go +++ b/modules/caddyhttp/caddyauth/basicauth.go @@ -21,6 +21,7 @@ import ( "fmt" weakrand "math/rand" "net/http" + "strings" "sync" "time" @@ -117,10 +118,18 @@ func (hba *HTTPBasicAuth) Provision(ctx caddy.Context) error { return fmt.Errorf("account %d: username and password are required", i) } - acct.password, err = base64.StdEncoding.DecodeString(acct.Password) - if err != nil { - return fmt.Errorf("base64-decoding password: %v", err) + // Passwords starting with '$' are likely in Modular Crypt Format, + // so we don't need to base64 decode them. But historically, we + // required redundant base64, so we try to decode it otherwise. + if strings.HasPrefix(acct.Password, "$") { + acct.password = []byte(acct.Password) + } else { + acct.password, err = base64.StdEncoding.DecodeString(acct.Password) + if err != nil { + return fmt.Errorf("base64-decoding password: %v", err) + } } + if acct.Salt != "" { acct.salt, err = base64.StdEncoding.DecodeString(acct.Salt) if err != nil { diff --git a/modules/caddyhttp/caddyauth/command.go b/modules/caddyhttp/caddyauth/command.go index 597681b6fc8..609de4e8e26 100644 --- a/modules/caddyhttp/caddyauth/command.go +++ b/modules/caddyhttp/caddyauth/command.go @@ -42,11 +42,13 @@ hash is written to stdout as a base64 string. Caddy is attached to a controlling tty, the plaintext will not be echoed. ---algorithm may be bcrypt or scrypt. If script, the default +--algorithm may be bcrypt or scrypt. If scrypt, the default parameters are used. Use the --salt flag for algorithms which require a salt to be provided (scrypt). + +Note that scrypt is deprecated. Please use 'bcrypt' instead. `, Flags: func() *flag.FlagSet { fs := flag.NewFlagSet("hash-password", flag.ExitOnError) @@ -112,13 +114,16 @@ func cmdHashPassword(fs caddycmd.Flags) (int, error) { } var hash []byte + var hashString string switch algorithm { case "bcrypt": hash, err = BcryptHash{}.Hash(plaintext, nil) + hashString = string(hash) case "scrypt": def := ScryptHash{} def.SetDefaults() hash, err = def.Hash(plaintext, salt) + hashString = base64.StdEncoding.EncodeToString(hash) default: return caddy.ExitCodeFailedStartup, fmt.Errorf("unrecognized hash algorithm: %s", algorithm) } @@ -126,9 +131,7 @@ func cmdHashPassword(fs caddycmd.Flags) (int, error) { return caddy.ExitCodeFailedStartup, err } - hashBase64 := base64.StdEncoding.EncodeToString(hash) - - fmt.Println(hashBase64) + fmt.Println(hashString) return 0, nil } diff --git a/modules/caddyhttp/caddyauth/hashes.go b/modules/caddyhttp/caddyauth/hashes.go index ad9e3363631..6505d187c55 100644 --- a/modules/caddyhttp/caddyauth/hashes.go +++ b/modules/caddyhttp/caddyauth/hashes.go @@ -64,6 +64,8 @@ func (BcryptHash) FakeHash() []byte { } // ScryptHash implements the scrypt KDF as a hash. +// +// DEPRECATED, please use 'bcrypt' instead. type ScryptHash struct { // scrypt's N parameter. If unset or 0, a safe default is used. N int `json:"N,omitempty"` @@ -88,8 +90,9 @@ func (ScryptHash) CaddyModule() caddy.ModuleInfo { } // Provision sets up s. -func (s *ScryptHash) Provision(_ caddy.Context) error { +func (s *ScryptHash) Provision(ctx caddy.Context) error { s.SetDefaults() + ctx.Logger(s).Warn("use of 'scrypt' is deprecated, please use 'bcrypt' instead") return nil } From 537125dffaa49e48d670ae7fc61e01b25e6a04a2 Mon Sep 17 00:00:00 2001 From: Francis Lavoie Date: Mon, 5 Sep 2022 15:21:47 -0400 Subject: [PATCH 3/3] Add TODO note Co-authored-by: Matt Holt --- modules/caddyhttp/caddyauth/basicauth.go | 1 + 1 file changed, 1 insertion(+) diff --git a/modules/caddyhttp/caddyauth/basicauth.go b/modules/caddyhttp/caddyauth/basicauth.go index 83a82d9570b..e090dacd7cd 100644 --- a/modules/caddyhttp/caddyauth/basicauth.go +++ b/modules/caddyhttp/caddyauth/basicauth.go @@ -118,6 +118,7 @@ func (hba *HTTPBasicAuth) Provision(ctx caddy.Context) error { return fmt.Errorf("account %d: username and password are required", i) } + // TODO: Remove support for redundantly-encoded b64-encoded hashes // Passwords starting with '$' are likely in Modular Crypt Format, // so we don't need to base64 decode them. But historically, we // required redundant base64, so we try to decode it otherwise.