Conversation
Fix CLARATION#575 Signed-off-by: Paul <devnoname120@gmail.com>
|
Signing applications should go through an entitlement merging process similar to what's done with Impactor here: https://github.com/khcrysalis/Impactor/blob/main/crates/plume_core/src/utils/mod.rs#L15-L75 The idea is that we would need to merge app groups along with keychain groups with their necessary identifiers that apps provide with their pre-existing entitlements. Feather should try to replicate Impactor's functionality, ideally. This pull request from what I've seen only makes new keychain groups using the identifier from the provisioning profile/supplied entitlements but doesn't consider other existing keychain groups that the app may use internally (again, pre-existing entitlements). For example, LiveContainer's default entitlements contain around 128 keychain groups for security. |
| /// Replaces wildcard keychain-access-groups in entitlements with the app's | ||
| /// bundle identifier so each signed app gets its own keychain namespace. | ||
| private func _modifyEntitlementsForKeychainIsolation(for app: URL) async throws { | ||
| guard _options.keychainIsolation, let cert = appCertificate else { |
There was a problem hiding this comment.
Options should be checked before the function is ran.
Also, certificate isn't required, provided entitlements seems to be a used as well.
| temporaryValue: temporaryOptions?.keychainIsolation | ||
| ) | ||
| } footer: { | ||
| Text(.localized("Replaces wildcard keychain groups with the app's bundle identifier, preventing sideloaded apps from accessing each other's keychain entries.")) |
There was a problem hiding this comment.
You're missing translation keys.
There was a problem hiding this comment.
Seems that it’s not the only missing one. I just copy-pasted the approach in
and I thought that it was all that was needed considering that this string was not referenced anywhere else.Once I’m back to my computer I will add this string and other missing strings (maybe in a follow-up PR to keep this PR tidy)
| return | ||
| } | ||
|
|
||
| // Determine the bundle identifier the signed app will use |
There was a problem hiding this comment.
This is AI generated, obviously, these comments aren't necessary.
Also instead of getting the identifier from Info.plist use the app object.
There was a problem hiding this comment.
Ok I will do the change and tone down comments
There was a problem hiding this comment.
Should I do _options.appIdentifier ?? app.identifier instead?
Why does _modifyPluginIdentifiers() use let oldIdentifier = infoDictionary["CFBundleIdentifier"] as? String instead of app.identifier?
Yes, this was intended
Does this matter in practice? I’m on my phone right now so not easy to check. Will try to take a look later.
Tbh this was actually my first approach where I replicated the behavior from Impactor. But then I wasn’t sure why the merge was needed at all in the first place because it seems like Feather just cobbles up its own entitlements file based on the Could you elaborate a bit on the purpose of this? As far as I understand keychain groups must all start with the appropriate team Id or they are invalid for the given cert. So in practice it seems to me that it would only be an issue for cloning some apps (which?) while maliciously escaping the keychain group for the app would require the attacker to know the team ID in advance and inject it in the IPA. If this is true, then it doesn’t look like that big of a deal to me. For LiveContainer I think it’s not a big deal as there is no good reason to install it with a developer account distrib cert, it’s only useful for non-paid certs. It needs to isolate apps between each other but normal apps don’t need to isolate from their extensions unless I missed something. |
Yes, the default keychain group isn't always use, many apps have custom ones with custom identifiers, like YouTube.
I mention LC as an example for its keychain groups, not saying its practical to use with Feather. |
Zsign just slaps the entitlements of the provisioning profile on every available binary within the application, it doesn't do additional checks for anything else and doesn't consider anything else. |
|
When I mention something like another app or specific functionality do note that I'm mentioning them for a reason, nothing more nothing less. |
|
@CLARATION Tbh the entitlements merging thing is a bit over my head. I gave it a few attempts for 2-3 hours but I’m not sure I understand enough about entitlements and |
|
All you're essentially doing is merging 2 plist files together into one, by taking keychain/appgroups from one file (the binary), and placing it into another (the provisioning profile). Then doing additional modifications like putting the TEAMID at the front so iOS accepts the new keychain/app groups (like what your logic does already). You just need a way to extract entitlements from the main binary. |
2 participants
Fix #575
Currently all apps sideloaded with a given
.mobileprovisionuse the same keychain group. This is a major security risk because any sideloaded app can snoop on other apps keychain entries (instagram tokens, telegram tokens, passwords for some apps, etc.).This means that any malicious app can defeat iOS keychain sandboxing entirely instead of only being able to access its own keychain entries. This also causes conflicts when multiple apps use the same names for keychain entries, e.g. for cloned apps.
This pull request adds a new option for keychain isolation, inspired by Impactor.
The keychain group name is deterministically derived from the bundle id of the app to make sure that installing new versions of said app won’t lose the keychain entries.
I tested on several apps and I confirm that it works properly. It also works on apps making heavy use of extensions/widgets with different bundle IDs (e.g. CARROT) and I confirm that it works: the keychain group is shared among them rather than having a unique group for each extension.
Before (105 items from a bunch of unrelated apps):
After (only the 2 items of the app):