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

feat: Traverse archives #231

Closed
wants to merge 7 commits into from
Closed

feat: Traverse archives #231

wants to merge 7 commits into from

Conversation

sethlu
Copy link
Contributor

@sethlu sethlu commented Jul 17, 2020

Resolves #229

@b-zurg
Copy link

b-zurg commented Jul 19, 2020

I just tried this out, my latest run didn't have the previous issues but did have this come up during notarization:

{
  "logFormatVersion": 1,
  "jobId": "22d77294-edb5-493c-9790-3e4afca0c58d",
  "status": "Invalid",
  "statusSummary": "Archive contains critical validation errors",
  "statusCode": 4000,
  "archiveFilename": "myApp.zip",
  "uploadDate": "2020-07-19T08:03:15Z",
  "sha256": "e418a864cea75096b9ddeb085e96c828c4f855279c051db0446cbb00352b969d",
  "ticketContents": null,
  "issues": [
    {
      "severity": "error",
      "code": null,
      "path": "myApp.zip/myApp.app/Contents/MacOS/myApp-app",
      "message": "The binary is not signed.",
      "docUrl": null,
      "architecture": "x86_64"
    },
    {
      "severity": "error",
      "code": null,
      "path": "myApp.zip/myApp.app/Contents/MacOS/myApp-app",
      "message": "The signature does not include a secure timestamp.",
      "docUrl": null,
      "architecture": "x86_64"
    },
    {
      "severity": "error",
      "code": null,
      "path": "myApp.zip/myApp.app/Contents/MacOS/myApp-app",
      "message": "The executable does not have the hardened runtime enabled.",
      "docUrl": null,
      "architecture": "x86_64"
    }
  ]
}

It seems everything but the final executable is being signed? I'm not sure.

Another thing to note, I think this option, though really useful in certain circumstances, is quite expensive. This last run added a significant amount of time to the signing process and I believe it's because running code-sign on every archive file's contents just takes a long time. As this is an opt-in feature that most people will probably not need to use perhaps it make sense for the option to either be boolean, or to take a glob pattern or array of archives that could be specified by the user.

@sethlu
Copy link
Contributor Author

sethlu commented Jul 19, 2020

It looks like on that executable binary we have the following issues...

  1. The binary is not signed.
  2. The signature does not include a secure timestamp.
  3. The executable does not have the hardened runtime enabled.

I'm not very sure where the first issue comes from as everything should be signed...

But for the second one we can probably resolve it by adding the electron-osx-sign --timestamp flag to tell the codesign tool to include a secure timestamp. See #200 (comment).

For the third issue, it seems like that the app bundle wasn't signed with hardened runtime enabled (there's the option electron-osx-sign --hardened-runtime that should take care of this).

Perhaps with these two tweaks, the first problem will disappear?


👌 The latest commit should give us a pattern matching behavior with the --traverse-archives option:

--traverse-archives, --traverse-archives=pattern/to/archive/1,pattern/to/archive/2
    Option to enable automation of signing binaries inside zip-like archives.
    Not specifying any pattern will lead to marking all binary files as potential zip-like archives.
    Disabled by default.

Let me know how if the secure timestamp & the runtime hardening do the work 😺
Hope the new changes speed up things a little.

PS: Due to work-related restrictions, I may need to start taking a break from maintaining this repository starting this upcoming week. I'll try to find someone from the Electron org to have this merged in case we don't get to finish it by the end of Sunday.

@sethlu sethlu force-pushed the traverse-archives branch from 942acd3 to a78b4fd Compare July 19, 2020 09:51
@axfelix
Copy link

axfelix commented Dec 10, 2020

Any update on getting this merged (along with the other PR that fixes spaces in filenames)? I'm having a hard time doing deep signing (#240) and I think those two PRs together should fix it...

@MichaelMauderer
Copy link

I'm running into the same issue. Anything that can be done to move this PR forward?

@dvigne dvigne mentioned this pull request Nov 12, 2021
@MarshallOfSound
Copy link
Member

Conflicting with the rewrite, if this change is still applicable please raise a new pull request. Apologies for the conflicts

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

Successfully merging this pull request may close these issues.

Cannot sign jar with .jnilib file inside it
5 participants