Skip to content

Commit 89d56ef

Browse files
committed
Implemented the API Gateway race-condition
1 parent c7d497a commit 89d56ef

5 files changed

Lines changed: 226 additions & 5 deletions

File tree

backend/SECURITY_PERFORMANCE_IMPROVEMENTS_SUMMARY.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,13 @@
1+
# API Gateway Security Patch
2+
3+
API gateway timestamps are accepted only as canonical, unsigned decimal Unix seconds that fit within JavaScript's safe integer range. Values with a decimal, sign, hexadecimal prefix, trailing data, or unsafe magnitude are rejected before signature verification.
4+
5+
Mutating signed requests use an atomic Redis `SET NX` reservation keyed by a SHA-256 digest of the API secret and signature. This closes the replay race between API Gateway instances. Configure `REDIS_URL` in multi-instance deployments; when configured Redis is unavailable, authentication fails closed with `API_GATEWAY_REPLAY_PROTECTION_UNAVAILABLE` (HTTP 503) rather than accepting a request without distributed replay protection.
6+
7+
The API-key middleware only accepts a pre-populated merchant context from the trusted x402 bridge (`req.x402`). A merchant object by itself is never sufficient to bypass API-key lookup and expiry checks. Requests using API gateway signatures must continue to send `x-api-key`, `x-api-signature` (`sha256=` plus 64 lowercase hexadecimal characters), and `x-api-timestamp`.
8+
9+
Regression coverage is provided by `src/lib/api-gateway-signature.test.js` and `src/lib/auth.test.js`. Run `npm test -- --run src/lib/api-gateway-signature.test.js src/lib/auth.test.js` from `backend/` to validate the patch.
10+
111
# Security and Performance Improvements Summary
212

313
**Project**: Stellar Payment API - Transaction Signer & Ledger Monitor

backend/src/lib/api-gateway-signature.js

Lines changed: 67 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import crypto from "node:crypto";
22
import { logger } from "./logger.js";
33
import { apiGatewaySignatureCacheSize, apiGatewayReplayBlockedTotal } from "./metrics.js";
4+
import { connectRedisClient } from "./redis.js";
45

56
const DEFAULT_SIGNATURE_WINDOW_SECONDS = 300;
67
// Minimum HMAC secret length to prevent signing with trivially weak keys
@@ -262,6 +263,62 @@ function buildCanonicalPayload({ method, path, timestamp, body }) {
262263
// state-changing requests, where re-execution has a real side effect.
263264
const REPLAY_PROTECTED_METHODS = new Set(["POST", "PUT", "PATCH", "DELETE"]);
264265

266+
function isReplayProtectedMethod(method) {
267+
return REPLAY_PROTECTED_METHODS.has(String(method || "GET").toUpperCase());
268+
}
269+
270+
function distributedReplayKey(secret, signatureHeader) {
271+
return `api-gateway:replay:${crypto
272+
.createHash("sha256")
273+
.update(`${secret}:${signatureHeader}`, "utf8")
274+
.digest("hex")}`;
275+
}
276+
277+
/**
278+
* Atomically reserve a mutating request signature across API instances.
279+
* Redis SET NX is used when REDIS_URL is configured; the verifier's local
280+
* cache remains the fallback for single-instance deployments.
281+
*/
282+
export async function reserveApiGatewaySignature({
283+
secret,
284+
signatureHeader,
285+
method,
286+
toleranceSeconds,
287+
redisClient,
288+
}) {
289+
if (!isReplayProtectedMethod(method)) return { reserved: true };
290+
291+
if (!process.env.REDIS_URL && !redisClient) return { reserved: true };
292+
293+
try {
294+
const client = redisClient || (await connectRedisClient());
295+
if (!client?.isOpen) {
296+
throw new Error("Redis is unavailable for distributed replay protection");
297+
}
298+
299+
const result = await client.set(
300+
distributedReplayKey(secret, signatureHeader),
301+
"1",
302+
{ NX: true, EX: Math.max(1, Math.ceil(toleranceSeconds)) },
303+
);
304+
305+
if (result !== "OK") {
306+
apiGatewayReplayBlockedTotal.inc();
307+
logger.warn("Rejected replayed API gateway signature from distributed cache");
308+
return { reserved: false, replay: true };
309+
}
310+
311+
return { reserved: true };
312+
} catch (err) {
313+
logger.error({ err }, "Distributed API gateway replay protection unavailable");
314+
return {
315+
reserved: false,
316+
code: "API_GATEWAY_REPLAY_PROTECTION_UNAVAILABLE",
317+
reason: "API gateway replay protection is temporarily unavailable",
318+
};
319+
}
320+
}
321+
265322
function signaturesEqual(a, b) {
266323
const aBuf = Buffer.from(a, "hex");
267324
const bBuf = Buffer.from(b, "hex");
@@ -381,13 +438,20 @@ export function verifyApiGatewayRequestSignature({
381438
return { valid: false, reason: "Missing or insufficient signature secret" };
382439
}
383440

384-
const timestamp = Number.parseInt(String(timestampHeader || ""), 10);
385-
if (!Number.isFinite(timestamp)) {
441+
const timestampValue = String(timestampHeader || "").trim();
442+
if (!/^[0-9]+$/.test(timestampValue)) {
386443
recordApiGatewaySignatureAttempt(clientIp, false, now);
387444
_recordCircuitBreakerFailure(now);
388445
logger.warn({ timestampHeader, clientIp }, "Missing or invalid x-api-timestamp header");
389446
return { valid: false, reason: "Missing or invalid x-api-timestamp header" };
390447
}
448+
const timestamp = Number(timestampValue);
449+
if (!Number.isSafeInteger(timestamp)) {
450+
recordApiGatewaySignatureAttempt(clientIp, false, now);
451+
_recordCircuitBreakerFailure(now);
452+
logger.warn({ clientIp }, "API gateway timestamp exceeds safe integer range");
453+
return { valid: false, reason: "Missing or invalid x-api-timestamp header" };
454+
}
391455

392456
const deltaSeconds = Math.abs(Math.floor(now / 1000) - timestamp);
393457
if (deltaSeconds > toleranceSeconds) {
@@ -424,7 +488,7 @@ export function verifyApiGatewayRequestSignature({
424488
// that has already been used within its own tolerance window is a
425489
// replay of a captured request, not a legitimate second use. Scoped to
426490
// state-changing methods only - see REPLAY_PROTECTED_METHODS.
427-
const isReplayProtected = REPLAY_PROTECTED_METHODS.has(String(method || "GET").toUpperCase());
491+
const isReplayProtected = isReplayProtectedMethod(method);
428492
if (isReplayProtected && _isReplayedSignature(receivedSignature, now)) {
429493
recordApiGatewaySignatureAttempt(clientIp, false, now);
430494
_recordCircuitBreakerFailure(now);

backend/src/lib/api-gateway-signature.test.js

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import {
77
_apiGatewayRateLimitState,
88
_verifiedSignatureCache,
99
getApiGatewaySignatureCacheStats,
10+
reserveApiGatewaySignature,
1011
} from "./api-gateway-signature.js";
1112

1213
// All secrets must be >= 16 characters (MIN_SECRET_LENGTH enforcement, issue #767)
@@ -91,6 +92,76 @@ describe("api-gateway-signature", () => {
9192
expect(result.reason).toMatch(/invalid x-api-signature/i);
9293
});
9394

95+
it.each(["1713916800abc", "1713916800.5", "+1713916800", "0x6611f800"])(
96+
"rejects non-canonical timestamp %s",
97+
(timestampHeader) => {
98+
const result = verifyApiGatewayRequestSignature({
99+
secret: VALID_SECRET,
100+
method: "GET",
101+
path: "/health",
102+
timestampHeader,
103+
signatureHeader: "sha256=" + "a".repeat(64),
104+
body: {},
105+
now: 1713916800 * 1000,
106+
});
107+
108+
expect(result.valid).toBe(false);
109+
expect(result.reason).toMatch(/invalid x-api-timestamp/i);
110+
},
111+
);
112+
113+
describe("distributed replay reservation", () => {
114+
it("atomically reserves a mutating signature with Redis SET NX", async () => {
115+
const set = vi.fn().mockResolvedValue("OK");
116+
const redisClient = { isOpen: true, set };
117+
118+
const result = await reserveApiGatewaySignature({
119+
secret: VALID_SECRET,
120+
signatureHeader: "sha256=" + "a".repeat(64),
121+
method: "POST",
122+
toleranceSeconds: 300,
123+
redisClient,
124+
});
125+
126+
expect(result).toEqual({ reserved: true });
127+
expect(set).toHaveBeenCalledWith(
128+
expect.stringMatching(/^api-gateway:replay:[a-f0-9]{64}$/),
129+
"1",
130+
{ NX: true, EX: 300 },
131+
);
132+
});
133+
134+
it("rejects a signature when Redis reports that it is already reserved", async () => {
135+
const redisClient = { isOpen: true, set: vi.fn().mockResolvedValue(null) };
136+
137+
const result = await reserveApiGatewaySignature({
138+
secret: VALID_SECRET,
139+
signatureHeader: "sha256=" + "b".repeat(64),
140+
method: "POST",
141+
toleranceSeconds: 300,
142+
redisClient,
143+
});
144+
145+
expect(result).toEqual({ reserved: false, replay: true });
146+
});
147+
148+
it("fails closed when configured Redis is unavailable", async () => {
149+
const redisClient = { isOpen: false, set: vi.fn() };
150+
151+
const result = await reserveApiGatewaySignature({
152+
secret: VALID_SECRET,
153+
signatureHeader: "sha256=" + "c".repeat(64),
154+
method: "POST",
155+
toleranceSeconds: 300,
156+
redisClient,
157+
});
158+
159+
expect(result.reserved).toBe(false);
160+
expect(result.code).toBe("API_GATEWAY_REPLAY_PROTECTION_UNAVAILABLE");
161+
expect(redisClient.set).not.toHaveBeenCalled();
162+
});
163+
});
164+
94165
// ── Security audit: minimum secret length (#767) ──────────────────────────
95166

96167
it("rejects signing with a secret shorter than the minimum length", () => {

backend/src/lib/auth.js

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
import bcrypt from "bcryptjs";
22
import { recordMerchantApiUsage } from "./api-usage.js";
3-
import { verifyApiGatewayRequestSignature } from "./api-gateway-signature.js";
3+
import {
4+
reserveApiGatewaySignature,
5+
verifyApiGatewayRequestSignature,
6+
} from "./api-gateway-signature.js";
47
import { queryWithRetry } from "./db.js";
58

69
const SALT_ROUNDS = 12;
@@ -73,14 +76,15 @@ export function createApiKeyAuth({
7376
supabaseClient = null, // unused for API key auth; retained for session-auth compat
7477
usageRecorder = recordMerchantApiUsage,
7578
verifyGatewaySignature = verifyApiGatewayRequestSignature,
79+
reserveGatewaySignature = reserveApiGatewaySignature,
7680
requireSignature = false,
7781
merchantLookup = defaultMerchantLookup,
7882
} = {}) {
7983
return async function requireApiKeyAuth(req, res, next) {
8084
try {
8185
// Another auth layer (e.g. x402 token bridge) may have already attached a
8286
// merchant context. If so, honor it and continue.
83-
if (req.merchant?.id) {
87+
if (req.x402 && req.merchant?.id) {
8488
try {
8589
await usageRecorder({ merchantId: req.merchant.id, req });
8690
} catch (usageError) {
@@ -134,6 +138,26 @@ export function createApiKeyAuth({
134138
...(signatureResult.rateLimitInfo && { rateLimitInfo: signatureResult.rateLimitInfo }),
135139
});
136140
}
141+
142+
const reservation = await reserveGatewaySignature({
143+
secret: apiKey,
144+
signatureHeader,
145+
method: req.method,
146+
toleranceSeconds: Number(
147+
process.env.API_GATEWAY_SIGNATURE_TOLERANCE_SECONDS || 300,
148+
),
149+
});
150+
if (!reservation.reserved) {
151+
return res.status(reservation.replay ? 401 : 503).json({
152+
error: reservation.replay
153+
? "Invalid API gateway signature"
154+
: "API gateway replay protection is temporarily unavailable",
155+
code: reservation.replay
156+
? "API_GATEWAY_REPLAY_DETECTED"
157+
: reservation.code,
158+
...(reservation.reason && { reason: reservation.reason }),
159+
});
160+
}
137161
}
138162

139163
// Block IPs that have exceeded the failed-attempt threshold (#767)

backend/src/lib/auth.test.js

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,10 +142,62 @@ describe("createApiKeyAuth", () => {
142142
timestampHeader: "1713916800",
143143
signatureHeader: "sha256=abcd",
144144
body: { amount: 1 },
145+
clientIp: "1.2.3.4",
145146
});
146147
expect(next).toHaveBeenCalledWith();
147148
});
148149

150+
it("does not trust a pre-populated merchant without x402 authentication", async () => {
151+
merchantLookup.mockResolvedValue(null);
152+
const req = createRequest({ "x-api-key": "invalid-key" }, { merchant: baseMerchant });
153+
154+
await middleware(req, res, next);
155+
156+
expect(merchantLookup).toHaveBeenCalledWith("invalid-key");
157+
expect(usageRecorder).not.toHaveBeenCalled();
158+
expect(res.status).toHaveBeenCalledWith(401);
159+
expect(next).not.toHaveBeenCalled();
160+
});
161+
162+
it("honors a merchant populated by the trusted x402 bridge", async () => {
163+
const req = createRequest({}, { merchant: baseMerchant, x402: { tx_hash: "tx-1" } });
164+
165+
await middleware(req, res, next);
166+
167+
expect(merchantLookup).not.toHaveBeenCalled();
168+
expect(usageRecorder).toHaveBeenCalledWith({ merchantId: baseMerchant.id, req });
169+
expect(next).toHaveBeenCalledWith();
170+
});
171+
172+
it("rejects a replay reported by the distributed signature reservation", async () => {
173+
merchantLookup.mockResolvedValue(baseMerchant);
174+
const reserveGatewaySignature = vi.fn().mockResolvedValue({
175+
reserved: false,
176+
replay: true,
177+
});
178+
middleware = createApiKeyAuth({
179+
merchantLookup,
180+
usageRecorder,
181+
verifyGatewaySignature,
182+
reserveGatewaySignature,
183+
});
184+
const req = createRequest({
185+
"x-api-key": "signed-api-key",
186+
"x-api-signature": "sha256=" + "a".repeat(64),
187+
"x-api-timestamp": "1713916800",
188+
});
189+
190+
await middleware(req, res, next);
191+
192+
expect(res.status).toHaveBeenCalledWith(401);
193+
expect(res.json).toHaveBeenCalledWith({
194+
error: "Invalid API gateway signature",
195+
code: "API_GATEWAY_REPLAY_DETECTED",
196+
});
197+
expect(merchantLookup).not.toHaveBeenCalled();
198+
expect(next).not.toHaveBeenCalled();
199+
});
200+
149201
it("rejects request when gateway signature verification fails", async () => {
150202
verifyGatewaySignature.mockReturnValue({
151203
valid: false,

0 commit comments

Comments
 (0)