Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/configurable hashing and encoding #355

Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions auth/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@ import (
"context"
"net/http"

"golang.org/x/crypto/bcrypt"

"github.com/volatiletech/authboss/v3"
)

Expand Down Expand Up @@ -77,7 +75,7 @@ func (a *Auth) LoginPost(w http.ResponseWriter, r *http.Request) error {
r = r.WithContext(context.WithValue(r.Context(), authboss.CTXKeyUser, pidUser))

var handled bool
err = bcrypt.CompareHashAndPassword([]byte(password), []byte(creds.GetPassword()))
err = a.Authboss.Core.Hasher.CompareHashAndPassword(password, creds.GetPassword())
if err != nil {
handled, err = a.Authboss.Events.FireAfter(authboss.EventAuthFail, w, r)
if err != nil {
Expand Down
1 change: 1 addition & 0 deletions auth/auth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ func testSetup() *testHarness {

harness.ab.Config.Core.BodyReader = harness.bodyReader
harness.ab.Config.Core.Logger = mocks.Logger{}
harness.ab.Config.Core.Hasher = mocks.Hasher{}
harness.ab.Config.Core.Responder = harness.responder
harness.ab.Config.Core.Redirector = harness.redirector
harness.ab.Config.Storage.SessionState = harness.session
Expand Down
20 changes: 16 additions & 4 deletions authboss.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,15 @@ func New() *Authboss {

// Init authboss, modules, renderers
func (a *Authboss) Init(modulesToLoad ...string) error {
// Create the hasher
// Have to do it in Init for backwards compatibility.
// If a user did not previously use defaults.SetCore() then they will
// suddenly start getting panics
// We also cannot use config.Defaults() so we can respect the user's BCryptCost
if a.Config.Core.Hasher == nil {
a.Config.Core.Hasher = NewBCryptHasher(a.Config.Modules.BCryptCost)
}

if len(modulesToLoad) == 0 {
modulesToLoad = RegisteredModules()
}
Expand All @@ -65,12 +74,12 @@ func (a *Authboss) Init(modulesToLoad ...string) error {
// in sessions for a user requires special mechanisms not currently provided
// by authboss.
func (a *Authboss) UpdatePassword(ctx context.Context, user AuthableUser, newPassword string) error {
pass, err := bcrypt.GenerateFromPassword([]byte(newPassword), a.Config.Modules.BCryptCost)
pass, err := a.Config.Core.Hasher.GenerateHash(newPassword)
if err != nil {
return err
}

user.PutPassword(string(pass))
user.PutPassword(pass)

storer := a.Config.Storage.Server
if err := storer.Save(ctx, user); err != nil {
Expand All @@ -89,14 +98,17 @@ func (a *Authboss) UpdatePassword(ctx context.Context, user AuthableUser, newPas
// Returns nil on success otherwise there will be an error. Simply a helper
// to do the bcrypt comparison.
func VerifyPassword(user AuthableUser, password string) error {
// TODO: function can be used ONLY if no custom hasher was configured in global ab.config
// function should be either deprecated, or he we should have access to global ab's config
// (also, we can't use defaults.NewBcryptHasher, because it will be cyclic dep)
return bcrypt.CompareHashAndPassword([]byte(user.GetPassword()), []byte(password))
}

// MWRequirements are user requirements for authboss.Middleware
// in order to access the routes in protects. Requirements is a bit-set integer
// to be able to easily combine requirements like so:
//
// authboss.RequireFullAuth | authboss.Require2FA
// authboss.RequireFullAuth | authboss.Require2FA
type MWRequirements int

// MWRespondOnFailure tells authboss.Middleware how to respond to
Expand Down Expand Up @@ -153,7 +165,7 @@ func MountedMiddleware(ab *Authboss, mountPathed, redirectToLogin, forceFullAuth
//
// requirements are set by logical or'ing together requirements. eg:
//
// authboss.RequireFullAuth | authboss.Require2FA
// authboss.RequireFullAuth | authboss.Require2FA
//
// failureResponse is how the middleware rejects the users that don't meet
// the criteria. This should be chosen from the MWRespondOnFailure constants.
Expand Down
1 change: 1 addition & 0 deletions authboss_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ func TestAuthbossUpdatePassword(t *testing.T) {

ab := New()
ab.Config.Storage.Server = storer
ab.Config.Core.Hasher = mockHasher{}

if err := ab.UpdatePassword(context.Background(), user, "hello world"); err != nil {
t.Error(err)
Expand Down
9 changes: 9 additions & 0 deletions config.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ type Config struct {

Modules struct {
// BCryptCost is the cost of the bcrypt password hashing function.
// Deprecated: Use Hasher instead.
BCryptCost int

// ConfirmMethod IS DEPRECATED! See MailRouteMethod instead.
Expand Down Expand Up @@ -239,6 +240,12 @@ type Config struct {
// Mailer is the mailer being used to send e-mails out via smtp
Mailer Mailer

// Hasher hashes passwords into hashes
Hasher Hasher

// CredsGenerator generates credentials (selector+verified+token)
CredsGenerator CredsGenerator

// Logger implies just a few log levels for use, can optionally
// also implement the ContextLogger to be able to upgrade to a
// request specific logger.
Expand Down Expand Up @@ -272,4 +279,6 @@ func (c *Config) Defaults() {
c.Modules.MailRouteMethod = http.MethodGet
c.Modules.RecoverLoginAfterRecovery = false
c.Modules.RecoverTokenDuration = 24 * time.Hour

c.Core.CredsGenerator = NewSha512CredsGenerator()
}
35 changes: 5 additions & 30 deletions confirm/confirm.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,9 @@ package confirm

import (
"context"
"crypto/rand"
"crypto/sha512"
"crypto/subtle"
"encoding/base64"
"fmt"
"io"
"net/http"
"net/url"
"path"
Expand All @@ -33,9 +30,6 @@ const (
// DataConfirmURL is the name of the e-mail template variable
// that gives the url to send to the user for confirmation.
DataConfirmURL = "url"

confirmTokenSize = 64
confirmTokenSplit = confirmTokenSize / 2
)

func init() {
Expand Down Expand Up @@ -128,7 +122,7 @@ func (c *Confirm) StartConfirmationWeb(w http.ResponseWriter, r *http.Request, h
func (c *Confirm) StartConfirmation(ctx context.Context, user authboss.ConfirmableUser, sendEmail bool) error {
logger := c.Authboss.Logger(ctx)

selector, verifier, token, err := GenerateConfirmCreds()
selector, verifier, token, err := c.Authboss.Core.CredsGenerator.GenerateCreds()
if err != nil {
return err
}
Expand Down Expand Up @@ -198,13 +192,14 @@ func (c *Confirm) Get(w http.ResponseWriter, r *http.Request) error {
return c.invalidToken(w, r)
}

if len(rawToken) != confirmTokenSize {
credsGenerator := c.Authboss.Core.CredsGenerator

if len(rawToken) != credsGenerator.TokenSize() {
logger.Infof("invalid confirm token submitted, size was wrong: %d", len(rawToken))
return c.invalidToken(w, r)
}

selectorBytes := sha512.Sum512(rawToken[:confirmTokenSplit])
verifierBytes := sha512.Sum512(rawToken[confirmTokenSplit:])
selectorBytes, verifierBytes := credsGenerator.ParseToken(string(rawToken))
selector := base64.StdEncoding.EncodeToString(selectorBytes[:])

storer := authboss.EnsureCanConfirm(c.Authboss.Config.Storage.Server)
Expand Down Expand Up @@ -296,23 +291,3 @@ func Middleware(ab *authboss.Authboss) func(http.Handler) http.Handler {
})
}
}

// GenerateConfirmCreds generates pieces needed for user confirm
// selector: hash of the first half of a 64 byte value
// (to be stored in the database and used in SELECT query)
// verifier: hash of the second half of a 64 byte value
// (to be stored in database but never used in SELECT query)
// token: the user-facing base64 encoded selector+verifier
func GenerateConfirmCreds() (selector, verifier, token string, err error) {
rawToken := make([]byte, confirmTokenSize)
if _, err = io.ReadFull(rand.Reader, rawToken); err != nil {
return "", "", "", err
}
selectorBytes := sha512.Sum512(rawToken[:confirmTokenSplit])
verifierBytes := sha512.Sum512(rawToken[confirmTokenSplit:])

return base64.StdEncoding.EncodeToString(selectorBytes[:]),
base64.StdEncoding.EncodeToString(verifierBytes[:]),
base64.URLEncoding.EncodeToString(rawToken),
nil
}
11 changes: 8 additions & 3 deletions confirm/confirm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ func testSetup() *testHarness {

harness.ab.Config.Core.BodyReader = harness.bodyReader
harness.ab.Config.Core.Logger = mocks.Logger{}
harness.ab.Config.Core.Hasher = mocks.Hasher{}
harness.ab.Config.Core.Mailer = harness.mailer
harness.ab.Config.Core.Redirector = harness.redirector
harness.ab.Config.Core.MailRenderer = harness.renderer
Expand Down Expand Up @@ -176,7 +177,7 @@ func TestGetSuccess(t *testing.T) {

harness := testSetup()

selector, verifier, token, err := GenerateConfirmCreds()
selector, verifier, token, err := harness.ab.Config.Core.CredsGenerator.GenerateCreds()
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -271,7 +272,7 @@ func TestGetUserNotFoundFailure(t *testing.T) {

harness := testSetup()

_, _, token, err := GenerateConfirmCreds()
_, _, token, err := harness.ab.Config.Core.CredsGenerator.GenerateCreds()
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -380,7 +381,9 @@ func TestMailURL(t *testing.T) {
func TestGenerateRecoverCreds(t *testing.T) {
t.Parallel()

selector, verifier, token, err := GenerateConfirmCreds()
credsGenerator := authboss.NewSha512CredsGenerator()

selector, verifier, token, err := credsGenerator.GenerateCreds()
if err != nil {
t.Error(err)
}
Expand All @@ -389,6 +392,8 @@ func TestGenerateRecoverCreds(t *testing.T) {
t.Error("the verifier and selector should be different")
}

confirmTokenSplit := credsGenerator.TokenSize() / 2

// base64 length: n = 64; 4*(64/3) = 85.3; round to nearest 4: 88
if len(verifier) != 88 {
t.Errorf("verifier length was wrong (%d): %s", len(verifier), verifier)
Expand Down
65 changes: 65 additions & 0 deletions creds_generator.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
package authboss

import (
"crypto/rand"
"crypto/sha512"
"encoding/base64"
"io"
)

type CredsGenerator interface {
// GenerateCreds generates pieces needed as credentials
// selector: to be stored in the database and used in SELECT query
// verifier: to be stored in database but never used in SELECT query
// token: the user-facing base64 encoded selector+verifier
GenerateCreds() (selector, verifier, token string, err error)

ParseToken(token string) (selectorBytes, verifierBytes []byte)

TokenSize() int
}


const (
tokenSize = 64
tokenSplit = tokenSize / 2
)

type Sha512CredsGenerator struct{}

func NewSha512CredsGenerator() *Sha512CredsGenerator {
return &Sha512CredsGenerator{}
}

// GenerateCreds generates pieces needed as credentials
// selector: hash of the first half of an N byte value
// (to be stored in the database and used in SELECT query)
// verifier: hash of the second half of an N byte value
// (to be stored in database but never used in SELECT query)
// token: the user-facing base64 encoded selector+verifier
func (cg *Sha512CredsGenerator) GenerateCreds() (selector, verifier, token string, err error) {
rawToken := make([]byte, tokenSize)
if _, err = io.ReadFull(rand.Reader, rawToken); err != nil {
return "", "", "", err
}

selectorBytes := sha512.Sum512(rawToken[:tokenSplit])
verifierBytes := sha512.Sum512(rawToken[tokenSplit:])

return base64.StdEncoding.EncodeToString(selectorBytes[:]),
base64.StdEncoding.EncodeToString(verifierBytes[:]),
base64.URLEncoding.EncodeToString(rawToken),
nil
}

func (cg *Sha512CredsGenerator) ParseToken(rawToken string) (selectorBytes, verifierBytes []byte) {
selectorBytes64 := sha512.Sum512([]byte(rawToken)[:tokenSplit])
selectorBytes = selectorBytes64[:]

verifierBytes64 := sha512.Sum512([]byte(rawToken)[tokenSplit:])
verifierBytes = verifierBytes64[:]

return
}

func (cg *Sha512CredsGenerator) TokenSize() int { return tokenSize }
40 changes: 40 additions & 0 deletions creds_generator_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
package authboss

import (
"encoding/base64"
"testing"
)

func TestCredsGenerator(t *testing.T) {
t.Parallel()

credsGenerator := NewSha512CredsGenerator()

selector, verifier, tokenEncoded, err := credsGenerator.GenerateCreds()
if err != nil {
t.Error(err)
}

// let's decode the token
tokenBytes, err := base64.URLEncoding.DecodeString(tokenEncoded)
token := string(tokenBytes)

if len(token) != credsGenerator.TokenSize() {
t.Error("token size is invalid", len(token))
}

selectorBytes, verifierBytes := credsGenerator.ParseToken(token)

// encode back and verify

selectorParsed := base64.StdEncoding.EncodeToString(selectorBytes[:])
verifierParsed := base64.StdEncoding.EncodeToString(verifierBytes[:])

if selectorParsed != selector {
t.Error("selector generated wrong", selector, selectorParsed)
}

if verifierParsed != verifier {
t.Error("verifier generated wrong", verifier, verifierParsed)
}
}
15 changes: 11 additions & 4 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,22 @@ module github.com/volatiletech/authboss/v3

go 1.19

require (
github.com/alexedwards/argon2id v1.0.0
github.com/friendsofgo/errors v0.9.2
github.com/pquerna/otp v1.4.0
golang.org/x/crypto v0.14.0
golang.org/x/oauth2 v0.6.0
)

require (
cloud.google.com/go v0.34.0 // indirect
github.com/boombuler/barcode v1.0.1 // indirect
github.com/davecgh/go-spew v1.1.1 // indirect
github.com/friendsofgo/errors v0.9.2
github.com/golang/protobuf v1.5.3 // indirect
github.com/pquerna/otp v1.4.0
golang.org/x/crypto v0.7.0
golang.org/x/oauth2 v0.6.0
golang.org/x/net v0.10.0 // indirect
golang.org/x/sys v0.13.0 // indirect
golang.org/x/xerrors v0.0.0-20220907171357-04be3eba64a2 // indirect
google.golang.org/appengine v1.6.7 // indirect
google.golang.org/protobuf v1.29.0 // indirect
)
Loading