log extension signature verification output on success (#210596)

package.json

@@ -1,7 +1,7 @@
 {
  "name": "code-oss-dev",
  "version": "1.89.0",
- "distro": "25df1b5602ba46d6b0f618dd227963d1163c83df",
+ "distro": "853f475a6e8e85ad811a2f8e49c3032bdb660558",
  "author": {
  "name": "Microsoft Corporation"
  },

src/vs/platform/extensionManagement/common/extensionManagement.ts

@@ -431,12 +431,6 @@ export enum ExtensionManagementErrorCode {
  Internal = 'Internal',
 }
 
-export enum ExtensionSignaturetErrorCode {
- UnknownError = 'UnknownError',
- PackageIsInvalidZip = 'PackageIsInvalidZip',
- SignatureArchiveIsInvalidZip = 'SignatureArchiveIsInvalidZip',
-}
-
 export class ExtensionManagementError extends Error {
  constructor(message: string, readonly code: ExtensionManagementErrorCode) {
  super(message);

src/vs/platform/extensionManagement/node/extensionDownloader.ts

@@ -17,11 +17,11 @@ import { CorruptZipMessage } from 'vs/base/node/zip';
 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, ExtensionSignaturetErrorCode, IExtensionGalleryService, IGalleryExtension, InstallOperation } from 'vs/platform/extensionManagement/common/extensionManagement';
+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 { ExtensionSignatureVerificationError, ExtensionSignatureVerificationCode, IExtensionSignatureVerificationService } from 'vs/platform/extensionManagement/node/extensionSignatureVerificationService';
 import { IFileService, IFileStatWithMetadata } from 'vs/platform/files/common/files';
-import { ILogService, LogLevel } from 'vs/platform/log/common/log';
+import { ILogService } from 'vs/platform/log/common/log';
 
 export class ExtensionsDownloader extends Disposable {
 
@@ -60,14 +60,11 @@ export class ExtensionsDownloader extends Disposable {
  if (verifySignature && this.shouldVerifySignature(extension)) {
  const signatureArchiveLocation = await this.downloadSignatureArchive(extension);
  try {
- verificationStatus = await this.extensionSignatureVerificationService.verify(location.fsPath, signatureArchiveLocation.fsPath, this.logService.getLevel() === LogLevel.Trace);
+ verificationStatus = await this.extensionSignatureVerificationService.verify(extension.identifier.id, location.fsPath, signatureArchiveLocation.fsPath);
  } catch (error) {
  const sigError = error as ExtensionSignatureVerificationError;
  verificationStatus = sigError.code;
- if (sigError.output) {
- this.logService.trace(`Extension signature verification details for ${extension.identifier.id} ${extension.version}:\n${sigError.output}`);
- }
- if (verificationStatus === ExtensionSignaturetErrorCode.PackageIsInvalidZip || verificationStatus === ExtensionSignaturetErrorCode.SignatureArchiveIsInvalidZip) {
+ if (verificationStatus === ExtensionSignatureVerificationCode.PackageIsInvalidZip || verificationStatus === ExtensionSignatureVerificationCode.SignatureArchiveIsInvalidZip) {
  try {
  // Delete the downloaded vsix before throwing the error
  await this.delete(location);
@@ -86,14 +83,6 @@ export class ExtensionsDownloader extends Disposable {
  }
  }
 
- if (verificationStatus === true) {
- this.logService.info(`Extension signature is verified: ${extension.identifier.id}`);
- } else if (verificationStatus === false) {
- this.logService.info(`Extension signature verification is not done: ${extension.identifier.id}`);
- } else {
- this.logService.warn(`Extension signature verification failed with error '${verificationStatus}': ${extension.identifier.id}`);
- }
-
  return { location, verificationStatus };
  }
 

src/vs/platform/extensionManagement/node/extensionSignatureVerificationService.ts

@@ -5,7 +5,7 @@
 
 import { getErrorMessage } from 'vs/base/common/errors';
 import { createDecorator } from 'vs/platform/instantiation/common/instantiation';
-import { ILogService } from 'vs/platform/log/common/log';
+import { ILogService, LogLevel } from 'vs/platform/log/common/log';
 import { ITelemetryService } from 'vs/platform/telemetry/common/telemetry';
 
 export const IExtensionSignatureVerificationService = createDecorator<IExtensionSignatureVerificationService>('IExtensionSignatureVerificationService');
@@ -18,30 +18,69 @@ export interface IExtensionSignatureVerificationService {
 
  /**
  * Verifies an extension file (.vsix) against a signature archive file.
+ * @param { string } extensionId The extension identifier.
  * @param { string } vsixFilePath The extension file path.
  * @param { string } signatureArchiveFilePath The signature archive file path.
- * @param { boolean } verbose A flag indicating whether or not to capture verbose detail in the event of an error.
  * @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 { 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, verbose: boolean): Promise<boolean>;
+ verify(extensionId: string, vsixFilePath: string, signatureArchiveFilePath: string): Promise<boolean>;
 }
 
 declare module vsceSign {
- export function verify(vsixFilePath: string, signatureArchiveFilePath: string, verbose: boolean): Promise<boolean>;
+ export function verify(vsixFilePath: string, signatureArchiveFilePath: string, verbose: boolean): Promise<ExtensionSignatureVerificationResult>;
+}
+
+export const enum ExtensionSignatureVerificationCode {
+ 'None' = 'None',
+ 'RequiredArgumentMissing' = 'RequiredArgumentMissing',
+ 'InvalidArgument' = 'InvalidArgument',
+ 'PackageIsUnreadable' = 'PackageIsUnreadable',
+ 'UnhandledException' = 'UnhandledException',
+ 'SignatureManifestIsMissing' = 'SignatureManifestIsMissing',
+ 'SignatureManifestIsUnreadable' = 'SignatureManifestIsUnreadable',
+ 'SignatureIsMissing' = 'SignatureIsMissing',
+ 'SignatureIsUnreadable' = 'SignatureIsUnreadable',
+ 'CertificateIsUnreadable' = 'CertificateIsUnreadable',
+ 'SignatureArchiveIsUnreadable' = 'SignatureArchiveIsUnreadable',
+ 'FileAlreadyExists' = 'FileAlreadyExists',
+ 'SignatureArchiveIsInvalidZip' = 'SignatureArchiveIsInvalidZip',
+ 'SignatureArchiveHasSameSignatureFile' = 'SignatureArchiveHasSameSignatureFile',
+
+ 'Success' = 'Success',
+ 'PackageIntegrityCheckFailed' = 'PackageIntegrityCheckFailed',
+ 'SignatureIsInvalid' = 'SignatureIsInvalid',
+ 'SignatureManifestIsInvalid' = 'SignatureManifestIsInvalid',
+ 'SignatureIntegrityCheckFailed' = 'SignatureIntegrityCheckFailed',
+ 'EntryIsMissing' = 'EntryIsMissing',
+ 'EntryIsTampered' = 'EntryIsTampered',
+ 'Untrusted' = 'Untrusted',
+ 'CertificateRevoked' = 'CertificateRevoked',
+ 'SignatureIsNotValid' = 'SignatureIsNotValid',
+ 'UnknownError' = 'UnknownError',
+ 'PackageIsInvalidZip' = 'PackageIsInvalidZip',
+ 'SignatureArchiveHasTooManyEntries' = 'SignatureArchiveHasTooManyEntries',
 }
 
 /**
- * An error raised during extension signature verification.
+ * Extension signature verification result
  */
-export interface ExtensionSignatureVerificationError extends Error {
- readonly code: string;
+export interface ExtensionSignatureVerificationResult {
+ readonly code: ExtensionSignatureVerificationCode;
  readonly didExecute: boolean;
  readonly output?: string;
 }
 
+export class ExtensionSignatureVerificationError extends Error {
+ constructor(
+ public readonly code: ExtensionSignatureVerificationCode,
+ ) {
+ super(code);
+ }
+}
+
 export class ExtensionSignatureVerificationService implements IExtensionSignatureVerificationService {
  declare readonly _serviceBrand: undefined;
 
@@ -67,45 +106,60 @@ export class ExtensionSignatureVerificationService implements IExtensionSignatur
  return this.moduleLoadingPromise;
  }
 
- public async verify(vsixFilePath: string, signatureArchiveFilePath: string, verbose: boolean): Promise<boolean> {
+ public async verify(extensionId: string, vsixFilePath: string, signatureArchiveFilePath: string): Promise<boolean> {
  let module: typeof vsceSign;
 
  try {
  module = await this.vsceSign();
  } catch (error) {
  this.logService.error('Could not load vsce-sign module', getErrorMessage(error));
+ this.logService.info(`Extension signature verification is not done: ${extensionId}`);
  return false;
  }
 
  const startTime = new Date().getTime();
- let verified: boolean | undefined;
- let error: ExtensionSignatureVerificationError | undefined;
+ let result: ExtensionSignatureVerificationResult;
 
  try {
- verified = await module.verify(vsixFilePath, signatureArchiveFilePath, verbose);
- return verified;
+ result = await module.verify(vsixFilePath, signatureArchiveFilePath, this.logService.getLevel() === LogLevel.Trace);
  } catch (e) {
- error = e;
- throw e;
- } finally {
- const duration = new Date().getTime() - startTime;
- type ExtensionSignatureVerificationClassification = {
- owner: 'sandy081';
- comment: 'Extension signature verification event';
- duration: { classification: 'SystemMetaData'; purpose: 'PerformanceAndHealth'; 'isMeasurement': true; comment: 'amount of time taken to verify the signature' };
- verified?: { classification: 'SystemMetaData'; purpose: 'FeatureInsight'; comment: 'verified status when succeeded' };
- error?: { classification: 'SystemMetaData'; purpose: 'FeatureInsight'; comment: 'error code when failed' };
- };
- type ExtensionSignatureVerificationEvent = {
- duration: number;
- verified?: boolean;
- error?: string;
+ result = {
+ code: ExtensionSignatureVerificationCode.UnknownError,
+ didExecute: false,
+ output: getErrorMessage(e)
  };
- this.telemetryService.publicLog2<ExtensionSignatureVerificationEvent, ExtensionSignatureVerificationClassification>('extensionsignature:verification', {
- duration,
- verified,
- error: error ? (error.code ?? 'unknown') : undefined,
- });
  }
+
+ const duration = new Date().getTime() - startTime;
+
+ this.logService.info(`Extension signature verification result for ${extensionId}: ${result.code}. Duration: ${duration}ms.`);
+ this.logService.trace(`Extension signature verification output for ${extensionId}:\n${result.output}`);
+
+ type ExtensionSignatureVerificationClassification = {
+ owner: 'sandy081';
+ comment: 'Extension signature verification event';
+ extensionId: { classification: 'SystemMetaData'; purpose: 'FeatureInsight'; comment: 'extension identifier' };
+ code: { classification: 'SystemMetaData'; purpose: 'FeatureInsight'; comment: 'result code of the verification' };
+ duration: { classification: 'SystemMetaData'; purpose: 'PerformanceAndHealth'; 'isMeasurement': true; comment: 'amount of time taken to verify the signature' };
+ didExecute: { classification: 'SystemMetaData'; purpose: 'FeatureInsight'; comment: 'whether the verification was executed' };
+ };
+ type ExtensionSignatureVerificationEvent = {
+ extensionId: string;
+ code: string;
+ duration: number;
+ didExecute: boolean;
+ };
+ this.telemetryService.publicLog2<ExtensionSignatureVerificationEvent, ExtensionSignatureVerificationClassification>('extensionsignature:verification', {
+ extensionId,
+ code: result.code,
+ duration,
+ didExecute: result.didExecute
+ });
+
+ if (result.code === ExtensionSignatureVerificationCode.Success) {
+ return true;
+ }
+
+ throw new ExtensionSignatureVerificationError(result.code);
  }
 }

src/vs/platform/extensionManagement/test/node/installGalleryExtensionTask.test.ts

@@ -48,8 +48,7 @@ class TestExtensionsScanner extends mock<ExtensionsScanner>() {
 class TestExtensionSignatureVerificationService extends mock<IExtensionSignatureVerificationService>() {
 
  constructor(
- private readonly verificationResult: string | boolean,
- private readonly didExecute: boolean) {
+ private readonly verificationResult: string | boolean) {
  super();
  }
 
@@ -59,7 +58,6 @@ class TestExtensionSignatureVerificationService extends mock<IExtensionSignature
  }
  const error = Error(this.verificationResult);
  (error as any).code = this.verificationResult;
- (error as any).didExecute = this.didExecute;
  throw error;
  }
 }
@@ -132,7 +130,7 @@ suite('InstallGalleryExtensionTask Tests', () => {
  const disposables = ensureNoDisposablesAreLeakedInTestSuite();
 
  test('if verification is enabled by default, the task completes', async () => {
- const testObject = new TestInstallGalleryExtensionTask(aGalleryExtension('a', { isSigned: true }), anExtensionsDownloader({ isSignatureVerificationEnabled: true, verificationResult: true, didExecute: true }), disposables.add(new DisposableStore()));
+ const testObject = new TestInstallGalleryExtensionTask(aGalleryExtension('a', { isSigned: true }), anExtensionsDownloader({ isSignatureVerificationEnabled: true, verificationResult: true }), disposables.add(new DisposableStore()));
 
  await testObject.run();
 
@@ -141,7 +139,7 @@ suite('InstallGalleryExtensionTask Tests', () => {
  });
 
  test('if verification is enabled in stable, the task completes', async () => {
- const testObject = new TestInstallGalleryExtensionTask(aGalleryExtension('a', { isSigned: true }), anExtensionsDownloader({ isSignatureVerificationEnabled: true, verificationResult: true, didExecute: true, quality: 'stable' }), disposables.add(new DisposableStore()));
+ const testObject = new TestInstallGalleryExtensionTask(aGalleryExtension('a', { isSigned: true }), anExtensionsDownloader({ isSignatureVerificationEnabled: true, verificationResult: true, quality: 'stable' }), disposables.add(new DisposableStore()));
 
  await testObject.run();
 
@@ -150,7 +148,7 @@ suite('InstallGalleryExtensionTask Tests', () => {
  });
 
  test('if verification is disabled by setting set to false, the task skips verification', async () => {
- const testObject = new TestInstallGalleryExtensionTask(aGalleryExtension('a', { isSigned: true }), anExtensionsDownloader({ isSignatureVerificationEnabled: false, verificationResult: 'error', didExecute: false }), disposables.add(new DisposableStore()));
+ const testObject = new TestInstallGalleryExtensionTask(aGalleryExtension('a', { isSigned: true }), anExtensionsDownloader({ isSignatureVerificationEnabled: false, verificationResult: 'error' }), disposables.add(new DisposableStore()));
 
  await testObject.run();
 
@@ -159,7 +157,7 @@ suite('InstallGalleryExtensionTask Tests', () => {
  });
 
  test('if verification is disabled because the module is not loaded, the task skips verification', async () => {
- const testObject = new TestInstallGalleryExtensionTask(aGalleryExtension('a', { isSigned: true }), anExtensionsDownloader({ isSignatureVerificationEnabled: true, verificationResult: false, didExecute: false }), disposables.add(new DisposableStore()));
+ const testObject = new TestInstallGalleryExtensionTask(aGalleryExtension('a', { isSigned: true }), anExtensionsDownloader({ isSignatureVerificationEnabled: true, verificationResult: false }), disposables.add(new DisposableStore()));
 
  await testObject.run();
 
@@ -169,7 +167,7 @@ suite('InstallGalleryExtensionTask Tests', () => {
 
  test('if verification fails to execute, the task completes', async () => {
  const errorCode = 'ENOENT';
- const testObject = new TestInstallGalleryExtensionTask(aGalleryExtension('a', { isSigned: true }), anExtensionsDownloader({ isSignatureVerificationEnabled: true, verificationResult: errorCode, didExecute: false }), disposables.add(new DisposableStore()));
+ const testObject = new TestInstallGalleryExtensionTask(aGalleryExtension('a', { isSigned: true }), anExtensionsDownloader({ isSignatureVerificationEnabled: true, verificationResult: errorCode }), disposables.add(new DisposableStore()));
 
  await testObject.run();
 
@@ -180,7 +178,7 @@ suite('InstallGalleryExtensionTask Tests', () => {
  test('if verification fails', async () => {
  const errorCode = 'IntegrityCheckFailed';
 
- const testObject = new TestInstallGalleryExtensionTask(aGalleryExtension('a', { isSigned: true }), anExtensionsDownloader({ isSignatureVerificationEnabled: true, verificationResult: errorCode, didExecute: true }), disposables.add(new DisposableStore()));
+ const testObject = new TestInstallGalleryExtensionTask(aGalleryExtension('a', { isSigned: true }), anExtensionsDownloader({ isSignatureVerificationEnabled: true, verificationResult: errorCode }), disposables.add(new DisposableStore()));
 
  await testObject.run();
 
@@ -189,7 +187,7 @@ suite('InstallGalleryExtensionTask Tests', () => {
  });
 
  test('if verification succeeds, the task completes', async () => {
- const testObject = new TestInstallGalleryExtensionTask(aGalleryExtension('a', { isSigned: true }), anExtensionsDownloader({ isSignatureVerificationEnabled: true, verificationResult: true, didExecute: true }), disposables.add(new DisposableStore()));
+ const testObject = new TestInstallGalleryExtensionTask(aGalleryExtension('a', { isSigned: true }), anExtensionsDownloader({ isSignatureVerificationEnabled: true, verificationResult: true }), disposables.add(new DisposableStore()));
 
  await testObject.run();
 
@@ -198,7 +196,7 @@ suite('InstallGalleryExtensionTask Tests', () => {
  });
 
  test('task completes for unsigned extension', async () => {
- const testObject = new TestInstallGalleryExtensionTask(aGalleryExtension('a', { isSigned: false }), anExtensionsDownloader({ isSignatureVerificationEnabled: true, verificationResult: true, didExecute: false }), disposables.add(new DisposableStore()));
+ const testObject = new TestInstallGalleryExtensionTask(aGalleryExtension('a', { isSigned: false }), anExtensionsDownloader({ isSignatureVerificationEnabled: true, verificationResult: true }), disposables.add(new DisposableStore()));
 
  await testObject.run();
 
@@ -207,15 +205,15 @@ suite('InstallGalleryExtensionTask Tests', () => {
  });
 
  test('task completes for an unsigned extension even when signature verification throws error', async () => {
- const testObject = new TestInstallGalleryExtensionTask(aGalleryExtension('a', { isSigned: false }), anExtensionsDownloader({ isSignatureVerificationEnabled: true, verificationResult: 'error', didExecute: true }), disposables.add(new DisposableStore()));
+ const testObject = new TestInstallGalleryExtensionTask(aGalleryExtension('a', { isSigned: false }), anExtensionsDownloader({ isSignatureVerificationEnabled: true, verificationResult: 'error' }), disposables.add(new DisposableStore()));
 
  await testObject.run();
 
  assert.strictEqual(testObject.verificationStatus, false);
  assert.strictEqual(testObject.installed, true);
  });
 
- function anExtensionsDownloader(options: { isSignatureVerificationEnabled: boolean; verificationResult: boolean | string; didExecute: boolean; quality?: string }): ExtensionsDownloader {
+ function anExtensionsDownloader(options: { isSignatureVerificationEnabled: boolean; verificationResult: boolean | string; quality?: string }): ExtensionsDownloader {
  const logService = new NullLogService();
  const fileService = disposables.add(new FileService(logService));
  const fileSystemProvider = disposables.add(new InMemoryFileSystemProvider());
@@ -235,7 +233,7 @@ suite('InstallGalleryExtensionTask Tests', () => {
  },
  });
  instantiationService.stub(IConfigurationService, new TestConfigurationService(isBoolean(options.isSignatureVerificationEnabled) ? { extensions: { verifySignature: options.isSignatureVerificationEnabled } } : undefined));
- instantiationService.stub(IExtensionSignatureVerificationService, new TestExtensionSignatureVerificationService(options.verificationResult, !!options.didExecute));
+ instantiationService.stub(IExtensionSignatureVerificationService, new TestExtensionSignatureVerificationService(options.verificationResult));
  return disposables.add(instantiationService.createInstance(ExtensionsDownloader));
  }