Skip to content

Commit d041485

Browse files
authored
Merge pull request #957 from Dstack-TEE/codex/fix-kms-node-auth-safety
[STACKED on #956] fix(kms): align Node authorization safety
2 parents c88969d + 5fd14a3 commit d041485

8 files changed

Lines changed: 107 additions & 47 deletions

File tree

dstack/kms/auth-eth-bun/index.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -208,6 +208,7 @@ describe('API Compatibility Tests', () => {
208208
}));
209209

210210
expect(response.status).toBe(400);
211+
expect(await response.json()).toEqual({ isAllowed: false, reason: 'invalid authorization request', gatewayAppId: '' });
211212
}
212213
expect(mockReadContract).not.toHaveBeenCalled();
213214
});

dstack/kms/auth-eth-bun/index.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -218,6 +218,7 @@ const publicRpcEndpoint = (value: string): string => {
218218
};
219219

220220
const backendUnavailable = 'authorization backend unavailable';
221+
const invalidRequest = { isAllowed: false, reason: 'invalid authorization request', gatewayAppId: '' };
221222

222223
// health check and info endpoint
223224
app.get('/', async (c) => {
@@ -249,7 +250,9 @@ app.get('/', async (c) => {
249250

250251
// app boot authentication
251252
app.post('/bootAuth/app',
252-
zValidator('json', BootInfoSchema),
253+
zValidator('json', BootInfoSchema, (result, c) => {
254+
if (!result.success) return c.json(invalidRequest, 400);
255+
}),
253256
async (c) => {
254257
try {
255258
const bootInfo = c.req.valid('json');
@@ -268,7 +271,9 @@ app.post('/bootAuth/app',
268271

269272
// KMS boot authentication
270273
app.post('/bootAuth/kms',
271-
zValidator('json', BootInfoSchema),
274+
zValidator('json', BootInfoSchema, (result, c) => {
275+
if (!result.success) return c.json(invalidRequest, 400);
276+
}),
272277
async (c) => {
273278
try {
274279
const bootInfo = c.req.valid('json');

dstack/kms/auth-eth/script/Upgrade.s.sol

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ contract UpgradeKmsToV2 is Script {
5858
vm.startBroadcast();
5959

6060
// Upgrade to a specific contract version
61-
Upgrades.upgradeProxy(kmsProxy, "contracts/test-utils/DstackKmsV2.sol:DstackKmsV2", "");
61+
Upgrades.upgradeProxy(kmsProxy, "DstackKmsV2.sol:DstackKmsV2", "");
6262

6363
vm.stopBroadcast();
6464

@@ -76,7 +76,7 @@ contract UpgradeAppToV2 is Script {
7676
vm.startBroadcast();
7777

7878
// Upgrade to a specific contract version
79-
Upgrades.upgradeProxy(appProxy, "contracts/test-utils/DstackAppV2.sol:DstackAppV2", "");
79+
Upgrades.upgradeProxy(appProxy, "DstackAppV2.sol:DstackAppV2", "");
8080

8181
vm.stopBroadcast();
8282

dstack/kms/auth-eth/src/main.test.ts

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,20 @@ describe('Server', () => {
6060
expect(mockCheckBoot).toHaveBeenCalledWith(mockBootInfo, false);
6161
});
6262

63+
it('should reject oversized and non-hex measurements before backend use', async () => {
64+
jest.clearAllMocks();
65+
for (const mrAggregated of ['0x' + 'ab'.repeat(33), 'not-hex']) {
66+
const response = await app.inject({
67+
method: 'POST',
68+
url: '/bootAuth/app',
69+
payload: { ...mockBootInfo, mrAggregated }
70+
});
71+
expect(response.statusCode).toBe(400);
72+
expect(JSON.parse(response.payload)).toEqual({ isAllowed: false, reason: 'invalid authorization request', gatewayAppId: '' });
73+
}
74+
expect(app.ethereum.checkBoot).not.toHaveBeenCalled();
75+
});
76+
6377
it('should return 400 for invalid boot info', async () => {
6478
const response = await app.inject({
6579
method: 'POST',
@@ -111,7 +125,7 @@ describe('Server', () => {
111125
expect(response.statusCode).toBe(200);
112126
const result = JSON.parse(response.payload);
113127
expect(result.isAllowed).toBe(false);
114-
expect(result.reason).toMatch(/Test backend error/);
128+
expect(result.reason).toBe('authorization backend unavailable');
115129
});
116130
});
117131
});

dstack/kms/auth-eth/src/server.ts

Lines changed: 58 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -19,17 +19,26 @@ export async function build(): Promise<FastifyInstance> {
1919
});
2020

2121
// Register schema for request/response validation
22+
const hex = (bytes: number, description: string) => ({
23+
type: 'string',
24+
pattern: `^(?:0x[0-9a-fA-F]{0,${bytes * 2}}|[0-9a-fA-F]{0,${bytes * 2}})$`,
25+
description,
26+
});
27+
2228
server.addSchema({
2329
$id: 'bootInfo',
2430
type: 'object',
2531
required: ['mrAggregated', 'osImageHash', 'appId', 'composeHash', 'instanceId', 'deviceId'],
2632
properties: {
27-
mrAggregated: { type: 'string', description: 'Aggregated MR measurement' },
28-
osImageHash: { type: 'string', description: 'OS Image hash' },
29-
appId: { type: 'string', description: 'Application ID' },
30-
composeHash: { type: 'string', description: 'Compose hash' },
31-
instanceId: { type: 'string', description: 'Instance ID' },
32-
deviceId: { type: 'string', description: 'Device ID' }
33+
mrAggregated: hex(32, 'Aggregated MR measurement'),
34+
osImageHash: hex(32, 'OS Image hash'),
35+
appId: hex(20, 'Application ID'),
36+
composeHash: hex(32, 'Compose hash'),
37+
instanceId: hex(20, 'Instance ID'),
38+
deviceId: hex(32, 'Device ID'),
39+
tcbStatus: { type: 'string', maxLength: 128, default: '' },
40+
advisoryIds: { type: 'array', maxItems: 128, items: { type: 'string', maxLength: 256 }, default: [] },
41+
mrSystem: { ...hex(32, 'System MR measurement'), default: '' },
3342
}
3443
});
3544

@@ -50,21 +59,45 @@ export async function build(): Promise<FastifyInstance> {
5059
const provider = new ethers.JsonRpcProvider(rpcUrl);
5160
server.decorate('ethereum', new EthereumBackend(provider, kmsContractAddr));
5261

53-
server.get('/', async (request, reply) => {
54-
const batch = await Promise.all([
55-
server.ethereum.getGatewayAppId(),
56-
server.ethereum.getChainId(),
57-
server.ethereum.getAppImplementation(),
58-
]);
59-
return {
60-
status: 'ok',
61-
kmsContractAddr: kmsContractAddr,
62-
ethRpcUrl: rpcUrl,
63-
gatewayAppId: batch[0],
64-
chainId: batch[1],
65-
appAuthImplementation: batch[2], // NOTE: for backward compatibility
66-
appImplementation: batch[2],
67-
};
62+
const publicRpcEndpoint = (value: string): string => {
63+
try {
64+
const endpoint = new URL(value);
65+
return `${endpoint.protocol}//${endpoint.host}`;
66+
} catch {
67+
return 'configured';
68+
}
69+
};
70+
const backendUnavailable = 'authorization backend unavailable';
71+
const invalidRequest = { isAllowed: false, reason: 'invalid authorization request', gatewayAppId: '' };
72+
73+
server.setErrorHandler((error, request, reply) => {
74+
if (typeof error === 'object' && error !== null && 'validation' in error) {
75+
return reply.code(400).send(invalidRequest);
76+
}
77+
request.log.error('authorization request failed');
78+
return reply.code(500).send({ status: 'error', message: backendUnavailable });
79+
});
80+
81+
server.get('/', async (_request, reply) => {
82+
try {
83+
const batch = await Promise.all([
84+
server.ethereum.getGatewayAppId(),
85+
server.ethereum.getChainId(),
86+
server.ethereum.getAppImplementation(),
87+
]);
88+
return {
89+
status: 'ok',
90+
kmsContractAddr: kmsContractAddr,
91+
ethRpcUrl: publicRpcEndpoint(rpcUrl),
92+
gatewayAppId: batch[0],
93+
chainId: batch[1],
94+
appAuthImplementation: batch[2], // NOTE: for backward compatibility
95+
appImplementation: batch[2],
96+
};
97+
} catch {
98+
_request.log.error('authorization backend health check failed');
99+
return reply.code(500).send({ status: 'error', message: backendUnavailable });
100+
}
68101
});
69102

70103
// Define routes
@@ -82,11 +115,11 @@ export async function build(): Promise<FastifyInstance> {
82115
try {
83116
return await server.ethereum.checkBoot(request.body, false);
84117
} catch (error) {
85-
console.error(error);
118+
request.log.error('application authorization backend failed');
86119
reply.code(200).send({
87120
isAllowed: false,
88121
gatewayAppId: '',
89-
reason: `${error instanceof Error ? error.message : String(error)}`
122+
reason: backendUnavailable
90123
});
91124
}
92125
});
@@ -106,12 +139,12 @@ export async function build(): Promise<FastifyInstance> {
106139
return await server.ethereum.checkBoot(request.body, true);
107140
} catch (error) {
108141
if (!(error instanceof Error && "Test backend error" == error.message)) {
109-
console.error(error);
142+
request.log.error('KMS authorization backend failed');
110143
}
111144
reply.code(200).send({
112145
isAllowed: false,
113146
gatewayAppId: '',
114-
reason: `${error instanceof Error ? error.message : String(error)}`
147+
reason: backendUnavailable
115148
});
116149
}
117150
});

dstack/kms/auth-eth/tsconfig.json

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,17 +2,26 @@
22
"compilerOptions": {
33
"target": "ES2020",
44
"module": "commonjs",
5-
"lib": ["es2020"],
5+
"lib": [
6+
"es2020"
7+
],
68
"strict": true,
79
"esModuleInterop": true,
810
"skipLibCheck": true,
911
"forceConsistentCasingInFileNames": true,
1012
"outDir": "./dist",
11-
"rootDir": ".",
13+
"rootDir": "./src",
1214
"resolveJsonModule": true,
13-
"types": ["node", "jest"],
15+
"types": [
16+
"node",
17+
"jest"
18+
],
1419
"baseUrl": "."
1520
},
16-
"include": ["src/**/*"],
17-
"exclude": ["node_modules"]
21+
"include": [
22+
"src/**/*"
23+
],
24+
"exclude": [
25+
"node_modules"
26+
]
1827
}

dstack/kms/dstack-app/compose-dev.yaml

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,17 +8,17 @@ services:
88
build:
99
context: .
1010
dockerfile_inline: |
11-
FROM node:18-alpine@sha256:06f7bbbcec00dd10c21a3a0962609600159601b5004d84aff142977b449168e9
11+
FROM node:20-bookworm-slim@sha256:2cf067cfed83d5ea958367df9f966191a942351a2df77d6f0193e162b5febfc0
1212
WORKDIR /app
1313
14-
RUN apk add --no-cache git
14+
RUN apt-get update && apt-get install -y --no-install-recommends git ca-certificates && rm -rf /var/lib/apt/lists/*
1515
RUN git clone ${GIT_REPOSITORY} && \
1616
cd dstack && \
1717
git checkout ${GIT_REV}
1818
WORKDIR /app/dstack/dstack/kms/auth-eth
19-
RUN npm install
20-
RUN npx tsc --project tsconfig.json
21-
CMD node dist/src/main.js
19+
RUN npm ci
20+
RUN npm run build
21+
CMD node dist/main.js
2222
environment:
2323
- HOST=0.0.0.0
2424
- PORT=8000

dstack/kms/dstack-app/docker-compose.yaml

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ services:
1010
dockerfile_inline: |
1111
FROM rust:1.92.0@sha256:48851a839d6a67370c9dbe0e709bedc138e3e404b161c5233aedcf2b717366e4
1212
WORKDIR /app
13-
RUN apk add --no-cache git build-base openssl-dev protobuf protobuf-dev perl
13+
RUN apt-get update && apt-get install -y --no-install-recommends git ca-certificates && rm -rf /var/lib/apt/lists/* build-base openssl-dev protobuf protobuf-dev perl
1414
RUN git clone https://github.com/a16z/helios && \
1515
cd helios && \
1616
git checkout 5c61864a167c16141a9a12b976c0e9398b332f07
@@ -32,18 +32,16 @@ services:
3232
build:
3333
context: .
3434
dockerfile_inline: |
35-
FROM node:18-alpine@sha256:06f7bbbcec00dd10c21a3a0962609600159601b5004d84aff142977b449168e9
35+
FROM node:20-bookworm-slim@sha256:2cf067cfed83d5ea958367df9f966191a942351a2df77d6f0193e162b5febfc0
3636
WORKDIR /app
3737
38-
RUN apk add --no-cache git
38+
RUN apt-get update && apt-get install -y --no-install-recommends git ca-certificates && rm -rf /var/lib/apt/lists/*
3939
RUN git clone https://github.com/Dstack-TEE/dstack.git && \
4040
cd dstack && \
4141
git checkout 78057c975fe4b9e21f557fb888d72eeecfb21178
4242
WORKDIR /app/dstack/kms/auth-eth
43-
RUN npm install && \
44-
npx hardhat typechain && \
45-
npx tsc --project tsconfig.json
46-
CMD node dist/src/main.js
43+
RUN npm ci && npm run build
44+
CMD node dist/main.js
4745
environment:
4846
- HOST=0.0.0.0
4947
- PORT=8000

0 commit comments

Comments
 (0)