Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
6 tasks done
rcvalle opened this issue Oct 7, 2021 · 33 comments
Open
6 tasks done

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

rcvalle opened this issue Oct 7, 2021 · 33 comments
Assignees
Labels
A-sanitizers Area: Sanitizers for correctness and code quality C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC PG-exploit-mitigations Project group: Exploit mitigations

Comments

@rcvalle
Copy link
Member

rcvalle commented Oct 7, 2021

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:

Unresolved Questions

  • Should integers always be normalized?

Implementation history

  1. Add LLVM CFI support to the Rust compiler #89652
  2. Add documentation for LLVM CFI support rustc-dev-guide#1234
  3. Add fine-grained LLVM CFI support to the Rust compiler #95548
  4. RFC: Improve C types for cross-language LLVM CFI support rfcs#3296
  5. octicon-git-merge-merged [LLVM] Add CFI integer types normalization to Clang compiler
  6. Add cross-language LLVM CFI support to the Rust compiler #105452

Bonus implementation

  1. Add LLVM KCFI support to the Rust compiler #105109
  2. Add documentation for LLVM KCFI support rustc-dev-guide#1529

Known issues

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

@rcvalle rcvalle added the C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC label Oct 7, 2021
@rcvalle
Copy link
Member Author

rcvalle commented Oct 7, 2021

r? @eddyb

rcvalle added a commit to rcvalle/rustc-dev-guide that referenced this issue Oct 14, 2021
This commit adds initial documentation for LLVM Control Flow Integrity
(CFI) support to the Rust compiler (see rust-lang/rust#89652 and
rust-lang/rust#89653).
rcvalle added a commit to rcvalle/rust that referenced this issue Oct 25, 2021
This commit adds LLVM Control Flow Integrity (CFI) support to the Rust
compiler. It initially provides forward-edge control flow protection for
Rust-compiled code only by aggregating function pointers in groups
identified by their number of arguments.

Forward-edge control flow protection for C or C++ and Rust -compiled
code "mixed binaries" (i.e., for when C or C++ and Rust -compiled code
share the same virtual address space) will be provided in later work as
part of this project by defining and using compatible type identifiers
(see Type metadata in the design document in the tracking issue rust-lang#89653).

LLVM CFI can be enabled with -Zsanitizer=cfi and requires LTO (i.e.,
-Clto).
@RalfJung
Copy link
Member

RalfJung commented Jul 26, 2023 via email

@rcvalle
Copy link
Member Author

rcvalle commented Jul 26, 2023

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.)

@Noratrieb
Copy link
Member

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
Copy link
Member

RalfJung commented Dec 10, 2023

@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
Copy link
Member

workingjubilee commented Dec 10, 2023

@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
Copy link
Member

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

@rcvalle
Copy link
Member Author

rcvalle commented Dec 13, 2023

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
Copy link
Member

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
Copy link
Member Author

rcvalle commented Dec 13, 2023

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
Copy link
Member

They are different types but ABI-compatible.

@rcvalle
Copy link
Member Author

rcvalle commented Dec 13, 2023

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
Copy link
Member

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
Copy link
Member Author

rcvalle commented Dec 13, 2023

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
Copy link
Member

RalfJung commented Dec 13, 2023

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
Copy link
Member Author

rcvalle commented Dec 13, 2023

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.

@Noratrieb
Copy link
Member

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
Copy link
Member Author

rcvalle commented Dec 13, 2023

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
Copy link
Member

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.

@Noratrieb
Copy link
Member

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
Copy link
Member Author

rcvalle commented Dec 13, 2023

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

@workingjubilee
Copy link
Member

workingjubilee commented Dec 13, 2023

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
Copy link
Contributor

@rustbot label A-sanitizers

@rustbot rustbot added the A-sanitizers Area: Sanitizers for correctness and code quality label Apr 8, 2024
veselypeta pushed a commit to veselypeta/cherillvm that referenced this issue Aug 8, 2024
This commit adds a new option (i.e.,
`-fsanitize-cfi-icall-normalize-integers`) for normalizing integer types
as vendor extended types for cross-language LLVM CFI/KCFI support with
other languages that can't represent and encode C/C++ integer types.

Specifically, integer types are encoded as their defined representations
(e.g., 8-bit signed integer, 16-bit signed integer, 32-bit signed
integer, ...) for compatibility with languages that define
explicitly-sized integer types (e.g., i8, i16, i32, ..., in Rust).

``-fsanitize-cfi-icall-normalize-integers`` is compatible with
``-fsanitize-cfi-icall-generalize-pointers``.

This helps with providing cross-language CFI support with the Rust
compiler and is an alternative solution for the issue described and
alternatives proposed in the RFC
rust-lang/rfcs#3296.

For more information about LLVM CFI/KCFI and cross-language LLVM
CFI/KCFI support for the Rust compiler, see the design document in the
tracking issue rust-lang/rust#89653.

Reviewed By: pcc, samitolvanen

Differential Revision: https://reviews.llvm.org/D139395
veselypeta pushed a commit to veselypeta/cherillvm that referenced this issue Aug 9, 2024
This commit adds a new option (i.e.,
`-fsanitize-cfi-icall-normalize-integers`) for normalizing integer types
as vendor extended types for cross-language LLVM CFI/KCFI support with
other languages that can't represent and encode C/C++ integer types.

Specifically, integer types are encoded as their defined representations
(e.g., 8-bit signed integer, 16-bit signed integer, 32-bit signed
integer, ...) for compatibility with languages that define
explicitly-sized integer types (e.g., i8, i16, i32, ..., in Rust).

``-fsanitize-cfi-icall-normalize-integers`` is compatible with
``-fsanitize-cfi-icall-generalize-pointers``.

This helps with providing cross-language CFI support with the Rust
compiler and is an alternative solution for the issue described and
alternatives proposed in the RFC
rust-lang/rfcs#3296.

For more information about LLVM CFI/KCFI and cross-language LLVM
CFI/KCFI support for the Rust compiler, see the design document in the
tracking issue rust-lang/rust#89653.

Relands b1e9ab7 with fixes.

Reviewed By: pcc, samitolvanen

Differential Revision: https://reviews.llvm.org/D139395
veselypeta pushed a commit to veselypeta/cherillvm that referenced this issue Aug 14, 2024
This commit adds a new option (i.e.,
`-fsanitize-cfi-icall-normalize-integers`) for normalizing integer types
as vendor extended types for cross-language LLVM CFI/KCFI support with
other languages that can't represent and encode C/C++ integer types.

Specifically, integer types are encoded as their defined representations
(e.g., 8-bit signed integer, 16-bit signed integer, 32-bit signed
integer, ...) for compatibility with languages that define
explicitly-sized integer types (e.g., i8, i16, i32, ..., in Rust).

``-fsanitize-cfi-icall-normalize-integers`` is compatible with
``-fsanitize-cfi-icall-generalize-pointers``.

This helps with providing cross-language CFI support with the Rust
compiler and is an alternative solution for the issue described and
alternatives proposed in the RFC
rust-lang/rfcs#3296.

For more information about LLVM CFI/KCFI and cross-language LLVM
CFI/KCFI support for the Rust compiler, see the design document in the
tracking issue rust-lang/rust#89653.

Reviewed By: pcc, samitolvanen

Differential Revision: https://reviews.llvm.org/D139395
veselypeta pushed a commit to veselypeta/cherillvm that referenced this issue Aug 15, 2024
This commit adds a new option (i.e.,
`-fsanitize-cfi-icall-normalize-integers`) for normalizing integer types
as vendor extended types for cross-language LLVM CFI/KCFI support with
other languages that can't represent and encode C/C++ integer types.

Specifically, integer types are encoded as their defined representations
(e.g., 8-bit signed integer, 16-bit signed integer, 32-bit signed
integer, ...) for compatibility with languages that define
explicitly-sized integer types (e.g., i8, i16, i32, ..., in Rust).

``-fsanitize-cfi-icall-normalize-integers`` is compatible with
``-fsanitize-cfi-icall-generalize-pointers``.

This helps with providing cross-language CFI support with the Rust
compiler and is an alternative solution for the issue described and
alternatives proposed in the RFC
rust-lang/rfcs#3296.

For more information about LLVM CFI/KCFI and cross-language LLVM
CFI/KCFI support for the Rust compiler, see the design document in the
tracking issue rust-lang/rust#89653.

Relands b1e9ab7 with fixes.

Reviewed By: pcc, samitolvanen

Differential Revision: https://reviews.llvm.org/D139395
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-sanitizers Area: Sanitizers for correctness and code quality C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC PG-exploit-mitigations Project group: Exploit mitigations
Projects
None yet
Development

No branches or pull requests

6 participants