Skip to content

Commit

Permalink
caddyauth: Drop support for scrypt
Browse files Browse the repository at this point in the history
  • Loading branch information
francislavoie committed Feb 10, 2024
1 parent c78ebb3 commit 948bc41
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 140 deletions.
36 changes: 12 additions & 24 deletions modules/caddyhttp/caddyauth/basicauth.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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()
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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
Expand Down
9 changes: 3 additions & 6 deletions modules/caddyhttp/caddyauth/caddyfile.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ func init() {
// parseCaddyfile sets up the handler from Caddyfile tokens. Syntax:
//
// basicauth [<matcher>] [<hash_algorithm> [<realm>]] {
// <username> <hashed_password_base64> [<salt_base64>]
// <username> <hashed_password>
// ...
// }
//
Expand Down Expand Up @@ -58,8 +58,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)
}
Expand All @@ -69,8 +67,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()
}
Expand All @@ -82,7 +80,6 @@ func parseCaddyfile(h httpcaddyfile.Helper) (caddyhttp.MiddlewareHandler, error)
ba.AccountList = append(ba.AccountList, Account{
Username: username,
Password: b64Pwd,
Salt: b64Salt,
})
}

Expand Down
20 changes: 3 additions & 17 deletions modules/caddyhttp/caddyauth/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ package caddyauth
import (
"bufio"
"bytes"
"encoding/base64"
"fmt"
"os"
"os/signal"
Expand All @@ -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
Expand All @@ -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)
},
Expand All @@ -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())
Expand Down Expand Up @@ -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)
}
Expand Down
97 changes: 4 additions & 93 deletions modules/caddyhttp/caddyauth/hashes.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,17 +16,14 @@ package caddyauth

import (
"crypto/subtle"

Check failure on line 18 in modules/caddyhttp/caddyauth/hashes.go

View workflow job for this annotation

GitHub Actions / govulncheck

"crypto/subtle" imported and not used

Check failure on line 18 in modules/caddyhttp/caddyauth/hashes.go

View workflow job for this annotation

GitHub Actions / lint (linux)

"crypto/subtle" imported and not used (typecheck)

Check failure on line 18 in modules/caddyhttp/caddyauth/hashes.go

View workflow job for this annotation

GitHub Actions / lint (linux)

"crypto/subtle" imported and not used) (typecheck)
"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.
Expand All @@ -41,7 +38,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
Expand All @@ -53,7 +50,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)
}

Expand All @@ -64,94 +61,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)
)

0 comments on commit 948bc41

Please sign in to comment.