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

Convert to Rust #564

Open
bifurcation opened this issue Sep 15, 2021 · 3 comments
Open

Convert to Rust #564

bifurcation opened this issue Sep 15, 2021 · 3 comments

Comments

@bifurcation
Copy link
Contributor

We should convert libsrtp to Rust, in the interest of better memory safety.

We should be able to do this while maintaining the same C interface, so that consumers of binary library will simply have to re-link. I have done enough work in a branch to validate that this is possible; the re-implementation there links against the existing C tests and passes them. Consumers that rebuild the library (e.g., for packaging) will need to have a Rust toolchain.

This is obviously a large change with a risk of regressions (e.g., w.r.t. performance), so I would propose a multi-stage process to gradually develop and integrate Rust code:

  1. Replace the tests in crypto/test and test with equivalents in Rust (see this branch for a prototype). This will ensure that we have Rust support in the CI toolchain, and facilitate testing against Rust library code later. And it has minimal impact on consumers.
  2. Add a parallel Rust implementation in several stages, following the modularity boundaries in the existing code
    • Modules in roughly dependency / ease-of-implementation order:
      • Key limits
      • Replay DB
      • Crypto algorithms
      • Crypto kernel
      • Packet parsing
      • SRTP protocol logic
    • At each stage:
      • Re-implement the module in Rust and add a C FFI interface for it
      • Have a switch in the build system that replaces the C modules with the corresponding Rust modules
      • Use the test suite to verify the correctness and performance of the new module
  3. Maintain parallel C and Rust implementations for some time while we build confidence in the Rust implementation. Any PRs in this period should maintain parity.
  4. Once the Rust implementation has had some deployment experience, remove the C implementation. Keep enough scaffolding to expose the C FFI interface.

(We may be able to do (4) at a per-module level, e.g., removing the C replay DB once the Rust one is validated and landed.)

If folks are in general agreement with this objective and high-level plan, we can then make some more precise issues/PRs.

@fluffy
Copy link
Member

fluffy commented Sep 15, 2021

This API is not awesome from point of view of what we would do today. We will end up with people that want to have the C version due to support issues. I wonder if it would be better just to make a separate Rust version perhaps with an mapping layer to allow people to switch to the rust version with no API changes if they use the mapping layer.

@bifurcation
Copy link
Contributor Author

Re: API - Yeah, I was definitely thinking that we could make a Rust API that is nicer, as long as there's a way to map most of the C API semantics to it. The rs branch already does some of this (e.g., getting rid of globals), but it could be cleaned up further.

Re: Keeping C around long-term - I guess removing the C code (4) might be a little aspirational. But it seems like there are ways we could get there incrementally. For example, we could switch the default from building C code to building Rust code. As long as both are supported under the same C API, though, we should have them both in the same repo, so that for example they are covered by the same test suite. To be clear, the project here is different from a clean Rust re-implementation — the goal is to replace the C implementation in shipping binaries, so compatibility is important as well as safety.

@mike19791022
Copy link

hello

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants