Skip to content

Commit

Permalink
unc - adopt setting and handling of allow list (#5)
Browse files Browse the repository at this point in the history
* unc - adopt setting and handling of allow list

* unc - set allow list on server too

* unc - pick our patched node.js for now

* bump electron

* unc - ignore sync is not needed with machine scope

* unc - use process set directly

* 🆙 22.5.1
  • Loading branch information
bpasero authored May 4, 2023
1 parent 0591d3e commit 6a995c4
Show file tree
Hide file tree
Showing 17 changed files with 203 additions and 22 deletions.
2 changes: 1 addition & 1 deletion build/gulpfile.reh.js
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ function nodejs(platform, arch) {

if (platform === 'win32') {
if (product.nodejsRepository) {
return assetFromGithub(product.nodejsRepository, nodeVersion, name => name === `win-${arch}-node.exe`)
return assetFromGithub(product.nodejsRepository, nodeVersion, name => name === `win-${arch}-patched-node.exe`)
.pipe(rename('node.exe'));
}

Expand Down
12 changes: 6 additions & 6 deletions build/lib/electron.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions build/lib/electron.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ function darwinBundleDocumentTypes(types: { [name: string]: string | string[] },
}

export const config = {
version: product.electronRepository ? '22.4.8' : util.getElectronVersion(),
version: product.electronRepository ? '22.5.1' : util.getElectronVersion(),
productAppName: product.nameLong,
companyName: 'Microsoft Corporation',
copyright: 'Copyright (C) 2023 Microsoft. All rights reserved',
Expand Down Expand Up @@ -212,7 +212,7 @@ function getElectron(arch: string): () => NodeJS.ReadWriteStream {
}

async function main(arch = process.arch): Promise<void> {
const version = product.electronRepository ? '22.4.8' : util.getElectronVersion();
const version = product.electronRepository ? '22.5.1' : util.getElectronVersion();
const electronPath = path.join(root, '.build', 'electron');
const versionFile = path.join(electronPath, 'version');
const isUpToDate = fs.existsSync(versionFile) && fs.readFileSync(versionFile, 'utf8') === `${version}`;
Expand Down
5 changes: 5 additions & 0 deletions src/vs/base/common/errorMessage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,11 @@ function stackToString(stack: string[] | string | undefined): string | undefined

function detectSystemErrorMessage(exception: any): string {

// Custom node.js error from us
if (exception.code === 'ERR_UNC_HOST_NOT_ALLOWED') {
return `${exception.message}. Please update the 'security.allowedUNCHosts' setting if you want to allow this host.`;
}

// See https://nodejs.org/api/errors.html#errors_class_system_error
if (typeof exception.code === 'string' && typeof exception.errno === 'number' && typeof exception.syscall === 'string') {
return nls.localize('nodeExceptionMessage', "A system error occurred ({0})", exception.message);
Expand Down
65 changes: 65 additions & 0 deletions src/vs/base/node/unc.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
/*---------------------------------------------------------------------------------------------
* Copyright (c) Microsoft Corporation. All rights reserved.
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/

import { isWindows } from 'vs/base/common/platform';

function processUNCHostAllowlist(): Set<string> | undefined {

// The property `process.uncHostAllowlist` is not available in official node.js
// releases, only in our own builds, so we have to probe for availability

const processWithUNCHostAllowlist = process as typeof process & { readonly uncHostAllowlist?: Set<string> };

return processWithUNCHostAllowlist.uncHostAllowlist;
}

export function setUNCHostAllowlist(allowedHosts: string[]): void {
if (!isWindows) {
return;
}

const allowlist = processUNCHostAllowlist();
if (allowlist) {
allowlist.clear();

for (const allowedHost of allowedHosts) {
allowlist.add(allowedHost);
}
}
}

export function getUNCHostAllowlist(): string[] {
const allowlist = processUNCHostAllowlist();
if (allowlist) {
return Array.from(allowlist);
}

return [];
}

export function addUNCHostToAllowlist(allowedHost: string): void {
if (!isWindows) {
return;
}

const allowlist = processUNCHostAllowlist();
if (allowlist) {
allowlist.add(allowedHost);
}
}

export function toUNCHostAllowlist(arg0: unknown): string[] {
const allowedUNCHosts = new Set<string>();

if (Array.isArray(arg0)) {
for (const host of arg0) {
if (typeof host === 'string') {
allowedUNCHosts.add(host);
}
}
}

return Array.from(allowedUNCHosts);
}
9 changes: 9 additions & 0 deletions src/vs/code/electron-main/app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
*--------------------------------------------------------------------------------------------*/

import { app, BrowserWindow, dialog, protocol, session, Session, systemPreferences, WebFrameMain } from 'electron';
import { setUNCHostAllowlist, toUNCHostAllowlist } from 'vs/base/node/unc';
import { validatedIpcMain } from 'vs/base/parts/ipc/electron-main/ipcMain';
import { hostname, release } from 'os';
import { VSBuffer } from 'vs/base/common/buffer';
Expand Down Expand Up @@ -311,6 +312,14 @@ export class CodeApplication extends Disposable {
}

//#endregion

//#region UNC Host Allowlist (Windows)

if (isWindows) {
setUNCHostAllowlist(toUNCHostAllowlist(this.configurationService.getValue<unknown>('security.allowedUNCHosts')));
}

//#endregion
}

private registerListeners(): void {
Expand Down
14 changes: 13 additions & 1 deletion src/vs/code/node/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,12 @@ import { getStdinFilePath, hasStdinWithoutTty, readFromStdin, stdinDataListener
import { createWaitMarkerFileSync } from 'vs/platform/environment/node/wait';
import product from 'vs/platform/product/common/product';
import { CancellationTokenSource } from 'vs/base/common/cancellation';
import { randomPath } from 'vs/base/common/extpath';
import { isUNC, randomPath } from 'vs/base/common/extpath';
import { Utils } from 'vs/platform/profiling/common/profiling';
import { FileAccess } from 'vs/base/common/network';
import { cwd } from 'vs/base/common/process';
import { addUNCHostToAllowlist } from 'vs/base/node/unc';
import { URI } from 'vs/base/common/uri';

function shouldSpawnCliProcess(argv: NativeParsedArgs): boolean {
return !!argv['install-source']
Expand Down Expand Up @@ -116,6 +118,16 @@ export async function main(argv: string[]): Promise<any> {
const source = args._[0];
const target = args._[1];

// Windows: set the paths as allowed UNC paths given
// they are explicitly provided by the user as arguments
if (isWindows) {
for (const path of [source, target]) {
if (isUNC(path)) {
addUNCHostToAllowlist(URI.file(path).authority);
}
}
}

// Validate
if (
!source || !target || source === target || // make sure source and target are provided and are not the same
Expand Down
7 changes: 6 additions & 1 deletion src/vs/platform/files/node/diskFileSystemProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -707,6 +707,7 @@ export class DiskFileSystemProvider extends AbstractDiskFileSystemProvider imple
return error; // avoid double conversion
}

let resultError: Error | string = error;
let code: FileSystemProviderErrorCode;
switch (error.code) {
case 'ENOENT':
Expand All @@ -725,11 +726,15 @@ export class DiskFileSystemProvider extends AbstractDiskFileSystemProvider imple
case 'EACCES':
code = FileSystemProviderErrorCode.NoPermissions;
break;
case 'ERR_UNC_HOST_NOT_ALLOWED':
resultError = `${error.message}. Please update the 'security.allowedUNCHosts' setting if you want to allow this host.`;
code = FileSystemProviderErrorCode.Unknown;
break;
default:
code = FileSystemProviderErrorCode.Unknown;
}

return createFileSystemProviderError(error, code);
return createFileSystemProviderError(resultError, code);
}

private async toFileSystemProviderWriteError(resource: URI | undefined, error: NodeJS.ErrnoException): Promise<FileSystemProviderError> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ import { ITelemetryService } from 'vs/platform/telemetry/common/telemetry';
import { ILifecycleMainService } from 'vs/platform/lifecycle/electron-main/lifecycleMainService';
import { removeDangerousEnvVariables } from 'vs/base/common/processes';
import { deepClone } from 'vs/base/common/objects';
import { isWindows } from 'vs/base/common/platform';
import { getUNCHostAllowlist } from 'vs/base/node/unc';

export interface IUtilityProcessConfiguration {

Expand Down Expand Up @@ -259,6 +261,9 @@ export class UtilityProcess extends Disposable {
env['VSCODE_CRASH_REPORTER_SANDBOXED_HINT'] = '1'; // TODO@bpasero remove me once sandbox is final
}
env['VSCODE_CRASH_REPORTER_PROCESS_TYPE'] = configuration.type;
if (isWindows) {
env['NODE_UNC_HOST_ALLOWLIST'] = getUNCHostAllowlist().join('\\');
}

// Remove any environment variables that are not allowed
removeDangerousEnvVariables(env);
Expand Down
37 changes: 35 additions & 2 deletions src/vs/platform/windows/electron-main/windowsMainService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,9 @@
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/

import { app, BrowserWindow, WebContents } from 'electron';
import { app, BrowserWindow, WebContents, shell } from 'electron';
import { Promises } from 'vs/base/node/pfs';
import { addUNCHostToAllowlist } from 'vs/base/node/unc';
import { hostname, release } from 'os';
import { coalesce, distinct, firstOrDefault } from 'vs/base/common/arrays';
import { CancellationToken } from 'vs/base/common/cancellation';
Expand Down Expand Up @@ -1013,7 +1014,7 @@ export class WindowsMainService extends Disposable implements IWindowsMainServic
return openable.fileUri;
}

private async doResolveFilePath(path: string, options: IPathResolveOptions): Promise<IPathToOpen<ITextEditorOptions> | undefined> {
private async doResolveFilePath(path: string, options: IPathResolveOptions, skipHandleUNCError?: boolean): Promise<IPathToOpen<ITextEditorOptions> | undefined> {

// Extract line/col information from path
let lineNumber: number | undefined;
Expand Down Expand Up @@ -1083,6 +1084,11 @@ export class WindowsMainService extends Disposable implements IWindowsMainServic
};
}
} catch (error) {

if (error.code === 'ERR_UNC_HOST_NOT_ALLOWED' && !skipHandleUNCError) {
return this.onUNCHostNotAllowed(path, options);
}

const fileUri = URI.file(path);

// since file does not seem to exist anymore, remove from recent
Expand All @@ -1101,6 +1107,33 @@ export class WindowsMainService extends Disposable implements IWindowsMainServic
return undefined;
}

private async onUNCHostNotAllowed(path: string, options: IPathResolveOptions): Promise<IPathToOpen<ITextEditorOptions> | undefined> {
const uri = URI.file(path);

const { response } = await this.dialogMainService.showMessageBox({
type: 'warning',
buttons: [
localize({ key: 'yes', comment: ['&& denotes a mnemonic'] }, "&&Yes"),
localize({ key: 'no', comment: ['&& denotes a mnemonic'] }, "&&No"),
localize({ key: 'learnMore', comment: ['&& denotes a mnemonic'] }, "&&Learn More"),
],
message: localize('confirmOpenMessage', "The host '{0}' was not found in the list of allowed hosts. Do you want to open it anyway?", uri.authority),
detail: localize('confirmOpenDetail', "The path '{0}' uses a host that is not allowed. Unless you trust the host, you should press 'No'", getPathLabel(uri, { os: OS, tildify: this.environmentMainService }))
});

if (response === 0) {
addUNCHostToAllowlist(uri.authority);

return this.doResolveFilePath(path, options, true /* do not handle UNC error again */);
}

if (response === 2) {
shell.openExternal('https://aka.ms/vscode-windows-unc');
}

return undefined;
}

private doResolveRemotePath(path: string, options: IPathResolveOptions): IPathToOpen<ITextEditorOptions> | undefined {
const first = path.charCodeAt(0);
const remoteAuthority = options.remoteAuthority;
Expand Down
8 changes: 8 additions & 0 deletions src/vs/server/node/remoteExtensionHostAgentCli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ import { ExtensionsProfileScannerService } from 'vs/platform/extensionManagement
import { LogService } from 'vs/platform/log/common/logService';
import { LoggerService } from 'vs/platform/log/node/loggerService';
import { localize } from 'vs/nls';
import { setUNCHostAllowlist, toUNCHostAllowlist } from 'vs/base/node/unc';

class CliMain extends Disposable {

Expand All @@ -66,7 +67,14 @@ class CliMain extends Disposable {
async run(): Promise<void> {
const instantiationService = await this.initServices();
await instantiationService.invokeFunction(async accessor => {
const configurationService = accessor.get(IConfigurationService);
const logService = accessor.get(ILogService);

// On Windows, configure the UNC allow list based on settings
if (process.platform === 'win32') {
setUNCHostAllowlist(toUNCHostAllowlist(configurationService.getValue<unknown>('security.allowedUNCHosts')));
}

try {
await this.doRun(instantiationService.createInstance(ExtensionManagementCLI, new ConsoleLogger(logService.getLevel(), false)));
} catch (error) {
Expand Down
11 changes: 11 additions & 0 deletions src/vs/server/node/remoteExtensionHostAgentServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,10 @@ import { createRegExp, escapeRegExpCharacters } from 'vs/base/common/strings';
import { URI } from 'vs/base/common/uri';
import { generateUuid } from 'vs/base/common/uuid';
import { findFreePort } from 'vs/base/node/ports';
import { setUNCHostAllowlist, toUNCHostAllowlist } from 'vs/base/node/unc';
import { PersistentProtocol } from 'vs/base/parts/ipc/common/ipc.net';
import { NodeSocket, WebSocketNodeSocket } from 'vs/base/parts/ipc/node/ipc.net';
import { IConfigurationService } from 'vs/platform/configuration/common/configuration';
import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation';
import { ILogService } from 'vs/platform/log/common/log';
import { IProductService } from 'vs/platform/product/common/productService';
Expand Down Expand Up @@ -712,6 +714,15 @@ export async function createServer(address: string | net.AddressInfo | null, arg
initUnexpectedErrorHandler((error: any) => logService.error(error));
});

// On Windows, configure the UNC allow list based on settings
instantiationService.invokeFunction((accessor) => {
const configurationService = accessor.get(IConfigurationService);

if (process.platform === 'win32') {
setUNCHostAllowlist(toUNCHostAllowlist(configurationService.getValue<unknown>('security.allowedUNCHosts')));
}
});

//
// On Windows, exit early with warning message to users about potential security issue
// if there is node_modules folder under home drive or Users folder.
Expand Down
19 changes: 18 additions & 1 deletion src/vs/workbench/browser/workbench.contribution.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { Registry } from 'vs/platform/registry/common/platform';
import { localize } from 'vs/nls';
import { IConfigurationRegistry, Extensions as ConfigurationExtensions, ConfigurationScope } from 'vs/platform/configuration/common/configurationRegistry';
import { isMacintosh, isWindows, isLinux, isWeb, isNative } from 'vs/base/common/platform';
import { ConfigurationMigrationWorkbenchContribution, workbenchConfigurationNodeBase } from 'vs/workbench/common/configuration';
import { ConfigurationMigrationWorkbenchContribution, securityConfigurationNodeBase, workbenchConfigurationNodeBase } from 'vs/workbench/common/configuration';
import { isStandalone } from 'vs/base/browser/browser';
import { IWorkbenchContributionsRegistry, Extensions as WorkbenchExtensions } from 'vs/workbench/common/contributions';
import { LifecyclePhase } from 'vs/workbench/services/lifecycle/common/lifecycle';
Expand Down Expand Up @@ -680,4 +680,21 @@ const registry = Registry.as<IConfigurationRegistry>(ConfigurationExtensions.Con
}
}
});

// Security
registry.registerConfiguration({
...securityConfigurationNodeBase,
'properties': {
'security.allowedUNCHosts': {
'type': 'array',
'items': {
'type': 'string'
},
'default': [],
'markdownDescription': localize('security.allowedUNCHosts', 'A set of UNC host names to allow without user confirmation. If a UNC host is being accessed that is not allowed via this setting or has not been acknowledged via user confirmation, an error will occur and the operation stopped. A restart is required when changing this setting. Find out more about this setting at https://aka.ms/vscode-windows-unc.'),
'included': isWindows,
'scope': ConfigurationScope.MACHINE
}
}
});
})();
Loading

0 comments on commit 6a995c4

Please sign in to comment.