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

Add notarization to unified mac client #2154

Closed
karanbirsingh opened this issue Feb 12, 2020 · 12 comments
Closed

Add notarization to unified mac client #2154

karanbirsingh opened this issue Feb 12, 2020 · 12 comments
Labels
bug Something isn't working status: resolved This issue has been merged into main and deployed to canary.

Comments

@karanbirsingh
Copy link
Contributor

Describe the bug

Starting Feb 3, the startup workflow on MacOS Catalina is different for unnotarized applications:

We need to check how the unified mac client runs on Catalina, and consider notarizing the app (in addition to existing signing) to remove extra end-user work.

@karanbirsingh karanbirsingh added the bug Something isn't working label Feb 12, 2020
@msft-github-bot msft-github-bot added the status: new This issue is new and requires triage by DRI. label Feb 12, 2020
@smoralesd smoralesd assigned ferBonnin and unassigned smoralesd Feb 13, 2020
@smoralesd smoralesd added status: ready for triage This issue is ready to be triaged by the Accessibility Insights team. and removed status: new This issue is new and requires triage by DRI. labels Feb 13, 2020
@msft-github-bot
Copy link
Collaborator

This issue has been marked as ready for team triage; we will triage it in our weekly review and update the issue. Thank you for contributing to Accessibility Insights!

@ferBonnin ferBonnin added the status: ready for work This issue is ready to be worked on. label Feb 18, 2020
@msft-github-bot msft-github-bot removed the status: ready for triage This issue is ready to be triaged by the Accessibility Insights team. label Feb 18, 2020
@ferBonnin
Copy link
Contributor

Per conversation with Peter, let's do this.

@jalkire jalkire self-assigned this Jun 17, 2020
@jalkire
Copy link
Contributor

jalkire commented Jun 24, 2020

Added the ESRP notarization task to our signed build. Resulted in an error with no details. Unassigning myself to start on feature work.

@jalkire jalkire removed their assignment Jun 24, 2020
@dbjorge dbjorge self-assigned this Aug 3, 2020
@dbjorge dbjorge added the status: active This issue is currently being worked on by someone. label Aug 3, 2020
@msft-github-bot msft-github-bot removed the status: ready for work This issue is ready to be worked on. label Aug 3, 2020
@dbjorge
Copy link
Contributor

dbjorge commented Aug 3, 2020

Picking this up where John left off. Next few items I'm going to try, based on Apple's notarization docs:

  • Bring John's branch up to date with master
  • Verify that electron-builder's output has the hardened runtime enabled (electron-builder's mac config docs suggest that hardenedRuntime=true is the default configuration, but I want to double check because we've seen misleading behavior from electron-builder config before)
  • Verify that electron-builder's output has an entitlements file, and/or try setting one explicitly. When doing mac codesigning electron-builder defers to electron-osx-sign, which has a default entitlements file, but since we're doing our own signing maybe we aren't getting that benefit?
  • Verify the bundle ID is getting set correctly at build time
  • Double check our ESRP parameters
  • Reach out to ESRP support
  • Find examples of other teams using ESRP notarization and see if they're doing anything exciting

@dbjorge
Copy link
Contributor

dbjorge commented Aug 3, 2020

Followed the VS Code team's example; the key realization is that electron-builder uses the codesigning process (specifically, the electron-osx-sign package) as the means to add the "hardened runtime"/entitlements configuration, but we actually skip this step intentionally since we want ESRP to handle the codesigning. Thankfully, we can ask ESRP to handle the same hardened runtime configuration by passing a parameter for it to the initial codesigning step (not the notarize step). There were a few other minor changes I made during testing that I kept, but I think that was the key one.

The VS Code folks, naturally, have a much more complex mac build process than us; they actually manually invoke electron-osx-sign themselves using a developer cert in the build agent's keychain (presumably to get tight control over the specific entitlements/hardening configuration), and then send the already-developer-signed file to ESRP to get codesigned again with release certs, and then send it to ESRP again to get the signed packages notarized. For now I've omitted the manual electron-osx-sign step, but it might be something to explore if this version doesn't work out.

Results: Build #Signing-478 with first examples of notarize step succeeding.

@waabid is testing the resulting artifacts now as I write this, fingers crossed :)

@dbjorge
Copy link
Contributor

dbjorge commented Aug 3, 2020

Wahaj found that the app crashes on startup. The relevant snippet of error is:

Process:               Accessibility Insights for Android - Insider [98982]
Path:                  /Applications/Accessibility Insights for Android - Insider.app/Contents/MacOS/Accessibility Insights for Android - Insider
Identifier:            com.microsoft.accessibilityinsights.unified.insider
Version:               2020.803.9 (2020.803.9.2020.803.9)
Code Type:             X86-64 (Native)
Parent Process:        ??? [1]
Responsible:           Accessibility Insights for Android - Insider [98982]
User ID:               501

Date/Time:             2020-08-03 15:10:51.195 -0700
OS Version:            Mac OS X 10.15.5 (19F101)
Report Version:        12
Bridge OS Version:     4.5 (17P5300)
Anonymous UUID:        D58B1136-DAB4-4384-860F-9D25A6897970

Sleep/Wake UUID:       7C8CC498-A472-4469-8CC4-EF94FA722520

Time Awake Since Boot: 59000 seconds
Time Since Wake:       4900 seconds

System Integrity Protection: enabled

Crashed Thread:        0  Dispatch queue: com.apple.main-thread

Exception Type:        EXC_BAD_ACCESS (Code Signature Invalid)
Exception Codes:       0x0000000000000032, 0x0000316972d02040
Exception Note:        EXC_CORPSE_NOTIFY

Termination Reason:    Namespace CODESIGNING, Code 0x2

Related issues:

Karan suggested a second example to look at (Bot Framework):

They're using the same basic strategy as VS Code (performing a pre-ESRP-signing step that uses the codesign tool on the dev box to assign an enlistments file and a hardening config, then sending that to ESRP. Like VS Code, they're signing a .app file (not a .dmg like we're doing), so I can only hope it's a similar process.

@dbjorge
Copy link
Contributor

dbjorge commented Aug 7, 2020

Talked to the ESRP team today and confirmed with them that following the examples/guidance from the VS Code and BotFramework-Emulator repos is correct. At a high level, the process is:

  • ESRP folks add our build/release admin group as a "derived" member of the general Microsoft account for the Apple Developer Program
  • We generate a (derived) Apple Developer ID Certificate from the Apple Developer Program portal
  • We store the certificate (base64-encoded) and its password in the KeyVault instance with the rest of our publishing secrets
  • Our release/signing builds consume that certificate/password via a KeyVault-linked variable group and add it to a .keychain file on the build agent
  • We use codesign (either directly like BotEmulator-Framework, via electron-osx-sign like VS Code, or via electron-builder (which uses electron-osx-sign) to apply the "hardened runtime" option and the entitlements .plists (for our main .app and all its helpers). This must happen after electron-builder produces the release directory structure but before the structure is packed into .dmg/.zip files
    • This does result in signing the .apps with our derived Developer ID cert; the signature doesn't matter so much, this is mostly a side effect of how you set the entitlements
  • We pack the .zip/.dmg as we do now
  • We send it to ESRP for codesigning (with a special hardened runtime option)
    • ESRP knows how to maintain the existing entitlements we applied earlier as part of the "official" signature
  • We send it to ESRP again for notarization

@dbjorge
Copy link
Contributor

dbjorge commented Aug 11, 2020

We've now worked with the ESRP folks and provisioned an Apple developer account that can generate the derived cert we'll need. I've also hooked up placeholder secrets in our usual keyvault and piped them into the signing build. The next steps all require an actual mac for testing/validation:

  • Generating the certificate we need requires using a mac to create a "certificate signing request" and uploading that to the Apple Developer Portal.
  • It's not clear which strategy for using the cert will be easiest (we may be able to just use electron-builder directly by passing it the appropriate CSC_* environment variables, but I haven't tested this and there's probably a reason BotFramework and VS Code aren't doing that). If that doesn't work, we'll need to iterate on splitting the electron-builder step in our build into separate "prepare folder structure" and "package into dmg" steps, so we can insert the codesign step in between them.

I've asked Peter to have a mac provisioned for me; moving this feature back to ready for work until I receive it or someone else with a Mac can pick it up, rolling onto web compliance feature in meantime.

@dbjorge dbjorge added status: ready for work This issue is ready to be worked on. and removed status: active This issue is currently being worked on by someone. labels Aug 11, 2020
@dbjorge dbjorge removed their assignment Aug 11, 2020
@ferBonnin
Copy link
Contributor

@ferBonnin to schedule a small feature for this

@ferBonnin ferBonnin added the status: needs investigation This issue requires investigation (PM, Design, or SWE) by the Accessibility Insights team. label Feb 1, 2021
@ghost
Copy link

ghost commented Feb 1, 2021

This issue requires additional investigation by the Accessibility Insights team. When the issue is ready to be triaged again, we will update the issue with the investigation result and add "status: ready for triage". Thank you for contributing to Accessibility Insights!

@ghost ghost removed the status: ready for work This issue is ready to be worked on. label Feb 1, 2021
@waabid
Copy link
Member

waabid commented Oct 19, 2021

Note: further work on this still requires a Mac; it can be done without but it'd be really tedious (using build agents for debugging).

Did some work on this and here is where we currently stand:

Branch: https://github.com/waabid/accessibility-insights-web/tree/macNotarization

What's completed:

  • The pipeline is fully setup to codesign and notarize our Mac clients. This includes getting the appropriate cert (now in keyvault), defining entitlements1, codesigning and sending our app for hardening/notarization. The codesigning occurs during our unsigned build and we harden and notarize in our signed build.

What's not working:

  • We're seeing errors in the notarization step. The error doesn't provide a lot of information ("The signature of the binary is invalid") but upon further investigation, it may be due to improper signing (which itself may be a sign of improper folder structure). Some info about that can be found here: https://developer.apple.com/forums/thread/128166

What to do next:

  • Running the verify command codesign -v -vvv --deep <path to app> locally from downloaded unsigned build bits (any channel is okay) points out issues with files not being correctly signed. We must work through these issues I believe before we can pass the notarization step (it seems). How we resolve these issues is unclear (the above link should provide some guidance on that front as well). It could be folder structure or we could manually sign each file (via another script) instead of using the "--deep" flag and finally I believe entitlements can also play a role.

1 It's unclear what entitlements we specifically need for our application; further investigation may be required. The ones being used are a result of looking at what the BotFramework/VSCode teams are doing. This could be a potential reason for issues with signing as adding certain entitlements has reduced errors found using the verify command. More information about the entitlements that concern us can be found here: https://developer.apple.com/documentation/security/hardened_runtime

@ferBonnin ferBonnin added the status: active This issue is currently being worked on by someone. label Apr 25, 2022
@ghost ghost removed the status: needs investigation This issue requires investigation (PM, Design, or SWE) by the Accessibility Insights team. label Apr 25, 2022
@waabid waabid added the status: resolved This issue has been merged into main and deployed to canary. label Apr 25, 2022
@ghost ghost removed the status: active This issue is currently being worked on by someone. label Apr 25, 2022
@waabid
Copy link
Member

waabid commented Apr 25, 2022

This has been completed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working status: resolved This issue has been merged into main and deployed to canary.
Projects
None yet
Development

No branches or pull requests

7 participants