Skip to content

Commit

Permalink
core: fix origin checks problems
Browse files Browse the repository at this point in the history
Both `mini-browser` and `plugin-ext` contributes WebSocket checks refuse
WebSocket connections originating from within their served content, but
it prevented all connections when serving from the `{{hostname}}`
pattern.

This commit handles the case when such a pattern is used and disables
the origin check in such a case.

This is not totally fool-proof as some host/pattern combinations can
still lead to problems.

e.g. host is `sweet-potato.com` and the pattern is
`{{uuid}}-{{hostname}}`. The generated regex will be `^.+-.+$` which
still matches `sweet-potato.com`.

You should always include a unique string inside the pattern to avoid
such situations, like `{{uuid}}-webview-{{hostname}}`.

This commit also moves the `mini-browser` file serving to
`/mini-browser` to avoid conflicts when hosting on the same root origin.

Signed-off-by: Paul <[email protected]>
  • Loading branch information
paul-marechal committed Dec 15, 2020
1 parent ae35f9c commit 4ebb910
Show file tree
Hide file tree
Showing 6 changed files with 32 additions and 12 deletions.
9 changes: 6 additions & 3 deletions .vscode/launch.json
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,8 @@
],
"env": {
"NODE_ENV": "development",
"THEIA_WEBVIEW_EXTERNAL_ENDPOINT": "${env:THEIA_WEBVIEW_EXTERNAL_ENDPOINT}"
"THEIA_WEBVIEW_EXTERNAL_ENDPOINT": "${env:THEIA_WEBVIEW_EXTERNAL_ENDPOINT}",
"THEIA_MINI_BROWSER_HOST_PATTERN": "${env:THEIA_MINI_BROWSER_HOST_PATTERN}"
},
"sourceMaps": true,
"outFiles": [
Expand Down Expand Up @@ -110,7 +111,8 @@
],
"env": {
"NODE_ENV": "development",
"THEIA_WEBVIEW_EXTERNAL_ENDPOINT": "${env:THEIA_WEBVIEW_EXTERNAL_ENDPOINT}"
"THEIA_WEBVIEW_EXTERNAL_ENDPOINT": "${env:THEIA_WEBVIEW_EXTERNAL_ENDPOINT}",
"THEIA_MINI_BROWSER_HOST_PATTERN": "${env:THEIA_MINI_BROWSER_HOST_PATTERN}"
},
"sourceMaps": true,
"outFiles": [
Expand Down Expand Up @@ -173,7 +175,8 @@
],
"env": {
"THEIA_DEFAULT_PLUGINS": "local-dir:${workspaceFolder}/plugins",
"THEIA_WEBVIEW_EXTERNAL_ENDPOINT": "${env:THEIA_WEBVIEW_EXTERNAL_ENDPOINT}"
"THEIA_WEBVIEW_EXTERNAL_ENDPOINT": "${env:THEIA_WEBVIEW_EXTERNAL_ENDPOINT}",
"THEIA_MINI_BROWSER_HOST_PATTERN": "${env:THEIA_MINI_BROWSER_HOST_PATTERN}"
},
"stopOnEntry": false,
"sourceMaps": true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ export class MiniBrowserEnvironment implements FrontendApplicationContribution {

getEndpoint(uuid: string, hostname?: string): Endpoint {
return new Endpoint({
path: MiniBrowserEndpoint.PATH,
host: this._hostPattern
.replace('{{uuid}}', uuid)
.replace('{{hostname}}', hostname || this.getDefaultHostname()),
Expand Down
1 change: 1 addition & 0 deletions packages/mini-browser/src/common/mini-browser-endpoint.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
* will be replace by a random uuid value.
*/
export namespace MiniBrowserEndpoint {
export const PATH = '/mini-browser';
export const HOST_PATTERN_ENV = 'THEIA_MINI_BROWSER_HOST_PATTERN';
export const HOST_PATTERN_DEFAULT = '{{uuid}}.mini-browser.{{hostname}}';
}
2 changes: 1 addition & 1 deletion packages/mini-browser/src/node/mini-browser-endpoint.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ export class MiniBrowserEndpoint implements BackendApplicationContribution, Mini
protected async attachRequestHandler(app: Application): Promise<void> {
const miniBrowserApp = express();
miniBrowserApp.get('*', async (request, response) => this.response(await this.getUri(request), response));
app.use(vhost(await this.getVirtualHostRegExp(), miniBrowserApp));
app.use(MiniBrowserEndpointNS.PATH, vhost(await this.getVirtualHostRegExp(), miniBrowserApp));
}

protected async response(uri: string, response: Response): Promise<Response> {
Expand Down
9 changes: 7 additions & 2 deletions packages/mini-browser/src/node/mini-browser-ws-validator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,18 +28,23 @@ export class MiniBrowserWsRequestValidator implements WsRequestValidatorContribu

protected miniBrowserHostRe: RegExp;

protected serveSameOrigin: boolean = false;

@postConstruct()
protected postConstruct(): void {
const pattern = process.env[MiniBrowserEndpoint.HOST_PATTERN_ENV] || MiniBrowserEndpoint.HOST_PATTERN_DEFAULT;
if (pattern === '{{hostname}}') {
this.serveSameOrigin = true;
}
const vhostRe = pattern
.replace('.', '\\.')
.replace(/\./g, '\\.')
.replace('{{uuid}}', '.+')
.replace('{{hostname}}', '.+');
this.miniBrowserHostRe = new RegExp(vhostRe, 'i');
}

async allowWsUpgrade(request: http.IncomingMessage): Promise<boolean> {
if (request.headers.origin) {
if (request.headers.origin && !this.serveSameOrigin) {
const origin = url.parse(request.headers.origin);
if (origin.host && this.miniBrowserHostRe.test(origin.host)) {
// If the origin comes from the WebViews, refuse:
Expand Down
22 changes: 16 additions & 6 deletions packages/plugin-ext/src/main/node/plugin-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ export class PluginApiContribution implements BackendApplicationContribution, Ws

protected webviewExternalEndpointRegExp: RegExp;

protected serveSameOrigin: boolean = false;

@postConstruct()
protected postConstruct(): void {
const webviewExternalEndpoint = this.webviewExternalEndpoint();
Expand All @@ -45,7 +47,7 @@ export class PluginApiContribution implements BackendApplicationContribution, Ws
}

allowWsUpgrade(request: http.IncomingMessage): MaybePromise<boolean> {
if (request.headers.origin) {
if (request.headers.origin && !this.serveSameOrigin) {
const origin = url.parse(request.headers.origin);
if (origin.host && this.webviewExternalEndpointRegExp.test(origin.host)) {
// If the origin comes from the WebViews, refuse:
Expand All @@ -55,17 +57,25 @@ export class PluginApiContribution implements BackendApplicationContribution, Ws
return true;
}

/**
* Returns a RegExp pattern matching the expected WebView endpoint's host.
*/
protected webviewExternalEndpoint(): string {
protected webviewExternalEndpointPattern(): string {
let endpointPattern;
if (environment.electron.is()) {
endpointPattern = WebviewExternalEndpoint.defaultPattern;
} else {
endpointPattern = process.env[WebviewExternalEndpoint.pattern] || WebviewExternalEndpoint.defaultPattern;
}
return `^${endpointPattern
if (endpointPattern === '{{hostname}}') {
this.serveSameOrigin = true;
}
return endpointPattern;
}

/**
* Returns a RegExp pattern matching the expected WebView endpoint's host.
*/
protected webviewExternalEndpoint(): string {
return `^${this.webviewExternalEndpointPattern()
.replace(/\./g, '\\.')
.replace('{{uuid}}', '.+')
.replace('{{hostname}}', '.+')}$`;
}
Expand Down

0 comments on commit 4ebb910

Please sign in to comment.