From 142c6b11b397e68ba2264324eb5301a54779bced Mon Sep 17 00:00:00 2001 From: Klaus Post Date: Tue, 23 Nov 2021 09:51:53 -0800 Subject: [PATCH] Reduce JWT overhead for internode tokens (#13738) Since JWT tokens remain valid for up to 15 minutes, we don't have to regenerate tokens for every call. Cache tokens for matching access+secret+audience for up to 15 seconds. ``` BenchmarkAuthenticateNode/uncached-32 270567 4179 ns/op 2961 B/op 33 allocs/op BenchmarkAuthenticateNode/cached-32 7684824 157.5 ns/op 48 B/op 1 allocs/op ``` Reduces internode call allocations a great deal. --- cmd/bootstrap-peer-server.go | 2 +- cmd/jwt.go | 45 ++++++++++++++++++++++++++++++++---- cmd/jwt_test.go | 22 ++++++++++++++---- cmd/lock-rest-client.go | 4 ++-- cmd/peer-rest-client.go | 4 ++-- cmd/storage-rest-client.go | 4 ++-- go.mod | 1 + 7 files changed, 65 insertions(+), 17 deletions(-) diff --git a/cmd/bootstrap-peer-server.go b/cmd/bootstrap-peer-server.go index 5944ec41b..32ca5bb3a 100644 --- a/cmd/bootstrap-peer-server.go +++ b/cmd/bootstrap-peer-server.go @@ -247,7 +247,7 @@ func newBootstrapRESTClient(endpoint Endpoint) *bootstrapRESTClient { Path: bootstrapRESTPath, } - restClient := rest.NewClient(serverURL, globalInternodeTransport, newAuthToken) + restClient := rest.NewClient(serverURL, globalInternodeTransport, newCachedAuthToken()) restClient.HealthCheckFn = nil return &bootstrapRESTClient{endpoint: endpoint, restClient: restClient} diff --git a/cmd/jwt.go b/cmd/jwt.go index 172dbb777..288a3cb37 100644 --- a/cmd/jwt.go +++ b/cmd/jwt.go @@ -25,6 +25,7 @@ import ( jwtgo "github.com/golang-jwt/jwt/v4" jwtreq "github.com/golang-jwt/jwt/v4/request" + lru "github.com/hashicorp/golang-lru" "github.com/minio/minio/internal/auth" xjwt "github.com/minio/minio/internal/jwt" "github.com/minio/minio/internal/logger" @@ -81,6 +82,35 @@ func authenticateJWTUsersWithCredentials(credentials auth.Credentials, expiresAt return jwt.SignedString([]byte(serverCred.SecretKey)) } +// cachedAuthenticateNode will cache authenticateNode results for given values up to ttl. +func cachedAuthenticateNode(ttl time.Duration) func(accessKey, secretKey, audience string) (string, error) { + type key struct { + accessKey, secretKey, audience string + } + type value struct { + created time.Time + res string + err error + } + cache, err := lru.NewARC(100) + if err != nil { + logger.LogIf(GlobalContext, err) + return authenticateNode + } + return func(accessKey, secretKey, audience string) (string, error) { + k := key{accessKey: accessKey, secretKey: secretKey, audience: audience} + v, ok := cache.Get(k) + if ok { + if val, ok := v.(*value); ok && time.Since(val.created) < ttl { + return val.res, val.err + } + } + s, err := authenticateNode(accessKey, secretKey, audience) + cache.Add(k, &value{created: time.Now(), res: s, err: err}) + return s, err + } +} + func authenticateNode(accessKey, secretKey, audience string) (string, error) { claims := xjwt.NewStandardClaims() claims.SetExpiry(UTCNow().Add(defaultInterNodeJWTExpiry)) @@ -152,9 +182,14 @@ func webRequestAuthenticate(req *http.Request) (*xjwt.MapClaims, bool, error) { return claims, owner, nil } -func newAuthToken(audience string) string { - cred := globalActiveCred - token, err := authenticateNode(cred.AccessKey, cred.SecretKey, audience) - logger.CriticalIf(GlobalContext, err) - return token +// newCachedAuthToken returns a token that is cached up to 15 seconds. +// If globalActiveCred is updated it is reflected at once. +func newCachedAuthToken() func(audience string) string { + fn := cachedAuthenticateNode(15 * time.Second) + return func(audience string) string { + cred := globalActiveCred + token, err := fn(cred.AccessKey, cred.SecretKey, audience) + logger.CriticalIf(GlobalContext, err) + return token + } } diff --git a/cmd/jwt_test.go b/cmd/jwt_test.go index d85b8ccd0..b7f10476e 100644 --- a/cmd/jwt_test.go +++ b/cmd/jwt_test.go @@ -21,6 +21,7 @@ import ( "net/http" "os" "testing" + "time" jwtgo "github.com/golang-jwt/jwt/v4" "github.com/minio/minio/internal/auth" @@ -224,11 +225,22 @@ func BenchmarkAuthenticateNode(b *testing.B) { } creds := globalActiveCred - b.ResetTimer() - b.ReportAllocs() - for i := 0; i < b.N; i++ { - authenticateNode(creds.AccessKey, creds.SecretKey, "") - } + b.Run("uncached", func(b *testing.B) { + fn := authenticateNode + b.ResetTimer() + b.ReportAllocs() + for i := 0; i < b.N; i++ { + fn(creds.AccessKey, creds.SecretKey, "aud") + } + }) + b.Run("cached", func(b *testing.B) { + fn := cachedAuthenticateNode(time.Second) + b.ResetTimer() + b.ReportAllocs() + for i := 0; i < b.N; i++ { + fn(creds.AccessKey, creds.SecretKey, "aud") + } + }) } func BenchmarkAuthenticateWeb(b *testing.B) { diff --git a/cmd/lock-rest-client.go b/cmd/lock-rest-client.go index 44e709db7..0bdd10d23 100644 --- a/cmd/lock-rest-client.go +++ b/cmd/lock-rest-client.go @@ -151,10 +151,10 @@ func newlockRESTClient(endpoint Endpoint) *lockRESTClient { Path: pathJoin(lockRESTPrefix, lockRESTVersion), } - restClient := rest.NewClient(serverURL, globalInternodeTransport, newAuthToken) + restClient := rest.NewClient(serverURL, globalInternodeTransport, newCachedAuthToken()) restClient.ExpectTimeouts = true // Use a separate client to avoid recursive calls. - healthClient := rest.NewClient(serverURL, globalInternodeTransport, newAuthToken) + healthClient := rest.NewClient(serverURL, globalInternodeTransport, newCachedAuthToken()) healthClient.ExpectTimeouts = true healthClient.NoMetrics = true restClient.HealthCheckFn = func() bool { diff --git a/cmd/peer-rest-client.go b/cmd/peer-rest-client.go index 085ee3421..b6d0ec2a0 100644 --- a/cmd/peer-rest-client.go +++ b/cmd/peer-rest-client.go @@ -958,9 +958,9 @@ func newPeerRESTClient(peer *xnet.Host) *peerRESTClient { Path: peerRESTPath, } - restClient := rest.NewClient(serverURL, globalInternodeTransport, newAuthToken) + restClient := rest.NewClient(serverURL, globalInternodeTransport, newCachedAuthToken()) // Use a separate client to avoid recursive calls. - healthClient := rest.NewClient(serverURL, globalInternodeTransport, newAuthToken) + healthClient := rest.NewClient(serverURL, globalInternodeTransport, newCachedAuthToken()) healthClient.ExpectTimeouts = true healthClient.NoMetrics = true diff --git a/cmd/storage-rest-client.go b/cmd/storage-rest-client.go index 1a3bd1419..5b52b251e 100644 --- a/cmd/storage-rest-client.go +++ b/cmd/storage-rest-client.go @@ -725,11 +725,11 @@ func newStorageRESTClient(endpoint Endpoint, healthcheck bool) *storageRESTClien Path: path.Join(storageRESTPrefix, endpoint.Path, storageRESTVersion), } - restClient := rest.NewClient(serverURL, globalInternodeTransport, newAuthToken) + restClient := rest.NewClient(serverURL, globalInternodeTransport, newCachedAuthToken()) if healthcheck { // Use a separate client to avoid recursive calls. - healthClient := rest.NewClient(serverURL, globalInternodeTransport, newAuthToken) + healthClient := rest.NewClient(serverURL, globalInternodeTransport, newCachedAuthToken()) healthClient.ExpectTimeouts = true healthClient.NoMetrics = true restClient.HealthCheckFn = func() bool { diff --git a/go.mod b/go.mod index 5e2335180..8f9c1a03c 100644 --- a/go.mod +++ b/go.mod @@ -33,6 +33,7 @@ require ( github.com/gomodule/redigo v2.0.0+incompatible github.com/google/uuid v1.3.0 github.com/gorilla/mux v1.8.0 + github.com/hashicorp/golang-lru v0.5.4 github.com/inconshreveable/mousetrap v1.0.0 github.com/jcmturner/gokrb5/v8 v8.4.2 github.com/json-iterator/go v1.1.12