[PITCH] Support for binary dependencies

This summary is exactly why I think it's wrong to think of allowsBinary as a security feature and design it with that in mind. If you're going to audit every update to every part of your dependency graph then you'll be well aware of any added dependencies, because you'll have to audit them too. In the much more likely situation where you're not going to do that, whether the malware is in a binary or the source code is essentially irrelevant. You need to trust your dependencies for better reasons than the source being theoretically, but not practically, inspectable. Consider all the unintentionally introduced security issues that survive for years or decades in open source software.

2 Likes

I don't quite understand how moving the opt-in to a configuration file solves this in the general case. Each package needs to be able to work and be tested in isolation, so now the author has to add the new binary to the configuration file of every package in the chain. That seems like a very similar burden to me.

Sorry if I wasn't clear enough -- I don't mean that the whitelist should be global to the user's machine, I meant that the whitelist should apply to the entire dependency graph of a specific root package. The config file is stored at .swiftpm/config in your repository's root directory (again, see SE-0219).

Is that what you were concerned about? I can update my post to try and clarify.

It distributes the load better.

With the allowsBinary flag, for A => B => C => D => E where E adds a dependency on binary package F, E must add the flag, followed by D, followed by C, followed by B, finally followed by A.

With the whitelist approach, if E adds the dependency on binary package F and the new version of E is compatible with the version constraints in the package dependency graph, A can begin using it immediately, without B, C, D having to do anything. That's very valuable.

Think of it in terms of a dependency graph; this allows better "parallelization" of effort.

Ah right, that makes sense.

I think I would prefer the whitelist to be on a binary artifact vs. a package basis, though, since one package can have multiple binary targets. Maybe it could even be combined with my idea of using URL prefix matches for opt-ins, such that one can optionally trust all binaries coming from a certain domain.

Yes, I like that.

Do we want to require that the binary artifacts always come from a remote URL, though? What if a developer wants to store the binary artifact inside the binary package's repo and reference it using a relative path? Whitelisting becomes a little more complicated if that is possible.

Not quite sure, the current proposal requires it, but I was actually thinking it might be good to also allow referencing artifacts from the repository. I don't think anyone has brought it up as something they would like to see on the thread so far.

That clears it. Thanks!

If we do, it would make whitelisting more complicated since you'd need to tie that relative path to the package identity somehow.

@rballard and me discussed the opt-in part of this proposal and came to the following observations:

  • It is true that strictly speaking there is no difference between binary and source dependencies in terms of trust, as some folks on this thread also commented. This makes a per binary packages opt-in during development time less important
  • The difference between having zero vs. any binary dependencies is still significant, for example it will be harder to port the package to a new platform if there are any binary dependencies. Because of this, it would make sense to offer package authors a way to opt-out of binary packages wholesale in order to not add a transitive dependency on one accidentally. It warrants further discussion to figure out whether this is better suited to manifest API or a separate configuration option. There might also be a case to be made to make developers opt-in to binary packages once instead.
  • Increasing the visibility of binary packages being used is still a worthwhile problem to solve, but it can be tackled as a workflow feature by tools built on top off libSwiftPM and doesn't have to be part of this proposal.
6 Likes

Have read through this thread but not fully digested the small details yet - however for the moment, I did want to throw my support in for using XCFramework, as I the company I work for distributes a binary framework that includes resources like storyboards. It's pretty important the final solution support that for wide adoption in the enterprise community especially...

I also am -1 on the whole idea of the "allowsBinary" flag which I don't see adding any benefit and confusing a lot of users that do try and add a binary only framework. For most people adding a binary framework and a source-based framework are equivalent, if they never review the source they are pulling from.

I finally found time to update the proposal a bit with things that were discussed here. I already pushed the changes to git, but I can't figure out how I can edit my post here. (Help welcome :slight_smile:)

To summarise what changed:

Allows binary flag removed

I removed the allows binary flag from the proposal since the discussion showcased that it is not a real security benefit to have it. However, I added an opt-out mechanism as @NeoNacho suggested to allow package creators to disallow any transitive binary dependency. We should discuss wether we want that flag in the manifest or a config file and how to name it.

Platform formats

I updated the section which platform are supported in what format. I took the overview @jakepetroules. Since this seems quite solid. However, I think we should still discuss if we want to support Linux/Windows from the beginning or defer that to a later point.

Updated ArtifactCondition

The first proposed conditions didn't really cover anything useful. I switched them over to the LLVM triplet now as suggest by multiple people. I still think there needs to be more, especially with Linux support like the core-libs-foundation ABI. Would the LLVM triplet be enough for initial Apple platforms support though? @ddunbar @NeoNacho

I still need to add the hash to a the binaryTarget declaration to prevent some open security concerns.

.o file format

Right now the proposal still uses XCFrameworks for Apple platforms. There are two open things to discuss. 1. Use .o files for the above mentioned reasons? 2. If we use .o files can we put them into XCFrameworks for Apple platforms to make the multi architecture support easier?

Packages resolved behaviour

This is still untouched I want to tackle that topic once I am starting to implement these parts, to get a grasp what is needed here. Though input is very welcome!

I want to continue implementing parts of this proposal in the coming weeks. Right now I am still a bit in the dark how I am going to tackle the whole linking parts, but once I am there from the implementation side I should know more.

3 Likes

Either there is some sort of bug, or the thread has been somehow locked to prohibit edits. In any case it is not just you. Normally there is a pencil icon at the bottom of one’s own posts. But it seems to be missing on this thread...?!? I cannot edit any of my posts on this particular thread either.

Edit: But I can edit this one?!? (I still cannot edit the earlier posts though.)

It looks like you can't edit a post in Discourse after a certain amount of time. There's a setting to extend this time, including extending it indefinitely, that should probably be considered.

2 Likes

Regarding .o files: XCFrameworks do not currently support .o files, but they do support static archives (which are really just ar archives containing .o files). But static archives have library semantics, meaning that the linker will only use those .o files that provide the symbols needed to resolve unresolved symbol references from other .o files. This means that if the package code contains, for example, a class whose name is only mentioned in a storyboard or xib file, or is in some other way only looked up at runtime, then that code won't be linked into the product.

In the short term it might be possible to convince Xcode to use the -force_load flag when linking against the library, so that the linker bypasses the usual static library semantics and includes all the .o files, but in the long run it seems cleaner to lobby for an extension of the XCFrameworks format to also allow it to contain .o files (with non-library semantics, i.e. that all the .o files are always linked in).

It's a bit unfortunate to have different formats for the binary artifacts on different platforms, but perhaps that will be inevitable anyway as support for new platforms are added.

I just wanted to point out that the allowsBinary seems a bit futile as I don't think it solves any major security risk that doesn't already exist today. As a package author I can already embed a dynamic library in my sources and load it at run-time:

Sources/DyLibLoader/dylib_loader.h

// Empty

Sources/DyLibLoader/dylib_loader.c

#include <string.h>
#include <unistd.h>
#include <fcntl.h>
#include <dlfcn.h>

#define TMP_FILE_TEMPLATE "/tmp/dylib.XXXXXXXX"

void load_dylib(char const* buffer, size_t len) {
  char file_name[sizeof(TMP_FILE_TEMPLATE)];
  int fd;

  // Temporary file to hold our dynamic lib
  strcpy(file_name, TMP_FILE_TEMPLATE);
  mktemp(file_name);

  // Write the content of our dynamic lib
  fd = open(file_name, O_CREAT | O_RDWR, S_IRWXU);
  write(fd, buffer, len);

  // Load our dynamic lib
  dlopen(file_name, RTLD_NOW | RTLD_GLOBAL);
}

// Content of the dynamic library binary is embedded here
char const dylib[] = {
  0xcf, 0xfa, 0xed, 0xfe, ...
};

static __attribute__((constructor)) void on_lib_loaded() {
  load_dylib(dylib, sizeof(dylib));
}

Package.swift

// swift-tools-version:5.1

import PackageDescription

let package = Package(
    name: "SwiftPackageWithEmbeddedBinary",
    products: [ .library(name: "DyLibLoader", targets: ["DyLibLoader"]) ],
    targets: [ .target(name: "DyLibLoader", dependencies: []) ]
)

Besides, one could engineer a Swift package with plain swift files that conceal malicious code just as easily as with a closed-source binary. The security risk mentioned by others exists regardless of whether SwiftPM adds support for binary dependencies or not and pertains more to realm of trust than anything else.

I think the proposal is great, but I would completely eliminate the allowsBinary flag in any form.

7 Likes

Thanks for the update!

I'd like to propose that we could simplify the proposal to make progress. We could do that in two ways:

  • Only support binaries on Apple platforms for now. As far as I know, these are the only platforms that have ABI stability at this point, so they seem like the prime target for this at this point in time. I think that could potentially mean we could get rid of ArtifactCondition for now, because those would essentially be handled by XCFrameworks themselves?
  • Restrict the supported artifacts to what XCFrameworks support today. There are arguments to be made for supporting .o files, but also a few complications. There is also the desire of framework authors who are already shipping binary frameworks as XCFrameworks to be able to essentially package up their existing artifacts for consumption by SwiftPM. On balance, it seems to me that we can start by supporting existing projects initially and address support for .o files in a future proposal.
9 Likes

I'd support this restriction at this time.

That seems like a reasonable approach to me.

Thanks @NeoNacho for the reply. I can get fully behind these restrictions as this would make the initial implementation way easier. I think going with XCframeworks for now is also the better choice for community since most of the current binary shipped artifacts in Darwin platforms are frameworks. I will work this into the proposal.

1 Like