caddyauth: Speed up basicauth provision, deprecate scrypt (#4720)

* caddyauth: Speed up basicauth provisioning, precalculate fake password

* Deprecate scrypt, allow using decoded bcrypt hashes

* Add TODO note

Co-authored-by: Matt Holt <mholt@users.noreply.github.com>

Co-authored-by: Matt Holt <mholt@users.noreply.github.com>

3 files changed, 44 insertions, 10 deletions

diff --git a/modules/caddyhttp/caddyauth/basicauth.go b/modules/caddyhttp/caddyauth/basicauth.go
index 33be70d..e090dac 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"
 
@@ -94,7 +95,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)
 		}
@@ -117,10 +118,19 @@ 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)
+		// 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.
+		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 {
@@ -271,9 +281,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/command.go b/modules/caddyhttp/caddyauth/command.go
index 597681b..609de4e 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 63bfe1b..6505d18 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,7 +56,16 @@ 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.
+//
+// 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"`
@@ -80,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
 }
 
@@ -123,6 +134,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
 }