feat(#284): Reduce timestamp validation window to 60s with replay attack prevention

Security improvements:
- Reduce timestamp tolerance from 5 minutes to 60 seconds
- Add nonce-based replay attack prevention using Redis
- Store signature nonce with 60s TTL matching tolerance window
- Reject replayed messages with same signature

Changes:
- Update SignatureService.TIMESTAMP_TOLERANCE_MS to 60s
- Add Redis client injection to SignatureService
- Make verifyConnectionRequest async for nonce checking
- Create RedisProvider for shared Redis client
- Update ConnectionService to await signature verification
- Add comprehensive test coverage for replay prevention

Part of M7.1 Remediation Sprint P1 security fixes.

Fixes #284

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
This commit is contained in:
2026-02-03 21:43:01 -06:00
parent 61e2bf7063
commit 3bba2f1c33
38 changed files with 888 additions and 23 deletions

View File

@@ -0,0 +1,54 @@
/**
* Redis Provider
*
* Provides Redis/Valkey client instance for the application.
*/
import { Logger } from "@nestjs/common";
import type { Provider } from "@nestjs/common";
import Redis from "ioredis";
/**
* Factory function to create Redis client instance
*/
function createRedisClient(): Redis {
const logger = new Logger("RedisProvider");
const valkeyUrl = process.env.VALKEY_URL ?? "redis://localhost:6379";
logger.log(`Connecting to Valkey at ${valkeyUrl}`);
const client = new Redis(valkeyUrl, {
maxRetriesPerRequest: 3,
retryStrategy: (times) => {
const delay = Math.min(times * 50, 2000);
logger.warn(
`Valkey connection retry attempt ${times.toString()}, waiting ${delay.toString()}ms`
);
return delay;
},
reconnectOnError: (err) => {
logger.error("Valkey connection error:", err.message);
return true;
},
});
client.on("connect", () => {
logger.log("Connected to Valkey");
});
client.on("error", (err) => {
logger.error("Valkey error:", err.message);
});
return client;
}
/**
* Redis Client Provider
*
* Provides a singleton Redis client instance for dependency injection.
*/
export const RedisProvider: Provider = {
provide: "REDIS_CLIENT",
useFactory: createRedisClient,
};

View File

@@ -102,7 +102,7 @@ describe("ConnectionService", () => {
provide: SignatureService,
useValue: {
signMessage: vi.fn().mockResolvedValue("mock-signature"),
verifyConnectionRequest: vi.fn().mockReturnValue({ valid: true }),
verifyConnectionRequest: vi.fn().mockResolvedValue({ valid: true }),
},
},
{
@@ -441,7 +441,7 @@ describe("ConnectionService", () => {
});
it("should create pending connection for valid request", async () => {
vi.spyOn(signatureService, "verifyConnectionRequest").mockReturnValue({ valid: true });
vi.spyOn(signatureService, "verifyConnectionRequest").mockResolvedValue({ valid: true });
vi.spyOn(prismaService.federationConnection, "create").mockResolvedValue(mockConnection);
const result = await service.handleIncomingConnectionRequest(mockWorkspaceId, mockRequest);
@@ -451,7 +451,7 @@ describe("ConnectionService", () => {
});
it("should reject request with invalid signature", async () => {
vi.spyOn(signatureService, "verifyConnectionRequest").mockReturnValue({
vi.spyOn(signatureService, "verifyConnectionRequest").mockResolvedValue({
valid: false,
error: "Invalid signature",
});
@@ -462,7 +462,7 @@ describe("ConnectionService", () => {
});
it("should log incoming connection attempt", async () => {
vi.spyOn(signatureService, "verifyConnectionRequest").mockReturnValue({ valid: true });
vi.spyOn(signatureService, "verifyConnectionRequest").mockResolvedValue({ valid: true });
vi.spyOn(prismaService.federationConnection, "create").mockResolvedValue(mockConnection);
const auditSpy = vi.spyOn(auditService, "logIncomingConnectionAttempt");
@@ -477,7 +477,7 @@ describe("ConnectionService", () => {
});
it("should log connection created on success", async () => {
vi.spyOn(signatureService, "verifyConnectionRequest").mockReturnValue({ valid: true });
vi.spyOn(signatureService, "verifyConnectionRequest").mockResolvedValue({ valid: true });
vi.spyOn(prismaService.federationConnection, "create").mockResolvedValue(mockConnection);
const auditSpy = vi.spyOn(auditService, "logIncomingConnectionCreated");
@@ -492,7 +492,7 @@ describe("ConnectionService", () => {
});
it("should log connection rejected on invalid signature", async () => {
vi.spyOn(signatureService, "verifyConnectionRequest").mockReturnValue({
vi.spyOn(signatureService, "verifyConnectionRequest").mockResolvedValue({
valid: false,
error: "Invalid signature",
});

View File

@@ -286,7 +286,7 @@ export class ConnectionService {
});
// Verify signature
const validation = this.signatureService.verifyConnectionRequest(request);
const validation = await this.signatureService.verifyConnectionRequest(request);
if (!validation.valid) {
const errorMsg: string = validation.error ?? "Unknown error";

View File

@@ -20,6 +20,7 @@ import { OIDCService } from "./oidc.service";
import { CommandService } from "./command.service";
import { FederationAgentService } from "./federation-agent.service";
import { PrismaModule } from "../prisma/prisma.module";
import { RedisProvider } from "../common/providers/redis.provider";
@Module({
imports: [
@@ -52,6 +53,7 @@ import { PrismaModule } from "../prisma/prisma.module";
],
controllers: [FederationController, FederationAuthController],
providers: [
RedisProvider,
FederationService,
CryptoService,
FederationAuditService,

View File

@@ -9,10 +9,12 @@ import { Test, TestingModule } from "@nestjs/testing";
import { SignatureService } from "./signature.service";
import { FederationService } from "./federation.service";
import { generateKeyPairSync } from "crypto";
import type Redis from "ioredis";
describe("SignatureService", () => {
let service: SignatureService;
let mockFederationService: Partial<FederationService>;
let mockRedis: Partial<Redis>;
// Test keypair
const { publicKey, privateKey } = generateKeyPairSync("rsa", {
@@ -37,6 +39,12 @@ describe("SignatureService", () => {
}),
};
mockRedis = {
get: vi.fn().mockResolvedValue(null),
set: vi.fn().mockResolvedValue("OK"),
setex: vi.fn().mockResolvedValue("OK"),
};
const module: TestingModule = await Test.createTestingModule({
providers: [
SignatureService,
@@ -44,6 +52,10 @@ describe("SignatureService", () => {
provide: FederationService,
useValue: mockFederationService,
},
{
provide: "REDIS_CLIENT",
useValue: mockRedis,
},
],
}).compile();
@@ -168,16 +180,16 @@ describe("SignatureService", () => {
expect(result).toBe(true);
});
it("should accept timestamps within 5 minutes", () => {
const fourMinutesAgo = Date.now() - 4 * 60 * 1000;
const result = service.validateTimestamp(fourMinutesAgo);
it("should accept timestamps within 60 seconds", () => {
const fiftySecondsAgo = Date.now() - 50 * 1000;
const result = service.validateTimestamp(fiftySecondsAgo);
expect(result).toBe(true);
});
it("should reject timestamps older than 5 minutes", () => {
const sixMinutesAgo = Date.now() - 6 * 60 * 1000;
const result = service.validateTimestamp(sixMinutesAgo);
it("should reject timestamps older than 60 seconds", () => {
const twoMinutesAgo = Date.now() - 2 * 60 * 1000;
const result = service.validateTimestamp(twoMinutesAgo);
expect(result).toBe(false);
});
@@ -226,7 +238,7 @@ describe("SignatureService", () => {
});
describe("verifyConnectionRequest", () => {
it("should verify a valid connection request", () => {
it("should verify a valid connection request", async () => {
const timestamp = Date.now();
const request = {
instanceId: "instance-123",
@@ -239,13 +251,14 @@ describe("SignatureService", () => {
const signature = service.sign(request, privateKey);
const signedRequest = { ...request, signature };
const result = service.verifyConnectionRequest(signedRequest);
const result = await service.verifyConnectionRequest(signedRequest);
expect(result.valid).toBe(true);
expect(result.error).toBeUndefined();
expect(mockRedis.setex).toHaveBeenCalled();
});
it("should reject request with invalid signature", () => {
it("should reject request with invalid signature", async () => {
const request = {
instanceId: "instance-123",
instanceUrl: "https://test.example.com",
@@ -255,13 +268,13 @@ describe("SignatureService", () => {
signature: "invalid-signature",
};
const result = service.verifyConnectionRequest(request);
const result = await service.verifyConnectionRequest(request);
expect(result.valid).toBe(false);
expect(result.error).toContain("signature");
});
it("should reject request with expired timestamp", () => {
it("should reject request with expired timestamp", async () => {
const expiredTimestamp = Date.now() - 10 * 60 * 1000; // 10 minutes ago
const request = {
instanceId: "instance-123",
@@ -274,10 +287,92 @@ describe("SignatureService", () => {
const signature = service.sign(request, privateKey);
const signedRequest = { ...request, signature };
const result = service.verifyConnectionRequest(signedRequest);
const result = await service.verifyConnectionRequest(signedRequest);
expect(result.valid).toBe(false);
expect(result.error).toContain("timestamp");
});
});
describe("replay attack prevention", () => {
it("should reject replayed message with same signature", async () => {
const timestamp = Date.now();
const request = {
instanceId: "instance-123",
instanceUrl: "https://test.example.com",
publicKey,
capabilities: {},
timestamp,
};
const signature = service.sign(request, privateKey);
const signedRequest = { ...request, signature };
// First request should succeed
const result1 = await service.verifyConnectionRequest(signedRequest);
expect(result1.valid).toBe(true);
// Mock Redis to indicate nonce was already used
mockRedis.get = vi.fn().mockResolvedValue("1");
// Second request with same signature should be rejected
const result2 = await service.verifyConnectionRequest(signedRequest);
expect(result2.valid).toBe(false);
expect(result2.error).toContain("replay");
});
it("should store nonce with 60 second TTL", async () => {
const timestamp = Date.now();
const request = {
instanceId: "instance-123",
instanceUrl: "https://test.example.com",
publicKey,
capabilities: {},
timestamp,
};
const signature = service.sign(request, privateKey);
const signedRequest = { ...request, signature };
await service.verifyConnectionRequest(signedRequest);
expect(mockRedis.setex).toHaveBeenCalledWith(expect.stringContaining("nonce:"), 60, "1");
});
it("should allow different messages with different signatures", async () => {
const timestamp1 = Date.now();
const request1 = {
instanceId: "instance-123",
instanceUrl: "https://test.example.com",
publicKey,
capabilities: {},
timestamp: timestamp1,
};
const signature1 = service.sign(request1, privateKey);
const signedRequest1 = { ...request1, signature: signature1 };
const result1 = await service.verifyConnectionRequest(signedRequest1);
expect(result1.valid).toBe(true);
// Different timestamp creates different signature
const timestamp2 = Date.now() + 1;
const request2 = {
instanceId: "instance-123",
instanceUrl: "https://test.example.com",
publicKey,
capabilities: {},
timestamp: timestamp2,
};
const signature2 = service.sign(request2, privateKey);
const signedRequest2 = { ...request2, signature: signature2 };
// Reset mock to simulate nonce not found
mockRedis.get = vi.fn().mockResolvedValue(null);
const result2 = await service.verifyConnectionRequest(signedRequest2);
expect(result2.valid).toBe(true);
});
});
});

View File

@@ -4,7 +4,7 @@
* Handles message signing and verification for federation protocol.
*/
import { Injectable, Logger } from "@nestjs/common";
import { Injectable, Logger, Inject } from "@nestjs/common";
import { createSign, createVerify } from "crypto";
import { FederationService } from "./federation.service";
import type {
@@ -12,14 +12,19 @@ import type {
SignatureValidationResult,
ConnectionRequest,
} from "./types/connection.types";
import type Redis from "ioredis";
@Injectable()
export class SignatureService {
private readonly logger = new Logger(SignatureService.name);
private readonly TIMESTAMP_TOLERANCE_MS = 5 * 60 * 1000; // 5 minutes
private readonly TIMESTAMP_TOLERANCE_MS = 60 * 1000; // 60 seconds
private readonly CLOCK_SKEW_TOLERANCE_MS = 60 * 1000; // 1 minute for future timestamps
private readonly NONCE_TTL_SECONDS = 60; // Nonce TTL matches tolerance window
constructor(private readonly federationService: FederationService) {}
constructor(
private readonly federationService: FederationService,
@Inject("REDIS_CLIENT") private readonly redis: Redis
) {}
/**
* Sign a message with a private key
@@ -153,7 +158,7 @@ export class SignatureService {
/**
* Verify a connection request signature
*/
verifyConnectionRequest(request: ConnectionRequest): SignatureValidationResult {
async verifyConnectionRequest(request: ConnectionRequest): Promise<SignatureValidationResult> {
// Extract signature and create message for verification
const { signature, ...message } = request;
@@ -165,14 +170,30 @@ export class SignatureService {
};
}
// Check for replay attack (nonce already used)
const nonceKey = `nonce:${signature}`;
const nonceExists = await this.redis.get(nonceKey);
if (nonceExists) {
this.logger.warn("Replay attack detected: signature already used");
return {
valid: false,
error: "Request rejected: potential replay attack detected",
};
}
// Verify signature using the public key from the request
const result = this.verify(message, signature, request.publicKey);
if (!result.valid) {
const errorMsg = result.error ?? "Unknown error";
this.logger.warn(`Connection request signature verification failed: ${errorMsg}`);
return result;
}
// Store nonce to prevent replay attacks
await this.redis.setex(nonceKey, this.NONCE_TTL_SECONDS, "1");
return result;
}