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

macos: Update our macOS entitlements #639

Merged
merged 1 commit into from
Oct 22, 2024
Merged

macos: Update our macOS entitlements #639

merged 1 commit into from
Oct 22, 2024

Conversation

apyrgio
Copy link
Contributor

@apyrgio apyrgio commented Dec 6, 2023

Our entitlements were last updated when Dangerzone was considering using HyperKit to spawn VMs (9158d02, on 2021-06-30). Now that we use Docker Desktop, we can make them stricter.

Fixes #638

@apyrgio apyrgio force-pushed the 2023-12-entitlements branch 2 times, most recently from 890f1ae to 2f1fbd3 Compare December 6, 2023 15:55
Copy link
Contributor

@deeplow deeplow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apparently this is not working. Reverting my review.

@apyrgio
Copy link
Contributor Author

apyrgio commented Jan 30, 2024

This PR is blocked because the suggested entitlements break mounting files in the container. This will have to wait until we resolve #625, in which case, we will pass files via the container's standard streams.

@apyrgio apyrgio added this to the 0.7.0 milestone Jun 3, 2024
@almet almet removed this from the 0.7.0 milestone Jun 12, 2024
@almet almet force-pushed the 2023-12-entitlements branch from 2f1fbd3 to 5d779a2 Compare October 17, 2024 15:18
@almet
Copy link
Member

almet commented Oct 17, 2024

Now that #625 has been merged in main, I've rebased this on top.

@almet almet dismissed deeplow’s stale review October 17, 2024 15:20

Relaxing Deeplow from his dangerzone duties :-)

@apyrgio
Copy link
Contributor Author

apyrgio commented Oct 21, 2024

Just tested this PR once more. I'm afraid that the recently merged on-host conversion PR does not solve this issue. I've encountered the following issues:

Accessing the Docker socket

Opening the GUI app shows the following error message:

Screenshot From 2024-10-21 18-43-13

Note that there's indeed no Unix Domain socket there. When I attempt to specify it via the DOCKER_HOST envvar, I get this error:

Screenshot From 2024-10-21 18-44-35

Accessing files from the Dangerzone CLI

Pointing the dangerzone-cli to a file in the host fails with "file is not found":

user@macos ~ % /Applications/Dangerzone.app/Contents/MacOS/dangerzone-cli dangerzone/tests/test_docs/sample-pdf.pdf
Input file not found: make sure you typed it correctly.
user@macos ~ % ls dangerzone/tests/test_docs/sample-pdf.pdf
dangerzone/tests/test_docs/sample-pdf.pdf

My understanding is that sandboxed apps cannot read any file in the host, but instead have to open a browser window, and let the user choose the file on their own. See: https://developer.apple.com/documentation/security/accessing-files-from-the-macos-app-sandbox

@apyrgio
Copy link
Contributor Author

apyrgio commented Oct 21, 2024

One question I had while looking into this is, what's the deal with Docker Desktop? How can the Docker CLI access the Unix Domain Socket of the Docker server?

Turns out that two things happen. First, Docker is not a sandboxed app, as we can see from the missing entitlement in the output below:

user@macos ~ % codesign -d --entitlements - /Applications/Docker.app/Contents/MacOS/Docker\ Desktop.app/Contents/MacOS/Docker\ Desktop
[Dict]
        [Key] com.apple.application-identifier
        [Value]
                [String] 9BNSXJN65R.com.docker.docker
        [Key] com.apple.security.application-groups
        [Value]
                [Array]
                        [String] group.com.docker
        [Key] com.apple.security.cs.allow-jit
        [Value]
                [Bool] true
        [Key] com.apple.security.cs.allow-unsigned-executable-memory
        [Value]
                [Bool] true

Second, the Docker CLI belongs in the same application group as Docker Desktop (group.com.docker), meaning that it has unconditional access to files that Docker Desktop has created:

user@macos ~ % codesign -d --entitlements - /usr/local/bin/docker
[Dict]
        [Key] com.apple.application-identifier
        [Value]
                [String] 9BNSXJN65R.com.docker.docker
        [Key] com.apple.security.application-groups
        [Value]
                [Array]
                        [String] group.com.docker

Remove some macOS entitlements that are not necessary for the current
iteration of Dangerzone. Those are the ability to run as a hypervisor,
and the ability to accept network connections. They are a relic from
when we were experimenting with VMs, instead of relying on Docker
Desktop.
@apyrgio apyrgio force-pushed the 2023-12-entitlements branch from 5d779a2 to f524207 Compare October 21, 2024 16:16
@apyrgio
Copy link
Contributor Author

apyrgio commented Oct 21, 2024

Still, even though we can't go full app sandbox here, it doesn't mean that we can't ditch some older entitlements that are no longer used. I have updated this PR to remove some that I found, and tested that the resulting .dmg works on macOS.

@apyrgio apyrgio added this to the 0.8.0 milestone Oct 21, 2024
Copy link
Member

@almet almet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the investigation on this. Too bad it's not possible. Neverhtless, let's merge the removal of these old permissions :-)

@apyrgio apyrgio merged commit f524207 into main Oct 22, 2024
90 checks passed
@apyrgio apyrgio deleted the 2023-12-entitlements branch October 22, 2024 13:04
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.

Make our macOS entitlements stricter
3 participants