Skip to content

Commit

Permalink
mini-browser, webview: warn if unsecure
Browse files Browse the repository at this point in the history
Add a new `FrontendApplicationConfiguration` field `securityWarnings`
that drives the binding of guards in different modules, as well as
adding/removing preferences. When enabled, these modules will do checks
for known configuration issues that may cause security vulnerabilities.
When disabled, applications will run like they used to, skipping checks.

Check for unsecure host patterns when deploying `mini-browser` and
`webview` content. `{{hostname}}` is known to cause vulnerabilities in
applications, so we currently check for those by default.

New preferences:

`mini-browser.previewFile.preventUnsecure: 'ask' | 'alwaysOpen' |
'alwaysPrevent'`

Theia will prompt the user before loading the local content into the
preview iframe. You can either open, prevent, always open, or always
prevent.

`mini-browser.warnIfUnsecure: boolean`

Theia will prompt a warning upon starting the frontend if the configured
host pattern is unsecure.

`webview.warnIfUnsecure: boolean`

Theia will prompt a warning upon starting the frontend if the configured
host pattern is unsecure.
  • Loading branch information
paul-marechal committed Jun 8, 2021
1 parent 2880525 commit 9635952
Show file tree
Hide file tree
Showing 12 changed files with 354 additions and 22 deletions.
10 changes: 9 additions & 1 deletion dev-packages/application-package/src/application-props.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,8 @@ export namespace ApplicationProps {
config: {
applicationName: 'Eclipse Theia',
defaultTheme: 'dark',
defaultIconTheme: 'none'
defaultIconTheme: 'none',
securityWarnings: true,
}
},
generator: {
Expand Down Expand Up @@ -122,6 +123,13 @@ export interface FrontendApplicationConfig extends ApplicationConfig {
*/
readonly applicationName: string;

/**
* Control if security checks will be bound and executed in your application.
*
* Defaults to `true`.
*/
readonly securityWarnings?: boolean

/**
* Electron specific configuration.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,38 +14,52 @@
* SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
********************************************************************************/

import { EnvVariablesServer } from '@theia/core/lib/common/env-variables';
import { Endpoint, FrontendApplicationContribution } from '@theia/core/lib/browser';
import { inject, injectable, postConstruct } from '@theia/core/shared/inversify';
import { MiniBrowserEndpoint } from '../../common/mini-browser-endpoint';
import { EnvVariablesServer } from '@theia/core/lib/common/env-variables';
import { inject, injectable, optional, postConstruct } from '@theia/core/shared/inversify';
import { v4 } from 'uuid';
import { MiniBrowserEndpoint } from '../../common/mini-browser-endpoint';
import { MiniBrowserGuard } from '../mini-browser-guard';
import { MiniBrowserConfiguration } from '../mini-browser-configuration';

/**
* Fetch values from the backend's environment.
* Fetch values from the backend's environment and caches them locally.
* Helps with deploying various mini-browser endpoints.
*/
@injectable()
export class MiniBrowserEnvironment implements FrontendApplicationContribution {

protected _hostPatternPromise: Promise<string>;
protected _hostPattern: string;

@inject(MiniBrowserGuard) @optional()
protected miniBrowserGuard?: MiniBrowserGuard;

@inject(MiniBrowserConfiguration)
protected miniBrowserConfiguration: MiniBrowserConfiguration;

@inject(EnvVariablesServer)
protected readonly environment: EnvVariablesServer;

@postConstruct()
protected postConstruct(): void {
this._hostPatternPromise = this.environment.getValue(MiniBrowserEndpoint.HOST_PATTERN_ENV)
.then(envVar => envVar?.value || MiniBrowserEndpoint.HOST_PATTERN_DEFAULT);
.then(envVar => envVar?.value || MiniBrowserEndpoint.HOST_PATTERN_DEFAULT)
.then(pattern => this.miniBrowserConfiguration.hostPattern = pattern);
if (this.miniBrowserGuard) {
this._hostPatternPromise.then(
pattern => this.miniBrowserGuard!.onSetHostPattern(pattern)
);
}
}

async onStart(): Promise<void> {
this._hostPattern = await this._hostPatternPromise;
await this._hostPatternPromise;
}

getEndpoint(uuid: string, hostname?: string): Endpoint {
return new Endpoint({
path: MiniBrowserEndpoint.PATH,
host: this._hostPattern
host: this.miniBrowserConfiguration.hostPattern!
.replace('{{uuid}}', uuid)
.replace('{{hostname}}', hostname || this.getDefaultHostname()),
});
Expand Down
13 changes: 10 additions & 3 deletions packages/mini-browser/src/browser/location-mapper-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,13 @@
* SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
********************************************************************************/

import { inject, injectable, named } from '@theia/core/shared/inversify';
import { inject, injectable, named, optional } from '@theia/core/shared/inversify';
import URI from '@theia/core/lib/common/uri';
import { Endpoint } from '@theia/core/lib/browser';
import { MaybePromise, Prioritizeable } from '@theia/core/lib/common/types';
import { ContributionProvider } from '@theia/core/lib/common/contribution-provider';
import { MiniBrowserEnvironment } from './environment/mini-browser-environment';
import { MiniBrowserGuard } from './mini-browser-guard';

/**
* Contribution for the `LocationMapperService`.
Expand Down Expand Up @@ -128,14 +129,17 @@ export class LocationWithoutSchemeMapper implements LocationMapper {
@injectable()
export class FileLocationMapper implements LocationMapper {

@inject(MiniBrowserGuard) @optional()
protected miniBrowserGuard?: MiniBrowserGuard;

@inject(MiniBrowserEnvironment)
protected readonly miniBrowserEnvironment: MiniBrowserEnvironment;
protected miniBrowserEnvironment: MiniBrowserEnvironment;

canHandle(location: string): MaybePromise<number> {
return location.startsWith('file://') ? 1 : 0;
}

map(location: string): MaybePromise<string> {
async map(location: string): Promise<string> {
const uri = new URI(location);
if (uri.scheme !== 'file') {
throw new Error(`Only URIs with 'file' scheme can be mapped to an URL. URI was: ${uri}.`);
Expand All @@ -144,6 +148,9 @@ export class FileLocationMapper implements LocationMapper {
if (rawLocation.charAt(0) === '/') {
rawLocation = rawLocation.substr(1);
}
if (this.miniBrowserGuard) {
await this.miniBrowserGuard.onFileLocationMap(rawLocation);
}
return this.miniBrowserEnvironment.getRandomEndpoint().getRestUrl().resolve(rawLocation).toString();
}

Expand Down
23 changes: 23 additions & 0 deletions packages/mini-browser/src/browser/mini-browser-configuration.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
/********************************************************************************
* Copyright (C) 2021 Ericsson and others.
*
* This program and the accompanying materials are made available under the
* terms of the Eclipse Public License v. 2.0 which is available at
* http://www.eclipse.org/legal/epl-2.0.
*
* This Source Code may also be made available under the following Secondary
* Licenses when the conditions for such availability set forth in the Eclipse
* Public License v. 2.0 are satisfied: GNU General Public License, version 2
* with the GNU Classpath Exception which is available at
* https://www.gnu.org/software/classpath/license.html.
*
* SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
********************************************************************************/

export const MiniBrowserConfiguration = Symbol('MiniBrowserConfiguration');
export interface MiniBrowserConfiguration {
/**
* The host pattern used to serve mini-browser content.
*/
hostPattern?: string
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ import '../../src/browser/style/index.css';

import { ContainerModule } from '@theia/core/shared/inversify';
import URI from '@theia/core/lib/common/uri';
import { FrontendApplicationConfigProvider } from '@theia/core/lib/browser/frontend-application-config-provider';
import { createPreferenceProxy, PreferenceService, PreferenceContribution } from '@theia/core/lib/browser';
import { OpenHandler } from '@theia/core/lib/browser/opener-service';
import { WidgetFactory } from '@theia/core/lib/browser/widget-manager';
import { bindContributionProvider } from '@theia/core/lib/common/contribution-provider';
Expand All @@ -39,8 +41,22 @@ import {
LocationMapper,
LocationWithoutSchemeMapper,
} from './location-mapper-service';
import { MiniBrowserPreferences, MiniBrowserPreferencesSchema } from './mini-browser-preferences';
import { MiniBrowserGuard } from './mini-browser-guard';
import { MiniBrowserConfiguration } from './mini-browser-configuration';

const frontendConfig = FrontendApplicationConfigProvider.get();

export default new ContainerModule(bind => {
bind<MiniBrowserConfiguration>(MiniBrowserConfiguration).toConstantValue({});
bind(PreferenceContribution).toConstantValue({ schema: MiniBrowserPreferencesSchema });
bind(MiniBrowserPreferences).toDynamicValue(
ctx => createPreferenceProxy(ctx.container.get(PreferenceService), MiniBrowserPreferencesSchema)
).inSingletonScope();

if (frontendConfig.securityWarnings) {
bind(MiniBrowserGuard).toSelf().inSingletonScope();
}

bind(MiniBrowserContent).toSelf();
bind(MiniBrowserContentFactory).toFactory(context => (props: MiniBrowserProps) => {
Expand Down Expand Up @@ -77,5 +93,7 @@ export default new ContainerModule(bind => {
bind(LocationMapper).toService(LocationWithoutSchemeMapper);
bind(LocationMapperService).toSelf().inSingletonScope();

bind(MiniBrowserService).toDynamicValue(context => WebSocketConnectionProvider.createProxy(context.container, MiniBrowserServicePath)).inSingletonScope();
bind(MiniBrowserService).toDynamicValue(
ctx => WebSocketConnectionProvider.createProxy(ctx.container, MiniBrowserServicePath)
).inSingletonScope();
});
115 changes: 115 additions & 0 deletions packages/mini-browser/src/browser/mini-browser-guard.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
/********************************************************************************
* Copyright (C) 2021 Ericsson and others.
*
* This program and the accompanying materials are made available under the
* terms of the Eclipse Public License v. 2.0 which is available at
* http://www.eclipse.org/legal/epl-2.0.
*
* This Source Code may also be made available under the following Secondary
* Licenses when the conditions for such availability set forth in the Eclipse
* Public License v. 2.0 are satisfied: GNU General Public License, version 2
* with the GNU Classpath Exception which is available at
* https://www.gnu.org/software/classpath/license.html.
*
* SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
********************************************************************************/

import { MessageService } from '@theia/core';
import { PreferenceService, PreferenceScope } from '@theia/core/lib/browser';
import { inject, injectable } from '@theia/core/shared/inversify';
import { MiniBrowserPreferences, IMiniBrowserPreferences } from './mini-browser-preferences';
import { MiniBrowserConfiguration } from './mini-browser-configuration';

/**
* Checks for known security issues with the mini-browser.
* Can be controlled through preferences.
*/
@injectable()
export class MiniBrowserGuard {

@inject(MessageService)
protected messageService: MessageService;

@inject(PreferenceService)
protected preferenceService: PreferenceService;

@inject(MiniBrowserConfiguration)
protected miniBrowserConfiguration: MiniBrowserConfiguration;

@inject(MiniBrowserPreferences)
protected miniBrowserPreferences: MiniBrowserPreferences;

async onSetHostPattern(hostPattern: string): Promise<void> {
await this.miniBrowserPreferences.ready;
if (this.miniBrowserPreferences['mini-browser.warnIfUnsecure']) {
if (this.isHostPatternUnsecure(hostPattern)) {
this.messageService.warn(
'`mini-browser` is currently configured to serve `file:` resources on the same origin as the application, this is known to be unsecure. ' +
`Current pattern: \`${hostPattern}\``,
{ timeout: 5000 },
/* actions: */ 'Ok', 'Don\'t show again',
).then(action => {
if (action === 'Don\'t show again') {
this.setMiniBrowserPreference('mini-browser.warnIfUnsecure', false);
}
});
}
}
}

/**
* Will throw if the location should not be opened, according to the current configurations.
*/
async onFileLocationMap(location: string): Promise<void> {
await this.miniBrowserPreferences.ready;
if (this.isHostPatternUnsecure(this.miniBrowserConfiguration.hostPattern!)) {
if (this.miniBrowserPreferences['mini-browser.previewFile.preventUnsecure'] === 'alwaysPrevent') {
throw this.preventOpeningLocation(location);
}
if (this.miniBrowserPreferences['mini-browser.previewFile.preventUnsecure'] === 'ask') {
await this.askOpenFileUnsecurely(location);
}
}
}

protected isHostPatternUnsecure(hostPattern: string): boolean {
return hostPattern === '{{hostname}}';
}

protected async askOpenFileUnsecurely(location: string): Promise<void> {
const action = await this.messageService.warn(
'You are about to open a local file with the same origin as this application, this unsecure and the displayed document might access this application services. ' +
`File: \`${location}\``,
/* actions: */ 'Open', 'Always Open', 'Prevent', 'Always Prevent'
);
switch (action) {
case 'Always Prevent':
this.setMiniBrowserPreference('mini-browser.previewFile.preventUnsecure', 'alwaysPrevent');
case 'Prevent':
case undefined:
throw this.preventOpeningLocation(location);
case 'Always Open':
this.setMiniBrowserPreference('mini-browser.previewFile.preventUnsecure', 'alwaysPrevent');
case 'Open':
return;
}
}

protected preventOpeningLocation(location: string): Error {
const message = `Prevented opening ${location}.`;
this.messageService.warn(
`${message} See the \`mini-browser.previewFile.preventUnsecure\` preference to control this behavior.`,
{ timeout: 10_000 },
/* actions: */ 'Ok'
);
return new Error(message);
}

protected setMiniBrowserPreference<K extends keyof IMiniBrowserPreferences>(
preference: K,
value: IMiniBrowserPreferences[K],
scope: PreferenceScope = PreferenceScope.User
): void {
this.preferenceService.set(preference, value, scope);
}
}
51 changes: 51 additions & 0 deletions packages/mini-browser/src/browser/mini-browser-preferences.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
/********************************************************************************
* Copyright (C) 2021 Ericsson and others.
*
* This program and the accompanying materials are made available under the
* terms of the Eclipse Public License v. 2.0 which is available at
* http://www.eclipse.org/legal/epl-2.0.
*
* This Source Code may also be made available under the following Secondary
* Licenses when the conditions for such availability set forth in the Eclipse
* Public License v. 2.0 are satisfied: GNU General Public License, version 2
* with the GNU Classpath Exception which is available at
* https://www.gnu.org/software/classpath/license.html.
*
* SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
********************************************************************************/

import { PreferenceSchema, PreferenceProxy } from '@theia/core/lib/browser';
import { FrontendApplicationConfigProvider } from '@theia/core/lib/browser/frontend-application-config-provider';

const frontendConfig = FrontendApplicationConfigProvider.get();

export const MiniBrowserPreferencesSchema: PreferenceSchema = {
properties: {}
};

if (frontendConfig.securityWarnings) {
MiniBrowserPreferencesSchema.properties['mini-browser.previewFile.preventUnsecure'] = {
scope: 'application',
description: 'What to do when you open a resource with the mini-browser in an unsecure manner.',
enum: [
'ask',
'alwaysOpen',
'alwaysPrevent',
],
default: 'ask'
};
MiniBrowserPreferencesSchema.properties['mini-browser.warnIfUnsecure'] = {
scope: 'application',
type: 'boolean',
description: 'Warns users that the mini-browser is currently deployed unsecurely.',
default: true,
};
}

export interface IMiniBrowserPreferences {
'mini-browser.previewFile.preventUnsecure'?: 'ask' | 'alwaysOpen' | 'alwaysPrevent'
'mini-browser.warnIfUnsecure'?: boolean
}

export const MiniBrowserPreferences = Symbol('GitPreferences');
export type MiniBrowserPreferences = PreferenceProxy<IMiniBrowserPreferences>;
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ export class ElectronMiniBrowserEnvironment extends MiniBrowserEnvironment {

protected getDefaultHostname(): string {
const query = self.location.search
.substr(1)
.substr(1) // remove leading `?`
.split('&')
.map(entry => entry
.split('=', 2)
Expand Down
Loading

0 comments on commit 9635952

Please sign in to comment.