Skip to content

Commit

Permalink
Merge pull request microsoft#169777 from dtivel/dtivel/unknown-error
Browse files Browse the repository at this point in the history 
Extensions:  warn not fail on UnknownError during signature verification
  • Loading branch information
@sandy081
sandy081 authored Dec 26, 2022
2 parents a606c86 + 8fff554 commit 32e8665
Show file tree
Hide file tree
Showing 5 changed files with 58 additions and 40 deletions.
20 changes: 13 additions & 7 deletions src/vs/platform/extensionManagement/common/abstractExtensionManagementService.ts
View file
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
View file
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
15 changes: 10 additions & 5 deletions src/vs/platform/extensionManagement/node/extensionManagementService.ts
View file
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 @@ -633,7 +633,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 @@ -735,9 +736,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 @@ -841,6 +842,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
2 changes: 1 addition & 1 deletion src/vs/platform/extensionManagement/node/extensionSignatureVerificationService.ts
View file
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
20 changes: 10 additions & 10 deletions src/vs/platform/extensionManagement/test/node/installGalleryExtensionTask.test.ts
View file
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

0 comments on commit 32e8665

Please sign in to comment.