POLL: Should time restrict the version of serde that is used?

[jhpratt]

Context: serde-rs/serde#2538

There is a roughly 8 second improvement in compile time for the initial (non-incremental) build, with incremental builds being slightly slower. My thoughts are present here. One correction is that it is not a 3 millisecond improvement, but a 3 millisecond regression per invocation.

Due to the change in serde being a potential security vulnerability, I changed the semver requirement to be <= 1.0.171, which excludes all versions with pre-built binaries. This has caused a fair amount of disagreement about the reasonableness of my action.

Voting

If you support the change and want versions of serde with pre-built binaries to be unsupported by time, please react with 👍.

If you do not support the change and want all versions of serde to be supported by time, including those with pre-built binaries, please react with 👎

All other reactions will be ignored.

This poll will remain open until 2022-08-22 0:00 UTC. After voting closes, a new release will be made if necessary. For the result to be valid, a minimum of 20 total votes are required.

Discussion

I will be leaving the discussion unlocked, but will not be responding to any messages. However, all replies will be monitored for compliance with the code of conduct.

---

Results

The results can be viewed here.

[epage]

Something missing from this is the implications of this change. By putting an upper bound on a version requirement, it can make impossible to resolve dependency combinations that are confusing to end users, especially when these are transitive dependencies (e.g. reddit thread)

See https://doc.rust-lang.org/nightly/cargo/reference/specifying-dependencies.html#multiple-requirements for more details

[pinkforest]

Probably long run only.

IMO on short term time can patch-bump and relax it when this has been vetted and ironed out more at ecosystem lvl

fwiw - It could have been two release streams where major 1 gives you from source and major 2 would give the binary

This would have been informed choice IMO opting into more experimental builds with binaries

The hard truth now is there will be fragmentation regardless - whether it's the top level binary or libraries in the middle

[wezm]

I think the pinning also prevents swapping serde with something like https://github.com/commons-rs/serde-deblobbed since the latter uses the newer version number (see note in their README).

[pinkforest]

That serde-deblobbed patch is broken regardless as it doesn't provide alternative versions and .patch doesn't operate that way.

This just will fragment the ecosystem there is no question of that and nothing will change here given either pin-relaxed or not.

[decathorpe]

I plan to publish a serde-compatible (if possible) fork on crates.io under a different name, I just didn't have the time to do that yet. The -deblobbed fork was just a first step that I could do relatively quickly, but it's only a short-term / stop-gap solution.

Also, what do you mean "patch doesn't operate that way"? I tested the change locally and it looked like it worked (except for a version conflict due to the time crate that I noted in the README).

[AlexTMjugador]

First of all, I appreciate that this poll is being done and that there is a plan in place to deal with whatever decision serde makes going forward (see the context linked in the first post). I think both are necessary to handle this situation well.

---

However, I'd like to continue discussing the security implications of this change, as I believe they are more nuanced than they might appear at first glance, and those nuances may justify changing the actions taken. I also think that, in general, time is not in a good position to make such decisions on behalf of its users. The following paragraphs will be a follow up to what I said in 500f8e4#r124957986, where I discussed the possibility that crates can already run arbitrary code on build via build scripts and procedural macros.

That code can be reviewed (and frequently is by organizations). The binary cannot.

That's a reasonable a priori view, but it's not complete. To begin the discussion, let's keep in mind what I think we can all agree are the essential items to review when assessing whether or not a software supply chain is secure:

• The origin of the supply chain (i.e., where do you get binaries, source code and other build artifacts from). This determines the set of vulnerabilities a malicious actor can use to compromise your security objectives.
• The content of the supply chain (i.e., what binaries and build artifacts your build system ends up with). This determines if the supply chain does bad things.

In the case of the precompiled serde_derive binary, its origin is the serde_derive crate itself, as far as I can tell. That binary targets Linux systems, so the exposure of any threat in it is limited to Linux systems. That reduces or even nullifies its risk in cases where Windows, MacOS or BSD systems are used for builds. (Which means that, as things stand, this measure does nothing but potentially create unsolvable dependency trees for Windows, MacOS and BSD users.) In turn, the serde_derive crate is either downloaded from crates.io, or read from a local vendor copy, for folks that use, for example, cargo vendor.

The relevant contents of serde_derive for this security discussion are the source code of its build scripts, procedural macros, and this precompiled Linux binary. Therefore, a supply chain security policy must define:

• The process to decide how to trust serde_derive's source code.
• The process to decide how to trust serde_derive's precompiled binary.

Approaches to deciding whether to trust serde_derive's source include (but are not limited to) performing code reviews, or trusting the maintainers and other parts of the ecosystem to "do the right thing". (Indeed, if the opportunity and time costs of enforcing a stricter process are greater than the value and/or cost of recovering the information assets you are dealing with, the optimal choice is to "be lazy".)

On the other hand, the approaches to deciding whether to trust serde_derive's precompiled binary are fundamentally similar. The difference is that it is usually harder to verify that a binary is safe to use, especially when it's not clear how exactly it was built. But note the emphasis on the word "usually": source code can also be obfuscated, at which point it is no longer directly readable by humans, and should be treated like an opaque binary. (By the way, given how smart David seems to be, judging by the code he writes, I think that, if he was up to something nasty, he would agree that it is far more stealthy and effective to obfuscate some malicious payload on the build scripts or proc macros that most people trust without further ado than distribute a precompiled binary that will make security-conscious people wary.)

But the possibility of source code being obfuscated is not the only reason why I said "usually". As much as I don't like it, the vast majority of computers in the world are running binaries whose source code and build process are not publicly known: from Intel Management Engine firmware to NVIDIA drivers, passing through proprietary operating systems and beyond, there are many binaries with unknown source code running on a typical system, and any one of them can create security vulnerabilities. In practice, how do people even get some sense sense of security with all these black boxes of code? There are several approaches, and some of them can be easier than reviewing source code under some circumstances:

• Do not any run binary whose build process is not known for certain. This is the trivial solution to the problem, but it may have prohibitively high usability costs, as there may be no practical alternatives for them. In the context of the time crate, this means not running the serde_derive precompiled binary. This can be achieved by:
  • Using mandatory access control checks (e.g., AppArmor, SELinux) to prevent serde_derive from spawning processes. (But this would make it panic, which in my opinion is not a good way to handle an error from which one can recover from by falling back to traditional proc macro expansion.)
  • Targeting musl instead of glibc for Linux platforms, or CPU architectures other than x64: currently, serde_derive only uses the precompiled binary for the x86_64-unknown-linux-gnu target.
• Analyze the behavior of a particular binary in a sandbox environment, and/or use static analysis tools such as disassemblers to reverse engineer its logic (edit: to be fair, reverse engineering optimized machine code is indeed pretty difficult, but note that this is only one of several possible approaches), and keep track of changes made to that binary. If you can be sure that a binary is safe to use no matter how it was built, and that binary hasn't been modified, then you can be sure that binary does not introduce vulnerabilities. In the context of the serde_derive crate, this means verifying that the precompiled binaries are secure, and extending that trust to an approved set of binaries. This can be done by comparing hashes of the serde_derive crate files from a cryptographically secure hash function.
• Sandboxing the build process. If builds are always done in isolated environments (e.g., well-secured containers, dedicated virtual machines with limited network access), malicious code may not even have a chance to compromise valuable information assets.

Given this walkthrough of how would a responsible and resourceful developer would evaluate security vulnerabilities and ways to address them, I think it's abundantly clear that the solution of enforcing stricter than necessary version requirements on serde is ill-suited to tackle security concerns. It's reasonable to define threat models where the cost of breaking a build is much greater than the cost (perhaps already paid) of enforcing other security policies with respect to precompiled binaries, and not only time can't know that, but it forces a vulnerability prevention choice that may be inappropriate for a given policy. (It is no coincidence that none of the ways I suggested to address security concerns with these binaries deal with semver requirements.)

As I said in other posts, the proper way for time to deal with this security issue is to at least contact serde_derive developers about it, and get them to commit to an action plan. With an action plan in place, security risks can be better assessed, and more informed decisions can be made faster, with hopefully less collateral damage to people who rely on the "ecosystem doing the right thing for them" approach.

By the way, may I remember you all of the fact fact that, as far as I know, three weeks have gone by without serde_derive effectively addressing these prebuilt binaries concerns? That's plenty of time for a library such as serde to try to compromise lots of information assets, if it wanted to do so. If we are really serious about security, we should focus our resources on getting serde_derive to do things better, not working around its changes on individual crates, which is an inefficient use of developer time.

[pinkforest]

This is basic supply-chain security ... and a lot about trust when it's lost

Given this change was never done in a manner that ensured

0. .. reasonable disclosure / informed opt-in of this fact

1. .. reproducible builds were possible / straightforward

2. .. auditability and fitness into existing build workflows beforehand breaking them

3. .. development environment now running non-auditable/reproducible binaries

4. .. opt-out mechanism that would mitigate above

This exposes potentially the time development environment to unaudited binaries creating uncertainty at the development stage

Even if someone for once makes auditable / reproducible build it will take long time until this gets into the tooling given wide ecosystem.

This change flew through unnoticed - granted https://crates.io/crates/cackle gave a heads-up

It was easy to be naive when it was not properly documented though.

Many FOSS things require informed "non-free" or so opt-in - not by default - mechanism to binary blobs

Using precompiled binaries at libraries is IMO not the way to go and I am glad time made the choice of not opting into the bins.

This isn't just a security issue but this is a build issue that was supposed to be "Experiment" given changelog.

Does that tell anyone about opting into something knowingly ? I don't think so.

Besides that it takes agency away and this was done silently in the supply-chain.

We should not be operating at reverse-engineering binaries level or changing my workflows to audit any upstream code changes.

The code-changes as per current workflows are designed for source-level changes and not running binary-level reproducible builds.

Sorry but until the system evolves - these kind of changes should not be pushed like this and anyone is welcome to opt-out.

And by opting out,

time will save their contributors and their environments from exposure of the non-audited changing binary dependencies.

Let's keep in mind that people have different build & development environments.

And the fact that it is reversible and temporary which can be relaxed after this has been vetted and ironed out.

So needless to say I am woting 👍

[pinkforest]

I am working both ends ICYMI - I'm trying hard to get opt-out mechanism at least

Cargo.lock gets overwritten all the time. This is about protecting the development environments.

time can choose to support whatever serde version it chooses.

And time not supporting given serde version is implementation detail at best especially given no SemVer breakage.

There is nothing controversial about what time is doing and less so than opting into binaries at project development.

[AlexTMjugador]

time can choose to support whatever serde version it chooses.

And time not supporting given serde version is implementation detail at best especially given no SemVer breakage.

There is nothing controsersial about what time is doing and less so than opting into binaries at project development.

As Ed said, not supporting some serde versions is no longer an implementation detail when it causes unsolvable dependency trees. It also can stop being an implementation detail when it causes several versions of serde to be built, which negatively impacts build times.

My point is that time is not in a good position to take these decisions, and so it shouldn't. Security vulnerabilities should be fixed at their source, so they have the widest fix possible, and not worked around like this. (Edit: or at least be better thought out; if my analysis is correct, non-Linux platforms are not even affected by this.)

[pinkforest]

Possibly in the long run but hopefully when that becomes a bottleneck we will have that opt-out I'm working currently.

There is going to be fragmenation regardless - time can patch-bump and relax it when this has been vetted and ironed out.

[AlexTMjugador]

Of course, some fragmentation is to be expected now, so that's fair. For what it's worth, I'm looking forward to that opt-out too, for reasons that go beyond security.

[danakj]

This is well stated. Some ecosystem fracture around serde is now required unless/until they (or Rust) provide a way to avoid the binary distribution. By avoiding the prebuilts in other libraries it helps avoid further fracture around them as well.

If time ever required serde with prebuilts, then time would also no longer be a viable dependency either for projects that care about supply chain security, auditability and reproducibility. Or complex and unreliable local patching would be needed but that doesn’t feel like a long term nor even really a good short term solution. The prebuilts are unfortunately viral.

[pacak]

1. serde is not the only crate that can use faster compiling proc macros and I'm afraid once other maintainers realize that they also can get away with it - dealing with blob macros can turn into an exciting game of whack-a-mole.
2. compilation time for derive macro is not even a problem, compilation time for the generated code is. in prod on my $day_job switching to precompiled blob (even if it worked - it does not) will make exactly zero speedup since serde derive macro is not in a critical path, in fact it will make it slower due to binary spawn overhead. But even then, shaving off 10 seconds of about 20 minutes of the compilation wall-time that can be attributed to serde directly... 🎉?
3. this problem should probably addressed by cargo with wasm.

Decisions by both serde and time are wrong from the rust ecosystem point of view, but one by time moves things back where they supposed to be so I support them. 👍

[AlexTMjugador]

I'd like to mention that @pinkforest has submitted a PR to serde addressing these security concerns. I think that some readers of this poll may find it interesting to take a look at it.

[sagudev]

I voted 👎🏾, because there are workarounds that this change would disable: serde-rs/serde#2538 (comment)

Also taking options away isn't any better and we should wait for things to cold down.

[pinkforest]

That work/around is DoA in any case regardless what happens here.

[mati865]

How about declaring serde 1.0.171 as the latest "officially" supported and testing it on CI (by running cargo update to pin specific version) but not limiting the users who accept bundled binary?
I'll surely restrict its version in binaries because that's the right place to do that but I can understand users who do not care much about it.

[uklotzde]

I hope that everyone voting with thumbs up is aware of the implications of their decision? Effectively causing a split of the whole Rust ecosystem that has dependencies on serde. This is inevitable if only some but not all crates decide to restrict the versions of serde. You could only use the libraries from either side, but not both. More and more version conflicts will arise.

Personally I don't understand why libraries try to act as a guardian for their users without any necessity. Upper version bounds could be applied by binaries in their local scope without affecting everyone else.

[lu-zero]

For many practical purposes serde with higher version number is broken on linux-x86_64, so pinning the last working one while sorting out what can be done to unbreak it is sensible.

If we can come up with a solution to make everybody happy the next release the pinning can be relaxed.

I'd suggest to make cargo directly interact with sccache so not just syn can be cached but also the proc-macros using it could. But right now I do not have time to play with the concept :(

[AlexTMjugador]

Let me suggest an alternative view of the implications of "libraries acting as a guardian for their users", which I think is an interesting point of view.

This time, serde_derive tried to "act as a guardian" of build performance improvements, thinking that its users would get over it, or agree with it. It can be argued that decision was a bad judgement call.

What prevents time from making similar misjudgements in the future if it insists on "acting as a guardian" over issues that are not its responsibility, nor does it have the full knowledge to decide properly? If anything, this serde_derive situation makes me wary of how a developer can single-handedly make it into news articles for bad reasons.

I reiterate my desire to fix problems as broadly, correctly, and as close to the source as possible. Ill-conceived workarounds are, in my opinion, a recipe for burnout, churn, and inefficiency.

[pacak]

There's a chance that the situation will be resolved: serde-rs/serde#2584 (comment)

[pinkforest]

Ok the issue has been resolved - see:
#614
#615

We've spent some time auditing the previous versions.

[Be-ing]

I speculate that the strong social pressure from @jhpratt's actions with this crate contributed to this outcome.

[AlexTMjugador]

I speculate that the strong social pressure from @jhpratt's actions with this crate contributed to this outcome.

I agree. On a personal note, I'm thankful to Jacob for bringing more eyes to the situation (and even making me think more deeply about security). But I would like to meet him for other reasons, not because things are breaking and bad workarounds are being made 😅

[uklotzde]

I speculate that the strong social pressure from @jhpratt's actions with this crate contributed to this outcome.

That's not an excuse for publishing a patch release without review that breaks builds.

[ssokolow]

1. Serde doesn't follow semver. It'll likely be on 1.0.x forever now that it's hit major version 1 because it merges minor and patch bumps into a single number and only breaks compatibility by accident.
2. dtolnay probably didn't think it was a breaking change for anyone.

[jhpratt]

As the deadline how now been reached, the vote is over. The result is clear: 121 in favor (70%) and 51 opposed (30%). I have saved this page with archive.org for verifiability: link.

Given this result, the versions of serde that contain unreproduced executables (via serde_derive) will remain unsupported. As there was a release last night that removed this, expect the most recent versions of serde to be supported in the near future. Unfortunately, Cargo does not support multiple ranges, so the older versions of serde will be unsupported. However, if and when the binaries are fully reproduced, the range will be expanded to what it previously was.

Thank you to everyone who provided input! There were far more people who voted than I expected. I will be closing this discussion as the poll has ended, but it will not be locked (such that further comments can be made).