Skip to content

Commit

Permalink
bench/bug: caching bcrypt function for auth (#1696)
Browse files Browse the repository at this point in the history
Fixes #1622 Fixes #1640

bcrypt is slow by design. We are able to get approx 10rps for basic auth
natively due to bcrypt.

bench-no-cache          10     137680538 ns/op     1343793 B/op
26030 allocs/op
bench-cache-valid     1000       1689715 ns/op      104782 B/op
774 allocs/op

This PR bypasses bcrypt validation when that password has already been
validated.

cache enabled ttl. default 60s
  • Loading branch information
asoorm authored and buger committed Jul 20, 2018
1 parent 4ae183c commit 10e7ce8
Show file tree
Hide file tree
Showing 5 changed files with 144 additions and 34 deletions.
4 changes: 3 additions & 1 deletion api_loader.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,12 @@ import (
"net/url"
"sort"
"strings"
"time"

"github.com/Sirupsen/logrus"
"github.com/gorilla/mux"
"github.com/justinas/alice"
"github.com/pmylund/go-cache"

"github.com/TykTechnologies/tyk/apidef"
"github.com/TykTechnologies/tyk/config"
Expand Down Expand Up @@ -356,7 +358,7 @@ func processSpec(spec *APISpec, apisByListen map[string]int,
mainLog.WithField("api_name", spec.Name).Info("Checking security policy: OAuth")
}

if mwAppendEnabled(&authArray, &BasicAuthKeyIsValid{baseMid}) {
if mwAppendEnabled(&authArray, &BasicAuthKeyIsValid{baseMid, cache.New(60*time.Second, 60*time.Minute)}) {
mainLog.WithField("api_name", spec.Name).Info("Checking security policy: Basic")
}

Expand Down
8 changes: 6 additions & 2 deletions apidef/api_definitions.go
Original file line number Diff line number Diff line change
Expand Up @@ -329,8 +329,12 @@ type APIDefinition struct {
AllowedAuthorizeTypes []osin.AuthorizeRequestType `bson:"allowed_authorize_types" json:"allowed_authorize_types"`
AuthorizeLoginRedirect string `bson:"auth_login_redirect" json:"auth_login_redirect"`
} `bson:"oauth_meta" json:"oauth_meta"`
Auth Auth `bson:"auth" json:"auth"`
UseBasicAuth bool `bson:"use_basic_auth" json:"use_basic_auth"`
Auth Auth `bson:"auth" json:"auth"`
UseBasicAuth bool `bson:"use_basic_auth" json:"use_basic_auth"`
BasicAuth struct {
DisableCaching bool `bson:"disable_caching" json:"disable_caching"`
CacheTTL int `bson:"cache_ttl" json:"cache_ttl"`
} `bson:"basic_auth" json:"basic_auth"`
UseMutualTLSAuth bool `bson:"use_mutual_tls_auth" json:"use_mutual_tls_auth"`
ClientCertificates []string `bson:"client_certificates" json:"client_certificates"`
UpstreamCertificates map[string]string `bson:"upstream_certificates" json:"upstream_certificates"`
Expand Down
3 changes: 2 additions & 1 deletion multiauth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (

"github.com/justinas/alice"
"github.com/lonelycode/go-uuid/uuid"
"github.com/pmylund/go-cache"

"github.com/TykTechnologies/tyk/user"
)
Expand Down Expand Up @@ -77,7 +78,7 @@ func getMultiAuthStandardAndBasicAuthChain(spec *APISpec) http.Handler {
chain := alice.New(mwList(
&IPWhiteListMiddleware{baseMid},
&IPBlackListMiddleware{BaseMiddleware: baseMid},
&BasicAuthKeyIsValid{baseMid},
&BasicAuthKeyIsValid{baseMid, cache.New(60*time.Second, 60*time.Minute)},
&AuthKey{baseMid},
&VersionCheck{BaseMiddleware: baseMid},
&KeyExpired{baseMid},
Expand Down
104 changes: 77 additions & 27 deletions mw_basic_auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,22 +5,30 @@ import (
"errors"
"net/http"
"strings"
"time"

"github.com/Sirupsen/logrus"
"github.com/pmylund/go-cache"
"github.com/spaolacci/murmur3"
"golang.org/x/crypto/bcrypt"

"github.com/TykTechnologies/tyk/apidef"
"github.com/TykTechnologies/tyk/user"
)

const defaultBasicAuthTTL = time.Duration(60) * time.Second

// BasicAuthKeyIsValid uses a username instead of
type BasicAuthKeyIsValid struct {
BaseMiddleware
cache *cache.Cache
}

func (k *BasicAuthKeyIsValid) Name() string {
return "BasicAuthKeyIsValid"
}

// EnabledForSpec checks if UseBasicAuth is set in the API definition.
func (k *BasicAuthKeyIsValid) EnabledForSpec() bool {
return k.Spec.UseBasicAuth
}
Expand Down Expand Up @@ -75,39 +83,22 @@ func (k *BasicAuthKeyIsValid) ProcessRequest(w http.ResponseWriter, r *http.Requ
if !keyExists {
logEntry.Info("Attempted access with non-existent user.")

// Fire Authfailed Event
AuthFailed(k, r, token)

// Report in health check
reportHealthValue(k.Spec, KeyFailure, "-1")

return k.requestForBasicAuth(w, "User not authorised")
return k.handleAuthFail(w, r, token)
}

// Ensure that the username and password match up
var passMatch bool
switch session.BasicAuthData.Hash {
case user.HashBCrypt:
err := bcrypt.CompareHashAndPassword([]byte(session.BasicAuthData.Password), []byte(authValues[1]))
if err == nil {
passMatch = true

if err := k.compareHashAndPassword(session.BasicAuthData.Password, authValues[1], logEntry); err != nil {
logEntry.Warn("Attempted access with existing user, failed password check.")
return k.handleAuthFail(w, r, token)
}
case user.HashPlainText:
if session.BasicAuthData.Password == authValues[1] {
passMatch = true
}
}

if !passMatch {
logEntry.Info("Attempted access with existing user but failed password check.")

// Fire Authfailed Event
AuthFailed(k, r, token)

// Report in health check
reportHealthValue(k.Spec, KeyFailure, "-1")
if session.BasicAuthData.Password != authValues[1] {

return k.requestForBasicAuth(w, "User not authorised")
logEntry.Warn("Attempted access with existing user, failed password check.")
return k.handleAuthFail(w, r, token)
}
}

// Set session state on context, we will need it later
Expand All @@ -116,6 +107,65 @@ func (k *BasicAuthKeyIsValid) ProcessRequest(w http.ResponseWriter, r *http.Requ
ctxSetSession(r, &session, keyName, false)
}

// Request is valid, carry on
return nil, http.StatusOK
}

func (k *BasicAuthKeyIsValid) handleAuthFail(w http.ResponseWriter, r *http.Request, token string) (error, int) {

// Fire Authfailed Event
AuthFailed(k, r, token)

// Report in health check
reportHealthValue(k.Spec, KeyFailure, "-1")

return k.requestForBasicAuth(w, "User not authorised")
}

func (k *BasicAuthKeyIsValid) doBcryptWithCache(cacheDuration time.Duration, hashedPassword []byte, password []byte) error {
if err := bcrypt.CompareHashAndPassword(hashedPassword, password); err != nil {

return err
}

hasher := murmur3.New32()
hasher.Write(password)
k.cache.Set(string(hashedPassword), string(hasher.Sum(nil)), cacheDuration)

return nil
}

func (k *BasicAuthKeyIsValid) compareHashAndPassword(hash string, password string, logEntry *logrus.Entry) error {

cacheEnabled := !k.Spec.BasicAuth.DisableCaching
passwordBytes := []byte(password)
hashBytes := []byte(hash)

if !cacheEnabled {

logEntry.Debug("cache disabled")
return bcrypt.CompareHashAndPassword(hashBytes, passwordBytes)
}

cacheTTL := defaultBasicAuthTTL // set a default TTL, then override based on BasicAuth.CacheTTL
if k.Spec.BasicAuth.CacheTTL > 0 {
cacheTTL = time.Duration(k.Spec.BasicAuth.CacheTTL) * time.Second
}

cachedPass, inCache := k.cache.Get(hash)
if !inCache {

logEntry.Debug("cache enabled: miss: bcrypt")
return k.doBcryptWithCache(cacheTTL, hashBytes, passwordBytes)
}

hasher := murmur3.New32()
hasher.Write(passwordBytes)
if cachedPass.(string) != string(hasher.Sum(nil)) {

logEntry.Warn("cache enabled: hit: failed auth: bcrypt")
return bcrypt.CompareHashAndPassword(hashBytes, passwordBytes)
}

logEntry.Debug("cache enabled: hit: success")
return nil
}
59 changes: 56 additions & 3 deletions mw_basic_auth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,15 @@ func genAuthHeader(username, password string) string {
return fmt.Sprintf("Basic %s", encodedPass)
}

func testPrepareBasicAuth() *user.SessionState {
func testPrepareBasicAuth(cacheDisabled bool) *user.SessionState {
session := createStandardSession()
session.BasicAuthData.Password = "password"
session.AccessRights = map[string]user.AccessDefinition{"test": {APIID: "test", Versions: []string{"v1"}}}
session.OrgID = "default"

buildAndLoadAPI(func(spec *APISpec) {
spec.UseBasicAuth = true
spec.BasicAuth.DisableCaching = cacheDisabled
spec.UseKeylessAccess = false
spec.Proxy.ListenPath = "/"
spec.OrgID = "default"
Expand All @@ -36,7 +37,7 @@ func TestBasicAuth(t *testing.T) {
ts := newTykTestServer()
defer ts.Close()

session := testPrepareBasicAuth()
session := testPrepareBasicAuth(false)

validPassword := map[string]string{"Authorization": genAuthHeader("user", "password")}
wrongPassword := map[string]string{"Authorization": genAuthHeader("user", "wrong")}
Expand All @@ -60,7 +61,7 @@ func BenchmarkBasicAuth(b *testing.B) {
ts := newTykTestServer()
defer ts.Close()

session := testPrepareBasicAuth()
session := testPrepareBasicAuth(false)

validPassword := map[string]string{"Authorization": genAuthHeader("user", "password")}
wrongPassword := map[string]string{"Authorization": genAuthHeader("user", "wrong")}
Expand All @@ -86,3 +87,55 @@ func BenchmarkBasicAuth(b *testing.B) {
}...)
}
}

func BenchmarkBasicAuth_CacheEnabled(b *testing.B) {
b.ReportAllocs()

ts := newTykTestServer()
defer ts.Close()

session := testPrepareBasicAuth(false)

validPassword := map[string]string{"Authorization": genAuthHeader("user", "password")}

// Create base auth based key
ts.Run(b, test.TestCase{
Method: "POST",
Path: "/tyk/keys/defaultuser",
Data: session,
AdminAuth: true,
Code: 200,
})

for i := 0; i < b.N; i++ {
ts.Run(b, []test.TestCase{
{Method: "GET", Path: "/", Headers: validPassword, Code: 200},
}...)
}
}

func BenchmarkBasicAuth_CacheDisabled(b *testing.B) {
b.ReportAllocs()

ts := newTykTestServer()
defer ts.Close()

session := testPrepareBasicAuth(true)

validPassword := map[string]string{"Authorization": genAuthHeader("user", "password")}

// Create base auth based key
ts.Run(b, test.TestCase{
Method: "POST",
Path: "/tyk/keys/defaultuser",
Data: session,
AdminAuth: true,
Code: 200,
})

for i := 0; i < b.N; i++ {
ts.Run(b, []test.TestCase{
{Method: "GET", Path: "/", Headers: validPassword, Code: 200},
}...)
}
}

0 comments on commit 10e7ce8

Please sign in to comment.