Invalid signing of Sparkle (or apps including it)

[leo-hydraulic]

Steps to reproduce:

1. curl -L -O https://github.com/sparkle-project/Sparkle/releases/download/2.4.1/Sparkle-2.4.1.tar.xz
2. tar xzvf Sparkle-2.4.1.tar.xz
3. rcodesign sign Sparkle.framework
4. codesign -v Sparkle.framework

Expected: No output (Sparkle gets correctly signed.) Actual: Sparkle.framework: a sealed resource is missing or invalid

This comment seems to suggest that rcodesign will skip sealing nested app bundles inside frameworks, which seems to be what's missing in the signed contents produced by the commands above.

[leo-hydraulic]

It seems that Apple's codesign treats directories as nested bundles if: 1) It matches a "nested" rule; 2) It contains a "." in the name.

From their source code, method ResourceBuilder::scan: https://opensource.apple.com/source/Security/Security-57740.1.18/OSX/libsecurity_codesigning/lib/resources.cpp.auto.html

[indygreg]

I looked at this issue a little bit today.

When signing an app bundle with Sparkle.framework, I see rcodesign recurse into the Autoupdate.app bundle. e.g. Sparkle.framework/Versions/A/Resources/Autoupdate.app when signing the Sparkle.framework/Versions/A bundle. That's obviously wrong: Autoupdate.app should be signed as a bundle and its signature sealed into Sparkle.framework.

You pointed me to code where we clearly see logic where it looks for a nested rule having a . in the name. What I'm still wrapping my head around is which rules2 entry / rule is actually triggering this.

In rcodesign, the following rule is being used for Sparkle's Autoupdate.app:

<key>^Resources/</key>
<dict>
  <key>weight</key>
  <real>20</real>
</dict>

The only nested rules we have are:

<key>^(Frameworks|SharedFrameworks|PlugIns|Plug-ins|XPCServices|Helpers|MacOS|Library/(Automator|Spotlight|LoginItems))/</key>
<dict>
    <key>nested</key>
    <true/>
    <key>weight</key>
    <real>10</real>
</dict>
...
<key>^[^/]+$</key>
<dict>
    <key>nested</key>
    <true/>
    <key>weight</key>
    <real>10</real>
</dict>

What I fail to understand here is how Apple's signer is arriving at a nested rule for paths Resources/Autoupdate.app/*.

The sorting rules for rules is:

1. Find a rule that matches pattern
2. If an exclusion rule use it
3. Collect all matching rules and take one with highest weight

Assuming the evaluation path is e.g. Sparkle.framework/Versions/A, then Sparkle.framework/Versions/A/Resources/Autoupdate.app/* will always match the ^Resources/ rule.

Now, I noticed the code you linked to only applies this rule for directories. rcodesign actually doesn't apply these rules to directories: only symlinks and files. I'm starting to question if we should change this. But even if we did, I'm not seeing how we'd ever hit the nested rule.

As code comments in bundle_signing.rs highlight, I don't have high confidence that we're doing nested bundle signing correctly. We're accumulating various code hacks to try to coerce things into working. But I want to say that if we implement rules processing correctly we shouldn't need any of this and we can use rules for everything. So the question is, which parts of rules processing aren't we implementing correctly? I'm just not seeing how we trigger a nested rule.

[leo-hydraulic]

Which version of Sparkle did you observe this in? On our tests we've been using Sparkle-2.4.1, which has only Sparkle.framework/Versions/B and doesn't have an Autoupdate.app in Resources. On top of that, are you positive that Apple's codesign will detect it as a nested bundle if you sign it with the --deep option? I don't think it would.

[leo-hydraulic]

Looking a it further, perhaps the issue is that rcodesign doesn't apply the rules when computing nested bundles under directory_bundle.rs (DirectoryBundle.nested_bundles)? It just uses the DirectoryBundle.new_from_path constructor to detect "bundleness".

[leo-hydraulic]

Another issue in code_resources.rs: process_nested_bundle is signing bundles that match regular file rules, it should only sign the ones that match nested rules.

[leo-hydraulic]

Or being more precise: even if those "stray" bundles are signed, they shouldn't be sealed as a bundle (I just verified that Apple's codesign doesn't). Only bundles in paths that match nested rules get sealed with cdhash and requirements. For the others, everything gets included as regular resources, even the _CodeSignature/CodeResources file.

[indygreg]

I think the root problem here is I never really understood what the nested flag did. The lack of meaningful comments about it in code_resources.rs are the giveaway. rcodesign grew a bit of code to work around that fact and we've been signing nested bundles incorrectly ever since.

I suspect the best way out of this is to effectively rewrite our pile of code for iterating bundles and to rewrite it in terms of following the resources rules and their metadata rather than invent custom heuristics for finding bundles.

[indygreg]

I believe this is fixed on the main branch. (I rewrote bundle traversal and shored up support for recursive signing of Mach-O binaries in bundles.)

Please reopen if this is still a problem. Please provide a modern link to a Sparkle framework or app including it that fails with the main branch.