[Pre-RFC] Cargo Safety Rails

  • Feature Name: cargo_safetyrails
  • Start Date: 2017-07-13
  • RFC PR: (leave this empty)
  • Rust Issue: (leave this empty)

Summary

This RFC proposes Cargo Safety Rails, allowing crates to ensure that dependent crates don’t use unsafe language features.

By declaring a dependency “untrusted”, the user tells cargo to compile it with the -F unsafe_code option, preventing the dependency from using the unsafe keyword.

Motivation

Type safety allows the programmer to have strong guarantees about what incorporated code can, and cannot, do. For example, if I incorporate a Rust library, I don’t have to worry about it accessing arbitrary memory, modifying my private variables etc. However, this is only true as long as the library code doesn’t use unsafe features of the language.

Currently, unsafe features are guarded in Rust with the unsafe keyword: If I want to do something that might violate type-safety (or borrow rules), I have to wrap that code in unsafe. In addition, the compiler let’s us prevent a crate from using the unsafe keyword entirely by either using the -F unsafe_code compiler flag or adding #![forbid(unsafe_code)] to the crate’s top-level module.

It is also possible to ask Cargo to compile all code with the unsafe keyword forbidden by setting the RUSTCFLAGS environment variable.

But what if some of my dependencies should be allowed to unsafe (like a trusted binding to a C-library, a hardware interface layer or data structure that achieves high efficiency through some memory tricks)? In theory I could vendor every one of my dependencies and manually add #![forbid(unsafe_code)] to them, but that imposes a high barrier for updating exactly the dependencies I should be able to update with relatively little auditing. Moreover, as dependency trees become larger, this becomes unwieldy.

The safety rails feature would, instead, allow Cargo users to specify, declaratively, which dependencies the don’t trust completely and be confident they will not be allowed to use unsafe language features, without having to audit each version of the code, while allowing trusted dependencies to do so. It would also result in a explicit list in the Cargo.toml file of dependencies that deserve extra scrutiny.

Side note: I believe @josh did some analysis of crates.io and found that a relatively small number of crates currently use unsafe features, and those that do are often C-bindings. This would imply that a feature allowing a Cargo use to disable unsafe for some dependencies would actually be usable with crates.io immediately.

Detailed design

Since this is a Pre-RFC, I would appreciate feedback on this initial design proposal. I’ve tried to come up with a design that doesn’t change the experience for Cargo users who wouldn’t use this feature (e.g. dependencies should be trusted by default).

User Interface

The dependency section of the manifest format has a new, optional, field called “safety”, which can either be “trusted”, “untrusted” or “inherit”:

[dependencies.awesome_widget]
features = [ ... ]
safety = "untrusted"

[dependencies.carefully_written_c_binding]
features = [ ... ]
safety = "trusted"

Semantics

When a dependency with “untrusted” safety, cargo adds it and all of it’s dependencies to a list of crates that will be compiled with -F unsafe_code.

Setting the safety of a dependency to “trusted” will remove it from this list, meaning the user trusts this crate to use the unsafe features, regardless of whether it’s used directly or through a dependency.

A dependency with safety “inherit” is trusted by default, unless it is in the dependency graph of an “untrusted” dependency.

The core crate is always trusted.

Propogation

Safety specifications in “trusted” dependencies can only serve to constrain the list set of trusted crates. That is, only the manifest file for the artifact being compiled can constrain the declare a dependency “trusted” and have it take affect, but any “trusted” dependency can declare it’s dependency “untrusted”.

“Untrusted” dependencies have no affect on the safety of their dependencies since their dependencies are untrusted by default, and they should not be allowed to add “trusted” signifiers.

How We Teach This

The terms “trusted” and “untrusted” were chosen to be distinct from “safe”/“unsafe”, but to have a similarly intuitive meaning. In particular, the declaration:

safety = "trusted"

Means that the user trusts the dependency to preserve Rust safety semantics, even if it happens to use unsafe features, while

safety = "untrusted"

means the user doesn’t trust the dependency to preserve type-safety, and wants to compiler/build system to vouch that it doesn’t use the unsafe keyword.

In this sense, this feature should be presented as a way of enforcing an existing concept (type-safety and the unsafe keyword guard) in the build system.

This feature does rely on a relatively nuanced understanding of type-safety, how it can be broken, and how it can be preserved despite using the unsafe keyword. So, presentation probably has to rely on the reader having intermediate familiarity with both the language and Cargo. Since the design makes the feature transparent to users who don’t use it, it’s probably OK to defer explanation to more hidden parts of the documentation.

Drawbacks

It’s not clear yet how many applications could actually use this kind of feature, since it’s not clear how much of crates.io can be compiled without unsafe (though initial results imply that a lot of it).

Moreover, this design might require users to list a large number of dependencies they don’t directly depend on only to be able to mark them “trusted” because their dependencies use them. Over time, as dependency graphs change, this could result in a large list of unused dependencies.

Alternatives

I’m not entirely sure! That’s what I’m asing y’all for!

My main thought is that the design uses a default trusted model in order to maintain compatibility with existing crates. An untrusted by default might yield a cleaner design with more concise Cargo.toml. It might be possible to get the best of both worlds with a Cargo flag that enables the feature (so, without the flag, all crates are effectively trusted).

Unresolved questions

Are there crates, other than core that should be trusted by default? std and collections are likely candidates, but I wonder how that impacts std-less bare-metal applications.

18 Likes

I like the idea.

One thing I would hope to see is that a crate can detect if it's being compiled with unsafe allowed or not, so if it is using unsafe purely for performance optimizations it can fallback to a slower safe version. For example I have used the unsafe String::as_mut_vec function when I have been dealing with purely ASCII characters, but it would be possible to provide a safe version that does something like operate on a separate Vec then call String::from_utf8 if I'm not allowed to use unsafe.

I wonder if it would make sense to have these dependencies explicitly marked as unused. It may be nice to not need to specify a version number for them and just take whatever version is actually used transitively, and to have Cargo warn if you are trusting a crate that is no longer a part of your dependency tree so you can cleanup old crates.

5 Likes

I totally agree. It would be good if this could be done without changes to the compiler, e.g. as a cfg variable perhaps? Would require reserving a feature name?

Agreed that it would alleviate this problem, though specifying version bounds for "trusted" dependencies is probably a practice we'd want to encourage (have you really audited all possible versions?)

I like it! Do you have plans or thoughts for other effects the “safety” key might have? I’m a little adverse to adding an additional indirection layer of nomenclature. Compare against an interface such as:

[dependencies.xxx]
features = [ ... ]
allow_unsafe = "no"|"only_this_crate"|"recursively"

Where the exact action (‘is unsafe allowed’) is written in direct language, and the possible recursion to allowing unsafe code in dependencies is explicit as well.

1 Like

Agreed. I think that syntax might be more clear. I think the only potential caveat is that because there it propagates in a way that the unsafe_code item doesn't, it might be confusing. But, with sufficiently well chosen values, that might not be a problem.

I have some comments related to indirect dependencies.

In my opinion, the best way to manage unsafety in Rust is to isolate unsafe code in small crates or modules with safe public APIs. So if I have a library that wants to do something that can’t be done in safe Rust, typically I’d write my “main” library in safe Rust, and make it depend on smaller libraries that provide safe wrappers for unsafe code. This means the crates containing unsafe code typically end up as indirect dependencies of larger applications.

For a real-world example, the csv-core crate contains no unsafe code, but it depends on the memchr crate which does. As a consumer of csv-core, I would want to use “Safety Rails” to forbid unsafe code in csv-core but not in memchr. So I have the following questions:

  1. Can I override the safety of an indirect dependency without making it a direct dependency?

  2. Are safety settings only used from the Cargo.toml of the top-level project being built? (That is, are dependencies’ Cargo.toml files ignored for this purpose?)

  3. If my project depends on crate A with safety = "trusted" and crate B with safety = "untrusted", and both A and B depend on crate C, then how is crate C compiled? The rules as stated imply that C will be compiled with -F unsafe_code like an untrusted crate, but it’s probably worth discussing this explicitly.

5 Likes

If std is not trusted by default, how would one make it trusted? Currently, adding a [dependencies.std] section to Cargo.toml will tell Cargo to find and build std from source, rather than leaving it for the linker to find. (And since it's not on crates.io, it would also require a path or git key, and then the project that uses it cannot be published on crates.io either.)

@mbrubeck excellent questions. They exactly crystallize the design space.

With the design I've suggested, I think basically no, although it is technically part of your dependency graph, so I'm not totally sure how useful it would be to avoid that.

I tried to get at this a bit in the "Propagation" section: the idea is that they are ignore unless a "trusted" dependency further constrains the set of "trusted" crates. The goal is to let trusted dependencies that use unsafe to be more careful about which of their dependencies are also using unsafe (because as a trusted crate author I want to be able to say strong things about my code). But untrusted crate's safety settings are ignored, and trusted crates can't mark things trusted that are not (those settings will simply be ignored).

Agreed, that was exactly the intention. I'll add this more explicitly. On that note, I believe this serves to make the design simpler, but I have not yet convinced myself that this is The Right(tm) behavior... Working through some examples would probably help.

Ah, you’re right. std probably should be default trusted. In general, it’s probably the right choice to make everything that "ships with rustc" trusted by default. A core library with an unsoundness or safety bug is bigger problem than on that should be solved here most likely.

1 Like

People are probably already aware, but in case they aren’t, Safe Haskell is a similar mechanism for Haskell (since 2012). We probably should learn from it.

4 Likes

Indeed! There are important lessons to learn from Safe Haskell’s experience, I think.

(The author of Safe Haskell was my labmate/roomate/biz-partner for 6 years and a research project we worked on was one of the motivations for it. So my opinions about takeaways should be taken accounting for my very strong bias)

  1. Safety declarations at the module level seemed to have been a mistake. Basically, package authors seemed to have a hard time figuring out which modules should be marked Unsafe vs. Safe vs. Trustworthy.

  2. Safe Haskell proved to be a pretty decent forcing function to push package maintainers towards using safer constructs. In the Haskell world this was a more difficult problem than currently in Rust due to how prevalent certain language extensions are and because Haskell doesn’t have efficient binary strings in the core libraries (so Binary, bytestring, Vector etc were all unsafe).

  3. Forcing package authors to mark their modules/packages as trustworthy was a big barrier. For example, until aeson (one of the popular JSON parsing libraries) marked itself trustworthy, it was difficult to do anything useful Web-related with Safe Haskell (our research project was web-related, and we basically ended up wrapping a bunch of packages in tiny packages that just declared themselves Trustworthy). Better to let the end-user just decide which packages to trust. This was problematic in Haskell in a way it isn’t in Rust because Haskell doesn’t/didn’t have an equivalent of the unsafe keyword so the notion of trust management and language safety gating ended up being conflated in Safe Haskell

2 Likes

What about specifying a list of trusted crates out of both the direct and the indirect dependencies?

An example Cargo.toml could look like this:

[security]
allowed_unsafe = ["crossbeam", "binding-to-some-clib"]

Once an allowed_unsafe key is added, all other crates that are compiled prior to this crates must not include unsafe code. Of course there should also be an inverted version of this:

[security]
deny_unsafe = ["some-networking-lib", "some-decoder"]

This solves the problems with indirect dependencies by simply ignoring which dependencies are indirect and which aren’t.

8 Likes

I think this is a good idea. Others have already mentioned two points that I agree with:

  1. Let's not try to conflate "trust" with "safety" and just use safe/unsafe in all nomenclature.
  2. I want to be able to exempt non-child dependent crates from the no-unsafe rule, for purposes of FFI bindings and the usecase that @mbrubeck proposed. This kind of specification is not currently supported by cargo but it is actually needed already. For example, if you want to enable a feature on a grandchild crate that is not re-exported by the child crate, you have to depend on it directly.

It would be interesting to add a way to the cfg() mechanism to detect lint status. For example: #[cfg(lint=forbid(unsafe_code)]

1 Like

I strongly support having some kind of feature like this. My main issue with the OP post is that the nomenclature used is extremely unintuitive to me, for a lot of reasons, so my counter-suggestion would be that we simply call the feature "allow_unsafe" instead of "Safety Rails" and have syntax like this:

// This is the explicit opt-in
allow_unsafe_default = false

// These whitelists can include any crates at all, not just your direct dependencies
// Presumably they would have default values with all the "ships with rustc" crates
allow_unsafe_whitelist = ["std", "memchr", ... ]
allow_unsafe_dependencies_whitelist = ["std", ... ]

[dependencies.awesome_widget]
features = [ ... ]

[dependencies.carefully_written_c_binding]
features = [ ... ]
allow_unsafe = true

[dependencies.awesome_library_that_depends_on_some_carefully_written_c_binding]
features = [ ... ]
allow_unsafe_dependencies = true
allow_unsafe = false // just to be clear this library should not directly contain any unsafe code

(I don't know .toml very well but hopefully this is close to what it actually allows)

I am okay with long keys like "allow_unsafe_dependencies_whitelist" because this seems like the sort of feature where explicitness and zero ambiguity are more important than optimal ergonomics. If you're just trying to get stuff done, you won't be using this feature anyway. There might be a more optimal design where allow_unsafe takes several string keys, but I couldn't think of a set of terms for only the crate, only dependencies, both and neither that I was happy with (in the sense that each key's meaning would be completely unambiguous even in isolation, to a developer unfamiliar with this feature).


But I think we might be imagining somewhat different use cases for this feature. In my mind, this feature is only useful if a dependency containing unsafe code is a serious thing that must be audited or avoided or fixed or have some other action taken besides simply adding to a whitelist. That's why I found it puzzling to see this line in the Drawbacks section:

Moreover, this design might require users to list a large number of dependencies they don't directly depend on only to be able to mark them "trusted" because their dependencies use them.

To me, this is only a drawback if you aren't the target audience for this feature anyway. If you are the target audience, you're gonna be doing more than listing these dependencies in a .toml file. I am definitely not part of the target audience today, but I would be if I ever worked on fighter jet software or pacemaker software or some other sort of zero-fault-tolerance work. I also find the idea of "propogating" these attributes by default very strange in that context. Unless I've explicitly said in my own .toml file that "I trust BurntSushi to never allow anything in his dependencies that I wouldn't allow in mine", I wouldn't want all of the allow_unsafe attributes in regex and walkdir to automatically apply to my crates. Not because there's anything morally wrong with those crates, but because I might be following the Rust equivalent of MISRA and categorically forbidding something like dynamic allocation that most crates would have no issue with.

Is that anywhere near what everyone else has in mind?

Adding indirect dependencies as direct dependencies has the following disadvantages. If your project depends on crate A, and A depends on B, then:

  1. If A disables default features of B, or disables it completely on some platforms using [target.*.dependencies], then you need to do the same in your own manifest. Failing to do this will silently cause extra features or crates to be built.

  2. If a new version of A removes its dependency on B, or switches to new major version of B, you now need to make the exact same change in your own manifest too. Failing to do so will cause extra versions of B to be built and linked into your project.

  3. If your library is compatible with multiple versions of A that depend on incompatible versions of B (say, "A 1.2.4" depends on "B 1.0.0", and "A 1.2.5" depends on "B 2.0.0"), or where one version of A depends on B and another doesn't, then there's no simple way for your dependencies to match both versions. Some of your library's consumers will have unnecessary versions of B linked into their applications, or you will need to expose Cargo features and require your consumers to specify them based on which version of A they are locked to.

  4. If you have indirect dependencies on both B 1.0 and B 2.0 at the same time, you cannot add both of them as direct dependencies, so there is no way in this proposal to mark both of them as "trusted."

[edited to add item #4]

1 Like

I’m opposed to this. This demonizes unsafe as if it wasn’t a legitimate part of the language.

For library authors this would create a great pressure not to use unsafe at all, even in places where it makes sense.

It’s often possible to avoid unsafe at cost of needless runtime overhead, much greater complexity, and less elegant APIs. This feature would indirectly incentivise these things.

12 Likes

Trying to summarize some of what’s been said so far a bit:

In general, people seem to think just sticking to terms like allow_unsafe is more natural, rather that introducing new terms like “trust”.

@Nemo157 would like for dependencies to be able to tell if it’s allowed to use unsafe or not so I can use more optimal code if it can. @jethrogb proposed it might good to do this through the cfg() mechanism.

@mbrubeck pointed out that there are non-cosmetic issues with forcing non-direct dependencies to be direct dependencies and @lxrec & @konstin proposed syntax that is independent from the dependency declarations.

@kornel opposes the proposal because it might discourage library authors from using unsafe.

4 Likes

@kornel I think I understand your concern, though isn’t the name of the keyword unsafe already somewhat communicating it’s not a totally “legitimate part of the language”?

In any case, the suggestion is certainly not that no one should ever use unsafe, but rather that there is a way for library users to leverage the build system to help them verify which parts of their dependency graph is technically able to do arbitrary things and which is strictly bound by the type system. In fact, I imagine a common use case might be for authors of libraries that use unsafe to help reason about what is happening within their unsafe blocks if they call code from dependencies).

I suspect most users will not use this feature (which is why the proposal has it off by default). in those case, it would have no impact on how users of a library perceive that library relative to today.

3 Likes

I don't think that unsafe should be something every Rust developer has to be familiar with. Its a legitimate part of Rust, but only for a tiny subset of usecases like FFI with existing C libraries or the OS or providing low level constructs.

Its generally a good policy to keep the use of unsafe as low as possible, and even better to not use unsafe at all except its actually required.

Even at the young age of Rust we've already had a security incident due to misuse of unsafe, where rust-base64 was vulnerable to a buffer overflow simply because the author was using unsafe for a problem "to make it faster".

2 Likes

This concept seems hostile to the health of the crates.io ecosystem.

This makes changes which are obviously patch updates (adding some unsafe code to optimize a routine) into breaking changes on my clients because someone (anyone?) in the chain declared they don’t trust me. This makes updating libraries even more of a headache.

I also don’t like that all forms of this proposal encourages even more micro-crates (which I consider a net-negative). I don’t want people to make foo and foo_unsafe_parts crates to work around the problems this system introduces.

Reducing the amount of unsafe code in the world is a noble cause, but I think at some point you need to back off the technical aspects and focus on the social ones.

9 Likes