From 91e34a0929e52e5ad73fe0b2f08feeee866a93f0 Mon Sep 17 00:00:00 2001 From: Ilia Denisov Date: Sun, 24 May 2026 02:42:09 +0200 Subject: [PATCH] fix(gateway): verify client signature before payload_hash MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ARCHITECTURE.md §15 "Verification order" specifies signature verification (step 4) before payload_hash (step 5), but the authenticated-edge decorator chain wrapped the payload-hash gate outside the signature gate, so the hash was checked first. gateway/README.md and gateway/docs/flows.md had drifted to match the code (hash-first), leaving ARCHITECTURE.md as the lone source describing the intended order. Swap the two decorators in server.go so the signature gate runs first, and align README + flows.md to ARCHITECTURE.md. Signature-first is the cryptographically sound order: the signature covers the payload_hash field, so the request is authenticated before any of its content is processed. Observable side effect: a request carrying a tampered payload_hash whose signature was computed over the original hash is now rejected at the signature gate (UNAUTHENTICATED "invalid request signature") instead of the hash gate (INVALID_ARGUMENT). Security is unchanged — both refusals happen before the payload is handled. The four payload-hash unit tests re-sign over the tampered hash so they keep exercising the hash gate; the cross-service integration test signs over the overridden hash and already accepts both codes. Refs #39 Co-Authored-By: Claude Opus 4.7 --- gateway/README.md | 4 ++-- gateway/docs/flows.md | 2 +- gateway/internal/grpcapi/payload_hash.go | 4 ++-- .../grpcapi/payload_hash_integration_test.go | 12 ++++++++++++ gateway/internal/grpcapi/server.go | 4 ++-- gateway/internal/grpcapi/signature.go | 4 ++-- 6 files changed, 21 insertions(+), 9 deletions(-) diff --git a/gateway/README.md b/gateway/README.md index a1f13eb..e9a927b 100644 --- a/gateway/README.md +++ b/gateway/README.md @@ -579,8 +579,8 @@ ingress. 2. Check whether `protocol_version` is supported. 3. Resolve `device_session_id` through `SessionCache`. 4. Reject unknown or revoked sessions. -5. Verify that `payload_hash` matches raw `payload_bytes`. -6. Verify the client signature using the public key from session cache. +5. Verify the client signature using the public key from session cache. +6. Verify that `payload_hash` matches raw `payload_bytes`. 7. Verify that `timestamp_ms` is inside the accepted freshness window. 8. Verify anti-replay by checking `device_session_id + request_id`. 9. Apply authenticated rate limit and edge policy checks. diff --git a/gateway/docs/flows.md b/gateway/docs/flows.md index f6c0fa3..bf91750 100644 --- a/gateway/docs/flows.md +++ b/gateway/docs/flows.md @@ -38,8 +38,8 @@ sequenceDiagram Gateway->>Gateway: validate envelope + protocol_version Gateway->>Backend: GET /api/v1/internal/sessions/{device_session_id} Backend-->>Gateway: session record - Gateway->>Gateway: verify payload_hash Gateway->>Gateway: verify Ed25519 signature + Gateway->>Gateway: verify payload_hash Gateway->>Gateway: verify freshness window Gateway->>Replay: reserve(device_session_id, request_id, ttl) Replay-->>Gateway: accepted diff --git a/gateway/internal/grpcapi/payload_hash.go b/gateway/internal/grpcapi/payload_hash.go index 5ea1fbe..0f8586a 100644 --- a/gateway/internal/grpcapi/payload_hash.go +++ b/gateway/internal/grpcapi/payload_hash.go @@ -12,8 +12,8 @@ import ( "google.golang.org/grpc/status" ) -// payloadHashVerifyingService applies payload-hash verification after session -// lookup and before any later auth or routing step runs. +// payloadHashVerifyingService applies payload-hash verification after +// client-signature verification and before any later auth or routing step runs. type payloadHashVerifyingService struct { edgev1.UnimplementedGatewayServer diff --git a/gateway/internal/grpcapi/payload_hash_integration_test.go b/gateway/internal/grpcapi/payload_hash_integration_test.go index 6ee9def..c0f853c 100644 --- a/gateway/internal/grpcapi/payload_hash_integration_test.go +++ b/gateway/internal/grpcapi/payload_hash_integration_test.go @@ -27,6 +27,9 @@ func TestExecuteCommandRejectsPayloadHashWithInvalidLength(t *testing.T) { req := newValidExecuteCommandRequest() req.PayloadHash = []byte("short") + // Signature verification now precedes the payload-hash gate, so re-sign + // over the tampered hash to keep the signature valid and exercise the gate. + req.Signature = signRequest(req.GetProtocolVersion(), req.GetDeviceSessionId(), req.GetMessageType(), req.GetTimestampMs(), req.GetRequestId(), req.GetPayloadHash()) _, err := client.ExecuteCommand(context.Background(), connect.NewRequest(req)) require.Error(t, err) @@ -51,6 +54,9 @@ func TestExecuteCommandRejectsPayloadHashMismatch(t *testing.T) { req := newValidExecuteCommandRequest() sum := sha256.Sum256([]byte("other")) req.PayloadHash = sum[:] + // Signature verification now precedes the payload-hash gate, so re-sign + // over the tampered hash to keep the signature valid and exercise the gate. + req.Signature = signRequest(req.GetProtocolVersion(), req.GetDeviceSessionId(), req.GetMessageType(), req.GetTimestampMs(), req.GetRequestId(), req.GetPayloadHash()) _, err := client.ExecuteCommand(context.Background(), connect.NewRequest(req)) require.Error(t, err) @@ -74,6 +80,9 @@ func TestSubscribeEventsRejectsPayloadHashWithInvalidLength(t *testing.T) { req := newValidSubscribeEventsRequest() req.PayloadHash = []byte("short") + // Signature verification now precedes the payload-hash gate, so re-sign + // over the tampered hash to keep the signature valid and exercise the gate. + req.Signature = signRequest(req.GetProtocolVersion(), req.GetDeviceSessionId(), req.GetMessageType(), req.GetTimestampMs(), req.GetRequestId(), req.GetPayloadHash()) err := subscribeEventsError(t, context.Background(), client, req) require.Error(t, err) @@ -98,6 +107,9 @@ func TestSubscribeEventsRejectsPayloadHashMismatch(t *testing.T) { req := newValidSubscribeEventsRequest() sum := sha256.Sum256([]byte("other")) req.PayloadHash = sum[:] + // Signature verification now precedes the payload-hash gate, so re-sign + // over the tampered hash to keep the signature valid and exercise the gate. + req.Signature = signRequest(req.GetProtocolVersion(), req.GetDeviceSessionId(), req.GetMessageType(), req.GetTimestampMs(), req.GetRequestId(), req.GetPayloadHash()) err := subscribeEventsError(t, context.Background(), client, req) require.Error(t, err) diff --git a/gateway/internal/grpcapi/server.go b/gateway/internal/grpcapi/server.go index 33ec375..f112e3e 100644 --- a/gateway/internal/grpcapi/server.go +++ b/gateway/internal/grpcapi/server.go @@ -128,8 +128,8 @@ func NewServer(cfg config.AuthenticatedGRPCConfig, deps ServerDependencies) *Ser cfg: cfg, service: newEnvelopeValidatingService( newSessionLookupService( - newPayloadHashVerifyingService( - newSignatureVerifyingService( + newSignatureVerifyingService( + newPayloadHashVerifyingService( newFreshnessAndReplayService( newAuthenticatedRateLimitService( finalService, diff --git a/gateway/internal/grpcapi/signature.go b/gateway/internal/grpcapi/signature.go index f92eb04..f4a10e0 100644 --- a/gateway/internal/grpcapi/signature.go +++ b/gateway/internal/grpcapi/signature.go @@ -12,8 +12,8 @@ import ( "google.golang.org/grpc/status" ) -// signatureVerifyingService applies client-signature verification after -// payload integrity checks and before later auth or routing steps run. +// signatureVerifyingService applies client-signature verification after session +// lookup and before payload-hash verification and later routing steps run. type signatureVerifyingService struct { edgev1.UnimplementedGatewayServer -- 2.52.0