caddyauth: Drop support for `scrypt` (#6091)

4 files changed, 22 insertions, 142 deletions

diff --git a/modules/caddyhttp/caddyauth/basicauth.go b/modules/caddyhttp/caddyauth/basicauth.go
index f30a8691a..52a5a08c1 100644
--- a/modules/caddyhttp/caddyauth/basicauth.go
+++ b/modules/caddyhttp/caddyauth/basicauth.go
@@ -108,7 +108,6 @@ func (hba *HTTPBasicAuth) Provision(ctx caddy.Context) error {
 
 		acct.Username = repl.ReplaceAll(acct.Username, "")
 		acct.Password = repl.ReplaceAll(acct.Password, "")
-		acct.Salt = repl.ReplaceAll(acct.Salt, "")
 
 		if acct.Username == "" || acct.Password == "" {
 			return fmt.Errorf("account %d: username and password are required", i)
@@ -127,13 +126,6 @@ func (hba *HTTPBasicAuth) Provision(ctx caddy.Context) error {
 			}
 		}
 
-		if acct.Salt != "" {
-			acct.salt, err = base64.StdEncoding.DecodeString(acct.Salt)
-			if err != nil {
-				return fmt.Errorf("base64-decoding salt: %v", err)
-			}
-		}
-
 		hba.Accounts[acct.Username] = acct
 	}
 	hba.AccountList = nil // allow GC to deallocate
@@ -172,7 +164,7 @@ func (hba HTTPBasicAuth) Authenticate(w http.ResponseWriter, req *http.Request)
 
 func (hba HTTPBasicAuth) correctPassword(account Account, plaintextPassword []byte) (bool, error) {
 	compare := func() (bool, error) {
-		return hba.Hash.Compare(account.password, plaintextPassword, account.salt)
+		return hba.Hash.Compare(account.password, plaintextPassword)
 	}
 
 	// if no caching is enabled, simply return the result of hashing + comparing
@@ -181,7 +173,7 @@ func (hba HTTPBasicAuth) correctPassword(account Account, plaintextPassword []by
 	}
 
 	// compute a cache key that is unique for these input parameters
-	cacheKey := hex.EncodeToString(append(append(account.password, account.salt...), plaintextPassword...))
+	cacheKey := hex.EncodeToString(append(account.password, plaintextPassword...))
 
 	// fast track: if the result of the input is already cached, use it
 	hba.HashCache.mu.RLock()
@@ -231,7 +223,7 @@ type Cache struct {
 	mu *sync.RWMutex
 	g  *singleflight.Group
 
-	// map of concatenated hashed password + plaintext password + salt, to result
+	// map of concatenated hashed password + plaintext password, to result
 	cache map[string]bool
 }
 
@@ -274,37 +266,33 @@ func (c *Cache) makeRoom() {
 // comparison.
 type Comparer interface {
 	// Compare returns true if the result of hashing
-	// plaintextPassword with salt is hashedPassword,
-	// false otherwise. An error is returned only if
+	// plaintextPassword is hashedPassword, false
+	// otherwise. An error is returned only if
 	// there is a technical/configuration error.
-	Compare(hashedPassword, plaintextPassword, salt []byte) (bool, error)
+	Compare(hashedPassword, plaintextPassword []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
+// given a plaintext. Hashing modules which implement
 // this interface can be used with the hash-password
 // subcommand as well as benefitting from anti-timing
 // 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)
+	Hash(plaintext []byte) ([]byte, error)
 	FakeHash() []byte
 }
 
-// Account contains a username, password, and salt (if applicable).
+// Account contains a username and password.
 type Account struct {
 	// A user's username.
 	Username string `json:"username"`
 
-	// The user's hashed password, base64-encoded.
+	// The user's hashed password, in Modular Crypt Format (with `$` prefix)
+	// or base64-encoded.
 	Password string `json:"password"`
 
-	// The user's password salt, base64-encoded; for
-	// algorithms where external salt is needed.
-	Salt string `json:"salt,omitempty"`
-
-	password, salt []byte
+	password []byte
 }
 
 // Interface guards
diff --git a/modules/caddyhttp/caddyauth/caddyfile.go b/modules/caddyhttp/caddyauth/caddyfile.go
index d46a2a881..cc92477e5 100644
--- a/modules/caddyhttp/caddyauth/caddyfile.go
+++ b/modules/caddyhttp/caddyauth/caddyfile.go
@@ -29,7 +29,7 @@ func init() {
 // parseCaddyfile sets up the handler from Caddyfile tokens. Syntax:
 //
 //	basic_auth [<matcher>] [<hash_algorithm> [<realm>]] {
-//	    <username> <hashed_password_base64> [<salt_base64>]
+//	    <username> <hashed_password>
 //	    ...
 //	}
 //
@@ -64,8 +64,6 @@ func parseCaddyfile(h httpcaddyfile.Helper) (caddyhttp.MiddlewareHandler, error)
 	switch hashName {
 	case "bcrypt":
 		cmp = BcryptHash{}
-	case "scrypt":
-		cmp = ScryptHash{}
 	default:
 		return nil, h.Errf("unrecognized hash algorithm: %s", hashName)
 	}
@@ -75,8 +73,8 @@ func parseCaddyfile(h httpcaddyfile.Helper) (caddyhttp.MiddlewareHandler, error)
 	for h.NextBlock(0) {
 		username := h.Val()
 
-		var b64Pwd, b64Salt string
-		h.Args(&b64Pwd, &b64Salt)
+		var b64Pwd string
+		h.Args(&b64Pwd)
 		if h.NextArg() {
 			return nil, h.ArgErr()
 		}
@@ -88,7 +86,6 @@ func parseCaddyfile(h httpcaddyfile.Helper) (caddyhttp.MiddlewareHandler, error)
 		ba.AccountList = append(ba.AccountList, Account{
 			Username: username,
 			Password: b64Pwd,
-			Salt:     b64Salt,
 		})
 	}
 
diff --git a/modules/caddyhttp/caddyauth/command.go b/modules/caddyhttp/caddyauth/command.go
index b93b7a402..c9f440060 100644
--- a/modules/caddyhttp/caddyauth/command.go
+++ b/modules/caddyhttp/caddyauth/command.go
@@ -17,7 +17,6 @@ package caddyauth
 import (
 	"bufio"
 	"bytes"
-	"encoding/base64"
 	"fmt"
 	"os"
 	"os/signal"
@@ -33,7 +32,7 @@ import (
 func init() {
 	caddycmd.RegisterCommand(caddycmd.Command{
 		Name:  "hash-password",
-		Usage: "[--algorithm <name>] [--salt <string>] [--plaintext <password>]",
+		Usage: "[--plaintext <password>] [--algorithm <name>]",
 		Short: "Hashes a password and writes base64",
 		Long: `
 Convenient way to hash a plaintext password. The resulting
@@ -43,17 +42,10 @@ 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 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.
+--algorithm currently only supports 'bcrypt', and is the default.
 `,
 		CobraFunc: func(cmd *cobra.Command) {
 			cmd.Flags().StringP("plaintext", "p", "", "The plaintext password")
-			cmd.Flags().StringP("salt", "s", "", "The password salt")
 			cmd.Flags().StringP("algorithm", "a", "bcrypt", "Name of the hash algorithm")
 			cmd.RunE = caddycmd.WrapCommandFuncForCobra(cmdHashPassword)
 		},
@@ -65,7 +57,6 @@ func cmdHashPassword(fs caddycmd.Flags) (int, error) {
 
 	algorithm := fs.String("algorithm")
 	plaintext := []byte(fs.String("plaintext"))
-	salt := []byte(fs.String("salt"))
 
 	if len(plaintext) == 0 {
 		fd := int(os.Stdin.Fd())
@@ -117,13 +108,8 @@ func cmdHashPassword(fs caddycmd.Flags) (int, error) {
 	var hashString string
 	switch algorithm {
 	case "bcrypt":
-		hash, err = BcryptHash{}.Hash(plaintext, nil)
+		hash, err = BcryptHash{}.Hash(plaintext)
 		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)
 	}
diff --git a/modules/caddyhttp/caddyauth/hashes.go b/modules/caddyhttp/caddyauth/hashes.go
index 324cf1e1e..ce3df901e 100644
--- a/modules/caddyhttp/caddyauth/hashes.go
+++ b/modules/caddyhttp/caddyauth/hashes.go
@@ -15,18 +15,13 @@
 package caddyauth
 
 import (
-	"crypto/subtle"
-	"encoding/base64"
-
 	"golang.org/x/crypto/bcrypt"
-	"golang.org/x/crypto/scrypt"
 
 	"github.com/caddyserver/caddy/v2"
 )
 
 func init() {
 	caddy.RegisterModule(BcryptHash{})
-	caddy.RegisterModule(ScryptHash{})
 }
 
 // BcryptHash implements the bcrypt hash.
@@ -41,7 +36,7 @@ func (BcryptHash) CaddyModule() caddy.ModuleInfo {
 }
 
 // Compare compares passwords.
-func (BcryptHash) Compare(hashed, plaintext, _ []byte) (bool, error) {
+func (BcryptHash) Compare(hashed, plaintext []byte) (bool, error) {
 	err := bcrypt.CompareHashAndPassword(hashed, plaintext)
 	if err == bcrypt.ErrMismatchedHashAndPassword {
 		return false, nil
@@ -53,7 +48,7 @@ func (BcryptHash) Compare(hashed, plaintext, _ []byte) (bool, error) {
 }
 
 // Hash hashes plaintext using a random salt.
-func (BcryptHash) Hash(plaintext, _ []byte) ([]byte, error) {
+func (BcryptHash) Hash(plaintext []byte) ([]byte, error) {
 	return bcrypt.GenerateFromPassword(plaintext, 14)
 }
 
@@ -64,94 +59,8 @@ func (BcryptHash) FakeHash() []byte {
 	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"`
-
-	// scrypt's r parameter. If unset or 0, a safe default is used.
-	R int `json:"r,omitempty"`
-
-	// scrypt's p parameter. If unset or 0, a safe default is used.
-	P int `json:"p,omitempty"`
-
-	// scrypt's key length parameter (in bytes). If unset or 0, a
-	// safe default is used.
-	KeyLength int `json:"key_length,omitempty"`
-}
-
-// CaddyModule returns the Caddy module information.
-func (ScryptHash) CaddyModule() caddy.ModuleInfo {
-	return caddy.ModuleInfo{
-		ID:  "http.authentication.hashes.scrypt",
-		New: func() caddy.Module { return new(ScryptHash) },
-	}
-}
-
-// Provision sets up s.
-func (s *ScryptHash) Provision(ctx caddy.Context) error {
-	s.SetDefaults()
-	ctx.Logger().Warn("use of 'scrypt' is deprecated, please use 'bcrypt' instead")
-	return nil
-}
-
-// SetDefaults sets safe default parameters, but does
-// not overwrite existing values. Each default parameter
-// is set independently; it does not check to ensure
-// that r*p < 2^30. The defaults chosen are those as
-// recommended in 2019 by
-// https://godoc.org/golang.org/x/crypto/scrypt.
-func (s *ScryptHash) SetDefaults() {
-	if s.N == 0 {
-		s.N = 32768
-	}
-	if s.R == 0 {
-		s.R = 8
-	}
-	if s.P == 0 {
-		s.P = 1
-	}
-	if s.KeyLength == 0 {
-		s.KeyLength = 32
-	}
-}
-
-// Compare compares passwords.
-func (s ScryptHash) Compare(hashed, plaintext, salt []byte) (bool, error) {
-	ourHash, err := scrypt.Key(plaintext, salt, s.N, s.R, s.P, s.KeyLength)
-	if err != nil {
-		return false, err
-	}
-	if hashesMatch(hashed, ourHash) {
-		return true, nil
-	}
-	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)
-}
-
-// 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
-}
-
 // Interface guards
 var (
-	_ Comparer          = (*BcryptHash)(nil)
-	_ Comparer          = (*ScryptHash)(nil)
-	_ Hasher            = (*BcryptHash)(nil)
-	_ Hasher            = (*ScryptHash)(nil)
-	_ caddy.Provisioner = (*ScryptHash)(nil)
+	_ Comparer = (*BcryptHash)(nil)
+	_ Hasher   = (*BcryptHash)(nil)
 )