feat(script): Make Script helper work in strict FIPS mode by avoiding client-side SHA-1 (#3700)

* script: avoid client-side SHA1 in FIPS mode * modifying the script.go to add new constructor to explicitly control SHA-1 Behaviour(strict FIPS Builds - FIPS140) * fixing the lint errors * Add unit and integration tests for NewScriptServerSHA * fixing the mod format issue * Update go.mod Co-authored-by: Nedyalko Dyakov <1547186+ndyakov@users.noreply.github.com> * Update go.sum Co-authored-by: Nedyalko Dyakov <1547186+ndyakov@users.noreply.github.com> * Update go.sum Co-authored-by: Nedyalko Dyakov <1547186+ndyakov@users.noreply.github.com> * Update script_server_sha_integration_test.go Co-authored-by: Nedyalko Dyakov <1547186+ndyakov@users.noreply.github.com> * Update go.mod Moved the dependency under require block --------- Co-authored-by: Nedyalko Dyakov <1547186+ndyakov@users.noreply.github.com>
2026-10-22 19:49:00 +03:00 · 2026-03-28 05:45:51 -05:00
parent f37dbd0bc1
commit b506c612a4
4 changed files with 400 additions and 8 deletions
--- a/go.mod
+++ b/go.mod
@@ -8,9 +8,10 @@ require (
 	github.com/cespare/xxhash/v2 v2.3.0
 	github.com/dgryski/go-rendezvous v0.0.0-20200823014737-9f7001d12a5f
 	github.com/zeebo/xxh3 v1.1.0
+	go.uber.org/atomic v1.11.0
 )

-require go.uber.org/atomic v1.11.0
+

 require (
 	github.com/klauspost/cpuid/v2 v2.2.10 // indirect
--- a/script.go
+++ b/script.go
@@ -6,6 +6,7 @@ import (
 	"encoding/hex"
 	"errors"
 	"io"
+	"sync"
 )

 type Scripter interface {
@@ -24,28 +25,69 @@ var (
 )

 type Script struct {
-	src, hash string
+	src       string
+	mu        sync.RWMutex
+	hash      string
+	serverSHA bool // if true: do not compute SHA-1 in Go; load digest from Redis (SCRIPT LOAD)
 }

 func NewScript(src string) *Script {
 	h := sha1.New()
 	_, _ = io.WriteString(h, src)
+
 	return &Script{
-		src:  src,
-		hash: hex.EncodeToString(h.Sum(nil)),
+		src:       src,
+		hash:      hex.EncodeToString(h.Sum(nil)),
+		serverSHA: false,
+	}
+}
+
+// NewScriptServerSHA creates a Script that avoids computing SHA-1 in Go.
+// The digest is obtained from Redis via SCRIPT LOAD (server-side hashing),
+// then EVALSHA/EVALSHA_RO is used.
+func NewScriptServerSHA(src string) *Script {
+	return &Script{
+		src:       src,
+		serverSHA: true,
 	}
 }

 func (s *Script) Hash() string {
+	s.mu.RLock()
+	defer s.mu.RUnlock()
 	return s.hash
 }

 func (s *Script) Load(ctx context.Context, c Scripter) *StringCmd {
-	return c.ScriptLoad(ctx, s.src)
+	cmd := c.ScriptLoad(ctx, s.src)
+	if err := cmd.Err(); err == nil {
+		s.mu.Lock()
+		s.hash = cmd.Val()
+		s.mu.Unlock()
+	}
+	return cmd
 }

 func (s *Script) Exists(ctx context.Context, c Scripter) *BoolSliceCmd {
-	return c.ScriptExists(ctx, s.hash)
+	s.mu.RLock()
+	hash := s.hash
+	serverSHA := s.serverSHA
+	s.mu.RUnlock()
+	if hash == "" && serverSHA {
+		// For server-side scripts, obtain digest from Redis first.
+		// If hash is empty, it means SCRIPT LOAD was not called yet, so we check existence of empty hash which will return false.
+		// This avoids unnecessary SCRIPT LOAD just to check existence.
+		if err := s.ensureHash(ctx, c); err != nil {
+			return c.ScriptExists(ctx, "")
+		}
+		s.mu.RLock()
+		hash = s.hash
+		s.mu.RUnlock()
+	}
+	if hash == "" {
+		return c.ScriptExists(ctx, "")
+	}
+	return c.ScriptExists(ctx, hash)
 }

 func (s *Script) Eval(ctx context.Context, c Scripter, keys []string, args ...interface{}) *Cmd {
@@ -56,12 +98,94 @@ func (s *Script) EvalRO(ctx context.Context, c Scripter, keys []string, args ...
 	return c.EvalRO(ctx, s.src, keys, args...)
 }

+// ensureHash ensures that s.hash is populated by using SCRIPT LOAD.
+// It never calls SHA-1 in Go; Redis computes and returns the digest.
+func (s *Script) ensureHash(ctx context.Context, c Scripter) error {
+	// Fast path: read lock, return if hash is already set.
+	s.mu.RLock()
+	if s.hash != "" {
+		s.mu.RUnlock()
+		return nil
+	}
+	s.mu.RUnlock()
+
+	// Slow path: acquire write lock and load.
+	s.mu.Lock()
+	if s.hash != "" {
+		s.mu.Unlock()
+		return nil
+	}
+	cmd := c.ScriptLoad(ctx, s.src)
+	if err := cmd.Err(); err != nil {
+		s.mu.Unlock()
+		return err
+	}
+	s.hash = cmd.Val()
+	s.mu.Unlock()
+	return nil
+}
+
 func (s *Script) EvalSha(ctx context.Context, c Scripter, keys []string, args ...interface{}) *Cmd {
-	return c.EvalSha(ctx, s.hash, keys, args...)
+	// Default behavior: use client-side SHA-1 computed in NewScript.
+	if !s.serverSHA {
+		s.mu.RLock()
+		hash := s.hash
+		s.mu.RUnlock()
+		return c.EvalSha(ctx, hash, keys, args...)
+	}
+
+	// Server-side SHA via SCRIPT LOAD + EVALSHA.
+	if err := s.ensureHash(ctx, c); err != nil {
+		return s.Eval(ctx, c, keys, args...)
+	}
+
+	s.mu.RLock()
+	hash := s.hash
+	s.mu.RUnlock()
+
+	r := c.EvalSha(ctx, hash, keys, args...)
+	if HasErrorPrefix(r.Err(), "NOSCRIPT") {
+		// Script cache was flushed; reload and retry once.
+		if err := s.ensureHash(ctx, c); err != nil {
+			return s.Eval(ctx, c, keys, args...)
+		}
+		s.mu.RLock()
+		hash = s.hash
+		s.mu.RUnlock()
+		return c.EvalSha(ctx, hash, keys, args...)
+	}
+
+	return r
 }

 func (s *Script) EvalShaRO(ctx context.Context, c Scripter, keys []string, args ...interface{}) *Cmd {
-	return c.EvalShaRO(ctx, s.hash, keys, args...)
+	if !s.serverSHA {
+		s.mu.RLock()
+		hash := s.hash
+		s.mu.RUnlock()
+		return c.EvalShaRO(ctx, hash, keys, args...)
+	}
+
+	if err := s.ensureHash(ctx, c); err != nil {
+		return s.EvalRO(ctx, c, keys, args...)
+	}
+
+	s.mu.RLock()
+	hash := s.hash
+	s.mu.RUnlock()
+
+	r := c.EvalShaRO(ctx, hash, keys, args...)
+	if HasErrorPrefix(r.Err(), "NOSCRIPT") {
+		if err := s.ensureHash(ctx, c); err != nil {
+			return s.EvalRO(ctx, c, keys, args...)
+		}
+		s.mu.RLock()
+		hash = s.hash
+		s.mu.RUnlock()
+		return c.EvalShaRO(ctx, hash, keys, args...)
+	}
+
+	return r
 }

 // Run optimistically uses EVALSHA to run the script. If script does not exist
--- a/script_server_sha_integration_test.go
+++ b/script_server_sha_integration_test.go
@@ -0,0 +1,74 @@
+package redis_test
+
+import (
+	. "github.com/bsm/ginkgo/v2"
+	. "github.com/bsm/gomega"
+	"github.com/redis/go-redis/v9"
+)
+
+var _ = Describe("NewScriptServerSHA", func() {
+	var client *redis.Client
+
+	BeforeEach(func() {
+		client = redis.NewClient(redisOptions())
+		Expect(client.FlushDB(ctx).Err()).NotTo(HaveOccurred())
+	})
+
+	AfterEach(func() {
+		Expect(client.Close()).NotTo(HaveOccurred())
+	})
+
+	It("should reload script after SCRIPT FLUSH", func() {
+		s := redis.NewScriptServerSHA("return 1")
+
+		// First run should succeed (loads script via SCRIPT LOAD)
+		err := s.Run(ctx, client, nil).Err()
+		Expect(err).NotTo(HaveOccurred())
+
+		// Flush all scripts from Redis
+		err = client.Do(ctx, "SCRIPT", "FLUSH").Err()
+		Expect(err).NotTo(HaveOccurred())
+
+		// Second run should still succeed (reloads script after NOSCRIPT)
+		err = s.Run(ctx, client, nil).Err()
+		Expect(err).NotTo(HaveOccurred())
+	})
+
+	It("should run script with RunRO", func() {
+		s := redis.NewScriptServerSHA("return 1")
+
+		err := s.RunRO(ctx, client, nil).Err()
+		Expect(err).NotTo(HaveOccurred())
+	})
+
+	It("should run script with EvalSha", func() {
+		s := redis.NewScriptServerSHA("return KEYS[1]")
+
+		result, err := s.EvalSha(ctx, client, []string{"mykey"}).Result()
+		Expect(err).NotTo(HaveOccurred())
+		Expect(result).To(Equal("mykey"))
+	})
+
+	It("should run script with EvalShaRO", func() {
+		s := redis.NewScriptServerSHA("return ARGV[1]")
+
+		result, err := s.EvalShaRO(ctx, client, nil, "myarg").Result()
+		Expect(err).NotTo(HaveOccurred())
+		Expect(result).To(Equal("myarg"))
+	})
+
+	It("should cache hash after first load", func() {
+		s := redis.NewScriptServerSHA("return 42")
+
+		// Hash should be empty before first run
+		Expect(s.Hash()).To(Equal(""))
+
+		// Run the script
+		result, err := s.Run(ctx, client, nil).Result()
+		Expect(err).NotTo(HaveOccurred())
+		Expect(result).To(Equal(int64(42)))
+
+		// Hash should be populated after run
+		Expect(s.Hash()).NotTo(Equal(""))
+	})
+})
--- a/script_server_sha_unit_test.go
+++ b/script_server_sha_unit_test.go
@@ -0,0 +1,193 @@
+package redis
+
+import (
+	"context"
+	"strings"
+	"sync"
+	"testing"
+
+	"github.com/redis/go-redis/v9/internal/proto"
+)
+
+// fakeScripter simulates enough of Scripter for unit testing Script behavior.
+// It records which redis commands were invoked and can simulate NOSCRIPT.
+type fakeScripter struct {
+	mu sync.Mutex
+
+	// call counters
+	scriptLoadCalls int
+	evalCalls       int
+	evalShaCalls    int
+	evalROCalls     int
+	evalShaROCalls  int
+
+	// behavior controls
+	hashToReturn string
+
+	// If set, the first EvalSha/EvalShaRO returns a NOSCRIPT error.
+	failFirstEvalShaWithNoScr   bool
+	failFirstEvalShaROWithNoScr bool
+}
+
+func (f *fakeScripter) ScriptLoad(ctx context.Context, script string) *StringCmd {
+	f.mu.Lock()
+	f.scriptLoadCalls++
+	f.mu.Unlock()
+
+	cmd := NewStringCmd(ctx)
+	cmd.SetVal(f.hashToReturn)
+	return cmd
+}
+
+func (f *fakeScripter) ScriptExists(ctx context.Context, hashes ...string) *BoolSliceCmd {
+	cmd := NewBoolSliceCmd(ctx)
+	vals := make([]bool, len(hashes))
+	for i := range vals {
+		vals[i] = true
+	}
+	cmd.SetVal(vals)
+	return cmd
+}
+
+func (f *fakeScripter) Eval(ctx context.Context, script string, keys []string, args ...interface{}) *Cmd {
+	f.mu.Lock()
+	f.evalCalls++
+	f.mu.Unlock()
+
+	cmd := NewCmd(ctx)
+	cmd.SetErr(nil)
+	return cmd
+}
+
+func (f *fakeScripter) EvalSha(ctx context.Context, sha1 string, keys []string, args ...interface{}) *Cmd {
+	f.mu.Lock()
+	f.evalShaCalls++
+	callNum := f.evalShaCalls
+	fail := f.failFirstEvalShaWithNoScr && callNum == 1
+	f.mu.Unlock()
+
+	cmd := NewCmd(ctx)
+	if fail {
+		// Use RedisError so go-redis NOSCRIPT detection triggers.
+		cmd.SetErr(proto.RedisError("NOSCRIPT No matching script. Please use EVAL."))
+		return cmd
+	}
+	cmd.SetErr(nil)
+	return cmd
+}
+
+func (f *fakeScripter) EvalRO(ctx context.Context, script string, keys []string, args ...interface{}) *Cmd {
+	f.mu.Lock()
+	f.evalROCalls++
+	f.mu.Unlock()
+
+	cmd := NewCmd(ctx)
+	cmd.SetErr(nil)
+	return cmd
+}
+
+func (f *fakeScripter) EvalShaRO(ctx context.Context, sha1 string, keys []string, args ...interface{}) *Cmd {
+	f.mu.Lock()
+	f.evalShaROCalls++
+	callNum := f.evalShaROCalls
+	fail := f.failFirstEvalShaROWithNoScr && callNum == 1
+	f.mu.Unlock()
+
+	cmd := NewCmd(ctx)
+	if fail {
+		cmd.SetErr(proto.RedisError("NOSCRIPT No matching script. Please use EVAL."))
+		return cmd
+	}
+	cmd.SetErr(nil)
+	return cmd
+}
+
+func TestNewScriptServerSHA_Run_UsesScriptLoadAndEvalSha_NoEval(t *testing.T) {
+	ctx := context.Background()
+	c := &fakeScripter{hashToReturn: strings.Repeat("a", 40)}
+
+	s := NewScriptServerSHA("return 1")
+	if err := s.Run(ctx, c, []string{"k"}).Err(); err != nil {
+		t.Fatalf("Run() err: %v", err)
+	}
+
+	if c.scriptLoadCalls != 1 {
+		t.Fatalf("expected ScriptLoad 1 call, got %d", c.scriptLoadCalls)
+	}
+	if c.evalShaCalls != 1 {
+		t.Fatalf("expected EvalSha 1 call, got %d", c.evalShaCalls)
+	}
+	if c.evalCalls != 0 {
+		t.Fatalf("expected Eval 0 calls, got %d", c.evalCalls)
+	}
+}
+
+func TestNewScriptServerSHA_Run_RetriesOnNoScript_NoEvalFallback(t *testing.T) {
+	ctx := context.Background()
+	c := &fakeScripter{
+		hashToReturn:              strings.Repeat("b", 40),
+		failFirstEvalShaWithNoScr: true,
+	}
+
+	s := NewScriptServerSHA("return 1")
+	if err := s.Run(ctx, c, []string{"k"}).Err(); err != nil {
+		t.Fatalf("Run() err: %v", err)
+	}
+
+	// Expected:
+	// - ScriptLoad once (hash cached in Script)
+	// - EvalSha twice (first NOSCRIPT, second retry)
+	if c.scriptLoadCalls != 1 {
+		t.Fatalf("expected ScriptLoad 1 call (hash cached), got %d", c.scriptLoadCalls)
+	}
+	if c.evalShaCalls != 2 {
+		t.Fatalf("expected EvalSha 2 calls (retry), got %d", c.evalShaCalls)
+	}
+	// Critical: serverSHA path should NOT fallback to Eval
+	if c.evalCalls != 0 {
+		t.Fatalf("expected Eval 0 calls, got %d", c.evalCalls)
+	}
+}
+
+func TestNewScriptServerSHA_RunRO_UsesScriptLoadAndEvalShaRO_NoEvalRO(t *testing.T) {
+	ctx := context.Background()
+	c := &fakeScripter{hashToReturn: strings.Repeat("c", 40)}
+
+	s := NewScriptServerSHA("return 1")
+	if err := s.RunRO(ctx, c, []string{"k"}).Err(); err != nil {
+		t.Fatalf("RunRO() err: %v", err)
+	}
+
+	if c.scriptLoadCalls != 1 {
+		t.Fatalf("expected ScriptLoad 1 call, got %d", c.scriptLoadCalls)
+	}
+	if c.evalShaROCalls != 1 {
+		t.Fatalf("expected EvalShaRO 1 call, got %d", c.evalShaROCalls)
+	}
+	if c.evalROCalls != 0 {
+		t.Fatalf("expected EvalRO 0 calls, got %d", c.evalROCalls)
+	}
+}
+
+func TestNewScriptServerSHA_RunRO_RetriesOnNoScript_NoEvalROFallback(t *testing.T) {
+	ctx := context.Background()
+	c := &fakeScripter{
+		hashToReturn:                strings.Repeat("d", 40),
+		failFirstEvalShaROWithNoScr: true,
+	}
+
+	s := NewScriptServerSHA("return 1")
+	if err := s.RunRO(ctx, c, []string{"k"}).Err(); err != nil {
+		t.Fatalf("RunRO() err: %v", err)
+	}
+
+	if c.scriptLoadCalls != 1 {
+		t.Fatalf("expected ScriptLoad 1 call (hash cached), got %d", c.scriptLoadCalls)
+	}
+	if c.evalShaROCalls != 2 {
+		t.Fatalf("expected EvalShaRO 2 calls (retry), got %d", c.evalShaROCalls)
+	}
+	if c.evalROCalls != 0 {
+		t.Fatalf("expected EvalRO 0 calls, got %d", c.evalROCalls)
+	}
+}