back
6 comments
Hey @dochtmann :)

Isn't rustls [1] also built on very unsafe groundwork? Namely ring [2], which, according to github, contains 47.3% Assembly and some C as well.

I'm not trolling here - we were discussing this a lot in my peer group lately.

[1] https://github.com/ctz/rustls [2] https://github.com/briansmith/ring

One thing to keep in mind is that the low level building blocks of crypto algorithms can be relatively easy to test, compared to higher level protocol and application code. For example, a block cipher takes simple inputs, usually a couple of fixed-length arrays and maybe some integer flags. There might be a ton of assembly under the covers, but that assembly isn't responsible for reasoning about pointer lifetimes or parsing data formats or any of the usual things that tend to trip up unsafe code. (Like a TLS implementation!) Instead, the block cipher is a pure mathematical function of those inputs, and that makes it relatively easy to come up with a set of test vectors that cover the function. This also means that the C code and Rust code for the same block cipher tend to look very similar.

Now there definitely are some tricky requirements in crypto code that application code doesn't need to deal with, like constant-time requirements. But auditing for those isn't really any harder in assembly or C than it is in Rust. In the end, porting these sorts of core crypto algorithms from C to Rust tends to be more interesting from a build systems and tooling perspective than from a correctness perspective.

The goal of the ring project is to be much safer than OpenSSL without any notable decrease in performance. That is, my goal is to give you memory safety "for free" if you switch from OpenSSL/BoringSSL to ring. In some cases it is better than free because we end up being faster.

The assembly code in ring is some of the most heavily-tested code in the world. It's fuzzed pretty much continuously in various projects that use it, and a bunch of testing has been done on it. It is from BoringSSL, and much of it is shared with OpenSSL and/or Linux kernel. As we are able to replace the assembly code with safer code, we'll continue to do so, just like we've replaced most of the C code with which we started.

thank you for your work in this. we've been using rust-tls/ ring for some time now in our product.
It uses ring for cryptographic operations, yes. However, note that all the ASM in ring is meticulously kept up to date with BoringSSL upstream, which ring was derived from. Plus, there was pretty successful third-party security audit last year.

Also, the goal is definitely to bring all of that code into Rust, unfortunately Rust lacked the features to do that safely (things like const generics).

Could I ask why const generics would be a blocker? I see how it would help facilitate more elegant APIs as well as better stack allocated structures. However is it not possible to live without this, possibly with a slightly more clumsy design?
>Plus, there was pretty successful third-party security audit last year.

The security audit referenced by the post suggests offering EverCrypt as an optional alternative to ring. Are you going to act on the audit's recommendation, or continue to only offer ring?

> Isn't rustls [1] also built on very unsafe groundwork?

Depending on what you mean by "groundwork" literally everything is. Hardware doesn't obey Rust's rules, and you need to interface with hardware to get input, and do output, so literally every program will have unsafe code at the base.

The key difference is that Rust gives you the tools to explicitly demarcate what is safe, and what is not, and build safe abstractions on top of (hopefully validated) unsafe foundations.

> Hardware doesn't obey Rust's rules

Neither does the OS where rustls is running.

I think Rust will have more adoption and more libraries like Rustls will be developed. I also think that when this happens, also more exploits targeting Rust code will exist too. I guess the excuse (sorry for using this word) will be: "In fact, the Rust code is still safe. What happened is that a pointer returned by (or used in) an underlying C library got messed up with a very clever timing attack, and somehow the pointer emerged into Rust code... etc.".

Also, ring enforces everyone to use the latest version by pruning older versions from crates.io, which means your builds will fail every time they update.

Main reason I stopped using it.

Of course, if our claim is that we want to avoid security bugs, and we accept the principle that (without some more specific definition) all bugs are security bugs, then any time ring fixes a bug we want to avoid using the old version...

Now for all I know, ring has never fixed any bugs and it just loves adding new API features so that this pruning has no desirable security properties at all, but in principle I can see that this is the equivalent of the standard boilerplate Linux release text which tells you that you should update to the latest kernel because they fixed bugs.

If you have a complete threat model and if you are capable of the insight needed to examine all changes and determine how they impact that model, you could successfully choose whether to upgrade based on whether a new version fixes a bug you care about. But chances are you don't have such a model and even if you did you aren't capable of the inhuman levels of insight needed, even in a language like Rust (and forgetting that we're talking about this because large parts of ring aren't even in Rust).

crates.io and the Rust community adheres to semantic versioning.

If ring wants to notify me that I should update, they should send an email to a security mailing list, open a CVE, register the cve in any of the rust services to notify users with those dependencies (there are some, like crev), etc.

Pruning your releases from crates.io just means that I am going to be annoyed the first time it happens, will start looking for a solution the second time it happens, and it won't happen a third time (and it didn't). If you want to wake me up a Saturday at 4 am, the world better be on fire.

This is probably the only dependency I can remember as being... more than annoying, toxic. I still prevent any of my dependencies from ending up with ring as a dependency. If that shows up in our dependency tree, CI fails, and that change cannot be committed. Unfortunately, this pruning of old releases was only one of the issues with ring (there were others, like cross-compiling it wasn't easy, etc.). All in all it was a no brainer to drop it as a dependency.

I don't think I've ever met a rust dev with something nice to say about `ring`. In a meetup a couple of years ago another rustacean said: "`ring` is so secure that it protects you from using it in your projects". Sums it pretty well.

The library has couple of thousands of daily downloads so for the latest version, and like 25k daily downloads for other versions, so maybe things changed now.

When I merged security fixes from BoringSSL/OpenSSL, I yanked the old versions of ring that didn't have the security fixes. I thought that was a pretty reasonable policy, however people who like to comment in these forums disagreed very loudly, so I stopped doing that. Not sure that's better, but there's less complaining.

In general, my initial thinking was based too much on the assumption that people would help maintain the things that depend on ring to update them to the latest release. It turns out there's less cooperative maintenance like that than I expected.

I loved your policy and appreciated how principled it was. People here are too harsh and most of them don't write code in Rust where regular updates are much more the norm than other communities.
Yanking releases which have bugs / vulnerabilities in them is very much not the norm in the Rust community.

This is why projects like https://github.com/RustSec exist.

> In general, my initial thinking was based too much on the assumption that people would help maintain the things that depend on ring to update them to the latest release. It turns out there's less cooperative maintenance like that than I expected.

I think that's a reasonable assumption.

What isn't reasonable is to expect people to "upgrade right now". Not everybody lives in your time zone, so when you yank a dependency, you might be breaking a workflow in the other part of the world at 3 am, and if some webserver doesn't deploy or whatever, somebody will get a call.

I'm not suggesting this is an easy problem to solve, but there is a wide range of options before "never upgrading" and "force an upgrade right now". Some of these are supported by Cargo via Cargo.lock, etc. so the responsibility for how this is handled doesn't fall on one library or person.

Building a secure system is also not the exclusive responsibility of `ring`. If I'm building a secure system, I have to assume that `ring` will have a bug that's exploitable at some point, and that someone will use it in a zero-day, and my system needs to be secure even if that happens.

So "updating right now" doesn't buy me much. Its something that can wait until after a meeting, or after my vacation, or until monday. Its not a "the world is on fire" situation, even though it would still be pretty severe.

I'd still like to get "notified" ASAP and asynchronously somehow. While updating ring is low effort, the update still needs to "internal QA,..." etc. at companies, and that takes people's time that must be planned on.

In a properly-designed CI/CD system, a dependency getting yanked isn't an emergency unless you choose to treat it as such. In particular, if you don't want your build to fail because some dependency got yanked then you need to use a Cargo.lock, and you need to ensure that you're not overzealous in your use of cargo-deny and similar tools.

I'm not sure if you were affected by this, but Cargo introduced a (regression) bug a couple years ago that caused it to fail when a crate got yanked when it shouldn't have. This bug was eventually fixed, but lots of people blamed ring for this bug. If this Cargo bug hadn't been introduced then most people who were using Cargo correctly wouldn't have been negatively affected by ring's old policy.

> If you want to wake me up a Saturday at 4 am, the world better be on fire.

In a previous role I actually have had things set so that I might be woken at 4am on a Saturday, IIRC specifically under certain conditions it'd play "Straight out of Compton" at full volume on my Hi Fi to ensure my attention, which gave me about 10 seconds before it gets real loud.

I was leak hunting, specifically looking for a huge leak in a production system that we couldn't reproduce on smaller test systems - so I needed to wake up, attempt to diagnose the leak and then (regardless of whether successful or not) mitigate it (kill the bloated process, one transaction fails but everything else will auto-repair) and go back to bed.

But what I don't understand here is, why are you constantly rebuilding and alerting on failure? A CI flag can wait until Monday stand-up, are you auto-deploying any change of state even when there aren't any humans around to cause that? Why? That strikes me as up there with Apache's "But your Good OCSP response expires in 18 hours, so I stapled this newer Bad one instead" in terms of terrible mistakes.

If instead your new builds fail after pruning, the human who is causing a new build can decide what to do about that when it happens, no need for anybody to be woken at 4am.

Your Saturday 4am may be someone else's Friday 3pm. Humans may be around and doing their normal work.
Which is why you don't[1] push to production on Friday at 3pm.

[1] As always there are exceptions to this rule

I thought crates.io doesn't allow removal?

You can yank crate versions, but that doesn't affect builds which lock a version via cargo.lock so failing builds shouldn't be an issue.

They meant yanking.
That hasn't been the case for a long time, a year or more.
Your builds will only fail if you don't check in your lockfile.
That issue should be fixed in the next release. It is definitely a nuisance, but fixing it is not easy due to the assembly code involved.
Had the same thought the other day: https://cryptologie.net/article/520/cryptography-and-assembl...

Not a fan of assembly for cryptographic code.

This looks great. As I understand it, the major reason openssl is still in use (at all, really) -is a very unhealthy dependency on it's demented and complex Api... Has anything changed that makes it easier to switch from broken ssl lib to a new "perfect" one?

Are we likely to see stdlib changes in python, ruby etc?

The site mentions:

> Enforce a no-panic policy to eliminate the potential for undefined behavior when Rustls is used across the C language boundary.

Is this a thing? Does panic open up for undefined behavior when using a rust library via C?

I can't recall seeing it mentioned before/in other rust threads/projects?

I whish you the best of luck- libopenssl is terrifying :)

Panicking (unwinding) across an FFI boundary is very much undefined behaviour. You'd get analogous cross language issues with C++ exceptions. Or mixing GCC ABI unwinding with MSVC ABI unwinding.

Nonetheless, there is a working group, proposing to make it defined under certain circumstances. https://blog.rust-lang.org/inside-rust/2020/02/27/ffi-unwind...

But is this undefined behavior in the typical c sense (lol, I see you call function possibly_undefined() I'll just make you a sandwich and clobber all registers instead) - or undefined behavior in the sense that it should crash - but might return?
I’m not sure I'd draw a distinction. People used to say you should initialise your variables in C89 to NULL, so you'd get a crash if you forget to properly set the variable. But enough security bugs have been caused by null pointer dereferences.

In the Rust case, IIRC, Rust annotates functions with the LLVM attribute no_unwind. I'd expect bad things, thanks to optimisation passes, if an unwind were to start. Doubly so if mixing GCC with MSVC ABI.

> is this undefined behavior in the typical c sense (lol, I see you call function possibly_undefined() I'll just make you a sandwich and clobber all registers instead)

Yes.

Thank you for that link, I thought (production build) rust panic was simply a core dump. I didn't realize there was stack unwinding going on.
There are two options at the moment [1]:

  - panic=abort (uncatchable)
  - panic=unwind.
With the latter, you would need a catch_unwind at the FFI boundary.

[1] https://doc.rust-lang.org/cargo/reference/profiles.html#pani...

I see. I would think that for a library, in production, all you'd want for panic was a crash. (I guess this is the abort case).

AAre the panics being discussed for removal in rust tls "soft" errors that should not crash?

There's a categorization here (I don't have access to the triage spreadsheet though):

https://github.com/ctz/rustls/issues/447#issuecomment-820719...

It seems like there's been the most effort on category 4, the ones that can't be reached but can often be proven away with refactoring and typestate. Removing these ensures that the APIs don't expose Result types to callers unnecessarily.

Category 1-3 have different severity, but they are all planned to be folded into Result error values.

On the Python angle, I recently spent some time building Python bindings for another Rust project with PyO3 and was quite happy with the result. Since then, I’ve been toying with the idea about building a somewhat compatible replacement for Python’s ssl module based on rustls. I’d be happy to work on that, but I’d need to find some way to fund it (Patreon or GitHub Sponsors might work, but so far I haven’t had a lot of luck gaining sponsors there).
What does this mean?

> Make it possible to configure server-side connections based on client input.

The other three bullet points I can immediately understand what they're achieving (the "No-panic policy" is trickiest but since I'm learning Rust I knew what that means, and it also links a ticket describing more)

But for this one I haven't a clue, maybe it should be obvious, and somebody else will explain.

Using (defined) properties of the TLS ClientHello to determine how the server will respond.

For example, changing the certificate used based on the ALPN identity, the SNI server host, and the advertised client ciphersuites.

I've been doing this by implementing ResolvesServerCert, which has access to the ClientHello. It covers the acme alpn use case.

Though you can't use it to pick other properties, like making an ALPN protocol conditional on SNI.

Rustls depends upon Ring. Ring itself has a lot of code from BoringSSL, an OpenSSL fork.

Long term, is this OK? How do we ensure the assembly and C code in Ring is as safe as Rustls itself?

See my other answer.
RFC 7250 support (Raw Public Keys) is not in the list¹ of “Current features”, but neither is it listed under “Possible future features” or even “Non-features”. GnuTLS supports this feature, so GnuTLS is what I use. What is the status of this feature in Rustls?

EDIT: I found issue #423², but the reporter is not making a compelling case, since the use case presented is for a prospective new application, yet to be written.

1. https://github.com/ctz/rustls#current-features

2. https://github.com/ctz/rustls/issues/423

I don't think the list of possible future features has been kept especially up-to-date -- we should probably fix that (or get rid of the list). If you want to weigh in on that issue to discuss why you think it's a valuable feature to add, that would be great!
I don’t have a Github account, so I won’t, sorry.
If you reply here, I'm happy to relay your comments to that issue.
My comment is basically “I have a project with components written in C which might benefit from Rust, and would probably also benefit from Rustls. But the project requires RFC 7250 support, which only GnuTLS currently has.”
Congrats for securing the contract! Looking forward to the IP address stuff especially.