Tracking Issue for LLVM Control Flow Integrity (CFI) Support for Rust

[rcvalle]

This is a tracking issue for the LLVM Control Flow Integrity (CFI) Support for Rust project (see the design document).

Steps

LLVM CFI support for Rust work will be implemented in these steps:

• [x] Add option to the Rust compiler driver and frontend. (#89652)
• [x] Add support for emitting type metadata and checks to the Rust compiler code generation. (#89652)
• [x] Implement Rust-compiled code only LLVM CFI support by using Rust-specific type metadata identifiers. (#89652, #95548)
• [x] Define type metadata identifiers for cross-language LLVM CFI support. (See the design document.)
• [x] Change initial implementation to use the defined type metadata identifiers for cross-language LLVM CFI support. (#95548, rust-lang/rfcs#3296, #105452)
• [x] Create documentation and tests. (#89652, rust-lang/rustc-dev-guide#1234, #95548, #105452)

Unresolved Questions

• Should integers always be normalized?

Implementation history

1. 89652

2. rust-lang/rustc-dev-guide#1234

3. 95548

4. rust-lang/rfcs#3296
5. [LLVM] Add CFI integer types normalization to Clang compiler

6. 105452

Bonus implementation

1. 105109

2. rust-lang/rustc-dev-guide#1529

Known issues

https://github.com/rust-lang/rust/issues?q=is:open+label:PG-exploit-mitigations+CFI

[rcvalle]

r? @eddyb

[rcvalle]

@rustbot label +PG-exploit-mitigations

[rustbot]

Error: Label PG-exploit-mitigations can only be set by Rust team members

Please file an issue on GitHub at triagebot if there's a problem with this bot, or reach out on #t-infra on Zulip.

[rcvalle]

@rustbot label +PG-exploit-mitigations

[rustbot]

Error: Label PG-exploit-mitigations can only be set by Rust team members

Please file an issue on GitHub at triagebot if there's a problem with this bot, or reach out on #t-infra on Zulip.

[rcvalle]

@rustbot label +PG-exploit-mitigations

[rustbot]

Error: Label PG-exploit-mitigations can only be set by Rust team members

Please file an issue on GitHub at triagebot if there's a problem with this bot, or reach out on #t-infra on Zulip.

[rcvalle]

@rustbot label +PG-exploit-mitigations

[rustbot]

Error: Label PG-exploit-mitigations can only be set by Rust team members

Please file an issue on GitHub at triagebot if there's a problem with this bot, or reach out on #t-infra on Zulip.

[RalfJung]

All of the "steps" listed in the issue have been checked. What is still being tracked here, what is still missing?

[rcvalle]

All of the "steps" listed in the issue have been checked. What is still being tracked here, what is still missing?

I was planning to close it when I finish fixing all the known issues--I've two PRs open that should fix most of them: #113593 and #113708--but I'm also okay with closing it now. What do you think?

[RalfJung]

Either work for me, I was just wondering about the current status. :)

Is there an issue tracking the availability on stable?

[rcvalle]

Either work for me, I was just wondering about the current status. :) Is there an issue tracking the availability on stable?

Not yet, but I'll create one right after. (FWIW, here is our roadmap for the rest of 2023 and beyond: https://hackmd.io/@rcvalle/H1epy5Xqn.)

[Nilstrieb]

Is there a reason why unnormalized integers are supported in rustc at all? It is clearly not something that is supported by Rusts model and is something that is very C specific. I doubt there's a significant security benefit from not normalizing them, though I do not know details. Should that just be removed with only normalized integers being supported? I also have some concerns about the cfi-types crate. If someone is supposed to use this crate (or roll their own aliases) to use the CFI feature, I think that would have a huge negative impact on the feature. Either it would just be unusable in practice because dependencies doing FFI don't use it or these dependencies would all receive patches to use this crate, which seems quite bad too. With integer normalization, I don't see why this crate would be needed, as rustc can just pick the correct CFI encoding for its primitive integers, making the crate redundant and making CFI just work for everyone.

[RalfJung]

@Nilstrieb what are unnormalized integers, which issue are you referring to?

I read through the readme at https://crates.io/crates/cfi-types, and that part seems to be about the idiosyncrasies of C integer types. Is it really required to import those idiosyncrasies into every language that wants to have CFI interop with C? Even many C codebases stopped using short, int, long, long long and switched to explicit sizes instead.

[workingjubilee]

@RalfJung These "idiosyncrasies", as you call them, are "unnormalized integers".

The "normalized integers" are a CFI mapping using the integer types with their integer width, so in a C data model where c_short and c_int are both 16-bits (a legal Standard C data model), they both meet the i16 form.

The "unnormalized integers" violate that.

[RalfJung]

Ah okay. I guess I am agreeing with Nilstrieb then, only normalized integers really make sense for Rust.

[rcvalle]

Sorry for the late replies. I'm out traveling for speaking on a conference and then on vacation.

Enabling the integer normalization option by default in rustc wouldn't help on cross-language CFI because most Rust integers have the same encoding as when normalized already (i.e., encoded as u<length><type-name> as vendor extended type), and most of the integer normalization work is actually done by the foreign language compiler (i.e., Clang). It'd just degrade the protection of a program written in Rust only (i.e., when a foreign language isn't used), and we want programs written in Rust only to have the best protection by default.

The cfi_types crate helps in other scenarios, such as when a precompiled library needs to be linked to a program written in Rust, but wasn't compiled either with integer normalization or CFI enabled, and recompiling it isn't an option (and avoid adding e.g. https://github.com/rcvalle/rust/blob/55e3dc487fdf8025fb99301883e24af15323912a/library/std/src/sys/unix/thread_local_dtor.rs#L23 thoroughout the code).

The integer normalization option also overrides the cfi_types creates use, so there are no incompatibility problems if a crate decides or needs to use it by default.

[RalfJung]

It'd just degrade the protection of a program written in Rust only

How that?

We explicitly document u64 and usize to be ABI-compatible on 64bit platforms. So if the function takes usize and the caller uses type u64, that is completely legal and should not be stopped by CFI.

[rcvalle]

It'd just degrade the protection of a program written in Rust only

How that?

We explicitly document u64 and usize to be ABI-compatible on 64bit platforms. So if the function takes usize and the caller uses type u64, that is completely legal and should not be stopped by CFI.

Are usize and u64 interchangeable in the Rust language as types or are they different types?

[RalfJung]

They are different types but ABI-compatible.

[rcvalle]

They are different types but ABI-compatible.

Which means that for CFI they're different types, but can be used in place of one another if the proper conversion is done at the call site, and CFI will allow that.

[RalfJung]

It'd just degrade the protection of a program written in Rust only

Could you give an example of that? I'm not very familiar with how CFI works. Here's an example of a program that should be accepted, even though call-site-type and callee-type are not the same.

Based on what @Nilstrieb says, we should at least clearly document than cross-language CFI is only expected to work with normalized integers.

[rcvalle]

It'd just degrade the protection of a program written in Rust only

Could you give an example of that? I'm not very familiar with how CFI works. Here's an example of a program that should be accepted, even though call-site-type and callee-type are not the same.

Your example demonstrates exactly what an attempt to redirect the control flow of a program by an attacker exploiting a (likely memory corruption) vulnerability would look like, and what CFI protects against, and why the integer normalization would degrage the security of programs written in Rust only (because it would allow it).

An example that would work would be to have the correct function declaration/type for the function pointer and convert the type instead.

Based on what @Nilstrieb says, we should at least clearly document than cross-language CFI is only expected to work with normalized integers.

Yes, it's documented at https://doc.rust-lang.org/beta/unstable-book/compiler-flags/sanitizer.html#controlflowintegrity.

[RalfJung]

Rust explicitly allows certain ABI mismatches. Those are not a sign of an attack. There's no chance of data misinterpretation or so here, behavior is entirely well-defined. If CFI rejects those programs then it rejects valid Rust programs. That's bad.

Here's another example of a valid program.

Yes, it's documented at https://doc.rust-lang.org/beta/unstable-book/compiler-flags/sanitizer.html#controlflowintegrity.

Ah, great. :)

[rcvalle]

Rust explicitly allows certain ABI mismatches. Those are not a sign of an attack. There's no chance of data misinterpretation or so here, behavior is entirely well-defined. If CFI rejects those programs then it rejects valid Rust programs. That's bad.

Here's another example of a valid program.

What I'm saying is that these examples demonstrate it, not that these are cases that can and would be exploited. These demonstrate/have the same effect of an attacker redirecting the control flow of a program by exploiting a memory corruption vulnerability and overwriting a function pointer to perform a code reuse attack, redirecting the control flow to existing code they can benefit from.

This is how type-based control flow protection works. It works at the language's type system level, and not the ABI level, and introduces such constraints (and in Rust case, these are unsafe operations/transmutes only, other languages are not), but provides ways to allow these with additional options, such as with the integer normalization and pointer generalization options.

[Nilstrieb]

I think it's problematic to support "extra protection" by rejecting valid Rust programs. If CFI sees adoption in the ecosystem, people will start opening issues for crates that write correct code to get them to "fix their code" and support CFI. It can also be confusing to users, who start using CFI and then get crashes for their correct code. Documenting this as "If you want to use CFI on Rust, remember to pass this 'make it work' option" is also not ideal. Instead of having a 'make it work' option, it should just always work :). All of this is assuming that there is not a significant loss of security with normalized integers. It's obvious that there's gonna be some loss, but I do not know how much, I assume not a lot. If you can show that the security benefit is significant (and not just 1% or similar which I remember reading somewhere? Not sure though.) then it may make sense to add unnormalized integers as an opt-in. But without significant benefit, I think it's a pretty bad idea to support it.

[rcvalle]

I think it's problematic to support "extra protection" by rejecting valid Rust programs. If CFI sees adoption in the ecosystem, people will start opening issues for crates that write correct code to get them to "fix their code" and support CFI. It can also be confusing to users, who start using CFI and then get crashes for their correct code. Documenting this as "If you want to use CFI on Rust, remember to pass this 'make it work' option" is also not ideal. Instead of having a 'make it work' option, it should just always work :). All of this is assuming that there is not a significant loss of security with normalized integers. It's obvious that there's gonna be some loss, but I do not know how much, I assume not a lot. If you can show that the security benefit is significant (and not just 1% or similar which I remember reading somewhere? Not sure though.) then it may make sense to add unnormalized integers as an opt-in. But without significant benefit, I think it's a pretty bad idea to support it.

As I mentioned in another thread already, I wish fine-grained type-based CFI was an enable and forget mitigation as many others because I want as many users to use it as possible, but unfortunately it does require some code care and maintenance for use. (And it's not enabling integer normalization by default that is going to change that.)

And It's actually common for large code bases to have those CFI violations when CFI support is initially implemented. For example, the Linux kernel had many that had to be fixed to have it initially supported and enabled.

[RalfJung]

It is also fairly common for large C codebases to have UB. That does not apply to Rust though. In Rust we have a very clear baseline for what a program and a library is expected to ensure: no UB. Period.

Having a "protection" mechanism apply higher standards than that is highly problematic and would need to be widely discussed in the project. I certainly think it's a bad idea. Protection mechanisms should detect UB. They shouldn't raise false positives in non-UB programs.

Rust is not C, so not all things that make sense in C also make sense in Rust.

[Nilstrieb]

Are you saying that codebases require some care to get working because they're filled with bugs that CFI exposes? If that's the case then that's great and we want that! I just see a significant difference between "it doesn't work because there's UB" and "it doesn't work because the sanitizer doesn't understand Rust semantics". The former is awesome and a feature, the latter is a bug that I'd like to see eliminated with integer normalization.

[rcvalle]

Happy to continue this discussion in a new Zulip thread under project-exploit-mitigations. Let's move it there if you all agree.

[workingjubilee]

The cfi_types crate helps in other scenarios, such as when a precompiled library needs to be linked to a program written in Rust, but wasn't compiled either with integer normalization or CFI enabled, and recompiling it isn't an option

We should simply not support that. If clang wants to make arbitrary choices about how CFI works without considering cross-language compatibility, then we should simply refuse to be bullied by that, rather than treating it as dictating terms to us. We do not have the resources to play whack-a-mole games of chasing after language implementations that choose to implement recklessly inconsiderate semantics to interop.

[Jules-Bertholet]

@rustbot label A-sanitizers