Browse Source

fix: use stricter options in SecStaticCodeCheckValidity (#33368)

* fix: use stricter options in SecStaticCodeCheckValidity

* Update patches/squirrel.mac/fix_use_kseccschecknestedcode_kseccsstrictvalidate_in_the_sec.patch

Co-authored-by: John Kleinschmidt <[email protected]>

Co-authored-by: John Kleinschmidt <[email protected]>
Samuel Attard 3 years ago
parent
commit
956406a193

+ 1 - 0
patches/squirrel.mac/.patches

@@ -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 - 0
patches/squirrel.mac/fix_use_kseccschecknestedcode_kseccsstrictvalidate_in_the_sec.patch

@@ -0,0 +1,21 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Samuel Attard <[email protected]>
+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);
+ 		};

+ 131 - 6
spec-main/api-autoupdater-darwin-spec.ts

@@ -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 = '';
@@ -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) => {
@@ -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
@@ -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');
     });
@@ -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);
       });
@@ -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',