-
Notifications
You must be signed in to change notification settings - Fork 37
Connect: Set up entitlements and provisioning profile #1033
Conversation
I'm running a build on 11.0.0-dev.5 which is this branch rebased on top of |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, just a few minor adjustments.
Checked and double-checked, but it looks like you got it all down. 👍
The real test is whether the resulting binary works. Give me a nudge if you have any problems, I can help you diagnose/debug.
Co-authored-by: Alan Parra <[email protected]>
LGTM. Let us know how it goes after you take the final binary for a spin. |
Sure thing, though idk when that will happen as the darwin builders seem to remain broken. |
11.0.0-dev.5 got built but I made a mistake when rebasing my branch on top of @kimlisa's and I forgot to include the change to the bundle identifier. 🤦 I'll give it another go tomorrow. |
I built 11.0.0-dev.8 which is again just this branch rebased on top of Lisa's. Here's the .dmg file and this was the drone build. Opening the app fails because it seems that its signature is invalid. (cc @kimlisa) I need to find a faster way to verify if everything's okay than building a tag on drone which takes ~45 minutes, I'll look at the instructions Alan left in tshdev. codesign -dv --verbose=4
codesign -v -vvvv
The report compiled by macOS
|
Yep, that's tough.
Looking at Console.app during launch, the following caught my attention:
Followed by (abridged):
Comparing $ codesign -dvv /Applications/tsh-v10.0.2.app
Executable=/Applications/tsh-v10.0.2.app/Contents/MacOS/tsh
Identifier=QH8AA5B8UP.com.gravitational.teleport.tsh <-- STARTS WITH TEAM ID
Format=app bundle with Mach-O thin (x86_64)
CodeDirectory v=20500 size=628469 flags=0x10300(hard,kill,runtime) hashes=19628+7 location=embedded
Signature size=8987
Authority=Developer ID Application: Gravitational Inc. (QH8AA5B8UP)
Authority=Developer ID Certification Authority
Authority=Apple Root CA
Timestamp=19 Jul 2022 12:22:00
Info.plist entries=23
TeamIdentifier=QH8AA5B8UP
Runtime Version=11.3.0
Sealed Resources version=2 rules=13 files=2
Internal requirements count=1 size=204
$ codesign -dvv Teleport\ Connect.app
Executable=/Users/alan/Downloads/Teleport Connect.app/Contents/MacOS/Teleport Connect
Identifier=com.gravitational.teleport.connect <-- NO TEAM ID
Format=app bundle with Mach-O thin (x86_64)
CodeDirectory v=20500 size=494 flags=0x10000(runtime) hashes=4+7 location=embedded
Signature size=9067
Authority=Developer ID Application: Gravitational Inc. (QH8AA5B8UP)
Authority=Developer ID Certification Authority
Authority=Apple Root CA
Timestamp=28 Jul 2022 12:24:14
Info.plist entries=30
TeamIdentifier=QH8AA5B8UP
Runtime Version=12.3.0
Sealed Resources version=2 rules=13 files=18
Internal requirements count=1 size=196 Based on the above, my guess would be to prepend the Team ID to Connect's Identifier: Note that there's also a difference between tsh flags ( |
11.0.0-dev.11 has an updated identifier but fails with the same error. Looking at codesign output, the remaining differences between Connect and tsh.app is the flags that you mentioned Alan and Runtime Version. I haven't looked at Console yet, first I want to make a local dev setup, otherwise I'm finding myself just doing the least amount of changes possible in hopes that the ~45 minutes build comes out alright. I forgot yesterday that I need 2FA to log in to the dev account so I'll get the dev cert today and try again on Monday. |
> Note that up until Electron 12, the > com.apple.security.cs.allow-unsigned-executable-memory entitlement was > required as well. However, it should not be used anymore if it can > be avoided. https://www.electronjs.org/docs/latest/tutorial/code-signing#using-electron-forge
This fixes the problem with child processes having unsatisfied entitlements. Taken from here: https://github.com/electron/osx-sign/blob/db611f0eda8931f1ac8caf11a11dfb08187f1d3b/entitlements/default.mas.child.plist
Made some progress today. I set up a local env for signing. Thanks to adding Now the tshd process launched by the main process seems to get killed soon after launching but at least this doesn't cause any error alerts pop up. I'll look into that on Tuesday once I'm back.
|
"target": "dmg", | ||
"type": "distribution", | ||
"hardenedRuntime": true, | ||
"entitlements": "assets/entitlements.mac.plist", | ||
"entitlementsInherit": "assets/entitlements.mac.plist", | ||
"entitlementsInherit": "assets/entitlements-inherit.mac.plist", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have you tried it without letting the other processes inherit the entitlements? Ideally we don't the permissions to propagate to binaries other than tsh
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, I haven't but that's a good point. Matter of fact, we need only the tsh process to have access to those keychain-access-groups as for now it'd be the only process interacting with Touch ID.
I believe the Electron child processes need to have com.apple.security.cs.allow-jit
set to true and guessing from the errors that I've seen, all child processes must have at least com.apple.developer.team-identifier
set to the same value as the main process.
I'll see what I can do here.
I understand entitlements a little bit better now so I went a step back. tl;dr The entitlements for keychain-access-groups are granted for the main process but not the helper processes somehow. What I did today: I made a working build with minimal entitlements, that is only the ones required by Electron (
(source)
So, the app with minimal entitlements works just fine. Mind you, I just simply provide the same entitlements file for both the main binary as well as the child binaries for now ( The moment I add
What's interesting is that those messages are shown only for the helper processes and not for the main process. To me this suggests that somehow the main process is eligible to those entitlements but child processes are not. Adding When viewing all messages in Console and not just errors I can see this:
I also discovered that for notarization my dev build used the wrong app ID. I think the next step would be to check why those errors are thrown for helper processes but not for the main process. I checked the entitlements with |
@codingllama Do you know if Connect will be able to simply provide tsh.app app ID as the keychain access group and call it a day? The example in Sharing Access to Keychain Items Among a Collection of Apps shows two apps sharing an arbitrary key rather than the app ID of the first app. OTOH I'm not sure how macOS would differentiate between an arbitrary key and an app ID when reading keychain access group but maybe there's something to it.
From a section talking about app groups vs keychain access groups:
|
Good to see you are making progress, @ravicious!
I think that's the question: ideally we want to entitle tsh and nothing else. I wonder if we can get the Electron builder's output, take it apart and tweak it ourselves, so we get the exact setup we want. Signing and notarizing apps is nothing new to us at this point.
Yep, that's why I have separate folders for "tsh" and "tshdev".
That's a good question. My assumption was that yes, it would work, but we could try something like
Yep, I saw that too. Default groups do play a part, but we can tweak the tsh code to use specific groups if necessary. Let's figure out the setup first, then we can consider further changes. |
Right, I can see how this would be very important. We certainly can run whatever electron-builder is running manually at the cost of having to maintain those scripts ourselves. From what I managed to gather, there was at least one instance where a change in Chromium required changes to how Electron apps are signed. Though tbh, even at this point it might be easier to do all of this manually and just add those entitlements to tsh somehow rather than trying to figure out how to make those entitlements work when electron-builder adds them to all binaries within the bundle. |
Actually, we might be able to have a custom signing process just for the tsh binary… |
Okay, I'm getting somewhere. I managed to sign tsh with a custom set of entitlemets but…
https://developer.apple.com/forums/thread/128166 I thought I'd be able to avoid this but it seems that it's actually required. I'll create a new provisioning profile for tshdev that's tied to my cert and I'll see how it goes. I might not have time for that today though. BTW, up until today I didn't know how to check which cert exactly a provisioning profile is tied to. Turn out you just right click -> Get Info and it's there in the preview section. |
Glad to see you are making progress, Rafal!
That's exactly what we do for tsh.app
Good to know, I have a bunch of exoteric commands noted somewhere to get cert from the profile. |
Closing this PR in favor of #1116. This one is a little messy and my final solution is much different than what I initially thought I'd need to do. |
Closes gravitational/webapps.e#325.
This should let us use the same Touch ID credentials that tsh.app uses.
keychain-access-groups
only to the tsh process or the main process + tsh if possible.com.apple.security.cs.allow-jit
only to the main process and the Electron child processes.