From 66771d32e04ba1719cc0f0a81d480ddfdc9f237c Mon Sep 17 00:00:00 2001 From: MariaDima Date: Tue, 6 Jun 2017 13:30:42 +0200 Subject: [PATCH] feat(electron-updater): ensure that update only to the application signed with same cert Close #1187 --- docs/Options.md | 1 + docs/api/electron-updater.md | 21 ---- .../electron-builder-http/src/httpExecutor.ts | 5 +- .../src/options/winOptions.ts | 8 ++ .../src/publish/PublishManager.ts | 9 +- packages/electron-builder/src/winPackager.ts | 4 + packages/electron-updater/src/NsisUpdater.ts | 75 ++++++++++++- test/jestSetup.js | 2 + .../out/__snapshots__/nsisUpdaterTest.js.snap | 65 ++++++++++- test/src/nsisUpdaterTest.ts | 103 +++++++++--------- test/typings/jest-ex.d.ts | 1 + 11 files changed, 207 insertions(+), 87 deletions(-) diff --git a/docs/Options.md b/docs/Options.md index e692990e608..626a5488e5c 100644 --- a/docs/Options.md +++ b/docs/Options.md @@ -567,6 +567,7 @@ Windows Specific Options ([win](#Config-win)). | rfc3161TimeStampServer| string | The URL of the RFC 3161 time stamp server. Defaults to `http://timestamp.comodoca.com/rfc3161`. | | timeStampServer| string | The URL of the time stamp server. Defaults to `http://timestamp.verisign.com/scripts/timstamp.dll`. | | publisherName| string \| Array<string> \| null | [The publisher name](https://github.com/electron-userland/electron-builder/issues/1187#issuecomment-278972073), exactly as in your code signed certificate. Several names can be provided. Defaults to common name from your code signing certificate. | +| forceCodeSigningVerification = true| boolean | Whether to verify the signature of an available update before installation. The [publisher name](WinBuildOptions#publisherName) will be used for the signature verification. | diff --git a/docs/api/electron-updater.md b/docs/api/electron-updater.md index 24abf2fc2c8..9288cbef0e8 100644 --- a/docs/api/electron-updater.md +++ b/docs/api/electron-updater.md @@ -224,18 +224,12 @@ Developer API only. See [[Auto Update]] for user documentation. * [electron-updater/out/NsisUpdater](#module_electron-updater/out/NsisUpdater) * [.NsisUpdater](#NsisUpdater) ⇐ [AppUpdater](Auto-Update#AppUpdater) * [`.quitAndInstall(isSilent)`](#module_electron-updater/out/NsisUpdater.NsisUpdater+quitAndInstall) - * [`.doDownloadUpdate(versionInfo, fileInfo, cancellationToken)`](#module_electron-updater/out/NsisUpdater.NsisUpdater+doDownloadUpdate) ⇒ Promise<string> ### NsisUpdater ⇐ [AppUpdater](Auto-Update#AppUpdater) **Kind**: class of [electron-updater/out/NsisUpdater](#module_electron-updater/out/NsisUpdater) **Extends**: [AppUpdater](Auto-Update#AppUpdater) - -* [.NsisUpdater](#NsisUpdater) ⇐ [AppUpdater](Auto-Update#AppUpdater) - * [`.quitAndInstall(isSilent)`](#module_electron-updater/out/NsisUpdater.NsisUpdater+quitAndInstall) - * [`.doDownloadUpdate(versionInfo, fileInfo, cancellationToken)`](#module_electron-updater/out/NsisUpdater.NsisUpdater+doDownloadUpdate) ⇒ Promise<string> - #### `nsisUpdater.quitAndInstall(isSilent)` @@ -245,21 +239,6 @@ Developer API only. See [[Auto Update]] for user documentation. | --- | --- | | isSilent | boolean | - - -#### `nsisUpdater.doDownloadUpdate(versionInfo, fileInfo, cancellationToken)` ⇒ Promise<string> -Start downloading update manually. You can use this method if `autoDownload` option is set to `false`. - -**Kind**: instance method of [NsisUpdater](#NsisUpdater) -**Returns**: Promise<string> - Path to downloaded file. -**Access**: protected - -| Param | Type | -| --- | --- | -| versionInfo | [VersionInfo](Publishing-Artifacts#VersionInfo) | -| fileInfo | [FileInfo](Auto-Update#FileInfo) | -| cancellationToken | [CancellationToken](electron-builder-http#CancellationToken) | - ## electron-updater/out/PrivateGitHubProvider diff --git a/packages/electron-builder-http/src/httpExecutor.ts b/packages/electron-builder-http/src/httpExecutor.ts index ef70b92e4c4..d2a0297174b 100644 --- a/packages/electron-builder-http/src/httpExecutor.ts +++ b/packages/electron-builder-http/src/httpExecutor.ts @@ -229,8 +229,9 @@ function configurePipes(options: DownloadOptions, response: any, destination: st } } - if (options.sha512 != null) { - streams.push(new DigestTransform(options.sha512, "sha512", "base64")) + const sha512 = options.sha512 + if (sha512 != null) { + streams.push(new DigestTransform(sha512, "sha512", sha512.length === 128 && !sha512.includes("+") && !sha512.includes("Z") && !sha512.includes("=") ? "hex" : "base64")) } else if (options.sha2 != null) { streams.push(new DigestTransform(options.sha2, "sha256", "hex")) diff --git a/packages/electron-builder/src/options/winOptions.ts b/packages/electron-builder/src/options/winOptions.ts index e277d516dea..839df748684 100644 --- a/packages/electron-builder/src/options/winOptions.ts +++ b/packages/electron-builder/src/options/winOptions.ts @@ -72,6 +72,14 @@ export interface WinBuildOptions extends PlatformSpecificBuildOptions { * Defaults to common name from your code signing certificate. */ readonly publisherName?: string | Array | null + + /** + * Whether to verify the signature of an available update before installation. + * The [publisher name](WinBuildOptions#publisherName) will be used for the signature verification. + * + * @default true + */ + readonly forceCodeSigningVerification?: boolean } export interface CommonNsisOptions { diff --git a/packages/electron-builder/src/publish/PublishManager.ts b/packages/electron-builder/src/publish/PublishManager.ts index 27bc40dd581..772a802af4c 100644 --- a/packages/electron-builder/src/publish/PublishManager.ts +++ b/packages/electron-builder/src/publish/PublishManager.ts @@ -91,9 +91,12 @@ export class PublishManager implements PublishContext { let publishConfig = publishConfigs[0] if (packager.platform === Platform.WINDOWS) { - const publisherName = await (packager).computedPublisherName.value - if (publisherName != null) { - publishConfig = Object.assign({publisherName: publisherName}, publishConfig) + const winPackager = packager + if (winPackager.isForceCodeSigningVerification) { + const publisherName = await winPackager.computedPublisherName.value + if (publisherName != null) { + publishConfig = Object.assign({publisherName: publisherName}, publishConfig) + } } } diff --git a/packages/electron-builder/src/winPackager.ts b/packages/electron-builder/src/winPackager.ts index 6bcfa1aea4c..0c3af299657 100644 --- a/packages/electron-builder/src/winPackager.ts +++ b/packages/electron-builder/src/winPackager.ts @@ -82,6 +82,10 @@ export class WinPackager extends PlatformPackager { return publisherName == null ? null : asArray(publisherName) }) + get isForceCodeSigningVerification(): boolean { + return this.platformSpecificBuildOptions.forceCodeSigningVerification !== false + } + constructor(info: BuildInfo) { super(info) } diff --git a/packages/electron-updater/src/NsisUpdater.ts b/packages/electron-updater/src/NsisUpdater.ts index 9ccae722adf..be90773d0b7 100644 --- a/packages/electron-updater/src/NsisUpdater.ts +++ b/packages/electron-updater/src/NsisUpdater.ts @@ -1,4 +1,5 @@ -import { spawn } from "child_process" +import BluebirdPromise from "bluebird-lst" +import { execFile, spawn } from "child_process" import { DownloadOptions } from "electron-builder-http" import { CancellationError, CancellationToken } from "electron-builder-http/out/CancellationToken" import { PublishConfiguration, VersionInfo } from "electron-builder-http/out/publishOptions" @@ -18,10 +19,7 @@ export class NsisUpdater extends AppUpdater { super(options, app) } - /** - * Start downloading update manually. You can use this method if `autoDownload` option is set to `false`. - * @returns {Promise} Path to downloaded file. - */ + /*** @private */ protected async doDownloadUpdate(versionInfo: VersionInfo, fileInfo: FileInfo, cancellationToken: CancellationToken) { const downloadOptions: DownloadOptions = { skipDirCreation: true, @@ -57,6 +55,17 @@ export class NsisUpdater extends AppUpdater { throw e } + const signatureVerificationStatus = await this.verifySignature(tempFile) + if (signatureVerificationStatus != null) { + try { + await remove(tempDir) + } + finally { + // noinspection ThrowInsideFinallyBlockJS + throw new Error(`New version ${this.versionInfo!.version} is not signed by the application owner: ${signatureVerificationStatus}`) + } + } + if (logger != null) { logger.info(`New version ${this.versionInfo!.version} has been downloaded to ${tempFile}`) } @@ -67,6 +76,62 @@ export class NsisUpdater extends AppUpdater { return tempFile } + // $certificateInfo = (Get-AuthenticodeSignature 'xxx\yyy.exe' + // | where {$_.Status.Equals([System.Management.Automation.SignatureStatus]::Valid) -and $_.SignerCertificate.Subject.Contains("CN=siemens.com")}) + // | Out-String ; if ($certificateInfo) { exit 0 } else { exit 1 } + private async verifySignature(tempUpdateFile: string): Promise { + const updateConfig = await this.loadUpdateConfig() + const publisherName = updateConfig.publisherName + if (publisherName == null) { + return null + } + + return await new BluebirdPromise((resolve, reject) => { + const commonNameConstraint = (Array.isArray(publisherName) ? >publisherName : [publisherName]).map(it => `$_.SignerCertificate.Subject.Contains('CN=${it},')`).join(" -or ") + const constraintCommand = `where {$_.Status.Equals([System.Management.Automation.SignatureStatus]::Valid) -and (${commonNameConstraint})}` + const verifySignatureCommand = `Get-AuthenticodeSignature '${tempUpdateFile}' | ${constraintCommand}` + const powershellChild = spawn("powershell.exe", [(`$certificateInfo = (${verifySignatureCommand}) | Out-String ; if ($certificateInfo) { exit 0 } else { exit 1 }`)]) + powershellChild.on("error", reject) + powershellChild.on("exit", code => { + if (code !== 1) { + resolve(null) + return + } + + execFile("powershell.exe", [`Get-AuthenticodeSignature '${tempUpdateFile}' | ConvertTo-Json -Compress`], {maxBuffer: 4 * 1024000}, (error, stdout, stderr) => { + if (error != null) { + reject(error) + return + } + + if (stderr) { + reject(new Error(`Cannot execute Get-AuthenticodeSignature: ${stderr}`)) + return + } + + const data = JSON.parse(stdout) + delete data.PrivateKey + delete data.IsOSBinary + delete data.SignatureType + const signerCertificate = data.SignerCertificate + if (signerCertificate != null) { + delete signerCertificate.Archived + delete signerCertificate.Extensions + delete signerCertificate.Handle + delete signerCertificate.HasPrivateKey + } + delete data.Path + + const result = JSON.stringify(data, (name, value) => name === "RawData" ? undefined : value, 2) + if (this.logger != null) { + this.logger.info(`Sign verification failed, installer signed with incorrect certificate: ${result}`) + } + resolve(result) + }) + }) + }) + } + private addQuitHandler() { if (this.quitHandlerAdded) { return diff --git a/test/jestSetup.js b/test/jestSetup.js index 70dd47e29da..e84bb2b391c 100644 --- a/test/jestSetup.js +++ b/test/jestSetup.js @@ -20,9 +20,11 @@ skip.ifAll = skip const isMac = process.platform === "darwin" test.ifMac = isMac ? test : skip test.ifNotWindows = isWindows ? skip : test +test.ifWindows = isWindows ? test : skip skip.ifMac = skip skip.ifLinux = skip +skip.ifWindows = skip skip.ifNotWindows = skip skip.ifCi = skip skip.ifNotCi = skip diff --git a/test/out/__snapshots__/nsisUpdaterTest.js.snap b/test/out/__snapshots__/nsisUpdaterTest.js.snap index 691dde5bb5d..42a335dafe7 100644 --- a/test/out/__snapshots__/nsisUpdaterTest.js.snap +++ b/test/out/__snapshots__/nsisUpdaterTest.js.snap @@ -77,12 +77,20 @@ Array [ exports[`file url github 1`] = ` Object { "name": "TestApp-Setup-1.1.0.exe", - "sha2": "f2ca1bb6c7e907d06dafe4687e579fce76b37e4e93b7605022da52e6ccc26fd2", - "sha512": undefined, + "sha2": undefined, + "sha512": "xrTrW8dzWYlPnu71Y4lpLIAuIurBZJvZmqEZyz1rzM3CbbE1Z+T+P5qYYZgwmhmXdYPOpvnmYKa0HGdgXggwtQ==", "url": "https://github.com/develar/__test_nsis_release/releases/download/v1.1.0/TestApp-Setup-1.1.0.exe", } `; +exports[`file url github 2`] = ` +Array [ + "checking-for-update", + "update-available", + "update-downloaded", +] +`; + exports[`file url github pre-release 1`] = ` Object { "name": "TestApp-Setup-1.5.2-beta.3.exe", @@ -93,6 +101,14 @@ Object { `; exports[`file url github pre-release 2`] = ` +Array [ + "checking-for-update", + "update-available", + "update-downloaded", +] +`; + +exports[`file url github pre-release 3`] = ` Object { "path": "TestApp Setup 1.5.2-beta.3.exe", "releaseName": "v1.5.2-beta.3", @@ -114,6 +130,51 @@ Object { } `; +exports[`invalid signature 1`] = ` +"New version 1.1.0 is not signed by the application owner: { + \\"SignerCertificate\\": { + \\"FriendlyName\\": \\"\\", + \\"IssuerName\\": { + \\"Name\\": \\"CN=StartCom Class 2 Object CA, OU=StartCom Certification Authority, O=StartCom Ltd., C=IL\\", + \\"Oid\\": \\"System.Security.Cryptography.Oid\\" + }, + \\"NotAfter\\": \\"/Date(1516526235000)/\\", + \\"NotBefore\\": \\"/Date(1453367835000)/\\", + \\"PrivateKey\\": null, + \\"PublicKey\\": { + \\"Key\\": \\"System.Security.Cryptography.RSACryptoServiceProvider\\", + \\"Oid\\": \\"System.Security.Cryptography.Oid\\", + \\"EncodedKeyValue\\": \\"System.Security.Cryptography.AsnEncodedData\\", + \\"EncodedParameters\\": \\"System.Security.Cryptography.AsnEncodedData\\" + }, + \\"SerialNumber\\": \\"18CB5EC53FB14EC2DBB44BD1518AF901\\", + \\"SubjectName\\": { + \\"Name\\": \\"CN=Vladimir Krivosheev, O=Vladimir Krivosheev, L=Grunwald, S=Bayern, C=DE\\", + \\"Oid\\": \\"System.Security.Cryptography.Oid\\" + }, + \\"SignatureAlgorithm\\": { + \\"Value\\": \\"1.2.840.113549.1.1.11\\", + \\"FriendlyName\\": \\"sha256RSA\\" + }, + \\"Thumbprint\\": \\"32F67D8F957E740C692ADD8CD5A5E463992193DD\\", + \\"Version\\": 3, + \\"Issuer\\": \\"CN=StartCom Class 2 Object CA, OU=StartCom Certification Authority, O=StartCom Ltd., C=IL\\", + \\"Subject\\": \\"CN=Vladimir Krivosheev, O=Vladimir Krivosheev, L=Grunwald, S=Bayern, C=DE\\" + }, + \\"TimeStamperCertificate\\": null, + \\"Status\\": 0, + \\"StatusMessage\\": \\"Signature verified.\\" +}" +`; + +exports[`invalid signature 2`] = ` +Array [ + "checking-for-update", + "update-available", + "error", +] +`; + exports[`sha512 mismatch error event 1`] = ` Object { "name": "TestApp Setup 1.1.0.exe", diff --git a/test/src/nsisUpdaterTest.ts b/test/src/nsisUpdaterTest.ts index a4680d0908e..1f18941fa8f 100644 --- a/test/src/nsisUpdaterTest.ts +++ b/test/src/nsisUpdaterTest.ts @@ -206,76 +206,46 @@ test("checkForUpdates several times", async () => { test("file url github", async () => { const updater = new NsisUpdater() updater.updateConfigPath = await writeUpdateConfig({ - provider: "github", - owner: "develar", - repo: "__test_nsis_release", - }) - tuneNsisUpdater(updater) - - const actualEvents: Array = [] - const expectedEvents = ["checking-for-update", "update-available", "update-downloaded"] - for (const eventName of expectedEvents) { - updater.addListener(eventName, () => { - actualEvents.push(eventName) - }) - } - - const updateCheckResult = await updater.checkForUpdates() - expect(updateCheckResult.fileInfo).toMatchSnapshot() - await assertThat(path.join(await updateCheckResult.downloadPromise)).isFile() - - expect(actualEvents).toEqual(expectedEvents) + provider: "github", + owner: "develar", + repo: "__test_nsis_release", + }) + await validateDownload(updater) }) test("file url github pre-release", async () => { const updater = new NsisUpdater(null, createTestApp("1.5.0-beta.1")) updater.updateConfigPath = await writeUpdateConfig({ - provider: "github", - owner: "develar", - repo: "__test_nsis_release", - }) - tuneNsisUpdater(updater) - - const actualEvents: Array = [] - const expectedEvents = ["checking-for-update", "update-available", "update-downloaded"] - for (const eventName of expectedEvents) { - updater.addListener(eventName, () => { - actualEvents.push(eventName) - }) - } + provider: "github", + owner: "develar", + repo: "__test_nsis_release", + }) - const updateCheckResult = await updater.checkForUpdates() - expect(updateCheckResult.fileInfo).toMatchSnapshot() + const updateCheckResult = await validateDownload(updater) expect(updateCheckResult.versionInfo).toMatchSnapshot() - - await assertThat(path.join(await updateCheckResult.downloadPromise)).isFile() - - expect(actualEvents).toEqual(expectedEvents) }) test.skip("file url github private", async () => { const updater = new NsisUpdater() updater.updateConfigPath = await writeUpdateConfig({ - provider: "github", - owner: "develar", - repo: "__test_nsis_release_private", - }) - tuneNsisUpdater(updater) + provider: "github", + owner: "develar", + repo: "__test_nsis_release_private", + }) + await validateDownload(updater) +}) - const actualEvents: Array = [] - const expectedEvents = ["checking-for-update", "update-available", "update-downloaded"] - for (const eventName of expectedEvents) { - updater.addListener(eventName, () => { - actualEvents.push(eventName) - }) - } +async function validateDownload(updater: NsisUpdater) { + tuneNsisUpdater(updater) + const actualEvents = trackEvents(updater) const updateCheckResult = await updater.checkForUpdates() expect(updateCheckResult.fileInfo).toMatchSnapshot() await assertThat(path.join(await updateCheckResult.downloadPromise)).isFile() - expect(actualEvents).toEqual(expectedEvents) -}) + expect(actualEvents).toMatchSnapshot() + return updateCheckResult +} test("test error", async () => { const updater: NsisUpdater = new NsisUpdater() @@ -290,7 +260,7 @@ test("test download progress", async () => { const updater = new NsisUpdater() updater.updateConfigPath = await writeUpdateConfig({ provider: "generic", - url: "https://develar.s3.amazonaws.com/test", + url: "https://develar.s3.amazonaws.com/test" }) tuneNsisUpdater(updater) updater.autoDownload = false @@ -311,6 +281,31 @@ test("test download progress", async () => { expect(lastEvent.transferred).toBe(lastEvent.total) }) +test.ifAll.ifWindows("valid signature", async () => { + const updater = new NsisUpdater() + updater.updateConfigPath = await writeUpdateConfig({ + provider: "github", + owner: "develar", + repo: "__test_nsis_release", + publisherName: ["Vladimir Krivosheev"], + }) + await validateDownload(updater) +}) + +test.ifAll.ifWindows("invalid signature", async () => { + const updater = new NsisUpdater() + updater.updateConfigPath = await writeUpdateConfig({ + provider: "github", + owner: "develar", + repo: "__test_nsis_release", + publisherName: ["Foo Bar"], + }) + tuneNsisUpdater(updater) + const actualEvents = trackEvents(updater) + await assertThat(updater.checkForUpdates().then(it => it.downloadPromise)).throws() + expect(actualEvents).toMatchSnapshot() +}) + test("cancel download with progress", async () => { const updater = new NsisUpdater() updater.updateConfigPath = await writeUpdateConfig({ @@ -341,7 +336,7 @@ test("cancel download with progress", async () => { expect(cancelled).toBe(true) }) -async function writeUpdateConfig(data: GenericServerOptions | GithubOptions | BintrayOptions): Promise { +async function writeUpdateConfig(data: GenericServerOptions | GithubOptions | BintrayOptions | any): Promise { const updateConfigPath = path.join(await tmpDir.getTempFile("update-config"), "app-update.yml") await outputFile(updateConfigPath, safeDump(data)) return updateConfigPath diff --git a/test/typings/jest-ex.d.ts b/test/typings/jest-ex.d.ts index f66dfde7132..39aafb69c57 100644 --- a/test/typings/jest-ex.d.ts +++ b/test/typings/jest-ex.d.ts @@ -2,6 +2,7 @@ declare module jest { interface It { ifNotWindows: jest.It ifMac: jest.It + ifWindows: jest.It ifNotCi: jest.It ifCi: jest.It ifNotCiMac: jest.It