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

Enable App Sandbox on macOS for increased security #3149

Draft
wants to merge 50 commits into
base: develop
Choose a base branch
from

Conversation

kthchew
Copy link
Contributor

@kthchew kthchew commented Nov 29, 2024

Important

If you want to test this, keep in mind the following:

  • Make a backup before testing.
  • Opening the sandboxed app for the first time will move existing data to a new directory. If you later revert to the current version of Prism Launcher, it will not know where the data is and start from the setup screen again. The new location is ~/Library/Containers/org.prismlauncher.PrismLauncher/Data/Library/Application Support/PrismLauncher. To revert, manually move that folder back to ~/Library/Application Support/PrismLauncher.
  • It is highly recommended to delete ~/Library/Containers/org.prismlauncher.PrismLauncher and ~/Library/Application Scripts/org.prismlauncher.PrismLauncher after testing. Be sure to move your data out before deleting these folders – see above. If you do not delete these folders and App Sandbox is ever added to Prism Launcher proper, the system will likely show security warnings every time you open the launcher because the identity of the app generated from a PR is different from its identity during a release, and the system cannot tell that they are the same.
  • If you want to use the launcher's Java downloader, it's recommended to select a Java directory that is not in the container (i.e. not in ~/Library/Containers/org.prismlauncher.PrismLauncher/Data). You'll likely encounter crashes if the Java directory is in the container due to what I believe to be macOS bugs that I'm hoping Apple will fix (FB16070451).

Extends off the work in #1190 to enable App Sandbox to reduce potential damage by vulnerabilities in the launcher, game, and mods, as well as outright malicious mods.

On recent macOS versions, it also protects sensitive data in the Prism Launcher container (such as account tokens) from other running processes, even those with root privileges, using System Integrity Protection (unless user consent or Full Disk Access is given).

NOTE: This comment should not be taken to mean that Prism Launcher's data is protected from itself or mods. A malicious mod can still steal Minecraft account tokens, since it is within the sandbox, as is expected to be the case on any platform with a sandbox. This provides protection from other malware external to the game (e.g. a random script someone downloads).

tl;dr, does this work?

The game should work, and be sandboxed, as long as you

  • use default instance, etc. directories (or at least one within the sandbox container, so not on an external drive for example) you can now choose other directories :) it's possible there'll be problems with instances on other drives though, need to test
  • use Java installed in the normal system-defined location (/Library/Java/JavaVirtualMachines), or another directory allowed by default by the sandbox, (edit: or by the launcher's java downloader)
    • the launcher’s Java downloader won’t work now works :)
    • If you use the Java downloader to download Java to a directory located in the sandbox container, the game may crash when the narrator tries to speak. This is due to a macOS issue (see below). It can be worked around, but in this case I'll wait for a fix.
  • if applicable, ensure wrapper scripts pass environment variables down to java (see below)

This isn’t fully ready yet for review/merging. See below for more known issues.

The Quarantine Issue™

Based on the discussion in #1190, we found that sandboxed apps quarantine all files they write to. This prevents a class of simple sandbox escapes. However, Minecraft (and some mods) extract native dynamic libraries to the filesystem at runtime, causing the system to deny them and show an error when they are loaded, and the game won't run. Getting around this requires a bit of a hack. When loading the library, the process calls the dlopen C function. By injecting a library into the game with the DYLD_INSERT_LIBRARIES environment variable (similar to LD_PRELOAD on Linux), we can use a technique called "dyld interposing" to replace any dlopen function calls to call our own implementation. This is where we can remove the quarantine attribute.

Actually removing the quarantine attribute must be done from a non-sandboxed process. (There'd be no point if the sandboxed process could just remove it itself.) Thus, a new non-sandboxed XPC service is added that removes quarantine on a provided file. There are a few checks and measures to attempt to stop malicious code from abusing it for a sandbox escape, though these might be expanded. Note the following principles I used to decide how to design these preventative checks in the XPC service (mostly found through trial and error rather than documentation...):

  • A sandboxed app can use LaunchServices (e.g. a call to NSWorkspace.shared.open() or using /usr/bin/open) to open a file it has access to in a similar manner to if you double clicked it in Finder. Such file opens occur outside the sandbox (for example, opening a text file will open a text editor with the text editor's sandbox, or lack thereof. Same applies to a regular app. This has to be the case, otherwise these apps often wouldn't work.)
    • If such a file is quarantined, it must pass Gatekeeper (be signed and notarized) if the system determines that opening the file can lead to code execution. If it is not quarantined, it will generally get through without a prompt. This is why it is important to be careful when removing quarantine.
  • A sandboxed app has limited capability to customize how a file is opened. It can only choose to open a file using the default app for that file (which can be changed, generally done in Finder by customizing a file's "Open With" attribute), or any app installed that has declared that it can open that file type (usually determined by a file extension), and a few "safe" apps (like TextEdit). Attempts to open such a file with a different app fail with an error due to the sandbox.
    • Therefore, by whitelisting only particular file extensions that we need to have their quarantine removed, we can look at which apps can open such files and determine how to safely dequarantine these kinds of files.
  • Terminal.app declares that it can open executable files without a file extension (chmod +x). It also declares that it can open .dylib files, whether they have executable permission or not.
    • If a file opened in Terminal has executable permission (chmod +x), it will run immediately. If it does not, it may still launch Terminal, but will not actually execute.
    • Sandboxed apps are not supposed to be allowed to open any file in Terminal. The system rats out your app by showing the user a popup telling them what file your app tried to open in Terminal.
  • iTerm (a commonly installed third-party Terminal alternative) makes the same declarations above. It also indicates it can open .tmp and .jnilib files.
    • Sandboxed apps can open files in iTerm, unlike Terminal.
    • iTerm, by default, asks for confirmation to run executables launched in such a manner, so this method of sandbox escape relies on this being disabled or the user approving the launch.
    • iTerm similarly will not be able to execute code from a file without the execute permission, even if approved.
    • (In general it's not possible to analyze every possible third-party app that could open each of these files, but this is just a fairly common one. In practice an exploit that attempts to use one of these apps would of course rely on that app being installed, which narrows the scope significantly.)
  • Even if an app allows execution with dyld environment variables, if a sandboxed app tries to use these variables to open an app with LaunchServices (outside the sandbox), these environment variables will have no effect, preventing code injection into a more privileged process with a library alone.

Essentially, for the file extensions I whitelisted, they are safe to dequarantine as long as you strip away their execute permission and don't allow them to have a customized "Open With" attribute (which leads to us having no idea the properties of whatever app this refers to). Libraries can still be loaded without the execute bit, so this is fine. Note that further attempts by the sandboxed process to modify the file, such as readding execute permission, will lead to the file becoming quarantined once again.

The Java child process unfortunately can't directly talk to the XPC service. The launcher thus uses a Unix socket to talk to the game. The transposed dlopen will send a message through the socket, triggering the launcher to ask the XPC service to unquarantine the library, before calling the actual dlopen.

graph LR;
    QuarantineRemovalService-- XPC ---Prism
    Prism-- XPC ---QuarantineRemovalService
    Prism-- Unix socket ---Minecraft
    Minecraft-- Unix socket ---Prism
Loading

The code is a bit of a mess right now since it's practically a proof-of-concept.

Note

Interposition isn't documented public API by Apple, so it's technically unsupported and only recommended for use in developer tools. The alternative methods have some issues though:

  • We could distributed modified Java runtimes that run the same code before any calls to dlopen to avoid interposition, but then user-installed "regular" JREs won't work.
  • There might be a way to do something equivalent at the Java level (e.g. intercept calls to System.loadLibrary etc.) but I'm not too familiar with such methods and they appear to be more complex. For example, it seems like you can't directly write to Unix sockets from Java.

Known issues/TODO

Java downloader doesn't work (edit: fixed for Mojang Java in 653309d and others in 0410f5a)

Java can be downloaded, but it won't appear in the list afterwards (but will be present on disk). If you manually choose the downloaded Java, it won't run.

There are two problems here. One is the same as above - the quarantine applied on the file, but this time by the launcher. The Java executables won't be allowed to run as a result. The fix is, again, to offload this work to an unsandboxed XPC service (e.g. to lift the quarantine), but without any of the complicated interposing or sockets this time, since this is directly from the launcher itself.

The other problem is that for some reason, the sandbox denies the app from executing any executables located in the sandbox container directory, even without quarantine. (You can see in Console that a process-exec on Java is denied, even after you remove the quarantine.) I believe this is a macOS bug, as the documentation claims the app “can run programs located [in the container].” I filed a bug with Apple about this (FB15963761).

Note that the launcher seems to be able to run Java that is installed system-wide just fine.

Crash due to narrator when using Java downloader

There is an issue where the game crashes when attempting to load the narrator, for some reason. The error in the logs is of the form

*** Terminating app due to uncaught exception 'NSInvalidArgumentException', reason: '-[AUAudioUnit_RemoteV2 setSpeechSynthesisOutputMetadataBlock:]: unrecognized selector sent to instance 0x159609200'

This occurs when the Java installation used is contained in the sandbox directory. If that condition is true, it occurs regardless of whether or not the game is sandboxed. The current workarounds are to

  1. Use Java installed in a directory that is not contained in ~/Library/Containers/org.prismlauncher.PrismLauncher/Data, which can be downloaded by the launcher's Java downloader, or
  2. Disable narrator in your game options (might need to be done with a text editor outside of the game)

Further testing: This appears to be a macOS issue if the executable trying to use text-to-speech APIs is contained in a sandbox container on macOS Sequoia. The issue does not appear to affect macOS Sonoma. I made a minimally reproducible case and reported it to Apple (FB16070451).

Migrating data (default locations) (edit: added in 32c0d85)

Sandboxed apps have a different "home directory." Normally ~ is /Users/USER, but now it's /Users/USER/Library/Containers/org.prismlauncher.PrismLauncher/Data (at least this is the implementation detail in current macOS versions), so data must be migrated.

This is fairly trivial for users who don't change their data directories from the default, as macOS lets you use a container-migration.plist to indicate what should be moved.

Accommodating non-default data directories (edit: implemented in 503c702)

The situation is more complicated if someone customizes where data, such as instances, are stored. The container migration system described above is declarative and can't accommodate this situation (and arguably the launcher shouldn't move that data if the location isn't standard, anyway). macOS grants sandboxed apps access to directories if they select it in a native file picker.

Therefore, the current state is that if you go to Settings > Launcher and use the "Browse" buttons to choose some directory for instances, etc. outside of the sandbox container, Prism will be allowed to access that directory for that session. But if quit and reopened, access will be lost and you'll need to do this again. The solution to maintaining this access is to store security-scoped bookmarks rather than just a path, which will likely require some more native code (I'm not sure if Qt has anything to help with this).

Picking alternate directories will thus require the use of the file browser rather than allowing users to type in a path. From a simple test in Xcode, it looks like a non-sandboxed app can save security-scoped bookmarks in advance, and if it later adopts the App Sandbox, the bookmarks will work, allowing for a seamless update. It would require an intermediate update that saves these bookmarks rather than paths in the config, however. Otherwise, after an update that migrates to the sandbox, users would need to select the relevant locations in the file picker manually to grant access.

Sparkle updater (edit: fixed in e62eb76)

Some changes are needed as described at https://sparkle-project.org/documentation/sandboxing/.

Also, if a non-sandboxed version is built separately, the feeds for each version should likely be separated.

Ability to disable sandbox by instance (?) (edit: separated build in 670d69e)

It's described in #1146 that users should be able to disable the sandbox. This might be doable with an XPC service, though it's risky if you naively do it based on an instance setting, since a malicious mod in a sandboxed instance could just disable the sandbox (as the settings are contained inside the sandbox container). It might be easier and safer to have two versions of Prism Launcher, one sandboxed and one not.

User-defined allowed directories (?) (edit: added in 65e5c1d with a caveat)

#1146 also describes a feature where users can choose what directories are allowed. This could be possible by presenting a file picker and storing the security-scoped bookmarks as described above.

Edit: this is now added

Screenshot 2024-12-11 at 9 17 29 PM

unfortunately, while you can get read/write access to user-selected directories, you don't get permission to execute programs in those directories, so users still need to place their programs and scripts in "typical" locations (e.g. /usr/local, /opt/homebrew, places indicated in the entitlements like ~/bin, etc.)

Pre/post launch scripts and wrappers (?)

Not sure how this solution or the sandbox itself interacts with these or how people use them in the wild.

In any case, a wrapper command chosen by a user must make sure to pass down the DYLD_INSERT_LIBRARIES and XPC_MIDDLEMAN_SOCKET environment variables to java.

Steam overlay (edit: resolved in 88153e7 – probably)

Steam doesn't support the App Sandbox, so the overlay is unlikely to ever work with it enabled. Possibly another argument for maintaining a non-sandboxed version. https://partner.steamgames.com/doc/store/application/platforms

Apparently it works if you allow the IPC, and the browser even works too – some of the names feel a bit "random" but I tested a VM and saw similar prefixes for the names, so I think the entitlements used should work generically.

Mods (mainly Discord RPC - supported in fb0d41d and 5e3fb20)

Most mods seem to work fine. Some won’t work by virtue of what the sandbox does (Discord RPC is an obvious example). We could see if there are entitlements that allow for this.

Long usernames may not work (edit: fixed in ceb5356)

There’s an added reliance on Unix sockets, but the max length of a Unix socket path on macOS is 104 bytes for historical reasons. (It’s around there on other operating systems that support Unix sockets, too.) The sandbox container temporary directory is quite long: /Users/USER/Library/Containers/org.prismlauncher.PrismLauncher/Data/tmp/. Right now the socket name is prism-launcher, but if I shorten the socket name down to just one character, that leaves 34 bytes left for the name of the home folder (subtracting the null terminator in the C string) (edit: done in 3beeb10). I think most users’ usernames are less than 34 characters, though it could still conceivably be a problem as macOS seems to allow home folder names up to 63 characters long in the System Settings GUI. If that’s the case it might be better to use something like a local TCP socket.

edit: swapped to using unnamed Unix sockets and passing the file descriptor to the game, which avoids this problem

Other odd things

Recent versions of Minecraft take a long time to load without allowing reads to /dev/fd/

EDIT: This issue was fixed by OpenJDK and the fix should be available in future releases.

tl;dr Java 8 is OK. Java 17.0.10 or earlier is OK. Java 17.0.11 and later or any version of Java 21 will have long load times without this entitlement because of either a macOS or a Java bug.

The game won’t appear for a long time, but does load eventually (as in, after waiting for several minutes…) The jspawnhelper process freezes for a while, and if you continuously force quit it, Minecraft will start up much quicker, though that’s obviously not a good solution…

Older versions of MC don’t show this behavior (e.g. 1.8.9, 1.12.2, 1.15.2, 1.16.5 seem to be fine). It seems to be related to Java 8 vs later Java versions. (1.16.5 is on Java 8 and uses LWJGL 3, and works. 1.17.1 is on Java 17, and has problems.)

I used a debugger on jspawnhelper and tracked it down to a close call it is attempting to do. From looking at the source, I tracked the likely cause of this freeze to the fix to https://bugs.openjdk.org/browse/JDK-8307990. Using versions of Java before that (e.g. 17.0.10) that resolve the error.

Luckily, applying a read-only entitlement to /dev/fd/ resolves the issue. I don’t think this adds much risk, although if this ends up being fixed by either Apple or OpenJDK, it should probably be removed.

Signed-off-by: Kenneth Chew <79120643+kthchew@users.noreply.github.com>
Signed-off-by: Kenneth Chew <79120643+kthchew@users.noreply.github.com>
Signed-off-by: Kenneth Chew <79120643+kthchew@users.noreply.github.com>
Signed-off-by: Kenneth Chew <79120643+kthchew@users.noreply.github.com>
Signed-off-by: Kenneth Chew <79120643+kthchew@users.noreply.github.com>
Also changes the way signing is done. A script is added to code each code item individually. Avoiding `--deep` is required to maintain the entitlements on Sparkle, otherwise they would be overwritten. It is also discouraged in general by Apple - see [`--deep` Considered Harmful](https://forums.developer.apple.com/forums/thread/129980).

Signed-off-by: Kenneth Chew <79120643+kthchew@users.noreply.github.com>
Signed-off-by: Kenneth Chew <79120643+kthchew@users.noreply.github.com>
Signed-off-by: Kenneth Chew <79120643+kthchew@users.noreply.github.com>
Signed-off-by: Kenneth Chew <79120643+kthchew@users.noreply.github.com>
Signed-off-by: Kenneth Chew <79120643+kthchew@users.noreply.github.com>
Signed-off-by: Kenneth Chew <79120643+kthchew@users.noreply.github.com>
…cess

This issue appears to be a bug between a call to `close()` in `jspawnhelper` and the App Sandbox. This was reported to Apple as FB15992343 and OpenJDK as JDK-8345256.

Signed-off-by: Kenneth Chew <79120643+kthchew@users.noreply.github.com>
…s on macOS

This enables sandboxed apps to maintain access to user-selected items. In addition, for both sandboxed and nonsandboxed apps it can keep track of directories even if they are moved or renamed, and can remember access to directories in "sensitive" locations (such as the Documents folder or external drives).

Signed-off-by: Kenneth Chew <79120643+kthchew@users.noreply.github.com>
Signed-off-by: Kenneth Chew <79120643+kthchew@users.noreply.github.com>
@kthchew kthchew force-pushed the feat/macos-sandbox branch from bc2bc34 to 91aed41 Compare December 4, 2024 07:26
Signed-off-by: Kenneth Chew <79120643+kthchew@users.noreply.github.com>
Signed-off-by: Kenneth Chew <79120643+kthchew@users.noreply.github.com>
…iginal file

Signed-off-by: Kenneth Chew <79120643+kthchew@users.noreply.github.com>
Signed-off-by: Kenneth Chew <79120643+kthchew@users.noreply.github.com>
…f file

Signed-off-by: Kenneth Chew <79120643+kthchew@users.noreply.github.com>
This allows the Java downloader to work for Mojang Java installs in spite of the sandbox applying a quarantine.

Also apply a sandbox exception to allow execution of Java.

Signed-off-by: Kenneth Chew <79120643+kthchew@users.noreply.github.com>
Unlike the default sandbox quarantine, an executable file with a quarantine type that indicates that the file was downloaded is allowed to run. However, it is subject to Gatekeeper checks. Since Adoptium and Zulu are correctly signed and notarized, this isn't a problem and lets us offload the work to verifying their integrity to the OS. Some additional changes were made to how extraction is done to avoid invalidating the code signature.

Signed-off-by: Kenneth Chew <79120643+kthchew@users.noreply.github.com>
@kthchew kthchew force-pushed the feat/macos-sandbox branch from 1bc2a91 to 0410f5a Compare December 8, 2024 04:00
Signed-off-by: Kenneth Chew <79120643+kthchew@users.noreply.github.com>
@kthchew kthchew force-pushed the feat/macos-sandbox branch 2 times, most recently from 62e537a to 9b4a91e Compare December 8, 2024 06:46
Signed-off-by: Kenneth Chew <79120643+kthchew@users.noreply.github.com>
@kthchew kthchew force-pushed the feat/macos-sandbox branch from 9b4a91e to e62eb76 Compare December 8, 2024 07:09
Also allow bookmark creation when migrating from an older version without bookmarks

Signed-off-by: Kenneth Chew <79120643+kthchew@users.noreply.github.com>
Signed-off-by: Kenneth Chew <79120643+kthchew@users.noreply.github.com>
Signed-off-by: Kenneth Chew <79120643+kthchew@users.noreply.github.com>
@kthchew kthchew force-pushed the feat/macos-sandbox branch from a504b96 to 45b7439 Compare December 9, 2024 06:48
For convenience, also add reset buttons for these paths

Signed-off-by: Kenneth Chew <79120643+kthchew@users.noreply.github.com>
Signed-off-by: Kenneth Chew <79120643+kthchew@users.noreply.github.com>
Signed-off-by: Kenneth Chew <79120643+kthchew@users.noreply.github.com>
Signed-off-by: Kenneth Chew <79120643+kthchew@users.noreply.github.com>
Signed-off-by: Kenneth Chew <79120643+kthchew@users.noreply.github.com>
Signed-off-by: Kenneth Chew <79120643+kthchew@users.noreply.github.com>
Note: Loading the steam libraries into the launcher itself seems to cause a crash, which is why the `allow-dyld-environment-variables` entitlement was removed.
Signed-off-by: Kenneth Chew <79120643+kthchew@users.noreply.github.com>
Signed-off-by: Kenneth Chew <79120643+kthchew@users.noreply.github.com>
Signed-off-by: Kenneth Chew <79120643+kthchew@users.noreply.github.com>
Now a pair of Unix socket file descriptors are made when opening an instance, and one of the descriptors is sent to the instance through an environment variable rather than using a path to a file. This gets around an issue where long usernames may break.

Signed-off-by: Kenneth Chew <79120643+kthchew@users.noreply.github.com>
Merge in the removal of hardened runtime exceptions, and the split of the ad-hoc and regular entitlements that came with it.

Signed-off-by: Kenneth Chew <79120643+kthchew@users.noreply.github.com>

# Conflicts:
#	.github/workflows/build.yml
#	program_info/App.entitlements
Signed-off-by: Kenneth Chew <79120643+kthchew@users.noreply.github.com>
This isn't correct, as the instance directory can be arbitrarily set and may lead to natives being written outside the home directory.

Signed-off-by: Kenneth Chew <79120643+kthchew@users.noreply.github.com>
Link locations and the existence of directories are now explicitly checked, and most work is done in the enumerator where some details are prefetched.

Signed-off-by: Kenneth Chew <79120643+kthchew@users.noreply.github.com>
Explicitly disallow applications and bundles, and skip files that already aren't quarantined.

Signed-off-by: Kenneth Chew <79120643+kthchew@users.noreply.github.com>
Signed-off-by: Kenneth Chew <79120643+kthchew@users.noreply.github.com>
Most libraries do not have an entrypoint (i.e. a `main` method that runs when they are executed directly) so this is not an issue.

Signed-off-by: Kenneth Chew <79120643+kthchew@users.noreply.github.com>
Signed-off-by: Kenneth Chew <79120643+kthchew@users.noreply.github.com>
Signed-off-by: Kenneth Chew <79120643+kthchew@users.noreply.github.com>
Signed-off-by: Kenneth Chew <79120643+kthchew@users.noreply.github.com>
Signed-off-by: Kenneth Chew <79120643+kthchew@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant