cisco / libsrtp

Library for SRTP (Secure Realtime Transport Protocol)
Other
1.23k stars 476 forks source link

Convert to Rust #564

Open bifurcation opened 3 years ago

bifurcation commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 2 years ago

hello