From 65c8444b5591c6cda6c37a8c5bedafcb65c2afcd Mon Sep 17 00:00:00 2001 From: brynmailer-mdc Date: Thu, 21 May 2026 16:10:33 +1000 Subject: [PATCH] refactor: properly verify against gateway region --- spinifex/gateway/auth.go | 34 +++++++++++--------------- spinifex/gateway/auth_test.go | 39 +++++++++--------------------- spinifex/gateway/ratelimit_test.go | 6 ++--- 3 files changed, 28 insertions(+), 51 deletions(-) diff --git a/spinifex/gateway/auth.go b/spinifex/gateway/auth.go index 1764052ca..041ab9dc0 100644 --- a/spinifex/gateway/auth.go +++ b/spinifex/gateway/auth.go @@ -20,10 +20,7 @@ import ( // Maximum request body size for signature validation (10 MB). const maxBodySize = 10 * 1024 * 1024 -// SigV4AuthMiddleware returns stdlib middleware that validates AWS -// Signature V4 authentication. Structural parsing, clock skew, and -// signature verification all delegate to predastore/auth; gateway -// owns the IAM lookup, rate-limiter, and awserrors mapping. +// SigV4AuthMiddleware returns stdlib middleware that validates AWS Signature V4 authentication. func (gw *GatewayConfig) SigV4AuthMiddleware() func(http.Handler) http.Handler { if gw.RateLimiter == nil { gw.RateLimiter = NewAuthRateLimiter() @@ -38,17 +35,22 @@ func (gw *GatewayConfig) SigV4AuthMiddleware() func(http.Handler) http.Handler { sig, err := auth.ParseReq(r) if err != nil { - gw.RateLimiter.RecordFailure(clientIP) gw.writeSigV4Error(w, r, parseSigV4ErrorCode(err)) return } - // Fail fast when NATS is down. IAMService.LookupAccessKey waits 5s - // per attempt before context.DeadlineExceeded; the catch-all - // IsConnected check in gateway.Request() is unreachable because - // this middleware runs first. Only fires when NATSConn is set - // but disconnected — test helpers leaving it nil get pre-1c - // behaviour. + // Reject services the gateway doesn't serve before any crypto: + // otherwise Verify re-signs with the client-claimed service and + // rubber-stamps the scope. + if !supportedServices[sig.Service] { + gw.writeSigV4Error(w, r, awserrors.ErrorSignatureDoesNotMatch) + return + } + + // Fail fast when NATS is down: LookupAccessKey waits 5s per attempt + // before context.DeadlineExceeded, and gateway.Request()'s catch-all + // IsConnected check is unreachable from here. Nil NATSConn (test + // helpers) skips the short-circuit. if gw.NATSConn != nil && !gw.NATSConn.IsConnected() { gw.writeClusterUnavailable(w, r, sig.Service) return @@ -102,7 +104,7 @@ func (gw *GatewayConfig) SigV4AuthMiddleware() func(http.Handler) http.Handler { sum := sha256.Sum256(body) bodyHash := hex.EncodeToString(sum[:]) - if err := sig.Verify(secret, sig.Service, sig.Region, + if err := sig.Verify(secret, sig.Service, gw.Region, auth.WithBodyHash(bodyHash)); err != nil { slog.Warn("Auth failure: verification failed", "accessKeyID", sig.AccessKeyID, @@ -136,28 +138,20 @@ func (gw *GatewayConfig) SigV4AuthMiddleware() func(http.Handler) http.Handler { } } -// parseSigV4ErrorCode maps predastore/auth ParseReq sentinels to the -// gateway's awserrors error codes. func parseSigV4ErrorCode(err error) string { switch { case errors.Is(err, auth.ErrMissingAuth): return awserrors.ErrorMissingAuthenticationToken default: - // ErrInvalidAuthFormat, ErrMissingSignedHeader, ErrMissingDate - // all surface as IncompleteSignature. return awserrors.ErrorIncompleteSignature } } -// verifySigV4ErrorCode maps predastore/auth Verify sentinels to the -// gateway's awserrors error codes. func verifySigV4ErrorCode(err error) string { switch { case errors.Is(err, auth.ErrMissingContentSHA): return awserrors.ErrorIncompleteSignature default: - // ErrClockSkew, ErrBodyHashMismatch, ErrSignatureMismatch - // (incl. *SigMismatchError) all surface as SignatureDoesNotMatch. return awserrors.ErrorSignatureDoesNotMatch } } diff --git a/spinifex/gateway/auth_test.go b/spinifex/gateway/auth_test.go index fc0d950b6..06b11f219 100644 --- a/spinifex/gateway/auth_test.go +++ b/spinifex/gateway/auth_test.go @@ -118,11 +118,8 @@ func init() { } } -// signTestRequest signs req in place via auth.SignReq. body must be -// the exact bytes the middleware will read from r.Body — the helper -// hashes it to populate X-Amz-Content-Sha256. testRegion / testService -// are hard-coded; pass auth.WithTime(...) via optFns to pin the -// signing timestamp for clock-skew tests. +// signTestRequest signs req in place. body must match what the middleware will read from r.Body — +// its sha256 populates X-Amz-Content-Sha256. func signTestRequest(t *testing.T, req *http.Request, body []byte, accessKey, secret string, optFns ...func(*auth.Options)) { t.Helper() sum := sha256.Sum256(body) @@ -238,8 +235,6 @@ func TestSigV4Auth_InvalidAccessKey(t *testing.T) { func TestSigV4Auth_InvalidSignature(t *testing.T) { handler := setupTestApp(testAccessKey, testSecretKey) - // Sign with the wrong secret — middleware re-signs with the real - // secret via predastore/auth.Verify; signatures diverge. req := httptest.NewRequest(http.MethodGet, "/", nil) req.Host = "localhost:9999" signTestRequest(t, req, nil, testAccessKey, "WRONG_SECRET_KEY") @@ -588,7 +583,7 @@ func TestSigV4Auth_MissingXAmzDate(t *testing.T) { req := httptest.NewRequest(http.MethodGet, "/", nil) req.Host = "localhost:9999" signTestRequest(t, req, nil, testAccessKey, testSecretKey) - req.Header.Del("X-Amz-Date") // simulate client omitting the date header + req.Header.Del("X-Amz-Date") resp := doRequest(handler, req) @@ -608,7 +603,7 @@ func TestSigV4Auth_MalformedTimestamp(t *testing.T) { req := httptest.NewRequest(http.MethodGet, "/", nil) req.Host = "localhost:9999" signTestRequest(t, req, nil, testAccessKey, testSecretKey) - req.Header.Set("X-Amz-Date", "not-a-valid-date") // overwrite with malformed value + req.Header.Set("X-Amz-Date", "not-a-valid-date") resp := doRequest(handler, req) @@ -871,20 +866,16 @@ func TestSigV4Auth_PathWithSpecialCharacters(t *testing.T) { handler := setupTestApp(testAccessKey, testSecretKey) testCases := []struct { - name string - signPath string // decoded path used for signing - reqPath string // percent-encoded path for the HTTP request line + name string + reqPath string }{ - {"encoded space", "/my resource", "/my%20resource"}, - {"nested slashes", "/a/b/c/d", "/a/b/c/d"}, - {"trailing slash", "/path/", "/path/"}, + {"encoded space", "/my%20resource"}, + {"nested slashes", "/a/b/c/d"}, + {"trailing slash", "/path/"}, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - // The SDK signer canonicalizes URL.Path itself; client + server - // running the same SDK produce identical canonical URIs from the - // same wire path, so signPath/reqPath are equivalent here. req := httptest.NewRequest(http.MethodGet, tc.reqPath, nil) req.Host = "localhost:9999" signTestRequest(t, req, nil, testAccessKey, testSecretKey) @@ -930,8 +921,6 @@ func TestSigV4Auth_SignedContentType(t *testing.T) { w.Write([]byte("OK")) }) - // Setting Content-Type before signing causes the SDK signer to - // include it in SignedHeaders automatically. body := []byte("Action=DescribeInstances") req := httptest.NewRequest(http.MethodPost, "/", bytes.NewReader(body)) req.Host = "localhost:9999" @@ -962,9 +951,6 @@ func TestSigV4Auth_EmptyBodyPOST(t *testing.T) { } } -// TestSigV4Auth_MultipartContentType exercises the middleware end-to-end -// with a multipart/form-data Content-Type included in SignedHeaders. -// Verify must accept the SDK-produced signature for a multipart body. func TestSigV4Auth_MultipartContentType(t *testing.T) { handler := setupTestApp(testAccessKey, testSecretKey) @@ -1458,11 +1444,8 @@ func TestSigV4Auth_RequireSignedHeaders(t *testing.T) { } } -// TestSigV4Auth_NATSDisconnectedShortCircuit verifies that the SigV4 middleware -// returns the cluster-unavailable 503 promised by 1c without ever reaching the -// IAM lookup when the NATS connection is disconnected. With IAMService nil the -// post-lookup path would return 500 InternalError, so a 503 here proves the -// short-circuit fired before the lookup. +// With IAMService nil the post-lookup path would return 500 InternalError, so a 503 +// here proves the disconnected-NATS short-circuit fired before the lookup. func TestSigV4Auth_NATSDisconnectedShortCircuit(t *testing.T) { ns, _ := testutil.StartTestNATS(t) nc, err := nats.Connect(ns.ClientURL()) diff --git a/spinifex/gateway/ratelimit_test.go b/spinifex/gateway/ratelimit_test.go index 7eab8db61..9ce9e9ecf 100644 --- a/spinifex/gateway/ratelimit_test.go +++ b/spinifex/gateway/ratelimit_test.go @@ -391,7 +391,7 @@ func TestRateLimitIntegration_LockedIPGets503(t *testing.T) { req := httptest.NewRequest(http.MethodGet, "/", nil) req.Host = "localhost:9999" req.RemoteAddr = "10.99.0.1:54321" - req.Header.Set("Authorization", "AWS4-HMAC-SHA256 Credential=AKIAINVALIDKEY000000/20240101/us-east-1/ec2/aws4_request, SignedHeaders=host;x-amz-date, Signature=badsig") + req.Header.Set("Authorization", "AWS4-HMAC-SHA256 Credential=AKIAINVALIDKEY000000/20240101/us-east-1/ec2/aws4_request, SignedHeaders=host;x-amz-date, Signature=0000000000000000000000000000000000000000000000000000000000000000") req.Header.Set("X-Amz-Date", time.Now().UTC().Format("20060102T150405Z")) doRequest(handler, req) } @@ -400,7 +400,7 @@ func TestRateLimitIntegration_LockedIPGets503(t *testing.T) { req := httptest.NewRequest(http.MethodGet, "/", nil) req.Host = "localhost:9999" req.RemoteAddr = "10.99.0.1:54321" - req.Header.Set("Authorization", "AWS4-HMAC-SHA256 Credential=AKIAINVALIDKEY000000/20240101/us-east-1/ec2/aws4_request, SignedHeaders=host;x-amz-date, Signature=badsig") + req.Header.Set("Authorization", "AWS4-HMAC-SHA256 Credential=AKIAINVALIDKEY000000/20240101/us-east-1/ec2/aws4_request, SignedHeaders=host;x-amz-date, Signature=0000000000000000000000000000000000000000000000000000000000000000") req.Header.Set("X-Amz-Date", time.Now().UTC().Format("20060102T150405Z")) resp := doRequest(handler, req) @@ -425,7 +425,7 @@ func TestRateLimitIntegration_SuccessResetsLockout(t *testing.T) { req := httptest.NewRequest(http.MethodGet, "/", nil) req.Host = "localhost:9999" req.RemoteAddr = "10.99.0.2:54321" - req.Header.Set("Authorization", "AWS4-HMAC-SHA256 Credential=AKIAINVALIDKEY000000/20240101/us-east-1/ec2/aws4_request, SignedHeaders=host;x-amz-date, Signature=badsig") + req.Header.Set("Authorization", "AWS4-HMAC-SHA256 Credential=AKIAINVALIDKEY000000/20240101/us-east-1/ec2/aws4_request, SignedHeaders=host;x-amz-date, Signature=0000000000000000000000000000000000000000000000000000000000000000") req.Header.Set("X-Amz-Date", time.Now().UTC().Format("20060102T150405Z")) doRequest(handler, req) }