Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ASAR files in extraResources are not included in integrity calculations #8660

Closed
t3chguy opened this issue Nov 4, 2024 · 14 comments · Fixed by #8805
Closed

ASAR files in extraResources are not included in integrity calculations #8660

t3chguy opened this issue Nov 4, 2024 · 14 comments · Fixed by #8805

Comments

@t3chguy
Copy link
Contributor

t3chguy commented Nov 4, 2024

  • Electron-Builder Version: 25.1.8
  • Node Version: v23.1.0
  • Electron Version: ^33.0.0
  • Electron Type (current, beta, nightly): current
  • Target: macOS & Windows

electron-builder does a seemingly fine job supporting EnableEmbeddedAsarIntegrityValidation unless your extraResources contains additional asar files. The files are not considered in the ASAR integrity resolution at https://github.com/electron-userland/electron-builder/blob/master/packages/app-builder-lib/src/platformPackager.ts#L305-L310 as the extraResources are not copied until https://github.com/electron-userland/electron-builder/blob/master/packages/app-builder-lib/src/platformPackager.ts#L318

Empirically the computeData function is never called for mac universal packages so the fix there would need to be different.

Copy link
Contributor

github-actions bot commented Jan 4, 2025

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 30 days.

@github-actions github-actions bot added the Stale label Jan 4, 2025
@t3chguy
Copy link
Contributor Author

t3chguy commented Jan 5, 2025

Still an issue AFAICT

@github-actions github-actions bot removed the Stale label Jan 6, 2025
@mmaietta
Copy link
Collaborator

I migrated electron-builder to electron/asar that now handles calculating file integrity. Can you please try next tag of electron-builder?

@t3chguy
Copy link
Contributor Author

t3chguy commented Jan 22, 2025

That unfortunately does not seem to have resolved the issue

 ~/D/macos  ./Element.app/Contents/MacOS/Element --profile TTTT                                                                                            Wed 22 Jan 13:31:14 2025
/Users/t3chguy/Library/Application Support/Element-TTTT exists: no
/Users/t3chguy/Library/Application Support/Riot-TTTT exists: no
[51380:0122/133125.917907:FATAL:archive.cc(239)] Failed to get integrity for validatable asar archive: Resources/webapp.asar

the Info.plist still only contains the main app.asar

    <key>ElectronAsarIntegrity</key>
    <dict>  
      <key>Resources/app.asar</key>
      <dict>
        <key>algorithm</key>
        <string>SHA256</string>
        <key>hash</key>  
        <string>18c2a952ac84bd95c5310ecd2639c90cd86c118720429c4fe99f1411de852050</string>
      </dict>
    </dict>

The issue looks to be that https://github.com/electron-userland/electron-builder/blob/master/packages/app-builder-lib/src/platformPackager.ts#L318 happens after computeData on https://github.com/electron-userland/electron-builder/blob/master/packages/app-builder-lib/src/platformPackager.ts#L308 so additional resources are not considered in the asar calculation.

@mmaietta
Copy link
Collaborator

ElectronAsarIntegrity is only for asar as the name suggests. Unpacked files are also calculated in the asar integrity IIRC as they are tracked internally within the asar header.
Extra resources (within the context of electron-builder) on the other hand are supplemental to the asar and exist outside an asar's domain boundary.

What are these?

~/D/macos  ./Element.app/Contents/MacOS/Element --profile TTTT                                                                                            

/Users/t3chguy/Library/Application Support/Element-TTTT exists: no
/Users/t3chguy/Library/Application Support/Riot-TTTT exists: no
[51380:0122/133125.917907:FATAL:archive.cc(239)] Failed to get integrity for validatable asar archive: Resources/webapp.asar

Can you provide a sample repo for this that I could debug deeper with? AFAICT currently, there is no issue here per the reasoning I gave above.

@t3chguy
Copy link
Contributor Author

t3chguy commented Jan 22, 2025

ElectronAsarIntegrity is only for asar as the name suggests.

Indeed, but one can have multiple asars. The only rule with asar integrity is that the electron main process code lives in app.asar. ASAR integrity checks affect all loaded ASARs though, not just app.asar.

I've dug a bit deeper and there's another slightly different issue for macOS Universal builds due to the Info.plist being mangled by @electron/universal around https://github.com/electron/universal/blob/caa0567b76bc06d0014e6fc5afb6d70d47651971/src/index.ts#L364-L366 - in theory could be fixed on this side using infoPlistToIgnore - have opened electron/universal#116 for that side of things

The non-universal issue with additional asars added via resources can be fixed via a patch akin to

Subject: [PATCH] Remove stale handler for `extend-info`
---
Index: packages/app-builder-lib/src/asar/integrity.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/packages/app-builder-lib/src/asar/integrity.ts b/packages/app-builder-lib/src/asar/integrity.ts
--- a/packages/app-builder-lib/src/asar/integrity.ts	(revision 443ee8debd4e4d73d9f63a31fb93ce1dbed2538c)
+++ b/packages/app-builder-lib/src/asar/integrity.ts	(date 1737562196873)
@@ -1,13 +1,17 @@
 import BluebirdPromise from "bluebird-lst"
+import { FilterStats, statOrNull, walk } from "builder-util"
 import { createHash } from "crypto"
 import { createReadStream } from "fs"
 import { readdir } from "fs/promises"
 import * as path from "path"
+import { FileMatcher } from "../fileMatcher"
 import { readAsarHeader, NodeIntegrity } from "./asar"
 
 export interface AsarIntegrityOptions {
   readonly resourcesPath: string
+  readonly resourcesFinalPath: string
   readonly resourcesRelativePath: string
+  readonly extraResourceMatchers: Array<FileMatcher> | null
 }
 
 export interface HeaderHash {
@@ -19,12 +23,48 @@
   [key: string]: HeaderHash
 }
 
-export async function computeData({ resourcesPath, resourcesRelativePath }: AsarIntegrityOptions): Promise<AsarIntegrity> {
+const asarFilter = (file: string) => {
+  return file.endsWith(".asar")
+}
+
+export async function computeData({ resourcesPath, resourcesRelativePath, resourcesFinalPath, extraResourceMatchers }: AsarIntegrityOptions): Promise<AsarIntegrity> {
   // sort to produce constant result
-  const names = (await readdir(resourcesPath)).filter(it => it.endsWith(".asar")).sort()
+  const names = (await readdir(resourcesPath)).filter(asarFilter).sort()
   const checksums = await BluebirdPromise.map(names, it => hashHeader(path.join(resourcesPath, it)))
 
   const result: AsarIntegrity = {}
+
+  if (extraResourceMatchers != null && extraResourceMatchers.length > 0) {
+    const matches = (
+      await BluebirdPromise.map(extraResourceMatchers, async (matcher: FileMatcher): Promise<[realPath: string, targetPath: string][]> => {
+        const fromStat = await statOrNull(matcher.from)
+        if (fromStat == null) {
+          return []
+        }
+
+        if (fromStat.isFile()) {
+          if (asarFilter(matcher.from)) {
+            return [[matcher.from, matcher.to] as const]
+          }
+          return []
+        }
+
+        if (matcher.isEmpty() || matcher.containsOnlyIgnore()) {
+          matcher.prependPattern("**/*")
+        }
+        const matcherFilter = matcher.createFilter()
+        const results = await walk(matcher.from, (file: string, stats: FilterStats) => matcherFilter(file, stats) && asarFilter(file))
+        return results.map(it => [it, matcher.to] as const)
+      })
+    ).flat(1)
+
+    await BluebirdPromise.each(matches, async ([realPath, targetPath]) => {
+      const prefix = path.relative(resourcesFinalPath, targetPath)
+      const key = path.join(resourcesRelativePath, prefix, path.basename(realPath))
+      result[key] = await hashHeader(realPath)
+    })
+  }
+
   for (let i = 0; i < names.length; i++) {
     result[path.join(resourcesRelativePath, names[i])] = checksums[i]
   }
Index: packages/app-builder-lib/src/platformPackager.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/packages/app-builder-lib/src/platformPackager.ts b/packages/app-builder-lib/src/platformPackager.ts
--- a/packages/app-builder-lib/src/platformPackager.ts	(revision 443ee8debd4e4d73d9f63a31fb93ce1dbed2538c)
+++ b/packages/app-builder-lib/src/platformPackager.ts	(date 1737562196907)
@@ -301,11 +301,12 @@
 
     if (framework.beforeCopyExtraFiles != null) {
       const resourcesRelativePath = this.platform === Platform.MAC ? "Resources" : isElectronBased(framework) ? "resources" : ""
+      const resourcesFinalPath = this.getResourcesDir(appOutDir)
 
       await framework.beforeCopyExtraFiles({
         packager: this,
         appOutDir,
-        asarIntegrity: asarOptions == null || disableAsarIntegrity ? null : await computeData({ resourcesPath, resourcesRelativePath }),
+        asarIntegrity: asarOptions == null || disableAsarIntegrity ? null : await computeData({ resourcesPath, resourcesFinalPath, resourcesRelativePath, extraResourceMatchers }),
         platformName,
       })
     }

With this patch, the Windows INTEGRITY ELECTRONASAR value is [{"file":"resources\\webapp.asar","alg":"SHA256","value":"151c87674deb1f9e0ad252fa19b388a5c40d5b26241fd5de203e9937a5dccb28"},{"file":"resources\\app.asar","alg":"SHA256","value":"fa4820d2a6867f59b74467bd0bf7717f3a98ebec6e82738c0cf79aa3f5da2273"}] as expected.

Alternatively, would be cleaner if one could generate the asar integrity after copying resources, but that seemed like a more invasive change.

Can you provide a sample repo for this that I could debug deeper with? AFAICT currently, there is no issue here per the reasoning I gave above.

I can but definitely not right now, spent too much time digging into this already.
The fundamentals are thus, we have app.asar holding the Electron main process code.
webapp.asar holding the (effectively) standalone webapp code, which is loaded via protocol.registerFileProtocol pointing at paths within that asar. If only one asar signature is provided then this explodes at runtime.

[7727:0122/164306.527723:FATAL:archive.cc(239)] Failed to get integrity for validatable asar archive: Resources/webapp.asar

@mmaietta
Copy link
Collaborator

This is brilliant documentation, thank you so much. I'll take a look at this later tonight or at least this week.

@t3chguy
Copy link
Contributor Author

t3chguy commented Jan 22, 2025

I managed to come up with a user-land workeraround for macOS Universal packages, will tackle Windows tomorrow

    afterPack: async (context: AfterPackContext) => {
        const packager = context.packager;

        if (packager.platform === Platform.LINUX) return; // no ASAR integrity support on Linux
        if (packager.platform === Platform.MAC && context.arch !== Arch.universal) return; // We only need to generate asar on universal Mac builds

        const framework = packager.info.framework;
        const resourcesPath = packager.getResourcesDir(context.appOutDir);
        const resourcesRelativePath =
            packager.platform === Platform.MAC ? "Resources" : isElectronBased(framework) ? "resources" : "";

        const integrity = await computeData({
            resourcesPath,
            resourcesRelativePath,
        });

        if (packager.platform === Platform.WINDOWS) {
			// TODO
        } else if (packager.platform === Platform.MAC) {
            const plistPath = path.join(resourcesPath, "..", "Info.plist");
            const data = plist.parse(await readFile(plistPath, "utf8")) as unknown as Writable<plist.PlistObject>;
            data["ElectronAsarIntegrity"] = integrity as unknown as Writable<plist.PlistValue>;
            await writeFile(plistPath, plist.build(data));
        }
    },

fortunately most of the guts of app-builder-lib responsible were exported

@t3chguy
Copy link
Contributor Author

t3chguy commented Jan 23, 2025

Managed a workaround for Windows & macOS in element-hq/element-desktop#1979 - I call the following in afterPack

async function injectAsarIntegrity(context: AfterPackContext) {
    const packager = context.packager;

    if (packager.platform === Platform.LINUX) return; // no ASAR integrity support on Linux
    if (packager.platform === Platform.MAC && context.arch !== Arch.universal) return; // We only need to generate asar on universal Mac builds

    const framework = packager.info.framework;
    const resourcesPath = packager.getResourcesDir(context.appOutDir);
    const resourcesRelativePath =
        packager.platform === Platform.MAC ? "Resources" : isElectronBased(framework) ? "resources" : "";

    const asarIntegrity = await computeData({
        resourcesPath,
        resourcesRelativePath,
    });

    if (packager.platform === Platform.WINDOWS) {
        const executablePath = path.join(context.appOutDir, `${packager.appInfo.productFilename}.exe`);

        const buffer = await readFile(executablePath);
        const executable = NtExecutable.from(buffer);
        const resource = NtExecutableResource.from(executable);

        const integrityList = Array.from(Object.entries(asarIntegrity)).map(
            ([file, { algorithm: alg, hash: value }]) => ({
                file: path.win32.normalize(file),
                alg,
                value,
            }),
        );

        // We edit the resource that electron-builder already wrote to ensure it includes all asar files
        const electronAsarResource = resource.entries.find((entry) => entry.id === "ELECTRONASAR")!;
        electronAsarResource.bin = Buffer.from(JSON.stringify(integrityList));

        resource.outputResource(executable);

        await writeFile(executablePath, Buffer.from(executable.generate()));
    } else if (packager.platform === Platform.MAC) {
        const plistPath = path.join(resourcesPath, "..", "Info.plist");
        const data = plist.parse(await readFile(plistPath, "utf8")) as unknown as Writable<plist.PlistObject>;
        data["ElectronAsarIntegrity"] = asarIntegrity as unknown as Writable<plist.PlistValue>;
        await writeFile(plistPath, plist.build(data));
    }
}

@mmaietta
Copy link
Collaborator

This is fantastic and definitely wasn't a use case (multiple asar's) I was aware of during implementation.

I'll try and work on a PR (with you CC'd for credit) this weekend unless you'd like to open one before then.

@t3chguy
Copy link
Contributor Author

t3chguy commented Jan 23, 2025

I'd prefer to defer to you on doing it in the most appropriate way for the project, my patch in #8660 (comment) feels awful and if you can refactor to do asar integrity after copying extraResources it'd be a lot cleaner but it wasn't an avenue I was comfortable in without fear of breaking everything else

@mmaietta
Copy link
Collaborator

Quick update, implementation is done, just trying to set up some unit tests for it now

@mmaietta
Copy link
Collaborator

@t3chguy released in next tag v26.0.0, I've exited alpha (finally). Please let me know if you run into any issues

@t3chguy
Copy link
Contributor Author

t3chguy commented Jan 27, 2025

@mmaietta amazing work, did hit one issue though unfortunately: #8812 - seems to be running happily otherwise! (though am still needing to re-calculate asar integrity for macOS universal builds in afterPack due to electron/universal#116 )

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants