Skip to content

Commit f921bd8

Browse files
sandy081sandy081
authored
log extension signature verification output on success (microsoft#210596)
1 parent 330d143 commit f921bd8

File tree

5 files changed

+104
-69
lines changed

5 files changed

+104
-69
lines changed

‎package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
{
22
"name": "code-oss-dev",
33
"version": "1.89.0",
4-
"distro": "25df1b5602ba46d6b0f618dd227963d1163c83df",
4+
"distro": "853f475a6e8e85ad811a2f8e49c3032bdb660558",
55
"author": {
66
"name": "Microsoft Corporation"
77
},

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

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -431,12 +431,6 @@ export enum ExtensionManagementErrorCode {
431431
Internal = 'Internal',
432432
}
433433

434-
export enum ExtensionSignaturetErrorCode {
435-
UnknownError = 'UnknownError',
436-
PackageIsInvalidZip = 'PackageIsInvalidZip',
437-
SignatureArchiveIsInvalidZip = 'SignatureArchiveIsInvalidZip',
438-
}
439-
440434
export class ExtensionManagementError extends Error {
441435
constructor(message: string, readonly code: ExtensionManagementErrorCode) {
442436
super(message);

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

Lines changed: 5 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,11 @@ import { CorruptZipMessage } from 'vs/base/node/zip';
1717
import { IConfigurationService } from 'vs/platform/configuration/common/configuration';
1818
import { INativeEnvironmentService } from 'vs/platform/environment/common/environment';
1919
import { ExtensionVerificationStatus } from 'vs/platform/extensionManagement/common/abstractExtensionManagementService';
20-
import { ExtensionManagementError, ExtensionManagementErrorCode, ExtensionSignaturetErrorCode, IExtensionGalleryService, IGalleryExtension, InstallOperation } from 'vs/platform/extensionManagement/common/extensionManagement';
20+
import { ExtensionManagementError, ExtensionManagementErrorCode, IExtensionGalleryService, IGalleryExtension, InstallOperation } from 'vs/platform/extensionManagement/common/extensionManagement';
2121
import { ExtensionKey, groupByExtension } from 'vs/platform/extensionManagement/common/extensionManagementUtil';
22-
import { ExtensionSignatureVerificationError, IExtensionSignatureVerificationService } from 'vs/platform/extensionManagement/node/extensionSignatureVerificationService';
22+
import { ExtensionSignatureVerificationError, ExtensionSignatureVerificationCode, IExtensionSignatureVerificationService } from 'vs/platform/extensionManagement/node/extensionSignatureVerificationService';
2323
import { IFileService, IFileStatWithMetadata } from 'vs/platform/files/common/files';
24-
import { ILogService, LogLevel } from 'vs/platform/log/common/log';
24+
import { ILogService } from 'vs/platform/log/common/log';
2525

2626
export class ExtensionsDownloader extends Disposable {
2727

@@ -60,14 +60,11 @@ export class ExtensionsDownloader extends Disposable {
6060
if (verifySignature && this.shouldVerifySignature(extension)) {
6161
const signatureArchiveLocation = await this.downloadSignatureArchive(extension);
6262
try {
63-
verificationStatus = await this.extensionSignatureVerificationService.verify(location.fsPath, signatureArchiveLocation.fsPath, this.logService.getLevel() === LogLevel.Trace);
63+
verificationStatus = await this.extensionSignatureVerificationService.verify(extension.identifier.id, location.fsPath, signatureArchiveLocation.fsPath);
6464
} catch (error) {
6565
const sigError = error as ExtensionSignatureVerificationError;
6666
verificationStatus = sigError.code;
67-
if (sigError.output) {
68-
this.logService.trace(`Extension signature verification details for ${extension.identifier.id} ${extension.version}:\n${sigError.output}`);
69-
}
70-
if (verificationStatus === ExtensionSignaturetErrorCode.PackageIsInvalidZip || verificationStatus === ExtensionSignaturetErrorCode.SignatureArchiveIsInvalidZip) {
67+
if (verificationStatus === ExtensionSignatureVerificationCode.PackageIsInvalidZip || verificationStatus === ExtensionSignatureVerificationCode.SignatureArchiveIsInvalidZip) {
7168
try {
7269
// Delete the downloaded vsix before throwing the error
7370
await this.delete(location);
@@ -86,14 +83,6 @@ export class ExtensionsDownloader extends Disposable {
8683
}
8784
}
8885

89-
if (verificationStatus === true) {
90-
this.logService.info(`Extension signature is verified: ${extension.identifier.id}`);
91-
} else if (verificationStatus === false) {
92-
this.logService.info(`Extension signature verification is not done: ${extension.identifier.id}`);
93-
} else {
94-
this.logService.warn(`Extension signature verification failed with error '${verificationStatus}': ${extension.identifier.id}`);
95-
}
96-
9786
return { location, verificationStatus };
9887
}
9988

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

Lines changed: 86 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55

66
import { getErrorMessage } from 'vs/base/common/errors';
77
import { createDecorator } from 'vs/platform/instantiation/common/instantiation';
8-
import { ILogService } from 'vs/platform/log/common/log';
8+
import { ILogService, LogLevel } from 'vs/platform/log/common/log';
99
import { ITelemetryService } from 'vs/platform/telemetry/common/telemetry';
1010

1111
export const IExtensionSignatureVerificationService = createDecorator<IExtensionSignatureVerificationService>('IExtensionSignatureVerificationService');
@@ -18,30 +18,69 @@ export interface IExtensionSignatureVerificationService {
1818

1919
/**
2020
* Verifies an extension file (.vsix) against a signature archive file.
21+
* @param { string } extensionId The extension identifier.
2122
* @param { string } vsixFilePath The extension file path.
2223
* @param { string } signatureArchiveFilePath The signature archive file path.
23-
* @param { boolean } verbose A flag indicating whether or not to capture verbose detail in the event of an error.
2424
* @returns { Promise<boolean> } A promise with `true` if the extension is validly signed and trusted;
2525
* otherwise, `false` because verification is not enabled (e.g.: in the OSS version of VS Code).
2626
* @throws { ExtensionSignatureVerificationError } An error with a code indicating the validity, integrity, or trust issue
2727
* found during verification or a more fundamental issue (e.g.: a required dependency was not found).
2828
*/
29-
verify(vsixFilePath: string, signatureArchiveFilePath: string, verbose: boolean): Promise<boolean>;
29+
verify(extensionId: string, vsixFilePath: string, signatureArchiveFilePath: string): Promise<boolean>;
3030
}
3131

3232
declare module vsceSign {
33-
export function verify(vsixFilePath: string, signatureArchiveFilePath: string, verbose: boolean): Promise<boolean>;
33+
export function verify(vsixFilePath: string, signatureArchiveFilePath: string, verbose: boolean): Promise<ExtensionSignatureVerificationResult>;
34+
}
35+
36+
export const enum ExtensionSignatureVerificationCode {
37+
'None' = 'None',
38+
'RequiredArgumentMissing' = 'RequiredArgumentMissing',
39+
'InvalidArgument' = 'InvalidArgument',
40+
'PackageIsUnreadable' = 'PackageIsUnreadable',
41+
'UnhandledException' = 'UnhandledException',
42+
'SignatureManifestIsMissing' = 'SignatureManifestIsMissing',
43+
'SignatureManifestIsUnreadable' = 'SignatureManifestIsUnreadable',
44+
'SignatureIsMissing' = 'SignatureIsMissing',
45+
'SignatureIsUnreadable' = 'SignatureIsUnreadable',
46+
'CertificateIsUnreadable' = 'CertificateIsUnreadable',
47+
'SignatureArchiveIsUnreadable' = 'SignatureArchiveIsUnreadable',
48+
'FileAlreadyExists' = 'FileAlreadyExists',
49+
'SignatureArchiveIsInvalidZip' = 'SignatureArchiveIsInvalidZip',
50+
'SignatureArchiveHasSameSignatureFile' = 'SignatureArchiveHasSameSignatureFile',
51+
52+
'Success' = 'Success',
53+
'PackageIntegrityCheckFailed' = 'PackageIntegrityCheckFailed',
54+
'SignatureIsInvalid' = 'SignatureIsInvalid',
55+
'SignatureManifestIsInvalid' = 'SignatureManifestIsInvalid',
56+
'SignatureIntegrityCheckFailed' = 'SignatureIntegrityCheckFailed',
57+
'EntryIsMissing' = 'EntryIsMissing',
58+
'EntryIsTampered' = 'EntryIsTampered',
59+
'Untrusted' = 'Untrusted',
60+
'CertificateRevoked' = 'CertificateRevoked',
61+
'SignatureIsNotValid' = 'SignatureIsNotValid',
62+
'UnknownError' = 'UnknownError',
63+
'PackageIsInvalidZip' = 'PackageIsInvalidZip',
64+
'SignatureArchiveHasTooManyEntries' = 'SignatureArchiveHasTooManyEntries',
3465
}
3566

3667
/**
37-
* An error raised during extension signature verification.
68+
* Extension signature verification result
3869
*/
39-
export interface ExtensionSignatureVerificationError extends Error {
40-
readonly code: string;
70+
export interface ExtensionSignatureVerificationResult {
71+
readonly code: ExtensionSignatureVerificationCode;
4172
readonly didExecute: boolean;
4273
readonly output?: string;
4374
}
4475

76+
export class ExtensionSignatureVerificationError extends Error {
77+
constructor(
78+
public readonly code: ExtensionSignatureVerificationCode,
79+
) {
80+
super(code);
81+
}
82+
}
83+
4584
export class ExtensionSignatureVerificationService implements IExtensionSignatureVerificationService {
4685
declare readonly _serviceBrand: undefined;
4786

@@ -67,45 +106,60 @@ export class ExtensionSignatureVerificationService implements IExtensionSignatur
67106
return this.moduleLoadingPromise;
68107
}
69108

70-
public async verify(vsixFilePath: string, signatureArchiveFilePath: string, verbose: boolean): Promise<boolean> {
109+
public async verify(extensionId: string, vsixFilePath: string, signatureArchiveFilePath: string): Promise<boolean> {
71110
let module: typeof vsceSign;
72111

73112
try {
74113
module = await this.vsceSign();
75114
} catch (error) {
76115
this.logService.error('Could not load vsce-sign module', getErrorMessage(error));
116+
this.logService.info(`Extension signature verification is not done: ${extensionId}`);
77117
return false;
78118
}
79119

80120
const startTime = new Date().getTime();
81-
let verified: boolean | undefined;
82-
let error: ExtensionSignatureVerificationError | undefined;
121+
let result: ExtensionSignatureVerificationResult;
83122

84123
try {
85-
verified = await module.verify(vsixFilePath, signatureArchiveFilePath, verbose);
86-
return verified;
124+
result = await module.verify(vsixFilePath, signatureArchiveFilePath, this.logService.getLevel() === LogLevel.Trace);
87125
} catch (e) {
88-
error = e;
89-
throw e;
90-
} finally {
91-
const duration = new Date().getTime() - startTime;
92-
type ExtensionSignatureVerificationClassification = {
93-
owner: 'sandy081';
94-
comment: 'Extension signature verification event';
95-
duration: { classification: 'SystemMetaData'; purpose: 'PerformanceAndHealth'; 'isMeasurement': true; comment: 'amount of time taken to verify the signature' };
96-
verified?: { classification: 'SystemMetaData'; purpose: 'FeatureInsight'; comment: 'verified status when succeeded' };
97-
error?: { classification: 'SystemMetaData'; purpose: 'FeatureInsight'; comment: 'error code when failed' };
98-
};
99-
type ExtensionSignatureVerificationEvent = {
100-
duration: number;
101-
verified?: boolean;
102-
error?: string;
126+
result = {
127+
code: ExtensionSignatureVerificationCode.UnknownError,
128+
didExecute: false,
129+
output: getErrorMessage(e)
103130
};
104-
this.telemetryService.publicLog2<ExtensionSignatureVerificationEvent, ExtensionSignatureVerificationClassification>('extensionsignature:verification', {
105-
duration,
106-
verified,
107-
error: error ? (error.code ?? 'unknown') : undefined,
108-
});
109131
}
132+
133+
const duration = new Date().getTime() - startTime;
134+
135+
this.logService.info(`Extension signature verification result for ${extensionId}: ${result.code}. Duration: ${duration}ms.`);
136+
this.logService.trace(`Extension signature verification output for ${extensionId}:\n${result.output}`);
137+
138+
type ExtensionSignatureVerificationClassification = {
139+
owner: 'sandy081';
140+
comment: 'Extension signature verification event';
141+
extensionId: { classification: 'SystemMetaData'; purpose: 'FeatureInsight'; comment: 'extension identifier' };
142+
code: { classification: 'SystemMetaData'; purpose: 'FeatureInsight'; comment: 'result code of the verification' };
143+
duration: { classification: 'SystemMetaData'; purpose: 'PerformanceAndHealth'; 'isMeasurement': true; comment: 'amount of time taken to verify the signature' };
144+
didExecute: { classification: 'SystemMetaData'; purpose: 'FeatureInsight'; comment: 'whether the verification was executed' };
145+
};
146+
type ExtensionSignatureVerificationEvent = {
147+
extensionId: string;
148+
code: string;
149+
duration: number;
150+
didExecute: boolean;
151+
};
152+
this.telemetryService.publicLog2<ExtensionSignatureVerificationEvent, ExtensionSignatureVerificationClassification>('extensionsignature:verification', {
153+
extensionId,
154+
code: result.code,
155+
duration,
156+
didExecute: result.didExecute
157+
});
158+
159+
if (result.code === ExtensionSignatureVerificationCode.Success) {
160+
return true;
161+
}
162+
163+
throw new ExtensionSignatureVerificationError(result.code);
110164
}
111165
}

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

Lines changed: 12 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,7 @@ class TestExtensionsScanner extends mock<ExtensionsScanner>() {
4848
class TestExtensionSignatureVerificationService extends mock<IExtensionSignatureVerificationService>() {
4949

5050
constructor(
51-
private readonly verificationResult: string | boolean,
52-
private readonly didExecute: boolean) {
51+
private readonly verificationResult: string | boolean) {
5352
super();
5453
}
5554

@@ -59,7 +58,6 @@ class TestExtensionSignatureVerificationService extends mock<IExtensionSignature
5958
}
6059
const error = Error(this.verificationResult);
6160
(error as any).code = this.verificationResult;
62-
(error as any).didExecute = this.didExecute;
6361
throw error;
6462
}
6563
}
@@ -132,7 +130,7 @@ suite('InstallGalleryExtensionTask Tests', () => {
132130
const disposables = ensureNoDisposablesAreLeakedInTestSuite();
133131

134132
test('if verification is enabled by default, the task completes', async () => {
135-
const testObject = new TestInstallGalleryExtensionTask(aGalleryExtension('a', { isSigned: true }), anExtensionsDownloader({ isSignatureVerificationEnabled: true, verificationResult: true, didExecute: true }), disposables.add(new DisposableStore()));
133+
const testObject = new TestInstallGalleryExtensionTask(aGalleryExtension('a', { isSigned: true }), anExtensionsDownloader({ isSignatureVerificationEnabled: true, verificationResult: true }), disposables.add(new DisposableStore()));
136134

137135
await testObject.run();
138136

@@ -141,7 +139,7 @@ suite('InstallGalleryExtensionTask Tests', () => {
141139
});
142140

143141
test('if verification is enabled in stable, the task completes', async () => {
144-
const testObject = new TestInstallGalleryExtensionTask(aGalleryExtension('a', { isSigned: true }), anExtensionsDownloader({ isSignatureVerificationEnabled: true, verificationResult: true, didExecute: true, quality: 'stable' }), disposables.add(new DisposableStore()));
142+
const testObject = new TestInstallGalleryExtensionTask(aGalleryExtension('a', { isSigned: true }), anExtensionsDownloader({ isSignatureVerificationEnabled: true, verificationResult: true, quality: 'stable' }), disposables.add(new DisposableStore()));
145143

146144
await testObject.run();
147145

@@ -150,7 +148,7 @@ suite('InstallGalleryExtensionTask Tests', () => {
150148
});
151149

152150
test('if verification is disabled by setting set to false, the task skips verification', async () => {
153-
const testObject = new TestInstallGalleryExtensionTask(aGalleryExtension('a', { isSigned: true }), anExtensionsDownloader({ isSignatureVerificationEnabled: false, verificationResult: 'error', didExecute: false }), disposables.add(new DisposableStore()));
151+
const testObject = new TestInstallGalleryExtensionTask(aGalleryExtension('a', { isSigned: true }), anExtensionsDownloader({ isSignatureVerificationEnabled: false, verificationResult: 'error' }), disposables.add(new DisposableStore()));
154152

155153
await testObject.run();
156154

@@ -159,7 +157,7 @@ suite('InstallGalleryExtensionTask Tests', () => {
159157
});
160158

161159
test('if verification is disabled because the module is not loaded, the task skips verification', async () => {
162-
const testObject = new TestInstallGalleryExtensionTask(aGalleryExtension('a', { isSigned: true }), anExtensionsDownloader({ isSignatureVerificationEnabled: true, verificationResult: false, didExecute: false }), disposables.add(new DisposableStore()));
160+
const testObject = new TestInstallGalleryExtensionTask(aGalleryExtension('a', { isSigned: true }), anExtensionsDownloader({ isSignatureVerificationEnabled: true, verificationResult: false }), disposables.add(new DisposableStore()));
163161

164162
await testObject.run();
165163

@@ -169,7 +167,7 @@ suite('InstallGalleryExtensionTask Tests', () => {
169167

170168
test('if verification fails to execute, the task completes', async () => {
171169
const errorCode = 'ENOENT';
172-
const testObject = new TestInstallGalleryExtensionTask(aGalleryExtension('a', { isSigned: true }), anExtensionsDownloader({ isSignatureVerificationEnabled: true, verificationResult: errorCode, didExecute: false }), disposables.add(new DisposableStore()));
170+
const testObject = new TestInstallGalleryExtensionTask(aGalleryExtension('a', { isSigned: true }), anExtensionsDownloader({ isSignatureVerificationEnabled: true, verificationResult: errorCode }), disposables.add(new DisposableStore()));
173171

174172
await testObject.run();
175173

@@ -180,7 +178,7 @@ suite('InstallGalleryExtensionTask Tests', () => {
180178
test('if verification fails', async () => {
181179
const errorCode = 'IntegrityCheckFailed';
182180

183-
const testObject = new TestInstallGalleryExtensionTask(aGalleryExtension('a', { isSigned: true }), anExtensionsDownloader({ isSignatureVerificationEnabled: true, verificationResult: errorCode, didExecute: true }), disposables.add(new DisposableStore()));
181+
const testObject = new TestInstallGalleryExtensionTask(aGalleryExtension('a', { isSigned: true }), anExtensionsDownloader({ isSignatureVerificationEnabled: true, verificationResult: errorCode }), disposables.add(new DisposableStore()));
184182

185183
await testObject.run();
186184

@@ -189,7 +187,7 @@ suite('InstallGalleryExtensionTask Tests', () => {
189187
});
190188

191189
test('if verification succeeds, the task completes', async () => {
192-
const testObject = new TestInstallGalleryExtensionTask(aGalleryExtension('a', { isSigned: true }), anExtensionsDownloader({ isSignatureVerificationEnabled: true, verificationResult: true, didExecute: true }), disposables.add(new DisposableStore()));
190+
const testObject = new TestInstallGalleryExtensionTask(aGalleryExtension('a', { isSigned: true }), anExtensionsDownloader({ isSignatureVerificationEnabled: true, verificationResult: true }), disposables.add(new DisposableStore()));
193191

194192
await testObject.run();
195193

@@ -198,7 +196,7 @@ suite('InstallGalleryExtensionTask Tests', () => {
198196
});
199197

200198
test('task completes for unsigned extension', async () => {
201-
const testObject = new TestInstallGalleryExtensionTask(aGalleryExtension('a', { isSigned: false }), anExtensionsDownloader({ isSignatureVerificationEnabled: true, verificationResult: true, didExecute: false }), disposables.add(new DisposableStore()));
199+
const testObject = new TestInstallGalleryExtensionTask(aGalleryExtension('a', { isSigned: false }), anExtensionsDownloader({ isSignatureVerificationEnabled: true, verificationResult: true }), disposables.add(new DisposableStore()));
202200

203201
await testObject.run();
204202

@@ -207,15 +205,15 @@ suite('InstallGalleryExtensionTask Tests', () => {
207205
});
208206

209207
test('task completes for an unsigned extension even when signature verification throws error', async () => {
210-
const testObject = new TestInstallGalleryExtensionTask(aGalleryExtension('a', { isSigned: false }), anExtensionsDownloader({ isSignatureVerificationEnabled: true, verificationResult: 'error', didExecute: true }), disposables.add(new DisposableStore()));
208+
const testObject = new TestInstallGalleryExtensionTask(aGalleryExtension('a', { isSigned: false }), anExtensionsDownloader({ isSignatureVerificationEnabled: true, verificationResult: 'error' }), disposables.add(new DisposableStore()));
211209

212210
await testObject.run();
213211

214212
assert.strictEqual(testObject.verificationStatus, false);
215213
assert.strictEqual(testObject.installed, true);
216214
});
217215

218-
function anExtensionsDownloader(options: { isSignatureVerificationEnabled: boolean; verificationResult: boolean | string; didExecute: boolean; quality?: string }): ExtensionsDownloader {
216+
function anExtensionsDownloader(options: { isSignatureVerificationEnabled: boolean; verificationResult: boolean | string; quality?: string }): ExtensionsDownloader {
219217
const logService = new NullLogService();
220218
const fileService = disposables.add(new FileService(logService));
221219
const fileSystemProvider = disposables.add(new InMemoryFileSystemProvider());
@@ -235,7 +233,7 @@ suite('InstallGalleryExtensionTask Tests', () => {
235233
},
236234
});
237235
instantiationService.stub(IConfigurationService, new TestConfigurationService(isBoolean(options.isSignatureVerificationEnabled) ? { extensions: { verifySignature: options.isSignatureVerificationEnabled } } : undefined));
238-
instantiationService.stub(IExtensionSignatureVerificationService, new TestExtensionSignatureVerificationService(options.verificationResult, !!options.didExecute));
236+
instantiationService.stub(IExtensionSignatureVerificationService, new TestExtensionSignatureVerificationService(options.verificationResult));
239237
return disposables.add(instantiationService.createInstance(ExtensionsDownloader));
240238
}
241239

0 commit comments

Comments
 (0)