Skip to content

Commit

Permalink
fix: use stricter options in SecStaticCodeCheckValidity (#33379)
Browse files Browse the repository at this point in the history 
* fix: use stricter options in SecStaticCodeCheckValidity

* Update patches/squirrel.mac/fix_use_kseccschecknestedcode_kseccsstrictvalidate_in_the_sec.patch

Co-authored-by: John Kleinschmidt <jkleinsc@electronjs.org>

Co-authored-by: Samuel Attard <samuel.r.attard@gmail.com>
Co-authored-by: Samuel Attard <sam@electronjs.org>
Co-authored-by: John Kleinschmidt <jkleinsc@electronjs.org>
  • Loading branch information
@trop @MarshallOfSound @jkleinsc
4 people committed Mar 22, 2022
1 parent f983099 commit 68292f0
Show file tree
Hide file tree
Showing 3 changed files with 153 additions and 6 deletions.
1 change: 1 addition & 0 deletions patches/squirrel.mac/.patches
View file
@@ -1,2 +1,3 @@
build_add_gn_config.patch
fix_ensure_that_self_is_retained_until_the_racsignal_is_complete.patch
fix_use_kseccschecknestedcode_kseccsstrictvalidate_in_the_sec.patch
21 changes: 21 additions & 0 deletions patches/squirrel.mac/fix_use_kseccschecknestedcode_kseccsstrictvalidate_in_the_sec.patch
View file
@@ -0,0 +1,21 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Samuel Attard <samuel.r.attard@gmail.com>
Date: Mon, 21 Mar 2022 15:14:19 -0700
Subject: fix: use kSecCSCheckNestedCode | kSecCSStrictValidate in the Sec
validate call

This ensures that Squirrel.Mac validates the nested bundles (nested Frameworks) including the incoming Squirrel.Mac framework.

diff --git a/Squirrel/SQRLCodeSignature.m b/Squirrel/SQRLCodeSignature.m
index f8754dbd6a1490d2b50f1014e2daa5c1f71b2103..2f5e27c1ae5c5bd514abe33d4cd42c4724656c07 100644
--- a/Squirrel/SQRLCodeSignature.m
+++ b/Squirrel/SQRLCodeSignature.m
@@ -124,7 +124,7 @@ - (RACSignal *)verifyBundleAtURL:(NSURL *)bundleURL {
}

CFErrorRef validityError = NULL;
- result = SecStaticCodeCheckValidityWithErrors(staticCode, kSecCSCheckAllArchitectures, (__bridge SecRequirementRef)self.requirement, &validityError);
+ result = SecStaticCodeCheckValidityWithErrors(staticCode, kSecCSCheckNestedCode | kSecCSStrictValidate | kSecCSCheckAllArchitectures, (__bridge SecRequirementRef)self.requirement, &validityError);
@onExit {
if (validityError != NULL) CFRelease(validityError);
};
137 changes: 131 additions & 6 deletions spec-main/api-autoupdater-darwin-spec.ts
View file
Expand Up @@ -6,14 +6,14 @@ import * as fs from 'fs-extra';
import * as os from 'os';
import * as path from 'path';
import { AddressInfo } from 'net';
import { ifdescribe } from './spec-helpers';
import { ifdescribe, ifit } from './spec-helpers';

const features = process._linkedBinding('electron_common_features');

const fixturesPath = path.resolve(__dirname, 'fixtures');

// We can only test the auto updater on darwin non-component builds
ifdescribe(process.platform === 'darwin' && process.arch !== 'arm64' && !process.mas && !features.isComponentBuild())('autoUpdater behavior', function () {
ifdescribe(process.platform === 'darwin' && !(process.env.CI && process.arch === 'arm64') && !process.mas && !features.isComponentBuild())('autoUpdater behavior', function () {
this.timeout(120000);

let identity = '';
Expand Down Expand Up @@ -115,8 +115,11 @@ ifdescribe(process.platform === 'darwin' && process.arch !== 'arm64' && !process

const cachedZips: Record<string, string> = {};

const getOrCreateUpdateZipPath = async (version: string, fixture: string) => {
const key = `${version}-${fixture}`;
const getOrCreateUpdateZipPath = async (version: string, fixture: string, mutateAppPostSign?: {
mutate: (appPath: string) => Promise<void>,
mutationKey: string,
}) => {
const key = `${version}-${fixture}-${mutateAppPostSign?.mutationKey || 'no-mutation'}`;
if (!cachedZips[key]) {
let updateZipPath: string;
await withTempDirectory(async (dir) => {
Expand All @@ -127,6 +130,7 @@ ifdescribe(process.platform === 'darwin' && process.arch !== 'arm64' && !process
(await fs.readFile(appPJPath, 'utf8')).replace('1.0.0', version)
);
await signApp(secondAppPath);
await mutateAppPostSign?.mutate(secondAppPath);
updateZipPath = path.resolve(dir, 'update.zip');
await spawn('zip', ['-r', '--symlinks', updateZipPath, './'], {
cwd: dir
Expand All @@ -143,10 +147,12 @@ ifdescribe(process.platform === 'darwin' && process.arch !== 'arm64' && !process
}
});

it('should fail to set the feed URL when the app is not signed', async () => {
// On arm64 builds the built app is self-signed by default so the setFeedURL call always works
ifit(process.arch !== 'arm64')('should fail to set the feed URL when the app is not signed', async () => {
await withTempDirectory(async (dir) => {
const appPath = await copyApp(dir);
const launchResult = await launchApp(appPath, ['http://myupdate']);
console.log(launchResult);
expect(launchResult.code).to.equal(1);
expect(launchResult.out).to.include('Could not get code signature for running application');
});
Expand Down Expand Up @@ -259,12 +265,16 @@ ifdescribe(process.platform === 'darwin' && process.arch !== 'arm64' && !process
nextVersion: string;
startFixture: string;
endFixture: string;
mutateAppPostSign?: {
mutate: (appPath: string) => Promise<void>,
mutationKey: string,
}
}, fn: (appPath: string, zipPath: string) => Promise<void>) => {
await withTempDirectory(async (dir) => {
const appPath = await copyApp(dir, opts.startFixture);
await signApp(appPath);

const updateZipPath = await getOrCreateUpdateZipPath(opts.nextVersion, opts.endFixture);
const updateZipPath = await getOrCreateUpdateZipPath(opts.nextVersion, opts.endFixture, opts.mutateAppPostSign);

await fn(appPath, updateZipPath);
});
Expand Down Expand Up @@ -311,6 +321,121 @@ ifdescribe(process.platform === 'darwin' && process.arch !== 'arm64' && !process
});
});

it('should hit the download endpoint when an update is available and fail when the zip signature is invalid', async () => {
await withUpdatableApp({
nextVersion: '2.0.0',
startFixture: 'update',
endFixture: 'update',
mutateAppPostSign: {
mutationKey: 'add-resource',
mutate: async (appPath) => {
const resourcesPath = path.resolve(appPath, 'Contents', 'Resources', 'app', 'injected.txt');
await fs.writeFile(resourcesPath, 'demo');
}
}
}, async (appPath, updateZipPath) => {
server.get('/update-file', (req, res) => {
res.download(updateZipPath);
});
server.get('/update-check', (req, res) => {
res.json({
url: `http://localhost:${port}/update-file`,
name: 'My Release Name',
notes: 'Theses are some release notes innit',
pub_date: (new Date()).toString()
});
});
const launchResult = await launchApp(appPath, [`http://localhost:${port}/update-check`]);
logOnError(launchResult, () => {
expect(launchResult).to.have.property('code', 1);
expect(launchResult.out).to.include('Code signature at URL');
expect(launchResult.out).to.include('a sealed resource is missing or invalid');
expect(requests).to.have.lengthOf(2);
expect(requests[0]).to.have.property('url', '/update-check');
expect(requests[1]).to.have.property('url', '/update-file');
expect(requests[0].header('user-agent')).to.include('Electron/');
expect(requests[1].header('user-agent')).to.include('Electron/');
});
});
});

it('should hit the download endpoint when an update is available and fail when the ShipIt binary is a symlink', async () => {
await withUpdatableApp({
nextVersion: '2.0.0',
startFixture: 'update',
endFixture: 'update',
mutateAppPostSign: {
mutationKey: 'modify-shipit',
mutate: async (appPath) => {
const shipItPath = path.resolve(appPath, 'Contents', 'Frameworks', 'Squirrel.framework', 'Resources', 'ShipIt');
await fs.remove(shipItPath);
await fs.symlink('/tmp/ShipIt', shipItPath, 'file');
}
}
}, async (appPath, updateZipPath) => {
server.get('/update-file', (req, res) => {
res.download(updateZipPath);
});
server.get('/update-check', (req, res) => {
res.json({
url: `http://localhost:${port}/update-file`,
name: 'My Release Name',
notes: 'Theses are some release notes innit',
pub_date: (new Date()).toString()
});
});
const launchResult = await launchApp(appPath, [`http://localhost:${port}/update-check`]);
logOnError(launchResult, () => {
expect(launchResult).to.have.property('code', 1);
expect(launchResult.out).to.include('Code signature at URL');
expect(launchResult.out).to.include('a sealed resource is missing or invalid');
expect(requests).to.have.lengthOf(2);
expect(requests[0]).to.have.property('url', '/update-check');
expect(requests[1]).to.have.property('url', '/update-file');
expect(requests[0].header('user-agent')).to.include('Electron/');
expect(requests[1].header('user-agent')).to.include('Electron/');
});
});
});

it('should hit the download endpoint when an update is available and fail when the Electron Framework is modified', async () => {
await withUpdatableApp({
nextVersion: '2.0.0',
startFixture: 'update',
endFixture: 'update',
mutateAppPostSign: {
mutationKey: 'modify-eframework',
mutate: async (appPath) => {
const shipItPath = path.resolve(appPath, 'Contents', 'Frameworks', 'Electron Framework.framework', 'Electron Framework');
await fs.appendFile(shipItPath, Buffer.from('123'));
}
}
}, async (appPath, updateZipPath) => {
server.get('/update-file', (req, res) => {
res.download(updateZipPath);
});
server.get('/update-check', (req, res) => {
res.json({
url: `http://localhost:${port}/update-file`,
name: 'My Release Name',
notes: 'Theses are some release notes innit',
pub_date: (new Date()).toString()
});
});
const launchResult = await launchApp(appPath, [`http://localhost:${port}/update-check`]);
logOnError(launchResult, () => {
expect(launchResult).to.have.property('code', 1);
expect(launchResult.out).to.include('Code signature at URL');
expect(launchResult.out).to.include(' main executable failed strict validation');
expect(requests).to.have.lengthOf(2);
expect(requests[0]).to.have.property('url', '/update-check');
expect(requests[1]).to.have.property('url', '/update-file');
expect(requests[0].header('user-agent')).to.include('Electron/');
expect(requests[1].header('user-agent')).to.include('Electron/');
});
});
});

it('should hit the download endpoint when an update is available and update successfully when the zip is provided with JSON update mode', async () => {
await withUpdatableApp({
nextVersion: '2.0.0',
Expand Down

0 comments on commit 68292f0

Please sign in to comment.