Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extensions: warn not fail on UnknownError during signature verification #169777

Merged
merged 3 commits into from
Dec 26, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,18 @@ import { IProductService } from 'vs/platform/product/common/productService';
import { ITelemetryService } from 'vs/platform/telemetry/common/telemetry';
import { IUserDataProfilesService } from 'vs/platform/userDataProfile/common/userDataProfile';

export const enum ExtensionVerificationStatus {
'Verified' = 'Verified',
'Unverified' = 'Unverified',
'UnknownError' = 'UnknownError',
}

export type InstallExtensionTaskOptions = InstallOptions & InstallVSIXOptions & { readonly profileLocation: URI };
export interface IInstallExtensionTask {
readonly identifier: IExtensionIdentifier;
readonly source: IGalleryExtension | URI;
readonly operation: InstallOperation;
readonly wasVerified?: boolean;
readonly verificationStatus?: ExtensionVerificationStatus;
run(): Promise<{ local: ILocalExtension; metadata: Metadata }>;
waitUntilTaskIsFinished(): Promise<{ local: ILocalExtension; metadata: Metadata }>;
cancel(): void;
Expand Down Expand Up @@ -235,7 +241,7 @@ export abstract class AbstractExtensionManagementService extends Disposable impl
const durationSinceUpdate = isUpdate ? undefined : (new Date().getTime() - task.source.lastUpdated) / 1000;
reportTelemetry(this.telemetryService, isUpdate ? 'extensionGallery:update' : 'extensionGallery:install', {
extensionData: getGalleryExtensionTelemetryData(task.source),
wasVerified: task.wasVerified,
verificationStatus: task.verificationStatus,
duration: new Date().getTime() - startTime,
durationSinceUpdate
});
Expand All @@ -251,7 +257,7 @@ export abstract class AbstractExtensionManagementService extends Disposable impl
if (!URI.isUri(task.source)) {
reportTelemetry(this.telemetryService, task.operation === InstallOperation.Update ? 'extensionGallery:update' : 'extensionGallery:install', {
extensionData: getGalleryExtensionTelemetryData(task.source),
wasVerified: task.wasVerified,
verificationStatus: task.verificationStatus,
duration: new Date().getTime() - startTime,
error
});
Expand Down Expand Up @@ -680,7 +686,7 @@ function toExtensionManagementError(error: Error): ExtensionManagementError {
return e;
}

export function reportTelemetry(telemetryService: ITelemetryService, eventName: string, { extensionData, wasVerified, duration, error, durationSinceUpdate }: { extensionData: any; wasVerified?: boolean; duration?: number; durationSinceUpdate?: number; error?: Error }): void {
export function reportTelemetry(telemetryService: ITelemetryService, eventName: string, { extensionData, verificationStatus, duration, error, durationSinceUpdate }: { extensionData: any; verificationStatus?: ExtensionVerificationStatus; duration?: number; durationSinceUpdate?: number; error?: Error }): void {
let errorcode: ExtensionManagementErrorCode | undefined;
let errorcodeDetail: string | undefined;

Expand All @@ -705,7 +711,7 @@ export function reportTelemetry(telemetryService: ITelemetryService, eventName:
"errorcode": { "classification": "CallstackOrException", "purpose": "PerformanceAndHealth" },
"errorcodeDetail": { "classification": "CallstackOrException", "purpose": "PerformanceAndHealth" },
"recommendationReason": { "retiredFromVersion": "1.23.0", "classification": "SystemMetaData", "purpose": "FeatureInsight", "isMeasurement": true },
"wasVerified" : { "classification": "SystemMetaData", "purpose": "FeatureInsight" },
"verificationStatus" : { "classification": "SystemMetaData", "purpose": "FeatureInsight" },
"${include}": [
"${GalleryExtensionTelemetryData}"
]
Expand All @@ -729,13 +735,13 @@ export function reportTelemetry(telemetryService: ITelemetryService, eventName:
"duration" : { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "isMeasurement": true },
"errorcode": { "classification": "CallstackOrException", "purpose": "PerformanceAndHealth" },
"errorcodeDetail": { "classification": "CallstackOrException", "purpose": "PerformanceAndHealth" },
"wasVerified" : { "classification": "SystemMetaData", "purpose": "FeatureInsight" },
"verificationStatus" : { "classification": "SystemMetaData", "purpose": "FeatureInsight" },
"${include}": [
"${GalleryExtensionTelemetryData}"
]
}
*/
telemetryService.publicLog(eventName, { ...extensionData, wasVerified, success: !error, duration, errorcode, errorcodeDetail, durationSinceUpdate });
telemetryService.publicLog(eventName, { ...extensionData, verificationStatus, success: !error, duration, errorcode, errorcodeDetail, durationSinceUpdate });
}

export abstract class AbstractExtensionTask<T> {
Expand Down
41 changes: 24 additions & 17 deletions src/vs/platform/extensionManagement/node/extensionDownloader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,10 @@ import { generateUuid } from 'vs/base/common/uuid';
import { Promises as FSPromises } from 'vs/base/node/pfs';
import { IConfigurationService } from 'vs/platform/configuration/common/configuration';
import { INativeEnvironmentService } from 'vs/platform/environment/common/environment';
import { ExtensionVerificationStatus } from 'vs/platform/extensionManagement/common/abstractExtensionManagementService';
import { ExtensionManagementError, ExtensionManagementErrorCode, IExtensionGalleryService, IGalleryExtension, InstallOperation } from 'vs/platform/extensionManagement/common/extensionManagement';
import { ExtensionKey, groupByExtension } from 'vs/platform/extensionManagement/common/extensionManagementUtil';
import { ExtensionSignatureVerificationError, IExtensionSignatureVerificationService } from 'vs/platform/extensionManagement/node/extensionSignatureVerificationService';
import { TargetPlatform } from 'vs/platform/extensions/common/extensions';
import { IFileService, IFileStatWithMetadata } from 'vs/platform/files/common/files';
import { ILogService } from 'vs/platform/log/common/log';
import { IProductService } from 'vs/platform/product/common/productService';
Expand All @@ -33,7 +33,6 @@ export class ExtensionsDownloader extends Disposable {
private readonly cleanUpPromise: Promise<void>;

constructor(
private readonly targetPlatform: Promise<TargetPlatform>,
@INativeEnvironmentService environmentService: INativeEnvironmentService,
@IFileService private readonly fileService: IFileService,
@IExtensionGalleryService private readonly extensionGalleryService: IExtensionGalleryService,
Expand All @@ -48,7 +47,7 @@ export class ExtensionsDownloader extends Disposable {
this.cleanUpPromise = this.cleanUp();
}

async download(extension: IGalleryExtension, operation: InstallOperation): Promise<{ readonly location: URI; verified: boolean }> {
async download(extension: IGalleryExtension, operation: InstallOperation): Promise<{ readonly location: URI; readonly verificationStatus: ExtensionVerificationStatus }> {
await this.cleanUpPromise;

const location = joinPath(this.extensionsDownloadDir, this.getName(extension));
Expand All @@ -58,31 +57,39 @@ export class ExtensionsDownloader extends Disposable {
throw new ExtensionManagementError(error.message, ExtensionManagementErrorCode.Download);
}

let verified: boolean = false;
if (await this.checkForVerification(extension)) {
let verificationStatus: ExtensionVerificationStatus = ExtensionVerificationStatus.Unverified;

if (await this.shouldVerifySignature(extension)) {
const signatureArchiveLocation = await this.downloadSignatureArchive(extension);
try {
verified = await this.extensionSignatureVerificationService.verify(location.fsPath, signatureArchiveLocation.fsPath);
this.logService.info(`Verified extension: ${extension.identifier.id}`, verified);
const verified = await this.extensionSignatureVerificationService.verify(location.fsPath, signatureArchiveLocation.fsPath);
if (verified) {
verificationStatus = ExtensionVerificationStatus.Verified;
}
this.logService.info(`Extension signature verification: ${extension.identifier.id}. Verification status: ${verificationStatus}.`);
} catch (error) {
await this.delete(signatureArchiveLocation);
await this.delete(location);
throw new ExtensionManagementError((error as ExtensionSignatureVerificationError).code, ExtensionManagementErrorCode.Signature);
const code: string = (error as ExtensionSignatureVerificationError).code;

if (code === 'UnknownError') {
verificationStatus = ExtensionVerificationStatus.UnknownError;
this.logService.warn(`Extension signature verification: ${extension.identifier.id}. Verification status: ${verificationStatus}.`);
} else {
await this.delete(signatureArchiveLocation);
await this.delete(location);

throw new ExtensionManagementError(code, ExtensionManagementErrorCode.Signature);
}
}
}

return { location, verified };
return { location, verificationStatus };
}

private async checkForVerification(extension: IGalleryExtension): Promise<boolean> {
private async shouldVerifySignature(extension: IGalleryExtension): Promise<boolean> {
if (!extension.isSigned) {
return false;
}
const targetPlatform = await this.targetPlatform;
// Signing module has issue in this platform - https://github.com/microsoft/vscode/issues/164726
if (targetPlatform === TargetPlatform.LINUX_ARMHF) {
return false;
}

const value = this.configurationService.getValue('extensions.verifySignature');
if (isBoolean(value)) {
return value;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import { extract, ExtractError, IFile, zip } from 'vs/base/node/zip';
import * as nls from 'vs/nls';
import { IDownloadService } from 'vs/platform/download/common/download';
import { INativeEnvironmentService } from 'vs/platform/environment/common/environment';
import { AbstractExtensionManagementService, AbstractExtensionTask, IInstallExtensionTask, InstallExtensionTaskOptions, IUninstallExtensionTask, joinErrors, UninstallExtensionTaskOptions } from 'vs/platform/extensionManagement/common/abstractExtensionManagementService';
import { AbstractExtensionManagementService, AbstractExtensionTask, ExtensionVerificationStatus, IInstallExtensionTask, InstallExtensionTaskOptions, IUninstallExtensionTask, joinErrors, UninstallExtensionTaskOptions } from 'vs/platform/extensionManagement/common/abstractExtensionManagementService';
import {
ExtensionManagementError, ExtensionManagementErrorCode, IExtensionGalleryService, IExtensionIdentifier, IExtensionManagementService, IGalleryExtension, IGalleryMetadata, ILocalExtension, InstallOperation,
Metadata, InstallOptions, InstallVSIXOptions
Expand Down Expand Up @@ -87,7 +87,7 @@ export class ExtensionManagementService extends AbstractExtensionManagementServi
const extensionLifecycle = this._register(instantiationService.createInstance(ExtensionsLifecycle));
this.extensionsScanner = this._register(instantiationService.createInstance(ExtensionsScanner, extension => extensionLifecycle.postUninstall(extension)));
this.manifestCache = this._register(new ExtensionsManifestCache(environmentService, this));
this.extensionsDownloader = this._register(instantiationService.createInstance(ExtensionsDownloader, this.getTargetPlatform()));
this.extensionsDownloader = this._register(instantiationService.createInstance(ExtensionsDownloader));

const extensionsWatcher = this._register(new ExtensionsWatcher(this, this.extensionsScannerService, userDataProfilesService, extensionsProfileScannerService, uriIdentityService, fileService, logService));
this._register(extensionsWatcher.onDidChangeExtensionsByAnotherSource(e => this.onDidChangeExtensionsFromAnotherSource(e)));
Expand Down Expand Up @@ -635,7 +635,8 @@ export class ExtensionsScanner extends Disposable {

abstract class InstallExtensionTask extends AbstractExtensionTask<{ local: ILocalExtension; metadata: Metadata }> implements IInstallExtensionTask {

public wasVerified: boolean = false;
protected _verificationStatus = ExtensionVerificationStatus.Unverified;
get verificationStatus() { return this._verificationStatus; }

protected _operation = InstallOperation.Install;
get operation() { return isUndefined(this.options.operation) ? this._operation : this.options.operation; }
Expand Down Expand Up @@ -737,9 +738,9 @@ export class InstallGalleryExtensionTask extends InstallExtensionTask {
return { local, metadata };
}

const { location, verified } = await this.extensionsDownloader.download(this.gallery, this._operation);
const { location, verificationStatus } = await this.extensionsDownloader.download(this.gallery, this._operation);
try {
this.wasVerified = !!verified;
this._verificationStatus = verificationStatus;
this.validateManifest(location.fsPath);
const local = await this.installExtension({ zipPath: location.fsPath, key: ExtensionKey.create(this.gallery), metadata }, token);
return { local, metadata };
Expand Down Expand Up @@ -843,6 +844,10 @@ class InstallExtensionInProfileTask implements IInstallExtensionTask {
readonly source = this.task.source;
readonly operation = this.task.operation;

get verificationStatus() {
return this.task.verificationStatus;
}

private readonly promise: Promise<{ local: ILocalExtension; metadata: Metadata }>;

constructor(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ export interface IExtensionSignatureVerificationService {
* @param { string } signatureArchiveFilePath The signature archive file path.
* @returns { Promise<boolean> } A promise with `true` if the extension is validly signed and trusted;
* otherwise, `false` because verification is not enabled (e.g.: in the OSS version of VS Code).
* @throws { ExtensionVerificationError } An error with a code indicating the validity, integrity, or trust issue
* @throws { ExtensionSignatureVerificationError } An error with a code indicating the validity, integrity, or trust issue
* found during verification or a more fundamental issue (e.g.: a required dependency was not found).
*/
verify(vsixFilePath: string, signatureArchiveFilePath: string): Promise<boolean>;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,12 @@ import { mock } from 'vs/base/test/common/mock';
import { IConfigurationService } from 'vs/platform/configuration/common/configuration';
import { TestConfigurationService } from 'vs/platform/configuration/test/common/testConfigurationService';
import { INativeEnvironmentService } from 'vs/platform/environment/common/environment';
import { ExtensionVerificationStatus } from 'vs/platform/extensionManagement/common/abstractExtensionManagementService';
import { ExtensionManagementError, ExtensionManagementErrorCode, getTargetPlatform, IExtensionGalleryService, IGalleryExtension, IGalleryExtensionAssets, ILocalExtension } from 'vs/platform/extensionManagement/common/extensionManagement';
import { getGalleryExtensionId } from 'vs/platform/extensionManagement/common/extensionManagementUtil';
import { ExtensionsDownloader } from 'vs/platform/extensionManagement/node/extensionDownloader';
import { ExtensionsScanner, InstallGalleryExtensionTask } from 'vs/platform/extensionManagement/node/extensionManagementService';
import { IExtensionSignatureVerificationService } from 'vs/platform/extensionManagement/node/extensionSignatureVerificationService';
import { TargetPlatform } from 'vs/platform/extensions/common/extensions';
import { IFileService } from 'vs/platform/files/common/files';
import { FileService } from 'vs/platform/files/common/fileService';
import { InMemoryFileSystemProvider } from 'vs/platform/files/common/inMemoryFilesystemProvider';
Expand Down Expand Up @@ -93,7 +93,7 @@ suite('InstallGalleryExtensionTask Tests', () => {

await testObject.run();

assert.strictEqual(testObject.wasVerified, true);
assert.strictEqual(testObject.verificationStatus, ExtensionVerificationStatus.Verified);
assert.strictEqual(testObject.installed, true);
});

Expand All @@ -102,7 +102,7 @@ suite('InstallGalleryExtensionTask Tests', () => {

await testObject.run();

assert.strictEqual(testObject.wasVerified, false);
assert.strictEqual(testObject.verificationStatus, ExtensionVerificationStatus.Unverified);
assert.strictEqual(testObject.installed, true);
});

Expand All @@ -111,7 +111,7 @@ suite('InstallGalleryExtensionTask Tests', () => {

await testObject.run();

assert.strictEqual(testObject.wasVerified, false);
assert.strictEqual(testObject.verificationStatus, ExtensionVerificationStatus.Unverified);
assert.strictEqual(testObject.installed, true);
});

Expand All @@ -120,7 +120,7 @@ suite('InstallGalleryExtensionTask Tests', () => {

await testObject.run();

assert.strictEqual(testObject.wasVerified, false);
assert.strictEqual(testObject.verificationStatus, ExtensionVerificationStatus.Unverified);
assert.strictEqual(testObject.installed, true);
});

Expand All @@ -135,7 +135,7 @@ suite('InstallGalleryExtensionTask Tests', () => {
assert.ok(e instanceof ExtensionManagementError);
assert.strictEqual(e.code, ExtensionManagementErrorCode.Signature);
assert.strictEqual(e.message, errorCode);
assert.strictEqual(testObject.wasVerified, false);
assert.strictEqual(testObject.verificationStatus, ExtensionVerificationStatus.Unverified);
assert.strictEqual(testObject.installed, false);
return;
}
Expand All @@ -149,7 +149,7 @@ suite('InstallGalleryExtensionTask Tests', () => {

await testObject.run();

assert.strictEqual(testObject.wasVerified, true);
assert.strictEqual(testObject.verificationStatus, ExtensionVerificationStatus.Verified);
assert.strictEqual(testObject.installed, true);
});

Expand All @@ -158,7 +158,7 @@ suite('InstallGalleryExtensionTask Tests', () => {

await testObject.run();

assert.strictEqual(testObject.wasVerified, false);
assert.strictEqual(testObject.verificationStatus, ExtensionVerificationStatus.Unverified);
assert.strictEqual(testObject.installed, true);
});

Expand All @@ -167,7 +167,7 @@ suite('InstallGalleryExtensionTask Tests', () => {

await testObject.run();

assert.strictEqual(testObject.wasVerified, false);
assert.strictEqual(testObject.verificationStatus, ExtensionVerificationStatus.Unverified);
assert.strictEqual(testObject.installed, true);
});

Expand All @@ -192,7 +192,7 @@ suite('InstallGalleryExtensionTask Tests', () => {
});
instantiationService.stub(IConfigurationService, new TestConfigurationService(isBoolean(isSignatureVerificationEnabled) ? { extensions: { verifySignature: isSignatureVerificationEnabled } } : undefined));
instantiationService.stub(IExtensionSignatureVerificationService, new TestExtensionSignatureVerificationService(verificationResult));
return instantiationService.createInstance(ExtensionsDownloader, Promise.resolve(TargetPlatform.LINUX_X64));
return instantiationService.createInstance(ExtensionsDownloader);
}

function aGalleryExtension(name: string, properties: Partial<IGalleryExtension> = {}, galleryExtensionProperties: any = {}, assets: Partial<IGalleryExtensionAssets> = {}): IGalleryExtension {
Expand Down