Sparkle: Secure architecture for updating in a sandboxed environment

[kornelski]

🔥 Here's the new implementation that will be Sparkle 2.0: https://github.com/sparkle-project/Sparkle/tree/ui-separation-and-xpc 🔥 It implements the changes discussed in the issue below.

---

There has been a very old branch with a sandbox support, but Andy has rejected it due to unresolved security issues.

So I'm opening an issue here to start a discussion how we can support sandboxed applications in Sparkle while keeping the sandbox as tight as possible.

I think minimal requirements are:

• The XPC helper _must not_ trust the host app at all, so it _must_ verify integrity of downloaded update and unpack it by itself. The current approach of telling XPC helper "hey, install this path, I've unpacked it and checked it's ok" can't be secure.
• the XPC can't trust the host to supply a valid DSA key (a compromised host could give a valid and signed update, but signed by the wrong person). The XPC helper _must_ find the trusted DSA key by itself (i.e. find the app's path and read Info.plist from there) and refuse to update any other path.
• Apple Code Signing verification code needs to be refactored (or not supported in sandboxed environment), because the host app checks against its own signature, but the host app is not supposed to be doing the checking.

[fernandomorgan]

Hi, updating my status - so, I do have a fork working since May or so, but I am still waiting the legal department ok to publish it.

I don't think it is in finished in terms of having completely addressing Andy's requirements BUT it does at least code signing validation in the XPC copy process.

I am mailing legal today - I do need to write a small doc about the changes, so I share the blame of the lateness - but I probably won't get an answer until tomorrow (California time), and then I will update this item.

I have distributed around 10 updates or so to around 1000 users, mostly in 10.9. There is a very small percentage in 10.8 and only test machines in 10.7. For a smaller group of 20-30 people I send around 5+ upgrades every day (our dev team, etc).

[SevenBits]

@fernandomorgan That's great! Look forward to hearing more on this!

[fernandomorgan]

Ok, here is the first release - I used a fork from your repo, Pornel

https://github.com/yahoo/Sparkle

[SevenBits]

@fernandomorgan Could you perhaps include documentation on how to set it up? Are any special steps required to get it to work with sandboxing?

[fernandomorgan]

actually you don't need to do anything. If your app isn't sandboxed, it uses the old copy. If the app is sandboxed, it will use a XPC helper - com.yahoo.Sparkle.SandboxService.xpc

I will add a note on distributing this helper

[jakepetroules]

@fernandomorgan Can you rebase on the latest master and send us a pull request please?

[fernandomorgan]

Somewhere, somehow, I guess some git info from the old Pornells fork was lost.

[kornelski]

I'll rebase it, don't worry

[fernandomorgan]

great! thanks

[kornelski]

The yahooxpc branch is ready for review.

Due to recent massive reorganization of the repository (sigh) it wasn't easy to merge and there are some messy merge conflicts/regressions, but at least we can start reviewing security of it and test it on top of the codebase.

[jakepetroules]

Are you sure you pushed? It looks like it's still 32 commits behind master.

[kornelski]

ok, 32 more

[jakepetroules]

OK, I've fixed this up enough that it actually builds. Some things might have been lost during your merging, Xcode complains of a missing implementation for a method in SUUpdater. Probably should compare diffs manually to look things over.

[kornelski]

Ah, yes. I've been brutally skipping over conflicting bits that didn't seem worth pulling out, e.g. log statements, 10.4 support.

One notable thing I've skipped was support for JSON appcasts. While JSON is fashionable, I don't think it solves enough problems, also xkcd-927.

[SevenBits]

I'd be happy to test things tomorrow; I should have some free time. I've been waiting for this feature for quite awhile so it is nice to see this actually happening.

On Jul 3, 2014, at 7:46 PM, Jake Petroules [email protected] wrote:

OK, I've fixed this up enough that it actually builds. Some things might have been lost during your merging, Xcode complains of a missing implementation for a method in SUUpdater. Probably should compare diffs manually to look things over.

—
Reply to this email directly or view it on GitHub.

[jakepetroules]

JSON does not have XML schema validation either. :)

[fernandomorgan]

Yeah. The thing with JSON is that it's pretty easy for the build process to generate a JSON file automatically. That's how I use it

The fork is 10.7+ only because of JSON. Originally used sbjson for 10.6+ later support but we dropped it. Currently only testing from 10.7 to 10.10 but we use both sandboxed and non-sb versions.

Sent from my iPad

On Jul 3, 2014, at 5:01 PM, Jake Petroules [email protected] wrote:

JSON does not have XML schema validation either. :)

—
Reply to this email directly or view it on GitHub.

[jtbandes]

Any updates on this review? Any way others can help?

[SevenBits]

I’ve been trying it out and will write a full report when I’m done. Others may certainly help.

On Jul 7, 2014, at 3:43 PM, Jacob Bandes-Storch [email protected] wrote:

Any updates on this review? Any way others can help?

—
Reply to this email directly or view it on GitHub.

[fernandomorgan]

Thanks! I am hoping to continue improving it too

On Monday, July 7, 2014 12:44 PM, SevenBits [email protected] wrote:

I’ve been trying it out and will write a full report when I’m done. Others may certainly help.

On Jul 7, 2014, at 3:43 PM, Jacob Bandes-Storch [email protected] wrote:

Any updates on this review? Any way others can help?

—
Reply to this email directly or view it on GitHub.

—
Reply to this email directly or view it on GitHub.

[SevenBits]

Just to confirm, is the XPC support present in the master branch of this repository and not just in Fernando's fork?

[fernandomorgan]

AFAIK, it's only in the yahooxpc branch/  https://github.com/yahoo/Sparkle )

it needs the XPC service - check the sandbox_service target)  then SUBasicUpdateDriver's 

• (void)installWithToolAndRelaunch:(BOOL)relaunch displayingUserInterface:(BOOL)showUI

has this to determine if we are in a sandbox environment or not 

   NSDictionary* environ = [[NSProcessInfoprocessInfo] environment];
    BOOL inSandbox = (nil != [environ objectForKey:@"APP_SANDBOX_CONTAINER_ID"]);
BOOLrunning10_7 = floor(NSAppKitVersionNumber) > NSAppKitVersionNumber10_6;
BOOL useXPC = running10_7 && inSandbox &&
    [[NSFileManagerdefaultManager] fileExistsAtPath: [[hostbundlePath] stringByAppendingPathComponent:@"Contents/XPCServices/com.yahoo.Sparkle.SandboxService.xpc"]];
    SULog(@"installWithToolAndRelaunch - using xpc=%d", useXPC);

  
... and in a sandbox, we us the XPC service instead

if( useXPC )
[SUXPC launchTaskWithLaunchPath: relaunchToolPath arguments:arguments];
else
        [NSTask launchedTaskWithLaunchPath: relaunchToolPath arguments: arguments];

the XPC service has to include the code signing verifier to, at a minimum (there's room to improve on this), to validate the signature before installing.

On Monday, July 7, 2014 1:00 PM, SevenBits [email protected] wrote:

Just to confirm, is the XPC support present in the master branch of this repository and not just in Fernando's fork?
—
Reply to this email directly or view it on GitHub.

[jakepetroules]

@fernandomorgan This is a better way of checking if you're sandboxed:

- (BOOL)isSandboxed
{
    BOOL sandboxed = NO;

    SecStaticCodeRef code = NULL;
    SecStaticCodeCreateWithPath((__bridge CFURLRef)[self bundleURL], kSecCSDefaultFlags, &code);

    SecRequirementRef requirement = NULL;
    SecRequirementCreateWithString(CFSTR("entitlement[\"com.apple.security.app-sandbox\"] exists"), kSecCSDefaultFlags, &requirement);

    if (code && requirement)
        sandboxed = (SecStaticCodeCheckValidity(code, kSecCSBasicValidateOnly, requirement) == errSecSuccess);
    if (requirement)
        CFRelease(requirement);
    if (code)
        CFRelease(code);

    return sandboxed;
}

@SevenBits It's in the yahooxpc branch for now, we've not yet merged it into master.

[grouchal]

I have been trying the yahooxpc branch and have tried to configure the Test App to work with the xpc code for an update. It seems the branch is not working at the moment for a few reasons:

• The sandbox_service target doesn't include the main.m file for the sandbox_service.
• The sandbox_service doesn't include the sparkle.framework
• The SANDBOX_SERVICE_NAME_STR seems to be empty
• The Test App doesn't have the sandbox_service included in the build phases and installed in the XPCServices folder
• The current code has the xpc check commented out and the check uses the empty SANDBOX_SERVICE_NAME_STR which causes it not to work.
• The update that is downloaded from the default service is not signed properly and causes update erros.

I have worked around all of these issues and can get the service to work, but think before the xpc changes are merged into the master these issues should be addressed to make it more obvious for others.

[SevenBits]

I found those same issues as well. They definitely need to be addressed.

On Jul 16, 2014, at 5:22 AM, Al Briggs [email protected] wrote:

I have been trying the yahooxpc branch and have tried to configure the Test App to work with the xpc code for an update. It seems the branch is not working at the moment for a few reasons:

The sandbox_service target doesn't include the main.m file for the sandbox_service.
The sandbox_service doesn't include the sparkle.framework
The SANDBOX_SERVICE_NAME_STR seems to be empty
The Test App doesn't have the sandbox_service included in the build phases and installed in the XPCServices folder
The current code has the xpc check commented out and the check uses the empty SANDBOX_SERVICE_NAME_STR which causes it not to work.
The update that is downloaded from the default service is not signed properly and causes update erros.
I have worked around all of these issues and can get the service to work, but think before the xpc changes are merged into the master these issues should be addressed to make it more obvious for others.

—
Reply to this email directly or view it on GitHub.

[jtbandes]

Testing this stuff out, it seems like the problem experienced by the current master branch, when the host is sandboxed, is that +[NSTask launchedTaskWithLaunchPath:arguments:] fails, raising the exception Couldn't posix_spawn: error 1 and logging the following to the console:

kernel[0]: exec of ~/Library/Containers/.../.Sparkle/Autoupdate.app/Contents/MacOS/Autoupdate denied since it was quarantined by ... and created without user consent, qtn-flags was 0x00000006`

However, it seems the same error occurs on this branch (only in the XPC service, since it's the one doing the +launch...). Is nobody else experiencing this? Setting up LSFileQuarantineExcludedPathPatterns seems like it would be the wrong solution, especially since the non-XPC version has the same issue.

[fernandomorgan]

There's some code specifically to remove the quarantine bit. Might be missing

Sent from my iPad

On Jul 17, 2014, at 5:21 PM, Jacob Bandes-Storch [email protected] wrote:

Testing this stuff out, it seems like the problem experienced by the current master branch, when the host is sandboxed, is that +[NSTask launchedTaskWithLaunchPath:arguments:] fails, raising the exception Couldn't posix_spawn: error 1 and logging the following to the console:

kernel[0]: exec of ~/Library/Containers/.../.Sparkle/Autoupdate.app/Contents/MacOS/Autoupdate denied since it was quarantined by ... and created without user consent, qtn-flags was 0x00000006`

However, it seems the same error occurs on this branch (only in the XPC service, since it's the one doing the +launch...). Is nobody else experiencing this? Setting up LSFileQuarantineExcludedPathPatterns seems like it would be the wrong solution, especially since the non-XPC version has the same issue.

—
Reply to this email directly or view it on GitHub.

[jtbandes]

It seems the removexattr call is failing; errno is set to EPERM. This kind of makes sense. But I'm not sure why it's working for anyone else.

[MaddTheSane]

387 added 10.10 code for removal of the quarantine information.

[jtbandes]

I'm testing on 10.9, by the way — I think the sandbox may enforce the quarantine attr.

[fernandomorgan]

Just wanted to warn about a situation I just noticed. It might be rare, but, someone might find it useful

When updating a sandboxed application, Sparkle checks the code signing before copying the new files. In 10.7 and 10.8 only (didn’t see this in 10.9 or 10.10), if somehow the original code was code signed with a .DS_Store but it wasn’t in the build itself, it will fail.

My scenario was with a framework that I manually code signed in my machine and then uploaded to a git repo, but as I have .DS_Store in my .gitignore (we have several team members in windows) our build machine created a new distro without the .DS_Store file. So, probably not a common one, but, adding it here as reference/warning

F

[grouchal]

OK I have created a pull request to get the example app to work for yahooxpc - but there is a problem with the signing of the example app update - this needs to be fixed too.

[fernandomorgan]

I am sure everyone did got this, but Apple published a updated note on code signing for 10.9.5 and Yosemite Dev5, and this also includes information on code signing v1 versus v2

https://developer.apple.com/library/prerelease/mac/technotes/tn2206/_index.html

[grouchal]

OK - now that 1.8 has been released can the xpc stuff be brought into the master branch from the xpc branch?

I would really like to use the master branch for our software and perhaps sanded updates could be the main point of the 1.9 release?

[SevenBits]

I second this. This should be merged into the master branch and work should be done to release it as soon as is feasible. Sandboxed updates are a big necessity for developers, especially those deploying through the App Store, and this is needed to prevent requiring completely different code bases (or one significantly more complicated code base).

The longer that this is put off, the more headaches there will be. There very likely are security holes in this present implementation, but it's better to start working on it now than to let it chill on ice indefinitely.

On Sep 19, 2014, at 3:40 AM, Al Briggs [email protected] wrote:

OK - now that 1.8 has been released can the xlc stuff be brought into the master branch?

I would really like to use the master branch for our software and perhaps sanded updates could be the main point of the 1.9 release?

—
Reply to this email directly or view it on GitHub.

[jakepetroules]

Yes, we should start working on getting that merged...

[grouchal]

I shall see what the issues are and report back - or even create a pull request if it not to difficult to do a merge at this point

[jakepetroules]

The biggest obstacle is merging master into yahoo_xpc first, since there are quite a few conflicts.

[kornelski]

I'll merge that

[grouchal]

OK I shall leave it to you @pornel

[kornelski]

I've done more merging on the branch. But I'm going for holidays now, so I'll leave it to you to merge it to master (it may need few small fixes).

[jakepetroules]

I've done some more fixes and merged master into yahooxpc. So, after final review and testing and whatever else may be needed, yahooxpc should be almost ready to merge into master.

[grouchal]

It seems that all that needs to be done now is review and test the yahooxpc branch - is there anything I can do to help with this or does it now rely on you guys?

[SevenBits]

Has any progress been made on this?

[pfandrade]

Hello, have been trying out this branch for a sandboxed app. A couple of problems I found:

1. SUBasicUpdateDriver is looking for the xpc process in the wrong place:

- [[NSFileManager defaultManager] fileExistsAtPath: [sparkleBundle.bundlePath stringByAppendingPathComponent:[NSString stringWithFormat:@"Contents/XPCServices/%@.xpc", @(SPARKLE_SANDBOX_SERVICE_NAME)]]];
+ [[NSFileManager defaultManager] fileExistsAtPath: [sparkleBundle.bundlePath stringByAppendingPathComponent:[NSString stringWithFormat:@"XPCServices/%@.xpc", @(SPARKLE_SANDBOX_SERVICE_NAME)]]];

It's an easy fix.

1. I'm also seeing the problem where AutoUpdate can't be executed because of the quarantine.

01/12/14 11:12:38,000 kernel[0]: exec of (…)/.Sparkle/Autoupdate.app/Contents/MacOS/Autoupdate denied since it was quarantined by Secrets and created without user consent, qtn-flags was 0x00000006

I haven't been able to solve this one… I've been playing with LSFileQuarantineEnabled and LSFileQuarantineExcludedPathPatterns both it appears the quarantine bit is always set.

Is anyone using this branch successfully?

PS: Using OS X 10.10

[fernandomorgan]

I used this fork in 10.7 to 10.10 in around 3000 Macs but I think the sandboxed version in the main project isn’t finished.

https://github.com/yahoo/Sparkle https://github.com/yahoo/Sparkle

Unfortunately, my project that used Sparkle was put on hold and I haven’t had much time (or guinea pigs - I mean, testers) with Sparkle.

F

On Dec 1, 2014, at 4:32 AM, Paulo Andrade [email protected] wrote:

Hello, have been trying out this branch for a sandboxed app. A couple of problems I found:

SUBasicUpdateDriver is looking for the xpc process in the wrong place:

• [[NSFileManager defaultManager] fileExistsAtPath: [sparkleBundle.bundlePath stringByAppendingPathComponent:[NSString stringWithFormat:@"Contents/XPCServices/%@.xpc", @(SPARKLE_SANDBOX_SERVICE_NAME)]]];
• [[NSFileManager defaultManager] fileExistsAtPath: [sparkleBundle.bundlePath stringByAppendingPathComponent:[NSString stringWithFormat:@"XPCServices/%@.xpc", @(SPARKLE_SANDBOX_SERVICE_NAME)]]];
  It's an easy fix.

I'm also seeing the problem where AutoUpdate can't be executed because of the quarantine.
01/12/14 11:12:38,000 kernel[0]: exec of (…)/.Sparkle/Autoupdate.app/Contents/MacOS/Autoupdate denied since it was quarantined by Secrets and created without user consent, qtn-flags was 0x00000006
I haven't been able to solve this one… I've been playing with LSFileQuarantineEnabled and LSFileQuarantineExcludedPathPatterns both it appears the quarantine bit is always set.

Is anyone using this branch successfully?

PS: Using OS X 10.10

—
Reply to this email directly or view it on GitHub https://github.com/sparkle-project/Sparkle/issues/363#issuecomment-65058377.

[pfandrade]

Your branch has the same problem related to AutoUpdate and the quarantine bit, at least running on my 10.10.1 machine.

[kainjow]

@pfandrade I believe the quarantine info is stored as an extended attribute. You could try recursively removexattr(path, "com.apple.quarantine", 0).

[fernandomorgan]

yes, its what is done here:

SUPlainInstallerInternals.m

• (void)releaseFromQuarantine:(NSString*)root

you can look the attributes from the command line with xattr -l

the call above removes them - and as far as I know, this is the original code (from Andy M)

F

On Dec 1, 2014, at 7:21 AM, Kevin Wojniak [email protected] wrote:

@pfandrade https://github.com/pfandrade I believe the quarantine info is stored as an extended attribute. You could try recursively removexattr(path, "com.apple.quarantine", 0).

—
Reply to this email directly or view it on GitHub https://github.com/sparkle-project/Sparkle/issues/363#issuecomment-65079377.

[kainjow]

Well for starters the return value of removexattr and errno is not checked, so who knows if that's even working.

[kainjow]

BTW, not a big deal, but the extended attribute name can be UTF-8 (An extended attribute's name is a simple NULL-terminated UTF-8 string.), so I would change

const char *attr = [name cStringUsingEncoding:NSASCIIStringEncoding];

to

const char *attr = [name UTF8String];

[pfandrade]

This may be a problem with my machine, but the +releaseFromQuarantine: isn't working. I can see it attempting to remove the attribute but it has no effect.

To attempt to debug this problem I've enabled sandboxing for the "Sparkle Test App" and confirm that I also see the same problem here.

For OS X >= 10.10 Sparkle uses [fileURL setResourceValue:[NSNull null] forKey:NSURLQuarantinePropertiesKey error:&error] which is failing silently (i.e. no error is returned).

I've tried falling back to removexattr function call (used for OS < 10.10) and it also fails returning -1.

Since nobody has seen this, I'm guessing is this behaviour has changed in a recent version of OS X. That, or my mac is screwed up somehow.

Can someone test this on their Mac? (i.e. sandbox the Sparkle Sample App and see if it works?)

[kainjow]

One more thing :)

setResourceValue: for NSURLQuarantinePropertiesKey is completely ignoring the NSError value. So it seems if this quarantine stuff is causing the updater to fail, then it needs to be a mandatory part of the installer and failures need to cause the updater to fail.

[kainjow]

I created a test sandboxed app and tried removexattr on a Safari downloaded file which was moved into the sandbox's container folder. It is failing with EPERM on 10.9.5.

Here's why - found in /System/Library/Sandbox/Profiles/application.sb

(deny file-write-xattr (xattr "com.apple.quarantine") (with no-log))

[SevenBits]

@kainjow Well, that's certainly a problem. Are we stuck, then?

[kainjow]

@SevenBits not sure what can be done. File a bug report with Apple to allow some type of workaround?

My question: why are sandboxed apps being distributed out of the MAS?

[ttscoff]

Because customer support is way easier when you have both MAS and
non-MAS versions if they have identical containers and permissions
settings... (i.e. that way they're both a pain in the ass)

On 22 Dec 2014, at 13:11, Kevin Wojniak wrote:

@SevenBits not sure what can be done. File a bug report with Apple to
allow some type of workaround?

My question: why are sandboxed apps being distributed out of the MAS?

---

Reply to this email directly or view it on GitHub:
https://github.com/sparkle-project/Sparkle/issues/363#issuecomment-67876687

[jtbandes]

...and so they're less potentially detrimental to the user's machine in case of compromise. The same reason MAS apps are sandboxed.

[SevenBits]

@kainjow Because supporting two code bases for the same application is no easy chore. It's enough of a job simply having two application builds, one with Sparkle and one without, and even if we could use macros to alleviate some of the work, non-sandboxed versions will still have different support directories, etc, so you need different sets of code. It's not as easy as it appears to be.

Here's an idea: the new application (i.e the updated copy) is copied over the old version by an XPC service, right? So why not add the code to remove the quarantine in the XPC service? It seems that removing the quarantine bit is blocked in sandboxed apps, but since the XPC service can run with separate permissions, and thus not be sandboxed, couldn't the XPC sandbox take care of that before it copies so that the new app can be launched successfully?

[kainjow]

@SevenBits worth a try. I was under the impression if the app is sandboxed, the XPC helper would have to be too. But if the OS isn't enforcing that (and only MAS verification is), it may work.

After briefly searching, I didn't find any documentation stating either way.

[kornelski]

I've realized doing update securely is harder than it seems :(

• The XPC helper _must not_ trust the host app at all, so it _must_ verify integrity of downloaded update and unpack it by itself. The current approach of telling XPC helper "hey, install this path, I've unpacked it and checked it's ok" can't be secure.
• the XPC can't trust the host to supply a valid DSA key (a compromised host could give a valid and signed update, but signed by the wrong person). The XPC helper _must_ find the trusted DSA key by itself (i.e. find the app's path and read Info.plist from there) and refuse to update any other path.
• Apple Code Signing verification code needs to be refactored (or not supported in sandboxed environment), because the host app checks against its own signature, but the host app is not supposed to be doing the checking.

[kornelski]

Alternatively, we could release version of Sparkle in the current state — without this verification. This would effectively be a complete sandbox bypass (a compromised "sandboxed" app can trick the XPC helper into replacing the app with malware).

It would solve problem of maintaining two codebases for sandboxed and non-sandboxed environment, and wouldn't be any more insecure than no sandbox at all. We just need to be clear that sandboxing + sparkle = no sandbox security.

[SevenBits]

@kainjow This approach would work. It only depends on whether or not @pornel feels comfortable taking it. XPC services do _not_ have to be sandboxed if the main app is; that is a Mac App Store requirement, but it is not enforced by the system and is perfectly valid. Therefore, I think that any code related to handling sandboxing should be done in one or more XPC services, so that they do not inherit the app sandbox.

@pornel Because people have been waiting on this for a long time, I say release it now as a "preview" or "beta" pending a more secure architecture for handing sandbox, _but_ I would make the potential vulnerabilities very clear and leave it up to developers as to whether or not this is a risk they are willing to take.

[bdolman]

I tried @SevenBits suggestion to make the XPC service remove the quarantine bit. It worked and I was able to complete an app update successfully.

My quick and dirty approach was to do this directly in peer_event_handler immediately before launching the task, but there's probably a better place to do it. Does someone already have this implemented or should I submit a newbie PR?

Personally I'm okay with the security caveats of this approach. In my case I'd be happy releasing a non-sandboxed version of my app for beta testing, but it would be such a pain to maintain sandboxed vs. non-sandboxed versions.

[SevenBits]

@bdolman @pornel @jakepetroules That's great, so my theory is confirmed... I think that someone should just bite the bullet and release their fork of the application with this feature. I think that everyone is hoping that someone else will swoop in and finish it. We should either recommend people to an unofficial fork with this feature included or just all band together to do this... it will take work and some design time, but the alternative is this (and 1.9) stalling indefinitely.

[SevenBits]

@pornel Any particular reason why this was pushed back to 2.0? Is 1.9 even out yet? Under Releases it just says 1.8 as latest...

Also, @fernandomorgan @pornel @jakepetroules Which repository has the more updated Spatkle code? @fernandomorgan at yahoo/Sparkle or the yahooxpc branch of this repository? All they both equal? I would like to start using this in a shipping app (right now, Sparkle is forcing users to manually download all new updates, which is pretty annoying) and I would like to know what specifically I will need to evaluate. I think that if we can at least get some features working, then that would be better than nothing.

[kornelski]

Because of the "secure" part.

I can't make the current code secure — this branch has the same flaw like any other XPC flavor of Sparkle floating around: allows compromised host app to tell XPC helper to execute arbitrary code and completely escape the sandbox.

Proper architecture must move almost all of Sparkle's code to the XPC process (reading DSA key, archive validation and decompression).

I don't think I can do that in the near future, and I don't want to block Sparkle's progress on that.

[xhacker]

@bdolman: Do you have a fork for the fix? I’m trying to integrate Sparkle into a sandboxed app.

[ksuther]

@xhacker: Fantastical 2 uses this fork of Sparkle: https://github.com/ksuther/Sparkle/tree/xpc

It has the changes from the yahooxpc branch reintegrated into master so it can track master more easily.

[SevenBits]

@ksuther And does this fork work with sandboxing out of the box?

On May 17, 2015, at 2:37 PM, Kent Sutherland [email protected] wrote:

@xhacker https://github.com/xhacker: Fantastical 2 uses this fork of Sparkle: https://github.com/ksuther/Sparkle/tree/xpc https://github.com/ksuther/Sparkle/tree/xpc
It has the changes from the yahooxpc branch reintegrated into master so it can track master more easily.

—
Reply to this email directly or view it on GitHub https://github.com/sparkle-project/Sparkle/issues/363#issuecomment-102833791.

[ksuther]

@SevenBits: Yes, just remember that all the caveats mentioned above apply. This is a subpar long term solution.

[SevenBits]

@ksuther Yes, I am aware. I am only looking at this as a temporary workaround until @pornel can find time to re-write the necessary parts of Sparkle to make this fully secure.

[sweetppro]

sorry for the hijack,
Im in the same boat here, I just need a sandboxed version of Sparkle that works till a more secure version is released.

@ksuther I keep getting this error, using the xpc fork:
Sparkle: Can't mount DMG

it downloads just fine, but won't mount.
Is there something special i need to do to get this working?

[fernandomorgan]

That generally means that the DMG file is corrupted
You should be able to manually download the DMG and open it.

On May 17, 2015, at 8:13 PM, SweetPPro [email protected] wrote:

sorry for the hijack,
Im in the same boat here, I just need a sandboxed version of Sparkle that works till a more secure version is released.

I keep getting this error, using the xpc fork:
Sparkle: Can't mount DMG

it downloads just fine, but won't mount.
Is there something special i need to do to get this working?

—
Reply to this email directly or view it on GitHub https://github.com/sparkle-project/Sparkle/issues/363#issuecomment-102906897.

[sweetppro]

hmm,
it does download just fine, and I can even manually mount the file which Sparkle downloads.

but Sparkle gives these errors:
Sparkle: Extracting /Users/..../Library/Containers/..../Data/Library/Caches/..../Sparkle/MyApp 1.5/MyApp.dmg as a DMG
Sparkle: hdiutil failed with code: 1 data: < Sparkle: Can't mount DMG /Users/..../Library/Containers/..../Data/Library/Caches/..../Sparkle/MyApp 1.5/MyApp.dmg

[ksuther]

@sweetppro It's entirely possible my branch doesn't work with dmgs, I only use it with zips.

[sweetppro]

@ksuther i did notice that you were using a zip archive, ill see if I can fix this myself tonight.... otherwise ill just use a zip too.
:)

edit.... ok, this is an issue with hdiutil not working under the sandbox....
guess ill just use a zip. /edit....

[fernandomorgan]

oh right, I only used it with zips also, I am sorry.

On May 17, 2015, at 9:35 PM, Kent Sutherland [email protected] wrote:

@sweetppro https://github.com/sweetppro It's entirely possible my branch doesn't work with dmgs, I only use it with zips.

—
Reply to this email directly or view it on GitHub https://github.com/sparkle-project/Sparkle/issues/363#issuecomment-102920077.

[SevenBits]

@ksuther I've adopted your fork, it seems to be working acceptably. I'll let everyone know if I find issues.

[digitalmoksha]

Has anyone used the DevMate fork? They claim it's sandbox enabled. Can it be used without the whole Devmate api?

[SevenBits]

@digitalmoksha I personally wasn't even aware of this fork until you just pointed it out, so, no, I've never tried it. But you'd probably be better off contacting the developers directly (you can get names and emails from the git commit logs). Ideally, I don't imagine that they'd restrict it to their own APIs, especially when you consider that they probably forked it for use in one of their products; you should be able to set it up like a normal Sparkle installation. In any case, any third-party fork is undesirable, although necessary, until the necessary features (XPC service support, better security, etc.) are integrated into the main tree.

On Jul 17, 2015, at 5:17 PM, digitalMoksha [email protected] wrote:

Has anyone used the DevMate fork? They claim it's sandbox enabled. Can it be used without the whole Devmate api?

—
Reply to this email directly or view it on GitHub.

[digitalmoksha]

@SevenBits Totally agree that a 3rd-party fork is undesirable. Would much prefer to have a solid solution here. I hope work is still going on with it. But if the devmate fork has solved the security issues, it might be worth taking a look at to bring it back here.

[kornelski]

The devmate fork disables all security of the sandbox and allows replacement of any executable (that the privileged helper has access to) with arbitrary code.

This is because, like all Sparkle forks so far, it doesn't do signature verification in the XPC helper:

https://github.com/DevMate/Sparkle/blob/master/Installation%20XPC%20Service/install_service.m#L24

It makes the mistake of trusting the host application, which in sandbox security model could be malicious.

I'm not aware of any Sparkle fork that would be any more secure than not having sandbox at all.

[digitalmoksha]

ok, so being that they are all insecure, i think the @ksuther fork would be better, since it seems to be more closely tied with this main project.

Am I right in understanding what the options are for my app:

1. Turn off sandboxing. Normal Sparkle is still "insecure", insomuch as there are no enhanced security mechanisms in place. The host could still be malicious.
2. Turn on sandboxing, and use the ksuther fork. Still "insecure", but app itself (assuming the installation isn't compromised, which could still happen above) is more secure due to the sandboxing.

If these assumptions are correct, is there any benefit of 1 over 2? I'm not trying to be snarky, just want to pick the best path to run down.

[ksuther]

There's a significant benefit of 2 over 1 if you're shipping on the Mac App Store. It allows the MAS and non-MAS versions to share a container and everything except for Sparkle.

[digitalmoksha]

You're right, the sharing is the other huge benefit, which is another reason I started down this road. Ok, off to use your fork. btw, thanks for the work you've done on that...

[ksuther]

No problem, although I only deserve the credit in as much that I went through the old yahooxpc branch and reintegrated it with master.

[digitalmoksha]

Quick question: does that branch "require" sandboxing, or does it run with or without it?

[ksuther]

It does not, if sandboxing is off it'll behave like master.

[digitalmoksha]

Perfect, thx

[SevenBits]

@ksuther I experimented with your custom fork a bit today... I can't get sandboxing support to actually work. It seems that Autoupdate.app can't be run because it's been quarantined by my app. Does your fork use the XPC service to remove the quarantine bit?

[ksuther]

@SevenBits It should, the XPC service has the quarantine handling in it.

[SevenBits]

@ksuther Yeah, I certainly expected it too, but instead it hangs on the Installing update... phase. Here are the error messages:

8/10/15 5:47:36.961 PM Mac Linux USB Loader[805]: Sparkle: ===== old =====
8/10/15 5:47:36.963 PM Mac Linux USB Loader[805]: Sparkle: Extracting /Users/user/Library/Containers/com.sevenbits.Mac-Linux-USB-Loader/Data/Library/Caches/com.sevenbits.Mac-Linux-USB-Loader/Sparkle/Mac Linux USB Loader 9999 2/Mac Linux USB Loader.zip using 'ditto -x -k - "$DESTINATION"'
8/10/15 5:47:43.000 PM kernel[0]: Mac Linux USB Lo[805] Unable to quarantine: 93
8/10/15 5:47:43.000 PM kernel[0]: Mac Linux USB Lo[805] Unable to quarantine: 93
8/10/15 5:47:43.000 PM kernel[0]: Mac Linux USB Lo[805] Unable to quarantine: 93
8/10/15 5:47:43.000 PM kernel[0]: Mac Linux USB Lo[805] Unable to quarantine: 93
8/10/15 5:47:43.000 PM kernel[0]: exec of /Users/user/Library/Containers/com.sevenbits.Mac-Linux-USB-Loader/Data/Library/Caches/com.sevenbits.Mac-Linux-USB-Loader/Sparkle/Autoupdate.app/Contents/MacOS/Autoupdate denied since it was quarantined by Mac Linux USB Loader and created without user consent, qtn-flags was 0x00000006
8/10/15 5:47:43.870 PM Mac Linux USB Loader[805]: Couldn't posix_spawn: error 1
8/10/15 5:47:43.874 PM Mac Linux USB Loader[805]: (
0 CoreFoundation 0x00007fff91a8b03c __exceptionPreprocess + 172
1 libobjc.A.dylib 0x00007fff9424c76e objc_exception_throw + 43
2 CoreFoundation 0x00007fff91a8aeed +[NSException raise:format:] + 205
3 Foundation 0x00007fff86944bbf -[NSConcreteTask launchWithDictionary:] + 3119
4 Foundation 0x00007fff86aed159 +[NSTask launchedTaskWithLaunchPath:arguments:] + 194
5 Sparkle 0x00000001030fbd1d -[SUBasicUpdateDriver installWithToolAndRelaunch:displayingUserInterface:] + 3895
6 Sparkle 0x000000010310adaa -[SUUIBasedUpdateDriver installWithToolAndRelaunch:] + 403
7 libsystem_trace.dylib 0x00007fff9477ccd7 _os_activity_initiate + 75
8 AppKit 0x00007fff90f59eb1 -[NSApplication sendAction:to:from:] + 452
9 AppKit 0x00007fff90f6f946 -[NSControl sendAction:to:] + 86
10 AppKit 0x00007fff90f6f862 __26-[NSCell _sendActionFrom:]_block_invoke + 131
11 libsystem_trace.dylib 0x00007fff9477ccd7 _os_activity_initiate + 75
12 AppKit 0x00007fff90f6f7bf -[NSCell _sendActionFrom:] + 144
13 libsystem_trace.dylib 0x00007fff9477ccd7 _os_activity_initiate + 75
14 AppKit 0x00007fff90f6dcb3 -[NSCell trackMouse:inRect:ofView:untilMouseUp:] + 2821
15 AppKit 0x00007fff90fc634f -[NSButtonCell trackMouse:inRect:ofView:untilMouseUp:] + 770
16 AppKit 0x00007fff90f6c366 -[NSControl mouseDown:] + 714
17 AppKit 0x00007fff914d62dc -[NSWindow _reallySendEvent:isDelayedEvent:] + 14125
18 AppKit 0x00007fff90e65c86 -[NSWindow sendEvent:] + 470
19 AppKit 0x00007fff90e62212 -[NSApplication sendEvent:] + 2504
20 AppKit 0x00007fff90d8bb68 -[NSApplication run] + 711
21 AppKit 0x00007fff90d08244 NSApplicationMain + 1832
22 libdyld.dylib 0x00007fff90a595c9 start + 1
23 ??? 0x0000000000000001 0x0 + 1
)

[zorgiepoo]

@SevenBits Do you know what OS X version that is occurring on? I am pretty confident the releasing from quarantine procedure doesn't work on < OS X 10.10 (see #575). Releasing the quarantine on Autoupdate.app in a non-sandboxed application is not really necessary, so that could have gone unnoticed. Also, that fork's quarantine removal is a bit outdated from master, and in either case there's no error checking to see if removing it fails.

[SevenBits]

I'm using 10.10.4.

On Aug 10, 2015, at 8:05 PM, Mayur Pawashe [email protected] wrote:

@SevenBits Do you know what OS X version that is occurring on? I am pretty confident the releasing from quarantine procedure doesn't work on < OS X 10.10 (see #575). Releasing the quarantine on Autoupdate.app in a non-sandboxed application is not really necessary, so that could have gone unnoticed. Also, that fork's quarantine removal is a bit outdated from master, and in either case there's no error checking to see if removing it fails.

—
Reply to this email directly or view it on GitHub.

[zorgiepoo]

Then I'd see if the operation fails, not to ignore the error (which is what the current code does in SUPlainInstallerInternals.m), and log the error. Of course before that, you have to detect if there's a quarantine attribute on the file to begin with (see master's current code).

[SevenBits]

Alright, I'll do this.

On Aug 10, 2015, at 8:30 PM, Mayur Pawashe [email protected] wrote:

Then I'd see if the operation fails, not to ignore the error (which is what the current code does in SUPlainInstallerInternals.m), and log the error. Of course before that, you have to detect if there's a quarantine attribute on the file to begin with (see master's current code).

—
Reply to this email directly or view it on GitHub.

[SevenBits]

@zorgiepoo Well, I finally got around to testing this and like you suggested I had it not ignore the error. First, here's the code I used:

NSURL *rootURL = [NSURL fileURLWithPath:root];
[rootURL setResourceValue:[NSNull null] forKey:NSURLQuarantinePropertiesKey error:NULL];

// Only recurse if it's actually a directory.  Don't recurse into a
// root-level symbolic link.
NSDictionary *rootAttributes = [manager attributesOfItemAtPath:root error:nil];
NSString *rootType = rootAttributes[NSFileType];

if (rootType == NSFileTypeDirectory) {
    // The NSDirectoryEnumerator will avoid recursing into any contained
    // symbolic links, so no further type checks are needed.
    NSDirectoryEnumerator *directoryEnumerator = [manager enumeratorAtURL:rootURL includingPropertiesForKeys:nil options:(NSDirectoryEnumerationOptions)0 errorHandler:nil];

    for (NSURL *file in directoryEnumerator) {
        NSError *error;

        __autoreleasing id value;
        if ([file getResourceValue:&value forKey:NSURLQuarantinePropertiesKey error:NULL]) {
                NSDictionary *dict = (NSDictionary *)value;
                NSLog(@"%@: got attributes. %@", file, dict);
        }

        [file setResourceValue:[NSNull null] forKey:NSURLQuarantinePropertiesKey error:&error];
        if (error) {
                NSLog(@"Couldn't remove quarantine bit for %@: %@", file, error);
        }
    }
}

And here's what I got in the logs:

8/16/15 7:53:32.000 PM kernel[0]: Mac Linux USB Lo[2415] Unable to quarantine: 93

8/16/15 7:53:32.400 PM Mac Linux USB Loader[2415]: Couldn't remove quarantine bit for file:///Users/user/Library/Containers/com.sevenbits.Mac-Linux-USB-Loader/Data/Library/Caches/com.sevenbits.Mac-Linux-USB-Loader/Sparkle/Autoupdate.app/Contents/MacOS/: Error Domain=NSCocoaErrorDomain Code=256 "The file “MacOS” couldn’t be opened." UserInfo=0x60800006d140 {NSURL=file:///Users/user/Library/Containers/com.sevenbits.Mac-Linux-USB-Loader/Data/Library/Caches/com.sevenbits.Mac-Linux-USB-Loader/Sparkle/Autoupdate.app/Contents/MacOS/, NSFilePath=/Users/user/Library/Containers/com.sevenbits.Mac-Linux-USB-Loader/Data/Library/Caches/com.sevenbits.Mac-Linux-USB-Loader/Sparkle/Autoupdate.app/Contents/MacOS, NSUnderlyingError=0x608000451f10 "The operation couldn’t be completed. (OSStatus error -1427.)"}

8/16/15 7:53:32.401 PM Mac Linux USB Loader[2415]: file:///Users/user/Library/Containers/com.sevenbits.Mac-Linux-USB-Loader/Data/Library/Caches/com.sevenbits.Mac-Linux-USB-Loader/Sparkle/Autoupdate.app/Contents/MacOS/Autoupdate has quarantine bit.

[zorgiepoo]

@SevenBits That's not indicative of a real issue. Error -1427 indicates the attribute was not found. You should only be setting the resource value if getResourceValue:forKey:error: succeeds (i.e, there is a quarantine bit on the file).

By the way as a tidbit, you should never test the presence or do anything with error objects when using Cocoa methods until you've made sure the operation has failed (here, if setResourceValue:forKey:error: returned NO). Otherwise, it's possible to run into some nasty bugs.

[SevenBits]

@zorgiepoo This was just a quick hack to find out what the issue might be. I don't intend to keep this code in the current form when I ship. I'll make the changes though, just to be on the safe side.

I will run some more tests tomorrow. I'm confused as to why the system would say the quarantine bit doesn't exist, only to then refuse to run it because it does...

[zorgiepoo]

I think it's common for not having a quarantine bit set on _every_ file in the bundle.

[SevenBits]

@zorgiepoo Okay, so I've done more testing. I've looked at some of the issues you've pointed out to me. Firstly, it appears that the auto-updater can't be launched. After the update is downloaded, Sparkle tries to launch the auto-updater, which fails with this error reported to the console:

8/19/15 5:11:46.000 PM kernel[0]: exec of /Users/user/Library/Containers/com.sevenbits.Mac-Linux-USB-Loader/Data/Library/Caches/com.sevenbits.Mac-Linux-USB-Loader/Sparkle/Autoupdate.app/Contents/MacOS/Autoupdate denied since it was quarantined by Mac Linux USB Loader and created without user consent, qtn-flags was 0x00000006

To try to remove the remove the quarantine bit, I use this code in SUBasicUpdateDriver.m:

    // Get autoupdater out of quarantine!!
    NSLog(@"Attempting to un-quarantine %@!", relaunchToolPath);
    NSURL *file = [NSURL fileURLWithPath:relaunchToolPath];
    __autoreleasing id value;
    NSError *error;
    if ([file getResourceValue:&value forKey:NSURLQuarantinePropertiesKey error:NULL]) {
        NSDictionary *dict = (NSDictionary *)value;

        // Remove the attribute
        if (dict) {
            if (![file setResourceValue:[NSNull null] forKey:NSURLQuarantinePropertiesKey error:&error]) {
                NSLog(@"Couldn't remove quarantine bit for %@: %@", file, error);
            } else { NSLog(@"Un-quarantined %@", file); }
        } else NSLog(@"Value for dict: %@", dict);

This fails, though, still showing the error above. Any ideas?

[zorgiepoo]

If the error code is NSFileReadNoPermissionError or NSFileWriteNoPermissionError then I assume it's a permission error. I think both the XPC Service and main app in that fork are trying to release the quarantine. Probably only the service should be trying that, though. You are also only showing code that releases the quarantine of the bundle I think, but not code that recursively removes the quarantine. I dunno.

[SevenBits]

@zorgiepoo Alright, well, I'll give it a shot another time when I have more time to experiment.

[xhacker]

@ksuther: Thank you for sharing your fork. Did you do anything other than just using the fork? I tried so hard (including merging the current master into your branch) but still keep getting quarantine errors.

[ksuther]

Nothing special that I can think of, sorry. It's working on both 10.10 and 10.11.

[sweetppro]

@ksuther I keep getting the following error on 10.11.1:
*** Terminating app due to uncaught exception 'NSInternalInconsistencyException', reason: 'Couldn't posix_spawn: error 1'
terminating with uncaught exception of type NSException
abort() called

Process: org.sparkle-project.Sparkle.SandboxService

Is there anything special i need to do to get this working?

[ksuther]

@sweetppro There shouldn't be anything special you have to do, not sure why you'd be getting that.

[sweetppro]

@ksuther cool thanks, maybe I just need a reboot or something equally as simple. Ill have another stab in the morning.

[SevenBits]

@ksuther @sweetppro I know it's been a while since I've tried, but I know that it didn't work for me either. I spelt out the details here. Has the fork been updated since then?

[sweetppro]

its strange, cause i have an older version of the xpc branch working in a different app within the sandbox.I must have some build flags set somewhere to allow it to work. If I can't get the newest version working, ill just roll back to the working framework.

[danielfe]

I also had trouble. Please report back if you figure it out!

On Dec 4, 2015, at 1:25 AM, SweetPPro [email protected] wrote:

its strange, cause i have an older version of the xpc branch working in a different app within the sandbox.I must have some build flags set somewhere to allow it to work. If I can't get the newest version working, ill just roll back to the working framework.

—
Reply to this email directly or view it on GitHub https://github.com/sparkle-project/Sparkle/issues/363#issuecomment-161917949.

[SevenBits]

@danielfe @sweetppro Can you guys give more specific info, such as logs, etc? That might be helpful for others who are also having trouble.

[sweetppro]

ok @danielfe @SevenBits so this is what i did to get this to work.
I manually code signed Autoupdate.App, org.sparkle-project.Sparkle.SandboxService.xpc, and Sparkle.framework (in that order) using:
codesign --force --sign "Developer ID Application: <your name>" <path to be signed>

I also added the flags
@loader_path/../Frameworks for Linking>>Runpath Search Paths
"$(SRCROOT)" for Search Paths>>Framework Search Paths

with all these options, updating a .zip file via HTTPS works!

[vincedev]

I'm facing problems to update my sandboxed app using Sparkle.
Any update about Sparkle support for sandboxed apps ?

Sparkle: ===== KaraFun =====
Sparkle: Extracting /Users/vince/Library/Containers/com.mycompany.myapp/Data/Library/Caches/com.mycompany.myapp/Sparkle/MyApp/myapp_1.3.1.15.dmg as a DMG
Sparkle: hdiutil failed with code: 1 data: <<hdiutil: attach failed - Aucun processus enfant
>>
Sparkle: Can't mount DMG /Users/vince/Library/Containers/com.mycompany.myapp/Data/Library/Caches/com.mycompany.myapp/Sparkle/MyApp/myapp_1.3.1.15.dmg

[sweetppro]

@vincedev in my experience only zip archives work for updating in the sandbox. I have 2 files in my servers: the dmg for customer downloads, and the zip for updates.

[vincedev]

@sweetppro Thank you for the feedback. You mean you got it to work ? I didn't with zip either.
Did you use the latest release or the yahooxpc branch ?

[kornelski]

Please don't use yahooxpc branch, it has remote code execution vulnerability. All Sparkle forks are dangerous until they copy security fixes from 1.13.1.

Please also note that all forks of Sparkle that are compatible with sandbox disable all sandbox security by allowing the sandboxed application to replace itself with a non-sandboxed one with unrestricted privileges.

[fernandomorgan]

Unfortunately I can’t maintain the Yahoo fork anymore, and I won’t be able to test any changes except in my own El Capitan Mac.

On Feb 2, 2016, at 10:01 AM, Kornel [email protected] wrote:

Please don't use yahooxpc branch, it has remote code execution vulnerability.

Please also note that all forks of Sparkle that compatible with sandbox disable all sandbox security by allowing replacement of sandboxed application to replace itself with non-sandboxed one with full privileges.

—
Reply to this email directly or view it on GitHub https://github.com/sparkle-project/Sparkle/issues/363#issuecomment-178723431.

[SevenBits]

@vincedev It has been stated many times that you should not use the yahooxpc branch. It's insecure. Take this from @pornel:

Please also note that all forks of Sparkle that are compatible with sandbox disable all sandbox security by allowing the sandboxed application to replace itself with a non-sandboxed one with unrestricted privileges.

This is the issue we need to fix. Though these forks may allow you to update sandboxed apps they do so with an absolute absence of security -- if you use it, then your users will be 100% vulnerable to a MiTM attack when updating. Though I use Sparkle in my sandboxed app (Mac Linux USB Loader), it only alerts the users to updates. Hopefully one day I'll be able to get enough spare time to fully implement sandboxing support.

[fernandomorgan]

actually the yahoo fork has some of the authentications duplicated in the XPC process that copies the file, to minimize the risks. Don’t remember now, but I think almost all the validations done by the main process (digital signature, same signing entity, etc) are repeated in the XPC process, So even if some malware replaces it before the XPC process can execute, it will fail to copy.
But I haven’t looked at it for some years now :(

On Feb 2, 2016, at 10:17 AM, SevenBits [email protected] wrote:

@vincedev https://github.com/vincedev It has been stated many times that you should not use the yahooxpc branch. It's insecure. Take this from @pornel https://github.com/pornel:

Please also note that all forks of Sparkle that are compatible with sandbox disable all sandbox security by allowing the sandboxed application to replace itself with a non-sandboxed one with unrestricted privileges.

This is the issue we need to fix. Though these forks may allow you to update sandboxed apps they do so with an absolute absence of security -- if you use it, then your users will be 100% vulnerable to a MiTM attack when updating. Though I use Sparkle in my sandboxed app (Mac Linux USB Loader), it only alerts the users to updates. Hopefully one day I'll be able to get enough spare time to fully implement sandboxing support.

—
Reply to this email directly or view it on GitHub https://github.com/sparkle-project/Sparkle/issues/363#issuecomment-178731940.

[vincedev]

@pornel @SevenBits @fernandomorgan : Thanks for all the warnings.
So I'm back to deactivate sandboxing completely for my application, as the update feature is currently broken.
Is sandbox support being discussed somewhere ? Is it planned ?
I think sandboxing is a good thing, and as more and more applications will become sandboxed, this will become an important feature to support.
As I've read here this means redesigning some core parts of Sparkle, so I wonder if there is an official discussion on the subject (apart from this issue).

[kornelski]

That's the official discussion. We'd like to support sandbox, but it's hard, as it requires major architectural changes to be secure.

[xhacker]

@pornel: Is it okay to say if we are using HTTPS for both AppCast.xml and the zip file, then we are safe?

[kornelski]

You're safe from network MITM attacks, but you're not getting the extra security benefit of a sandbox. Sandbox is for defending the system from your application when it's compromised at run time (after it's installed)

For example if your app had a buffer overflow vulnerability in parsing of files, and user opened a maliciously crafted file, the file could take over the app. If the app was sandboxed, then the damage would be limited to permissions the app has. If the app isn't sandboxed, or has Sparkle that can replace it with unsandboxed app, then the attacker can do whatever the local user can.

[xhacker]

@pornel: Thanks I got it. But how could a sandboxed app include a part (Sparkle) which could replace it with an unsandboxed app?

[SevenBits]

Thanks I got it. But how could a sandboxed app include a part (Sparkle) which could replace it with an unsandboxed app?

Hence the problem with Sparkle. Apple won't let you replace your own app's binary for this reason.

[kornelski]

But how could a sandboxed app include a part (Sparkle) which could replace it with an unsandboxed app?

That's the problem. If the app is fully sandboxed, it can't update itself.

To update itself the app needs a "backdoor" that lets it bypass the sandbox. But the sandbox backdoor has to allow _only_ trusted updates and nothing else.

This is much trickier than it seems, because Sparkle can't trust _anything_ the—potentially compromised and malicious—host app says. The part of Sparkle running in the host app's process _can't even trust itself_, because it could have been patched at runtime by the compromised host app. Any information about the app's bundle could be falsified. A compromised host app could say "Please update me, I'm Finder.app and my appcast is evil.example.com/pwn.xml" or "I'm updated! Please relaunch me at /bin/rm -rf /".

Protecting from attacks from outside is much easier than from attacks from inside.

[SevenBits]

This is much trickier than it seems, because Sparkle can't trust anything the—potentially compromised and malicious—host app says. The part of Sparkle running in the host app's process can't even trust itself, because it could have been patched at runtime by the compromised host app. Any information about the app's bundle could be falsified. A compromised host app could say "Please update me, I'm Finder.app and my appcast is evil.example.com/pwn.xml" or "I'm updated! Please relaunch me at /bin/rm -rf /".

I'm a little confused about this, however. Surely the current version of Sparkle (without sandboxing support) already addresses these concerns? Can't the code that implement these checks be refactored into some common library shared between the library and the XPC service?

An alternative option would be to move the whole of Sparkle into an XPC service (at least the part that issues updates, since an XPC service can't display UI). There would be a lot of work involved initially, but this XPC service could do all of the checks itself and support both sandboxed and non-sandboxed apps.

[kornelski]

The code in current Sparkle doesn't protect itself from the host app being compromised. Currently Sparkle trusts the host app and uses [NSBundle mainBundle], etc. Current protections in Sparkle are focused on protecting from attacks from the network, but not from the local machine or the same process.

One solution I see is to turn Sparkle update code into a standalone command-line tool that only takes path of the app to update as the input. This way the worst thing that a compromised app could do would be to initiate a legitimate update for another app.

There's also a question whether we want to sandbox parts of Sparkle itself, so that even the part of Sparkle that can access the network won't trust the part of Sparkle that can write things to disk.

[zorgiepoo]

An alternative option would be to move the whole of Sparkle into an XPC service (at least the part that issues updates, since an XPC service can't display UI). There would be a lot of work involved initially, but this XPC service could do all of the checks itself and support both sandboxed and non-sandboxed apps

I've been looking at this recently and I don't think it's as bad as it sounds. We should want the ability to provide a custom UI without forking Sparkle anyway. This is just a matter of providing hooks in all the right places.

Essentially, everything Sparkle runs in an XPC process except for the UI. The XPC process doesn't use the "main bundle"; it doesn't even have to ask the host where it's located, it can find the bundle by itself relatively to where the service is located.

I had/still have to re-work assumptions in the code about Sparkle's host. But right now I have a toy program with Sparkle in a XPC service (non-sandboxed) that auto-updates the host's app (sandboxed) without any UI.

[SevenBits]

Essentially, everything Sparkle runs in an XPC process except for the UI. The XPC process doesn't use the "main bundle"; it doesn't even have to ask the host where it's located, it can find the bundle by itself relatively to where the service is located.

Yeah, this is exactly what I had in mind @zorgiepoo. This is probably a good thing to do anyway, notwithstanding sandboxing support, because it's probably a good idea to have the downloader in a separate process for security reasons.

[zorgiepoo]

The interesting thing to me is UI separation and removing assumptions about the current host app.

Worked on this bit more and I was able to hook up the UI to the host process. It's not ready to be taken seriously yet because important stuff like appcast/date handling (or lack of) is completely broken, and I still need to look at a bunch of other stuff, but..

This protocol is the guts of my changes.

One of my toy programs simply does something like:

/* Host... */
connectionToService = [[NSXPCConnection alloc] initWithServiceName:SERVICE_NAME];
connectionToService.exportedInterface = [NSXPCInterface interfaceWithProtocol:@protocol(SUUserUpdaterDriver)];
connectionToService.exportedObject = [[SUSparkleUserUpdaterDriver alloc] initWithHost:[[SUHost alloc] initWithBundle:[NSBundle mainBundle]]];

/*XPC Helper */
newConnection.remoteObjectInterface = [NSXPCInterface interfaceWithProtocol:@protocol(SUUserUpdaterDriver)];
//...
NSURL *appBundle = [[[[[NSBundle mainBundle] bundleURL] URLByDeletingLastPathComponent] URLByDeletingLastPathComponent] URLByDeletingLastPathComponent];
updater = [SUUpdater updaterForBundle:appBundle];
updater.userUpdaterDriver = newConnection.remoteObjectProxy;
[updater checkForUpdatesInBackground];

As the end result I'd envision Sparkle to be separated into two framework components: the updater core, and a UI (appkit classes/localizations/nibs..etc). I'd also be wondering if this general approach would meet the security requirements, BTW.

[invariant]

I have been following this discussion with interest. @zorgiepoo's approach here, putting the whole update mechanism into an XPC service which independently performs the update, makes a lot of sense to me. And a formal separation into the core and UI components could be very useful. Developers could choose to use just the core, and implement their own UI. If this approach can meet the security concerns for Sandboxing too then it could be a real winner.

[zorgiepoo]

Current fork of Sparkle: https://github.com/zorgiepoo/Sparkle/tree/ui-separation-and-xpc . It has major architectural changes, and is not completely backwards compatible with code that uses Sparkle's main branch.

It tries to address (or make possible):

• UI and Core separation - all core classes are guarded against importing AppKit; a supplemental UI can be provided by those using the framework.
• Fault tolerance - the host can handle the updater crashing if they're in different processes
• Privilege separation - the un-sandboxed helper doesn't trust the host app for anything important
• Process/System independence - eg: only the host app knows when the app will terminate or system will shut off, or only the updater knows if it has sufficient write permissions to install an update
• Efficiency/Idleness - updater can be terminated safely by the system if it has no immediate work to do; scheduled checks can be delegated to the host.

Notably New:

• SUUserDriver is a new protocol for determining how to handle user update interactions.
• SUStandardUserDriver class is the standard implementation of the above protocol. Another example is SUPopUpTitlebarUserDriver, which lives in the Test App (for fun!).

Breaking Changes:

• SUUpdater is no longer a UI kind of class. You can't instantiate it in a nib anymore. See the new SUStandardUpdateController to do that, although it's not interesting when it comes to XPC.
• SUUpdater changed its designated initializer, removed the old one, and no longer maintains singleton instances. This got in the way of being able to tear down instances properly, caused me a debugging nightmare when XPC connections were invalidated, and is otherwise IMO silly. Expect 3rd party subclasses of this class to break.
• Alert modal methods on SUUpdaterDelegate are deprecated; moved to SUStandardUserDriverDelegate
• Version displayer method is removed from SUUpdaterDelegate, moved to SUStandardUserDriverDelegate

Sandboxing concerns:

• The host app may need an entitlement for sharing user defaults to the helper; take a look at the Test app's entitlements. More testing may need to be done to see if user defaults can be read/written to from both the user driver's process and the updater's process, with it still being synchronized.
• Appcasts with release note URLs will fail to load if incoming network connections is not in the entitlements. The test app uses CDATA in the appcast which works fine.
• There may be a "storageTaskManagerExistsWithIdentifier" output warning that shows up without network entitlements, when the WebView is decoded from the nib. Seems harmless to me; maybe avoidable if it is not loaded from a nib.

Remote Concerns:

• Complications arise from the fact that the updater process has no way of knowing when the target process terminates, which may make the updater terminate. If the updater wants to do automatic updates on app termination, it must not only know when the app is about to terminate but the app's termination needs to be delayed until the updater says it's ready. Some of this is delegated to the user driver and its delegate. There is a bit of delay lag at the moment; maybe this can be tackled better..
• The standard user driver assumes it's in the app that is being updated, but it shouldn't be that hard to implement a user driver that doesn't assume this, though it's not something I'm looking at, at this moment.

Autoupdate Concerns:

• The autoupdate's UI app is still not decoupled or very customizable, but I think it should be. A todo..
• Due to XPC and removing some UI code in the updater, I had to do some trickery to get the autoupdate app to launch by using launch services instead of spawning a task (otherwise the task may have been prematurely terminated).
• The autoupdate app tries to find the target still-alive process by using NSRunningApplication - maybe that could cause issues for non-apps.
• It now sets a timeout if the target doesn't terminate and gives up.. Maybe a good idea or not.

Misc:

• The logic for not showing the permission prompt until the "2nd launch" is not sound anymore, probably. Might need to change not prompt until after first 24 hours or something. Because the XPC can terminate at any time and Sparkle should have resumability.
• Code signing support was refactored to not assume the current host.
• DMG archive code changed a bit, removing some legacy cruft. Unit tests pass, but haven't really tested it otherwise yet.
• While the classes are separated between core and UI, there's no build target or anything yet to build 'only' the core classes.
• The XPC and main app can share the Sparkle framework as is done in the Test App. See LD_RUNPATH_SEARCH_PATHS in ConfigTestAppHelper.xcconfig and ConfigTestApp.xcconfig

Play with SURemoteUpdateSettingsWindowController.m and TestAppHelper project in the test app, which shows how to set up a XPC connection and set up the helper. It's currently using my badly designed UI driver, but you can un/re-comment the self.userDriver = line out to use the standard implementation. I acknowledge that dealing with idleness in the XPC helper and dealing with interruptions and invalidations are tricky problems.

10.7 was dropped in my fork for architectural reasons (and in the test app for XPC reasons).

[SevenBits]

@zorgiepoo This is fantastic -- I don't think any of us were expecting anything tentative this soon. I'll definitely be taking a look at this. Now, the question is what @pornel thinks of this fork...

[ttscoff]

Agreed, this is great news and amazing progress.

On Mar 5, 2016, 7:29 PM -0600, [email protected], wrote:

@zorgiepoo(https://github.com/zorgiepoo)This is fantastic -- I don't think any of us were expecting anything tentative this soon. I'll definitely be taking a look at this. Now, the question is what@pornel(https://github.com/pornel)thinks of this fork...

—
Reply to this email directly orview it on GitHub(https://github.com/sparkle-project/Sparkle/issues/363#issuecomment-192777357).

[kornelski]

Thank you @zorgiepoo . This is great and exactly where I hoped Sparkle would be heading.

Some of the commits are cleanups that are good to have now and we can merge them already.

For breaking changes we'll have to decide on a strategy:

• Do we just call it Sparkle 2.0 with compat break? (the alternative would be to gradually evolve 1.x towards the new architecture, avoiding breakage where possible)
• Would we keep maintaining 1.x? There's a lot of apps out there, and some want to support old systems. I think it's fine to require apps supporting sandboxing to use new API, but OTOH having incompatible sandbox and non-sandbox versions of Sparkle is a complication.
• Can we add adapters to make simple apps using sparkle 1.x work with the new architecture? (e.g. still have nib-compatible SUUpdater that sets up the real updater and controller?)

[SevenBits]

@pornel : I've taken a cursory look at the code, and based on the changes made I'd recommended absolutely bumping the version number to 2.0. This is good because:

• It accurately reflects the large magnitude of changes made to Sparkle; and
• It alerts users to the fact that the new release would have incompatible breaking changes

In terms of still maintaining 1.x, perhaps you could release a version 1.15 with certain commits from 2.0 cherry-picked? They could be released simultaneously. I wouldn't do this forever, but it would provide a good stopgap measure until apps can be updated to version 2.0.

[kornelski]

I've meant 2.x as in semver (meaning it's incompatble), not senver (meaning a big change) ;)

I wouldn't do this forever, but it would provide a good stopgap measure until apps can be updated to version 2.0.

How can we get all developers on board? When will everyone be ready drop OS X 10.7? We've seen developers happy to stick to v1.5 forever, and I wouldn't like to get into situation like Python 3k.

[zorgiepoo]

Thanks, glad my progress is liked :).

Initially, I tried to avoid breaking compatibility but at some certain commit, I decided to let go.. We could support 1.x still for at least critical fixes, sure. Apple is a big driver in getting users and developers away from older OS's. I don't think Sparkle has a big impact there; not for 10.7, anyway.

I'll mention that the framework from my fork is independent from being sandboxed or not, and no XPC code is in the framework either, yet (might be useful to add helper code to make XPC easier to set up, though IDK). Currently there may be little code that doesn't support 10.7, but supporting it harms us not being able to use weak references, or import appkit in places that we shouldn't have to, or having many versions of the same nibs, or having to document a painful way to sandbox an app using 10.7's XPC APIs.

Can we add adapters to make simple apps using sparkle 1.x work with the new architecture? (e.g. still have nib-compatible SUUpdater that sets up the real updater and controller?)

SUStandardUpdaterController is my nib-adapter replacement for that. Renaming this to SUUpdater may cause a lot of other issues. I just added delegate outlets to it recently.

Security: the chain of trust in my fork is user driver -> updater -> autoupdate. Whilst the updater doesn't trust the user driver (which can be sandboxed), the autoupdate tool completely trusts the updater. Therefore, I may ponder over moving validation code into the autoupdate tool.

[sweetppro]

@zorgiepoo does your fork allow updates from .dmg? Im using one of the other sandboxed forks, and it only allows updating via .zip files. something to do with not being able to access the disk image mounter while sandboxed.

[zorgiepoo]

Yes it will allow dmg's. But as an aside, it's my personal belief that using dmg's for updates is kind of sucky - the unarchiving code is more complex, they are less slower to decompress, have to do a mount/copy/eject dance, etc..

I should note my fork is not really ready yet to be used. It's also not a simple "just turn sandboxing on"; it requires you to write a XPC service, currently. Check it out at your own risk.

I currently am looking into moving extraction/validation code into the Autoupdate tool, and move UI out from there, which could require another IPC mechanism. XPC services can't be used for privileged tasks (thinking non-admin users here), which is one reason why I'm launching the Autoupdate tool through launch services rather than by spawning a task. Apple would rather prefer us using something like SMJobBless/SMJobSubmit, but that has its own complications.

[zorgiepoo]

So I've moved extraction & validation code into the final installer (Sparkle's AutoUpdate) which as far as trust issues tackles the big concern of this thread. To clarify, these concerns are security issues that exist now even when Sparkle is not sandboxed. I have not looked much into say, defending against "symlink attacks" (as Andy mentioned previously) because I'm not really sure what that entails doing -- and there could be other concerns I'm not quite aware of.

You don't have to go through the trouble of sticking the entirety of Sparkle into its own service anymore. I just created a few XPC services inside the framework (one for downloading the appcast, downloading the update, testing for writability install permissions, launching our installer tool). I still need to create one for communicating to the installer, but for now just one entitlement is needed for that (see the test app).

I tried making other major changes on the way.. For example, AutoUpdate always attempts to install the app after validating the update is done, even if the user decides to quit (no more waiting for termination in the main app or in Sparkle). The regular and automatic update prompts have been merged together (no duplicated nibs where one has features the other doesn't). The ui-core separation has been simplified. The code has been drastically refactored as needed. And I created an issues list so I don't forget about things that need to be looked over or still (re-)added (some discussion has been taken place there already).

Somehow my fork came out in a way to tackle ui separation, process separation, and privilege separation. Ends up not entirely being related with one or the other, but it works out nicely in helping the code stay clean.

[zorgiepoo]

Recent progress with my fork:

1. No entitlements or custom XPC services are needed for Sandboxing. Just use the framework normally (recommended way).
2. Added customizable installer progress app that can show up after the host application has terminated. In practice, however, this may show up very rarely due to update installations usually occurring very quickly.
3. Automatically downloading updates is always allowed now unless the developer disables it, but an update alert shows up more often/instantly for critical updates, root-required write-access installs, and package type installations.
4. Added experimental sparkle-cli tool for updating an arbitrary Sparkle supported bundle. It makes use of process separation and has its own UI (command line interface), but does not make use of sandboxing. It works okay, but still has some issues.

fork | issues

[SevenBits]

@zorgiepoo Sounds awesome. Nice job. Any ETA?

[vphamdev]

@zorgiepoo this is fantastic work. I'm trying your branch right now and would love to start using it in a sandboxed app that is almost finished. let me know if I can help by testing anything!

[tt-tsquared]

@zorgiepoo looking great. I tried to use your branch on one of my sandboxed apps and it failed with this:

2016-04-18 13:50:20.711 XXXXX.app[70497:18659293] Sparkle: Failed to release quarantine on /Users/tthomas/Library/Containers/com.XXXXX/Data/Library/Caches/com.XXXXX/Sparkle/Autoupdate.app with error Error Domain=NSPOSIXErrorDomain Code=1 "Operation not permitted" UserInfo={NSLocalizedDescription=Failed to remove file quarantine on Autoupdate.app.}
2016-04-18 13:50:20.712 XXXXX[70497:18659293] launch path not accessible
2016-04-18 13:50:20.722 XXXXX[70497:18659293] (
0 CoreFoundation 0x00007fff8fc7d4f2 __exceptionPreprocess + 178
1 libobjc.A.dylib 0x00007fff9dd7073c objc_exception_throw + 48
2 CoreFoundation 0x00007fff8fce44bd +[NSException raise:format:] + 205
3 Foundation 0x00007fff97c85ffb -[NSConcreteTask launchWithDictionary:] + 476
4 Foundation 0x00007fff97da9be7 +[NSTask launchedTaskWithLaunchPath:arguments:] + 194
5 Sparkle 0x00000001002a782a -[SUBasicUpdateDriver installWithToolAndRelaunch:displayingUserInterface:] + 6970
6 Sparkle 0x00000001002a5a5c -[SUBasicUpdateDriver installWithToolAndRelaunch:] + 60
7 Sparkle 0x00000001002c8989 -[SUUIBasedUpdateDriver installWithToolAndRelaunch:] + 537
8 Sparkle 0x00000001002c874f -[SUUIBasedUpdateDriver installAndRestart:] + 63

[SevenBits]

@tt-tsquared I'm not sure exactly how @zorgiepoo has things now, but typically you cannot remove the quarantine bit from within the sandboxed app. The XPC service must do that.

[zorgiepoo]

Yep, you also can't spawn a task for AutoUpdate because it uses authorization based APIs. @tt-tsquared Your stack trace is indicating that what you're using is not my XPC branch.

I have been working with configuration type issues lately. Three recent major changes:

1. XPC Services aren't bundled inside Sparkle.framework anymore because they need to be signed specially with specified entitlements by the developer; Xcode's tooling doesn't properly handle bundling them in a framework and encourages to sign them beforehand. They're now also individually optional, but of course some or all are necessary if you sandbox your app. Growl took this same approach to sandboxing too.
2. Updates have to be signed with DSA signatures now, otherwise it's not secure because it doesn't fit with the security threat presented in this thread. The installer (Autoupdate) can't know if a download was served over http or https, nor should we weaken our privilege separation by sticking downloading into the installer.
3. I separated SUUpdater initialization and starting the updater into different methods, allowing graceful error handling when encountering invalid bundles, and more flexibility as to when to start the updater. Initialization also can't fail anymore. The behavior of SUStandardUpdaterController (controller for sticking the updater in a nib) has been unchanged.

Next I'll probably work more on robustness of the Sparkle & Autoupdate interaction, and fix some edge-case bugs..

Feel free to test the branch out, see if it works, report any issues, although I will warn you that you probably don't want to try using it seriously yet, and will be subject to changes. My branch can be built by doing make release in the command line which will spit out a distribution that's tgz'd. Decompress that and follow through the INSTALL.

Any ETA or what not I make no guarantee of; I just want to go through all the issues/concerns I have. Often this requires re-considering some decisions again & again.

[waltsc]

just want to chime in and say @zorgiepoo thank you for all your work here. i tried the branch and i think i have it working with a sandboxed app and all the XPC services.

[tt-tsquared]

@zorgiepoo yeah sorry about that bug report. I don't use git all that often and thought that copying the url for your branch and cloning it would actually clone your branch and not just point me at master. Along with others I want to thank you for doing a rewrite necessary for sandboxed apps.

[sweetppro]

@zorgiepoo I tried your branch but could figure out how to set the feed url?
This works in the official build:
[[SUUpdater sharedUpdater] setFeedURL:[NSURL URLWithString:appcastLink]];

what do I need to do to get to change the FeedURL in your build?

[zorgiepoo]

@sweetppro I removed sharedUpdater / singleton access. If you're instantiating a SUStandardUpdaterController object in a nib, you'll have to grab the updater from that instance. Alternatively you can implement the delegate method for providing the feed URL (I like this approach better because it doesn't touch the user defaults). See SUStandardUpdaterController.h, SUUpdater.h, SUUpdaterDelegate.h

[StevoGTA]

@zorgiepoo Thanks so much for your hard work on this branch. Updating sandboxed applications was a requirement for my client and I just completed the Sparkle integration for them - including Xcode automatically generating the needed Sparkle resources. This was in addition to building an entire website to handle initial downloads, appcast and release notes serving, and device info tracking. Nicely done!

[ksuther]

Dropping a note here because of Sierra changes that might be troublesome for Sparkle and sandboxing.

https://developer.apple.com/library/prerelease/content/technotes/tn2206/_index.html#//apple_ref/doc/uid/DTS40007919-CH1-TNTAG17 (see Shipping your Signed Code)

Apple now suggests using signed disk images or installer packages, and discourages zip files. Also, Sierra added an extra layer of isolation when launching from some paths which prevents the app from knowing its launch path.

[jtbandes]

Nothing mentioned about .xip archives?

[SevenBits]

@jtbandes Hmm, it's ambiguous. An xip archive is signed, but it's not a disk image. I'd think it would work fine without the special checks, but I'm not sure; I'd file a bug with Apple on this one. I plan to do so when I get home.

[kornelski]

We have discussion of xip in #821 To me it looks like a hacky format. I think a regular zip or tarball with a DSA signature will be faster and more reliable.

Sparkle removes quarantine from archives, so the new limitations imposed by Apple don't apply to updates.

[SevenBits]

@pornel Certainly for updates it wouldn't matter as much; but I was thinking more along the lines of initially distributed apps...

[zorgiepoo]

Regarding my fork/branch, there is an issue in the current Seira beta, where the sandbox disallows the host application from creating a listener for IPC to the installer - so I had to create another non-sandboxed XPC service to resolve that. I also filed a regression rdar for Apple because it wasn't an issue with previous OS's.

We updated Sparkle's documentation to address how developers should ship their application on their website due to the 10.12 changes, and it follows Apple's advice. XIP files have no effect on translocation, and thus shouldn't be used for initial distribution.

[zorgiepoo]

Ok, here's a big update of my fork in a long while... It has gone through a few rewrites recently, aimed at having a secure/modern/robust model.

• XPC APIs instead of CFMessagePort are now used for interacting between the updater (Sparkle) and installer (Autoupdate) now, which is more reliable and modern.
• Autoupdate is now a command line tool and launched as a launchd agent or daemon. Another agent is launched (Updater.app) to handle showing UI progress, serving an installation status service for the updater, and relaunching updates. AppKit is now minimized to the user drivers and to the agent application.
• Authorization is now handled before launching the installer agent/daemon which is before extracting the download. No traces of the deprecated AuthorizationExecuteWithPrivileges() are left.
• If authorization is needed after an update is downloaded automatically and silently, the update is aborted but Sparkle keeps a reference to the archive that was downloaded and the appcast item that was used, so that the user can be prompted later to install and resume the update. This is a little different than if authorization isn't needed in which case the installer is started in the background and waits for the user to resume it or for the application to terminate.
• Sparkle's App Icon is not used in the agent application for its dock icon nor for the authorization prompt from the updater. Instead, the icon used is from the bundle that is being updated which should have a 32x32 image representation.
• The job launcher XPC service includes a copy of the agent/daemon tools. If a developer uses this service in a sandboxed app or not, they can have the tools code signed in a proper signing directory. The launcher portion of the code that is used by this service or by Sparkle computes the paths to the tools on its own.
• External release notes are now supported without an outgoing connection entitlement, but they can't link to external resources like images or external style sheets without an exception entitlement.
• Logging is changed to use ASL because writing to the same location on disk in all processes and services is a big problem, especially with XPC services & launchd jobs.
• The appcast RSS length attribute is now expected to exist _and_ be valid. A log warning may pop up otherwise.
• "Interactive" package installations are now deprecated because they have several caveats. Guided package installations are now the default for packages, and you'd have to opt out of them to get an interactive package installation (eg: foo.sparkle_interactive.pkg).
• For package type installations, a sparkle:installationType=package or sparkle:installationType=interactive-package is required in the appcast enclosure item. This is necessary for Sparkle determining if authorization is needed before launching the installer. The installer after extraction makes sure this matches with the installation type found inside the archive.
• DSA Validation is now done before attempting extraction for delta updates & package updates.
• mdimport which isn't root-safe isn't invoked anymore. Its usage made little sense considering updates have to work outside of Sparkle too. Only after delta update extractions, the modification time of each mdimporter bundle found is updated.
• The old app after installation is not moved to the trash anymore. A few reasons span from user experience, consistency when running authorized, and file atomicity.
• macOS 10.8 and 10.9 compatibility has been tested more thoroughly. Authorization didn't used to work for those versions because a required info.plist key was missing for the launcher XPC service (took me forever to debug..)
• More work on sparkle-cli has been done, and work has been done to ensure that Sparkle's standard user interface/driver can be used to update other bundles remotely. There are some bundle caching issues though..

I added an installation document to the fork that gives an overview of how much of the installation process works, which also helps me ensure it's implemented right..

[SevenBits]

@zorgiepoo Awesome work! I just have a question about this section:

Instead, the icon used is from the bundle that is being updated which should have a 32x32 image representation.

Does this mean that we need to include another copy of our app’s icon? Or does your Sparkle fork find it and use it automatically?

[zorgiepoo]

@SevenBits The latter; Sparkle finds the icon just like it does in other places (eg: in the new version is available window). For the authorization dialog in particular, it looks for a 32x32 representation in the (probably) icns.

[zorgiepoo]

I changed the prefix to newfangled classes in my fork to SPU and added a deprecated SUUpdater bridge class - so my fork should be runtime/backwards compatible with many existing applications (such that you can drop the framework into some other working app). A couple of exceptions exist though. Also because many applications serving over https do not use DSA yet, I made it deprecated rather than outright removing it for now.

[zorgiepoo]

I'm nearing completion with my fork or up to the point I will hopefully stop working on it soon. I'm quite happy now with the architecture, but have not deployed it for real or have it reviewed (pornel has alerted me that he is quite busy at the moment).

I have changed the landing page to reflect an overview of the fork, so here it is. I may later try to back port small improvements from it to master when I get the chance.

[ksuther]

I've tried out your fork on a test project and it seems to work as advertised. I've gone through the steps of replacing my insecure XPC fork with it and successfully updated with it. I'm going to try the new SPU API and start deploying our betas with it and see if we run into any issues.

I haven't actually looked at the code much, but the overall architectural changes seem like a big improvement.

So based on the limited testing I've done so far, awesome work! Is there anything people can do to help with this?

[zorgiepoo]

Thanks, your fork was a helpful reference too!

Testing and code & documentation review (if familiar enough with Sparkle or Security and have the time - too much of the code has changed) is what I can think of that'd help.

These are some of the things I've been testing that I can think of:

• Test when the updater starts, it only prompts the user for permission to update if 3 hours have gone by since the first time the updater started.
• Test as standard user updating in /Applications - testing as admin user goes without saying too. Test canceling the authorization dialog as well as accepting it.
• Test updates that are downloaded automatically (user default SUAutomaticallyUpdate) if your application allows it. Test checking for updates after the update is downloaded and begun partial installation. Test that functionality as a standard user that needs to authenticate too, in which case an update alert will prompt immediately (same regarding critical marked updates).
• Test updates being installed when the application terminates, even if the user quits the installation process after the update is extracted. (The update won't be automatically extracted if the user has to authenticate first). Test terminating the Updater.app and Autoupdate processes in middle of installation or whenever to see if the update fails gracefully, during silent or non-silent installations.
• Test updating an application if it's in /tmp/ (which tests two things. one - being in a path that references a symbolic link, and two - requiring authentication despite writability because of matching owner/group)
• Test updating an application over a SMB or AFP network mount. This will probably be slow enough for the installer progress UI to show up after the application has been terminated as well (other probable case where it'd be slow enough is a guided package installation).
• Test checking for update information (probed updates).
• Test scheduled vs user initiated updates (debug version of Sparkle has shorter check intervals)
• Test whatever delegate methods you were using still work, or have replacements/alternatives that work.
• Test delta updates if you use those, and testing invalid signed delta update, and if original application was altered (in which case the full download is attempted).
• Test app when on a readonly volume. Behavior should be the same as official branch
• Test behavior on macOS 10.8 and 10.9 if you support those systems
• Test sandboxing... Not directly related to Sparkle, but keep an eye out open for user defaults migrating from sandboxed app -> non-sandboxed. macOS can have issues if you switch between such builds frequently.

I kind of want to look into merging the downloader services next if possible, but will do that in a separate branch first whenever I get to it.

[michelf]

I've done various tests with the ui-separation-and-xpc branch and could not find any problem with it. I'm currently leaning towards using this for the next version of a sandboxed app that currently isn't using Sparkle.

[zorgiepoo]

I've made some changes since I last reported.

The more important/relevant changes are:

• Move app termination from user driver to installer (Aug 25)
• Merge two downloader XPC services into single XPC service; don't want more services running than necessary. (Aug 20)

Other minor changes are:

• Add allow installer interaction methods to updater delegate (Sept 4)
• Let user driver know when updates found were user initiated (Aug 29)
• Add SparkleCore framework that contains only core of Sparkle and no AppKit linkage (Aug 21)

The minor changes I made I needed for an app I'm writing, although that app is not being sandboxed.

I've also been submitting some minor changes from my fork to this branch, performed some merges, and made some documentation changes.

[tt-tsquared]

@zorgiepoo again great job on this branch. I ripped out the old Sparkle branch I had used for Pastebot betas and replaced it with yours. Everything seems to be working well but will report any issues.
What is going to happen moving forward? Is this branch going to be the basis for the next major version of Sparkle? I get a zillion warnings about localized string files and nib/xib files missing. I might be able to get everything localized at some point if needed.

[sweetppro]

I agree,
Ive been running the @zorgiepoo branch for a few months now in all my Sandboxed apps without any major issues. I think im a couple of commits behind the latest, but everything works well. Is there any reason why its not full steam ahead on that branch?

[ksuther]

I've also been using this branch in our betas for a few months and haven't run into any problems yet (just testing on 10.11 and 10.12).

One thing I'd like to test out more is delta updates, but we're not currently using delta updates in betas.

[kornelski]

Great to know. We are merging changes from the fork back to master.

[zorgiepoo]

Is this branch going to be the basis for the next major version of Sparkle?

Perhaps! I'm going to soon prod pornel to pull in my fork as a branch so he can keep it maintained and in sync with the other branches, and not forget about it :-).

I get a zillion warnings about localized string files and nib/xib files missing. I might be able to get everything localized at some point if needed

I think the update alert in my branch handles a corner case that the current version of Sparkle doesn't handle for critical updates. Perhaps there's other localization strings I'm missing, is there a way you are generating these warnings?

[zorgiepoo]

Hey all, this is a short notice that I will (probably) not be able to continue working on Sparkle in the very near future (~10 days; at least, this would be the "safe" assumption). So, if you've any short term questions, I can try to address them but otherwise good luck! pornel pulled my branch in the repo although it's not merged into master yet. It will soon be up to you all to see where the project goes :).

[ksuther]

@zorgiepoo Thank you for all your contributions, Mac developers (and users) everywhere will be benefiting from them for a long time!

[digitalmoksha]

@zorgiepoo @pornel great job on this new architecture and all the work that went into it. Highly appreciated. It's a real service to Mac developers.

[milke]

I haven't been using (nor following) Sparkle for almost eight years (!) now and sure a lot of things have changed. I'm still browsing the code and framework structure in this branch and I'm about to start testing it in a production (sandboxed) application. Therefore I'd first like to thank to all contributors maintaining this project and keeping it up-to-date and especially to @zorgiepoo for bringing Sparkle into the era of (proper) sandboxing!!

There's one thing I don't understand though; the "INSTALL" document says at one point:

If you code sign your application, these XPC services will also need to be signed by you with the attached entitlements. Included is a script to automate this.

However, I don't understand what "the attached entitlements" really means. The only entitlements files I could found in the project are "org.sparkle-project.Downloader.entitlements" and "Sparkle-Test-App.entitlements". Are other XPC services sandboxed?

It may be I still haven't figured out the complete structure of the "sandboxed" framework, but just thought I should ask this first.

[digitalmoksha]

I'll try to chime in since I've been wrangling with this the last couple of days.

Related thread

Only the Downloader needs entitlements - the other xpc's do not. The codesign_xpc tool takes this into account - if there is an appropriately named entitlement file, it gets applied. Per the INSTALL instructions, since my app itself has the correct entitlements, I'm not including the download xpc.

What I've learned through much trial and error (maybe others would consider them obvious):

1. The Sparkle.framework needs to be sandboxed
2. The xpc services should exist in an XPCServices folder in MyApp.app/Contents. I tried placing them in the Sparkle.framework itself and signing them correctly. However verifying the signature of the app produces an error: unsealed contents present in the root directory of an embedded framework. In subcomponent: ....Contents/Frameworks/Sparkle.framework
3. Make sure you run a codesign -vvvv --deep MyApp.app to verify the entire app is signed correctly. I would love to see the generate_appcast do this as an extra sanity check.

[zorgiepoo]

@digitalmoksha is pretty correct here.

The downloader service is the only one that can have entitlements. The other XPC services need to be non-sandboxed.

The downloader service doesn't have to be sandboxed, but you most likely want it to be. It just has an exception in the entitlements to make network requests.

The test app has a couple of exceptions that you probably shouldn't worry about, so I wouldn't look at it closely.

If you use Xcode, you can add a build phase to add XPC services, after you've signed them.

Lastly, using the XPC services and sandboxing your application are somewhat orthogonal. Eg: you can make use of the XPC services even if your application is not sandboxed. If your application is sandboxed though, you will have to use the XPC services. If your sandboxed app already has a network exception to download data, you don't have to include the downloader service. XPC services provides privilege separation which is a prerequisite for sandboxing applications, but also fault tolerance, idleness..

[milke]

Thanks guys. Some things I've already figured out:

• I haven't really looked into codesign_xpc, I like to manage my own run scripts with custom file locations, but once I looked into it, things became clearer.
• XPC services must reside in MyApp.app/Contents/XPCServices and not embedded in a framework (like system frameworks do), I also learnt that one the hard way :-(
• I know the downloader service is optional and has to be used only if the main application is sandboxed and doesn't allow network access (which is my case). It can be used though in other cases too (improve responsibility separation, security and all that…).

My initial question popped-up without examining what XPC services (other than the downloader) do and I immediately wondered why they aren't sandboxed if this branch claims to provide Sparkle for properly sandboxed applications (properly in the sense not only having com.apple.security.app-sandbox = YES, but also taking care not to allow compromised host app to tell XPC helper to execute arbitrary code and completely escape the sandbox and all other things already discussed at the beginning of this thread). I guess I was just too quick to ask the question without studying the whole structure of the framework, with all of its helper components. Bear in mind the last time I've dealt with Sparkle, everything was done right from the framework code and the only external helper (if I remember correctly) was relaunch (or something like that), which relaunched the application after it has been updated :-)

[zorgiepoo]

I figure it'd be best to clear this up about security because it's causing confusion.

Ensuring proper privilege separation, and having a component (or process) running at a higher level of privileges that doesn't implicitly trust the input from another component running a lower or equivalent level of privileges, actually has very little to do with sandboxing. Supporting sandboxing securely is a small part of the overall picture of privilege separation.

Even if you don't sandbox your app, my branch is still more secure than master right now. And aside from XPC services (which again is orthogonal to sandboxing), to do proper privilege separation, you can't arbitrarily shell out for authorization like master currently does, which is why creating launchd agents and daemons was necessary. Because a daemon runs as root-level, what the installer is allowed to do is also carefully restricted (a privilege separation concept that is unrelated to sandboxing or even XPC services).

So you need a solid privilege separation model even if XPC services & entitlements aren't used. When you add sandboxing on top, that is adding an additional layer of defense. It appears that previous sandbox forks were rejected because the current security model of master (not the forks themselves) is lacking.

[milke]

After another nine months, I just want to chime in again to report my experiences on using @zorgiepoo fork (I haven't really checked if it was merged into master or ui-separation-and-xpc branches). I've been using it for updates of this application since mid-March. Firstly it was a limited number of closed-beta testers and since mid-August to roughly 2000 downloaded copies, as the application is now available as public beta. I haven't had problems nor anybody else has reported any. My configuration is pretty simple, so there may be still some hidden bugs, but I haven't hit any of them.

Once again, I'd like to thank to all contributors maintaining the project and especially @zorgiepoo for his hard work on his fork.

[IngmarStein]

I've also used it successfully for months in http://github.com/IngmarStein/Monolingual and have been looking forward to this being merged into master for quite some time now. Thanks @zorgiepoo!

[swisspol]

I went through the entire thread and linked description but I don’t get it. Can someone briefly explain how it is even possible for a sandbox app to update itself considering it doesn’t have read / write access to /Applications? How does going through launchd and XPC makes this not a problem? Or will the installer always prompt for root privileges when updating a sandboxes app? And how is all this not some sort of sandbox bypass Apple will eventually close?

[kornelski]

Can someone briefly explain how it is even possible for a sandbox app to update itself considering it doesn’t have read / write access to /Applications?

The app is split in two: main part that doesn't have access, and XPC that does.

This is secure if the XPC service doesn't trust the main part of the app and refuses to write anything that isn't externally signed to be a legitimate update.

And how is all this not some sort of sandbox bypass Apple will eventually close?

This may happen if Apple decides to kill all Mac software except the software in the Mac AppStore. However, so far Apple allows non-Store apps to voluntarily define their own sandbox permissions.

[swisspol]

Thanks. So basically the XPC service is not sandboxed or sandboxed with entitlements to read / write anywhere? Where is this defined in the code, at a glance I couldn’t find an entitlement file for the XPC service.

[balthisar]

I'm finally getting around to implementing this branch, but I'm confused about the deprecation for SUUpdater; right now I'm creating a singleton upon first reference, and not loading from a nib (because no Sparkle references in the App Store version). Later, I'm programmatically binding the UI via referencing the singleton, as needed (i.e., I'm not creating any bindings until the Preferences UI is loaded, which might never happen).

I could use the SPUUpdater, and write my own singleton, but it would look pretty much like SUUpdater does right now.

Right now these deprecation warnings are the only warnings in my code, and anal-retentive me is bothered by them. Is there a best practice for implementing the new API? Thanks.

[ksuther]

What I've found works well enough is to use SPUStandardUpdaterController in the app delegate (_updaterController = [[SPUStandardUpdaterController alloc] initWithUpdaterDelegate:self userDriverDelegate:nil]).

Then when the preferences are shown do something like [preferencesController setUpdater:[[self updaterController] updater]] and bind on self.updater in the nib.

Then it can be #ifdefed for Developer ID builds fairly easily.

[balthisar]

Thanks, @ksuther, that tip helped. I ended up keeping it simple by creating it my AppController (to hook up the menu, and give it a chance to run), and binding into an NSUserDefaultsController directly in the Preferences pane rather than binding to anything Sparkle-related.

[spearway]

I am trying to use this branch to update an application that is distributed on a dmg without any success, has this been tested?

[digitalmoksha]

I distribute my app on a dmg, and updating it with this branch works no problem. The app does have to be copied off the dmg first...

[sweetppro]

I have 9 apps which update via .dmg using this branch.
:)

[milke]

Same as with @digitalmoksha and @sweetppro.

[aonez]

@digitalmoksha, @sweetppro and @milke, have you done something special to change from master to this branch? I'm just getting this error when installing:

kernel: Sandbox: Process(XXX) deny(1) job-creation
com.apple.xpc.launchd[1]: (com.apple.xpc.launchd.user.domain.XXX.Aqua) Caller not allowed to perform action: Process.XXX, action = service submission, code = 1: Operation not permitted, uid = 501, euid = 501, gid = 20, egid = 20, asid = 100007
Process [Sparkle][XXX]: Submit error: Error Domain=CFErrorDomainLaunchd Code=4 "(null)"
Process [Sparkle][XXX]: Failed to submit installer job
Process [Sparkle][XXX]: Error: Failed to gain authorization required to update target

[milke]

@aonez, I haven't used master branch at all, I jumped straight to this one and I haven't experienced any problems. I don't know whether this branch made it into the official Sparkle 2.0 master branch yet.

[sweetppro]

I integrated this in ALL my apps ~2yrs ago? I remember there were no issues and it was pretty straight forward.

Make sure you add the XPCServices to your app.

Russell

Sent from my iPhone

On 28 May 2018, at 9:08 am, aONe [e-mail] wrote:

@milke as far as I can tell the fork merged into the branch, and it's still not in the master. You use it in a sandboxed environment, right?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

[aonez]

Make sure you add the XPCServices to your app.

That was it, thanks a lot @sweetppro!

[sweetppro]

Ah nice!

Glad it was a simple fix

Russell

Sent from my iPhone

On 28 May 2018, at 9:38 am, aONe [e-mail] wrote:

Make sure you add the XPCServices to your app.

That was it, thant a lot @sweetppro!

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

[tomeumasco]

Hi, I have a similar problem when trying to install the update.

[Sparkle] Error: Failed to gain authorization required to update target [Sparkle] Error: An error occurred while launching the installer. Please try again later.

but I do not know how to add XPCServices to my app. Can somebody help me?

Thanks for your time.

[digitalmoksha]

My build process is quite different than most (RubyMotion), but at the end of the day, you need the .xpc services in the XPCServices folder:

MyApp.app
   Contents
      ...
      Frameworks
         ...
         Sparkle.framework
      XPCServices
         org.sparkle-project.InstallerConnection.xpc
         org.sparkle-project.InstallerLauncher.xpc
         org.sparkle-project.InstallerStatus.xpc

[tomeumasco]

@digitalmoksha Oh, great. Now I got it.
Thanks a lot!

[kolpanic]

I've been working to get our apps updated to the latest sandboxed and hardened version.

Since SUUpdater is marked as deprecated, I'm trying to use SPUStandardUpdaterController instead. Since our apps are distributed both on MAS and directly, I set up Sparkle programmatically (instead of instantiating an object in a XIB) like this, appropriately wrapped in #ifs:

    lazy var sparkleUpdater: SPUStandardUpdaterController = {
         return SPUStandardUpdaterController(updaterDelegate: nil, userDriverDelegate: nil)
     }()

     @IBAction func checkForUpdates(_ sender: AnyObject?) {
         sparkleUpdater.checkForUpdates(sender as Any)
     }

While this works, I wonder about SPUStandardUpdaterController's updaterDelegate. I'm passing nil, but, if I wanted to pass an object, which delegate methods should be implemented (even though, AFAICT, they're all optional)?

[tonyarnold]

Are the edDSA changes from 1.21.x in this branch? The included generate_keys spits out old style DSA keys.

[kornelski]

The EdDSA changes haven't been ported to the new branch, sorry. I don't have time to do it. Any help is very welcome.

[IngmarStein]

Are there any current plans to merge this into master so that you don't have to maintain the branch indefinitely?

[kornelski]

The plan is to make the 2.x branch the recommended version.

However, regardless of how the repo is structured, I expect the 1.x version to be maintained for a while, until all apps using it upgrade to the new one. It took a long time for everyone to move from the "classic" v1.5b, and I'm worried that extra XPC requirements will make switch to 2.x take even longer.

[tonyarnold]

How can we help? I'm _just_ about to integrate the XPC branch into my main target (it's already integrated into a helper app we have), and I would like to see this merged with master and released.

Honestly, at a certain point I think you need to stop worrying about what the unwashed masses will or will not update to, and do what's right for you and the project.

[tkrajacic]

I second Tony's argument here.

I think many will benefit hugely from finally merging the XPC stuff and will help the project thrive. I think the current 2-branch situation is a huge pain and it has been going on for way too long.

[tonyarnold]

How far have the branches diverged as of today? I can see the last time changes were merged in was version 1.20.0 in November last year. I'll have a swing at rebasing the changes in - but I don't know much about the differences between the branches at this point (I guess I'm about learn!).

[kornelski]

1.x and 2.x fundamentally are not too different, but 2.x changed project layout and made some small refactorings that made rebasing impossible (some of the differences have been resolved in merge conflicts, and rebasing will throw these away).

I'd like to backport as many changes from 2.x to 1.x as possible, to make them more similar, and therefore easier to merge.

[tonyarnold]

I'd like to backport as many changes from 2.x to 1.x as possible, to make them more similar, and therefore easier to merge.

Ok, so a series of PRs that moves the changes from 2.x back master in a way that means you can still ship 1.x builds in the meantime? That sounds doable, and I'm happy to help out with that.

Are there major areas or related changes that you'd suggest might be good to tackle initially?

[kornelski]

Yes, unification will be useful. Unifying EdDSA is a priority, so it'd be best to start with areas it touches.

[tonyarnold]

Oh, I was suggesting that we merge the changes piece-by-piece from this XPC separation branch back onto master. The goal should be for this to supersede Sparkle 1.x, right?

[kornelski]

If having XPC code on a branch called master is the goal, then no merging needs to be done, it's as easy as renaming the existing branches.

The issue is bigger than this, as it's about supporting the ecosystem.

Even if Sparkle 2.x becomes the Sparkle today, there will be apps that use Sparkle 1.x. We know that telling people to upgrade doesn't work, especially when it requires re-doing the integration. Many will just never upgrade.

So we'll need to keep around a Sparkle 1.x-compatible branch in working condition, for years, in case we need to release fixes or catch up with macOS APIs.

The more similar Sparkle 1.x and 2.x are, the easier it will be to keep them both updated (e.g. adding stuff like Dark Mode, or using new HTTP APIs, localization fixes, etc.).

[tkrajacic]

Maybe a naïve question but can't we hide the new XPC stuff behind new API and have both variants in the main branch at the same time?

Sure, it would break the workflow for the XPC variant if people use this branch now, but it's not the main branch and not really released, so I think it would be fine?

[tonyarnold]

The issue is bigger than this, as it's about supporting the ecosystem.

I absolutely understand that, but at a certain point it becomes untenable to do both - especially when it's due to users who won't upgrade even if you planted a bomb under their butts. Lump in limited time from contributors, and you have a v2 that's been living to one side for _years_.

I say this with nothing but respect and understanding of what you're trying to do here: Other people's laziness really shouldn't be _your_ problem, @kornelski.

[michelf]

I could be mistaken, but I was under the impression that the XPC branch could be used without the XPC stuff in a non-sandboxed app, similar to Sparkle 1.x. Is there something else that would force developers to rework the way they embed the framework in a non-sandboxed app?

I too agree the current situation can't work for very long. Each accepted pull request to one or the other side adds more work to the pile of things to port to the other one. That can't be good.

[ksuther]

Even if Sparkle 1.x support doesn't end for a while, would it make sense to make the XPC branch master so that it is the new default? Right now Sparkle 2 is considered the secondary branch, and as long as it stays that way that's going to delay adoption of it.

[aonez]

I’m with @ksuther, making the XPC the default branch and keep maintaining the 1.x branch

[sweetppro]

why even bother maintaining the 1.x branch?
2.x supports macOS 10.9+ which was released in 2013!
10.15 comes out this year.
you should be looking to the future, not the past.

it seems like wasted resources supporting the 1.x branch.
Ive been using 2.x without issue for 2-3 years now...

why not just make a clean break?
do devs really even still need to support apps written for Mountain Lion

[digitalmoksha]

I've also been using the XPC branch for 3 years. I second putting 1.x into maintenance mode. Let's get the 2.x branch as the mainline development. Because it's really needed in order to keep moving forward with macOS. Critical fixes could continue to go into 1.x some time, but I don't think any new development/features should go into it.

[jakob]

PLEASE PLEASE PLEASE make the ui-separation-and-xpc the master branch. Or, if you don't want to continue supporting the old Sparkle for a long time, why not just move "ui-separation-and-xpc" to a new repo, for example. sparkle-project/Sparkle2?

Knowing that you have to check out a separate branch to get to the "good" Sparkle really doesn't help adoption of the new version...

[Iomegan]

Someone from the core team, please respond. 😊

[tkrajacic]

Sry to bump this, but is there any progress being made on the decision of how to proceed?

Is the core team even actively thinking about this - is there even a core team at that point?

I mean it has been months and no matter how thorough you are, the decision should have been made a long time ago. It feels just like nobody feels responsible for making it.

[Iomegan]

@kornelski It‘s not that there aren’t enough people to help. We just need a decision to be made. Drop 1.x support, it is not our responsibility if people don’t update.

[kornelski]

I don't have enough time to do the work required to make 2.x production quality.

If you want to help:

• backport changes from 1.x
• make PRs for the website to add proper documentation for 2.x. It's absolutely a blocker and won't be released unless the website has proper polished docs for 2.x integration.

[jakob]

If deprecation of 1.x is not not going to happen soon, what do you think about moving Sparkle 2 to a separate repo? Then at least the folks who use the new version would have a place to discuss the new version instead of all discussion being limited to this giant thread.

[digitalmoksha]

I'm not sure about opening another repo. I think that will make it even more unlikely that a fix on 1.x will get fixed on 2.x. We should be able open issues that begin with [2.x] or something. Or a template is created for use with 2.x issues and auto tag it.

I do think that there should be a real push that says if a fix is going into 1.x, it must also go into 2.x, or be shown that it's already fixed. Same with any enhancements - if it goes into 1.x, it's the submitters responsibility to get it onto 2.x as well.

We might also want a dedicated issue that talks about what is needed to get to 2.x, identify any PRs that haven't been ported over and open issues on them so the community can contribute. Same with the docs, break it into some smaller issues that is easier for someone to pick up and do.

[danmessing]

Is there a reason any new work is going into 1.x at this point? To me that seems like a waste of energy.

2.x is already being used in plenty of production code, so holding up its release based on getting it ready for that doesn't make a lot of sense to me. Similarily, great documentation would be awesome, but any documentation would be more useful to newcomers than having to dig through this thread.

Sorry to sound so negative, but the current state of affairs is frustrating for a lot of people. I very much appreciate the work everyone involved has put and continues to put into this project!

[spearway]

Could we start by something trivial rename the branch 2.x

[kornelski]

Sure, renaming the branch is fine. Adding bug templates for 2.x is fine.

The documentation is really important. It's absolutely unacceptable to switch the repo to 2.x and just keep website as-is with docs for 1.x. That would cause a lot of confusion and grief.

[spearway]

I agree with you we need to address the documentation, although as English is not my original language you don't want me anywhere near it.

At the memento we have a monolithic web site that does not reference any version how do you see that evolving? We need to put the plumbing in place and then put meat on the bones. The difficulty I can see is that most of us switched long ago and at least for me I have forgotten the pit falls…

[michelf]

It's worth mentioning there is already some sort of guide for the XPC branch by @DivineDominion:
https://christiantietze.de/posts/2019/06/sparkle-xpc-branch/
https://christiantietze.de/posts/2019/06/sparkle-xpc-or-no/
https://christiantietze.de/posts/2019/06/sparkle-xpc-setup/

[DivineDominion]

I'd be happy to help on the project docs as well! Does anyone have a vision or checklist I could work against? :) Like @danmessing, what would you have needed info on?

[kornelski]

https://github.com/sparkle-project/Sparkle/issues/1523