Skip to content

Commit

Permalink
#223942 Feedback on blocking extension installation when verification…
Browse files Browse the repository at this point in the history 
… fails (#228510)
  • Loading branch information
@sandy081
sandy081 authored Sep 13, 2024
1 parent 5503abe commit 365d0be
Show file tree
Hide file tree
Showing 7 changed files with 146 additions and 134 deletions.
43 changes: 13 additions & 30 deletions src/vs/platform/extensionManagement/common/abstractExtensionManagementService.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import { Emitter, Event } from '../../../base/common/event.js';
import { Disposable, toDisposable } from '../../../base/common/lifecycle.js';
import { ResourceMap } from '../../../base/common/map.js';
import { isWeb } from '../../../base/common/platform.js';
import { isDefined } from '../../../base/common/types.js';
import { URI } from '../../../base/common/uri.js';
import * as nls from '../../../nls.js';
import {
Expand All @@ -21,7 +20,8 @@ import {
IProductVersion, ExtensionGalleryErrorCode,
EXTENSION_INSTALL_SOURCE_CONTEXT,
DidUpdateExtensionMetadata,
UninstallExtensionInfo
UninstallExtensionInfo,
ExtensionSignatureVerificationCode
} from './extensionManagement.js';
import { areSameExtensions, ExtensionKey, getGalleryExtensionId, getGalleryExtensionTelemetryData, getLocalExtensionTelemetryData } from './extensionManagementUtil.js';
import { ExtensionType, IExtensionManifest, isApplicationScopedExtension, TargetPlatform } from '../../extensions/common/extensions.js';
Expand All @@ -32,7 +32,6 @@ import { ITelemetryService } from '../../telemetry/common/telemetry.js';
import { IUriIdentityService } from '../../uriIdentity/common/uriIdentity.js';
import { IUserDataProfilesService } from '../../userDataProfile/common/userDataProfile.js';

export type ExtensionVerificationStatus = boolean | string;
export type InstallableExtension = { readonly manifest: IExtensionManifest; extension: IGalleryExtension | URI; options: InstallOptions };

export type InstallExtensionTaskOptions = InstallOptions & { readonly profileLocation: URI; readonly productVersion: IProductVersion };
Expand All @@ -42,7 +41,7 @@ export interface IInstallExtensionTask {
readonly source: IGalleryExtension | URI;
readonly operation: InstallOperation;
readonly options: InstallExtensionTaskOptions;
readonly verificationStatus?: ExtensionVerificationStatus;
readonly verificationStatus?: ExtensionSignatureVerificationCode;
run(): Promise<ILocalExtension>;
waitUntilTaskIsFinished(): Promise<ILocalExtension>;
cancel(): void;
Expand Down Expand Up @@ -889,34 +888,12 @@ function reportTelemetry(telemetryService: ITelemetryService, eventName: string,
durationSinceUpdate
}: {
extensionData: any;
verificationStatus?:
ExtensionVerificationStatus;
verificationStatus?: ExtensionSignatureVerificationCode;
duration?: number;
durationSinceUpdate?: number;
source?: string;
error?: ExtensionManagementError | ExtensionGalleryError;
}): void {
let errorcode: string | undefined;
let errorcodeDetail: string | undefined;

if (isDefined(verificationStatus)) {
if (verificationStatus === true) {
verificationStatus = 'Verified';
} else if (verificationStatus === false) {
verificationStatus = 'Unverified';
} else {
errorcode = ExtensionManagementErrorCode.Signature;
errorcodeDetail = verificationStatus;
verificationStatus = 'Unverified';
}
}

if (error) {
errorcode = error.code;
if (error.code === ExtensionManagementErrorCode.Signature) {
errorcodeDetail = error.message;
}
}

/* __GDPR__
"extensionGallery:install" : {
Expand All @@ -925,7 +902,6 @@ function reportTelemetry(telemetryService: ITelemetryService, eventName: string,
"duration" : { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "isMeasurement": true },
"durationSinceUpdate" : { "classification": "SystemMetaData", "purpose": "FeatureInsight", "isMeasurement": true },
"errorcode": { "classification": "CallstackOrException", "purpose": "PerformanceAndHealth" },
"errorcodeDetail": { "classification": "CallstackOrException", "purpose": "PerformanceAndHealth" },
"recommendationReason": { "retiredFromVersion": "1.23.0", "classification": "SystemMetaData", "purpose": "FeatureInsight", "isMeasurement": true },
"verificationStatus" : { "classification": "SystemMetaData", "purpose": "FeatureInsight" },
"source": { "classification": "SystemMetaData", "purpose": "FeatureInsight" },
Expand All @@ -951,15 +927,22 @@ function reportTelemetry(telemetryService: ITelemetryService, eventName: string,
"success": { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "isMeasurement": true },
"duration" : { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "isMeasurement": true },
"errorcode": { "classification": "CallstackOrException", "purpose": "PerformanceAndHealth" },
"errorcodeDetail": { "classification": "CallstackOrException", "purpose": "PerformanceAndHealth" },
"verificationStatus" : { "classification": "SystemMetaData", "purpose": "FeatureInsight" },
"source": { "classification": "SystemMetaData", "purpose": "FeatureInsight" },
"${include}": [
"${GalleryExtensionTelemetryData}"
]
}
*/
telemetryService.publicLog(eventName, { ...extensionData, verificationStatus, success: !error, duration, errorcode, errorcodeDetail, durationSinceUpdate, source });
telemetryService.publicLog(eventName, {
...extensionData,
source,
duration,
durationSinceUpdate,
success: !error,
errorcode: error?.code,
verificationStatus: verificationStatus === ExtensionSignatureVerificationCode.Success ? 'Verified' : (verificationStatus ?? 'Unverified')
});
}

export abstract class AbstractExtensionTask<T> {
Expand Down
33 changes: 32 additions & 1 deletion src/vs/platform/extensionManagement/common/extensionManagement.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -462,14 +462,45 @@ export const enum ExtensionManagementErrorCode {
PostInstall = 'PostInstall',
CorruptZip = 'CorruptZip',
IncompleteZip = 'IncompleteZip',
Signature = 'Signature',
PackageNotSigned = 'PackageNotSigned',
SignatureVerificationInternal = 'SignatureVerificationInternal',
SignatureVerificationFailed = 'SignatureVerificationFailed',
NotAllowed = 'NotAllowed',
Gallery = 'Gallery',
Cancelled = 'Cancelled',
Unknown = 'Unknown',
Internal = 'Internal',
}

export enum ExtensionSignatureVerificationCode {
'Success' = 'Success',
'RequiredArgumentMissing' = 'RequiredArgumentMissing', // A required argument is missing.
'InvalidArgument' = 'InvalidArgument', // An argument is invalid.
'PackageIsUnreadable' = 'PackageIsUnreadable', // The extension package is unreadable.
'UnhandledException' = 'UnhandledException', // An unhandled exception occurred.
'SignatureManifestIsMissing' = 'SignatureManifestIsMissing', // The extension is missing a signature manifest file (.signature.manifest).
'SignatureManifestIsUnreadable' = 'SignatureManifestIsUnreadable', // The signature manifest is unreadable.
'SignatureIsMissing' = 'SignatureIsMissing', // The extension is missing a signature file (.signature.p7s).
'SignatureIsUnreadable' = 'SignatureIsUnreadable', // The signature is unreadable.
'CertificateIsUnreadable' = 'CertificateIsUnreadable', // The certificate is unreadable.
'SignatureArchiveIsUnreadable' = 'SignatureArchiveIsUnreadable',
'FileAlreadyExists' = 'FileAlreadyExists', // The output file already exists.
'SignatureArchiveIsInvalidZip' = 'SignatureArchiveIsInvalidZip',
'SignatureArchiveHasSameSignatureFile' = 'SignatureArchiveHasSameSignatureFile', // The signature archive has the same signature file.
'PackageIntegrityCheckFailed' = 'PackageIntegrityCheckFailed', // The package integrity check failed.
'SignatureIsInvalid' = 'SignatureIsInvalid', // The extension has an invalid signature file (.signature.p7s).
'SignatureManifestIsInvalid' = 'SignatureManifestIsInvalid', // The extension has an invalid signature manifest file (.signature.manifest).
'SignatureIntegrityCheckFailed' = 'SignatureIntegrityCheckFailed', // The extension's signature integrity check failed. Extension integrity is suspect.
'EntryIsMissing' = 'EntryIsMissing', // An entry referenced in the signature manifest was not found in the extension.
'EntryIsTampered' = 'EntryIsTampered', // The integrity check for an entry referenced in the signature manifest failed.
'Untrusted' = 'Untrusted', // An X.509 certificate in the extension signature is untrusted.
'CertificateRevoked' = 'CertificateRevoked', // An X.509 certificate in the extension signature has been revoked.
'SignatureIsNotValid' = 'SignatureIsNotValid', // The extension signature is invalid.
'UnknownError' = 'UnknownError', // An unknown error occurred.
'PackageIsInvalidZip' = 'PackageIsInvalidZip', // The extension package is not valid ZIP format.
'SignatureArchiveHasTooManyEntries' = 'SignatureArchiveHasTooManyEntries', // The signature archive has too many entries.
}

export class ExtensionManagementError extends Error {
constructor(message: string, readonly code: ExtensionManagementErrorCode) {
super(message);
Expand Down
51 changes: 21 additions & 30 deletions src/vs/platform/extensionManagement/node/extensionDownloader.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,11 @@ import { generateUuid } from '../../../base/common/uuid.js';
import { Promises as FSPromises } from '../../../base/node/pfs.js';
import { buffer, CorruptZipMessage } from '../../../base/node/zip.js';
import { INativeEnvironmentService } from '../../environment/common/environment.js';
import { ExtensionVerificationStatus, toExtensionManagementError } from '../common/abstractExtensionManagementService.js';
import { ExtensionManagementError, ExtensionManagementErrorCode, IExtensionGalleryService, IGalleryExtension, InstallOperation } from '../common/extensionManagement.js';
import { toExtensionManagementError } from '../common/abstractExtensionManagementService.js';
import { ExtensionManagementError, ExtensionManagementErrorCode, ExtensionSignatureVerificationCode, IExtensionGalleryService, IGalleryExtension, InstallOperation } from '../common/extensionManagement.js';
import { ExtensionKey, groupByExtension } from '../common/extensionManagementUtil.js';
import { fromExtractError } from './extensionManagementUtil.js';
import { ExtensionSignatureVerificationError, ExtensionSignatureVerificationCode, IExtensionSignatureVerificationService } from './extensionSignatureVerificationService.js';
import { IExtensionSignatureVerificationService } from './extensionSignatureVerificationService.js';
import { TargetPlatform } from '../../extensions/common/extensions.js';
import { IFileService, IFileStatWithMetadata } from '../../files/common/files.js';
import { ILogService } from '../../log/common/log.js';
Expand Down Expand Up @@ -57,37 +57,19 @@ export class ExtensionsDownloader extends Disposable {
this.cleanUpPromise = this.cleanUp();
}

async download(extension: IGalleryExtension, operation: InstallOperation, verifySignature: boolean, clientTargetPlatform?: TargetPlatform): Promise<{ readonly location: URI; readonly verificationStatus: ExtensionVerificationStatus }> {
async download(extension: IGalleryExtension, operation: InstallOperation, verifySignature: boolean, clientTargetPlatform?: TargetPlatform): Promise<{ readonly location: URI; readonly verificationStatus: ExtensionSignatureVerificationCode | undefined }> {
await this.cleanUpPromise;

const location = await this.downloadVSIX(extension, operation);

if (!verifySignature) {
return { location, verificationStatus: false };
}

if (!extension.isSigned) {
return { location, verificationStatus: 'PackageIsUnsigned' };
if (!verifySignature || !extension.isSigned) {
return { location, verificationStatus: undefined };
}

let signatureArchiveLocation;
try {
signatureArchiveLocation = await this.downloadSignatureArchive(extension);
} catch (error) {
try {
// Delete the downloaded VSIX if signature archive download fails
await this.delete(location);
} catch (error) {
this.logService.error(error);
}
throw error;
}

let verificationStatus;
try {
verificationStatus = await this.extensionSignatureVerificationService.verify(extension.identifier.id, extension.version, location.fsPath, signatureArchiveLocation.fsPath, clientTargetPlatform);
} catch (error) {
verificationStatus = (error as ExtensionSignatureVerificationError).code;
const verificationStatus = (await this.extensionSignatureVerificationService.verify(extension.identifier.id, extension.version, location.fsPath, signatureArchiveLocation.fsPath, clientTargetPlatform))?.code;
if (verificationStatus === ExtensionSignatureVerificationCode.PackageIsInvalidZip || verificationStatus === ExtensionSignatureVerificationCode.SignatureArchiveIsInvalidZip) {
try {
// Delete the downloaded vsix if VSIX or signature archive is invalid
Expand All @@ -97,16 +79,25 @@ export class ExtensionsDownloader extends Disposable {
}
throw new ExtensionManagementError(CorruptZipMessage, ExtensionManagementErrorCode.CorruptZip);
}
} finally {
return { location, verificationStatus };
} catch (error) {
try {
// Delete signature archive always
await this.delete(signatureArchiveLocation);
// Delete the downloaded VSIX if signature archive download fails
await this.delete(location);
} catch (error) {
this.logService.error(error);
}
throw error;
} finally {
if (signatureArchiveLocation) {
try {
// Delete signature archive always
await this.delete(signatureArchiveLocation);
} catch (error) {
this.logService.error(error);
}
}
}

return { location, verificationStatus };
}

private async downloadVSIX(extension: IGalleryExtension, operation: InstallOperation): Promise<URI> {
Expand Down
35 changes: 29 additions & 6 deletions src/vs/platform/extensionManagement/node/extensionManagementService.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,13 @@ import { extract, IFile, zip } from '../../../base/node/zip.js';
import * as nls from '../../../nls.js';
import { IDownloadService } from '../../download/common/download.js';
import { INativeEnvironmentService } from '../../environment/common/environment.js';
import { AbstractExtensionManagementService, AbstractExtensionTask, ExtensionVerificationStatus, IInstallExtensionTask, InstallExtensionTaskOptions, IUninstallExtensionTask, toExtensionManagementError, UninstallExtensionTaskOptions } from '../common/abstractExtensionManagementService.js';
import { AbstractExtensionManagementService, AbstractExtensionTask, IInstallExtensionTask, InstallExtensionTaskOptions, IUninstallExtensionTask, toExtensionManagementError, UninstallExtensionTaskOptions } from '../common/abstractExtensionManagementService.js';
import {
ExtensionManagementError, ExtensionManagementErrorCode, IExtensionGalleryService, IExtensionIdentifier, IExtensionManagementService, IGalleryExtension, ILocalExtension, InstallOperation,
Metadata, InstallOptions,
IProductVersion,
EXTENSION_INSTALL_CLIENT_TARGET_PLATFORM_CONTEXT,
ExtensionSignatureVerificationCode,
} from '../common/extensionManagement.js';
import { areSameExtensions, computeTargetPlatform, ExtensionKey, getGalleryExtensionId, groupByExtension } from '../common/extensionManagementUtil.js';
import { IExtensionsProfileScannerService, IScannedProfileExtension } from '../common/extensionsProfileScannerService.js';
Expand Down Expand Up @@ -61,7 +62,7 @@ export interface INativeServerExtensionManagementService extends IExtensionManag
markAsUninstalled(...extensions: IExtension[]): Promise<void>;
}

type ExtractExtensionResult = { readonly local: ILocalExtension; readonly verificationStatus?: ExtensionVerificationStatus };
type ExtractExtensionResult = { readonly local: ILocalExtension; readonly verificationStatus?: ExtensionSignatureVerificationCode };

const DELETED_FOLDER_POSTFIX = '.vsctmp';

Expand Down Expand Up @@ -342,15 +343,37 @@ export class ExtensionManagementService extends AbstractExtensionManagementServi
}
}

private async downloadExtension(extension: IGalleryExtension, operation: InstallOperation, verifySignature: boolean, clientTargetPlatform?: TargetPlatform): Promise<{ readonly location: URI; readonly verificationStatus: ExtensionVerificationStatus }> {
private async downloadExtension(extension: IGalleryExtension, operation: InstallOperation, verifySignature: boolean, clientTargetPlatform?: TargetPlatform): Promise<{ readonly location: URI; readonly verificationStatus: ExtensionSignatureVerificationCode | undefined }> {
if (verifySignature) {
const value = this.configurationService.getValue('extensions.verifySignature');
verifySignature = isBoolean(value) ? value : true;
}
const { location, verificationStatus } = await this.extensionsDownloader.download(extension, operation, verifySignature, clientTargetPlatform);

if (verificationStatus !== true && verifySignature && this.environmentService.isBuilt && !isLinux) {
throw new ExtensionManagementError(nls.localize('download failed', "Signature verification failed with '{0}' error.", verificationStatus === false ? 'NotExecuted' : verificationStatus), ExtensionManagementErrorCode.Signature);
if (verificationStatus !== ExtensionSignatureVerificationCode.Success && verifySignature && this.environmentService.isBuilt && !isLinux) {
if (!extension.isSigned) {
throw new ExtensionManagementError(nls.localize('not signed', "Extension is not signed."), ExtensionManagementErrorCode.PackageNotSigned);
}

if (!verificationStatus) {
throw new ExtensionManagementError(nls.localize('signature verification not executed', "Signature verification was not executed."), ExtensionManagementErrorCode.SignatureVerificationInternal);
}

switch (verificationStatus) {
case ExtensionSignatureVerificationCode.PackageIntegrityCheckFailed:
case ExtensionSignatureVerificationCode.SignatureIsInvalid:
case ExtensionSignatureVerificationCode.SignatureManifestIsInvalid:
case ExtensionSignatureVerificationCode.SignatureIntegrityCheckFailed:
case ExtensionSignatureVerificationCode.EntryIsMissing:
case ExtensionSignatureVerificationCode.EntryIsTampered:
case ExtensionSignatureVerificationCode.Untrusted:
case ExtensionSignatureVerificationCode.CertificateRevoked:
case ExtensionSignatureVerificationCode.SignatureIsNotValid:
case ExtensionSignatureVerificationCode.SignatureArchiveHasTooManyEntries:
throw new ExtensionManagementError(nls.localize('signature verification failed', "Signature verification failed with '{0}' error.", verificationStatus), ExtensionManagementErrorCode.SignatureVerificationFailed);
}

throw new ExtensionManagementError(nls.localize('signature verification failed', "Signature verification failed with '{0}' error.", verificationStatus), ExtensionManagementErrorCode.SignatureVerificationInternal);
}

return { location, verificationStatus };
Expand Down Expand Up @@ -932,7 +955,7 @@ class InstallExtensionInProfileTask extends AbstractExtensionTask<ILocalExtensio
private _operation = InstallOperation.Install;
get operation() { return this.options.operation ?? this._operation; }

private _verificationStatus: ExtensionVerificationStatus | undefined;
private _verificationStatus: ExtensionSignatureVerificationCode | undefined;
get verificationStatus() { return this._verificationStatus; }

readonly identifier: IExtensionIdentifier;
Expand Down
Loading

0 comments on commit 365d0be

Please sign in to comment.