WebAssembly / wasi-crypto

WASI Cryptography API Proposal
162 stars 25 forks source link

Remove Boring SSL #69

Closed rjzak closed 1 year ago

rjzak commented 2 years ago

Remove Boring SSL, only used in implementations/hostcalls/rust/src/signatures/rsa.rs.

Motivation:

In-Progress:

CC: @jedisct1

jedisct1 commented 2 years ago

That would be a dealbreaker for us.

Ignoring security concerns, the pure Rust RSA implementation is very slow. Way too slow for our requirements, and it doesn't even justify using hostcalls vs compiling to WebAssembly.

The RSA crate was originally used in that code, and got eventually replaced with BoringSSL for that reason.

Is C++ incompatible with static builds? That looks like a toolchain issue, a configuration issue, or something to fix in the boring crate rather than a permanent obstacle.

rjzak commented 2 years ago

Could we have a compromise where a feature flag could select RustCrypto over Boring? I'll take a closer look at Boring to see about the compilation issue. But Boring + Rust + musl results in trying to use musl-g++ (not resolved with sudo ln -s /bin/g++ /bin/musl-g++), and the compiler not finding several standard header files.

CC: @npmccallum

jedisct1 commented 2 years ago

The boring crate seems to always try to compile to the host architecture (compiling with cargo-zigbuild build --host=x86_64-unknown-linux-musl still makes it try to compile for ARM on my M1 Mac). But fixing this would make a lot of people happy :)

rjzak commented 2 years ago

Out of curiosity, what would you say was the performance boost by switching to boring?

tarcieri commented 2 years ago

I’m curious to know as well. We should open a tracking issue on the rsa crate about the performance difference

jedisct1 commented 2 years ago

Benchmark of JWT verification/signature:

Screen Shot 2022-07-26 at 18 39 38

(C@E module is pure WebAssembly)

npmccallum commented 2 years ago

@jedisct1 Did you build rsa in release mode with all the optimizations on?

jedisct1 commented 2 years ago

Of course, yes.

jedisct1 commented 2 years ago

boring already has some support for cross-compilation, as it can cross-compile for Android and iOS (see get_boringssl_cmake_config in the build.rs file).

npmccallum commented 2 years ago

@jedisct1 boring is a non-starter for Enarx given our hardware environment. Can we have both implementations behind a feature flag?

PiotrSikora commented 2 years ago

FYI, there is also bssl-sys crate (still under development?) by BoringSSL team. Perhaps that could work?

npmccallum commented 2 years ago

@PiotrSikora We will not use boring.

PiotrSikora commented 2 years ago

@npmccallum the original comment complains about C++ (which last time I checked, was optional in BoringSSL, and perhaps it being required is an issue with boring crate). I didn't use bssl-sys, but it looks like it's using bindgen, so it should be pure C. Is your issue with using BoringSSL in general or with the C++ bits?

jedisct1 commented 2 years ago

It shouldn't be too hard to support both. So, yes, it looks like a good path forward.

npmccallum commented 2 years ago

@PiotrSikora We have historically tried boring, openssl, and mbed. Trying to use them in our context causes a lot of problems (not just C/C++). We gave up after months of pain and now use RustCrypto for everything.

messense commented 2 years ago

I'm working on https://github.com/rust-lang/cmake-rs/pull/158 to improve cross compiling support in cmake-rs, hopefully it will make cross compiling boringssl easier.

npmccallum commented 2 years ago

The problem as I see it is that wasi-crypto is mixing crypto implementations. This requires everyone to tries to use it to adopt multiple crypto systems. Unfortunately, wasi-crypto has chosen RustCrypto for everything except RSA; but then adopts a completely different crypto system for RSA.

We need to decide on one crypto system for the reference implementation. If others want a boring-backed implementation of wasi-crypto then they should build one. But the reference implementation should not force people to adopt multiple crypto systems. Further, as a reference implementation, wasi-crypto should not chose its crypto system based on criteria like performance. Rather it should choose the crypto system which is the easiest for all parties to get working (that's the purpose of a reference implementation).

Given these criteria, there are only two choices for a Rust-language reference implementation: ring or RustCrypto. Both are pretty trivial to add as dependencies and will build in every situation where Rust builds native code. An argument could be made for both of them. But we should pick one and use that for the reference implementation.