sparkle-project / Sparkle

A software update framework for macOS
https://sparkle-project.org
Other
7.46k stars 1.05k forks source link

Improve security for validating archives #2590

Open zorgiepoo opened 4 months ago

zorgiepoo commented 4 months ago

Sparkle extracts zip, tar, and dmg archives before validating the archive and validating the Apple code signing identity of the app contained inside the archives. This allows key rotation which lets a developer change their EdDSA signing key or their Apple Code Signing identity. Key rotation has been used by developers when losing keys or transferring ownership.

However there can be exploits in extracting archives themselves. We can increase security by always validating archives before extraction. Any updater that validates the downloaded archive ought to really do this (validating the archive provides better security than only validating the app contained inside it after extraction). We will still allow key rotation but only through code signed disk images.

This proposal means that we now check the Apple code signing signature of disk images, and only accept zip or tar archives that have been signed with Sparkle's (Ed)DSA keys. If the Apple code signing identity of the disk image does not match the original app to be updated, we will check its EdDSA signature. Key rotation will not be possible with zip or tar archives anymore.

To support validating the code signing identity of a disk image matches the host app, we will need to write a custom code signing requirement that tests the disk image is Apple issued, Developer ID signed, and has a matching Team ID as the host app. We will need to ignore the code signing identifier of the file.

For external (out-of-app) updaters, we may need to carve out a legacy path to continue allowing updates using the old policy for apps using older versions of Sparkle.

This also means that we will move towards recommending serving code signed disk images, rather than zip files, for updates. Historically disk image files have been hacky and slow to extract. However the next release (2.7) should make disk image extraction efficiency (with modern compression/formatting options) and fine-grained progress reporting on par with zip files. And it turns out there are cases where zip files are actually not well specified and less reliable to extract (e.g. #2544, #1691) -- not to mention they are worse for avoiding issues relating to App translocation which impact updating through Sparkle.

I think we will continue to keep a deprecated / log warning for apps that don't use EdDSA signing. There are several reasons for this: to support local development/testing which often may use a non-Developer ID certificate, support more efficient updates via delta updates, better safety/fallback in case the Apple signing certificate needs to change, avoiding code signing checks in the framework to determine update validity.

This is all agnostic to serving over HTTPS (our strongest line of defense still). Even today, a successful exploit would need to first compromise a developer's server.

Now I will need to test this to see if it can be done.

TLDR; better validation security in exchange for:

kornelski commented 4 months ago

Security wise this is the right thing to do.

However, key management and signing are already painful and annoying. For users who have set up tarball based pipeline, switching to DMG may add to the frustration.

Can this be made easier somehow?

zorgiepoo commented 4 months ago

However, key management and signing are already painful and annoying. For users who have set up tarball based pipeline, switching to DMG may add to the frustration.

These users who are using tarballs or zip files need to switch if:

Otherwise they can continue to use the tarball based pipeline.

I'm not familiar with the track record of vulnerabilities in libarchive, ditto/Bom.framework, or disk images but I don't like assuming they're inherently safe. As far as both tar and zip goes, I have seen compatibility issues with them being unable to extract, if not vulnerabilities; ZIP is not a simple format and both formats kind of kludge in support for Apple metadata. In one way I think they are less safe/reliable since they are not Apple-controlled formats. Granted, Sparkle is not the biggest target for these formats, but then again Sparkle can automatically/remotely update code. On that point, it would be a little bit odd to only assume non-code signed DMGs are unsafe while zip/tar.* are safe. If we assume these three formats are safe to decompress, then there's not much to change here since all other formats need to be validated before extracted. Extracting updates before validating the archives when we have validation in place for them is a bit silly, though.

I don't believe the installer/decompressor can be sandboxed feasibly (cannot use AppSandbox, not going to depend on deprecated sandbox_init).

A quarantine period is not feasible here and adds unwanted complexity. I don't see a good user experience here if the archive has to be downloaded first. The installer runs in a “separate” boundary from the framework and is also not in the business of tracking persistent state or trusting the client here.

Key rotation is not supposed to be a security issue from Sparkle’s point of view so I’m not sure about warning for it, but this could be a separate orthogonal issue. In some cases the OS handles this by alerting the user when an app that changes its code signing identity, depending on the functionality used. App sandboxing is one such feature that will definitely provide an alert. This can be tricky to convey.

Code signed / notarized disk images are Apple's preferred way of distributing applications. Distributing apps this way minimizes issues with app translocation (impacts updating) and better integrates with notarization/stapling. In reality, nowadays tarballs and zip files are not any significantly better than disk images for updates and worse in other ways (I plan to improve Sparkle's dmg support in the next release). Some developers have been distributing separate tarballs/zips for updates despite distributing disk images from their websites because of Sparkle, not because they want to (tarballs particularly are rarely distributed to users directly). From that use case, developers will be happy to serve a code signed disk image for both websites and updates.

If I want to make key management simpler while having security validation of archives and aligning to the platform's way of distributing apps, I will re-consider (un-deprecate) the path that allows a developer to distribute their app without using any of Sparkle’s signing keys, and instead by serving signed disk images. This could allow dropping Sparkle’s signing keys too. We have had one such request for this before. Then, for users that want to use serve significantly more efficient updates than zip/tar/dmg or want Sparkle key rotation*, they can use Sparkle's keys.

Appreciate the feedback here by the way. I will ponder on it.

* rotation would still be possible using different Developer ID certificates, assuming they are not revoked/lost