Skip to content

Commit 389562f

Browse files
committed
chore(transport-bridge): rework origins check
1 parent 7324216 commit 389562f

File tree

4 files changed

+135
-39
lines changed

4 files changed

+135
-39
lines changed

packages/node-utils/src/http.ts

+11-27
Original file line numberDiff line numberDiff line change
@@ -342,7 +342,7 @@ export class HttpServer<T extends EventMap> extends TypedEmitter<T & BaseEvents>
342342
};
343343
}
344344

345-
const checkOrigin = ({
345+
export const checkOrigin = ({
346346
request,
347347
allowedOrigin,
348348
pathname,
@@ -362,16 +362,18 @@ const checkOrigin = ({
362362
}
363363

364364
if (origin) {
365-
isOriginAllowed = origins.some(o => {
366-
try {
365+
try {
366+
const checkedHostname = `.${new URL(origin).hostname}`;
367+
isOriginAllowed = origins.some(o =>
367368
// match from the end to allow subdomains
368-
return new URL(origin).hostname.endsWith(new URL(o).hostname);
369-
} catch (error) {
370-
// If parsing URL fails we don't want it to crash but silently logs error.
371-
logger.error(`Failed parsing URL: ${error}`);
372-
}
373-
});
369+
checkedHostname.endsWith('.' + o),
370+
);
371+
} catch (error) {
372+
// If parsing URL fails we don't want it to crash but silently logs error.
373+
logger.error(`Failed parsing URL: ${error}`);
374+
}
374375
}
376+
375377
if (!isOriginAllowed) {
376378
logger.warn(`Origin rejected for ${pathname}`);
377379
logger.warn(`- Received: origin: '${origin}'`);
@@ -439,24 +441,6 @@ const checkReferer = ({
439441
return true;
440442
};
441443

442-
/**
443-
* Built-middleware "allow origin"
444-
*/
445-
export const allowOrigins =
446-
(allowedOrigin: string[]): AnyRequestHandler =>
447-
(request, _response, next, { logger }) => {
448-
if (
449-
checkOrigin({
450-
request,
451-
allowedOrigin,
452-
pathname: request.url,
453-
logger,
454-
})
455-
) {
456-
next(request, _response);
457-
}
458-
};
459-
460444
/**
461445
* Built-middleware "allow referers"
462446
*/

packages/node-utils/src/index.ts

-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ export { ensureDirectoryExists } from './ensureDirectoryExists';
22
export { getFreePort } from './getFreePort';
33
export {
44
HttpServer,
5-
allowOrigins,
65
allowReferers,
76
parseBodyJSON,
87
parseBodyText,

packages/transport-bridge/src/http.ts

+31-11
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,10 @@ import {
88
RequestHandler,
99
RequestWithParams,
1010
Response,
11-
allowOrigins,
1211
parseBodyJSON,
1312
parseBodyText,
1413
} from '@trezor/node-utils';
14+
import { checkOrigin } from '@trezor/node-utils/src/http';
1515
import { AbstractApi } from '@trezor/transport/src/api/abstract';
1616
import { UNEXPECTED_ERROR } from '@trezor/transport/src/errors';
1717
import { Descriptor, PathPublic, Session } from '@trezor/transport/src/types';
@@ -221,16 +221,36 @@ export class TrezordNode {
221221
) {
222222
next(req, res);
223223
} else {
224-
allowOrigins([
225-
'https://sldev.cz',
226-
'https://trezor.io',
227-
'http://localhost',
228-
// When using Tor it will send string "null" as default, and it will not allow calling to localhost.
229-
// To allow it to be sent, you can go to about:config and set the attributes below:
230-
// "network.http.referer.hideOnionSource - false"
231-
// "network.proxy.allow_hijacking_localhost - false"
232-
'http://suite.trezoriovpjcahpzkrewelclulmszwbqpzmzgub37gbcjlvluxtruqad.onion',
233-
])(req, res, next, context);
224+
const isOriginAllowed = checkOrigin({
225+
request: req,
226+
allowedOrigin: [
227+
'sldev.cz',
228+
'trezor.io',
229+
'localhost',
230+
// When using Tor it will send string "null" as default, and it will not allow calling to localhost.
231+
// To allow it to be sent, you can go to about:config and set the attributes below:
232+
// "network.http.referer.hideOnionSource - false"
233+
// "network.proxy.allow_hijacking_localhost - false"
234+
'trezoriovpjcahpzkrewelclulmszwbqpzmzgub37gbcjlvluxtruqad.onion',
235+
],
236+
pathname: req.url,
237+
logger: context.logger,
238+
});
239+
240+
if (isOriginAllowed) {
241+
next(req, res);
242+
} else {
243+
// error handling identic to legacy trezord-go
244+
switch (req.url) {
245+
case '/enumerate':
246+
case '/listen':
247+
res.statusCode = 403;
248+
break;
249+
default:
250+
res.statusCode = 404;
251+
}
252+
res.end();
253+
}
234254
}
235255
},
236256
]);
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
import { controller as TrezorUserEnvLink } from './controller';
2+
3+
describe('headers test', () => {
4+
beforeAll(async () => {
5+
await TrezorUserEnvLink.connect();
6+
await TrezorUserEnvLink.startBridge();
7+
});
8+
9+
afterAll(async () => {
10+
await TrezorUserEnvLink.stopBridge();
11+
TrezorUserEnvLink.disconnect();
12+
});
13+
14+
const fixturesForbidden = [
15+
{
16+
endpoint: 'call',
17+
statusCode: 404,
18+
},
19+
{
20+
endpoint: 'send',
21+
statusCode: 404,
22+
},
23+
{
24+
endpoint: 'receive',
25+
statusCode: 404,
26+
},
27+
{
28+
endpoint: 'enumerate',
29+
statusCode: 403,
30+
},
31+
{
32+
endpoint: 'acquire',
33+
statusCode: 404,
34+
},
35+
{
36+
endpoint: 'release',
37+
statusCode: 404,
38+
},
39+
{
40+
endpoint: 'listen',
41+
statusCode: 403,
42+
},
43+
];
44+
45+
fixturesForbidden.forEach(f => {
46+
test(`origin: https://spoofedtrezor.io and endpoint ${f.endpoint} is forbidden`, async () => {
47+
// invalid
48+
const response = await fetch(`http://localhost:21325/${f.endpoint}`, {
49+
method: 'POST',
50+
headers: {
51+
Origin: 'https://spoofedtrezor.io',
52+
},
53+
});
54+
55+
expect(response.ok).toEqual(false);
56+
expect(response.status).toEqual(f.statusCode);
57+
});
58+
59+
test(`origin: https://zor.io and endpoint ${f.endpoint} is forbidden`, async () => {
60+
// invalid
61+
const response = await fetch(`http://localhost:21325/${f.endpoint}`, {
62+
method: 'POST',
63+
headers: {
64+
Origin: 'https://zor.io',
65+
},
66+
});
67+
68+
expect(response.ok).toEqual(false);
69+
expect(response.status).toEqual(f.statusCode);
70+
});
71+
});
72+
73+
const fixturesAllowed = [
74+
// todo: other endpoint need more unification steps between old and new
75+
{
76+
endpoint: 'enumerate',
77+
statusCode: 200,
78+
},
79+
];
80+
81+
fixturesAllowed.forEach(f => {
82+
test(`endpoint ${f.endpoint} with allowed origin`, async () => {
83+
const response = await fetch(`http://localhost:21325/${f.endpoint}`, {
84+
method: 'POST',
85+
headers: {
86+
Origin: 'https://trezor.io',
87+
},
88+
});
89+
90+
expect(response.status).toEqual(f.statusCode);
91+
});
92+
});
93+
});

0 commit comments

Comments
 (0)