remove unnecessary LRU for internode auth token (#20119)

removes contentious usage of mutexes in LRU, which were never really reused in any manner; we do not need it. To trust hosts, the correct way is TLS certs; this PR completely removes this dependency, which has never been useful. ``` 0 0% 100% 25.83s 26.76% github.com/hashicorp/golang-lru/v2/expirable.(*LRU[...]) 0 0% 100% 28.03s 29.04% github.com/hashicorp/golang-lru/v2/expirable.(*LRU[...]) ``` Bonus: use `x-minio-time` as a nanosecond to avoid unnecessary parsing logic of time strings instead of using a more straightforward mechanism.
minio · Jul 22, 2024 · 8e618d4 · 8e618d4
1 parent 3ef59d2
commit 8e618d4
Show file tree

Hide file tree

Showing 17 changed files with 58 additions and 475 deletions.
diff --git a/CREDITS b/CREDITS
diff --git a/cmd/common-main.go b/cmd/common-main.go
@@ -871,6 871,12 @@ func loadRootCredentials() {
  } else {
  globalActiveCred = auth.DefaultCredentials
  }
 
  var err error
  globalNodeAuthToken, err = authenticateNode(globalActiveCred.AccessKey, globalActiveCred.SecretKey)
  if err != nil {
  logger.Fatal(err, "Unable to generate internode credentials")
  }
 }
 
 // Initialize KMS global variable after valiadating and loading the configuration.

diff --git a/cmd/globals.go b/cmd/globals.go
@@ -310,6 310,7 @@ var (
  globalBootTime = UTCNow()
 
  globalActiveCred auth.Credentials
  globalNodeAuthToken string
  globalSiteReplicatorCred siteReplicatorCred
 
  // Captures if root credentials are set via ENV.

diff --git a/cmd/jwt.go b/cmd/jwt.go
@@ -24,10 24,8 @@ import (
 
  jwtgo "github.com/golang-jwt/jwt/v4"
  jwtreq "github.com/golang-jwt/jwt/v4/request"
- "github.com/hashicorp/golang-lru/v2/expirable"
  "github.com/minio/minio/internal/auth"
  xjwt "github.com/minio/minio/internal/jwt"
- "github.com/minio/minio/internal/logger"
  "github.com/minio/pkg/v3/policy"
 )
 
@@ -37,8 35,8 @@ const (
  // Default JWT token for web handlers is one day.
  defaultJWTExpiry = 24 * time.Hour
 
- // Inter-node JWT token expiry is 15 minutes.
- defaultInterNodeJWTExpiry = 15 * time.Minute
  // Inter-node JWT token expiry is 100 years approx.
  defaultInterNodeJWTExpiry = 100 * 365 * 24 * time.Hour
 )
 
 var (
@@ -50,17 48,10 @@ var (
  errMalformedAuth = errors.New("Malformed authentication input")
 )
 
-type cacheKey struct {
- accessKey, secretKey, audience string
-}
-
-var cacheLRU = expirable.NewLRU[cacheKey, string](1000, nil, 15*time.Second)
-
-func authenticateNode(accessKey, secretKey, audience string) (string, error) {
 func authenticateNode(accessKey, secretKey string) (string, error) {
  claims := xjwt.NewStandardClaims()
  claims.SetExpiry(UTCNow().Add(defaultInterNodeJWTExpiry))
  claims.SetAccessKey(accessKey)
- claims.SetAudience(audience)
 
  jwt := jwtgo.NewWithClaims(jwtgo.SigningMethodHS512, claims)
  return jwt.SignedString([]byte(secretKey))
@@ -141,27 132,9 @@ func metricsRequestAuthenticate(req *http.Request) (*xjwt.MapClaims, []string, b
  return claims, groups, owner, nil
 }
 
-// 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 := func(accessKey, secretKey, audience string) (s string, err error) {
- k := cacheKey{accessKey: accessKey, secretKey: secretKey, audience: audience}
-
- var ok bool
- s, ok = cacheLRU.Get(k)
- if !ok {
- s, err = authenticateNode(accessKey, secretKey, audience)
- if err != nil {
- return "", err
- }
- cacheLRU.Add(k, s)
- }
- return s, nil
- }
- return func(audience string) string {
- cred := globalActiveCred
- token, err := fn(cred.AccessKey, cred.SecretKey, audience)
- logger.CriticalIf(GlobalContext, err)
- return token
 // newCachedAuthToken returns the cached token.
 func newCachedAuthToken() func() string {
  return func() string {
  return globalNodeAuthToken
  }
 }
diff --git a/cmd/jwt_test.go b/cmd/jwt_test.go
@@ -107,7 107,7 @@ func BenchmarkParseJWTStandardClaims(b *testing.B) {
  }
 
  creds := globalActiveCred
- token, err := authenticateNode(creds.AccessKey, creds.SecretKey, "")
  token, err := authenticateNode(creds.AccessKey, creds.SecretKey)
  if err != nil {
  b.Fatal(err)
  }
@@ -138,7 138,7 @@ func BenchmarkParseJWTMapClaims(b *testing.B) {
  }
 
  creds := globalActiveCred
- token, err := authenticateNode(creds.AccessKey, creds.SecretKey, "")
  token, err := authenticateNode(creds.AccessKey, creds.SecretKey)
  if err != nil {
  b.Fatal(err)
  }
@@ -176,15 176,15 @@ func BenchmarkAuthenticateNode(b *testing.B) {
  b.ResetTimer()
  b.ReportAllocs()
  for i := 0; i < b.N; i  {
- fn(creds.AccessKey, creds.SecretKey, "aud")
  fn(creds.AccessKey, creds.SecretKey)
  }
  })
  b.Run("cached", func(b *testing.B) {
  fn := newCachedAuthToken()
  b.ResetTimer()
  b.ReportAllocs()
  for i := 0; i < b.N; i  {
- fn("aud")
  fn()
  }
  })
 }
diff --git a/cmd/lock-rest-server-common_test.go b/cmd/lock-rest-server-common_test.go
@@ -44,7 44,7 @@ func createLockTestServer(ctx context.Context, t *testing.T) (string, *lockRESTS
  },
  }
  creds := globalActiveCred
- token, err := authenticateNode(creds.AccessKey, creds.SecretKey, "")
  token, err := authenticateNode(creds.AccessKey, creds.SecretKey)
  if err != nil {
  t.Fatal(err)
  }

diff --git a/cmd/object-lambda-handlers.go b/cmd/object-lambda-handlers.go
@@ -19,6 19,7 @@ package cmd
 
 import (
  "crypto/subtle"
  "encoding/hex"
  "io"
  "net/http"
  "net/url"
@@ -33,6 34,7 @@ import (
 
  "github.com/minio/minio/internal/auth"
  levent "github.com/minio/minio/internal/config/lambda/event"
  "github.com/minio/minio/internal/hash/sha256"
  xhttp "github.com/minio/minio/internal/http"
  "github.com/minio/minio/internal/logger"
 )
@@ -77,16 79,13 @@ func getLambdaEventData(bucket, object string, cred auth.Credentials, r *http.Re
  return levent.Event{}, err
  }
 
- token, err := authenticateNode(cred.AccessKey, cred.SecretKey, u.RawQuery)
- if err != nil {
- return levent.Event{}, err
- }
  ckSum := sha256.Sum256([]byte(cred.AccessKey   u.RawQuery))
 
  eventData := levent.Event{
  GetObjectContext: &levent.GetObjectContext{
  InputS3URL: u.String(),
  OutputRoute: shortuuid.New(),
- OutputToken: token,
  OutputToken: hex.EncodeToString(ckSum[:]),
  },
  UserRequest: levent.UserRequest{
  URL: r.URL.String(),

diff --git a/cmd/storage-rest-server.go b/cmd/storage-rest-server.go
@@ -110,7 110,7 @@ func (s *storageRESTServer) writeErrorResponse(w http.ResponseWriter, err error)
 const DefaultSkewTime = 15 * time.Minute
 
 // validateStorageRequestToken will validate the token against the provided audience.
-func validateStorageRequestToken(token, audience string) error {
 func validateStorageRequestToken(token string) error {
  claims := xjwt.NewStandardClaims()
  if err := xjwt.ParseWithStandardClaims(token, claims, []byte(globalActiveCred.SecretKey)); err != nil {
  return errAuthentication
@@ -121,9 121,6 @@ func validateStorageRequestToken(token, audience string) error {
  return errAuthentication
  }
 
- if claims.Audience != audience {
- return errAuthentication
- }
  return nil
 }
 
@@ -136,20 133,24 @@ func storageServerRequestValidate(r *http.Request) error {
  }
  return errMalformedAuth
  }
- if err = validateStorageRequestToken(token, r.URL.RawQuery); err != nil {
 
  if err = validateStorageRequestToken(token); err != nil {
  return err
  }
 
- requestTimeStr := r.Header.Get("X-Minio-Time")
- requestTime, err := time.Parse(time.RFC3339, requestTimeStr)
  nanoTime, err := strconv.ParseInt(r.Header.Get("X-Minio-Time"), 10, 64)
  if err != nil {
  return errMalformedAuth
  }
- utcNow := UTCNow()
- delta := requestTime.Sub(utcNow)
 
  localTime := UTCNow()
  remoteTime := time.Unix(0, nanoTime)
 
  delta := remoteTime.Sub(localTime)
  if delta < 0 {
  delta *= -1
  }
 
  if delta > DefaultSkewTime {
  return errSkewedAuthTime
  }

diff --git a/cmd/storage-rest_test.go b/cmd/storage-rest_test.go
@@ -315,6 315,7 @@ func newStorageRESTHTTPServerClient(t testing.TB) *storageRESTClient {
  url.Path = t.TempDir()
 
  globalMinioHost, globalMinioPort = mustSplitHostPort(url.Host)
  globalNodeAuthToken, _ = authenticateNode(globalActiveCred.AccessKey, globalActiveCred.SecretKey)
 
  endpoint, err := NewEndpoint(url.String())
  if err != nil {

diff --git a/cmd/test-utils_test.go b/cmd/test-utils_test.go
@@ -83,6 83,8 @@ func TestMain(m *testing.M) {
  SecretKey: auth.DefaultSecretKey,
  }
 
  globalNodeAuthToken, _ = authenticateNode(auth.DefaultAccessKey, auth.DefaultSecretKey)
 
  // disable ENVs which interfere with tests.
  for _, env := range []string{
  crypto.EnvKMSAutoEncryption,

diff --git a/go.mod b/go.mod
@@ -32,7 32,6 @@ require (
  github.com/golang-jwt/jwt/v4 v4.5.0
  github.com/gomodule/redigo v1.9.2
  github.com/google/uuid v1.6.0
- github.com/hashicorp/golang-lru/v2 v2.0.7
  github.com/inconshreveable/mousetrap v1.1.0
  github.com/json-iterator/go v1.1.12
  github.com/klauspost/compress v1.17.9

diff --git a/go.sum b/go.sum
@@ -322,8 322,6 @@ github.com/hashicorp/go-uuid v1.0.3/go.mod h1:6SBZvOh/SIDV7/2o3Jml5SYk/TvGqwFJ/b
 github.com/hashicorp/golang-lru v0.5.0/go.mod h1:/m3WP610KZHVQ1SGc6re/UDhFvYD7pJ4Ao sR/qLZy8=
 github.com/hashicorp/golang-lru v1.0.2 h1:dV3g9Z/unq5DpblPpw Oqcv4dU/1omnb4Ok8iPY6p1c=
 github.com/hashicorp/golang-lru v1.0.2/go.mod h1:iADmTwqILo4mZ8BN3D2Q6 9jd8WM5uGBxy E8yxSoD4=
-github.com/hashicorp/golang-lru/v2 v2.0.7 h1:a bsQ5rvGLjzHuww6tVxozPZFVghXaHOwFs4luLUK2k=
-github.com/hashicorp/golang-lru/v2 v2.0.7/go.mod h1:QeFd9opnmA6QUJc5vARoKUSoFhyfM2/ZepoAG6RGpeM=
 github.com/hashicorp/hcl v1.0.0/go.mod h1:E5yfLk 7swimpb2L/Alb/PJmXilQ/rhwaUYs4T20WEQ=
 github.com/hashicorp/raft v1.3.9 h1:9yuo1aR0bFTr1cw7pj3S2Bk6MhJCsnr2NAxvIBrP2x4=
 github.com/hashicorp/raft v1.3.9/go.mod h1:4Ak7FSPnuvmb0GV6vgIAJ4vYT4bek9bb6Q 7HVbyzqM=

diff --git a/internal/grid/debug.go b/internal/grid/debug.go
@@ -169,13 169,13 @@ func dummyRequestValidate(r *http.Request) error {
  return nil
 }
 
-func dummyTokenValidate(token, audience string) error {
- if token == audience {
 func dummyTokenValidate(token string) error {
  if token == "debug" {
  return nil
  }
- return fmt.Errorf("invalid token. want %s, got %s", audience, token)
  return fmt.Errorf("invalid token. want empty, got %s", token)
 }
 
-func dummyNewToken(audience string) string {
- return audience
 func dummyNewToken() string {
  return "debug"
 }
diff --git a/internal/grid/grid.go b/internal/grid/grid.go
@@ -26,6 26,7 @@ import (
  "io"
  "net"
  "net/http"
  "strconv"
  "strings"
  "sync"
  "time"
@@ -208,8 209,8 @@ func ConnectWS(dial ContextDialer, auth AuthFn, tls *tls.Config) func(ctx contex
  dialer.NetDial = dial
  }
  header := make(http.Header, 2)
- header.Set("Authorization", "Bearer " auth(""))
- header.Set("X-Minio-Time", time.Now().UTC().Format(time.RFC3339))
  header.Set("Authorization", "Bearer " auth())
  header.Set("X-Minio-Time", strconv.FormatInt(time.Now().UnixNano(), 10))
 
  if len(header) > 0 {
  dialer.Header = ws.HandshakeHeaderHTTP(header)
@@ -225,4 226,4 @@ func ConnectWS(dial ContextDialer, auth AuthFn, tls *tls.Config) func(ctx contex
 }
 
 // ValidateTokenFn must validate the token and return an error if it is invalid.
-type ValidateTokenFn func(token, audience string) error
 type ValidateTokenFn func(token string) error
diff --git a/internal/grid/manager.go b/internal/grid/manager.go
@@ -245,7 245,7 @@ func (m *Manager) IncomingConn(ctx context.Context, conn net.Conn) {
  writeErr(fmt.Errorf("time difference too large between servers: %v", time.Since(cReq.Time).Abs()))
  return
  }
- if err := m.authToken(cReq.Token, cReq.audience()); err != nil {
  if err := m.authToken(cReq.Token); err != nil {
  writeErr(fmt.Errorf("auth token: %w", err))
  return
  }
@@ -257,10 257,10 @@ func (m *Manager) IncomingConn(ctx context.Context, conn net.Conn) {
 }
 
 // AuthFn should provide an authentication string for the given aud.
-type AuthFn func(aud string) string
 type AuthFn func() string
 
 // ValidateAuthFn should check authentication for the given aud.
-type ValidateAuthFn func(auth, aud string) string
 type ValidateAuthFn func(auth string) string
 
 // Connection will return the connection for the specified host.
 // If the host does not exist nil will be returned.

diff --git a/internal/grid/msg.go b/internal/grid/msg.go
@@ -262,14 262,9 @@ type connectReq struct {
  Token string
 }
 
-// audience returns the audience for the connect call.
-func (c *connectReq) audience() string {
- return fmt.Sprintf("%s-%d", c.Host, c.Time.Unix())
-}
-
 // addToken will add the token to the connect request.
 func (c *connectReq) addToken(fn AuthFn) {
- c.Token = fn(c.audience())
  c.Token = fn()
 }
 
 func (connectReq) Op() Op {

diff --git a/internal/rest/client.go b/internal/rest/client.go
@@ -28,6 28,7 @@ import (
  "net/http/httputil"
  "net/url"
  "path"
  "strconv"
  "strings"
  "sync"
  "sync/atomic"
@@ -95,9 96,9 @@ type Client struct {
  // TraceOutput will print debug information on non-200 calls if set.
  TraceOutput io.Writer // Debug trace output
 
- httpClient  *http.Client
- url  *url.URL
- newAuthToken func(audience string) string
  httpClient *http.Client
  url *url.URL
  auth func() string
 
  sync.RWMutex // mutex for lastErr
  lastErr error
@@ -188,10 189,10 @@ func (c *Client) newRequest(ctx context.Context, u url.URL, body io.Reader) (*ht
  }
  }
 
- if c.newAuthToken != nil {
- req.Header.Set("Authorization", "Bearer " c.newAuthToken(u.RawQuery))
  if c.auth != nil {
  req.Header.Set("Authorization", "Bearer " c.auth())
  }
- req.Header.Set("X-Minio-Time", time.Now().UTC().Format(time.RFC3339))
  req.Header.Set("X-Minio-Time", strconv.FormatInt(time.Now().UnixNano(), 10))
 
  if tc, ok := ctx.Value(mcontext.ContextTraceKey).(*mcontext.TraceCtxt); ok {
  req.Header.Set(xhttp.AmzRequestID, tc.AmzReqID)
@@ -387,7 388,7 @@ func (c *Client) Close() {
 }
 
 // NewClient - returns new REST client.
-func NewClient(uu *url.URL, tr http.RoundTripper, newAuthToken func(aud string) string) *Client {
 func NewClient(uu *url.URL, tr http.RoundTripper, auth func() string) *Client {
  connected := int32(online)
  urlStr := uu.String()
  u, err := url.Parse(urlStr)
@@ -404,7 405,7 @@ func NewClient(uu *url.URL, tr http.RoundTripper, newAuthToken func(aud string)
  clnt := &Client{
  httpClient: &http.Client{Transport: tr},
  url: u,
- newAuthToken: newAuthToken,
  auth:  auth,
  connected: connected,
  lastConn: time.Now().UnixNano(),
  MaxErrResponseSize: 4096,